From e2d9635074ad33cc8144adc434bcd90faae9c424 Mon Sep 17 00:00:00 2001 From: Claire Date: Mon, 22 Jan 2024 14:55:43 +0100 Subject: [PATCH] Add notification email on invalid second authenticator (#28822) --- app/controllers/auth/sessions_controller.rb | 5 ++++ app/mailers/user_mailer.rb | 12 ++++++++++ app/views/user_mailer/failed_2fa.html.haml | 24 +++++++++++++++++++ app/views/user_mailer/failed_2fa.text.erb | 15 ++++++++++++ config/locales/en.yml | 6 +++++ .../auth/sessions_controller_spec.rb | 20 +++++++++++++--- spec/mailers/previews/user_mailer_preview.rb | 5 ++++ spec/mailers/user_mailer_spec.rb | 18 ++++++++++++++ 8 files changed, 102 insertions(+), 3 deletions(-) create mode 100644 app/views/user_mailer/failed_2fa.html.haml create mode 100644 app/views/user_mailer/failed_2fa.text.erb diff --git a/app/controllers/auth/sessions_controller.rb b/app/controllers/auth/sessions_controller.rb index 6bc48a7804b..962b78de65f 100644 --- a/app/controllers/auth/sessions_controller.rb +++ b/app/controllers/auth/sessions_controller.rb @@ -181,6 +181,11 @@ class Auth::SessionsController < Devise::SessionsController ip: request.remote_ip, user_agent: request.user_agent ) + + # Only send a notification email every hour at most + return if redis.set("2fa_failure_notification:#{user.id}", '1', ex: 1.hour, get: true).present? + + UserMailer.failed_2fa(user, request.remote_ip, request.user_agent, Time.now.utc).deliver_later! end def second_factor_attempts_key(user) diff --git a/app/mailers/user_mailer.rb b/app/mailers/user_mailer.rb index 432b851b5e6..3b1a085cb88 100644 --- a/app/mailers/user_mailer.rb +++ b/app/mailers/user_mailer.rb @@ -191,6 +191,18 @@ class UserMailer < Devise::Mailer end end + def failed_2fa(user, remote_ip, user_agent, timestamp) + @resource = user + @remote_ip = remote_ip + @user_agent = user_agent + @detection = Browser.new(user_agent) + @timestamp = timestamp.to_time.utc + + I18n.with_locale(locale) do + mail subject: default_i18n_subject + end + end + private def default_devise_subject diff --git a/app/views/user_mailer/failed_2fa.html.haml b/app/views/user_mailer/failed_2fa.html.haml new file mode 100644 index 00000000000..e1da35ce062 --- /dev/null +++ b/app/views/user_mailer/failed_2fa.html.haml @@ -0,0 +1,24 @@ += content_for :heading do + = render 'application/mailer/heading', heading_title: t('user_mailer.failed_2fa.title'), heading_subtitle: t('user_mailer.failed_2fa.explanation'), heading_image_url: frontend_asset_url('images/mailer-new/heading/login.png') +%table.email-w-full{ cellspacing: 0, cellpadding: 0, border: 0, role: 'presentation' } + %tr + %td.email-body-padding-td + %table.email-inner-card-table{ cellspacing: 0, cellpadding: 0, border: 0, role: 'presentation' } + %tr + %td.email-inner-card-td.email-prose + %p= t 'user_mailer.failed_2fa.details' + %p + %strong #{t('sessions.ip')}: + = @remote_ip + %br/ + %strong #{t('sessions.browser')}: + %span{ title: @user_agent } + = t 'sessions.description', + browser: t("sessions.browsers.#{@detection.id}", default: @detection.id.to_s), + platform: t("sessions.platforms.#{@detection.platform.id}", default: @detection.platform.id.to_s) + %br/ + %strong #{t('sessions.date')}: + = l(@timestamp.in_time_zone(@resource.time_zone.presence), format: :with_time_zone) + = render 'application/mailer/button', text: t('settings.account_settings'), url: edit_user_registration_url + %p= t 'user_mailer.failed_2fa.further_actions_html', + action: link_to(t('user_mailer.suspicious_sign_in.change_password'), edit_user_registration_url) diff --git a/app/views/user_mailer/failed_2fa.text.erb b/app/views/user_mailer/failed_2fa.text.erb new file mode 100644 index 00000000000..c1dbf7d929e --- /dev/null +++ b/app/views/user_mailer/failed_2fa.text.erb @@ -0,0 +1,15 @@ +<%= t 'user_mailer.failed_2fa.title' %> + +=== + +<%= t 'user_mailer.failed_2fa.explanation' %> + +<%= t 'user_mailer.failed_2fa.details' %> + +<%= t('sessions.ip') %>: <%= @remote_ip %> +<%= t('sessions.browser') %>: <%= t('sessions.description', browser: t("sessions.browsers.#{@detection.id}", default: "#{@detection.id}"), platform: t("sessions.platforms.#{@detection.platform.id}", default: "#{@detection.platform.id}")) %> +<%= l(@timestamp.in_time_zone(@resource.time_zone.presence), format: :with_time_zone) %> + +<%= t 'user_mailer.failed_2fa.further_actions_html', action: t('user_mailer.suspicious_sign_in.change_password') %> + +=> <%= edit_user_registration_url %> diff --git a/config/locales/en.yml b/config/locales/en.yml index 89ca0ad72cd..83eaaa4552c 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1791,6 +1791,12 @@ en: extra: It's now ready for download! subject: Your archive is ready for download title: Archive takeout + failed_2fa: + details: 'Here are details of the sign-in attempt:' + explanation: Someone has tried to sign in to your account but provided an invalid second authentication factor. + further_actions_html: If this wasn't you, we recommend that you %{action} immediately as it may be compromised. + subject: Second factor authentication failure + title: Failed second factor authentication suspicious_sign_in: change_password: change your password details: 'Here are details of the sign-in:' diff --git a/spec/controllers/auth/sessions_controller_spec.rb b/spec/controllers/auth/sessions_controller_spec.rb index d238626c9dd..b663f55afac 100644 --- a/spec/controllers/auth/sessions_controller_spec.rb +++ b/spec/controllers/auth/sessions_controller_spec.rb @@ -265,21 +265,35 @@ RSpec.describe Auth::SessionsController 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) + + # Travel to the beginning of an hour to avoid crossing rate-limit buckets + travel_to '2023-12-20T10:00:00Z' 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 + + it 'sends a suspicious sign-in mail', :sidekiq_inline do + 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(UserMailer.deliveries.size).to eq(1) + expect(UserMailer.deliveries.first.to.first).to eq(user.email) + expect(UserMailer.deliveries.first.subject).to eq(I18n.t('user_mailer.failed_2fa.subject')) + end end context 'when using a valid OTP' do diff --git a/spec/mailers/previews/user_mailer_preview.rb b/spec/mailers/previews/user_mailer_preview.rb index 098c9cd901f..2722538e1aa 100644 --- a/spec/mailers/previews/user_mailer_preview.rb +++ b/spec/mailers/previews/user_mailer_preview.rb @@ -93,4 +93,9 @@ class UserMailerPreview < ActionMailer::Preview def suspicious_sign_in UserMailer.suspicious_sign_in(User.first, '127.0.0.1', 'Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:75.0) Gecko/20100101 Firefox/75.0', Time.now.utc) end + + # Preview this email at http://localhost:3000/rails/mailers/user_mailer/failed_2fa + def failed_2fa + UserMailer.failed_2fa(User.first, '127.0.0.1', 'Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:75.0) Gecko/20100101 Firefox/75.0', Time.now.utc) + end end diff --git a/spec/mailers/user_mailer_spec.rb b/spec/mailers/user_mailer_spec.rb index 4a439282480..404b8347028 100644 --- a/spec/mailers/user_mailer_spec.rb +++ b/spec/mailers/user_mailer_spec.rb @@ -135,6 +135,24 @@ describe UserMailer do 'user_mailer.suspicious_sign_in.subject' end + describe '#failed_2fa' do + let(:ip) { '192.168.0.1' } + let(:agent) { 'NCSA_Mosaic/2.0 (Windows 3.1)' } + let(:timestamp) { Time.now.utc } + let(:mail) { described_class.failed_2fa(receiver, ip, agent, timestamp) } + + it 'renders failed 2FA notification' do + receiver.update!(locale: nil) + + expect(mail) + .to be_present + .and(have_body_text(I18n.t('user_mailer.failed_2fa.explanation'))) + end + + include_examples 'localized subject', + 'user_mailer.failed_2fa.subject' + end + describe '#appeal_approved' do let(:appeal) { Fabricate(:appeal, account: receiver.account, approved_at: Time.now.utc) } let(:mail) { described_class.appeal_approved(receiver, appeal) }