Merge branch '27241-update-omniauth-oauth2' into 'master'

Update omniauth-oauth2 and remove error verification monkey-patch

See merge request gitlab-org/gitlab!73658

Gemfile.lock

@@ -863,7 +863,7 @@ GEM
     omniauth-oauth (1.1.0)
       oauth
       omniauth (~> 1.0)
-    omniauth-oauth2 (1.7.1)
+    omniauth-oauth2 (1.7.2)
       oauth2 (~> 1.4)
       omniauth (>= 1.9, < 3)
     omniauth-oauth2-generic (0.2.2)

config/initializers_before_autoloader/100_patch_omniauth_oauth2.rb

@@ -10,31 +10,11 @@
 module OmniAuth
   module Strategies
     class OAuth2
+      alias_method :original_callback_phase, :callback_phase
+
       def callback_phase
-        error = request.params["error_reason"].presence || request.params["error"].presence
-        # Monkey patch #1:
-        #
-        # Swap the order of these conditions around so the `state` param is verified *first*,
-        # before using the error params returned by the provider.
-        #
-        # This avoids content spoofing attacks by crafting a URL with malicious messages,
-        # because the `state` param is only present in the session after a valid OAuth2 authentication flow.
-        if !options.provider_ignores_state && (request.params["state"].to_s.empty? || request.params["state"] != session.delete("omniauth.state"))
-          fail!(:csrf_detected, CallbackError.new(:csrf_detected, "CSRF detected"))
-        elsif error
-          fail!(error, CallbackError.new(request.params["error"], request.params["error_description"].presence || request.params["error_reason"].presence, request.params["error_uri"]))
-        else
-          self.access_token = build_access_token
-          self.access_token = access_token.refresh! if access_token.expired?
-          super
-        end
-      rescue ::OAuth2::Error, CallbackError => e
-        fail!(:invalid_credentials, e)
-      rescue ::Timeout::Error, ::Errno::ETIMEDOUT => e
-        fail!(:timeout, e)
-      rescue ::SocketError => e
-        fail!(:failed_to_connect, e)
-      # Monkey patch #2:
+        original_callback_phase
+      # Monkey patch #1:
       #
       # Also catch errors from Faraday.
       # See https://github.com/omniauth/omniauth-oauth2/pull/129

spec/initializers/100_patch_omniauth_oauth2_spec.rb

@@ -2,12 +2,10 @@
 
 require 'spec_helper'
 
-RSpec.describe 'OmniAuth::Strategies::OAuth2', type: :strategy do
-  let(:strategy) { [OmniAuth::Strategies::OAuth2] }
-
+RSpec.describe 'OmniAuth::Strategies::OAuth2' do
   it 'verifies the gem version' do
     current_version = OmniAuth::OAuth2::VERSION
-    expected_version = '1.7.1'
+    expected_version = '1.7.2'
 
     expect(current_version).to eq(expected_version), <<~EOF
       New version #{current_version} of the `omniauth-oauth2` gem detected!
@@ -18,39 +16,18 @@ RSpec.describe 'OmniAuth::Strategies::OAuth2', type: :strategy do
     EOF
   end
 
-  context 'when a custom error message is passed from an OAuth2 provider' do
-    let(:message) { 'Please go to https://evil.com' }
-    let(:state) { 'secret' }
-    let(:callback_path) { '/users/auth/oauth2/callback' }
-    let(:params) { { state: state, error: 'evil_key', error_description: message } }
-    let(:error) { last_request.env['omniauth.error'] }
-
-    before do
-      env('rack.session', { 'omniauth.state' => state })
-    end
-
-    it 'returns the custom error message if the state is valid' do
-      get callback_path, **params
-
-      expect(error.message).to eq("evil_key | #{message}")
-    end
+  context 'when a Faraday exception is raised' do
+    where(exception: [Faraday::TimeoutError, Faraday::ConnectionFailed])
 
-    it 'returns the custom `error_reason` message if the `error_description` is blank' do
-      get callback_path, **params.merge(error_description: ' ', error_reason: 'custom reason')
-
-      expect(error.message).to eq('evil_key | custom reason')
-    end
-
-    it 'returns a CSRF error if the state is invalid' do
-      get callback_path, **params.merge(state: 'invalid')
-
-      expect(error.message).to eq('csrf_detected | CSRF detected')
-    end
+    with_them do
+      it 'passes the exception to OmniAuth' do
+        instance = OmniAuth::Strategies::OAuth2.new(double)
 
-    it 'returns a CSRF error if the state is missing' do
-      get callback_path, **params.without(:state)
+        expect(instance).to receive(:original_callback_phase) { raise exception, 'message' }
+        expect(instance).to receive(:fail!).with(:timeout, kind_of(exception))
 
-      expect(error.message).to eq('csrf_detected | CSRF detected')
+        instance.callback_phase
+      end
     end
   end
 end