From 7127d7287728fe8723a11c1de2d1786ceb05e47c Mon Sep 17 00:00:00 2001 From: Chris MacNaughton Date: Thu, 11 Jan 2024 15:24:33 -0500 Subject: [PATCH] Merge pull request from GHSA-qrqh-v2j6-3g7w A user failing to provide the correct TOTP code more than ten consecutive times will have their account locked, forcing an email reset or time based reset. --- .../concerns/authenticates_with_two_factor.rb | 11 +++++++++-- spec/controllers/sessions_controller_spec.rb | 12 ++++++++++++ 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/app/controllers/concerns/authenticates_with_two_factor.rb b/app/controllers/concerns/authenticates_with_two_factor.rb index 455e3f1..b1acd55 100644 --- a/app/controllers/concerns/authenticates_with_two_factor.rb +++ b/app/controllers/concerns/authenticates_with_two_factor.rb @@ -34,6 +34,7 @@ def authenticate_with_two_factor # rubocop:disable Metrics/AbcSize, Metrics/Cycl return handle_expired_attempt if attempt_expired? + handle_locked_user(user) if user.lock_strategy_enabled?(:failed_attempts) && user.send(:attempts_exceeded?) if 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] @@ -55,7 +56,7 @@ def authenticate_with_two_factor_via_otp(user) user.save! sign_in(user, message: :two_factor_authenticated, event: :authentication) else - # user.increment_failed_attempts! + user.increment_failed_attempts Rails.logger.warn("Failed Login: user=#{user.username} ip=#{request.remote_ip} method=OTP") flash.now[:alert] = _('Invalid two-factor code.') prompt_for_two_factor(user) @@ -124,7 +125,6 @@ def authenticate_with_two_factor_via_webauthn(user) # rubocop:disable Metrics/Ab end def handle_two_factor_failure(user, method) - # user.increment_failed_attempts! Rails.logger.warn("Failed Login: user=#{user.username} ip=#{request.remote_ip} method=#{method}") flash.now[:alert] = format(_('Authentication via %{method} device failed.'), method:) # rubocop:disable Style/FormatStringToken prompt_for_two_factor(user) @@ -145,6 +145,13 @@ def clear_two_factor_attempt!(purge: true) reset_session if purge end + def handle_locked_user(user) + user.lock_access! + + clear_two_factor_attempt! + redirect_to new_user_session_path, alert: _('An error occurred. Please sign in again.') + end + def handle_changed_user(_user) clear_two_factor_attempt! diff --git a/spec/controllers/sessions_controller_spec.rb b/spec/controllers/sessions_controller_spec.rb index 0b68bb6..25b51d5 100644 --- a/spec/controllers/sessions_controller_spec.rb +++ b/spec/controllers/sessions_controller_spec.rb @@ -227,6 +227,18 @@ def authenticate_2fa(user_params, otp_user_id: user.id) expect(flash.now[:alert]).to eq('It took too long to verify your authentication device. Please try again') end + it 'locks a user after many incorrect OTP attempts' do + expect(user.access_locked?).not_to be_truthy + started_at = Time.now.utc.to_s + 11.times do + post(:create, params: { user: { remember_me: '0', otp_attempt: 123_456 } }, +session: { otp_user_id: user.id, otp_started: started_at }) + end + # binding.pry + user.reload + expect(user.access_locked?).to be_truthy + end + it 'redirects to a known application' do allow(controller).to receive(:find_user).and_return(user) Application.create!(uid: 'test', internal: true, redirect_uri: 'https://example.com', name: 'test')