Skip to content

feat(passkeys): Add passwordless sync feature flags - #20959

Open
nshirley wants to merge 2 commits into
mainfrom
FXA-13141
Open

feat(passkeys): Add passwordless sync feature flags#20959
nshirley wants to merge 2 commits into
mainfrom
FXA-13141

Conversation

@nshirley

@nshirley nshirley commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Because

  • Passwordless sync sign-in needs a feature flag on both the backend and the
    frontend before the routes and UI paths that use it can be gated in later
    tickets in this epic.
  • Both flags default off, so this lands dark and is safe to deploy ahead of any
    of the behaviour it will eventually gate.

This pull request

  • Adds passkeys.passwordlessSyncEnabled to the auth-server convict block in
    packages/fxa-auth-server/config/index.ts (default false, env
    PASSKEYS__PASSWORDLESS_SYNC_ENABLED).
  • Adds isPasskeyPasswordlessSyncEnabled to
    packages/fxa-auth-server/lib/passkey-utils.ts, which throws
    AppError.featureNotEnabled() unless both the primary passkeys.enabled flag
    and the new flag are on — the same shape as the existing
    isPasskeyRegistrationEnabled and isPasskeyAuthenticationEnabled guards.
  • Adds passkeyPasswordlessSyncEnabled to the content-server feature flags in
    configuration.js, and passes it through in both beta-settings.js and
    routes/react-app/route-definition-index.js, matching how passkeysEnabled
    is wired.
  • Adds the flag to the featureFlags interface in
    packages/fxa-settings/src/lib/config.ts.
  • Enables the flag in local config for both services
    (fxa-auth-server/config/dev.json and the content-server
    local.json-dist template).

Issue that this pull request solves

Closes: FXA-13141

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: lib/passkey-utils.ts for the guard, and the
    three content-server files for flag plumbing.
  • Suggested review order: auth-server config and guard, then the content-server
    pass-throughs, then the local config enablement.
  • Risky or complex parts: none functionally — nothing reads either flag yet.
    The thing worth checking is that no plumbing site was missed, since a missed
    one fails silently rather than loudly (see below).

Screenshots (Optional)

Please attach the screenshots of the changes made in case of change in user interface.

Other information (Optional)

Verified on the running local services, not just statically:

  • auth-server reports passkeys.passwordlessSyncEnabled: true in its resolved
    config after restart, with /__heartbeat__ returning 200.
  • content-server exposes passkeyPasswordlessSyncEnabled: true in the
    meta[name="fxa-config"] tag on localhost:3030 — which is the exact source
    the functional tests read via ConfigPage.getConfig().
  • packages/fxa-auth-server/lib/passkey-utils.spec.ts passes 13/13, and
    fxa-settings typechecks with 0 errors.

Heads-up for anyone running this locally: the content-server
server/config/local.json is gitignored, so only the local.json-dist template
change is in this diff. local.json overlays last, so until you add
"passkeyPasswordlessSyncEnabled": true to your own copy, the flag reads
false locally even though -dist has it on. Functional tests in this epic
test.skip on missing flags, so that shows up as a green run rather than a
failure. I hit this during verification.

Ticket correction: FXA-13141 says "note there is no isPasskeyFeatureEnabled".
It does exist, at packages/fxa-auth-server/lib/passkey-utils.ts:23, and three
passkey management routes use it. I followed the code rather than the ticket.

Because:

* Passwordless sync sign-in needs a flag on both sides before the routes
  and UI paths that use it can be gated.

This commit:

* Adds `passkeys.passwordlessSyncEnabled` to auth-server config with an
  `isPasskeyPasswordlessSyncEnabled` guard that throws when disabled.
* Adds `passkeyPasswordlessSyncEnabled` to the content-server feature
  flags and the settings `featureFlags` interface.
* Enables both in local config. Defaults stay false.
Because:

* "Master switch" is terminology we're moving away from.

This commit:

* Renames it in the passkeys convict docs on both servers and in the
  passkey-utils test names. Comments and test names only.
@nshirley
nshirley marked this pull request as ready for review August 4, 2026 17:27
@nshirley
nshirley requested a review from a team as a code owner August 4, 2026 17:27
Copilot AI review requested due to automatic review settings August 4, 2026 17:27

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 a new “passwordless sync via passkeys” feature flag across the auth-server and content-server config surfaces, plus a backend guard and unit tests, so future routes/UI can be safely gated behind a dark-launched flag (default off).

Changes:

  • Auth-server: add passkeys.passwordlessSyncEnabled convict config + isPasskeyPasswordlessSyncEnabled guard (throws AppError.featureNotEnabled() unless both flags are enabled).
  • Content-server: add featureFlags.passkeyPasswordlessSyncEnabled convict flag and plumb it through to frontend config/meta.
  • Frontend typing/local dev: extend fxa-settings Config.featureFlags interface and enable the flags in local dev config templates.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.

Show a summary per file
File Description
packages/fxa-auth-server/config/index.ts Adds passkeys.passwordlessSyncEnabled convict config (default false, env var wiring) and updates wording “Master” → “Primary”.
packages/fxa-auth-server/lib/passkey-utils.ts Adds isPasskeyPasswordlessSyncEnabled guard consistent with existing passkey flag guards.
packages/fxa-auth-server/lib/passkey-utils.spec.ts Adds unit coverage for the new guard, including “undefined flag” behavior.
packages/fxa-auth-server/config/dev.json Enables the new backend flag in local dev config.
packages/fxa-content-server/server/lib/configuration.js Adds featureFlags.passkeyPasswordlessSyncEnabled convict entry and updates “Master” → “Primary” wording.
packages/fxa-content-server/server/lib/beta-settings.js Plumbs the new content-server feature flag into injected settings config.
packages/fxa-content-server/server/lib/routes/react-app/route-definition-index.js Passes through passkeyPasswordlessSyncEnabled into the React app config payload.
packages/fxa-content-server/server/config/local.json-dist Enables the new content-server flag in the local config template.
packages/fxa-settings/src/lib/config.ts Extends Config.featureFlags typing to include passkeyPasswordlessSyncEnabled (and minor formatting fixes).

* Requires both the primary `passkeys.enabled` flag and `passkeys.passwordlessSyncEnabled`.
* @throws AppError.featureNotEnabled if either flag is disabled
*/
export function isPasskeyPasswordlessSyncEnabled(

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.

I've probably commented on this before, but I don't love the pattern that's been established here. It's pretty unconventional to have an 'is' function throw instead of just return false. I feel like semantically being clear that an error is being raised would be better. e.g. requirePasskeyPasswordlessSyncEnabled(...) : void 0.

I don't feel like this should be addressed in this PR, cause it'd make thing incosistent, but might be a nice polish PR / follow up.

@dschom dschom 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.

LGTM

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