Merge branch '341104-fix-ldap-login' into 'master'

Fix LDAP sign-in when user cap set

See merge request gitlab-org/gitlab!75555

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

@@ -9,12 +9,26 @@ module EE
     module Auth
       module Ldap
         module User
+          extend ::Gitlab::Utils::Override
+
           def initialize(auth_hash)
             super
 
             set_external_with_external_groups
           end
 
+          override :find_user
+          def find_user
+            user = super
+
+            if activate_user_based_on_user_cap?(user)
+              user.activate
+              log_user_changes(user, 'LDAP', "user cap not reached yet, unblocking")
+            end
+
+            user
+          end
+
           private
 
           # Intended to be called during #initialize, and #save should be called

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

@@ -14,6 +14,24 @@ module EE
               super
             end
           end
+
+          def activate_user_based_on_user_cap?(user)
+            return false unless user&.activate_based_on_user_cap?
+
+            begin
+              !::User.user_cap_reached?
+            rescue ActiveRecord::QueryAborted => e
+              ::Gitlab::ErrorTracking.track_exception(e, user_email: user.email)
+              false
+            end
+          end
+
+          def log_user_changes(user, protocol, message)
+            ::Gitlab::AppLogger.info(
+              "#{protocol}(#{auth_hash.provider}) account \"#{auth_hash.uid}\" #{message} " \
+              "GitLab user \"#{user.name}\" (#{user.email})"
+            )
+          end
         end
       end
     end

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

@@ -32,32 +32,14 @@ module EE
 
           protected
 
-          def activate_user_based_on_user_cap?(user)
-            return false unless user&.activate_based_on_user_cap?
-
-            begin
-              !::User.user_cap_reached?
-            rescue ActiveRecord::QueryAborted => e
-              ::Gitlab::ErrorTracking.track_exception(e, saml_user_email: user.email)
-              false
-            end
-          end
-
           def block_user(user, reason)
             user.ldap_block
-            log_user_changes(user, "#{reason}, blocking")
+            log_user_changes(user, 'SAML', "#{reason}, blocking")
           end
 
           def unblock_user(user, reason)
             user.activate
-            log_user_changes(user, "#{reason}, unblocking")
-          end
-
-          def log_user_changes(user, message)
-            ::Gitlab::AppLogger.info(
-              "SAML(#{auth_hash.provider}) account \"#{auth_hash.uid}\" #{message} " \
-              "GitLab user \"#{user.name}\" (#{user.email})"
-            )
+            log_user_changes(user, 'SAML', "#{reason}, unblocking")
           end
 
           def user_in_required_group?

ee/spec/lib/gitlab/auth/ldap/user_spec.rb

@@ -145,4 +145,10 @@ RSpec.describe Gitlab::Auth::Ldap::User do
       end
     end
   end
+
+  describe '#find_user' do
+    it_behaves_like 'finding user when user cap is set' do
+      let(:o_auth_user) { ldap_user }
+    end
+  end
 end

ee/spec/lib/gitlab/auth/saml/user_spec.rb

@@ -145,65 +145,8 @@ RSpec.describe Gitlab::Auth::Saml::User do
           end
         end
 
-        context 'when a sign-up user cap has been set' do
-          before do
-            saml_user.gl_user.state = ::User::BLOCKED_PENDING_APPROVAL_STATE
-            stub_application_setting(new_user_signups_cap: new_user_signups_cap)
-          end
-
-          context 'when the user cap has been reached' do
-            let(:new_user_signups_cap) { 1 }
-
-            it 'does not activate the user' do
-              create(:user)
-
-              saml_user.save
-
-              expect(saml_user.find_user).to be_blocked
-            end
-          end
-
-          context 'when the user cap has not been reached' do
-            let(:new_user_signups_cap) { 100 }
-
-            before do
-              stub_omniauth_setting(block_auto_created_users: block)
-            end
-
-            context 'when the user can be activated based on user cap' do
-              let(:block) { false }
-
-              it 'activates the user' do
-                saml_user.save
-
-                expect(saml_user.find_user).to be_active
-              end
-
-              context 'when the query behind .user_cap_reached? times out' do
-                it 'does not activate the user' do
-                  allow(::User).to receive(:user_cap_reached?).and_raise(ActiveRecord::QueryAborted)
-
-                  saml_user.save
-
-                  expect(::Gitlab::ErrorTracking).to receive(:track_exception).with(
-                    instance_of(ActiveRecord::QueryAborted),
-                    saml_user_email: saml_user.gl_user.email
-                  )
-                  expect(saml_user.find_user).to be_blocked
-                end
-              end
-            end
-
-            context 'when the user cannot be activated based on user cap' do
-              let(:block) { true }
-
-              it 'does not activate the user' do
-                saml_user.save
-
-                expect(saml_user.find_user).to be_blocked
-              end
-            end
-          end
+        it_behaves_like 'finding user when user cap is set' do
+          let(:o_auth_user) { saml_user }
         end
       end
     end

ee/spec/support/shared_examples/auth/access_protocol_examples.rb

+# frozen_string_literal: true
+
+RSpec.shared_examples 'finding user when user cap is set' do
+  context 'when a sign-up user cap has been set' do
+    before do
+      gl_user.state = ::User::BLOCKED_PENDING_APPROVAL_STATE
+      stub_application_setting(new_user_signups_cap: new_user_signups_cap)
+    end
+
+    context 'when the user cap has been reached' do
+      let(:new_user_signups_cap) { 1 }
+
+      it 'does not activate the user' do
+        create(:user)
+
+        o_auth_user.save # rubocop:disable Rails/SaveBang
+
+        expect(o_auth_user.find_user).to be_blocked
+      end
+    end
+
+    context 'when the user cap has not been reached' do
+      let(:new_user_signups_cap) { 100 }
+
+      before do
+        stub_omniauth_setting(block_auto_created_users: block)
+      end
+
+      context 'when the user can be activated based on user cap' do
+        let(:block) { false }
+
+        it 'activates the user' do
+          o_auth_user.save # rubocop:disable Rails/SaveBang
+
+          expect(o_auth_user.find_user).to be_active
+        end
+
+        context 'when the query behind .user_cap_reached? times out' do
+          it 'does not activate the user' do
+            allow(::User).to receive(:user_cap_reached?).and_raise(ActiveRecord::QueryAborted)
+
+            o_auth_user.save # rubocop:disable Rails/SaveBang
+
+            expect(::Gitlab::ErrorTracking).to receive(:track_exception).with(
+              instance_of(ActiveRecord::QueryAborted),
+              user_email: o_auth_user.gl_user.email
+            )
+            expect(o_auth_user.find_user).to be_blocked
+          end
+        end
+      end
+
+      context 'when the user cannot be activated based on user cap' do
+        let(:block) { true }
+
+        it 'does not activate the user' do
+          o_auth_user.save # rubocop:disable Rails/SaveBang
+
+          expect(o_auth_user.find_user).to be_blocked
+        end
+      end
+    end
+  end
+end