DEV: Update to OmniAuth 2.0 (#25707)

Gemfile.lock

@@ -298,19 +298,20 @@ GEM
     oj (3.16.9)
       bigdecimal (>= 3.0)
       ostruct (>= 0.2)
-    omniauth (1.9.2)
+    omniauth (2.1.2)
       hashie (>= 3.4.6)
-      rack (>= 1.6.2, < 3)
+      rack (>= 2.2.3)
+      rack-protection
     omniauth-facebook (9.0.0)
       omniauth-oauth2 (~> 1.2)
-    omniauth-github (1.4.0)
+    omniauth-github (2.0.0)
-      omniauth (~> 1.5)
+      omniauth (~> 2.0)
-      omniauth-oauth2 (>= 1.4.0, < 2.0)
+      omniauth-oauth2 (~> 1.7.1)
-    omniauth-google-oauth2 (0.8.2)
+    omniauth-google-oauth2 (1.0.1)
       jwt (>= 2.0)
       oauth2 (~> 1.1)
-      omniauth (~> 1.1)
+      omniauth (~> 2.0)
-      omniauth-oauth2 (>= 1.6)
+      omniauth-oauth2 (~> 1.7.1)
     omniauth-oauth (1.2.1)
       oauth
       omniauth (>= 1.0, < 3)

config/initializers/009-omniauth.rb

@@ -1,8 +1,50 @@
 # frozen_string_literal: true
 
-require "openssl"
-
 require "middleware/omniauth_bypass_middleware"
 Rails.application.config.middleware.use Middleware::OmniauthBypassMiddleware
 
 OmniAuth.config.logger = Rails.logger
+OmniAuth.config.silence_get_warning = true
+
+OmniAuth.config.request_validation_phase = nil # We handle CSRF checks in before_request_phase
+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
+
+  # If the user is trying to reconnect to an existing account, store in session
+  request.session[:auth_reconnect] = !!request.params["reconnect"]
+
+  # If the client provided an origin, store in session to redirect back
+  request.session[:destination_url] = request.params["origin"]
+end
+
+OmniAuth.config.on_failure do |env|
+  exception = env["omniauth.error"]
+
+  # OmniAuth 2 doesn't give us any way to know for sure whether a failure was due to an
+  # explicit fail! call, or a rescued exception. But, this check is a pretty good guess:
+  is_rescued_error = exception&.message&.to_sym == env["omniauth.error.type"]
+
+  next OmniAuth::FailureEndpoint.call(env) if !is_rescued_error # let the default behavior handle it
+
+  case exception
+  when OAuth::Unauthorized
+    # OAuth1 (i.e. Twitter) makes a web request during the setup phase
+    # If it fails, Omniauth does not handle the error. Handle it here
+    env["omniauth.error.type"] = "request_error"
+  when JWT::InvalidIatError
+    # Happens for openid-connect (including google) providers, when the server clock is wrong
+    env["omniauth.error.type"] = "invalid_iat"
+  when CSRFTokenVerifier::InvalidCSRFToken
+    # Happens when CSRF token is missing from request
+    env["omniauth.error.type"] = "csrf_detected"
+  else
+    # default omniauth behavior is to redirect to /auth/failure with error.message in the URL
+    # We don't want to leak that kind of unhandled exception info, so re-raise it
+    raise exception
+  end
+
+  OmniAuth::FailureEndpoint.call(env)
+end

lib/middleware/omniauth_bypass_middleware.rb

@@ -1,70 +1,54 @@
 # frozen_string_literal: true
 
-require "csrf_token_verifier"
-
 # omniauth loves spending lots cycles in its magic middleware stack
 # this middleware bypasses omniauth middleware and only hits it when needed
 class Middleware::OmniauthBypassMiddleware
-  class AuthenticatorDisabled < StandardError
+  module OmniAuthStrategyCompatPatch
+    def callback_url
+      result = super
+      if script_name.present? && result.include?("#{script_name}#{script_name}")
+        result = result.gsub("#{script_name}#{script_name}", script_name)
+        Discourse.deprecate <<~MESSAGE
+          OmniAuth strategy '#{name}' included duplicate script_name in callback url. It's likely the callback_url method is concatenating `script_name` with `callback_path`.
+          OmniAuth v2 includes the `script_name` in the `callback_path` automatically, so the manual `script_name` call can be removed.
+          This issue has been automatically corrected, but the strategy should be updated to ensure subfolder compatibility with future versions of Discourse.
+        MESSAGE
+      end
+      result
+    end
+  end
+
+  class PatchedOmniAuthBuilder < OmniAuth::Builder
+    def use(strategy, *args, **kwargs, &block)
+      if !strategy.ancestors.include?(OmniAuthStrategyCompatPatch)
+        strategy.prepend(OmniAuthStrategyCompatPatch)
+      end
+      super(strategy, *args, **kwargs, &block)
+    end
   end
 
   def initialize(app, options = {})
     @app = app
-
-    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
-
-      # If the user is trying to reconnect to an existing account, store in session
-      request.session[:auth_reconnect] = !!request.params["reconnect"]
-
-      # If the client provided an origin, store in session to redirect back
-      request.session[:destination_url] = request.params["origin"]
-    end
   end
 
   def call(env)
-    if env["PATH_INFO"].start_with?("/auth")
+    return @app.call(env) unless env["PATH_INFO"].start_with?("/auth")
-      begin
-        # When only one provider is enabled, assume it can be completely trusted, and allow GET requests
-        only_one_provider =
-          !SiteSetting.enable_local_logins && Discourse.enabled_authenticators.length == 1
 
-        allow_get = only_one_provider || !SiteSetting.auth_require_interaction
+    # When only one provider is enabled, assume it can be completely trusted, and allow GET requests
+    only_one_provider =
+      !SiteSetting.enable_local_logins && Discourse.enabled_authenticators.length == 1
 
-        OmniAuth.config.allowed_request_methods = allow_get ? %i[get post] : [:post]
+    allow_get = only_one_provider || !SiteSetting.auth_require_interaction
 
-        omniauth =
+    OmniAuth.config.allowed_request_methods = allow_get ? %i[get post] : [:post]
-          OmniAuth::Builder.new(@app) do
-            Discourse.enabled_authenticators.each do |authenticator|
-              authenticator.register_middleware(self)
-            end
-          end
 
-        omniauth.call(env)
+    omniauth =
-      rescue AuthenticatorDisabled => e
+      PatchedOmniAuthBuilder.new(@app) do
-        #  Authenticator is disabled, pretend it doesn't exist and pass request to app
+        Discourse.enabled_authenticators.each do |authenticator|
-        @app.call(env)
+          authenticator.register_middleware(self)
-      rescue OAuth::Unauthorized => e
+        end
-        # OAuth1 (i.e. Twitter) makes a web request during the setup phase
-        # If it fails, Omniauth does not handle the error. Handle it here
-        env["omniauth.error.type"] ||= "request_error"
-        Rails.logger.error "Authentication failure! request_error: #{e.class}, #{e.message}"
-        OmniAuth::FailureEndpoint.call(env)
-      rescue JWT::InvalidIatError => e
-        # Happens for openid-connect (including google) providers, when the server clock is wrong
-        env["omniauth.error.type"] ||= "invalid_iat"
-        Rails.logger.error "Authentication failure! invalid_iat: #{e.class}, #{e.message}"
-        OmniAuth::FailureEndpoint.call(env)
-      rescue CSRFTokenVerifier::InvalidCSRFToken => e
-        # Happens when CSRF token is missing from request
-        env["omniauth.error.type"] ||= "csrf_detected"
-        OmniAuth::FailureEndpoint.call(env)
       end
-    else
+
-      @app.call(env)
+    omniauth.call(env)
-    end
   end
 end