diff --git a/app/controllers/concerns/two_factor_authenticatable_methods.rb b/app/controllers/concerns/two_factor_authenticatable_methods.rb index 56f4cc02a30..bcfee78cc87 100644 --- a/app/controllers/concerns/two_factor_authenticatable_methods.rb +++ b/app/controllers/concerns/two_factor_authenticatable_methods.rb @@ -33,7 +33,7 @@ def handle_valid_verification_for_authentication_context(auth_method:) mark_user_session_authenticated(auth_method:, authentication_type: :valid_2fa) disavowal_event, disavowal_token = create_user_event_with_disavowal(:sign_in_after_2fa) - if IdentityConfig.store.feature_new_device_alert_aggregation_enabled && new_device? + if new_device? if current_user.sign_in_new_device_at.blank? if sign_in_notification_timeframe_expired_event.present? current_user.update( diff --git a/app/mailers/user_mailer.rb b/app/mailers/user_mailer.rb index b5a097c61d2..74064a42085 100644 --- a/app/mailers/user_mailer.rb +++ b/app/mailers/user_mailer.rb @@ -112,19 +112,6 @@ def personal_key_sign_in(disavowal_token:) end end - def new_device_sign_in(date:, location:, device_name:, disavowal_token:) - with_user_locale(user) do - @login_date = date - @login_location = location - @device_name = device_name - @disavowal_token = disavowal_token - mail( - to: email_address.email, - subject: t('user_mailer.new_device_sign_in.subject', app_name: APP_NAME), - ) - end - end - # @param [Array] events Array of sign-in Event records (event types "sign_in_before_2fa", # "sign_in_after_2fa", "sign_in_unsuccessful_2fa") # @param [String] disavowal_token Token to generate URL for disavowing event diff --git a/app/services/user_alerts/alert_user_about_new_device.rb b/app/services/user_alerts/alert_user_about_new_device.rb index 28d70f4172f..90c17b7340f 100644 --- a/app/services/user_alerts/alert_user_about_new_device.rb +++ b/app/services/user_alerts/alert_user_about_new_device.rb @@ -2,26 +2,8 @@ module UserAlerts class AlertUserAboutNewDevice - def self.call(event:, device:, disavowal_token:) - return if IdentityConfig.store.feature_new_device_alert_aggregation_enabled - device_decorator = DeviceDecorator.new(device) - login_location = device_decorator.last_sign_in_location_and_ip - device_name = device_decorator.nice_name - - event.user.confirmed_email_addresses.each do |email_address| - UserMailer.with(user: event.user, email_address: email_address).new_device_sign_in( - date: device.last_used_at.in_time_zone('Eastern Time (US & Canada)'). - strftime('%B %-d, %Y %H:%M Eastern Time'), - location: login_location, - device_name: device_name, - disavowal_token: disavowal_token, - ).deliver_now_or_later - end - end - def self.schedule_alert(event:) - return if !IdentityConfig.store.feature_new_device_alert_aggregation_enabled || - event.user.sign_in_new_device_at.present? + return if event.user.sign_in_new_device_at.present? event.user.update(sign_in_new_device_at: event.created_at) end @@ -53,10 +35,8 @@ def self.send_alert(user:, disavowal_event:, disavowal_token:) end def self.sign_in_events_start_time(user:) - # Avoid scenarios where stale events may be reflected in the time since sign in: - # - # 1. The feature is enabled for a short time in a deployed environment before being disabled - # 2. In local development, the server is not always active and the job may not run until later + # Avoid scenarios where stale events may be reflected in the time since sign in, such as if + # the server is not always active and the job may not run until later. # # Typically, it's guaranteed that even in the worst-case of a sign-in occurring immediately # after a scheduled job run, it should take no longer than twice the scheduled delay. A small diff --git a/app/services/user_event_creator.rb b/app/services/user_event_creator.rb index 106cd0de154..4c7cd351596 100644 --- a/app/services/user_event_creator.rb +++ b/app/services/user_event_creator.rb @@ -84,7 +84,6 @@ def create_event_for_new_device(event_type:, user:, disavowal_token:) event, disavowal_token = create_user_event_with_disavowal(event_type, user, device) [device, event, disavowal_token] end - send_new_device_notification(event:, device:, disavowal_token:) [event, disavowal_token] else Device.transaction do @@ -111,10 +110,6 @@ def create_device_for_user(user) ) end - def send_new_device_notification(event:, device:, disavowal_token:) - UserAlerts::AlertUserAboutNewDevice.call(event:, device:, disavowal_token:) - end - # @return [Array(Event, String)] an (event, disavowal_token) tuple def create_event_for_device(event_type:, user:, device:, disavowal_token: nil) disavowal_token_fingerprint = if disavowal_token diff --git a/app/views/user_mailer/new_device_sign_in.html.erb b/app/views/user_mailer/new_device_sign_in.html.erb deleted file mode 100644 index ae3cd4c2b51..00000000000 --- a/app/views/user_mailer/new_device_sign_in.html.erb +++ /dev/null @@ -1,41 +0,0 @@ - - - - - - -
-   -
- - - - - -
-   -
- -

- <%= t( - 'user_mailer.new_device_sign_in.info', - app_name: APP_NAME, - ) %> -

- -
- -

<%= @login_date %>
<%= @login_location %>, <%= @device_name %>

- -

- <%= t( - 'user_mailer.new_device_sign_in.help_html', - app_name_html: link_to(APP_NAME, IdentityConfig.store.mailer_domain_name, class: 'gray'), - disavowal_link_html: link_to( - t('.disavowal_link'), - event_disavowal_url(disavowal_token: @disavowal_token), - ), - help_link_html: link_to(t('user_mailer.help_link_text'), MarketingSite.help_url), - contact_link_html: link_to(t('user_mailer.contact_link_text'), MarketingSite.contact_url), - ) %> -

diff --git a/config/application.yml.default b/config/application.yml.default index d21d14aaade..2f4e0f56e17 100644 --- a/config/application.yml.default +++ b/config/application.yml.default @@ -64,20 +64,20 @@ country_phone_number_overrides: '{}' database_host: '' database_name: '' database_password: '' +database_pool_idp: 5 database_read_replica_host: '' database_readonly_password: '' database_readonly_username: '' -database_username: '' -database_worker_jobs_host: '' -database_worker_jobs_name: '' -database_worker_jobs_password: '' -database_worker_jobs_username: '' -database_pool_idp: 5 database_socket: '' database_sslmode: 'verify-full' database_statement_timeout: 2_500 database_timeout: 5_000 +database_username: '' +database_worker_jobs_host: '' +database_worker_jobs_name: '' +database_worker_jobs_password: '' database_worker_jobs_sslmode: 'verify-full' +database_worker_jobs_username: '' deleted_user_accounts_report_configs: '[]' deliver_mail_async: false development_mailer_deliver_method: letter_opener @@ -115,7 +115,6 @@ enable_usps_verification: true event_disavowal_expiration_hours: 240 feature_idv_force_gpo_verification_enabled: false feature_idv_hybrid_flow_enabled: true -feature_new_device_alert_aggregation_enabled: true geo_data_file_path: 'geo_data/GeoLite2-City.mmdb' get_usps_proofing_results_job_cron: '0/30 * * * *' get_usps_proofing_results_job_reprocess_delay_minutes: 5 @@ -457,7 +456,6 @@ production: email_registrations_per_ip_track_only_mode: true enable_test_routes: false enable_usps_verification: false - feature_new_device_alert_aggregation_enabled: false hmac_fingerprinter_key: hmac_fingerprinter_key_queue: '[]' idv_sp_required: true diff --git a/config/initializers/job_configurations.rb b/config/initializers/job_configurations.rb index dc89d3f0dad..9e00efaa556 100644 --- a/config/initializers/job_configurations.rb +++ b/config/initializers/job_configurations.rb @@ -29,15 +29,11 @@ args: -> { [Time.zone.now] }, }, # Send new device alert notifications - create_new_device_alert_send_emails: ( - if IdentityConfig.store.feature_new_device_alert_aggregation_enabled - { - class: 'CreateNewDeviceAlert', - cron: cron_5m, - args: -> { [Time.zone.now] }, - } - end - ), + create_new_device_alert_send_emails: { + class: 'CreateNewDeviceAlert', + cron: cron_5m, + args: -> { [Time.zone.now] }, + }, # Send Total Monthly Auths Report to S3 total_monthly_auths: { class: 'Reports::TotalMonthlyAuthsReport', diff --git a/config/locales/en.yml b/config/locales/en.yml index 6de2c54883a..f1a3b37a92f 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1890,10 +1890,6 @@ user_mailer.new_device_sign_in_before_2fa.info_p2: If you recognize this activit user_mailer.new_device_sign_in_before_2fa.info_p3_html: Two-factor authentication protects your account from unauthorized access. If this wasn’t you, %{reset_password_link_html} immediately. user_mailer.new_device_sign_in_before_2fa.reset_password: reset your password user_mailer.new_device_sign_in_before_2fa.subject: New sign-in with your %{app_name} account -user_mailer.new_device_sign_in.disavowal_link: reset your password -user_mailer.new_device_sign_in.help_html: If you did not make this change, %{disavowal_link_html}. For more help, please visit the %{app_name_html} %{help_link_html} or %{contact_link_html}. -user_mailer.new_device_sign_in.info: Your %{app_name} account was just used to sign in on a new device. -user_mailer.new_device_sign_in.subject: New sign-in with your %{app_name} account user_mailer.password_changed.disavowal_link: reset your password user_mailer.password_changed.help_html: If you did not make this change, %{disavowal_link_html}. For more help, please visit the %{app_name_html} %{help_link_html} or %{contact_link_html}. user_mailer.password_changed.intro_html: You have a new password for your %{app_name_html} account. diff --git a/config/locales/es.yml b/config/locales/es.yml index 6ecef7c4140..2d5bcae878a 100644 --- a/config/locales/es.yml +++ b/config/locales/es.yml @@ -1902,10 +1902,6 @@ user_mailer.new_device_sign_in_before_2fa.info_p2: Si reconoce esta actividad, n user_mailer.new_device_sign_in_before_2fa.info_p3_html: La autenticación de dos factores protege su cuenta contra un acceso no autorizado. Si no fue usted, %{reset_password_link_html}. user_mailer.new_device_sign_in_before_2fa.reset_password: restablezca de inmediato su contraseña. user_mailer.new_device_sign_in_before_2fa.subject: Nuevo inicio de sesión con su cuenta de %{app_name} -user_mailer.new_device_sign_in.disavowal_link: restablezca su contraseña -user_mailer.new_device_sign_in.help_html: Si usted no hizo este cambio, %{disavowal_link_html}. Para obtener más ayuda, visite %{app_name_html} %{help_link_html} o %{contact_link_html}. -user_mailer.new_device_sign_in.info: Su cuenta de %{app_name} acaba de ser usada para iniciar sesión en un nuevo dispositivo. -user_mailer.new_device_sign_in.subject: Nuevo inicio de sesión con su cuenta de %{app_name} user_mailer.password_changed.disavowal_link: restablezca su contraseña user_mailer.password_changed.help_html: Si usted no hizo este cambio, %{disavowal_link_html}. Para obtener más ayuda, visite %{app_name_html} %{help_link_html} o %{contact_link_html}. user_mailer.password_changed.intro_html: Tiene una contraseña nueva para su cuenta de %{app_name_html}. diff --git a/config/locales/fr.yml b/config/locales/fr.yml index f7cb0fdc66e..a1489bc0f48 100644 --- a/config/locales/fr.yml +++ b/config/locales/fr.yml @@ -1890,10 +1890,6 @@ user_mailer.new_device_sign_in_before_2fa.info_p2: Si vous reconnaissez cette ac user_mailer.new_device_sign_in_before_2fa.info_p3_html: L’authentification à deux facteurs protège votre compte des accès non autorisés. Si vous n’êtes pas à l’origine de cette action, %{reset_password_link_html}. user_mailer.new_device_sign_in_before_2fa.reset_password: veuillez réinitialiser immédiatement votre mot de passe. user_mailer.new_device_sign_in_before_2fa.subject: Nouvelle connexion avec votre compte %{app_name} -user_mailer.new_device_sign_in.disavowal_link: réinitialisez votre mot de passe -user_mailer.new_device_sign_in.help_html: Si vous n’avez pas effectué ce changement, %{disavowal_link_html}. Pour plus d’aide, veuillez visiter le %{help_link_html} de %{app_name_html} ou %{contact_link_html}. -user_mailer.new_device_sign_in.info: Votre compte %{app_name} vient d’être utilisé pour une connexion sur un nouvel appareil. -user_mailer.new_device_sign_in.subject: Nouvelle connexion avec votre compte %{app_name} user_mailer.password_changed.disavowal_link: réinitialisez votre mot de passe user_mailer.password_changed.help_html: Si vous n’avez pas effectué ce changement, %{disavowal_link_html}. Pour plus d’aide, veuillez visiter le %{help_link_html} de %{app_name_html} ou %{contact_link_html}. user_mailer.password_changed.intro_html: Vous avez un nouveau mot de passe pour votre compte %{app_name_html}. diff --git a/config/locales/zh.yml b/config/locales/zh.yml index 0122bd5cb22..316555349c5 100644 --- a/config/locales/zh.yml +++ b/config/locales/zh.yml @@ -1903,10 +1903,6 @@ user_mailer.new_device_sign_in_before_2fa.info_p2: 如果你知道该活动, user_mailer.new_device_sign_in_before_2fa.info_p3_html: 双重身份验证保护你账户不受未经授权的访问。如果不是你,请马上 %{reset_password_link_html}. user_mailer.new_device_sign_in_before_2fa.reset_password: 重设密码 user_mailer.new_device_sign_in_before_2fa.subject: 你 %{app_name} 账户有新的登录 -user_mailer.new_device_sign_in.disavowal_link: 重设你的密码 -user_mailer.new_device_sign_in.help_html: 如果你没做此更改, %{disavowal_link_html}。要得到更多帮助,请访问 %{app_name_html} %{help_link_html} 或者 %{contact_link_html}。 -user_mailer.new_device_sign_in.info: 你的 %{app_name} 帐号刚刚在一个新设备上用于登录。 -user_mailer.new_device_sign_in.subject: 用你 %{app_name} 账户进行的新登录 user_mailer.password_changed.disavowal_link: 重设你的密码 user_mailer.password_changed.help_html: 如果你没做此更改, %{disavowal_link_html}。要得到更多帮助,请访问 %{app_name_html} %{help_link_html} 或者 %{contact_link_html}。 user_mailer.password_changed.intro_html: 你的 %{app_name_html} 账户有了新密码。 diff --git a/lib/identity_config.rb b/lib/identity_config.rb index 9412d21a65f..cce2257722a 100644 --- a/lib/identity_config.rb +++ b/lib/identity_config.rb @@ -136,7 +136,6 @@ def self.store config.add(:event_disavowal_expiration_hours, type: :integer) config.add(:feature_idv_force_gpo_verification_enabled, type: :boolean) config.add(:feature_idv_hybrid_flow_enabled, type: :boolean) - config.add(:feature_new_device_alert_aggregation_enabled, type: :boolean) config.add(:geo_data_file_path, type: :string) config.add(:get_usps_proofing_results_job_cron, type: :string) config.add(:get_usps_proofing_results_job_reprocess_delay_minutes, type: :integer) diff --git a/spec/controllers/concerns/two_factor_authenticatable_methods_spec.rb b/spec/controllers/concerns/two_factor_authenticatable_methods_spec.rb index f1dd1b2ad14..f14f9ec9c32 100644 --- a/spec/controllers/concerns/two_factor_authenticatable_methods_spec.rb +++ b/spec/controllers/concerns/two_factor_authenticatable_methods_spec.rb @@ -65,10 +65,9 @@ context 'when authenticating without new device sign in' do let(:user) { create(:user) } - context 'when alert aggregation feature is disabled' do + context 'with an existing device' do before do - allow(IdentityConfig.store).to receive(:feature_new_device_alert_aggregation_enabled). - and_return(false) + allow(controller).to receive(:new_device?).and_return(false) end it 'does not send an alert' do @@ -78,79 +77,60 @@ end end - context 'when alert aggregation feature is enabled' do + context 'with a new device' do before do - allow(IdentityConfig.store).to receive(:feature_new_device_alert_aggregation_enabled). - and_return(true) + allow(controller).to receive(:new_device?).and_return(true) end - context 'with an existing device' do - before do - allow(controller).to receive(:new_device?).and_return(false) + it 'sends the new device alert using 2fa event date' do + expect(UserAlerts::AlertUserAboutNewDevice).to receive(:send_alert) do |**args| + expect(user.reload.sign_in_new_device_at.change(usec: 0)).to eq( + args[:disavowal_event].created_at.change(usec: 0), + ) + expect(args[:user]).to eq(user) + expect(args[:disavowal_event]).to be_kind_of(Event) + expect(args[:disavowal_token]).to be_kind_of(String) end + result + end - it 'does not send an alert' do - expect(UserAlerts::AlertUserAboutNewDevice).to_not receive(:send_alert) - + context 'sign_in_notification_timeframe_expired missing' do + it 'tracks analytics event for missing timeframe_expired' do + stub_analytics result + + expect(@analytics).to have_logged_event( + :sign_in_notification_timeframe_expired_absent, + ) end end - context 'with a new device' do + context 'sign_in_notification_timeframe_expired present' do before do - allow(controller).to receive(:new_device?).and_return(true) + create( + :event, + user:, + event_type: :sign_in_notification_timeframe_expired, + created_at: 10.minutes.ago, + ) end - it 'sends the new device alert using 2fa event date' do - expect(UserAlerts::AlertUserAboutNewDevice).to receive(:send_alert) do |**args| - expect(user.reload.sign_in_new_device_at.change(usec: 0)).to eq( - args[:disavowal_event].created_at.change(usec: 0), - ) - expect(args[:user]).to eq(user) - expect(args[:disavowal_event]).to be_kind_of(Event) - expect(args[:disavowal_token]).to be_kind_of(String) - end - result + around do |ex| + freeze_time { ex.run } end - context 'sign_in_notification_timeframe_expired missing' do - it 'tracks analytics event for missing timeframe_expired' do - stub_analytics - result - - expect(@analytics).to have_logged_event( - :sign_in_notification_timeframe_expired_absent, - ) - end - end - - context 'sign_in_notification_timeframe_expired present' do - before do - create( - :event, - user:, - event_type: :sign_in_notification_timeframe_expired, - created_at: 10.minutes.ago, + it 'creates a new user event with disavowal' do + expect(UserAlerts::AlertUserAboutNewDevice).to receive(:send_alert) do + expect(user.reload.sign_in_new_device_at.change(usec: 0)).to eq( + 10.minutes.ago, ) end + stub_analytics + result - around do |ex| - freeze_time { ex.run } - end - - it 'creates a new user event with disavowal' do - expect(UserAlerts::AlertUserAboutNewDevice).to receive(:send_alert) do - expect(user.reload.sign_in_new_device_at.change(usec: 0)).to eq( - 10.minutes.ago, - ) - end - stub_analytics - result - - expect(@analytics).to_not have_logged_event( - :sign_in_notification_timeframe_expired_absent, - ) - end + expect(@analytics).to_not have_logged_event( + :sign_in_notification_timeframe_expired_absent, + ) end end end @@ -159,10 +139,9 @@ context 'when authenticating with new device sign in' do let(:user) { create(:user, sign_in_new_device_at: Time.zone.now) } - context 'when alert aggregation feature is disabled' do + context 'with an existing device' do before do - allow(IdentityConfig.store).to receive(:feature_new_device_alert_aggregation_enabled). - and_return(false) + allow(controller).to receive(:new_device?).and_return(false) end it 'does not send an alert' do @@ -172,35 +151,16 @@ end end - context 'when alert aggregation feature is enabled' do + context 'with a new device' do before do - allow(IdentityConfig.store).to receive(:feature_new_device_alert_aggregation_enabled). - and_return(true) + allow(controller).to receive(:new_device?).and_return(true) end - context 'with an existing device' do - before do - allow(controller).to receive(:new_device?).and_return(false) - end + it 'sends the new device alert' do + expect(UserAlerts::AlertUserAboutNewDevice).to receive(:send_alert). + with(user:, disavowal_event: kind_of(Event), disavowal_token: kind_of(String)) - it 'does not send an alert' do - expect(UserAlerts::AlertUserAboutNewDevice).to_not receive(:send_alert) - - result - end - end - - context 'with a new device' do - before do - allow(controller).to receive(:new_device?).and_return(true) - end - - it 'sends the new device alert' do - expect(UserAlerts::AlertUserAboutNewDevice).to receive(:send_alert). - with(user:, disavowal_event: kind_of(Event), disavowal_token: kind_of(String)) - - result - end + result end end end diff --git a/spec/features/event_disavowal_spec.rb b/spec/features/event_disavowal_spec.rb index 4c0b96a7fbf..0259a8ad3f7 100644 --- a/spec/features/event_disavowal_spec.rb +++ b/spec/features/event_disavowal_spec.rb @@ -3,65 +3,57 @@ RSpec.feature 'disavowing an action' do let(:user) { create(:user, :fully_registered, :with_personal_key) } - context 'with aggregated sign-in notifications enabled' do - before do - allow(IdentityConfig.store).to receive(:feature_new_device_alert_aggregation_enabled). - and_return(true) - user.devices << create(:device) - end + scenario 'disavowing a sign-in after 2fa' do + sign_in_live_with_2fa(user) + Capybara.reset_session! - scenario 'disavowing a sign-in after 2fa' do - sign_in_live_with_2fa(user) - Capybara.reset_session! + disavow_last_action_and_reset_password + end - disavow_last_action_and_reset_password + scenario 'disavowing a sign-in before 2fa' do + travel_to (IdentityConfig.store.new_device_alert_delay_in_minutes + 1).minutes.ago do + sign_in_user(user) end - scenario 'disavowing a sign-in before 2fa' do - travel_to (IdentityConfig.store.new_device_alert_delay_in_minutes + 1).minutes.ago do - sign_in_user(user) - end + Capybara.reset_session! + CreateNewDeviceAlert.new.perform(Time.zone.now) - Capybara.reset_session! - CreateNewDeviceAlert.new.perform(Time.zone.now) + disavow_last_action_and_reset_password + end - disavow_last_action_and_reset_password + scenario 'disavowing a sign-in after 2fa after new device timeframe expired' do + travel_to (IdentityConfig.store.new_device_alert_delay_in_minutes + 1).minutes.ago do + sign_in_user(user) end - scenario 'disavowing a sign-in after 2fa after new device timeframe expired' do - travel_to (IdentityConfig.store.new_device_alert_delay_in_minutes + 1).minutes.ago do - sign_in_user(user) - end - - CreateNewDeviceAlert.new.perform(Time.zone.now) + CreateNewDeviceAlert.new.perform(Time.zone.now) - expect_delivered_email_count(1) - expect_delivered_email( - to: [user.email_addresses.first.email], - subject: t('user_mailer.new_device_sign_in_before_2fa.subject', app_name: APP_NAME), - ) + expect_delivered_email_count(1) + expect_delivered_email( + to: [user.email_addresses.first.email], + subject: t('user_mailer.new_device_sign_in_before_2fa.subject', app_name: APP_NAME), + ) - fill_in_code_with_last_phone_otp - click_submit_default + fill_in_code_with_last_phone_otp + click_submit_default - expect_delivered_email_count(2) - expect_delivered_email( - to: [user.email_addresses.first.email], - subject: t('user_mailer.new_device_sign_in_after_2fa.subject', app_name: APP_NAME), - ) + expect_delivered_email_count(2) + expect_delivered_email( + to: [user.email_addresses.first.email], + subject: t('user_mailer.new_device_sign_in_after_2fa.subject', app_name: APP_NAME), + ) - disavow_last_action_and_reset_password - end + disavow_last_action_and_reset_password + end - context 'user with piv or cac' do - let(:user) { create(:user, :fully_registered, :with_piv_or_cac) } + context 'user with piv or cac' do + let(:user) { create(:user, :fully_registered, :with_piv_or_cac) } - scenario 'disavowing a sign-in after 2fa using piv or cac' do - signin_with_piv(user) - Capybara.reset_session! + scenario 'disavowing a sign-in after 2fa using piv or cac' do + signin_with_piv(user) + Capybara.reset_session! - disavow_last_action_and_reset_password - end + disavow_last_action_and_reset_password end end @@ -84,22 +76,6 @@ disavow_last_action_and_reset_password end - context 'when aggregated new device alerts is disabled' do - before do - allow(IdentityConfig.store).to receive( - :feature_new_device_alert_aggregation_enabled, - ).and_return(false) - end - scenario 'disavowing a new device sign in' do - allow(IdentityConfig.store).to receive(:otp_delivery_blocklist_maxretry).and_return(3) - signin(user.email, user.password) - Capybara.reset_session! - visit root_path - signin(user.email, user.password) - disavow_last_action_and_reset_password - end - end - scenario 'disavowing a personal key sign in' do signin(user.email, user.password) choose_another_security_option(:personal_key) diff --git a/spec/features/new_device_tracking_spec.rb b/spec/features/new_device_tracking_spec.rb index 3b81e1f62a3..69a20e0cbb9 100644 --- a/spec/features/new_device_tracking_spec.rb +++ b/spec/features/new_device_tracking_spec.rb @@ -5,53 +5,8 @@ let(:user) { create(:user, :fully_registered) } - context 'user has existing devices and aggregated new device alerts is disabled' do + context 'user has existing devices' do let(:user) { create(:user, :with_authenticated_device) } - before do - allow(IdentityConfig.store).to receive( - :feature_new_device_alert_aggregation_enabled, - ).and_return(false) - end - - it 'sends a user notification on signin' do - sign_in_live_with_2fa(user) - - expect(user.reload.devices.length).to eq 2 - - device = user.devices.order(created_at: :desc).first - - expect_delivered_email_count(1) - expect_delivered_email( - to: [user.email_addresses.first.email], - subject: t('user_mailer.new_device_sign_in.subject', app_name: APP_NAME), - body: [device.last_used_at.in_time_zone('Eastern Time (US & Canada)'). - strftime('%B %-d, %Y %H:%M Eastern Time'), - 'From 127.0.0.1 (IP address potentially located in United States)'], - ) - end - - context 'from existing device' do - before do - Capybara.current_session.driver.browser.current_session.cookie_jar[:device] = - user.devices.first.cookie_uuid - end - - it 'does not send a user notification on sign in' do - sign_in_live_with_2fa(user) - - expect(user.reload.devices.length).to eq 1 - expect_delivered_email_count(0) - end - end - end - - context 'user has existing devices and aggregated new device alerts is enabled' do - let(:user) { create(:user, :with_authenticated_device) } - before do - allow(IdentityConfig.store).to receive( - :feature_new_device_alert_aggregation_enabled, - ).and_return(true) - end it 'sends a user notification on signin' do sign_in_live_with_2fa(user) @@ -239,12 +194,10 @@ context 'user does not have existing devices' do it 'should not send any notifications' do - allow(UserMailer).to receive(:new_device_sign_in).and_call_original - sign_in_user(user) expect(user.devices.length).to eq 1 - expect(UserMailer).not_to have_received(:new_device_sign_in) + expect_delivered_email_count(0) end end diff --git a/spec/features/webauthn/hidden_spec.rb b/spec/features/webauthn/hidden_spec.rb index 7b1f67bae47..1ad7393b430 100644 --- a/spec/features/webauthn/hidden_spec.rb +++ b/spec/features/webauthn/hidden_spec.rb @@ -96,7 +96,6 @@ it 'redirects to options page on sign in and shows the option' do email ||= user.email_addresses.first.email password = user.password - allow(UserMailer).to receive(:new_device_sign_in).and_call_original visit new_user_session_path set_hidden_field('platform_authenticator_available', 'false') fill_in_credentials_and_submit(email, password) diff --git a/spec/features/webauthn/sign_in_spec.rb b/spec/features/webauthn/sign_in_spec.rb index 533c40d2062..5d1f8b0bd28 100644 --- a/spec/features/webauthn/sign_in_spec.rb +++ b/spec/features/webauthn/sign_in_spec.rb @@ -90,7 +90,6 @@ before do email ||= user.email_addresses.first.email password = user.password - allow(UserMailer).to receive(:new_device_sign_in).and_call_original visit new_user_session_path set_hidden_field('platform_authenticator_available', 'false') fill_in_credentials_and_submit(email, password) diff --git a/spec/mailers/previews/user_mailer_preview.rb b/spec/mailers/previews/user_mailer_preview.rb index 90dbfaf556f..a421a980eb7 100644 --- a/spec/mailers/previews/user_mailer_preview.rb +++ b/spec/mailers/previews/user_mailer_preview.rb @@ -40,15 +40,6 @@ def personal_key_sign_in personal_key_sign_in(disavowal_token: SecureRandom.hex) end - def new_device_sign_in - UserMailer.with(user: user, email_address: email_address_record).new_device_sign_in( - date: 'February 25, 2019 15:02', - location: 'Washington, DC', - device_name: 'Chrome 123 on macOS', - disavowal_token: SecureRandom.hex, - ) - end - def new_device_sign_in_after_2fa UserMailer.with(user: user, email_address: email_address_record).new_device_sign_in_after_2fa( events: [ diff --git a/spec/mailers/user_mailer_spec.rb b/spec/mailers/user_mailer_spec.rb index 9684a21a1fb..2584f10647e 100644 --- a/spec/mailers/user_mailer_spec.rb +++ b/spec/mailers/user_mailer_spec.rb @@ -160,50 +160,6 @@ it_behaves_like 'an email that respects user email locale preference' end - describe '#new_device_sign_in' do - date = 'February 25, 2019 15:02' - location = 'Washington, DC' - device_name = 'Chrome ABC on macOS 123' - disavowal_token = 'asdf1234' - let(:mail) do - UserMailer.with(user: user, email_address: email_address).new_device_sign_in( - date: date, - location: location, - device_name: device_name, - disavowal_token: disavowal_token, - ) - end - - it_behaves_like 'a system email' - - it 'sends to the current email' do - expect(mail.to).to eq [user.email] - end - - it 'renders the subject' do - expect(mail.subject).to eq t('user_mailer.new_device_sign_in.subject', app_name: APP_NAME) - end - - it 'renders the body' do - expect(mail.html_part.body). - to have_content( - strip_tags( - t( - 'user_mailer.new_device_sign_in.info', - date: date, - location: location, - device_name: device_name, - app_name: APP_NAME, - ), - ), - ) - expect(mail.html_part.body).to include( - '/events/disavow?disavowal_token=asdf1234', - ) - expect_email_body_to_have_help_and_contact_links - end - end - describe '#new_device_sign_in_before_2fa' do let(:event) { create(:event, event_type: :sign_in_before_2fa, user:, device: create(:device)) } subject(:mail) do diff --git a/spec/services/user_alerts/alert_user_about_new_device_spec.rb b/spec/services/user_alerts/alert_user_about_new_device_spec.rb index c69aa11899a..7c295e605f1 100644 --- a/spec/services/user_alerts/alert_user_about_new_device_spec.rb +++ b/spec/services/user_alerts/alert_user_about_new_device_spec.rb @@ -6,75 +6,13 @@ let(:disavowal_token) { 'the_disavowal_token' } let(:device) { create(:device, user: user) } - describe '.call' do - context 'aggregated new device alerts enabled' do - before do - allow(IdentityConfig.store).to receive( - :feature_new_device_alert_aggregation_enabled, - ).and_return(true) - end - - it 'does not send any emails' do - described_class.call(event:, device:, disavowal_token:) - - expect_delivered_email_count(0) - end - end - - context 'aggregated new device alerts disabled' do - before do - allow(IdentityConfig.store).to receive( - :feature_new_device_alert_aggregation_enabled, - ).and_return(false) - end - - it 'sends an email to all confirmed email addresses' do - user.email_addresses.destroy_all - confirmed_email_addresses = create_list(:email_address, 2, user: user) - create(:email_address, user: user, confirmed_at: nil) - - described_class.call(event:, device:, disavowal_token:) - - expect_delivered_email_count(2) - expect_delivered_email( - to: [confirmed_email_addresses[0].email], - subject: t('user_mailer.new_device_sign_in.subject', app_name: APP_NAME), - body: [disavowal_token], - ) - expect_delivered_email( - to: [confirmed_email_addresses[1].email], - subject: t('user_mailer.new_device_sign_in.subject', app_name: APP_NAME), - body: [disavowal_token], - ) - end - end - end - describe '.schedule_alert' do subject(:result) { described_class.schedule_alert(event:) } - context 'aggregated new device alerts enabled' do - before do - allow(IdentityConfig.store).to receive(:feature_new_device_alert_aggregation_enabled). - and_return(true) - end - - it 'sets the user sign_in_new_device_at value to time of the given event' do - expect { result }.to change { user.reload.sign_in_new_device_at&.change(usec: 0) }. - from(nil). - to(event.created_at.change(usec: 0)) - end - end - - context 'aggregated new device alerts disabled' do - before do - allow(IdentityConfig.store).to receive(:feature_new_device_alert_aggregation_enabled). - and_return(false) - end - - it 'does not set sign_in_new_device_at value' do - expect { result }.not_to change { user.reload.sign_in_new_device_at&.change(usec: 0) } - end + it 'sets the user sign_in_new_device_at value to time of the given event' do + expect { result }.to change { user.reload.sign_in_new_device_at&.change(usec: 0) }. + from(nil). + to(event.created_at.change(usec: 0)) end end diff --git a/spec/services/user_event_creator_spec.rb b/spec/services/user_event_creator_spec.rb index 6f15de15628..30686caa949 100644 --- a/spec/services/user_event_creator_spec.rb +++ b/spec/services/user_event_creator_spec.rb @@ -61,20 +61,6 @@ expect(event.device.last_ip).to eq(ip_address) expect(event.device.last_used_at).to be_within(1).of(Time.zone.now) end - - it 'alerts the user if they have other devices' do - allow(UserAlerts::AlertUserAboutNewDevice).to receive(:call) - create(:device, user: user) - - event, _disavowal_token = subject.create_user_event(event_type, user) - - expect(event).to be_a(Event) - expect(UserAlerts::AlertUserAboutNewDevice).to have_received(:call).with( - event: user.events.first, - device: user.events.first.device, - disavowal_token: instance_of(String), - ) - end end context 'when no device exists' do @@ -106,20 +92,6 @@ to(lambda { |value| value == Device.last.cookie_uuid }) end end - - it 'alerts the user if they have other devices' do - allow(UserAlerts::AlertUserAboutNewDevice).to receive(:call) - create(:device, user: user) - - event, _disavowal_token = subject.create_user_event(event_type, user) - - expect(event).to be_a(Event) - expect(UserAlerts::AlertUserAboutNewDevice).to have_received(:call).with( - event: user.events.first, - device: user.events.first.device, - disavowal_token: instance_of(String), - ) - end end end diff --git a/spec/support/features/session_helper.rb b/spec/support/features/session_helper.rb index 8805558e97f..d2ffb80101a 100644 --- a/spec/support/features/session_helper.rb +++ b/spec/support/features/session_helper.rb @@ -49,7 +49,6 @@ def sign_up_and_2fa_ial1_user end def signin(email, password) - allow(UserMailer).to receive(:new_device_sign_in).and_call_original visit new_user_session_path set_hidden_field('platform_authenticator_available', 'true') fill_in_credentials_and_submit(email, password) @@ -57,7 +56,6 @@ def signin(email, password) end def signin_with_piv(user = user_with_piv_cac) - allow(UserMailer).to receive(:new_device_sign_in).and_call_original visit new_user_session_path click_on t('account.login.piv_cac') fill_in_piv_cac_credentials_and_submit(user) @@ -75,7 +73,6 @@ def signin_with_piv_error(error) end def signin_with_bad_piv - allow(UserMailer).to receive(:new_device_sign_in).and_call_original visit new_user_session_path click_on t('account.login.piv_cac') fill_in_bad_piv_cac_credentials_and_submit