Skip to content

bug(Settings): Respect browser's pairing version - #21327

Merged
dschom merged 1 commit into
mainfrom
FXA-14629
Sep 25, 2026
Merged

dschom merged 1 commit into
mainfrom
FXA-14629

Conversation

@dschom

@dschom dschom commented Sep 25, 2026

Copy link
Copy Markdown
Contributor

Because

  • If a browser signals it supports pairing.version 1, we should not show a pairing v2 flow

This pull request

  • Fixes the logic in v2 gate to respect the firefox desktop pref, identity.fxaccounts.pairing.version

Issue that this pull request solves

Closes: FXA-14629

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)

  • Open nightly desktop, got to about config, search for pairing.version, make sure it says 1
  • Sign in to sync
  • Initiate a pairing flow (e.g. Sync menu > Add a device)
  • Observe that the v1 pairing is displayed.

Screenshots (Optional)

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

Other information (Optional)

Any other information that is important to this pull request.

Because:
- If a browser signals it supports pairing.version 1, we should not show a pairing v2 flow

This Commit:
- Fixes the logic in v2 gate to respect the firefox desktop pref, identity.fxaccounts.pairing.version
@dschom
dschom requested a review from a team as a code owner September 25, 2026 20:02
@vbudhram
vbudhram requested a lite review from Copilot September 25, 2026 20:10

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.

Copilot review overview

🟡 Changes recommended

The v=2 route can bypass the new browser capability gate.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 High severity

Open (1)
What changed in this PR

Updates pairing v2 gating to respect the browser’s reported pairing capability.

Changes:

  • Require pairing version 2 for v2 flows.
  • Update unit and page-level tests.
  • Refresh configuration and launcher documentation.
  • Critical issue: the user-controlled v=2 route can still bypass the capability gate.
File Description
packages/​fxa-settings/​src/​pages/​Pair/​Index/​index.test.tsx Tests v1/v2 pairing behavior.
packages/​fxa-settings/​src/​pages/​ConnectAnotherDevice/​index.test.tsx Tests authority routing decisions.
packages/​fxa-settings/​src/​lib/​pairing/​v2-gate.ts Enforces browser pairing capability.
packages/​fxa-settings/​src/​lib/​pairing/​v2-gate.test.ts Tests capability and version gating.
packages/​fxa-settings/​src/​lib/​config.ts Updates pairing configuration documentation.
packages/​fxa-dev-launcher/​README.md Documents local pairing requirements.
packages/​fxa-content-server/​server/​lib/​configuration.js Updates configuration descriptions.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread packages/fxa-settings/src/lib/pairing/v2-gate.ts
@vbudhram

vbudhram commented Sep 25, 2026 •

Copy link
Copy Markdown
Contributor

Verification: fixed

Stack with PAIRING_VERSION=2 and PAIRING_V2_MIN_VERSION_DESKTOP=0. Nightly signed in, with identity.fxaccounts.pairing.version = 1.

Entry Before (89a6de9354) After (fcef4a5789)
/connect_another_device, /pair v2 /pair/authority/scan_qr, spinner that never stops v1 choice screen

Before left, after right

Recording, before:

video.mp4

Recording, after:

video.mp4

@dschom
dschom merged commit 5af5419 into main Sep 25, 2026
20 of 22 checks passed
@dschom
dschom deleted the FXA-14629 branch September 25, 2026 20:58
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