Merge branch '194189_omniauth_redirect_loop' into 'master'

Prevent Omniauth signup redirect loop

See merge request gitlab-org/gitlab!22432

app/controllers/omniauth_callbacks_controller.rb

@@ -177,7 +177,7 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController
       message << _("Create a GitLab account first, and then connect it to your %{label} account.") % { label: label }
     end
 
-    flash[:notice] = message.join(' ')
+    flash[:alert] = message.join(' ')
     redirect_to new_user_session_path
   end
 

changelogs/unreleased/omniauth-redirect-loop.yml

+---
+title: "Prevent omniauth signup redirect loop"
+merge_request: 22432
+author: Balazs Nagy
+type: fixed

ee/lib/ee/gitlab/auth/saml/user.rb

@@ -12,8 +12,8 @@ module EE
             user = super
 
             if user_in_required_group?
-              unblock_user(user, "in required group") if user.persisted? && user.ldap_blocked?
-            elsif user.persisted?
+              unblock_user(user, "in required group") if user&.persisted? && user&.ldap_blocked?
+            elsif user&.persisted?
               block_user(user, "not in required group") unless user.blocked?
             else
               user = nil

spec/controllers/omniauth_callbacks_controller_spec.rb

@@ -287,6 +287,34 @@ describe OmniauthCallbacksController, type: :controller, do_not_mock_admin_mode:
       request.env['omniauth.auth'] = Rails.application.env_config['omniauth.auth']
     end
 
+    context 'sign up' do
+      before do
+        user.destroy
+      end
+
+      it 'denies login if sign up is enabled, but block_auto_created_users is set' do
+        post :saml, params: { SAMLResponse: mock_saml_response }
+
+        expect(flash[:alert]).to start_with 'Your account has been blocked.'
+      end
+
+      it 'accepts login if sign up is enabled' do
+        stub_omniauth_setting(block_auto_created_users: false)
+
+        post :saml, params: { SAMLResponse: mock_saml_response }
+
+        expect(request.env['warden']).to be_authenticated
+      end
+
+      it 'denies login if sign up is not enabled' do
+        stub_omniauth_setting(allow_single_sign_on: false, block_auto_created_users: false)
+
+        post :saml, params: { SAMLResponse: mock_saml_response }
+
+        expect(flash[:alert]).to start_with 'Signing in using your saml account without a pre-existing GitLab account is not allowed.'
+      end
+    end
+
     context 'with GitLab initiated request' do
       before do
         post :saml, params: { SAMLResponse: mock_saml_response }