Skip to content

Commit

Permalink
Merge pull request from GHSA-qrqh-v2j6-3g7w
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
ChrisMacNaughton authored Jan 11, 2024
1 parent f831512 commit 0a8dbdd
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 2 deletions.
11 changes: 9 additions & 2 deletions app/controllers/concerns/authenticates_with_two_factor.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand All @@ -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)
Expand Down Expand Up @@ -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)
Expand All @@ -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!

Expand Down
12 changes: 12 additions & 0 deletions spec/controllers/sessions_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down

0 comments on commit 0a8dbdd

Please sign in to comment.