diff --git a/lib/middleware/omniauth_bypass_middleware.rb b/lib/middleware/omniauth_bypass_middleware.rb index b8c65caac88..c9b6b93d565 100644 --- a/lib/middleware/omniauth_bypass_middleware.rb +++ b/lib/middleware/omniauth_bypass_middleware.rb @@ -13,27 +13,12 @@ class Middleware::OmniauthBypassMiddleware Discourse.plugins.each(&:notify_before_auth) - # 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 - # OpenSSL::SSL::VERIFY_PEER = OpenSSL::SSL::VERIFY_NONE if Rails.env.development? - @omniauth = - OmniAuth::Builder.new(app) do - Discourse.authenticators.each { |authenticator| authenticator.register_middleware(self) } - end - - @omniauth.before_request_phase do |env| + OmniAuth.config.before_request_phase do |env| request = ActionDispatch::Request.new(env) # Check for CSRF token in POST requests CSRFTokenVerifier.new.call(env) if request.request_method.downcase.to_sym != :get - # Check whether the authenticator is enabled - if !Discourse.enabled_authenticators.any? { |a| - a.name.to_sym == env["omniauth.strategy"].name.to_sym - } - raise AuthenticatorDisabled - end - # If the user is trying to reconnect to an existing account, store in session request.session[:auth_reconnect] = !!request.params["reconnect"] @@ -50,7 +35,14 @@ class Middleware::OmniauthBypassMiddleware !SiteSetting.enable_local_logins && Discourse.enabled_authenticators.length == 1 OmniAuth.config.allowed_request_methods = only_one_provider ? %i[get post] : [:post] - @omniauth.call(env) + omniauth = + OmniAuth::Builder.new(@app) do + Discourse.enabled_authenticators.each do |authenticator| + authenticator.register_middleware(self) + end + end + + omniauth.call(env) rescue AuthenticatorDisabled => e # Authenticator is disabled, pretend it doesn't exist and pass request to app @app.call(env) diff --git a/spec/requests/omniauth_callbacks_controller_spec.rb b/spec/requests/omniauth_callbacks_controller_spec.rb index c051ef6165c..576158f120e 100644 --- a/spec/requests/omniauth_callbacks_controller_spec.rb +++ b/spec/requests/omniauth_callbacks_controller_spec.rb @@ -1102,4 +1102,50 @@ RSpec.describe Users::OmniauthCallbacksController do Rails.application.env_config["omniauth.auth"] = OmniAuth.config.mock_auth[:google_oauth2] end end + + describe "with a fake provider" do + class FakeAuthenticator < Auth::ManagedAuthenticator + class Strategy + include OmniAuth::Strategy + def other_phase + [418, {}, "I am a teapot"] + end + end + + def name + "fake" + end + + def enabled? + false + end + + def register_middleware(omniauth) + omniauth.provider Strategy, name: :fake + end + end + + before do + DiscoursePluginRegistry.register_auth_provider( + Auth::AuthProvider.new(authenticator: FakeAuthenticator.new), + ) + OmniAuth.config.test_mode = false + end + + it "does not run 'other_phase' for disabled auth methods" do + get "/auth/fake/blah" + expect(response.status).to eq(404) + end + + it "does not leak 'other_phase' for disabled auth methods onto other methods" do + get "/auth/twitter/blah" + expect(response.status).to eq(404) + end + + it "runs 'other_phase' for enabled auth methods" do + FakeAuthenticator.any_instance.stubs(:enabled?).returns(true) + get "/auth/fake/blah" + expect(response.status).to eq(418) + end + end end