diff --git a/app/components/webauthn_verify_button_component.html.erb b/app/components/webauthn_verify_button_component.html.erb index 6f5940c6c98..f8a96854bbb 100644 --- a/app/components/webauthn_verify_button_component.html.erb +++ b/app/components/webauthn_verify_button_component.html.erb @@ -26,4 +26,5 @@ <%= hidden_field_tag :signature, '' %> <%= hidden_field_tag :client_data_json, '' %> <%= hidden_field_tag :webauthn_error, '' %> + <%= hidden_field_tag :screen_lock_error, '' %> <% end %> diff --git a/app/controllers/two_factor_authentication/webauthn_verification_controller.rb b/app/controllers/two_factor_authentication/webauthn_verification_controller.rb index 28ba92782ba..39b1e0a42ec 100644 --- a/app/controllers/two_factor_authentication/webauthn_verification_controller.rb +++ b/app/controllers/two_factor_authentication/webauthn_verification_controller.rb @@ -121,6 +121,7 @@ def form signature: params[:signature], credential_id: params[:credential_id], webauthn_error: params[:webauthn_error], + screen_lock_error: params[:screen_lock_error], ) end diff --git a/app/forms/webauthn_verification_form.rb b/app/forms/webauthn_verification_form.rb index 0ce31ead056..492df43286d 100644 --- a/app/forms/webauthn_verification_form.rb +++ b/app/forms/webauthn_verification_form.rb @@ -5,14 +5,16 @@ class WebauthnVerificationForm include ActionView::Helpers::TranslationHelper include Rails.application.routes.url_helpers + validates :screen_lock_error, + absence: { message: proc { |object| object.send(:screen_lock_error_message) } } validates :challenge, :authenticator_data, :client_data_json, :signature, :webauthn_configuration, - presence: { message: proc { |object| object.instance_eval { generic_error_message } } } + presence: { message: proc { |object| object.send(:generic_error_message) } } validates :webauthn_error, - absence: { message: proc { |object| object.instance_eval { generic_error_message } } } + absence: { message: proc { |object| object.send(:generic_error_message) } } validate :validate_assertion_response attr_reader :url_options, :platform_authenticator @@ -29,7 +31,8 @@ def initialize( client_data_json: nil, signature: nil, credential_id: nil, - webauthn_error: nil + webauthn_error: nil, + screen_lock_error: nil ) @user = user @platform_authenticator = platform_authenticator @@ -42,6 +45,7 @@ def initialize( @signature = signature @credential_id = credential_id @webauthn_error = webauthn_error + @screen_lock_error = screen_lock_error end def submit @@ -73,7 +77,8 @@ def self.domain_name :client_data_json, :signature, :credential_id, - :webauthn_error + :webauthn_error, + :screen_lock_error def validate_assertion_response return if webauthn_error.present? || webauthn_configuration.blank? || valid_assertion_response? @@ -127,13 +132,42 @@ def generic_error_message end end - def extra_analytics_attributes - auth_method = if webauthn_configuration&.platform_authenticator - 'webauthn_platform' - else - 'webauthn' - end + def screen_lock_error_message + if user_has_other_authentication_method? + t( + 'two_factor_authentication.webauthn_error.screen_lock_other_mfa_html', + link_html: link_to( + t('two_factor_authentication.webauthn_error.use_a_different_method'), + login_two_factor_options_path, + ), + ) + else + t( + 'two_factor_authentication.webauthn_error.screen_lock_no_other_mfa', + link_html: link_to( + t('two_factor_authentication.webauthn_error.use_a_different_method'), + login_two_factor_options_path, + ), + ) + end + end + def auth_method + if platform_authenticator? + 'webauthn_platform' + else + 'webauthn' + end + end + + def user_has_other_authentication_method? + MfaContext.new(user).two_factor_configurations.any? do |configuration| + !configuration.is_a?(WebauthnConfiguration) || + configuration.platform_authenticator? != platform_authenticator? + end + end + + def extra_analytics_attributes { multi_factor_auth_method: auth_method, webauthn_configuration_id: webauthn_configuration&.id, diff --git a/app/javascript/packages/webauthn/is-expected-error.spec.ts b/app/javascript/packages/webauthn/is-expected-error.spec.ts index 5943091862b..58834e29d05 100644 --- a/app/javascript/packages/webauthn/is-expected-error.spec.ts +++ b/app/javascript/packages/webauthn/is-expected-error.spec.ts @@ -1,4 +1,5 @@ import isExpectedWebauthnError from './is-expected-error'; +import { SCREEN_LOCK_ERROR } from './is-user-verification-screen-lock-error'; describe('isExpectedWebauthnError', () => { it('returns false for any error other than DOMException', () => { @@ -21,4 +22,18 @@ describe('isExpectedWebauthnError', () => { expect(result).to.be.true(); }); + + it('returns false for a screen lock error', () => { + const error = new DOMException(SCREEN_LOCK_ERROR, 'NotSupportedError'); + const result = isExpectedWebauthnError(error); + + expect(result).to.be.false(); + }); + + it('returns true for a screen lock error specified to have occurred during verification', () => { + const error = new DOMException(SCREEN_LOCK_ERROR, 'NotSupportedError'); + const result = isExpectedWebauthnError(error, { isVerifying: true }); + + expect(result).to.be.true(); + }); }); diff --git a/app/javascript/packages/webauthn/is-expected-error.ts b/app/javascript/packages/webauthn/is-expected-error.ts index 04d1c88aca7..7ee1c48babc 100644 --- a/app/javascript/packages/webauthn/is-expected-error.ts +++ b/app/javascript/packages/webauthn/is-expected-error.ts @@ -1,3 +1,5 @@ +import isUserVerificationScreenLockError from './is-user-verification-screen-lock-error'; + /** * Set of expected DOM exceptions, which occur based on some user behavior that is not noteworthy: * @@ -13,7 +15,21 @@ const EXPECTED_DOM_EXCEPTIONS: Set = new Set([ 'InvalidStateError', ]); -const isExpectedWebauthnError = (error: Error): boolean => - error instanceof DOMException && EXPECTED_DOM_EXCEPTIONS.has(error.name); +interface IsExpectedErrorOptions { + /** + * Whether the error happened in the context of a verification ceremony. + */ + isVerifying: boolean; +} + +function isExpectedWebauthnError( + error: Error, + { isVerifying }: Partial = {}, +): boolean { + return ( + (error instanceof DOMException && EXPECTED_DOM_EXCEPTIONS.has(error.name)) || + (!!isVerifying && isUserVerificationScreenLockError(error)) + ); +} export default isExpectedWebauthnError; diff --git a/app/javascript/packages/webauthn/is-user-verification-screen-lock-error.spec.ts b/app/javascript/packages/webauthn/is-user-verification-screen-lock-error.spec.ts new file mode 100644 index 00000000000..1be38b87bdd --- /dev/null +++ b/app/javascript/packages/webauthn/is-user-verification-screen-lock-error.spec.ts @@ -0,0 +1,17 @@ +import isUserVerificationScreenLockError, { + SCREEN_LOCK_ERROR, +} from './is-user-verification-screen-lock-error'; + +describe('isUserVerificationScreenLockError', () => { + it('returns false for an error that is not a screen lock error', () => { + const error = new DOMException('', 'NotSupportedError'); + + expect(isUserVerificationScreenLockError(error)).to.be.false(); + }); + + it('returns true for an error that is a screen lock error', () => { + const error = new DOMException(SCREEN_LOCK_ERROR, 'NotSupportedError'); + + expect(isUserVerificationScreenLockError(error)).to.be.true(); + }); +}); diff --git a/app/javascript/packages/webauthn/is-user-verification-screen-lock-error.ts b/app/javascript/packages/webauthn/is-user-verification-screen-lock-error.ts new file mode 100644 index 00000000000..bc442b5cb62 --- /dev/null +++ b/app/javascript/packages/webauthn/is-user-verification-screen-lock-error.ts @@ -0,0 +1,10 @@ +/** + * @see https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/modules/credentialmanagement/credentials_container.cc;l=432;drc=6d16761b175fd105f879a4e1803547381e97402d + */ +export const SCREEN_LOCK_ERROR = + 'The specified `userVerification` requirement cannot be fulfilled by this device unless the device is secured with a screen lock.'; + +const isUserVerificationScreenLockError = (error: Error): boolean => + error.message === SCREEN_LOCK_ERROR; + +export default isUserVerificationScreenLockError; diff --git a/app/javascript/packages/webauthn/webauthn-verify-button-element.spec.ts b/app/javascript/packages/webauthn/webauthn-verify-button-element.spec.ts index c9e6affc43c..5101427babd 100644 --- a/app/javascript/packages/webauthn/webauthn-verify-button-element.spec.ts +++ b/app/javascript/packages/webauthn/webauthn-verify-button-element.spec.ts @@ -3,6 +3,7 @@ import quibble from 'quibble'; import { screen } from '@testing-library/dom'; import userEvent from '@testing-library/user-event'; import '@18f/identity-submit-button/submit-button-element'; +import { SCREEN_LOCK_ERROR } from './is-user-verification-screen-lock-error'; import type { WebauthnVerifyButtonDataset } from './webauthn-verify-button-element'; describe('WebauthnVerifyButtonElement', () => { @@ -41,6 +42,7 @@ describe('WebauthnVerifyButtonElement', () => { + `; @@ -109,6 +111,7 @@ describe('WebauthnVerifyButtonElement', () => { client_data_json: '', signature: '', webauthn_error: 'NotAllowedError', + screen_lock_error: '', }); expect(trackError).not.to.have.been.called(); }); @@ -132,6 +135,7 @@ describe('WebauthnVerifyButtonElement', () => { client_data_json: '', signature: '', webauthn_error: 'CustomError', + screen_lock_error: '', }); expect(trackError).to.have.been.calledWith(error); }); @@ -155,6 +159,27 @@ describe('WebauthnVerifyButtonElement', () => { client_data_json: 'json', signature: 'sig', webauthn_error: '', + screen_lock_error: '', }); }); + + it('submits with NotSupportedError resulting from userVerification requirement', async () => { + const { form } = createElement(); + + verifyWebauthnDevice.throws(new DOMException(SCREEN_LOCK_ERROR, 'NotSupportedError')); + + const button = screen.getByRole('button', { name: 'Authenticate' }); + await userEvent.click(button); + await expect(form.submit).to.eventually.be.called(); + + expect(Object.fromEntries(new window.FormData(form))).to.deep.equal({ + credential_id: '', + authenticator_data: '', + client_data_json: '', + signature: '', + webauthn_error: 'NotSupportedError', + screen_lock_error: 'true', + }); + expect(trackError).not.to.have.been.called(); + }); }); diff --git a/app/javascript/packages/webauthn/webauthn-verify-button-element.ts b/app/javascript/packages/webauthn/webauthn-verify-button-element.ts index 21d8378df9c..d937c879f49 100644 --- a/app/javascript/packages/webauthn/webauthn-verify-button-element.ts +++ b/app/javascript/packages/webauthn/webauthn-verify-button-element.ts @@ -1,8 +1,9 @@ import { trackError } from '@18f/identity-analytics'; import type SubmitButtonElement from '@18f/identity-submit-button/submit-button-element'; import verifyWebauthnDevice from './verify-webauthn-device'; -import type { VerifyCredentialDescriptor } from './verify-webauthn-device'; import isExpectedWebauthnError from './is-expected-error'; +import isUserVerificationScreenLockError from './is-user-verification-screen-lock-error'; +import type { VerifyCredentialDescriptor } from './verify-webauthn-device'; export interface WebauthnVerifyButtonDataset extends DOMStringMap { credentials: string; @@ -58,10 +59,14 @@ class WebauthnVerifyButtonElement extends HTMLElement { this.setInputValue('client_data_json', result.clientDataJSON); this.setInputValue('signature', result.signature); } catch (error) { - if (!isExpectedWebauthnError(error)) { + if (!isExpectedWebauthnError(error, { isVerifying: true })) { trackError(error); } + if (isUserVerificationScreenLockError(error)) { + this.setInputValue('screen_lock_error', 'true'); + } + this.setInputValue('webauthn_error', error.name); } diff --git a/config/locales/two_factor_authentication/en.yml b/config/locales/two_factor_authentication/en.yml index bac52964f9d..606cff6630b 100644 --- a/config/locales/two_factor_authentication/en.yml +++ b/config/locales/two_factor_authentication/en.yml @@ -178,7 +178,14 @@ en: additional_methods_link: choose another authentication method connect_html: We were unable to connect the security key. Please try again or %{link_html}. + screen_lock_no_other_mfa: We couldn’t authenticate with face or touch unlock. + Try signing in on the device where you first set up face or touch + unlock. + screen_lock_other_mfa_html: We couldn’t authenticate with face or touch unlock. + %{link_html}, or try signing in on the device where you first set up + face or touch unlock. try_again: Face or touch unlock was unsuccessful. Please try again or %{link}. + use_a_different_method: Use a different authentication method webauthn_header_text: Connect your security key webauthn_platform_header_text: Use face or touch unlock webauthn_platform_use_key: Use face or touch unlock diff --git a/config/locales/two_factor_authentication/es.yml b/config/locales/two_factor_authentication/es.yml index c2a30013de9..abdd574883c 100644 --- a/config/locales/two_factor_authentication/es.yml +++ b/config/locales/two_factor_authentication/es.yml @@ -189,8 +189,16 @@ es: additional_methods_link: elija otro método de autenticación connect_html: No hemos podido conectar la clave de seguridad. Por favor, inténtelo de nuevo o %{link_html}. + screen_lock_no_other_mfa: No pudimos comprobar la autenticidad mediante + desbloqueo facial o táctil. Intente iniciar sesión en el dispositivo + donde configuró por primera vez el desbloqueo facial o táctil. + screen_lock_other_mfa_html: No pudimos comprobar la autenticidad mediante + desbloqueo facial o táctil. %{link_html} o intente iniciar sesión en el + dispositivo donde configuró por primera vez el desbloqueo facial o + táctil. try_again: El desbloqueo facial o táctil no fue exitoso. Por favor, inténtelo de nuevo o %{link}. + use_a_different_method: Utilice otro método de autenticación webauthn_header_text: Conecte su llave de seguridad webauthn_platform_header_text: Usar desbloqueo facial o táctil webauthn_platform_use_key: Usar desbloqueo facial o táctil diff --git a/config/locales/two_factor_authentication/fr.yml b/config/locales/two_factor_authentication/fr.yml index 52dee824d9f..b512cf3ea62 100644 --- a/config/locales/two_factor_authentication/fr.yml +++ b/config/locales/two_factor_authentication/fr.yml @@ -198,8 +198,17 @@ fr: additional_methods_link: choisir une autre méthode d’authentification connect_html: Nous n’avons pas pu connecter la clé de sécurité. Veuillez réessayer ou %{link_html}. + screen_lock_no_other_mfa: Nous n’avons pas pu nous authentifier avec le + déverrouillage facial ou tactile. Essayez de vous connecter sur + l’appareil sur lequel vous avez configuré le déverrouillage facial ou + tactile. + screen_lock_other_mfa_html: Nous n’avons pas pu nous authentifier avec le + déverrouillage facial ou tactile. %{link_html} ou essayez de vous + connecter sur l’appareil sur lequel vous avez configuré le + déverrouillage du visage ou du toucher. try_again: Le déverrouillage facial ou tactile n’a pas fonctionné. Veuillez réessayer ou %{link}. + use_a_different_method: Utilisez un autre moyen d’authentification webauthn_header_text: Connectez votre clé de sécurité webauthn_platform_header_text: Utilisez le déverrouillage facial ou tactile webauthn_platform_use_key: Utilisez le déverrouillage facial ou tactile 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 2431abb0291..8f1de36b8a6 100644 --- a/spec/controllers/two_factor_authentication/webauthn_verification_controller_spec.rb +++ b/spec/controllers/two_factor_authentication/webauthn_verification_controller_spec.rb @@ -291,7 +291,7 @@ multi_factor_auth_method_created_at: second_webauthn_platform_configuration.created_at.strftime('%s%L'), webauthn_configuration_id: nil, - frontend_error: 'NotAllowedError', + frontend_error: webauthn_error, ) patch :confirm, params: params diff --git a/spec/forms/webauthn_verification_form_spec.rb b/spec/forms/webauthn_verification_form_spec.rb index efd4cd2ca1e..8c78ab827a9 100644 --- a/spec/forms/webauthn_verification_form_spec.rb +++ b/spec/forms/webauthn_verification_form_spec.rb @@ -7,6 +7,7 @@ let(:user) { create(:user) } let(:challenge) { webauthn_challenge } let(:webauthn_error) { nil } + let(:screen_lock_error) { nil } let(:platform_authenticator) { false } let(:client_data_json) { verification_client_data_json } let!(:webauthn_configuration) do @@ -32,6 +33,7 @@ signature: signature, credential_id: credential_id, webauthn_error: webauthn_error, + screen_lock_error:, ) end @@ -166,6 +168,112 @@ end end + context 'when a screen lock error is present' do + let(:screen_lock_error) { 'true' } + + context 'user does not have another authentication method available' do + it 'returns unsuccessful result' do + expect(result.to_h).to eq( + success: false, + error_details: { + screen_lock_error: { present: true }, + }, + multi_factor_auth_method: 'webauthn', + webauthn_configuration_id: webauthn_configuration.id, + ) + end + + it 'provides error message not suggesting other method' do + expect(result.first_error_message).to eq t( + 'two_factor_authentication.webauthn_error.screen_lock_no_other_mfa', + link_html: link_to( + t('two_factor_authentication.webauthn_error.use_a_different_method'), + login_two_factor_options_path, + ), + ) + end + end + + context 'user has another WebAuthn method available' do + context 'the other MFA method is WebAuthn of the same attachment' do + let(:platform_authenticator) { false } + let(:user) { create(:user, :with_webauthn) } + + it 'returns unsuccessful result' do + expect(result.to_h).to eq( + success: false, + error_details: { + screen_lock_error: { present: true }, + }, + multi_factor_auth_method: 'webauthn', + webauthn_configuration_id: webauthn_configuration.id, + ) + end + + it 'provides error message not suggesting other method' do + expect(result.first_error_message).to eq t( + 'two_factor_authentication.webauthn_error.screen_lock_no_other_mfa', + link_html: link_to( + t('two_factor_authentication.webauthn_error.use_a_different_method'), + login_two_factor_options_path, + ), + ) + end + end + + context 'the other MFA method is WebAuthn of a different attachment' do + let(:platform_authenticator) { false } + let(:user) { create(:user, :with_webauthn_platform) } + + it 'returns unsuccessful result' do + expect(result.to_h).to eq( + success: false, + error_details: { + screen_lock_error: { present: true }, + }, + multi_factor_auth_method: 'webauthn', + webauthn_configuration_id: webauthn_configuration.id, + ) + end + + it 'provides error message suggesting other method' do + expect(result.first_error_message).to eq t( + 'two_factor_authentication.webauthn_error.screen_lock_other_mfa_html', + link_html: link_to( + t('two_factor_authentication.webauthn_error.use_a_different_method'), + login_two_factor_options_path, + ), + ) + end + end + + context 'the other MFA method is not a WebAuthn method' do + let(:user) { create(:user, :with_phone) } + + it 'returns unsuccessful result' do + expect(result.to_h).to eq( + success: false, + error_details: { + screen_lock_error: { present: true }, + }, + multi_factor_auth_method: 'webauthn', + webauthn_configuration_id: webauthn_configuration.id, + ) + end + + it 'provides error message suggesting other method' do + expect(result.first_error_message).to eq t( + 'two_factor_authentication.webauthn_error.screen_lock_other_mfa_html', + link_html: link_to( + t('two_factor_authentication.webauthn_error.use_a_different_method'), + login_two_factor_options_path, + ), + ) + end + end + end + end + context 'when origin is invalid' do before do allow(IdentityConfig.store).to receive(:domain_name).and_return('localhost:6666')