Writes specs43525-limit-number-of-failed-logins-using-ldap

3 files changed, 88 insertions, 45 deletions

diff --git a/app/controllers/omniauth_callbacks_controller.rb b/app/controllers/omniauth_callbacks_controller.rb
index fed8ba3ce28..5e6676ea513 100644
--- a/app/controllers/omniauth_callbacks_controller.rb
+++ b/app/controllers/omniauth_callbacks_controller.rb
@@ -21,9 +21,11 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController
   # Extend the standard implementation to also increment
   # the number of failed sign in attempts
   def failure
-    user = User.find_by_username(params[:username])
+    if params[:username].present? && AuthHelper.form_based_provider?(failed_strategy.name)
+      user = User.by_login(params[:username])
 
-    user&.increment_failed_attempts!
+      user&.increment_failed_attempts!
+    end
 
     super
   end
diff --git a/changelogs/unreleased/43525-limit-number-of-failed-logins-using-ldap.yml b/changelogs/unreleased/43525-limit-number-of-failed-logins-using-ldap.yml
new file mode 100644
index 00000000000..f30fea3c4a7
--- /dev/null
+++ b/changelogs/unreleased/43525-limit-number-of-failed-logins-using-ldap.yml
@@ -0,0 +1,5 @@
+---
+title: Limit the number of failed logins when using LDAP for authentication
+merge_request: 43525
+author:
+type: added
diff --git a/spec/controllers/omniauth_callbacks_controller_spec.rb b/spec/controllers/omniauth_callbacks_controller_spec.rb
index 9fd129e4ee9..5f0e8c5eca9 100644
--- a/spec/controllers/omniauth_callbacks_controller_spec.rb
+++ b/spec/controllers/omniauth_callbacks_controller_spec.rb
@@ -10,83 +10,119 @@ describe OmniauthCallbacksController do
     stub_omniauth_provider(provider, context: request)
   end
 
-  context 'github' do
+  context 'when the user is on the last sign in attempt' do
     let(:extern_uid) { 'my-uid' }
-    let(:provider) { :github }
 
-    it 'allows sign in' do
-      post provider
-
-      expect(request.env['warden']).to be_authenticated
+    before do
+      user.update(failed_attempts: User.maximum_attempts.pred)
+      subject.response = ActionDispatch::Response.new
     end
 
-    shared_context 'sign_up' do
-      let(:user) { double(email: 'new@example.com') }
+    context 'when using a form based provider' do
+      let(:provider) { :ldap }
+
+      it 'locks the user when sign in fails' do
+        allow(subject).to receive(:params).and_return(ActionController::Parameters.new(username: user.username))
+        request.env['omniauth.error.strategy'] = OmniAuth::Strategies::LDAP.new(nil)
+
+        subject.send(:failure)
 
-      before do
-        stub_omniauth_setting(block_auto_created_users: false)
+        expect(user.reload).to be_access_locked
       end
     end
 
-    context 'sign up' do
-      include_context 'sign_up'
+    context 'when using a button based provider' do
+      let(:provider) { :github }
 
-      it 'is allowed' do
-        post provider
+      it 'does not lock the user when sign in fails' do
+        request.env['omniauth.error.strategy'] = OmniAuth::Strategies::GitHub.new(nil)
 
-        expect(request.env['warden']).to be_authenticated
+        subject.send(:failure)
+
+        expect(user.reload).not_to be_access_locked
       end
     end
+  end
 
-    context 'when OAuth is disabled' do
-      before do
-        stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'false')
-        settings = Gitlab::CurrentSettings.current_application_settings
-        settings.update(disabled_oauth_sign_in_sources: [provider.to_s])
-      end
+  context 'strategies' do
+    context 'github' do
+      let(:extern_uid) { 'my-uid' }
+      let(:provider) { :github }
 
-      it 'prevents login via POST' do
+      it 'allows sign in' do
         post provider
 
-        expect(request.env['warden']).not_to be_authenticated
+        expect(request.env['warden']).to be_authenticated
       end
 
-      it 'shows warning when attempting login' do
-        post provider
+      shared_context 'sign_up' do
+        let(:user) { double(email: 'new@example.com') }
 
-        expect(response).to redirect_to new_user_session_path
-        expect(flash[:alert]).to eq('Signing in using GitHub has been disabled')
+        before do
+          stub_omniauth_setting(block_auto_created_users: false)
+        end
       end
 
-      it 'allows linking the disabled provider' do
-        user.identities.destroy_all
-        sign_in(user)
+      context 'sign up' do
+        include_context 'sign_up'
+
+        it 'is allowed' do
+          post provider
 
-        expect { post provider }.to change { user.reload.identities.count }.by(1)
+          expect(request.env['warden']).to be_authenticated
+        end
       end
 
-      context 'sign up' do
-        include_context 'sign_up'
+      context 'when OAuth is disabled' do
+        before do
+          stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'false')
+          settings = Gitlab::CurrentSettings.current_application_settings
+          settings.update(disabled_oauth_sign_in_sources: [provider.to_s])
+        end
 
-        it 'is prevented' do
+        it 'prevents login via POST' do
           post provider
 
           expect(request.env['warden']).not_to be_authenticated
         end
+
+        it 'shows warning when attempting login' do
+          post provider
+
+          expect(response).to redirect_to new_user_session_path
+          expect(flash[:alert]).to eq('Signing in using GitHub has been disabled')
+        end
+
+        it 'allows linking the disabled provider' do
+          user.identities.destroy_all
+          sign_in(user)
+
+          expect { post provider }.to change { user.reload.identities.count }.by(1)
+        end
+
+        context 'sign up' do
+          include_context 'sign_up'
+
+          it 'is prevented' do
+            post provider
+
+            expect(request.env['warden']).not_to be_authenticated
+          end
+        end
       end
     end
-  end
 
-  context 'auth0' do
-    let(:extern_uid) { '' }
-    let(:provider) { :auth0 }
+    context 'auth0' do
+      let(:extern_uid) { '' }
+      let(:provider) { :auth0 }
 
-    it 'does not allow sign in without extern_uid' do
-      post 'auth0'
+      it 'does not allow sign in without extern_uid' do
+        post 'auth0'
 
-      expect(request.env['warden']).not_to be_authenticated
-      expect(response.status).to eq(302)
-      expect(controller).to set_flash[:alert].to('Wrong extern UID provided. Make sure Auth0 is configured correctly.')
+        expect(request.env['warden']).not_to be_authenticated
+        expect(response.status).to eq(302)
+        expect(controller).to set_flash[:alert].to('Wrong extern UID provided. Make sure Auth0 is configured correctly.')
+      end
     end
   end
 end