diff --git a/app/controllers/api/internal/two_factor_authentication/auth_app_controller.rb b/app/controllers/api/internal/two_factor_authentication/auth_app_controller.rb index 4fb8f491c94..0d71428bc8e 100644 --- a/app/controllers/api/internal/two_factor_authentication/auth_app_controller.rb +++ b/app/controllers/api/internal/two_factor_authentication/auth_app_controller.rb @@ -6,6 +6,7 @@ module TwoFactorAuthentication class AuthAppController < ApplicationController include CsrfTokenConcern include ReauthenticationRequiredConcern + include MfaDeletionConcern before_action :render_unauthorized, unless: :recently_authenticated_2fa? @@ -37,10 +38,7 @@ def destroy analytics.auth_app_delete_submitted(**result) if result.success? - create_user_event(:authenticator_disabled) - revoke_remember_device(current_user) - event = PushNotification::RecoveryInformationChangedEvent.new(user: current_user) - PushNotification::HttpPush.deliver(event) + handle_successful_mfa_deletion(event_type: :authenticator_disabled) render json: { success: true } else render json: { success: false, error: result.first_error_message }, status: :bad_request diff --git a/app/controllers/api/internal/two_factor_authentication/piv_cac_controller.rb b/app/controllers/api/internal/two_factor_authentication/piv_cac_controller.rb index ca0425b1cf7..1079576606a 100644 --- a/app/controllers/api/internal/two_factor_authentication/piv_cac_controller.rb +++ b/app/controllers/api/internal/two_factor_authentication/piv_cac_controller.rb @@ -7,6 +7,7 @@ class PivCacController < ApplicationController include CsrfTokenConcern include ReauthenticationRequiredConcern include PivCacConcern + include MfaDeletionConcern before_action :render_unauthorized, unless: :recently_authenticated_2fa? @@ -38,9 +39,7 @@ def destroy analytics.piv_cac_delete_submitted(**result) if result.success? - create_user_event(:piv_cac_disabled) - revoke_remember_device(current_user) - deliver_push_notification + handle_successful_mfa_deletion(event_type: :piv_cac_disabled) clear_piv_cac_information render json: { success: true } else @@ -50,11 +49,6 @@ def destroy private - def deliver_push_notification - event = PushNotification::RecoveryInformationChangedEvent.new(user: current_user) - PushNotification::HttpPush.deliver(event) - end - def render_unauthorized render json: { error: 'Unauthorized' }, status: :unauthorized end diff --git a/app/controllers/api/internal/two_factor_authentication/webauthn_controller.rb b/app/controllers/api/internal/two_factor_authentication/webauthn_controller.rb index d34aef733e6..b5a291d949a 100644 --- a/app/controllers/api/internal/two_factor_authentication/webauthn_controller.rb +++ b/app/controllers/api/internal/two_factor_authentication/webauthn_controller.rb @@ -6,6 +6,7 @@ module TwoFactorAuthentication class WebauthnController < ApplicationController include CsrfTokenConcern include ReauthenticationRequiredConcern + include MfaDeletionConcern before_action :render_unauthorized, unless: :recently_authenticated_2fa? @@ -37,10 +38,7 @@ def destroy analytics.webauthn_delete_submitted(**result) if result.success? - create_user_event(:webauthn_key_removed) - revoke_remember_device(current_user) - event = PushNotification::RecoveryInformationChangedEvent.new(user: current_user) - PushNotification::HttpPush.deliver(event) + handle_successful_mfa_deletion(event_type: :webauthn_key_removed) render json: { success: true } else render json: { success: false, error: result.first_error_message }, status: :bad_request diff --git a/app/controllers/concerns/mfa_deletion_concern.rb b/app/controllers/concerns/mfa_deletion_concern.rb index 0f4c647aa2e..0b533882da1 100644 --- a/app/controllers/concerns/mfa_deletion_concern.rb +++ b/app/controllers/concerns/mfa_deletion_concern.rb @@ -4,7 +4,7 @@ module MfaDeletionConcern include RememberDeviceConcern def handle_successful_mfa_deletion(event_type:) - create_user_event(event_type) + create_user_event(event_type) if event_type revoke_remember_device(current_user) event = PushNotification::RecoveryInformationChangedEvent.new(user: current_user) PushNotification::HttpPush.deliver(event) diff --git a/app/controllers/users/auth_app_controller.rb b/app/controllers/users/auth_app_controller.rb index 34b07987fe5..32a6058dfd3 100644 --- a/app/controllers/users/auth_app_controller.rb +++ b/app/controllers/users/auth_app_controller.rb @@ -3,6 +3,7 @@ module Users class AuthAppController < ApplicationController include ReauthenticationRequiredConcern + include MfaDeletionConcern before_action :confirm_two_factor_authenticated before_action :confirm_recently_authenticated_2fa @@ -32,10 +33,7 @@ def destroy if result.success? flash[:success] = t('two_factor_authentication.auth_app.deleted') - create_user_event(:authenticator_disabled) - revoke_remember_device(current_user) - event = PushNotification::RecoveryInformationChangedEvent.new(user: current_user) - PushNotification::HttpPush.deliver(event) + handle_successful_mfa_deletion(event_type: :authenticator_disabled) redirect_to account_path else flash[:error] = result.first_error_message diff --git a/app/controllers/users/backup_code_setup_controller.rb b/app/controllers/users/backup_code_setup_controller.rb index dfc9619b8de..4d9ece3644a 100644 --- a/app/controllers/users/backup_code_setup_controller.rb +++ b/app/controllers/users/backup_code_setup_controller.rb @@ -4,6 +4,7 @@ module Users class BackupCodeSetupController < ApplicationController include TwoFactorAuthenticatableMethods include MfaSetupConcern + include MfaDeletionConcern include SecureHeadersConcern include ReauthenticationRequiredConcern @@ -58,10 +59,8 @@ def refreshed def delete current_user.backup_code_configurations.destroy_all - event = PushNotification::RecoveryInformationChangedEvent.new(user: current_user) - PushNotification::HttpPush.deliver(event) + handle_successful_mfa_deletion(event_type: nil) flash[:success] = t('notices.backup_codes_deleted') - revoke_remember_device(current_user) if in_multi_mfa_selection_flow? redirect_to authentication_methods_setup_path else diff --git a/app/controllers/users/edit_phone_controller.rb b/app/controllers/users/edit_phone_controller.rb index 9642cece9e4..9f829207829 100644 --- a/app/controllers/users/edit_phone_controller.rb +++ b/app/controllers/users/edit_phone_controller.rb @@ -4,6 +4,7 @@ module Users class EditPhoneController < ApplicationController include RememberDeviceConcern include ReauthenticationRequiredConcern + include MfaDeletionConcern before_action :confirm_two_factor_authenticated before_action :confirm_user_can_edit_phone @@ -29,9 +30,7 @@ def update def destroy track_deletion_analytics_event phone_configuration.destroy! - event = PushNotification::RecoveryInformationChangedEvent.new(user: current_user) - PushNotification::HttpPush.deliver(event) - revoke_remember_device(current_user) + handle_successful_mfa_deletion(event_type: :phone_removed) flash[:success] = t('two_factor_authentication.phone.delete.success') redirect_to account_url end @@ -55,7 +54,6 @@ def track_deletion_analytics_event success: true, phone_configuration_id: phone_configuration.id, ) - create_user_event(:phone_removed) end def phone_configuration diff --git a/app/controllers/users/piv_cac_controller.rb b/app/controllers/users/piv_cac_controller.rb index ec87ff792ad..ff157c17e7c 100644 --- a/app/controllers/users/piv_cac_controller.rb +++ b/app/controllers/users/piv_cac_controller.rb @@ -4,6 +4,7 @@ module Users class PivCacController < ApplicationController include ReauthenticationRequiredConcern include PivCacConcern + include MfaDeletionConcern before_action :confirm_two_factor_authenticated before_action :confirm_recently_authenticated_2fa @@ -33,9 +34,7 @@ def destroy analytics.piv_cac_delete_submitted(**result) if result.success? - create_user_event(:piv_cac_disabled) - revoke_remember_device(current_user) - deliver_push_notification + handle_successful_mfa_deletion(event_type: :piv_cac_disabled) clear_piv_cac_information flash[:success] = presenter.delete_success_alert_text @@ -48,11 +47,6 @@ def destroy private - def deliver_push_notification - event = PushNotification::RecoveryInformationChangedEvent.new(user: current_user) - PushNotification::HttpPush.deliver(event) - end - def form @form ||= form_class.new(user: current_user, configuration_id: params[:id]) end diff --git a/app/controllers/users/webauthn_controller.rb b/app/controllers/users/webauthn_controller.rb index c7dee0cf2a8..60cb69f8e47 100644 --- a/app/controllers/users/webauthn_controller.rb +++ b/app/controllers/users/webauthn_controller.rb @@ -3,6 +3,7 @@ module Users class WebauthnController < ApplicationController include ReauthenticationRequiredConcern + include MfaDeletionConcern before_action :confirm_two_factor_authenticated before_action :confirm_recently_authenticated_2fa @@ -33,10 +34,7 @@ def destroy if result.success? flash[:success] = presenter.delete_success_alert_text - create_user_event(:webauthn_key_removed) - revoke_remember_device(current_user) - event = PushNotification::RecoveryInformationChangedEvent.new(user: current_user) - PushNotification::HttpPush.deliver(event) + handle_successful_mfa_deletion(event_type: :webauthn_key_removed) redirect_to account_path else flash[:error] = result.first_error_message diff --git a/spec/controllers/concerns/mfa_deletion_concern_spec.rb b/spec/controllers/concerns/mfa_deletion_concern_spec.rb index 0193cbffaae..45bd0ccf3f5 100644 --- a/spec/controllers/concerns/mfa_deletion_concern_spec.rb +++ b/spec/controllers/concerns/mfa_deletion_concern_spec.rb @@ -38,5 +38,15 @@ result end + + context 'with nil event_type argument' do + let(:event_type) { nil } + + it 'does not create user event' do + expect(controller).not_to receive(:create_user_event) + + result + end + end end end