From 99308e5b7e61bec596ad8340604869a7039283a6 Mon Sep 17 00:00:00 2001 From: Valerie Pomerleau Date: Fri, 3 Jul 2026 12:59:39 -0700 Subject: [PATCH] fix(passkeys): handle authenticator user-verification failures MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Because: * Passkey registration threw a 500 and was captured in Sentry (FXA-AUTH-39Z) whenever an authenticator completed the ceremony without user verification (UV=0) — e.g. a roaming security key with no PIN — while the server requires UV for the AAL2 invariant. This is expected client/authenticator behaviour, not a server fault. * Users saw a generic "Passkey registration failed" with no guidance, and a stalled device prompt showed only a spinner. This commit: * Classifies the UV failure in the passkey adapter (UserVerificationRequiredError) and returns a 422 (errno PASSKEY_USER_VERIFICATION_REQUIRED = 233) instead of a 500, without capturing it in Sentry; keeps a passkey.registration.failed{reason=userVerificationFailed} metric for sizing. * Applies the same classification to the authentication path (metric only; keeps the existing 401, no distinct errno to avoid leaking credential capability). * Registers errno 233 on the client with a device-agnostic message guiding users to add a screen lock, PIN, or biometric, or choose another method. * Adds a non-blocking, device-agnostic info Banner (role=status/aria-live) under the "Creating passkey…" spinner once the ceremony stalls, plus a Storybook case. * Keeps user verification strictly required; adds unit and component tests. closes FXA-14080 --- libs/accounts/errors/src/app-error.ts | 9 ++ libs/accounts/errors/src/constants.ts | 1 + libs/accounts/errors/src/index.spec.ts | 16 +++ .../passkey/src/lib/passkey.service.spec.ts | 106 ++++++++++++++++++ .../passkey/src/lib/passkey.service.ts | 24 +++- .../passkey/src/lib/webauthn-adapter.spec.ts | 36 +++++- .../passkey/src/lib/webauthn-adapter.ts | 89 ++++++++++++--- .../components/Settings/PagePasskeyAdd/en.ftl | 4 + .../Settings/PagePasskeyAdd/index.stories.tsx | 13 ++- .../Settings/PagePasskeyAdd/index.test.tsx | 68 ++++++++++- .../Settings/PagePasskeyAdd/index.tsx | 41 ++++++- .../src/lib/auth-errors/auth-errors.test.ts | 10 ++ .../src/lib/auth-errors/auth-errors.ts | 5 + .../fxa-settings/src/lib/auth-errors/en.ftl | 1 + 14 files changed, 392 insertions(+), 31 deletions(-) diff --git a/libs/accounts/errors/src/app-error.ts b/libs/accounts/errors/src/app-error.ts index 5477bff3b79..eaf2b1800ea 100644 --- a/libs/accounts/errors/src/app-error.ts +++ b/libs/accounts/errors/src/app-error.ts @@ -1783,6 +1783,15 @@ export class AppError extends Error { }); } + static passkeyUserVerificationRequired() { + return new AppError({ + code: 422, + error: 'Unprocessable Entity', + errno: ERRNO.PASSKEY_USER_VERIFICATION_REQUIRED, + message: 'Passkey requires user verification (PIN or biometric)', + }); + } + static passkeyChallengeExpired() { return new AppError({ code: 401, diff --git a/libs/accounts/errors/src/constants.ts b/libs/accounts/errors/src/constants.ts index 5f38157592d..64f07233825 100644 --- a/libs/accounts/errors/src/constants.ts +++ b/libs/accounts/errors/src/constants.ts @@ -138,6 +138,7 @@ export const ERRNO = { PASSKEY_INVALID_NAME: 230, PASSKEY_DELETE_FAILED: 231, PASSKEY_RENAME_FAILED: 232, + PASSKEY_USER_VERIFICATION_REQUIRED: 233, // Jump to 238 to leave room for future passkey errors PASSKEY_CHALLENGE_EXPIRED: 238, ACCOUNT_DELETION_FAILED: 239, diff --git a/libs/accounts/errors/src/index.spec.ts b/libs/accounts/errors/src/index.spec.ts index 8aac4f12cc3..4a64c2ada19 100644 --- a/libs/accounts/errors/src/index.spec.ts +++ b/libs/accounts/errors/src/index.spec.ts @@ -434,6 +434,22 @@ describe('AppErrors', () => { }); }); + it('creates passkeyUserVerificationRequired', () => { + const result = AppError.passkeyUserVerificationRequired(); + expect(result).toBeInstanceOf(AppError); + expect(result).toMatchObject({ + errno: 233, + message: 'Passkey requires user verification (PIN or biometric)', + output: { + statusCode: 422, + payload: { + error: 'Unprocessable Entity', + errno: 233, + }, + }, + }); + }); + it('creates passkeyChallengeExpired', () => { const result = AppError.passkeyChallengeExpired(); expect(result).toBeInstanceOf(AppError); diff --git a/libs/accounts/passkey/src/lib/passkey.service.spec.ts b/libs/accounts/passkey/src/lib/passkey.service.spec.ts index f305382c8ce..923cef2794b 100644 --- a/libs/accounts/passkey/src/lib/passkey.service.spec.ts +++ b/libs/accounts/passkey/src/lib/passkey.service.spec.ts @@ -15,15 +15,28 @@ import { PasskeyService } from './passkey.service'; import { PasskeyManager } from './passkey.manager'; import { PasskeyChallengeManager } from './passkey.challenge.manager'; import { AppError } from '@fxa/accounts/errors'; +import * as Sentry from '@sentry/nestjs'; +// Keep the real UserVerificationRequiredError class (used for instanceof checks +// in the service) while stubbing the adapter's ceremony functions. jest.mock('./webauthn-adapter', () => ({ + ...jest.requireActual('./webauthn-adapter'), generateWebauthnRegistrationOptions: jest.fn(), verifyWebauthnRegistrationResponse: jest.fn(), generateWebauthnAuthenticationOptions: jest.fn(), verifyWebauthnAuthenticationResponse: jest.fn(), })); +// Stub only captureException so we can assert what does/doesn't reach Sentry. +jest.mock('@sentry/nestjs', () => ({ + ...jest.requireActual('@sentry/nestjs'), + captureException: jest.fn(), +})); + import * as webauthnAdapter from './webauthn-adapter'; +import { UserVerificationRequiredError } from './webauthn-adapter'; + +const mockCaptureException = Sentry.captureException as jest.Mock; const mockGenerateWebauthnRegistrationOptions = webauthnAdapter.generateWebauthnRegistrationOptions as jest.Mock; @@ -317,6 +330,84 @@ describe('PasskeyService', () => { }); }); + it('captures an unexpected verification error in Sentry', async () => { + const err = new Error('Invalid attestation format'); + mockVerifyWebauthnRegistrationResponse.mockRejectedValue(err); + await expect( + service.createPasskeyFromRegistrationResponse( + MOCK_UID, + mockResponse, + MOCK_CHALLENGE + ) + ).rejects.toThrow(); + expect(mockCaptureException).toHaveBeenCalledWith(err); + }); + + it('increments a verificationError metric on an unexpected error', async () => { + mockVerifyWebauthnRegistrationResponse.mockRejectedValue( + new Error('Invalid attestation format') + ); + await expect( + service.createPasskeyFromRegistrationResponse( + MOCK_UID, + mockResponse, + MOCK_CHALLENGE + ) + ).rejects.toThrow(); + expect(mockMetrics.increment).toHaveBeenCalledWith( + 'passkey.registration.failed', + { reason: 'verificationError' } + ); + }); + + it('throws passkeyUserVerificationRequired AppError when the authenticator does not verify the user', async () => { + mockVerifyWebauthnRegistrationResponse.mockRejectedValue( + new UserVerificationRequiredError() + ); + await expect( + service.createPasskeyFromRegistrationResponse( + MOCK_UID, + mockResponse, + MOCK_CHALLENGE + ) + ).rejects.toMatchObject({ + errno: 233, + message: 'Passkey requires user verification (PIN or biometric)', + code: 422, + }); + }); + + it('does not capture a user-verification failure in Sentry', async () => { + mockVerifyWebauthnRegistrationResponse.mockRejectedValue( + new UserVerificationRequiredError() + ); + await expect( + service.createPasskeyFromRegistrationResponse( + MOCK_UID, + mockResponse, + MOCK_CHALLENGE + ) + ).rejects.toThrow(); + expect(mockCaptureException).not.toHaveBeenCalled(); + }); + + it('increments a userVerificationFailed metric on a user-verification failure', async () => { + mockVerifyWebauthnRegistrationResponse.mockRejectedValue( + new UserVerificationRequiredError() + ); + await expect( + service.createPasskeyFromRegistrationResponse( + MOCK_UID, + mockResponse, + MOCK_CHALLENGE + ) + ).rejects.toThrow(); + expect(mockMetrics.increment).toHaveBeenCalledWith( + 'passkey.registration.failed', + { reason: 'userVerificationFailed' } + ); + }); + it('calls challengeManager.consumeRegistrationChallenge with challenge and uid', async () => { await service.createPasskeyFromRegistrationResponse( MOCK_UID, @@ -760,6 +851,21 @@ describe('PasskeyService', () => { ); }); + it('increments a userVerificationFailed metric when the authenticator does not verify the user', async () => { + ( + webauthnAdapter.verifyWebauthnAuthenticationResponse as jest.Mock + ).mockRejectedValue(new UserVerificationRequiredError()); + + await expect( + service.verifyAuthenticationResponse(mockResponse, MOCK_CHALLENGE) + ).rejects.toMatchObject({ errno: 227 }); + + expect(mockMetrics.increment).toHaveBeenCalledWith( + 'passkey.authentication.failed', + { reason: 'userVerificationFailed' } + ); + }); + it('throws a passkeyAuthenticationFailed AppError and logs when updatePasskeyAfterAuth returns false', async () => { mockManager.updatePasskeyAfterAuth.mockResolvedValue(false); diff --git a/libs/accounts/passkey/src/lib/passkey.service.ts b/libs/accounts/passkey/src/lib/passkey.service.ts index 0c45ede9232..e98d9e43f32 100644 --- a/libs/accounts/passkey/src/lib/passkey.service.ts +++ b/libs/accounts/passkey/src/lib/passkey.service.ts @@ -26,6 +26,7 @@ import { AuthenticationVerificationResult, generateWebauthnAuthenticationOptions, verifyWebauthnAuthenticationResponse, + UserVerificationRequiredError, } from './webauthn-adapter'; import { AppError } from '@fxa/accounts/errors'; @@ -172,6 +173,14 @@ export class PasskeyService { challenge: storedChallenge.challenge, }); } catch (err: unknown) { + if (err instanceof UserVerificationRequiredError) { + // Expected, user-actionable outcome (e.g. a PIN-less roaming security + // key). Not a server fault — surface a specific 4xx and skip Sentry. + this.metrics.increment('passkey.registration.failed', { + reason: 'userVerificationFailed', + }); + throw AppError.passkeyUserVerificationRequired(); + } Sentry.captureException(err); this.metrics.increment('passkey.registration.failed', { reason: 'verificationError', @@ -249,7 +258,7 @@ export class PasskeyService { credentialId, trimmed ); - } catch (err) { + } catch (err: unknown) { Sentry.captureException(err); this.metrics.increment('passkey.rename.failed', { reason: 'dbError', @@ -290,7 +299,7 @@ export class PasskeyService { let deleted = false; try { deleted = await this.passkeyManager.deletePasskey(uid, credentialId); - } catch (err) { + } catch (err: unknown) { Sentry.captureException(err); this.metrics.increment('passkey.delete.failed', { reason: 'dbError', @@ -463,7 +472,16 @@ export class PasskeyService { publicKey: passkey.publicKey, signCount: passkey.signCount, }); - } catch (err) { + } catch (err: unknown) { + if (err instanceof UserVerificationRequiredError) { + // Authenticator did not perform UV. Keep the generic 401 (no distinct + // errno — avoids leaking credential-capability details on sign-in), but + // tag the metric so this cause is separable from other failures. + this.metrics.increment('passkey.authentication.failed', { + reason: 'userVerificationFailed', + }); + throw AppError.passkeyAuthenticationFailed(); + } // simplewebauthn throws when signCount decrements if (err instanceof Error && /^Response counter value/.test(err.message)) { this.log?.warn('passkey.signCount.rollback', { diff --git a/libs/accounts/passkey/src/lib/webauthn-adapter.spec.ts b/libs/accounts/passkey/src/lib/webauthn-adapter.spec.ts index b6595d98b96..46605d74d23 100644 --- a/libs/accounts/passkey/src/lib/webauthn-adapter.spec.ts +++ b/libs/accounts/passkey/src/lib/webauthn-adapter.spec.ts @@ -9,6 +9,7 @@ import { generateWebauthnAuthenticationOptions, verifyWebauthnAuthenticationResponse, extractPrfEnabled, + UserVerificationRequiredError, } from './webauthn-adapter'; import { PasskeyConfig } from './passkey.config'; import { VirtualAuthenticator } from './virtual-authenticator'; @@ -366,7 +367,7 @@ describe('verifyWebauthnRegistrationResponse', () => { ).rejects.toThrow(); }); - it('rejects an attestation without user verification', async () => { + it('throws UserVerificationRequiredError for an attestation without user verification', async () => { const cred = VirtualAuthenticator.createCredential(); const challenge = randomBytes(32).toString('base64url'); @@ -387,7 +388,34 @@ describe('verifyWebauthnRegistrationResponse', () => { response: attestation, challenge, }) - ).rejects.toThrow(/user could not be verified/i); + ).rejects.toBeInstanceOf(UserVerificationRequiredError); + }); + + it('does not classify a non-UV failure (wrong rpId) as UserVerificationRequiredError', async () => { + const cred = VirtualAuthenticator.createCredential(); + const challenge = randomBytes(32).toString('base64url'); + + const options = await generateWebauthnRegistrationOptions(config, { + uid: Buffer.alloc(16, 0xaa).toString('hex'), + email: 'test@example.com', + challenge, + }); + + const attestation = VirtualAuthenticator.createAttestationResponse(cred, { + challenge: options.challenge, + origin: TEST_ORIGIN, + rpId: 'evil.example.com', + }); + + const attempt = verifyWebauthnRegistrationResponse(config, { + response: attestation, + challenge, + }); + // Still rejects (for the rpId mismatch) — just not classified as a UV failure. + await expect(attempt).rejects.toThrow(); + await expect(attempt).rejects.not.toBeInstanceOf( + UserVerificationRequiredError + ); }); }); @@ -642,7 +670,7 @@ describe('verifyWebauthnAuthenticationResponse', () => { ).rejects.toThrow(); }); - it('rejects an assertion without user verification', async () => { + it('throws UserVerificationRequiredError for an assertion without user verification', async () => { const { cred, stored } = await registerCredential(config); const challenge = randomBytes(32).toString('base64url'); @@ -665,6 +693,6 @@ describe('verifyWebauthnAuthenticationResponse', () => { publicKey: stored.publicKey, signCount: stored.signCount, }) - ).rejects.toThrow(/user could not be verified/i); + ).rejects.toBeInstanceOf(UserVerificationRequiredError); }); }); diff --git a/libs/accounts/passkey/src/lib/webauthn-adapter.ts b/libs/accounts/passkey/src/lib/webauthn-adapter.ts index 8181f534f51..54c9189d79f 100644 --- a/libs/accounts/passkey/src/lib/webauthn-adapter.ts +++ b/libs/accounts/passkey/src/lib/webauthn-adapter.ts @@ -17,6 +17,8 @@ import type { PublicKeyCredentialRequestOptionsJSON, AuthenticatorTransportFuture, AuthenticationExtensionsClientInputs, + VerifiedRegistrationResponse, + VerifiedAuthenticationResponse, } from '@simplewebauthn/server'; import { uuidTransformer } from '@fxa/shared/db/mysql/core'; @@ -124,6 +126,35 @@ export function extractPrfEnabled(extensions: unknown): boolean { ); } +/** + * Thrown when the authenticator completed a WebAuthn ceremony without performing + * user verification (the UV flag was not set) — e.g. a roaming security key with + * no PIN or biometric. Callers can surface a clear, actionable error and skip + * Sentry noise for this expected outcome, distinct from genuinely unexpected + * verification failures. + */ +export class UserVerificationRequiredError extends Error { + constructor() { + super('User verification was not performed by the authenticator'); + this.name = 'UserVerificationRequiredError'; + } +} + +/** + * SimpleWebAuthn throws a generic `Error` (it exposes no error code) when UV was + * required but the authenticator did not verify the user. "user could not be + * verified" is the invariant phrase shared by both the registration + * ("...was required...") and authentication ("...required...") messages, so + * matching on it isolates this one expected case from all other library throws. + * The adapter UV specs exercise the real library, so a wording change on a + * SimpleWebAuthn upgrade fails CI rather than silently regressing UV → 500. + */ +function isUserVerificationError(err: unknown): boolean { + return ( + err instanceof Error && /user could not be verified/i.test(err.message) + ); +} + /** * Verify a WebAuthn registration (attestation) response. * @@ -131,21 +162,33 @@ export function extractPrfEnabled(extensions: unknown): boolean { * @param input - Per-request inputs * @returns a discriminated union indicating verification success or failure; * `data` contains extracted credential info on success. - * @throws wrapped library function throws `Error` on invalid input. + * @throws {UserVerificationRequiredError} when the authenticator did not perform + * user verification (UV flag unset). + * @throws wrapped library function throws `Error` on other invalid input. * See source code for possible errors: https://github.com/MasterKale/SimpleWebAuthn/blob/320757144749c742e58ab3bb181087f9fbcac074/packages/server/src/registration/verifyRegistrationResponse.ts */ export async function verifyWebauthnRegistrationResponse( config: PasskeyConfig, input: VerifyRegistrationInput ): Promise { - const { verified, registrationInfo } = await verifyRegistrationResponse({ - response: input.response, - expectedChallenge: input.challenge, - expectedOrigin: config.allowedOrigins, - expectedRPID: config.rpId, - // Library defaults this to false; UV must be enforced for the AAL2 claim. - requireUserVerification: true, - }); + let result: VerifiedRegistrationResponse; + try { + result = await verifyRegistrationResponse({ + response: input.response, + expectedChallenge: input.challenge, + expectedOrigin: config.allowedOrigins, + expectedRPID: config.rpId, + // Library defaults this to false; UV must be enforced for the AAL2 claim. + requireUserVerification: true, + }); + } catch (err: unknown) { + if (isUserVerificationError(err)) { + throw new UserVerificationRequiredError(); + } + throw err; + } + + const { verified, registrationInfo } = result; if (!verified) { return { verified: false }; @@ -262,15 +305,25 @@ export async function verifyWebauthnAuthenticationResponse( counter: input.signCount, }; - const { verified, authenticationInfo } = await verifyAuthenticationResponse({ - response: input.response, - expectedChallenge: input.challenge, - expectedOrigin: config.allowedOrigins, - expectedRPID: config.rpId, - credential, - // Library defaults this to false; UV must be enforced for the AAL2 claim. - requireUserVerification: true, - }); + let result: VerifiedAuthenticationResponse; + try { + result = await verifyAuthenticationResponse({ + response: input.response, + expectedChallenge: input.challenge, + expectedOrigin: config.allowedOrigins, + expectedRPID: config.rpId, + credential, + // Library defaults this to false; UV must be enforced for the AAL2 claim. + requireUserVerification: true, + }); + } catch (err: unknown) { + if (isUserVerificationError(err)) { + throw new UserVerificationRequiredError(); + } + throw err; + } + + const { verified, authenticationInfo } = result; if (!verified) { return { verified: false }; diff --git a/packages/fxa-settings/src/components/Settings/PagePasskeyAdd/en.ftl b/packages/fxa-settings/src/components/Settings/PagePasskeyAdd/en.ftl index 55f51808b23..897a67e00a4 100644 --- a/packages/fxa-settings/src/components/Settings/PagePasskeyAdd/en.ftl +++ b/packages/fxa-settings/src/components/Settings/PagePasskeyAdd/en.ftl @@ -2,6 +2,10 @@ page-passkey-add-creating-heading = Creating passkey… page-passkey-add-follow-prompts = Follow the prompts on your device. +# Shown if the device prompt has not resolved after a delay. Non-blocking hint; +# an authenticator that cannot verify the user (no screen lock, PIN, or +# biometric) can make the browser prompt appear to hang. +page-passkey-add-slow-hint = Taking a while? Your device may need a screen lock, PIN, or biometric set up to create a passkey. page-passkey-add-cancel = Cancel ## Success / Error messages (shown in alert bar after returning to settings) diff --git a/packages/fxa-settings/src/components/Settings/PagePasskeyAdd/index.stories.tsx b/packages/fxa-settings/src/components/Settings/PagePasskeyAdd/index.stories.tsx index 193df42a0f9..fd133df9f94 100644 --- a/packages/fxa-settings/src/components/Settings/PagePasskeyAdd/index.stories.tsx +++ b/packages/fxa-settings/src/components/Settings/PagePasskeyAdd/index.stories.tsx @@ -75,7 +75,7 @@ const configWithPasskeys = { }, }; -export const CeremonyInProgress = () => { +const PasskeyAddStory = ({ slowHintDelayMs }: { slowHintDelayMs?: number }) => { initLocalAccount(); // In-memory history so navigation doesn't change the storybook iframe's // real URL (which would unmount the story). Memoized so toolbar-driven @@ -105,10 +105,19 @@ export const CeremonyInProgress = () => { value={mockSettingsContext({ alertBarInfo: new StoryAlertBarInfo() })} > - + ); }; + +export const CeremonyInProgress = () => ; + +// The ceremony has stalled (e.g. a security key with no PIN, or a device with +// no screen lock). A 0ms delay surfaces the non-blocking hint immediately here; +// production waits ~20s. +export const CeremonyStalledWithHint = () => ( + +); diff --git a/packages/fxa-settings/src/components/Settings/PagePasskeyAdd/index.test.tsx b/packages/fxa-settings/src/components/Settings/PagePasskeyAdd/index.test.tsx index 5cad61bba43..8e3748745e9 100644 --- a/packages/fxa-settings/src/components/Settings/PagePasskeyAdd/index.test.tsx +++ b/packages/fxa-settings/src/components/Settings/PagePasskeyAdd/index.test.tsx @@ -3,7 +3,13 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ import React from 'react'; -import { render, screen, waitFor, fireEvent } from '@testing-library/react'; +import { + render, + screen, + waitFor, + fireEvent, + act, +} from '@testing-library/react'; import '@testing-library/jest-dom'; import { LocationProvider } from '@reach/router'; @@ -160,6 +166,12 @@ describe('PagePasskeyAdd', () => { }); }); + afterEach(() => { + // Guarantee timer restoration even if a fake-timer test fails mid-assertion, + // so real-timer/waitFor tests later in the file aren't affected. + jest.useRealTimers(); + }); + it('shows loading modal on mount', () => { // Make the ceremony hang so we can see the modal mockBeginPasskeyRegistration.mockReturnValue(new Promise(() => {})); @@ -173,6 +185,35 @@ describe('PagePasskeyAdd', () => { expect(screen.getByTestId('passkey-add-cancel')).toBeInTheDocument(); }); + it('does not show the slow-ceremony hint before the delay', () => { + jest.useFakeTimers(); + // Ceremony hangs so the loading UI stays mounted. + mockBeginPasskeyRegistration.mockReturnValue(new Promise(() => {})); + renderPage(); + // Advance to just before the threshold — the hint must still be hidden. + act(() => { + jest.advanceTimersByTime(19_999); + }); + expect(screen.queryByText(/Taking a while\?/)).not.toBeInTheDocument(); + }); + + it('reveals the slow-ceremony hint once the ceremony stalls past the delay', () => { + jest.useFakeTimers(); + // Ceremony hangs so the loading UI stays mounted and the hint timer fires. + mockBeginPasskeyRegistration.mockReturnValue(new Promise(() => {})); + renderPage(); + + act(() => { + jest.advanceTimersByTime(20_000); + }); + + // Assert the live-region role too, so the aria-live contract added by the + // fix is covered, not just the copy. + expect(screen.getByRole('status')).toHaveTextContent( + 'Taking a while? Your device may need a screen lock, PIN, or biometric set up to create a passkey.' + ); + }); + it('completes ceremony and shows success alert', async () => { renderPage(); await waitFor(() => { @@ -378,6 +419,31 @@ describe('PagePasskeyAdd', () => { }); }); + it('shows the user-verification-required message when completePasskeyRegistration returns errno 233', async () => { + const uvError = { + errno: 233, + message: 'Passkey requires user verification (PIN or biometric)', + code: 422, + }; + mockCompletePasskeyRegistration.mockRejectedValue(uvError); + + renderPage(); + await waitFor(() => { + expect(mockAlertError).toHaveBeenCalledWith( + 'Your device or security key needs a screen lock, PIN, or biometric to be used as a passkey. Add one, or choose another method.' + ); + }); + expect( + GleanMetrics.accountPref.passkeyCreateSubmitFrontendError + ).toHaveBeenCalledWith({ + event: { reason: 'auth_error_233' }, + }); + expect(mockCaptureException).not.toHaveBeenCalled(); + expect(mockNavigateWithQuery).toHaveBeenCalledWith('/settings#security', { + replace: true, + }); + }); + it('shows "Passkey limit reached" when beginPasskeyRegistration returns errno 226', async () => { const limitError = { errno: 226, diff --git a/packages/fxa-settings/src/components/Settings/PagePasskeyAdd/index.tsx b/packages/fxa-settings/src/components/Settings/PagePasskeyAdd/index.tsx index 3c3b05ea1ba..566c7eeb587 100644 --- a/packages/fxa-settings/src/components/Settings/PagePasskeyAdd/index.tsx +++ b/packages/fxa-settings/src/components/Settings/PagePasskeyAdd/index.tsx @@ -2,7 +2,7 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ -import React, { useCallback, useEffect, useRef } from 'react'; +import React, { useCallback, useEffect, useRef, useState } from 'react'; import { RouteComponentProps } from '@reach/router'; import * as Sentry from '@sentry/browser'; @@ -26,6 +26,7 @@ import { import { MfaGuard, useMfaErrorHandler } from '../MfaGuard'; import LoadingSpinner from 'fxa-react/components/LoadingSpinner'; import { FtlMsg } from 'fxa-react/lib/utils'; +import { Banner } from '../../Banner'; import GleanMetrics from '../../../lib/glean'; import { getLocalizedErrorMessage, @@ -38,6 +39,12 @@ import { unsupportedPasskeyMessage, } from '../../../lib/passkeys/unsupported-message'; +// How long the ceremony can sit unresolved before we surface a non-blocking +// hint. Some authenticators (e.g. a PIN-less security key) make the browser's +// native prompt appear to hang while it waits on UV setup; the hint nudges the +// user without aborting legitimately-slow flows (e.g. cross-device QR). +const SLOW_CEREMONY_HINT_MS = 20_000; + export const MfaGuardPagePasskeyAdd = (_: RouteComponentProps) => { const alertBar = useAlertBar(); const navigateWithQuery = useNavigateWithQuery(); @@ -65,7 +72,13 @@ export const MfaGuardPagePasskeyAdd = (_: RouteComponentProps) => { ); }; -export const PagePasskeyAdd = () => { +export const PagePasskeyAdd = ({ + // Overridable so stories/tests can surface the stalled-ceremony hint without + // waiting out the real delay. Production always uses the default. + slowHintDelayMs = SLOW_CEREMONY_HINT_MS, +}: { + slowHintDelayMs?: number; +} = {}) => { const account = useAccount(); const authClient = useAuthClient(); const alertBar = useAlertBar(); @@ -77,6 +90,7 @@ export const PagePasskeyAdd = () => { const isMounted = useRef(true); const wasCanceled = useRef(false); const abortController = useRef(null); + const [showSlowHint, setShowSlowHint] = useState(false); const navigateToSettings = useCallback(() => { navigateWithQuery(SETTINGS_PATH + '#security', { replace: true }); @@ -105,6 +119,14 @@ export const PagePasskeyAdd = () => { }; }, []); + // Reveal a non-blocking "taking a while" hint if the ceremony hasn't resolved. + // The component navigates away on success/error, so this only fires while the + // native prompt is still open (the perceived-hang case). Cleared on unmount. + useEffect(() => { + const timer = setTimeout(() => setShowSlowHint(true), slowHintDelayMs); + return () => clearTimeout(timer); + }, [slowHintDelayMs]); + useEffect(() => { if (ceremonyStarted.current) return; ceremonyStarted.current = true; @@ -157,7 +179,7 @@ export const PagePasskeyAdd = () => { ftlMsgResolver.getMsg('page-passkey-add-success', 'Passkey created') ); navigateToSettings(); - } catch (error) { + } catch (error: unknown) { // handleCancel already showed the cancellation banner and navigated; // suppress the AbortError side effects so they don't double-fire. if (!isMounted.current || wasCanceled.current) return; @@ -270,6 +292,19 @@ export const PagePasskeyAdd = () => { Follow the prompts on your device.

+ {showSlowHint && ( + + )}