Restrict failed login attempts for users with 2FArestrict-failed-2fa-attempts

Copy logic from `Devise::Models::Lockable#valid_for_authentication?`, as
our custom login flow with two pages doesn't call this method. This will
increment the failed login counter, and lock the user's account once
they exceed the number of failed attempts.

Also ensure that users who are locked can't continue to submit 2FA
codes.

4 files changed, 69 insertions, 3 deletions

diff --git a/CHANGELOG b/CHANGELOG
index bfc312ca19c..aeaa1312b35 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -13,6 +13,7 @@ v 8.13.0 (unreleased)
   - Avoid database queries on Banzai::ReferenceParser::BaseParser for nodes without references
   - Fix permission for setting an issue's due date
   - Expose expires_at field when sharing project on API
+  - Restrict failed login attempts for users with 2FA enabled
   - Fix issue with page scrolling to top when closing or pinning sidebar (lukehowell)
   - Allow the Koding integration to be configured through the API
   - Added soft wrap button to repository file/blob editor
@@ -42,7 +43,7 @@ v 8.13.0 (unreleased)
   - Notify the Merger about merge after successful build (Dimitris Karakasilis)
   - Fix broken repository 500 errors in project list
   - Close todos when accepting merge requests via the API !6486 (tonygambone)
-  - Changed Slack service user referencing from full name to username (Sebastian Poxhofer) 
+  - Changed Slack service user referencing from full name to username (Sebastian Poxhofer)
   - Add Container Registry on/off status to Admin Area !6638 (the-undefined)
 
 v 8.12.4 (unreleased)
diff --git a/app/controllers/concerns/authenticates_with_two_factor.rb b/app/controllers/concerns/authenticates_with_two_factor.rb
index d5a8a962662..4c497711fc0 100644
--- a/app/controllers/concerns/authenticates_with_two_factor.rb
+++ b/app/controllers/concerns/authenticates_with_two_factor.rb
@@ -23,15 +23,24 @@ module AuthenticatesWithTwoFactor
   #
   # Returns nil
   def prompt_for_two_factor(user)
+    return locked_user_redirect(user) if user.access_locked?
+
     session[:otp_user_id] = user.id
     setup_u2f_authentication(user)
     render 'devise/sessions/two_factor'
   end
 
+  def locked_user_redirect(user)
+    flash.now[:alert] = 'Invalid Login or password'
+    render 'devise/sessions/new'
+  end
+
   def authenticate_with_two_factor
     user = self.resource = find_user
 
-    if user_params[:otp_attempt].present? && session[:otp_user_id]
+    if user.access_locked?
+      locked_user_redirect(user)
+    elsif user_params[:otp_attempt].present? && session[:otp_user_id]
       authenticate_with_two_factor_via_otp(user)
     elsif user_params[:device_response].present? && session[:otp_user_id]
       authenticate_with_two_factor_via_u2f(user)
@@ -50,8 +59,9 @@ module AuthenticatesWithTwoFactor
       remember_me(user) if user_params[:remember_me] == '1'
       sign_in(user)
     else
+      user.increment_failed_attempts!
       flash.now[:alert] = 'Invalid two-factor code.'
-      render :two_factor
+      prompt_for_two_factor(user)
     end
   end
 
@@ -65,6 +75,7 @@ module AuthenticatesWithTwoFactor
       remember_me(user) if user_params[:remember_me] == '1'
       sign_in(user)
     else
+      user.increment_failed_attempts!
       flash.now[:alert] = 'Authentication via U2F device failed.'
       prompt_for_two_factor(user)
     end
diff --git a/app/models/user.rb b/app/models/user.rb
index 6996740eebd..7f5a8562907 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -827,6 +827,22 @@ class User < ActiveRecord::Base
     todos_pending_count(force: true)
   end
 
+  # This is copied from Devise::Models::Lockable#valid_for_authentication?, as our auth
+  # flow means we don't call that automatically (and can't conveniently do so).
+  #
+  # See:
+  #   <https://github.com/plataformatec/devise/blob/v4.0.0/lib/devise/models/lockable.rb#L92>
+  #
+  def increment_failed_attempts!
+    self.failed_attempts ||= 0
+    self.failed_attempts += 1
+    if attempts_exceeded?
+      lock_access! unless access_locked?
+    else
+      save(validate: false)
+    end
+  end
+
   private
 
   def projects_union(min_access_level = nil)
diff --git a/spec/controllers/sessions_controller_spec.rb b/spec/controllers/sessions_controller_spec.rb
index 8f27e616c3e..48d69377461 100644
--- a/spec/controllers/sessions_controller_spec.rb
+++ b/spec/controllers/sessions_controller_spec.rb
@@ -109,6 +109,44 @@ describe SessionsController do
             end
           end
 
+          context 'when the user is on their last attempt' do
+            before do
+              user.update(failed_attempts: User.maximum_attempts.pred)
+            end
+
+            context 'when OTP is valid' do
+              it 'authenticates correctly' do
+                authenticate_2fa(otp_attempt: user.current_otp)
+
+                expect(subject.current_user).to eq user
+              end
+            end
+
+            context 'when OTP is invalid' do
+              before { authenticate_2fa(otp_attempt: 'invalid') }
+
+              it 'does not authenticate' do
+                expect(subject.current_user).not_to eq user
+              end
+
+              it 'warns about invalid login' do
+                expect(response).to set_flash.now[:alert]
+                  .to /Invalid Login or password/
+              end
+
+              it 'locks the user' do
+                expect(user.reload).to be_access_locked
+              end
+
+              it 'keeps the user locked on future login attempts' do
+                post(:create, user: { login: user.username, password: user.password })
+
+                expect(response)
+                  .to set_flash.now[:alert].to /Invalid Login or password/
+              end
+            end
+          end
+
           context 'when another user does not have 2FA enabled' do
             let(:another_user) { create(:user) }