Skip to content

Commit

Permalink
Add rate-limit of TOTP authentication attempts at controller level (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
ClearlyClaire committed Jan 24, 2024
1 parent e6072a8 commit 2e8943a
Show file tree
Hide file tree
Showing 4 changed files with 49 additions and 1 deletion.
22 changes: 22 additions & 0 deletions app/controllers/auth/sessions_controller.rb
@@ -1,6 +1,10 @@
# frozen_string_literal: true

class Auth::SessionsController < Devise::SessionsController
include Redisable

MAX_2FA_ATTEMPTS_PER_HOUR = 10

layout 'auth'

skip_before_action :require_no_authentication, only: [:create]
Expand Down Expand Up @@ -136,9 +140,23 @@ def clear_attempt_from_session
session.delete(:attempt_user_updated_at)
end

def clear_2fa_attempt_from_user(user)
redis.del(second_factor_attempts_key(user))
end

def check_second_factor_rate_limits(user)
attempts, = redis.multi do |multi|
multi.incr(second_factor_attempts_key(user))
multi.expire(second_factor_attempts_key(user), 1.hour)
end

attempts >= MAX_2FA_ATTEMPTS_PER_HOUR
end

def on_authentication_success(user, security_measure)
@on_authentication_success_called = true

clear_2fa_attempt_from_user(user)
clear_attempt_from_session

user.update_sign_in!(new_sign_in: true)
Expand Down Expand Up @@ -170,4 +188,8 @@ def on_authentication_failure(user, security_measure, failure_reason)
user_agent: request.user_agent
)
end

def second_factor_attempts_key(user)
"2fa_auth_attempts:#{user.id}:#{Time.now.utc.hour}"
end
end
5 changes: 5 additions & 0 deletions app/controllers/concerns/two_factor_authentication_concern.rb
Expand Up @@ -65,6 +65,11 @@ def authenticate_with_two_factor_via_webauthn(user)
end

def authenticate_with_two_factor_via_otp(user)
if check_second_factor_rate_limits(user)
flash.now[:alert] = I18n.t('users.rate_limited')
return prompt_for_two_factor(user)
end

if valid_otp_attempt?(user)
on_authentication_success(user, :otp)
else
Expand Down
1 change: 1 addition & 0 deletions config/locales/en.yml
Expand Up @@ -1684,6 +1684,7 @@ en:
follow_limit_reached: You cannot follow more than %{limit} people
invalid_otp_token: Invalid two-factor code
otp_lost_help_html: If you lost access to both, you may get in touch with %{email}
rate_limited: Too many authentication attempts, try again later.
seamless_external_login: You are logged in via an external service, so password and e-mail settings are not available.
signed_in_as: 'Signed in as:'
verification:
Expand Down
22 changes: 21 additions & 1 deletion spec/controllers/auth/sessions_controller_spec.rb
Expand Up @@ -262,7 +262,27 @@
end
end

context 'using a valid OTP' do
context 'when repeatedly using an invalid TOTP code before using a valid code' do
before do
stub_const('Auth::SessionsController::MAX_2FA_ATTEMPTS_PER_HOUR', 2)
end

it 'does not log the user in' do
# Travel to the beginning of an hour to avoid crossing rate-limit buckets
travel_to '2023-12-20T10:00:00Z'

Auth::SessionsController::MAX_2FA_ATTEMPTS_PER_HOUR.times do
post :create, params: { user: { otp_attempt: '1234' } }, session: { attempt_user_id: user.id, attempt_user_updated_at: user.updated_at.to_s }
expect(controller.current_user).to be_nil
end

post :create, params: { user: { otp_attempt: user.current_otp } }, session: { attempt_user_id: user.id, attempt_user_updated_at: user.updated_at.to_s }
expect(controller.current_user).to be_nil
expect(flash[:alert]).to match I18n.t('users.rate_limited')
end
end

context 'when using a valid OTP' do
before do
post :create, params: { user: { otp_attempt: user.current_otp } }, session: { attempt_user_id: user.id, attempt_user_updated_at: user.updated_at.to_s }
end
Expand Down

0 comments on commit 2e8943a

Please sign in to comment.