From f6da4d5e8e86aa9b3fe025a48f344f7fd558fb5e Mon Sep 17 00:00:00 2001 From: kevinsmaster5 Date: Mon, 3 Jul 2023 15:36:31 -0400 Subject: [PATCH 1/9] changelog: User-Facing Improvements, Error screens, Removed pathway and error page for stand alone error messaging --- .../webauthn_verification_controller.rb | 21 ++++++++----------- .../webauthn_verification/error.html.erb | 13 ------------ .../webauthn_verification_controller_spec.rb | 3 ++- 3 files changed, 11 insertions(+), 26 deletions(-) delete mode 100644 app/views/two_factor_authentication/webauthn_verification/error.html.erb diff --git a/app/controllers/two_factor_authentication/webauthn_verification_controller.rb b/app/controllers/two_factor_authentication/webauthn_verification_controller.rb index 01d87c985d6..95f931338d6 100644 --- a/app/controllers/two_factor_authentication/webauthn_verification_controller.rb +++ b/app/controllers/two_factor_authentication/webauthn_verification_controller.rb @@ -56,18 +56,15 @@ def handle_valid_webauthn def handle_invalid_webauthn is_platform_auth = params[:platform].to_s == 'true' if is_platform_auth - if presenter_for_two_factor_authentication_method.multiple_factors_enabled? - flash[:error] = t( - 'two_factor_authentication.webauthn_error.multiple_methods', - link: view_context.link_to( - t('two_factor_authentication.webauthn_error.additional_methods_link'), - login_two_factor_options_path, - ), - ) - redirect_to login_two_factor_webauthn_url(platform: params[:platform]) - else - redirect_to login_two_factor_webauthn_error_url - end + flash[:error] = t( + 'two_factor_authentication.webauthn_error.multiple_methods', + link: view_context.link_to( + t('two_factor_authentication.webauthn_error.additional_methods_link'), + login_two_factor_options_path, + ), + ) + redirect_to login_two_factor_webauthn_url(platform: params[:platform]) + else flash[:error] = t('errors.general') redirect_to login_two_factor_webauthn_url diff --git a/app/views/two_factor_authentication/webauthn_verification/error.html.erb b/app/views/two_factor_authentication/webauthn_verification/error.html.erb deleted file mode 100644 index 661315946fc..00000000000 --- a/app/views/two_factor_authentication/webauthn_verification/error.html.erb +++ /dev/null @@ -1,13 +0,0 @@ -<%= render( - 'idv/shared/error', - heading: t('two_factor_authentication.webauthn_error.title'), - options: [ - { - text: t('links.contact_support', app_name: APP_NAME), - url: MarketingSite.contact_url, - new_tab: true, - }, - ], - ) do %> -

<%= t('two_factor_authentication.webauthn_error.error_page_text') %>

-<% end %> diff --git a/spec/controllers/two_factor_authentication/webauthn_verification_controller_spec.rb b/spec/controllers/two_factor_authentication/webauthn_verification_controller_spec.rb index 9109b47b4e5..2c4314e5bcc 100644 --- a/spec/controllers/two_factor_authentication/webauthn_verification_controller_spec.rb +++ b/spec/controllers/two_factor_authentication/webauthn_verification_controller_spec.rb @@ -238,7 +238,8 @@ it 'redirects to webauthn error page ' do patch :confirm, params: params - expect(response).to redirect_to login_two_factor_webauthn_error_url + expect(response). + to redirect_to login_two_factor_webauthn_url(platform: params[:platform]) end end end From d6e1af7af1cb7e64baaf62ef6737fe590c519f2a Mon Sep 17 00:00:00 2001 From: kevinsmaster5 Date: Wed, 5 Jul 2023 10:16:36 -0400 Subject: [PATCH 2/9] remove unneeded context block --- .../webauthn_verification_controller_spec.rb | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/spec/controllers/two_factor_authentication/webauthn_verification_controller_spec.rb b/spec/controllers/two_factor_authentication/webauthn_verification_controller_spec.rb index 2c4314e5bcc..b9359a7ca67 100644 --- a/spec/controllers/two_factor_authentication/webauthn_verification_controller_spec.rb +++ b/spec/controllers/two_factor_authentication/webauthn_verification_controller_spec.rb @@ -228,20 +228,6 @@ patch :confirm, params: params end end - - context 'User only has webauthn as an MFA method' do - before do - allow_any_instance_of(TwoFactorAuthCode::WebauthnAuthenticationPresenter). - to receive(:multiple_factors_enabled?). - and_return(false) - end - - it 'redirects to webauthn error page ' do - patch :confirm, params: params - expect(response). - to redirect_to login_two_factor_webauthn_url(platform: params[:platform]) - end - end end end end From 4eaaf873b431af477221f7d11801c474c5cedb74 Mon Sep 17 00:00:00 2001 From: kevinsmaster5 Date: Wed, 5 Jul 2023 10:59:42 -0400 Subject: [PATCH 3/9] flatten context out defining multiple MFA' --- .../webauthn_verification_controller_spec.rb | 90 +++++++++---------- 1 file changed, 44 insertions(+), 46 deletions(-) diff --git a/spec/controllers/two_factor_authentication/webauthn_verification_controller_spec.rb b/spec/controllers/two_factor_authentication/webauthn_verification_controller_spec.rb index b9359a7ca67..9e56273bd80 100644 --- a/spec/controllers/two_factor_authentication/webauthn_verification_controller_spec.rb +++ b/spec/controllers/two_factor_authentication/webauthn_verification_controller_spec.rb @@ -181,52 +181,50 @@ controller.user_session[:webauthn_challenge] = webauthn_challenge end - context 'User has multiple MFA options' do - let(:view_context) { ActionController::Base.new.view_context } - before do - allow_any_instance_of(TwoFactorAuthCode::WebauthnAuthenticationPresenter). - to receive(:multiple_factors_enabled?). - and_return(true) - create( - :webauthn_configuration, - :platform_authenticator, - user: controller.current_user, - credential_id: credential_id, - credential_public_key: credential_public_key, - ) - end - - it 'redirects to webauthn show page' do - patch :confirm, params: params - expect(response).to redirect_to login_two_factor_webauthn_url(platform: true) - expect(subject.user_session[:auth_method]).to eq nil - expect(subject.user_session[TwoFactorAuthenticatable::NEED_AUTHENTICATION]).to eq true - end - - it 'displays flash error message' do - patch :confirm, params: params - expect(flash[:error]).to eq t( - 'two_factor_authentication.webauthn_error.multiple_methods', - link: view_context.link_to( - t('two_factor_authentication.webauthn_error.additional_methods_link'), - login_two_factor_options_path, - ), - ) - end - - it 'logs an event with error details' do - expect(@analytics).to receive(:track_mfa_submit_event).with( - hash_including( - success: false, - error_details: { webauthn_error: [webauthn_error] }, - context: UserSessionContext::AUTHENTICATION_CONTEXT, - multi_factor_auth_method: 'webauthn_platform', - webauthn_configuration_id: controller.current_user.webauthn_configurations.first.id, - ), - ) - - patch :confirm, params: params - end + let(:view_context) { ActionController::Base.new.view_context } + before do + allow_any_instance_of(TwoFactorAuthCode::WebauthnAuthenticationPresenter). + to receive(:multiple_factors_enabled?). + and_return(true) + create( + :webauthn_configuration, + user: controller.current_user, + credential_id: credential_id, + credential_public_key: credential_public_key, + platform_authenticator: true, + ) + end + + it 'redirects to webauthn show page' do + patch :confirm, params: params + expect(response).to redirect_to login_two_factor_webauthn_url(platform: true) + expect(subject.user_session[:auth_method]).to eq nil + expect(subject.user_session[TwoFactorAuthenticatable::NEED_AUTHENTICATION]).to eq true + end + + it 'displays flash error message' do + patch :confirm, params: params + expect(flash[:error]).to eq t( + 'two_factor_authentication.webauthn_error.multiple_methods', + link: view_context.link_to( + t('two_factor_authentication.webauthn_error.additional_methods_link'), + login_two_factor_options_path, + ), + ) + end + + it 'logs an event with error details' do + expect(@analytics).to receive(:track_mfa_submit_event).with( + hash_including( + success: false, + error_details: { webauthn_error: [webauthn_error] }, + context: UserSessionContext::AUTHENTICATION_CONTEXT, + multi_factor_auth_method: 'webauthn_platform', + webauthn_configuration_id: controller.current_user.webauthn_configurations.first.id, + ), + ) + + patch :confirm, params: params end end end From 42f3f9e7e464a49af16430545b309f92bb3cc991 Mon Sep 17 00:00:00 2001 From: kevinsmaster5 Date: Wed, 5 Jul 2023 13:57:46 -0400 Subject: [PATCH 4/9] remove unused i18n keys from yml files --- config/locales/two_factor_authentication/en.yml | 3 --- config/locales/two_factor_authentication/es.yml | 3 --- config/locales/two_factor_authentication/fr.yml | 4 ---- 3 files changed, 10 deletions(-) diff --git a/config/locales/two_factor_authentication/en.yml b/config/locales/two_factor_authentication/en.yml index da15da43a58..41123a93a86 100644 --- a/config/locales/two_factor_authentication/en.yml +++ b/config/locales/two_factor_authentication/en.yml @@ -185,10 +185,7 @@ en: to access your information. webauthn_error: additional_methods_link: choose another authentication method - error_page_text: You have face or touch unlock enabled for this account. Use the - same device and browser profile each time. multiple_methods: Face or touch unlock was unsuccessful. Please try again or %{link}. - title: We can’t identify your device webauthn_fallback: question: Don’t have your security key available? webauthn_header_text: Connect your security key diff --git a/config/locales/two_factor_authentication/es.yml b/config/locales/two_factor_authentication/es.yml index 78e35c0f696..1c68f412dc4 100644 --- a/config/locales/two_factor_authentication/es.yml +++ b/config/locales/two_factor_authentication/es.yml @@ -199,11 +199,8 @@ es: identificación de empleado gubernamental (PIV/CAC) webauthn_error: additional_methods_link: elija otro método de autenticación - error_page_text: Tiene habilitado el desbloqueo facial o táctil para esta - cuenta. Utilice cada vez el mismo dispositivo y perfil de navegador. multiple_methods: El desbloqueo facial o táctil no fue exitoso. Por favor, inténtelo de nuevo o %{link}. - title: No podemos reconocer su dispositivo webauthn_fallback: question: '¿No dispone de su llave de seguridad?' webauthn_header_text: Conecte su llave de seguridad diff --git a/config/locales/two_factor_authentication/fr.yml b/config/locales/two_factor_authentication/fr.yml index bf330768ad2..7acc7d276ae 100644 --- a/config/locales/two_factor_authentication/fr.yml +++ b/config/locales/two_factor_authentication/fr.yml @@ -205,12 +205,8 @@ fr: gouvernement (PIC/CAC) pour accéder à vos informations. webauthn_error: additional_methods_link: choisir une autre méthode d’authentification - error_page_text: Vous avez activé le déverrouillage facial ou tactile pour ce - compte. Utilisez le même appareil et le même profil de navigateur chaque - fois. multiple_methods: Le déverrouillage facial ou tactile n’a pas fonctionné. Veuillez réessayer ou %{link}. - title: Nous ne pouvons pas identifier votre appareil webauthn_fallback: question: Vous n’avez pas votre clé de sécurité avec vous? webauthn_header_text: Connectez votre clé de sécurité From 8a44fb6826b8f5b13711d982139eee2ea8a53ab6 Mon Sep 17 00:00:00 2001 From: kevinsmaster5 Date: Thu, 6 Jul 2023 15:22:05 -0400 Subject: [PATCH 5/9] undelete error view and the related locale phrases. add to error method in the controller the redirect to please try again view --- .../webauthn_verification_controller.rb | 11 ++++++++++- .../webauthn_verification/error.html.erb | 13 +++++++++++++ config/locales/two_factor_authentication/en.yml | 3 +++ config/locales/two_factor_authentication/es.yml | 3 +++ config/locales/two_factor_authentication/fr.yml | 4 ++++ 5 files changed, 33 insertions(+), 1 deletion(-) create mode 100644 app/views/two_factor_authentication/webauthn_verification/error.html.erb diff --git a/app/controllers/two_factor_authentication/webauthn_verification_controller.rb b/app/controllers/two_factor_authentication/webauthn_verification_controller.rb index 95f931338d6..f500d2d4ac1 100644 --- a/app/controllers/two_factor_authentication/webauthn_verification_controller.rb +++ b/app/controllers/two_factor_authentication/webauthn_verification_controller.rb @@ -27,7 +27,16 @@ def confirm handle_webauthn_result(result) end - def error; end + def error + flash[:error] = t( + 'two_factor_authentication.webauthn_error.multiple_methods', + link: view_context.link_to( + t('two_factor_authentication.webauthn_error.additional_methods_link'), + login_two_factor_options_path, + ), + ) + redirect_to login_two_factor_webauthn_url(platform: params[:platform]) + end private diff --git a/app/views/two_factor_authentication/webauthn_verification/error.html.erb b/app/views/two_factor_authentication/webauthn_verification/error.html.erb new file mode 100644 index 00000000000..661315946fc --- /dev/null +++ b/app/views/two_factor_authentication/webauthn_verification/error.html.erb @@ -0,0 +1,13 @@ +<%= render( + 'idv/shared/error', + heading: t('two_factor_authentication.webauthn_error.title'), + options: [ + { + text: t('links.contact_support', app_name: APP_NAME), + url: MarketingSite.contact_url, + new_tab: true, + }, + ], + ) do %> +

<%= t('two_factor_authentication.webauthn_error.error_page_text') %>

+<% end %> diff --git a/config/locales/two_factor_authentication/en.yml b/config/locales/two_factor_authentication/en.yml index 41123a93a86..eed55ba435d 100644 --- a/config/locales/two_factor_authentication/en.yml +++ b/config/locales/two_factor_authentication/en.yml @@ -184,8 +184,11 @@ en: You need to verify your identity using a government employee ID (PIV/CAC) to access your information. webauthn_error: + error_page_text: You have face or touch unlock enabled for this account. Use the + same device and browser profile each time. additional_methods_link: choose another authentication method multiple_methods: Face or touch unlock was unsuccessful. Please try again or %{link}. + title: We can’t identify your device webauthn_fallback: question: Don’t have your security key available? webauthn_header_text: Connect your security key diff --git a/config/locales/two_factor_authentication/es.yml b/config/locales/two_factor_authentication/es.yml index 1c68f412dc4..78e35c0f696 100644 --- a/config/locales/two_factor_authentication/es.yml +++ b/config/locales/two_factor_authentication/es.yml @@ -199,8 +199,11 @@ es: identificación de empleado gubernamental (PIV/CAC) webauthn_error: additional_methods_link: elija otro método de autenticación + error_page_text: Tiene habilitado el desbloqueo facial o táctil para esta + cuenta. Utilice cada vez el mismo dispositivo y perfil de navegador. multiple_methods: El desbloqueo facial o táctil no fue exitoso. Por favor, inténtelo de nuevo o %{link}. + title: No podemos reconocer su dispositivo webauthn_fallback: question: '¿No dispone de su llave de seguridad?' webauthn_header_text: Conecte su llave de seguridad diff --git a/config/locales/two_factor_authentication/fr.yml b/config/locales/two_factor_authentication/fr.yml index 7acc7d276ae..bf330768ad2 100644 --- a/config/locales/two_factor_authentication/fr.yml +++ b/config/locales/two_factor_authentication/fr.yml @@ -205,8 +205,12 @@ fr: gouvernement (PIC/CAC) pour accéder à vos informations. webauthn_error: additional_methods_link: choisir une autre méthode d’authentification + error_page_text: Vous avez activé le déverrouillage facial ou tactile pour ce + compte. Utilisez le même appareil et le même profil de navigateur chaque + fois. multiple_methods: Le déverrouillage facial ou tactile n’a pas fonctionné. Veuillez réessayer ou %{link}. + title: Nous ne pouvons pas identifier votre appareil webauthn_fallback: question: Vous n’avez pas votre clé de sécurité avec vous? webauthn_header_text: Connectez votre clé de sécurité From f0c772b79f24064d72ec59378066e7d354ffa1c5 Mon Sep 17 00:00:00 2001 From: kevinsmaster5 Date: Thu, 6 Jul 2023 15:23:55 -0400 Subject: [PATCH 6/9] fix lint errors --- .../webauthn_verification_controller.rb | 12 ++++++------ config/locales/two_factor_authentication/en.yml | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/app/controllers/two_factor_authentication/webauthn_verification_controller.rb b/app/controllers/two_factor_authentication/webauthn_verification_controller.rb index f500d2d4ac1..edf38ed91a0 100644 --- a/app/controllers/two_factor_authentication/webauthn_verification_controller.rb +++ b/app/controllers/two_factor_authentication/webauthn_verification_controller.rb @@ -29,12 +29,12 @@ def confirm def error flash[:error] = t( - 'two_factor_authentication.webauthn_error.multiple_methods', - link: view_context.link_to( - t('two_factor_authentication.webauthn_error.additional_methods_link'), - login_two_factor_options_path, - ), - ) + 'two_factor_authentication.webauthn_error.multiple_methods', + link: view_context.link_to( + t('two_factor_authentication.webauthn_error.additional_methods_link'), + login_two_factor_options_path, + ), + ) redirect_to login_two_factor_webauthn_url(platform: params[:platform]) end diff --git a/config/locales/two_factor_authentication/en.yml b/config/locales/two_factor_authentication/en.yml index eed55ba435d..da15da43a58 100644 --- a/config/locales/two_factor_authentication/en.yml +++ b/config/locales/two_factor_authentication/en.yml @@ -184,9 +184,9 @@ en: You need to verify your identity using a government employee ID (PIV/CAC) to access your information. webauthn_error: + additional_methods_link: choose another authentication method error_page_text: You have face or touch unlock enabled for this account. Use the same device and browser profile each time. - additional_methods_link: choose another authentication method multiple_methods: Face or touch unlock was unsuccessful. Please try again or %{link}. title: We can’t identify your device webauthn_fallback: From 39d23371619f7d18143ac2c3b806421906957304 Mon Sep 17 00:00:00 2001 From: kevinsmaster5 Date: Thu, 6 Jul 2023 16:31:05 -0400 Subject: [PATCH 7/9] change passed parameter to string --- .../webauthn_verification_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/two_factor_authentication/webauthn_verification_controller.rb b/app/controllers/two_factor_authentication/webauthn_verification_controller.rb index edf38ed91a0..b2277c41568 100644 --- a/app/controllers/two_factor_authentication/webauthn_verification_controller.rb +++ b/app/controllers/two_factor_authentication/webauthn_verification_controller.rb @@ -72,7 +72,7 @@ def handle_invalid_webauthn login_two_factor_options_path, ), ) - redirect_to login_two_factor_webauthn_url(platform: params[:platform]) + redirect_to login_two_factor_webauthn_url(platform: 'true') else flash[:error] = t('errors.general') From 227277b3a6e5db8ee07f44faeca91b2c0e2781cc Mon Sep 17 00:00:00 2001 From: kevinsmaster5 Date: Thu, 6 Jul 2023 16:52:17 -0400 Subject: [PATCH 8/9] revert error method to remove unneeded redirect --- .../webauthn_verification_controller.rb | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/app/controllers/two_factor_authentication/webauthn_verification_controller.rb b/app/controllers/two_factor_authentication/webauthn_verification_controller.rb index b2277c41568..0cd318efc29 100644 --- a/app/controllers/two_factor_authentication/webauthn_verification_controller.rb +++ b/app/controllers/two_factor_authentication/webauthn_verification_controller.rb @@ -27,16 +27,7 @@ def confirm handle_webauthn_result(result) end - def error - flash[:error] = t( - 'two_factor_authentication.webauthn_error.multiple_methods', - link: view_context.link_to( - t('two_factor_authentication.webauthn_error.additional_methods_link'), - login_two_factor_options_path, - ), - ) - redirect_to login_two_factor_webauthn_url(platform: params[:platform]) - end + def error; end private From c9b5bf10bcc35a1b9b71aa9e97bbfe2637e0568e Mon Sep 17 00:00:00 2001 From: kevinsmaster5 Date: Fri, 7 Jul 2023 09:17:52 -0400 Subject: [PATCH 9/9] tidy up line break --- .../webauthn_verification_controller.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/app/controllers/two_factor_authentication/webauthn_verification_controller.rb b/app/controllers/two_factor_authentication/webauthn_verification_controller.rb index 0cd318efc29..e7e4b4f66ec 100644 --- a/app/controllers/two_factor_authentication/webauthn_verification_controller.rb +++ b/app/controllers/two_factor_authentication/webauthn_verification_controller.rb @@ -64,7 +64,6 @@ def handle_invalid_webauthn ), ) redirect_to login_two_factor_webauthn_url(platform: 'true') - else flash[:error] = t('errors.general') redirect_to login_two_factor_webauthn_url