diff --git a/libs/accounts/passkey/src/lib/passkey.challenge.manager.in.spec.ts b/libs/accounts/passkey/src/lib/passkey.challenge.manager.in.spec.ts index 01e1916231f..40c7234fdfe 100644 --- a/libs/accounts/passkey/src/lib/passkey.challenge.manager.in.spec.ts +++ b/libs/accounts/passkey/src/lib/passkey.challenge.manager.in.spec.ts @@ -58,6 +58,7 @@ beforeAll(async () => { challengeTimeout: 1000 * 60 * 5, requestPrfAtRegistration: false, prfSalt: '', + requestPrfAtAuthentication: 'off', }); const moduleRef = await buildTestModule(redis, config, mockLogger); @@ -149,6 +150,7 @@ describe('PasskeyChallengeManager (integration)', () => { challengeTimeout: 1000, requestPrfAtRegistration: false, prfSalt: '', + requestPrfAtAuthentication: 'off', }); const moduleRef = await buildTestModule(redis, shortConfig, mockLogger); diff --git a/libs/accounts/passkey/src/lib/passkey.challenge.manager.spec.ts b/libs/accounts/passkey/src/lib/passkey.challenge.manager.spec.ts index dcd7a3dd25f..25ca7957f36 100644 --- a/libs/accounts/passkey/src/lib/passkey.challenge.manager.spec.ts +++ b/libs/accounts/passkey/src/lib/passkey.challenge.manager.spec.ts @@ -37,6 +37,7 @@ function makeConfig(overrides: Partial = {}): PasskeyConfig { rpId: 'accounts.firefox.com', requestPrfAtRegistration: false, prfSalt: '', + requestPrfAtAuthentication: 'off', ...overrides, }); diff --git a/libs/accounts/passkey/src/lib/passkey.config.ts b/libs/accounts/passkey/src/lib/passkey.config.ts index 30bc8ec43fa..90190637338 100644 --- a/libs/accounts/passkey/src/lib/passkey.config.ts +++ b/libs/accounts/passkey/src/lib/passkey.config.ts @@ -25,6 +25,18 @@ import type { */ export const MAX_PASSKEY_NAME_LENGTH = 255; +/** + * Scope of the PRF-at-authentication request (see + * {@link PasskeyConfig.requestPrfAtAuthentication}). + * + * - `off`: never request PRF at sign-in (kill switch). + * - `keys-required`: request PRF only for keys-required (Sync) sign-ins — the + * Phase-2-relevant cohort, lowest blast radius. + * - `all`: request PRF for every passkey sign-in, for fuller telemetry. + */ +export const PRF_AUTH_SCOPES = ['off', 'keys-required', 'all'] as const; +export type PrfAuthScope = (typeof PRF_AUTH_SCOPES)[number]; + /** * Configuration for passkey (WebAuthn) functionality. * @@ -117,6 +129,13 @@ export class PasskeyConfig { }) public prfSalt!: string; + /** + * Scope of the PRF request at sign-in. Off by default (kill switch). + * @see {@link PrfAuthScope} + */ + @IsIn(PRF_AUTH_SCOPES) + public requestPrfAtAuthentication!: PrfAuthScope; + /** * Creates a new PasskeyConfig instance by copying all fields from the * provided options object. @@ -131,6 +150,7 @@ export class PasskeyConfig { this.maxPasskeysPerUser = opts.maxPasskeysPerUser; this.requestPrfAtRegistration = opts.requestPrfAtRegistration; this.prfSalt = opts.prfSalt; + this.requestPrfAtAuthentication = opts.requestPrfAtAuthentication; this.residentKey = opts.residentKey; this.rpId = opts.rpId; } diff --git a/libs/accounts/passkey/src/lib/passkey.manager.in.spec.ts b/libs/accounts/passkey/src/lib/passkey.manager.in.spec.ts index ed70da4cdf7..dd2acb33ff2 100644 --- a/libs/accounts/passkey/src/lib/passkey.manager.in.spec.ts +++ b/libs/accounts/passkey/src/lib/passkey.manager.in.spec.ts @@ -36,6 +36,7 @@ describe('PasskeyManager (Integration)', () => { challengeTimeout: 30_000, requestPrfAtRegistration: false, prfSalt: '', + requestPrfAtAuthentication: 'off', }); const mockMetrics = { diff --git a/libs/accounts/passkey/src/lib/passkey.manager.spec.ts b/libs/accounts/passkey/src/lib/passkey.manager.spec.ts index 20bf6be2b4e..7c4219f0765 100644 --- a/libs/accounts/passkey/src/lib/passkey.manager.spec.ts +++ b/libs/accounts/passkey/src/lib/passkey.manager.spec.ts @@ -29,6 +29,7 @@ jest.mock('./passkey.repository', () => ({ insertPasskey: jest.fn(), updatePasskeyCounterAndLastUsed: jest.fn(), updatePasskeyName: jest.fn(), + updatePasskeyPrfEnabled: jest.fn(), })); const mockDb = {} as unknown as AccountDatabase; @@ -43,6 +44,7 @@ const mockConfig = new PasskeyConfig({ maxPasskeysPerUser: MOCK_MAX_PASSKEYS_PER_USER, requestPrfAtRegistration: false, prfSalt: '', + requestPrfAtAuthentication: 'off', }); const mockMetrics = { @@ -247,4 +249,34 @@ describe('PasskeyManager', () => { expect(PasskeyRepository.updatePasskeyName).not.toHaveBeenCalled(); }); }); + + describe('setPasskeyPrfEnabled', () => { + const uid = Buffer.alloc(16, 1).toString('hex'); + const credentialId = Buffer.alloc(32, 2).toString('base64url'); + + it('returns true when the repository rolls the flag forward', async () => { + ( + PasskeyRepository.updatePasskeyPrfEnabled as jest.Mock + ).mockResolvedValue(1); + + const result = await manager.setPasskeyPrfEnabled(uid, credentialId); + + expect(result).toBe(true); + expect(PasskeyRepository.updatePasskeyPrfEnabled).toHaveBeenCalledWith( + mockDb, + uid, + credentialId + ); + }); + + it('returns false when no row was updated (already true, not found, or wrong user)', async () => { + ( + PasskeyRepository.updatePasskeyPrfEnabled as jest.Mock + ).mockResolvedValue(0); + + const result = await manager.setPasskeyPrfEnabled(uid, credentialId); + + expect(result).toBe(false); + }); + }); }); diff --git a/libs/accounts/passkey/src/lib/passkey.manager.ts b/libs/accounts/passkey/src/lib/passkey.manager.ts index 375cc8d241d..1914590bd01 100644 --- a/libs/accounts/passkey/src/lib/passkey.manager.ts +++ b/libs/accounts/passkey/src/lib/passkey.manager.ts @@ -21,6 +21,7 @@ import { PasskeyRecord, updatePasskeyCounterAndLastUsed, updatePasskeyName, + updatePasskeyPrfEnabled, } from './passkey.repository'; import { PasskeyConfig, MAX_PASSKEY_NAME_LENGTH } from './passkey.config'; import { AppError } from '@fxa/accounts/errors'; @@ -166,6 +167,28 @@ export class PasskeyManager { return rowsUpdated > 0; } + /** + * Roll a passkey's `prfEnabled` flag forward from false to true. + * + * Monotonic: never flips an already-true flag back to false. Both uid and + * credentialId must match to prevent one user from flipping another user's + * passkey. + * + * @returns true if the flag was rolled false→true; false if it was already + * true, the credential was not found, or it belongs to another user. + */ + async setPasskeyPrfEnabled( + uid: string, + credentialId: string + ): Promise { + const rowsUpdated = await updatePasskeyPrfEnabled( + this.db, + uid, + credentialId + ); + return rowsUpdated > 0; + } + /** * Delete a passkey for a user. * diff --git a/libs/accounts/passkey/src/lib/passkey.provider.spec.ts b/libs/accounts/passkey/src/lib/passkey.provider.spec.ts index 282c7c6403d..9c65d9479de 100644 --- a/libs/accounts/passkey/src/lib/passkey.provider.spec.ts +++ b/libs/accounts/passkey/src/lib/passkey.provider.spec.ts @@ -26,6 +26,7 @@ const VALID_RAW_CONFIG: RawPasskeyConfig = { authenticatorAttachment: '', requestPrfAtRegistration: false, prfSalt: '', + requestPrfAtAuthentication: 'off', }; function buildModule(rawPasskeys: unknown) { @@ -106,6 +107,7 @@ describe('PasskeyConfigProvider', () => { expect(config!.residentKey).toBe('required'); expect(config!.requestPrfAtRegistration).toBe(false); expect(config!.prfSalt).toBe(''); + expect(config!.requestPrfAtAuthentication).toBe('off'); }); it('copies a configured prfSalt', async () => { @@ -197,5 +199,17 @@ describe('PasskeyConfigProvider', () => { 'property prfSalt has failed the following constraints' ); }); + + it('rejects a requestPrfAtAuthentication value outside the allowed scopes', async () => { + await expect(() => + buildModule({ + ...VALID_RAW_CONFIG, + requestPrfAtAuthentication: + 'sometimes' as unknown as RawPasskeyConfig['requestPrfAtAuthentication'], + }) + ).rejects.toThrow( + 'property requestPrfAtAuthentication has failed the following constraints' + ); + }); }); }); diff --git a/libs/accounts/passkey/src/lib/passkey.provider.ts b/libs/accounts/passkey/src/lib/passkey.provider.ts index 614df205859..68a1b563b98 100644 --- a/libs/accounts/passkey/src/lib/passkey.provider.ts +++ b/libs/accounts/passkey/src/lib/passkey.provider.ts @@ -6,7 +6,7 @@ import { LoggerService } from '@nestjs/common'; import { ConfigService } from '@nestjs/config'; import { LOGGER_PROVIDER } from '@fxa/shared/log'; import Redis from 'ioredis'; -import { PasskeyConfig } from './passkey.config'; +import { PasskeyConfig, PrfAuthScope } from './passkey.config'; import { validateSync } from 'class-validator'; import type { AuthenticatorAttachment, @@ -32,6 +32,7 @@ export type RawPasskeyConfig = { authenticatorAttachment: AuthenticatorAttachment | ''; requestPrfAtRegistration: boolean; prfSalt: string; + requestPrfAtAuthentication: PrfAuthScope; }; /** diff --git a/libs/accounts/passkey/src/lib/passkey.repository.in.spec.ts b/libs/accounts/passkey/src/lib/passkey.repository.in.spec.ts index e663c6b21f5..639ed840e5b 100644 --- a/libs/accounts/passkey/src/lib/passkey.repository.in.spec.ts +++ b/libs/accounts/passkey/src/lib/passkey.repository.in.spec.ts @@ -144,7 +144,11 @@ describe('PasskeyRepository (Integration)', () => { const uid1 = await createTestAccount(); const uid2 = await createTestAccount(); const passkey = PasskeyFactory({ uid: uidBuffer(uid1), signCount: 0 }); - await PasskeyRepository.insertPasskey(db, uid1, toNewPasskeyData(passkey)); + await PasskeyRepository.insertPasskey( + db, + uid1, + toNewPasskeyData(passkey) + ); // Wrong uid — WHERE clause should prevent the update const success = await PasskeyRepository.updatePasskeyCounterAndLastUsed( @@ -163,4 +167,113 @@ describe('PasskeyRepository (Integration)', () => { expect(found?.signCount).toBe(0); }); }); + + describe('updatePasskeyPrfEnabled (monotonic)', () => { + it('rolls prfEnabled forward from false to true', async () => { + const uid = await createTestAccount(); + const passkey = PasskeyFactory({ + uid: uidBuffer(uid), + prfEnabled: false, + }); + await PasskeyRepository.insertPasskey(db, uid, toNewPasskeyData(passkey)); + + const rows = await PasskeyRepository.updatePasskeyPrfEnabled( + db, + uid, + passkey.credentialId.toString('base64url') + ); + + expect(rows).toBe(1); + const found = await PasskeyRepository.findPasskeyByCredentialId( + db, + passkey.credentialId.toString('base64url') + ); + expect(found?.prfEnabled).toBe(true); + }); + + it('is a no-op (0 rows) when prfEnabled is already true', async () => { + const uid = await createTestAccount(); + const passkey = PasskeyFactory({ uid: uidBuffer(uid), prfEnabled: true }); + await PasskeyRepository.insertPasskey(db, uid, toNewPasskeyData(passkey)); + + const rows = await PasskeyRepository.updatePasskeyPrfEnabled( + db, + uid, + passkey.credentialId.toString('base64url') + ); + + expect(rows).toBe(0); + const found = await PasskeyRepository.findPasskeyByCredentialId( + db, + passkey.credentialId.toString('base64url') + ); + expect(found?.prfEnabled).toBe(true); + }); + + it('does not update when uid does not match the credential owner', async () => { + const uid1 = await createTestAccount(); + const uid2 = await createTestAccount(); + const passkey = PasskeyFactory({ + uid: uidBuffer(uid1), + prfEnabled: false, + }); + await PasskeyRepository.insertPasskey( + db, + uid1, + toNewPasskeyData(passkey) + ); + + const rows = await PasskeyRepository.updatePasskeyPrfEnabled( + db, + uid2, + passkey.credentialId.toString('base64url') + ); + + expect(rows).toBe(0); + const found = await PasskeyRepository.findPasskeyByCredentialId( + db, + passkey.credentialId.toString('base64url') + ); + expect(found?.prfEnabled).toBe(false); + }); + it('does not update when the credential does not match', async () => { + const uid = await createTestAccount(); + const passkey1 = PasskeyFactory({ + uid: uidBuffer(uid), + prfEnabled: false, + }); + const passkey2 = PasskeyFactory({ + uid: uidBuffer(uid), + prfEnabled: false, + }); + await PasskeyRepository.insertPasskey( + db, + uid, + toNewPasskeyData(passkey1) + ); + await PasskeyRepository.insertPasskey( + db, + uid, + toNewPasskeyData(passkey2) + ); + + const rows = await PasskeyRepository.updatePasskeyPrfEnabled( + db, + uid, + passkey1.credentialId.toString('base64url') + ); + // Should update passkey1 but not passkey2 + expect(rows).toBe(1); + const found1 = await PasskeyRepository.findPasskeyByCredentialId( + db, + passkey1.credentialId.toString('base64url') + ); + const found2 = await PasskeyRepository.findPasskeyByCredentialId( + db, + passkey2.credentialId.toString('base64url') + ); + expect(found1?.prfEnabled).toBe(true); + expect(found2?.prfEnabled).toBe(false); + }); + }); }); diff --git a/libs/accounts/passkey/src/lib/passkey.repository.ts b/libs/accounts/passkey/src/lib/passkey.repository.ts index e3e0da75698..a934965b744 100644 --- a/libs/accounts/passkey/src/lib/passkey.repository.ts +++ b/libs/accounts/passkey/src/lib/passkey.repository.ts @@ -274,6 +274,31 @@ export async function updatePasskeyName( return Number(result.numUpdatedRows); } +/** + * Roll a passkey's `prfEnabled` flag forward from false to true. Monotonic: the + * `prfEnabled = false` guard means an already-true flag is never rewritten, so + * capability detected on one device can't be erased by a device lacking PRF. + * Scoped to both uid and credentialId so one user can't flip another's passkey. + * + * @returns Rows updated: 1 on a false→true flip, 0 when already true, not found, + * or owned by another user. + */ +export async function updatePasskeyPrfEnabled( + db: AccountDatabase, + uid: string, + credentialId: string +): Promise { + const result = await db + .updateTable('passkeys') + .set({ prfEnabled: 1 }) + .where('uid', '=', uuidTransformer.to(uid)) + .where('credentialId', '=', base64urlToBuffer(credentialId)) + .where('prfEnabled', '=', false) + .executeTakeFirst(); + + return Number(result.numUpdatedRows); +} + /** * Delete a specific passkey for a user. * diff --git a/libs/accounts/passkey/src/lib/passkey.service.spec.ts b/libs/accounts/passkey/src/lib/passkey.service.spec.ts index 7e57ce64d3b..c9884c4f028 100644 --- a/libs/accounts/passkey/src/lib/passkey.service.spec.ts +++ b/libs/accounts/passkey/src/lib/passkey.service.spec.ts @@ -20,6 +20,9 @@ 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', () => ({ + // shouldRequestPrfAtAuth is a pure scope-decision helper with no deps; use + // the real implementation so the service's PRF gating is actually exercised. + // (Also keeps the real UserVerificationRequiredError for instanceof checks.) ...jest.requireActual('./webauthn-adapter'), generateWebauthnRegistrationOptions: jest.fn(), verifyWebauthnRegistrationResponse: jest.fn(), @@ -99,6 +102,7 @@ describe('PasskeyService', () => { findPasskeyByCredentialId: jest.fn(), updatePasskeyAfterAuth: jest.fn(), renamePasskey: jest.fn(), + setPasskeyPrfEnabled: jest.fn(), deletePasskey: jest.fn(), }; @@ -128,6 +132,7 @@ describe('PasskeyService', () => { residentKey: 'required', requestPrfAtRegistration: false, prfSalt: '', + requestPrfAtAuthentication: 'off', }); beforeEach(async () => { @@ -631,6 +636,7 @@ describe('PasskeyService', () => { ).toHaveBeenCalledWith(mockConfig, { challenge: MOCK_CHALLENGE, allowCredentials: [], + requestPrf: false, }); }); @@ -642,7 +648,7 @@ describe('PasskeyService', () => { it('includes user credential IDs in allowCredentials when uid is provided', async () => { mockManager.listPasskeysForUser.mockResolvedValue([mockPasskey]); - await service.generateAuthenticationChallenge(MOCK_UID); + await service.generateAuthenticationChallenge({ uid: MOCK_UID }); expect(mockManager.listPasskeysForUser).toHaveBeenCalledWith(MOCK_UID); expect( @@ -650,21 +656,75 @@ describe('PasskeyService', () => { ).toHaveBeenCalledWith(mockConfig, { challenge: MOCK_CHALLENGE, allowCredentials: [MOCK_CREDENTIAL_ID], + requestPrf: false, }); }); it('passes empty allowCredentials when user has no passkeys', async () => { mockManager.listPasskeysForUser.mockResolvedValue([]); - await service.generateAuthenticationChallenge(MOCK_UID); + await service.generateAuthenticationChallenge({ uid: MOCK_UID }); expect( webauthnAdapter.generateWebauthnAuthenticationOptions ).toHaveBeenCalledWith(mockConfig, { challenge: MOCK_CHALLENGE, allowCredentials: [], + requestPrf: false, }); }); + + it('requests PRF and increments passkey.prf.signin.requested under the all scope', async () => { + const configAll = new PasskeyConfig({ + ...mockConfig, + requestPrfAtAuthentication: 'all', + }); + const serviceAll = new PasskeyService( + mockManager as unknown as PasskeyManager, + mockChallengeManager as unknown as PasskeyChallengeManager, + configAll, + mockMetrics as never, + mockLogger as never + ); + + await serviceAll.generateAuthenticationChallenge({ keysRequired: false }); + + expect( + webauthnAdapter.generateWebauthnAuthenticationOptions + ).toHaveBeenCalledWith(configAll, { + challenge: MOCK_CHALLENGE, + allowCredentials: [], + requestPrf: true, + }); + expect(mockMetrics.increment).toHaveBeenCalledWith( + 'passkey.prf.signin.requested' + ); + }); + + it('requests PRF under keys-required scope only when keysRequired is true', async () => { + const configKeysRequired = new PasskeyConfig({ + ...mockConfig, + requestPrfAtAuthentication: 'keys-required', + }); + const serviceKeysRequired = new PasskeyService( + mockManager as unknown as PasskeyManager, + mockChallengeManager as unknown as PasskeyChallengeManager, + configKeysRequired, + mockMetrics as never, + mockLogger as never + ); + + await serviceKeysRequired.generateAuthenticationChallenge({ + keysRequired: true, + }); + + expect( + webauthnAdapter.generateWebauthnAuthenticationOptions + ).toHaveBeenCalledWith( + configKeysRequired, + expect.objectContaining({ requestPrf: true }) + ); + }); }); describe('verifyWebauthnAuthenticationResponse', () => { @@ -884,6 +944,89 @@ describe('PasskeyService', () => { { reason: 'updateFailed' } ); }); + + describe('PRF roll-forward', () => { + it('rolls prfEnabled forward when prfSupported is true and the flag is currently false', async () => { + mockManager.setPasskeyPrfEnabled.mockResolvedValue(true); + + const result = await service.verifyAuthenticationResponse( + mockResponse, + MOCK_CHALLENGE, + undefined, + true + ); + + expect(result).toEqual({ uid: MOCK_UID }); + expect(mockManager.setPasskeyPrfEnabled).toHaveBeenCalledWith( + MOCK_UID, + MOCK_CREDENTIAL_ID + ); + expect(mockMetrics.increment).toHaveBeenCalledWith( + 'passkey.prf.signin.update_applied' + ); + }); + + it('does not attempt a roll-forward when prfSupported is false', async () => { + await service.verifyAuthenticationResponse( + mockResponse, + MOCK_CHALLENGE, + undefined, + false + ); + + expect(mockManager.setPasskeyPrfEnabled).not.toHaveBeenCalled(); + }); + + it('does not attempt a roll-forward when the flag is already true (in-memory guard)', async () => { + mockManager.findPasskeyByCredentialId.mockResolvedValue({ + ...mockPasskey, + prfEnabled: true, + }); + + await service.verifyAuthenticationResponse( + mockResponse, + MOCK_CHALLENGE, + undefined, + true + ); + + expect(mockManager.setPasskeyPrfEnabled).not.toHaveBeenCalled(); + }); + + it('increments update_noop when the row was already flipped by a concurrent write', async () => { + mockManager.setPasskeyPrfEnabled.mockResolvedValue(false); + + await service.verifyAuthenticationResponse( + mockResponse, + MOCK_CHALLENGE, + undefined, + true + ); + + expect(mockMetrics.increment).toHaveBeenCalledWith( + 'passkey.prf.signin.update_noop' + ); + }); + + it('still returns uid and never fails sign-in when the roll-forward rejects', async () => { + mockManager.setPasskeyPrfEnabled.mockRejectedValue( + new Error('db down') + ); + + const result = await service.verifyAuthenticationResponse( + mockResponse, + MOCK_CHALLENGE, + undefined, + true + ); + + expect(result).toEqual({ uid: MOCK_UID }); + expect(mockMetrics.increment).toHaveBeenCalledWith( + 'passkey.prf.signin.update_failed', + { reason: 'dbError' } + ); + }); + }); }); describe('listPasskeysForUser', () => { diff --git a/libs/accounts/passkey/src/lib/passkey.service.ts b/libs/accounts/passkey/src/lib/passkey.service.ts index 63c5bf99932..7e5c712bf4b 100644 --- a/libs/accounts/passkey/src/lib/passkey.service.ts +++ b/libs/accounts/passkey/src/lib/passkey.service.ts @@ -27,6 +27,7 @@ import { generateWebauthnAuthenticationOptions, verifyWebauthnAuthenticationResponse, UserVerificationRequiredError, + shouldRequestPrfAtAuth, } from './webauthn-adapter'; import { AppError } from '@fxa/accounts/errors'; @@ -404,13 +405,17 @@ export class PasskeyService { /** * Generate WebAuthn authentication (assertion) options. * - * @param uid - Optional user ID. When provided, restricts authentication to - * the user's registered credentials (known-user flow). + * @param options.uid - Optional user ID. When provided, restricts + * authentication to the user's registered credentials (known-user flow). + * @param options.keysRequired - Whether this sign-in is a keys-required + * (Sync) flow, used to decide whether to request PRF under the + * `keys-required` scope. Defaults to false. * @returns WebAuthn authentication options */ async generateAuthenticationChallenge( - uid?: string + options: { uid?: string; keysRequired?: boolean } = {} ): Promise { + const { uid, keysRequired = false } = options; const challenge = await this.challengeManager.generateAuthenticationChallenge(); @@ -420,9 +425,18 @@ export class PasskeyService { allowCredentials = passkeys.map((p) => p.credentialId); } + const requestPrf = shouldRequestPrfAtAuth( + this.config.requestPrfAtAuthentication, + keysRequired + ); + if (requestPrf) { + this.metrics.increment('passkey.prf.signin.requested'); + } + return await generateWebauthnAuthenticationOptions(this.config, { challenge, allowCredentials, + requestPrf, }); } @@ -433,6 +447,8 @@ export class PasskeyService { * @param challenge - The challenge that was issued for this authentication. * @param expectedUid - Optional expected user ID. When provided, verifies that * the authenticating passkey belongs to this user. + * @param prfSupported - Whether the browser returned a PRF output at `get()`; + * when true, rolls `prfEnabled` forward false→true. * @returns Authentication result containing the uid of the authenticated user. * @throws {AppError} passkeyNotFound if the credential is not registered. * @throws {AppError} passkeyChallengeExpired if the challenge is invalid or expired. @@ -442,7 +458,8 @@ export class PasskeyService { async verifyAuthenticationResponse( response: AuthenticationResponseJSON, challenge: string, - expectedUid?: string + expectedUid?: string, + prfSupported?: boolean ): Promise { const credentialId = response.id; @@ -538,6 +555,27 @@ export class PasskeyService { this.metrics.increment('passkey.authentication.success'); this.log?.log('passkey.authenticated', { uid }); + // Best-effort roll-forward of newly detected PRF capability; a failure must + // never fail a verified sign-in. The guard skips a write when already set. + if (prfSupported && !passkey.prfEnabled) { + try { + const applied = await this.passkeyManager.setPasskeyPrfEnabled( + uid, + credentialId + ); + this.metrics.increment( + applied + ? 'passkey.prf.signin.update_applied' + : 'passkey.prf.signin.update_noop' + ); + } catch (err) { + Sentry.captureException(err); + this.metrics.increment('passkey.prf.signin.update_failed', { + reason: 'dbError', + }); + } + } + return { uid }; } } diff --git a/libs/accounts/passkey/src/lib/webauthn-adapter.spec.ts b/libs/accounts/passkey/src/lib/webauthn-adapter.spec.ts index d89f74a0065..ba227268a6d 100644 --- a/libs/accounts/passkey/src/lib/webauthn-adapter.spec.ts +++ b/libs/accounts/passkey/src/lib/webauthn-adapter.spec.ts @@ -10,6 +10,7 @@ import { verifyWebauthnAuthenticationResponse, extractPrfEnabled, UserVerificationRequiredError, + shouldRequestPrfAtAuth, } from './webauthn-adapter'; import { PasskeyConfig } from './passkey.config'; import { VirtualAuthenticator } from './virtual-authenticator'; @@ -29,6 +30,7 @@ function testConfig(overrides: Partial = {}): PasskeyConfig { challengeTimeout: 30_000, requestPrfAtRegistration: false, prfSalt: '', + requestPrfAtAuthentication: 'off', ...overrides, }); } @@ -493,6 +495,64 @@ describe('generateWebauthnAuthenticationOptions', () => { expect.objectContaining({ id: credId }), ]); }); + + it('requests the PRF extension with the configured salt when requestPrf is true and a salt is set', async () => { + const options = await generateWebauthnAuthenticationOptions( + testConfig({ prfSalt: TEST_PRF_SALT }), + { + challenge: randomBytes(32).toString('base64url'), + allowCredentials: [], + requestPrf: true, + } + ); + + expect(options.extensions).toEqual( + expect.objectContaining({ prf: { eval: { first: TEST_PRF_SALT } } }) + ); + }); + + it('omits the PRF extension when requestPrf is true but no salt is configured', async () => { + const options = await generateWebauthnAuthenticationOptions( + testConfig({ prfSalt: '' }), + { + challenge: randomBytes(32).toString('base64url'), + allowCredentials: [], + requestPrf: true, + } + ); + + expect(options.extensions ?? {}).not.toHaveProperty('prf'); + }); + + it('omits the PRF extension when requestPrf is false even with a salt', async () => { + const options = await generateWebauthnAuthenticationOptions( + testConfig({ prfSalt: TEST_PRF_SALT }), + { + challenge: randomBytes(32).toString('base64url'), + allowCredentials: [], + requestPrf: false, + } + ); + + expect(options.extensions ?? {}).not.toHaveProperty('prf'); + }); +}); + +describe('shouldRequestPrfAtAuth', () => { + it('never requests PRF under the "off" scope', () => { + expect(shouldRequestPrfAtAuth('off', true)).toBe(false); + expect(shouldRequestPrfAtAuth('off', false)).toBe(false); + }); + + it('always requests PRF under the "all" scope', () => { + expect(shouldRequestPrfAtAuth('all', true)).toBe(true); + expect(shouldRequestPrfAtAuth('all', false)).toBe(true); + }); + + it('requests PRF under "keys-required" only for a keys-required sign-in', () => { + expect(shouldRequestPrfAtAuth('keys-required', true)).toBe(true); + expect(shouldRequestPrfAtAuth('keys-required', false)).toBe(false); + }); }); describe('verifyWebauthnAuthenticationResponse', () => { diff --git a/libs/accounts/passkey/src/lib/webauthn-adapter.ts b/libs/accounts/passkey/src/lib/webauthn-adapter.ts index c893020b8b6..33921b0d62b 100644 --- a/libs/accounts/passkey/src/lib/webauthn-adapter.ts +++ b/libs/accounts/passkey/src/lib/webauthn-adapter.ts @@ -23,7 +23,26 @@ import type { import { uuidTransformer } from '@fxa/shared/db/mysql/core'; -import { PasskeyConfig } from './passkey.config'; +import { PasskeyConfig, PrfAuthScope } from './passkey.config'; + +/** + * Whether to request the PRF extension at sign-in for the given scope. + * `keysRequired` only matters under the `keys-required` scope. + */ +export function shouldRequestPrfAtAuth( + scope: PrfAuthScope, + keysRequired: boolean +): boolean { + switch (scope) { + case 'all': + return true; + case 'keys-required': + return keysRequired; + case 'off': + default: + return false; + } +} /** * A credential to exclude from a registration ceremony. When forwarded to the @@ -231,6 +250,8 @@ export interface AuthenticationOptionsInput { * Pass an empty array for usernameless/discoverable flows. */ allowCredentials: string[]; + /** When true (and a `prfSalt` is configured), request the PRF extension. */ + requestPrf?: boolean; } /** @@ -253,6 +274,16 @@ export async function generateWebauthnAuthenticationOptions( input.allowCredentials.length > 0 ? input.allowCredentials.map((id) => ({ id })) : undefined, + // `eval` (single salt) fits the discoverable flow; an allow-listed ceremony + // could use `evalByCredential`. Gated on a salt like registration — an empty + // eval breaks Windows Hello (FXA-13991). Cast: bundled type predates PRF. + ...(input.requestPrf && config.prfSalt + ? { + extensions: { + prf: { eval: { first: config.prfSalt } }, + } as AuthenticationExtensionsClientInputs, + } + : {}), }); } diff --git a/packages/fxa-auth-client/lib/client.ts b/packages/fxa-auth-client/lib/client.ts index 1ec4e84ad97..33e7f70c4c7 100644 --- a/packages/fxa-auth-client/lib/client.ts +++ b/packages/fxa-auth-client/lib/client.ts @@ -3697,12 +3697,24 @@ export default class AuthClient { * `allowCredentials` so the browser uses discoverable credentials, which * prevents account enumeration. * + * @param options.keysRequired Hints that this is a keys-required (Sync) + * sign-in, so the server may request PRF under the keys-required scope. * @param headers Optional additional headers */ async beginPasskeyAuthentication( + options: { keysRequired?: boolean } = {}, headers?: Headers ): Promise { - return this.request('POST', '/passkey/authentication/start', {}, headers); + const payload = + options.keysRequired === undefined + ? {} + : { keysRequired: options.keysRequired }; + return this.request( + 'POST', + '/passkey/authentication/start', + payload, + headers + ); } /** @@ -3718,6 +3730,9 @@ export default class AuthClient { * the login is not yet complete here and the server defers its login * notifications/metrics until keys are available. The caller owns this * decision, based on the browser's keys-optional capability. + * @param options.prfSupported Whether the browser returned a PRF output at + * `get()`; rolls `prfEnabled` forward server-side. Omit when PRF wasn't + * requested. Never the PRF output itself. * @param headers Optional additional headers */ async completePasskeyAuthentication( @@ -3726,6 +3741,7 @@ export default class AuthClient { options: { keysRequired: boolean; service?: string; + prfSupported?: boolean; metricsContext?: MetricsContext; }, headers?: Headers @@ -3735,12 +3751,16 @@ export default class AuthClient { challenge: string; keysRequired: boolean; service?: string; + prfSupported?: boolean; metricsContext?: MetricsContext; } = { response, challenge, keysRequired: options.keysRequired, ...(options.service ? { service: options.service } : {}), + ...(options.prfSupported !== undefined + ? { prfSupported: options.prfSupported } + : {}), ...(options.metricsContext ? { metricsContext: options.metricsContext } : {}), diff --git a/packages/fxa-auth-server/config/index.ts b/packages/fxa-auth-server/config/index.ts index 4119380cf43..41b4075952e 100644 --- a/packages/fxa-auth-server/config/index.ts +++ b/packages/fxa-auth-server/config/index.ts @@ -2737,6 +2737,12 @@ const convictConf = convict({ env: 'PASSKEYS__PRF_SALT', format: String, }, + requestPrfAtAuthentication: { + default: 'off', + doc: 'Scope of the WebAuthn PRF request at sign-in. "off" (kill switch) never requests PRF; "keys-required" requests it only for keys-required (Sync) sign-ins; "all" requests it for every passkey sign-in.', + env: 'PASSKEYS__REQUEST_PRF_AT_AUTHENTICATION', + format: ['off', 'keys-required', 'all'], + }, }, twilio: { credentialMode: { diff --git a/packages/fxa-auth-server/lib/routes/passkeys.spec.ts b/packages/fxa-auth-server/lib/routes/passkeys.spec.ts index 65ee11e8c1e..97adbf5ee1e 100644 --- a/packages/fxa-auth-server/lib/routes/passkeys.spec.ts +++ b/packages/fxa-auth-server/lib/routes/passkeys.spec.ts @@ -1092,7 +1092,19 @@ describe('passkeys routes', () => { ).toHaveBeenCalledTimes(1); expect( mockPasskeyService.generateAuthenticationChallenge - ).toHaveBeenCalledWith(); + ).toHaveBeenCalledWith({ keysRequired: undefined }); + }); + + it('forwards the keysRequired hint from the payload to the service', async () => { + await runTest('/passkey/authentication/start', { + auth: { credentials: {} }, + payload: { keysRequired: true }, + app: { ua: {} }, + }); + + expect( + mockPasskeyService.generateAuthenticationChallenge + ).toHaveBeenCalledWith({ keysRequired: true }); }); it('records glean.passkey.authenticationStarted with the request', async () => { @@ -1154,7 +1166,12 @@ describe('passkeys routes', () => { expect( mockPasskeyService.verifyAuthenticationResponse - ).toHaveBeenCalledWith(payload.response, payload.challenge); + ).toHaveBeenCalledWith( + payload.response, + payload.challenge, + undefined, + undefined + ); expect(db.createPasskeyVerifiedSessionToken).toHaveBeenCalledWith( expect.objectContaining({ uid: UID }) ); @@ -1166,6 +1183,23 @@ describe('passkeys routes', () => { }); }); + it('forwards prfSupported from the payload to verifyAuthenticationResponse', async () => { + await runTest('/passkey/authentication/finish', { + auth: { credentials: {} }, + app: { ua: {} }, + payload: { ...payload, prfSupported: true }, + }); + + expect( + mockPasskeyService.verifyAuthenticationResponse + ).toHaveBeenCalledWith( + payload.response, + payload.challenge, + undefined, + true + ); + }); + it('sets hasPassword false for passwordless accounts', async () => { db.account.mockResolvedValueOnce({ uid: UID, diff --git a/packages/fxa-auth-server/lib/routes/passkeys.ts b/packages/fxa-auth-server/lib/routes/passkeys.ts index e70368811ae..f8ba432b241 100644 --- a/packages/fxa-auth-server/lib/routes/passkeys.ts +++ b/packages/fxa-auth-server/lib/routes/passkeys.ts @@ -382,13 +382,21 @@ export class PasskeyHandler { * browser. No email or uid is accepted in the request body — discoverable * credentials only. * - * @param request - Unauthenticated Hapi request. + * @param request - Unauthenticated Hapi request. An optional `keysRequired` + * flag in the payload lets the server decide, under the `keys-required` + * PRF scope, whether to request the PRF extension in the returned options. * @returns WebAuthn authentication options to pass to `navigator.credentials.get`. */ async authenticationStart(request: AuthRequest) { await this.customs.checkIpOnly(request, 'passkeyAuthStart'); - const options = await this.service.generateAuthenticationChallenge(); + const { keysRequired } = (request.payload ?? {}) as { + keysRequired?: boolean; + }; + + const options = await this.service.generateAuthenticationChallenge({ + keysRequired, + }); this.glean.passkey.authenticationStarted(request); @@ -403,24 +411,29 @@ export class PasskeyHandler { * with flags the UI needs to drive the post-login flow. * * @param request - Unauthenticated Hapi request containing `response`, - * `challenge`, and optional `service` / `keysRequired` in the payload. + * `challenge`, and optional `service` / `keysRequired` / `prfSupported` in + * the payload. * @returns Session token, uid, `verified`, and `hasPassword`. */ async authenticationFinish(request: AuthRequest) { await this.customs.checkIpOnly(request, 'passkeyAuthFinish'); - const { response, challenge, service, keysRequired } = request.payload as { - response: AuthenticationResponseJSON; - challenge: string; - service?: string; - keysRequired: boolean; - }; + const { response, challenge, service, keysRequired, prfSupported } = + request.payload as { + response: AuthenticationResponseJSON; + challenge: string; + service?: string; + keysRequired: boolean; + prfSupported?: boolean; + }; let uid: string; try { ({ uid } = await this.service.verifyAuthenticationResponse( response, - challenge + challenge, + undefined, + prfSupported )); } catch (err) { await recordSecurityEvent('account.passkey.authentication_failure', { @@ -906,6 +919,12 @@ export const passkeyRoutes = ( ...PASSKEYS_API_DOCS.PASSKEY_AUTHENTICATION_START_POST, pre: [{ method: authenticationEnabledCheck }], auth: false, + validate: { + payload: isA.object({ + // Hint for the keys-required PRF scope; omitted = not keys-required. + keysRequired: isA.boolean().optional(), + }), + }, response: { schema: isA.object({ challenge: isA.string().required(), @@ -959,6 +978,9 @@ export const passkeyRoutes = ( // When true, this login still needs Sync-scoped keys obtained via a // follow-up step, so the server defers login notifications/metrics. keysRequired: isA.boolean().required(), + // Whether the browser returned a PRF output at get(); rolls + // prfEnabled forward. Never the PRF output itself. + prfSupported: isA.boolean().optional(), metricsContext: METRICS_CONTEXT_SCHEMA, }), }, diff --git a/packages/fxa-settings/src/lib/glean/index.test.ts b/packages/fxa-settings/src/lib/glean/index.test.ts index e075343835b..3691df91d53 100644 --- a/packages/fxa-settings/src/lib/glean/index.test.ts +++ b/packages/fxa-settings/src/lib/glean/index.test.ts @@ -13,6 +13,7 @@ import * as event from 'fxa-shared/metrics/glean/web/event'; import * as reg from 'fxa-shared/metrics/glean/web/reg'; import * as login from 'fxa-shared/metrics/glean/web/login'; import * as accountPref from 'fxa-shared/metrics/glean/web/accountPref'; +import * as passkey from 'fxa-shared/metrics/glean/web/passkey'; import * as accountBanner from 'fxa-shared/metrics/glean/web/accountBanner'; import * as deleteAccount from 'fxa-shared/metrics/glean/web/deleteAccount'; import * as thirdPartyAuth from 'fxa-shared/metrics/glean/web/thirdPartyAuth'; @@ -830,6 +831,37 @@ describe('lib/glean', () => { sinon.assert.calledOnce(setEventReasonStub); sinon.assert.calledWith(setEventReasonStub, reason); }); + + it('submits passkey_signin_prf_support with supported and platform extras', async () => { + const spy = sandbox.spy(passkey.signinPrfSupport, 'record'); + GleanMetrics.passkey.signinPrfSupport({ + event: { supported: 'present', platform: 'macos' }, + }); + await GleanMetrics.isDone(); + sinon.assert.calledOnce(setEventNameStub); + sinon.assert.calledWith(setEventNameStub, 'passkey_signin_prf_support'); + sinon.assert.calledOnceWithExactly(spy, { + supported: 'present', + platform: 'macos', + }); + }); + + it('submits passkey_signin_retry_without_prf_request with reason and outcome', async () => { + const spy = sandbox.spy(passkey.signinRetryWithoutPrfRequest, 'record'); + GleanMetrics.passkey.signinRetryWithoutPrfRequest({ + event: { reason: 'UnknownError', outcome: 'success' }, + }); + await GleanMetrics.isDone(); + sinon.assert.calledOnce(setEventNameStub); + sinon.assert.calledWith( + setEventNameStub, + 'passkey_signin_retry_without_prf_request' + ); + sinon.assert.calledOnceWithExactly(spy, { + reason: 'UnknownError', + outcome: 'success', + }); + }); }); describe('loginTotpBackup', () => { diff --git a/packages/fxa-settings/src/lib/glean/index.ts b/packages/fxa-settings/src/lib/glean/index.ts index d3d8ab4d27b..2fbc54f220f 100644 --- a/packages/fxa-settings/src/lib/glean/index.ts +++ b/packages/fxa-settings/src/lib/glean/index.ts @@ -885,6 +885,18 @@ const recordEventMetric = ( reason: gleanPingMetrics?.event?.['reason'] || '', }); break; + case 'passkey_signin_prf_support': + passkey.signinPrfSupport.record({ + supported: gleanPingMetrics?.event?.['supported'] || '', + platform: gleanPingMetrics?.event?.['platform'] || '', + }); + break; + case 'passkey_signin_retry_without_prf_request': + passkey.signinRetryWithoutPrfRequest.record({ + reason: gleanPingMetrics?.event?.['reason'] || '', + outcome: gleanPingMetrics?.event?.['outcome'] || '', + }); + break; case 'passkey_enter_password_view': passkeyEnterPassword.view.record({ reason: gleanPingMetrics?.event?.['reason'] || '', diff --git a/packages/fxa-settings/src/lib/passkeys/prf-fallback.test.ts b/packages/fxa-settings/src/lib/passkeys/prf-fallback.test.ts index b1e795aaa56..34a7ff61898 100644 --- a/packages/fxa-settings/src/lib/passkeys/prf-fallback.test.ts +++ b/packages/fxa-settings/src/lib/passkeys/prf-fallback.test.ts @@ -4,12 +4,16 @@ import { createCredentialWithPrfFallback, + getCredentialWithPrfFallback, + extractPrfSupport, isRetriableWithoutPrf, stripPrfExtension, + stripPrfResults, } from './prf-fallback'; -import { createCredential } from './webauthn'; +import { createCredential, getCredential } from './webauthn'; import type { PublicKeyCredentialCreationOptionsJSON, + PublicKeyCredentialRequestOptionsJSON, PublicKeyCredentialJSON, } from './webauthn'; @@ -19,6 +23,9 @@ jest.mock('./webauthn'); const mockCreateCredential = createCredential as jest.MockedFunction< typeof createCredential >; +const mockGetCredential = getCredential as jest.MockedFunction< + typeof getCredential +>; const PRF_SALT = 'dGVzdC1wcmYtc2FsdA'; @@ -293,3 +300,136 @@ describe('createCredentialWithPrfFallback', () => { expect(onPrfFallback).not.toHaveBeenCalled(); }); }); + +const requestOptionsWithPrf: PublicKeyCredentialRequestOptionsJSON = { + challenge: 'Y2hhbGxlbmdl', + userVerification: 'required', + extensions: { prf: { eval: { first: PRF_SALT } } }, +}; + +const assertionResult: PublicKeyCredentialJSON = { + id: 'bW9jay1pZA', + rawId: 'bW9jay1yYXctaWQ', + type: 'public-key', + response: { + clientDataJSON: 'bW9jay1jbGllbnQtZGF0YQ', + authenticatorData: 'bW9jay1hdXRoLWRhdGE', + signature: 'bW9jay1zaWduYXR1cmU', + }, + clientExtensionResults: {}, +}; + +const assertionWithPrfOutput: PublicKeyCredentialJSON = { + ...assertionResult, + clientExtensionResults: { prf: { results: { first: new ArrayBuffer(32) } } }, +}; + +describe('getCredentialWithPrfFallback', () => { + it('returns the credential from the first attempt without retrying on success', async () => { + mockGetCredential.mockResolvedValueOnce(assertionWithPrfOutput); + + const result = await getCredentialWithPrfFallback(requestOptionsWithPrf); + + expect(result).toBe(assertionWithPrfOutput); + expect(mockGetCredential).toHaveBeenCalledTimes(1); + }); + + it('retries once without PRF on a retriable UnknownError', async () => { + mockGetCredential + .mockRejectedValueOnce(new DOMException('transient', 'UnknownError')) + .mockResolvedValueOnce(assertionResult); + + const result = await getCredentialWithPrfFallback(requestOptionsWithPrf); + + expect(result).toBe(assertionResult); + expect(mockGetCredential).toHaveBeenCalledTimes(2); + const [retriedOptions] = mockGetCredential.mock.calls[1]; + expect(retriedOptions.extensions ?? {}).not.toHaveProperty('prf'); + }); + + it('does not retry when the options carried no PRF extension', async () => { + const noPrf: PublicKeyCredentialRequestOptionsJSON = { + ...requestOptionsWithPrf, + extensions: undefined, + }; + mockGetCredential.mockRejectedValueOnce( + new DOMException('transient', 'UnknownError') + ); + + await expect(getCredentialWithPrfFallback(noPrf)).rejects.toThrow( + 'transient' + ); + expect(mockGetCredential).toHaveBeenCalledTimes(1); + }); + + it('does not retry a user-cancel (NotAllowedError) and rethrows it', async () => { + const cancel = new DOMException('cancelled', 'NotAllowedError'); + mockGetCredential.mockRejectedValueOnce(cancel); + + await expect( + getCredentialWithPrfFallback(requestOptionsWithPrf) + ).rejects.toBe(cancel); + expect(mockGetCredential).toHaveBeenCalledTimes(1); + }); + + it('reports the retry outcome to onPrfFallback', async () => { + const onPrfFallback = jest.fn(); + mockGetCredential + .mockRejectedValueOnce(new DOMException('transient', 'UnknownError')) + .mockResolvedValueOnce(assertionResult); + + await getCredentialWithPrfFallback( + requestOptionsWithPrf, + undefined, + onPrfFallback + ); + + expect(onPrfFallback).toHaveBeenCalledWith({ + reason: 'UnknownError', + outcome: 'success', + }); + }); +}); + +describe('extractPrfSupport', () => { + it('returns true when the get() output carries a PRF result', () => { + expect(extractPrfSupport(assertionWithPrfOutput)).toBe(true); + }); + + it('returns false when there is no PRF result', () => { + expect(extractPrfSupport(assertionResult)).toBe(false); + }); + + it('returns false when prf is present but results are absent', () => { + expect( + extractPrfSupport({ + ...assertionResult, + clientExtensionResults: { prf: {} }, + }) + ).toBe(false); + }); +}); + +describe('stripPrfResults', () => { + it('removes the prf results from clientExtensionResults', () => { + const stripped = stripPrfResults(assertionWithPrfOutput); + expect(stripped.clientExtensionResults).not.toHaveProperty('prf'); + }); + + it('preserves other clientExtensionResults entries', () => { + const stripped = stripPrfResults({ + ...assertionResult, + clientExtensionResults: { + prf: { results: { first: new ArrayBuffer(8) } }, + credProps: { rk: true }, + }, + }); + expect(stripped.clientExtensionResults).toEqual({ + credProps: { rk: true }, + }); + }); + + it('returns the same reference when there is no prf result', () => { + expect(stripPrfResults(assertionResult)).toBe(assertionResult); + }); +}); diff --git a/packages/fxa-settings/src/lib/passkeys/prf-fallback.ts b/packages/fxa-settings/src/lib/passkeys/prf-fallback.ts index 15707888fea..5c58b9a1932 100644 --- a/packages/fxa-settings/src/lib/passkeys/prf-fallback.ts +++ b/packages/fxa-settings/src/lib/passkeys/prf-fallback.ts @@ -4,8 +4,11 @@ import { createCredential, + getCredential, DEFAULT_TIMEOUT_MS, + type AuthenticationExtensionsJSON, type PublicKeyCredentialCreationOptionsJSON, + type PublicKeyCredentialRequestOptionsJSON, type PublicKeyCredentialJSON, } from './webauthn'; import { WebAuthnErrorType } from './webauthn-errors'; @@ -34,14 +37,15 @@ export function isRetriableWithoutPrf(error: unknown): boolean { } /** - * Returns the creation options with the PRF extension removed, preserving any - * other extensions. Used to retry registration without PRF when an - * authenticator rejects the PRF eval. Returns the original object unchanged - * (same reference) when there is no PRF extension to strip. + * Returns WebAuthn options with the PRF extension removed, preserving any other + * extensions. Works for both creation (registration) and request + * (authentication) options — the retry drops PRF when an authenticator rejects + * the PRF eval. Returns the original object unchanged (same reference) when + * there is no PRF extension to strip. */ -export function stripPrfExtension( - options: PublicKeyCredentialCreationOptionsJSON -): PublicKeyCredentialCreationOptionsJSON { +export function stripPrfExtension< + T extends { extensions?: AuthenticationExtensionsJSON }, +>(options: T): T { if (!options.extensions?.prf) { return options; } @@ -56,6 +60,43 @@ export function stripPrfExtension( }; } +/** + * True when the browser returned a PRF output at `get()`. At authentication the + * browser returns the PRF _output_ (`prf.results.first`), not the `prf.enabled` + * boolean seen at creation, so its presence is the support signal. Reads + * presence only — never the output value itself. + */ +export function extractPrfSupport( + credential: Pick +): boolean { + const prf = ( + credential.clientExtensionResults as { + prf?: { results?: { first?: unknown } }; + } + ).prf; + return prf?.results?.first != null; +} + +/** + * Returns the credential with any PRF extension results removed from + * `clientExtensionResults`, so the PRF output never reaches the server. + * + * Binary PRF outputs are `ArrayBuffer`s that already drop out when the + * credential is JSON-stringified for the wire, but this makes the guarantee + * explicit and independent of that serialisation quirk. Returns the original + * object unchanged (same reference) when there is no PRF result to strip. + */ +export function stripPrfResults( + credential: PublicKeyCredentialJSON +): PublicKeyCredentialJSON { + const results = credential.clientExtensionResults as { prf?: unknown }; + if (!results.prf) { + return credential; + } + const { prf: _prf, ...rest } = results; + return { ...credential, clientExtensionResults: rest }; +} + /** * Diagnostics surfaced when the PRF fallback retry fires. Lets the caller emit * telemetry (e.g. Glean) without coupling this util to any metrics layer, so we @@ -129,3 +170,55 @@ export async function createCredentialWithPrfFallback( throw error; } } + +/** + * Runs the authentication ceremony, silently retrying once without the PRF + * extension if the first attempt fails in a way PRF could have caused. + * + * The authentication analog of {@link createCredentialWithPrfFallback}: PRF is + * requested at sign-in only to detect capability (never to block sign-in), so a + * PRF-attributable failure must fall back to a normal, PRF-free assertion. Only + * an `UnknownError` triggers the retry, so a user cancel (`NotAllowedError`) or + * a timeout (`TimeoutError`) is non-retriable and never causes a re-prompt. + * + * Only retries when the original options actually carried a PRF extension — + * otherwise stripping is a no-op and the same error would recur. The retry is + * bounded by what remains of the original `timeoutMs` budget so the two + * attempts together never exceed a single timeout window. + * + * When the retry fires, `onPrfFallback` is invoked exactly once with the + * triggering error name and the retry outcome — purely for telemetry. + */ +export async function getCredentialWithPrfFallback( + options: PublicKeyCredentialRequestOptionsJSON, + timeoutMs?: number, + onPrfFallback?: (info: PrfFallbackInfo) => void +): Promise { + const startedAt = Date.now(); + try { + return await getCredential(options, timeoutMs); + } catch (error) { + if (isRetriableWithoutPrf(error) && options.extensions?.prf) { + const budget = timeoutMs ?? DEFAULT_TIMEOUT_MS; + const remaining = Math.max(0, budget - (Date.now() - startedAt)); + // With no budget left, a retry (timeoutMs=0) would abort immediately and + // throw a TimeoutError that masks the original error, so rethrow the + // original unchanged instead. + if (remaining > 0) { + const reason = (error as DOMException).name; + try { + const credential = await getCredential( + stripPrfExtension(options), + remaining + ); + onPrfFallback?.({ reason, outcome: 'success' }); + return credential; + } catch (retryError) { + onPrfFallback?.({ reason, outcome: 'failure' }); + throw retryError; + } + } + } + throw error; + } +} diff --git a/packages/fxa-settings/src/lib/passkeys/signin-flow.test.tsx b/packages/fxa-settings/src/lib/passkeys/signin-flow.test.tsx index 9296a53c526..0343c5ab0ab 100644 --- a/packages/fxa-settings/src/lib/passkeys/signin-flow.test.tsx +++ b/packages/fxa-settings/src/lib/passkeys/signin-flow.test.tsx @@ -10,6 +10,7 @@ import React from 'react'; import { usePasskeySignIn, resolvePasskeyService, + toCoarsePlatform, type PasskeySignInAuthClient, type PasskeySignInIntegration, } from './signin-flow'; @@ -73,6 +74,8 @@ jest.mock('../glean', () => ({ passkey: { buttonView: jest.fn(), authSuccess: jest.fn(), + signinPrfSupport: jest.fn(), + signinRetryWithoutPrfRequest: jest.fn(), }, }, })); @@ -960,7 +963,9 @@ describe('usePasskeySignIn', () => { 'fires passkey.auth_success with reason=%s on the no-Sync-password branch (surface=%s)', async (surface, expectedReason) => { const { args } = buildArgs({ surface }); - const { result } = renderHook(() => usePasskeySignIn(args), { wrapper }); + const { result } = renderHook(() => usePasskeySignIn(args), { + wrapper, + }); await act(async () => { await result.current.onClick(); }); @@ -1170,6 +1175,121 @@ describe('usePasskeySignIn', () => { ); }); }); + + describe('PRF at sign-in', () => { + const optionsWithPrf = { + challenge: CHALLENGE, + userVerification: 'required', + extensions: { prf: { eval: { first: 'c2FsdA' } } }, + }; + const credentialWithPrf = { + ...MOCK_CREDENTIAL, + clientExtensionResults: { + prf: { results: { first: new ArrayBuffer(32) } }, + }, + }; + + it('passes the keysRequired hint to beginPasskeyAuthentication', async () => { + const { args, spies } = buildArgs({ + integration: { + isSync: () => false, + isFirefoxNonSync: () => false, + isFirefoxMobileClient: () => false, + requiresPasswordForLogin: () => true, + } as unknown as PasskeySignInIntegration, + }); + + const { result } = renderHook(() => usePasskeySignIn(args), { wrapper }); + await act(async () => { + await result.current.onClick(); + }); + + expect(spies.beginPasskeyAuthentication).toHaveBeenCalledWith({ + keysRequired: true, + }); + }); + + it('records present support and sends prfSupported=true in the finish request when the output is present', async () => { + const { args, spies } = buildArgs(); + spies.beginPasskeyAuthentication.mockResolvedValue(optionsWithPrf); + (getCredential as jest.Mock).mockResolvedValue(credentialWithPrf); + + const { result } = renderHook(() => usePasskeySignIn(args), { wrapper }); + await act(async () => { + await result.current.onClick(); + }); + + expect(GleanMetrics.passkey.signinPrfSupport).toHaveBeenCalledWith({ + event: { supported: 'present', platform: expect.any(String) }, + }); + const [, , options] = spies.completePasskeyAuthentication.mock.calls[0]; + expect(options.prfSupported).toBe(true); + }); + + it('strips the PRF output from the credential before sending it to the server', async () => { + const { args, spies } = buildArgs(); + spies.beginPasskeyAuthentication.mockResolvedValue(optionsWithPrf); + (getCredential as jest.Mock).mockResolvedValue(credentialWithPrf); + + const { result } = renderHook(() => usePasskeySignIn(args), { wrapper }); + await act(async () => { + await result.current.onClick(); + }); + + const [sentCredential] = + spies.completePasskeyAuthentication.mock.calls[0]; + expect(sentCredential.clientExtensionResults).not.toHaveProperty('prf'); + }); + + it('records absent support and sends prfSupported=false when no output is present', async () => { + const { args, spies } = buildArgs(); + spies.beginPasskeyAuthentication.mockResolvedValue(optionsWithPrf); + (getCredential as jest.Mock).mockResolvedValue(MOCK_CREDENTIAL); + + const { result } = renderHook(() => usePasskeySignIn(args), { wrapper }); + await act(async () => { + await result.current.onClick(); + }); + + expect(GleanMetrics.passkey.signinPrfSupport).toHaveBeenCalledWith({ + event: { supported: 'absent', platform: expect.any(String) }, + }); + const [, , options] = spies.completePasskeyAuthentication.mock.calls[0]; + expect(options.prfSupported).toBe(false); + }); + + it('omits prfSupported and the support event when the server did not request PRF', async () => { + const { args, spies } = buildArgs(); + // Default beginPasskeyAuthentication returns options without extensions. + (getCredential as jest.Mock).mockResolvedValue(MOCK_CREDENTIAL); + + const { result } = renderHook(() => usePasskeySignIn(args), { wrapper }); + await act(async () => { + await result.current.onClick(); + }); + + expect(GleanMetrics.passkey.signinPrfSupport).not.toHaveBeenCalled(); + const [, , options] = spies.completePasskeyAuthentication.mock.calls[0]; + expect(options).not.toHaveProperty('prfSupported'); + }); + }); +}); + +describe('toCoarsePlatform', () => { + it('maps common user agents to a coarse platform, falling back to "other"', () => { + expect(toCoarsePlatform('Mozilla/5.0 (Windows NT 10.0; Win64; x64)')).toBe( + 'windows' + ); + expect(toCoarsePlatform('Mozilla/5.0 (Linux; Android 14)')).toBe('android'); + expect( + toCoarsePlatform('Mozilla/5.0 (iPhone; CPU iPhone OS 17_0 like Mac OS X)') + ).toBe('ios'); + expect( + toCoarsePlatform('Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7)') + ).toBe('macos'); + expect(toCoarsePlatform('Mozilla/5.0 (X11; Linux x86_64)')).toBe('linux'); + expect(toCoarsePlatform('some-unknown-agent')).toBe('other'); + }); }); describe('resolvePasskeyService', () => { diff --git a/packages/fxa-settings/src/lib/passkeys/signin-flow.ts b/packages/fxa-settings/src/lib/passkeys/signin-flow.ts index c39145001cf..231a3188367 100644 --- a/packages/fxa-settings/src/lib/passkeys/signin-flow.ts +++ b/packages/fxa-settings/src/lib/passkeys/signin-flow.ts @@ -29,11 +29,15 @@ import { resolveServiceOrClientId } from '../../models/integrations/utils'; import { queryParamsToMetricsContext } from '../metrics'; import type { QueryParams } from '../..'; import { - getCredential, handleWebAuthnError, isWebAuthnSupported, type PublicKeyCredentialJSON, } from './'; +import { + extractPrfSupport, + getCredentialWithPrfFallback, + stripPrfResults, +} from './prf-fallback'; import type { PasskeySignInGleanReason } from './webauthn-errors'; /** @@ -193,6 +197,21 @@ export type PasskeySignInAuthClient = Pick< | 'sessionResendVerifyCode' >; +/** + * Coarse OS/platform label for PRF-support telemetry, derived from the user + * agent. Deliberately low-cardinality (no versions) to segment PRF capability + * without fingerprinting. + */ +export function toCoarsePlatform(userAgent: string): string { + const ua = userAgent.toLowerCase(); + if (/windows/.test(ua)) return 'windows'; + if (/android/.test(ua)) return 'android'; + if (/iphone|ipad|ipod/.test(ua)) return 'ios'; + if (/mac os|macintosh/.test(ua)) return 'macos'; + if (/linux/.test(ua)) return 'linux'; + return 'other'; +} + /** * Shape of an entry in `authClient.account(...)`'s `emails` array. The * auth-client return type isn't formally typed; this local interface @@ -304,18 +323,39 @@ export function usePasskeySignIn({ setIsLoading(true); gleanEvents.submit(); + // True when this login still needs Sync-scoped keys that a follow-up + // password step will provide. Computed up front so it can also hint the + // server whether to request PRF under the keys-required scope; reused below + // to route to the password step. + const keysRequired = integration.requiresPasswordForLogin( + supportsKeysOptionalLogin + ); + try { // Discoverable credentials only — the Signin page's email field is // intentionally ignored. The browser surfaces all credentials for the - // RP and the user picks one. - const challengeOptions = await authClient.beginPasskeyAuthentication(); + // RP and the user picks one. The keysRequired hint lets the server decide + // whether to attach the PRF extension to the returned options. + const challengeOptions = await authClient.beginPasskeyAuthentication({ + keysRequired, + }); // Isolated try/catch so a network-layer TypeError (e.g. fetch failure) // from surrounding auth-client calls can't be miscategorised as a // WebAuthn error. let credential: PublicKeyCredentialJSON; try { - credential = await getCredential(challengeOptions); + // If the server attached PRF and the first attempt fails in a way PRF + // could have caused, retry once without PRF (never blocks sign-in). + credential = await getCredentialWithPrfFallback( + challengeOptions, + undefined, + ({ reason, outcome }) => { + GleanMetrics.passkey.signinRetryWithoutPrfRequest({ + event: { reason, outcome }, + }); + } + ); } catch (err) { if (err instanceof DOMException || err instanceof TypeError) { const categorized = handleWebAuthnError( @@ -331,22 +371,33 @@ export function usePasskeySignIn({ throw err; } + // Read support (presence only) before stripping the output from the + // credential; only meaningful when the server requested PRF. + const prfRequested = !!challengeOptions.extensions?.prf; + const prfSupported = prfRequested && extractPrfSupport(credential); + if (prfRequested) { + GleanMetrics.passkey.signinPrfSupport({ + event: { + supported: prfSupported ? 'present' : 'absent', + platform: toCoarsePlatform(navigator.userAgent), + }, + }); + } + credential = stripPrfResults(credential); + const serviceForRequest = resolvePasskeyService(integration); const metricsContext = queryParamsToMetricsContext(flowQueryParams); - const keysRequired = integration.requiresPasswordForLogin( - supportsKeysOptionalLogin - ); const completion = await authClient.completePasskeyAuthentication( credential, challengeOptions.challenge, { ...(serviceForRequest ? { service: serviceForRequest } : {}), - // True when this login still needs Sync-scoped keys that a - // follow-up password step will provide. The server uses it to - // defer its login metrics/email framing until keys exist; the - // client uses the same value below to route to that step. + // The server uses keysRequired to defer its login metrics/email + // framing until keys exist; the client uses the same value below to + // route to that step. keysRequired, + ...(prfRequested ? { prfSupported } : {}), metricsContext, } ); diff --git a/packages/fxa-shared/metrics/glean/fxa-ui-metrics.yaml b/packages/fxa-shared/metrics/glean/fxa-ui-metrics.yaml index e06744350a5..6178f1ccc2e 100644 --- a/packages/fxa-shared/metrics/glean/fxa-ui-metrics.yaml +++ b/packages/fxa-shared/metrics/glean/fxa-ui-metrics.yaml @@ -2946,6 +2946,69 @@ passkey: 'otplogin' and 'alternative_auth' are no-password surfaces, so they pair only with 'nopassword'/'createdpassword' (never 'withpassword'). type: string + signin_prf_support: + type: event + description: | + Records whether the WebAuthn PRF extension produced an output at passkey + sign-in (`navigator.credentials.get()`), i.e. whether the credential + supports PRF on this device. Early telemetry ahead of passwordless Sync + (Phase 2) to learn the true PRF capability of the passkey base, which the + registration-time `prfEnabled` flag under-reports. Only the presence of + the PRF output is recorded — never the output itself. + send_in_pings: + - events + notification_emails: + - vzare@mozilla.com + - fxa-staff@mozilla.com + bugs: + - https://mozilla-hub.atlassian.net/browse/FXA-14085 + data_reviews: + - https://bugzilla.mozilla.org/show_bug.cgi?id=1830504 + expires: never + data_sensitivity: + - interaction + extra_keys: + supported: + description: | + Whether the PRF output was present: "present" or "absent". + type: string + platform: + description: | + Coarse OS/platform label for the sign-in device, used to segment PRF + support (e.g. "windows", "macos", "android", "ios", "linux", + "other"). Derived client-side from the user agent; no fine-grained + version or fingerprinting data. + type: string + signin_retry_without_prf_request: + type: event + description: | + A passkey sign-in's first WebAuthn attempt failed in a way the optional + PRF extension could have caused (e.g. Windows Hello UnknownError, + FXA-13991), so the ceremony silently retried once without PRF. Mirrors + `account_pref.passkey_create_retry_without_prf_request` for the sign-in + (`navigator.credentials.get()`) path. + send_in_pings: + - events + notification_emails: + - vzare@mozilla.com + - fxa-staff@mozilla.com + bugs: + - https://mozilla-hub.atlassian.net/browse/FXA-14085 + data_reviews: + - https://bugzilla.mozilla.org/show_bug.cgi?id=1830504 + expires: never + data_sensitivity: + - interaction + extra_keys: + reason: + description: | + The DOMException name of the first-attempt error that triggered the + retry (e.g. UnknownError). + type: string + outcome: + description: | + Whether the retry without PRF succeeded: "success" or "failure". + type: string account_pref: view: diff --git a/packages/fxa-shared/metrics/glean/web/index.ts b/packages/fxa-shared/metrics/glean/web/index.ts index 7042b4a757b..cd9d73b0b38 100644 --- a/packages/fxa-shared/metrics/glean/web/index.ts +++ b/packages/fxa-shared/metrics/glean/web/index.ts @@ -23,7 +23,11 @@ export const stringEventPropertyNames = [ // String event extras passed to specific events but not backed by a global // `event`-module metric, so they are not set globally in populateMetrics. -export const stringEventExtraPropertyNames = ['mobile_device_count'] as const; +export const stringEventExtraPropertyNames = [ + 'mobile_device_count', + 'supported', + 'platform', +] as const; export type PropertyNameStringT = typeof stringEventPropertyNames; export type PropertyNameString = @@ -181,6 +185,8 @@ export const eventsMap = { passkey: { buttonView: 'passkey_button_view', authSuccess: 'passkey_auth_success', + signinPrfSupport: 'passkey_signin_prf_support', + signinRetryWithoutPrfRequest: 'passkey_signin_retry_without_prf_request', }, cad: { diff --git a/packages/fxa-shared/metrics/glean/web/passkey.ts b/packages/fxa-shared/metrics/glean/web/passkey.ts index bd7274e6068..6f9196a7bbc 100644 --- a/packages/fxa-shared/metrics/glean/web/passkey.ts +++ b/packages/fxa-shared/metrics/glean/web/passkey.ts @@ -48,3 +48,50 @@ export const buttonView = new EventMetricType<{ }, ['reason'] ); + +/** + * Records whether the WebAuthn PRF extension produced an output at passkey + * sign-in (`navigator.credentials.get()`), i.e. whether the credential + * supports PRF on this device. Early telemetry ahead of passwordless Sync + * (Phase 2) to learn the true PRF capability of the passkey base, which the + * registration-time `prfEnabled` flag under-reports. Only the presence of + * the PRF output is recorded — never the output itself. + * + * Generated from `passkey.signin_prf_support`. + */ +export const signinPrfSupport = new EventMetricType<{ + platform?: string; + supported?: string; +}>( + { + category: 'passkey', + name: 'signin_prf_support', + sendInPings: ['events'], + lifetime: 'ping', + disabled: false, + }, + ['platform', 'supported'] +); + +/** + * A passkey sign-in's first WebAuthn attempt failed in a way the optional + * PRF extension could have caused (e.g. Windows Hello UnknownError, + * FXA-13991), so the ceremony silently retried once without PRF. Mirrors + * `account_pref.passkey_create_retry_without_prf_request` for the sign-in + * (`navigator.credentials.get()`) path. + * + * Generated from `passkey.signin_retry_without_prf_request`. + */ +export const signinRetryWithoutPrfRequest = new EventMetricType<{ + outcome?: string; + reason?: string; +}>( + { + category: 'passkey', + name: 'signin_retry_without_prf_request', + sendInPings: ['events'], + lifetime: 'ping', + disabled: false, + }, + ['outcome', 'reason'] +);