Skip to content

feat(passkeys): request PRF at sign-in to record capability#20875

Open
nshirley wants to merge 1 commit into
mainfrom
FXA-14085
Open

feat(passkeys): request PRF at sign-in to record capability#20875
nshirley wants to merge 1 commit into
mainfrom
FXA-14085

Conversation

@nshirley

Copy link
Copy Markdown
Contributor

Because

  • Phase 2 (passwordless Sync) will unwrap Sync keys using the WebAuthn PRF
    extension at sign-in. Ahead of that we need to learn the true PRF capability
    of the existing passkey base as an early telemetry signal.
  • The registration-time prfEnabled flag is an incomplete, false-negative
    signal: it reads "no" for passkeys created before PRF-at-create was enabled,
    for platforms that only expose PRF at authentication, and for capability that
    appears later via an OS update. PRF capability is not fixed at creation for
    synced/platform passkeys, so evaluating at sign-in is the only way to learn
    the real capability of the base.

This pull request

  • Adds a scope-aware, default-off flag PASSKEYS__REQUEST_PRF_AT_AUTHENTICATION
    (off / keys-required / all) in packages/fxa-auth-server/config/index.ts
    and libs/accounts/passkey/src/lib/passkey.config.ts, and requests the PRF
    extension in generateWebauthnAuthenticationOptions
    (libs/accounts/passkey/src/lib/webauthn-adapter.ts) using the same static
    salt as registration. The scope decision is threaded via an optional
    keysRequired hint on POST /passkey/authentication/start.
  • In packages/fxa-settings/src/lib/passkeys/signin-flow.ts, detects PRF
    support from the get() output, strips the output (stripPrfResults in
    prf-fallback.ts) so it never reaches the server, and sends only a
    prfSupported boolean on authentication/finish. Adds a silent no-PRF retry
    (getCredentialWithPrfFallback).
  • Rolls prfEnabled forward false→true inside the verified
    verifyAuthenticationResponse flow
    (libs/accounts/passkey/src/lib/passkey.service.ts) — monotonic
    (updatePasskeyPrfEnabled in passkey.repository.ts) and best-effort, so a
    failure never blocks a verified sign-in.
  • Adds client Glean events (passkey.signin_prf_support,
    passkey.signin_retry_without_prf_request) and server StatsD
    (passkey.prf.signin.*) to back the kill switch.

Issue that this pull request solves

Closes: FXA-14085

Checklist

Put an x in the boxes that apply

  • My commit is GPG signed.
  • If applicable, I have modified or added tests which pass locally.
  • I have added necessary documentation (if appropriate).
  • I have verified that my changes render correctly in RTL (if appropriate).
  • I have manually reviewed all AI generated code.

How to review (Optional)

  • Key files/areas to focus on: webauthn-adapter.ts (PRF eval + scope helper),
    passkey.service.ts (roll-forward in the verify path), signin-flow.ts +
    prf-fallback.ts (detect / strip / retry), and routes/passkeys.ts (start
    hint + finish prfSupported).
  • Suggested review order: config/adapter → service roll-forward → route → client.
  • Risky or complex parts: PRF-output containment (must never reach the server)
    and the monotonic, best-effort roll-forward.

Screenshots (Optional)

No user-facing UI changes.

Other information (Optional)

  • Recording approach differs from the ticket's suggestion. Rather than a
    new session-token PATCH /passkey/{credentialId}/prf-enabled endpoint, this
    records capability via a prfSupported boolean on the existing
    authentication/finish request — smaller surface, no extra round trip, and no
    ownership re-check needed since the assertion already proves credential control.
  • Rollout: the flag ships off; the intended first enablement is
    keys-required (Sync flows, lowest blast radius), widening to all later for
    fuller population telemetry.

Comment thread libs/accounts/passkey/src/lib/passkey.repository.in.spec.ts
Comment thread libs/accounts/passkey/src/lib/passkey.repository.ts Outdated
Comment thread libs/accounts/passkey/src/lib/passkey.service.ts Outdated
Comment thread libs/accounts/passkey/src/lib/webauthn-adapter.spec.ts Outdated
Comment thread libs/accounts/passkey/src/lib/webauthn-adapter.ts Outdated
Comment thread libs/accounts/passkey/src/lib/webauthn-adapter.ts Outdated
Comment thread packages/fxa-auth-server/config/index.ts Outdated
Comment thread packages/fxa-auth-server/lib/routes/passkeys.ts Outdated
Comment thread packages/fxa-auth-server/lib/routes/passkeys.ts Outdated
Because:
 - Ahead of passwordless Sync (Phase 2) we need the true PRF capability of
   the passkey base; the registration-time prfEnabled flag under-reports it.

This commit:
 - Requests the WebAuthn PRF extension at passkey sign-in behind a new
   scope-aware, default-off flag (PASSKEYS__REQUEST_PRF_AT_AUTHENTICATION).
 - Detects PRF support client-side, strips the output so it never reaches
   the server, and sends only a boolean on authentication/finish.
 - Rolls prfEnabled forward false->true (monotonic, best-effort) inside the
   verified finish flow; never blocks sign-in.
 - Adds Glean + StatsD telemetry to back the kill switch.
@nshirley
nshirley marked this pull request as ready for review July 15, 2026 22:58
@nshirley
nshirley requested a review from a team as a code owner July 15, 2026 22:58
Copilot AI review requested due to automatic review settings July 15, 2026 22:58

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds opt-in PRF evaluation at passkey sign-in to measure real-world PRF capability (telemetry), while ensuring PRF outputs never reach the server and best-effort rolling prfEnabled forward upon verified authentication.

Changes:

  • Introduces a default-off, scope-based config flag to request PRF at authentication (off / keys-required / all) and threads a keysRequired hint through authentication/start.
  • Client: detects PRF output presence, strips PRF results before sending assertions to the server, and retries once without PRF on PRF-attributable failures; adds Glean events.
  • Server: optionally attaches PRF to authentication options and performs a monotonic, best-effort prfEnabled roll-forward during verified sign-in; adds StatsD signals.

Reviewed changes

Copilot reviewed 27 out of 27 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
packages/fxa-shared/metrics/glean/web/passkey.ts Adds new passkey sign-in PRF Glean event definitions.
packages/fxa-shared/metrics/glean/web/index.ts Extends allowed string event extra keys and maps new passkey events.
packages/fxa-shared/metrics/glean/fxa-ui-metrics.yaml Declares the two new passkey PRF-related event metrics and extras.
packages/fxa-settings/src/lib/passkeys/signin-flow.ts Implements PRF support detection, output stripping, keysRequired hinting, and finish payload prfSupported.
packages/fxa-settings/src/lib/passkeys/signin-flow.test.tsx Adds tests for PRF detection/stripping and keysRequired forwarding; adds toCoarsePlatform tests.
packages/fxa-settings/src/lib/passkeys/prf-fallback.ts Adds PRF support extraction, PRF result stripping, and a sign-in get() fallback retry without PRF.
packages/fxa-settings/src/lib/passkeys/prf-fallback.test.ts Adds unit tests for auth-time PRF fallback, extraction, and stripping helpers.
packages/fxa-settings/src/lib/glean/index.ts Wires new passkey PRF events into the Glean submission switch.
packages/fxa-settings/src/lib/glean/index.test.ts Adds tests validating the new Glean passkey PRF events and extras.
packages/fxa-auth-server/lib/routes/passkeys.ts Threads keysRequired into auth start; accepts prfSupported on finish and forwards to service; adds payload validation.
packages/fxa-auth-server/lib/routes/passkeys.spec.ts Adds/updates route tests for keysRequired and prfSupported forwarding.
packages/fxa-auth-server/config/index.ts Adds PASSKEYS__REQUEST_PRF_AT_AUTHENTICATION flag to server config.
packages/fxa-auth-client/lib/client.ts Adds keysRequired hint to beginPasskeyAuthentication and optional prfSupported to completePasskeyAuthentication.
libs/accounts/passkey/src/lib/webauthn-adapter.ts Adds PRF-at-auth decision helper and optional PRF extension attachment to authentication options.
libs/accounts/passkey/src/lib/webauthn-adapter.spec.ts Adds tests for PRF extension attachment and scope decision helper.
libs/accounts/passkey/src/lib/passkey.service.ts Requests PRF based on scope/keysRequired and performs best-effort monotonic roll-forward of prfEnabled.
libs/accounts/passkey/src/lib/passkey.service.spec.ts Adds tests for PRF gating metrics and prfEnabled roll-forward behavior.
libs/accounts/passkey/src/lib/passkey.repository.ts Adds monotonic updatePasskeyPrfEnabled DB update helper.
libs/accounts/passkey/src/lib/passkey.repository.in.spec.ts Adds integration tests for monotonic prfEnabled roll-forward semantics.
libs/accounts/passkey/src/lib/passkey.provider.ts Extends raw config typing to include requestPrfAtAuthentication.
libs/accounts/passkey/src/lib/passkey.provider.spec.ts Adds validation coverage for requestPrfAtAuthentication scope values.
libs/accounts/passkey/src/lib/passkey.manager.ts Adds setPasskeyPrfEnabled manager method wrapping the monotonic repository update.
libs/accounts/passkey/src/lib/passkey.manager.spec.ts Adds unit tests for setPasskeyPrfEnabled.
libs/accounts/passkey/src/lib/passkey.manager.in.spec.ts Updates integration config fixtures to include requestPrfAtAuthentication.
libs/accounts/passkey/src/lib/passkey.config.ts Adds PRF auth scopes/types and config field validation.
libs/accounts/passkey/src/lib/passkey.challenge.manager.spec.ts Updates config fixtures to include requestPrfAtAuthentication.
libs/accounts/passkey/src/lib/passkey.challenge.manager.in.spec.ts Updates integration config fixtures to include requestPrfAtAuthentication.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +428 to +434
const requestPrf = shouldRequestPrfAtAuth(
this.config.requestPrfAtAuthentication,
keysRequired
);
if (requestPrf) {
this.metrics.increment('passkey.prf.signin.requested');
}
@vpomerleau
vpomerleau self-requested a review July 16, 2026 18:04

@vpomerleau vpomerleau left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good! Just a few question, and I'll take this for a manual test run before circling back with approval!

// 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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this comment needed?

keysRequired
);
if (requestPrf) {
this.metrics.increment('passkey.prf.signin.requested');

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To align with other metrics names, should this be passkey.authentication.prf.requested?

* Whether to request the PRF extension at sign-in for the given scope.
* `keysRequired` only matters under the `keys-required` scope.
*/
export function shouldRequestPrfAtAuth(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks at the config - would it also be a good spot to centralize the flag and salt check?

.unknown(true)
.required(),
})
.unknown(true)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We may want to remove this unknown(true) as defence in depth to make sure that PRF output never reaches the server (or Sentry). We have stripUnknown set to true, so Joi should simply ignore any additional values that we aren't using without causing any breakage.

case 'passkey_signin_prf_support':
passkey.signinPrfSupport.record({
supported: gleanPingMetrics?.event?.['supported'] || '',
platform: gleanPingMetrics?.event?.['platform'] || '',

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is recording a platform extra needed, considering that Glean's built-in client_info.os is attached to every ping automatically?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants