Conversation
dschom
force-pushed
the
FXA-13072
branch
7 times, most recently
from
6f7e194 to
2c1700c
Compare
dschom
force-pushed
the
FXA-13072
branch
2 times, most recently
from
d0b3abb to
f296a7b
Compare
There was a problem hiding this comment.
Pull request overview
Adds an end-to-end passkey registration UI to the FxA Settings “Security” section, including a new “Create passkey” route guarded by MFA and UI to list existing passkeys.
Changes:
- Introduces
/settings/passkeys/addwith an MFA-guarded “ceremony in progress” modal and WebAuthn registration completion. - Adds a Passkeys row to the Security section that fetches and renders the user’s passkeys (plus max-limit banner behavior).
- Extends the Account model with JWT-scoped helpers for starting/finishing passkey registration.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/fxa-settings/src/models/Account.ts | Adds JWT-scoped wrappers for begin/complete passkey registration. |
| packages/fxa-settings/src/components/Settings/index.tsx | Registers the MFA-guarded /passkeys/add route behind a feature flag. |
| packages/fxa-settings/src/components/Settings/Security/index.tsx | Shows the Passkeys unit row in the Security section behind a feature flag. |
| packages/fxa-settings/src/components/Settings/UnitRowPasskey/index.tsx | Fetches passkeys via auth-client and renders the Passkeys unit row + subrows. |
| packages/fxa-settings/src/components/Settings/UnitRow/index.tsx | Tweaks CTA rendering logic to allow disabled CTAs to render actions container. |
| packages/fxa-settings/src/components/Settings/SubRow/index.tsx | Updates passkey subrow typing and adds “sign in only” messaging + MFA-guarded delete modal. |
| packages/fxa-settings/src/components/Settings/PagePasskeyAdd/index.tsx | Implements the WebAuthn registration ceremony and success/error handling. |
| *.test.tsx / *.stories.tsx / *.ftl | Updates tests/stories and adds localization strings for the new passkey UI. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const [passkeys, setPasskeys] = useState<Passkey[]>([]); | ||
| const [loading, setLoading] = useState(true); | ||
|
|
||
| const fetchPasskeys = useCallback(async () => { |
There was a problem hiding this comment.
Passkey display & delete are basically the whole point of my ticket 😭 The way you did it isn't the best though (see my PR). It's nice to have them for testing registration stuff out, but I recommend not including them when you merge it. Or... you can copy everything from my PR and merge all these in one go 🤔 (in hindsight these two tickets really should be done at the same time by one person, bc they are kinda mutually blocking)
There was a problem hiding this comment.
Okay, I'll remove this. Then you can rebase on this when it lands.
There was a problem hiding this comment.
Actually how about this. I'll land this PR and then you can replace this gen ai code with whatever you have that is better. It's worth noting that I think the ticket break down is fine. It is noted in the tracker that this ticket is blocking your ticket. We probably should have sequenced the work better.
| const [passkeys, setPasskeys] = useState<Passkey[]>([]); | ||
| const [loading, setLoading] = useState(true); | ||
|
|
||
| const fetchPasskeys = useCallback(async () => { |
There was a problem hiding this comment.
Passkey display & delete are basically the whole point of my ticket 😭 The way you did it isn't the best though (see my PR). It's nice to have them for testing registration stuff out, but I recommend not including them when you merge it. Or... you can copy everything from my PR and merge all these in one go 🤔 (in hindsight these two tickets really should be done at the same time by one person, bc they are kinda mutually blocking)
vpomerleau
left a comment
vpomerleau
left a comment
There was a problem hiding this comment.
I checked out the branch and successfully tested the registration flow in local environment. 🙌
Some of the issues I spotted are flagged in my comments or the copilot review.
In addition, I noticed that registration with 1 password fails. Might not be a big deal since production doesn't support extensions but the fix looks to be simple (sent via Slack).
dschom
force-pushed
the
FXA-13072
branch
2 times, most recently
from
a943dfb to
3d81170
Compare
|
@MagentaManifold This ticket is a blocker for your ticket. So it should land first, and then you can rebase and change as needed. I've looked at your PR and it looks good to me. So feel free to make changes as you see fit. |
vpomerleau
left a comment
vpomerleau
left a comment
There was a problem hiding this comment.
Thanks Dan! I've added a few comments, but I don't think any are blockers since this is feature flagged and we have more work coming up to finalize. 🚀
dschom
force-pushed
the
FXA-13072
branch
2 times, most recently
from
68c91a6 to
bbe91b5
Compare
Because
This pull request
Issue that this pull request solves
Closes: FXA-13072
Checklist
Put an
xin the boxes that applyHow to review (Optional)
In your .env file set
FEATURE_FLAGS_PASSKEYS_ENABLED=trueandFEATURE_FLAGS_PASSKEY_REGISTRATION_ENABLED=trueStart he stack
Go to settings page
Try adding one more passkeys
Try deleting passkeys
Suggested review order: Storybooks, Manual Testing
Risky or complex parts: I have only really tested this on mac. I'll try to test on adroid and iphone today. I do not have a windows machine to test with, and haven't manually be be able to trip the 'unsupported' state.
Screenshots (Optional)
Please attach the screenshots of the changes made in case of change in user interface.
Other information (Optional)
This PR was done almost entirely by Claude by hooking up the Figma and Jira MCP servers and directing Claude to do the ticket. I've looked over the code, but will be taking a harder look now this is officially up for review. One other minor thing to note, I had to manually add support for deleting the passkey since that was in Figma, but not directly called out in the ticket.