diff --git a/app/controllers/auth/sessions_controller.rb b/app/controllers/auth/sessions_controller.rb index afcf8b24b..17d75e1bb 100644 --- a/app/controllers/auth/sessions_controller.rb +++ b/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] @@ -136,9 +140,23 @@ class Auth::SessionsController < Devise::SessionsController 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) @@ -170,4 +188,8 @@ class Auth::SessionsController < Devise::SessionsController user_agent: request.user_agent ) end + + def second_factor_attempts_key(user) + "2fa_auth_attempts:#{user.id}:#{Time.now.utc.hour}" + end end diff --git a/app/controllers/concerns/two_factor_authentication_concern.rb b/app/controllers/concerns/two_factor_authentication_concern.rb index 27f2367a8..ade02f04b 100644 --- a/app/controllers/concerns/two_factor_authentication_concern.rb +++ b/app/controllers/concerns/two_factor_authentication_concern.rb @@ -65,6 +65,11 @@ module TwoFactorAuthenticationConcern 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 diff --git a/config/locales/en.yml b/config/locales/en.yml index ce20d5d1d..8aab40333 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -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: diff --git a/spec/controllers/auth/sessions_controller_spec.rb b/spec/controllers/auth/sessions_controller_spec.rb index d3db7aa1a..0941e2cb3 100644 --- a/spec/controllers/auth/sessions_controller_spec.rb +++ b/spec/controllers/auth/sessions_controller_spec.rb @@ -262,7 +262,27 @@ RSpec.describe Auth::SessionsController, type: :controller do 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