From 33b061975196e71562794da3ef0c3b0ee2bc0ada Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Thu, 1 Feb 2024 09:24:40 -0500 Subject: [PATCH] Remove routes associated with legacy WebAuthn deletion changelog: Internal, Code Quality, Remove unused WebAuthn deletion routes --- .../users/webauthn_setup_controller.rb | 55 -------------- .../users/webauthn_setup/delete.html.erb | 33 -------- config/locales/account/en.yml | 2 - config/locales/account/es.yml | 2 - config/locales/account/fr.yml | 3 - config/locales/forms/en.yml | 8 -- config/locales/forms/es.yml | 9 --- config/locales/forms/fr.yml | 10 --- config/locales/notices/en.yml | 2 - config/locales/notices/es.yml | 2 - config/locales/notices/fr.yml | 3 - config/routes.rb | 4 - .../users/webauthn_setup_controller_spec.rb | 76 ------------------- 13 files changed, 209 deletions(-) delete mode 100644 app/views/users/webauthn_setup/delete.html.erb diff --git a/app/controllers/users/webauthn_setup_controller.rb b/app/controllers/users/webauthn_setup_controller.rb index 298fc37f259..d8bd370c509 100644 --- a/app/controllers/users/webauthn_setup_controller.rb +++ b/app/controllers/users/webauthn_setup_controller.rb @@ -90,28 +90,6 @@ def confirm end end - def delete - if MfaPolicy.new(current_user).multiple_factors_enabled? - handle_successful_delete - else - handle_failed_delete - end - redirect_to account_two_factor_authentication_path - end - - def show_delete - @webauthn = WebauthnConfiguration.where( - user_id: current_user.id, id: delete_params[:id], - ).first - - if @webauthn - render 'users/webauthn_setup/delete' - else - flash[:error] = t('errors.general') - redirect_back fallback_location: new_user_session_url, allow_other_host: false - end - end - private def validate_existing_platform_authenticator @@ -142,35 +120,6 @@ def exclude_credentials current_user.webauthn_configurations.map(&:credential_id) end - def handle_successful_delete - webauthn = WebauthnConfiguration.find_by(user_id: current_user.id, id: delete_params[:id]) - return unless webauthn - - create_user_event(:webauthn_key_removed) - webauthn.destroy - revoke_remember_device(current_user) - event = PushNotification::RecoveryInformationChangedEvent.new(user: current_user) - PushNotification::HttpPush.deliver(event) - if webauthn.platform_authenticator - flash[:success] = t('notices.webauthn_platform_deleted') - else - flash[:success] = t('notices.webauthn_deleted') - end - track_delete(success: true, platform_authenticator: webauthn.platform_authenticator?) - end - - def handle_failed_delete - track_delete(success: false, platform_authenticator: nil) - end - - def track_delete(success:, platform_authenticator:) - analytics.webauthn_delete_submitted( - success:, - configuration_id: delete_params[:id], - platform_authenticator:, - ) - end - def save_challenge_in_session credential_creation_options = WebAuthn::Credential.options_for_create(user: current_user) user_session[:webauthn_challenge] = credential_creation_options.challenge.bytes.to_a @@ -224,9 +173,5 @@ def confirm_params :transports, ) end - - def delete_params - params.permit(:id) - end end end diff --git a/app/views/users/webauthn_setup/delete.html.erb b/app/views/users/webauthn_setup/delete.html.erb deleted file mode 100644 index ca15213da93..00000000000 --- a/app/views/users/webauthn_setup/delete.html.erb +++ /dev/null @@ -1,33 +0,0 @@ -<% if @webauthn.platform_authenticator %> - <% self.title = t('forms.webauthn_platform_delete.confirm') %> -<% else %> - <% self.title = t('forms.webauthn_delete.confirm') %> -<% end %> - -<%= render AlertIconComponent.new(icon_name: :warning, class: 'display-block margin-bottom-4') %> - -<%= render PageHeadingComponent.new do %> - <% if @webauthn.platform_authenticator %> - <%= t('forms.webauthn_platform_delete.confirm') %> - <% else %> - <%= t('forms.webauthn_delete.confirm') %> - <% end %> -<% end %> - -<% if @webauthn.platform_authenticator %> -

<%= t('forms.webauthn_platform_delete.caution', app_name: APP_NAME) %>

-<% else %> -

<%= t('forms.webauthn_delete.caution', app_name: APP_NAME) %>

-<% end %> - -<%= button_to(webauthn_setup_path(id: params[:id]), method: :delete, class: 'usa-button usa-button--big usa-button--wide margin-top-5 margin-bottom-2') do %> - <% if @webauthn.platform_authenticator %> - <%= t('account.index.webauthn_platform_confirm_delete') %> - <% else %> - <%= t('account.index.webauthn_confirm_delete') %> - <% end %> -<% end %> - -<%= link_to t('links.cancel'), - account_two_factor_authentication_path, - class: 'usa-button usa-button--big usa-button--wide usa-button--outline' %> diff --git a/config/locales/account/en.yml b/config/locales/account/en.yml index 8c51de05e9a..95819a1fc2e 100644 --- a/config/locales/account/en.yml +++ b/config/locales/account/en.yml @@ -54,10 +54,8 @@ en: success: We verified your information webauthn: Security key webauthn_add: Add security key - webauthn_confirm_delete: Yes, remove key webauthn_platform: Face or touch unlock webauthn_platform_add: Add face or touch unlock - webauthn_platform_confirm_delete: Yes, remove face or touch unlock items: delete_your_account: Delete your account personal_key: Personal key diff --git a/config/locales/account/es.yml b/config/locales/account/es.yml index daf1a7c82bf..a2ffe35568e 100644 --- a/config/locales/account/es.yml +++ b/config/locales/account/es.yml @@ -55,10 +55,8 @@ es: success: Verificamos tu información webauthn: Clave de seguridad webauthn_add: Añadir clave de seguridad - webauthn_confirm_delete: Si quitar la llave webauthn_platform: El desbloqueo facial o táctil webauthn_platform_add: Añadir el desbloqueo facial o táctil - webauthn_platform_confirm_delete: Si, quitar el desbloqueo facial o táctil items: delete_your_account: Eliminar su cuenta personal_key: Clave personal diff --git a/config/locales/account/fr.yml b/config/locales/account/fr.yml index 6fb436a6fb8..07f8887835a 100644 --- a/config/locales/account/fr.yml +++ b/config/locales/account/fr.yml @@ -58,11 +58,8 @@ fr: success: Nous avons vérifié vos informations webauthn: Clé de sécurité webauthn_add: Ajouter une clé de sécurité - webauthn_confirm_delete: Oui, supprimer la clé webauthn_platform: Le déverouillage facial ou déverrouillage par empreinte digitale webauthn_platform_add: Ajouter le déverouillage facial ou déverrouillage par empreinte digitale - webauthn_platform_confirm_delete: Oui, supprimer le déverouillage facial ou - déverrouillage par empreinte digitale items: delete_your_account: Supprimer votre compte personal_key: Clé personnelle diff --git a/config/locales/forms/en.yml b/config/locales/forms/en.yml index 290ffd074df..618ccbec39b 100644 --- a/config/locales/forms/en.yml +++ b/config/locales/forms/en.yml @@ -125,14 +125,6 @@ en: try_again: Use another phone number validation: required_checkbox: Please check this box to continue - webauthn_delete: - caution: If you remove your security key you won’t be able to use it to access - your %{app_name} account. - confirm: Are you sure you want to remove your security key? - webauthn_platform_delete: - caution: If you remove face or touch unlock you won’t be able to use it to - access your %{app_name} account. - confirm: Are you sure you want to remove face or touch unlock? webauthn_platform_setup: continue: Continue info_text: You’ll need to set up an additional authentication method after you diff --git a/config/locales/forms/es.yml b/config/locales/forms/es.yml index a4c1f511f86..123d5438dd8 100644 --- a/config/locales/forms/es.yml +++ b/config/locales/forms/es.yml @@ -132,15 +132,6 @@ es: try_again: Use otro número de teléfono. validation: required_checkbox: Marque esta casilla para continuar - webauthn_delete: - caution: Si elimina su clave de seguridad, no podrá usarla para acceder a su - cuenta %{app_name}. - confirm: '¿Estás seguro de que quieres eliminar tu clave de seguridad?' - webauthn_platform_delete: - caution: Si elimina su desbloqueo facial o táctil, no podrá usarla para acceder - a su cuenta %{app_name}. - confirm: '¿Estás seguro de que quieres eliminar tu desbloqueo facial o táctil de - seguridad?' webauthn_platform_setup: continue: Continuar info_text: Tendrá que configurar un método de autenticación adicional después de diff --git a/config/locales/forms/fr.yml b/config/locales/forms/fr.yml index c9e5fcd7fe5..23d0a94cc58 100644 --- a/config/locales/forms/fr.yml +++ b/config/locales/forms/fr.yml @@ -134,16 +134,6 @@ fr: try_again: Utilisez un autre numéro de téléphone validation: required_checkbox: Veuillez cocher cette case pour continuer - webauthn_delete: - caution: Si vous supprimez votre clé de sécurité, vous ne pourrez plus - l’utiliser pour accéder à votre compte %{app_name}. - confirm: Êtes-vous sûr de vouloir supprimer votre clé de sécurité? - webauthn_platform_delete: - caution: Si vous supprimez votre déverouillage facial ou déverrouillage par - empreinte digitale, vous ne pourrez plus l’utiliser pour accéder à votre - compte %{app_name}. - confirm: Êtes-vous sûr de vouloir supprimer votre déverouillage facial ou - déverrouillage par empreinte digitale? webauthn_platform_setup: continue: Continuer info_text: Vous aurez besoin de configurer une méthode d’authentification diff --git a/config/locales/notices/en.yml b/config/locales/notices/en.yml index e588579a4ba..c621d34246d 100644 --- a/config/locales/notices/en.yml +++ b/config/locales/notices/en.yml @@ -59,7 +59,5 @@ en: link: use a different email address text_html: Or, %{link_html} webauthn_configured: A security key was added to your account. - webauthn_deleted: Your security key was deleted from your account. webauthn_platform_configured: You used your device’s screen lock to add face or touch unlock to your account. - webauthn_platform_deleted: Face or touch unlock was deleted from your account. diff --git a/config/locales/notices/es.yml b/config/locales/notices/es.yml index 6113688a5e3..180544da64e 100644 --- a/config/locales/notices/es.yml +++ b/config/locales/notices/es.yml @@ -61,7 +61,5 @@ es: link: use un email diferente text_html: O %{link_html} webauthn_configured: Una llave de seguridad fue agregada a tu cuenta. - webauthn_deleted: Tu llave de seguridad fue eliminada de tu cuenta. webauthn_platform_configured: Usó el bloqueo de pantalla de su dispositivo para agregar el desbloqueo facial o táctil a su cuenta. - webauthn_platform_deleted: Desbloqueo facial o táctil fue eliminada de tu cuenta. diff --git a/config/locales/notices/fr.yml b/config/locales/notices/fr.yml index 06571ca65bb..0e366e5db32 100644 --- a/config/locales/notices/fr.yml +++ b/config/locales/notices/fr.yml @@ -64,9 +64,6 @@ fr: link: utilisez une adresse courriel différente text_html: Ou %{link_html} webauthn_configured: Une clé de sécurité a été ajoutée à votre compte. - webauthn_deleted: Votre clé de sécurité a été supprimée de votre compte. webauthn_platform_configured: Vous avez utilisé le verrouillage de l’écran de votre appareil pour ajouter le déverrouillage facial ou tactile à votre compte. - webauthn_platform_deleted: Déverouillage facial ou déverrouillage par empreinte - digitale a été ajoutée à votre compte. diff --git a/config/routes.rb b/config/routes.rb index ed8509775e4..4f74d0fce9f 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -235,10 +235,6 @@ get '/webauthn_setup' => 'users/webauthn_setup#new', as: :webauthn_setup patch '/webauthn_setup' => 'users/webauthn_setup#confirm' - # Deprecated routes: Remove once LG-11454 is fully deployed to production. - delete '/webauthn_setup' => 'users/webauthn_setup#delete' - get '/webauthn_setup_delete' => 'users/webauthn_setup#show_delete' - delete '/authenticator_setup' => 'users/totp_setup#disable', as: :disable_totp get '/authenticator_setup' => 'users/totp_setup#new' patch '/authenticator_setup' => 'users/totp_setup#confirm' diff --git a/spec/controllers/users/webauthn_setup_controller_spec.rb b/spec/controllers/users/webauthn_setup_controller_spec.rb index 8c934b4dd05..f230f738759 100644 --- a/spec/controllers/users/webauthn_setup_controller_spec.rb +++ b/spec/controllers/users/webauthn_setup_controller_spec.rb @@ -122,82 +122,6 @@ patch :confirm, params: params end end - - describe 'delete' do - let(:webauthn_configuration) { create(:webauthn_configuration, user: user) } - - it 'creates a webauthn key removed event' do - delete :delete, params: { id: webauthn_configuration.id } - - expect(response).to redirect_to(account_two_factor_authentication_path) - expect(flash.now[:success]).to eq t('notices.webauthn_deleted') - expect(WebauthnConfiguration.count).to eq(0) - expect( - Event.where( - user_id: controller.current_user.id, - event_type: :webauthn_key_removed, ip: '0.0.0.0' - ).count, - ).to eq 1 - end - - it 'revokes remember device cookies' do - expect(user.remember_device_revoked_at).to eq nil - freeze_time do - delete :delete, params: { id: webauthn_configuration.id } - expect(user.reload.remember_device_revoked_at).to eq Time.zone.now - end - end - - it 'tracks the delete in analytics' do - delete :delete, params: { id: webauthn_configuration.id } - - expect(@analytics).to have_logged_event( - :webauthn_delete_submitted, - success: true, - error_details: nil, - configuration_id: webauthn_configuration.id.to_s, - platform_authenticator: false, - ) - end - - it 'sends a recovery information changed event' do - expect(PushNotification::HttpPush).to receive(:deliver). - with(PushNotification::RecoveryInformationChangedEvent.new(user: user)) - - delete :delete, params: { id: webauthn_configuration.id } - end - - context 'when authenticator is the sole authentication method' do - let(:user) { create(:user) } - - it 'tracks the delete in analytics' do - delete :delete, params: { id: webauthn_configuration.id } - - expect(@analytics).to have_logged_event( - :webauthn_delete_submitted, - success: false, - error_details: nil, - configuration_id: webauthn_configuration.id.to_s, - platform_authenticator: nil, - ) - end - end - end - - describe 'show_delete' do - let(:webauthn_configuration) { create(:webauthn_configuration, user: user) } - - it 'renders page when configuration exists' do - get :show_delete, params: { id: webauthn_configuration.id } - expect(response).to render_template :delete - end - - it 'redirects when the configuration does not exist' do - get :show_delete, params: { id: '_' } - expect(response).to redirect_to(new_user_session_url) - expect(flash[:error]).to eq t('errors.general') - end - end end describe 'when signed in and account creation' do