fix(passkeys): send new-device login email and notifications on passkey sign-in#20723
fix(passkeys): send new-device login email and notifications on passkey sign-in#20723vpomerleau wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR aligns the passkey sign-in path with the password sign-in path by sending the missing post-sign-in security/notification signals (new-device-login email, attached-services login notification, metrics/security events) and by fixing a Settings Web-integration fallback that was incorrectly forwarding a first-party Settings clientId and mislabeling the email subject.
Changes:
- Send new-device-login email + attached-services login notification on passkey
authenticationFinish, and emit metrics/security events for non-Sync passkey sign-ins. - Forward flow
metricsContextthrough passkey auth finish so emails/metrics are attributed to the originating flow. - Gate forwarding of
clientIdas emailservicebehindisOAuthIntegration(in both passkey and password sign-in paths) to avoid mislabeling direct Settings sign-ins.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/fxa-settings/src/pages/Signin/container.tsx | Gate clientId forwarding behind isOAuthIntegration to avoid mislabeling Settings sign-ins. |
| packages/fxa-settings/src/pages/Signin/container.test.tsx | Add regression test ensuring WebIntegration fallback clientId is not forwarded as email service. |
| packages/fxa-settings/src/lib/passkeys/signin-flow.ts | Forward correct service (Firefox service vs OAuth clientId) and pass metricsContext into passkey finish. |
| packages/fxa-settings/src/lib/passkeys/signin-flow.test.tsx | Update/add tests for service forwarding rules and metricsContext forwarding in passkey sign-in. |
| packages/fxa-auth-server/lib/types.ts | Allow AuthClientInfoService.fetch to accept an optional service for generic “Mozilla” resolution. |
| packages/fxa-auth-server/lib/routes/passkeys.ts | Add post-sign-in notification sending for passkey auth finish; wire legacy mailer + oauth client info service; accept metricsContext. |
| packages/fxa-auth-server/lib/routes/passkeys.spec.ts | Add coverage for email subject behavior, legacy mailer fallback, attached-services notify, metrics + security events. |
| packages/fxa-auth-server/lib/routes/index.js | Pass mailer into passkeyRoutes wiring. |
| packages/fxa-auth-client/lib/client.ts | Extend passkey finish call to support forwarding metricsContext. |
| packages/functional-tests/tests/passkeyAuth/passkeyPasswordFallback.spec.ts | Assert new-device-login email subject for Sync passkey sign-in w/ password fallback. |
| packages/functional-tests/tests/passkeyAuth/passkey-signin.spec.ts | Assert new-device-login email subject for direct Settings (non-OAuth Web integration) passkey sign-in. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
vpomerleau
force-pushed
the
FXA-13905
branch
2 times, most recently
from
d714e54 to
62a597a
Compare
…ey sign-in
Because:
- Passkey sign-in sent no "New sign-in" email and skipped the
attached-services notification, metrics, and security event that password
sign-in emits; the email also lacked the relying-party name and flow
metrics context.
This commit:
- Sends the new-device-login email and notifyAttachedServices('login') on
passkey authenticationFinish (both sync and non-sync), reusing the shared
notifyAttachedServicesForAccountSession helper.
- Emits the account.login metrics event, flow signal, and security event for
non-sync sign-ins; the sync flow's /session/reauth already emits these.
- Names the relying party in the email for OAuth sign-ins (client forwards the
clientId), and uses the generic "New sign-in to your Mozilla account" for
non-OAuth and Sync sign-ins.
- Forwards the flow metricsContext through /passkey/authentication/finish so
the sign-in and its email are attributed to the flow.
- Only forwards the clientId for genuine OAuth integrations: a Web (non-OAuth)
integration's getClientId() can fall back to the first-party Settings client,
which must not name the email. Applied to both the password and passkey
paths so direct Settings sign-ins read "Mozilla account".
- Wires the legacy mailer + oauthClientInfoService into the passkey routes.
- Adds unit coverage (auth-server + settings) and functional assertions.
Closes: FXA-13905
vbudhram
left a comment
vbudhram
left a comment
There was a problem hiding this comment.
@vpomerleau Does this need to support Strapi customization? The new device emails are customized, but this wouldn't load them. Perhaps better as a follow up issue
| OLDSYNC_SCOPE | ||
| ); | ||
|
|
||
| const newDeviceLogin = await target.emailClient.waitForEmail( |
There was a problem hiding this comment.
Thanks for updating functional test
| ); | ||
| }); | ||
|
|
||
| it('forces the generic "Mozilla account" email for a sync sign-in (no Firefox/Sync framing before keys)', async () => { |
There was a problem hiding this comment.
The user intent was to sign into Sync, technically they didnt complete it. Still seems useful (maybe more simple?) to not have this special case.
There was a problem hiding this comment.
Product is requesting that we send a separate (additional) email when the Firefox sign-in is completed, that will go in a follow-up ticket - so this one notifies that access to the account was granted.
In testing these flows, I'd also noticed that emails were getting sent with "New sign-in to Firefox Account Settings", which we definitely don't want.
|
Good callout about the customization @vbudhram. We already have a generic fast-follow ticket for applying customization in the passkey pages and flows, so I added an additional requirement there about emails. |
Because
This pull request
notifyAttachedServices('login')on passkeyauthenticationFinish(both sync and non-sync), reusing the sharednotifyAttachedServicesForAccountSessionhelper.account.loginmetrics event, flow-complete signal, and security event for non-sync sign-ins; Sync skips these because/session/reauthalready emits them downstream (avoids double-counting).metricsContextthrough/passkey/authentication/finishso the sign-in and its email are attributed to the flow.clientIdFallback): a non-OAuth Web integration'sgetClientId()falls back to the first-party Settings client, which was sent as the emailserviceand mislabeled direct Settings sign-ins as "New sign-in to Firefox Accounts Settings". Now gated onisOAuthIntegrationin both the passkey (packages/fxa-settings/src/lib/passkeys/signin-flow.ts) and password (packages/fxa-settings/src/pages/Signin/container.tsx) sign-in paths, so direct Settings sign-ins read "Mozilla account" for both.oauthClientInfoServiceinto the passkey routes (packages/fxa-auth-server/lib/routes/passkeys.ts).Issue that this pull request solves
Closes: FXA-13905
Checklist
Put an
xin the boxes that applyHow to review (Optional)
passkeys.ts(sendPostSigninNotifications— the sync vs non-sync split); theisOAuthIntegrationclientId gate insignin-flow.tsandSignin/container.tsx.passkeys.ts→signin-flow.ts→container.tsx→ tests.account.loginat/session/reauth); the shared clientId-forwarding gate intentionally touches the password path.Screenshots (Optional)
N/A — no user-visible change (sign-in routing/notification logic only; no UI or FTL copy changed).
Other information (Optional)
/fxa-reviewrun — APPROVE (0 critical/high).passkeys.spec(66), settingssignin-flow(40) +Signincontainer (50), plus functional email-subject assertions (non-OAuth + two-step Sync). All green; functionalpasskeyAuth+signinsuites green.