Conversation
Because: - HackerOne #3678692 reports that an attacker holding a victim's email and password can register a device on an unverified session with attacker-controlled Web Push encryption keys, then trigger /session/verify/send_push from a duplicated session. The server encrypts the OTP with the attacker's public key and delivers it via AutoPush, allowing the attacker to decrypt the code and verify the session, fully bypassing sign-in confirmation. This commit: - Removes POST /session/verify/send_push and POST /session/verify/verify_push from fxa-auth-server, plus their swagger entries and unit tests. - Removes the verifyLogin push reason and notifyVerifyLoginRequest helper from lib/push.js and the push mock. - Removes sendLoginPushRequest, verifyLoginPushRequest, and the duplicate sendPushLoginRequest from fxa-auth-client. - Deletes the SigninPushCode and SigninPushCodeConfirm pages, routes, query-param model, and PushAuthImage helper from fxa-settings. - Deletes the legacy push-login views, templates, mixin, experiment, router entries, account/client delegate, and react-app route lists in fxa-content-server, and unwires the experiment registration. - Drops the dead smtp.pushVerificationUrl config set. Email OTP, TOTP, and recovery-code sign-in confirmation paths are unchanged. Hardening of POST /account/device and POST /session/duplicate to require verifiedSessionToken (also flagged in the report as enabling the attack chain) is out of scope here and tracked as a follow-up.
Because: - The push sign-in confirmation channel was removed in the previous commit, but the two FTL message IDs the deleted notifyVerifyLoginRequest helper consumed were left in the auth-server l10n source. They were still being extracted to translator deliverables despite having no runtime consumer. This commit: - Removes session-verify-send-push-title-2 and session-verify-send-push-body-2 from packages/fxa-auth-server/lib/l10n/server.ftl. The corresponding fxa-settings strings (signin-push-code-*) live in src-level page FTL files that were already deleted with the SigninPushCode and SigninPushCodeConfirm pages, so the generated public/locales/en output will rebuild without them on the next merge-ftl run.
vbudhram
commented
May 7, 2026
|
|
||
| const SESSION_SEND_PUSH_POST = { | ||
| ...TAGS_SESSION, | ||
| description: '/session/verify/send_push', |
Contributor
Author
There was a problem hiding this comment.
Normally I would say lets return for 501 (not implemented) or some similar code, but not clients have ever used this endpoint so fair to completely remove.
Contributor
There was a problem hiding this comment.
Pull request overview
Removes the unused “push-based sign-in verification” feature across FxA services (auth-server, auth-client, settings, and content-server), including endpoints, UI/routes, experiments, assets, docs, and tests.
Changes:
- Deletes auth-server push verification routes (
/session/verify/send_push,/session/verify/verify_push) plus associated push command/reason support and localization/doc entries. - Removes auth-client APIs and settings/content-server UI flows (React + legacy Backbone), including routing/experiment wiring and test coverage.
- Cleans up related mocks, assets, and query-param models that only supported the removed flow.
Reviewed changes
Copilot reviewed 47 out of 49 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| packages/fxa-settings/src/pages/Signin/SigninPushCodeConfirm/mocks.tsx | Removes story/test mock for deleted push confirm page |
| packages/fxa-settings/src/pages/Signin/SigninPushCodeConfirm/index.tsx | Deletes push login confirmation page UI |
| packages/fxa-settings/src/pages/Signin/SigninPushCodeConfirm/index.stories.tsx | Removes Storybook stories for deleted page |
| packages/fxa-settings/src/pages/Signin/SigninPushCodeConfirm/greencheck.svg | Removes page-specific asset |
| packages/fxa-settings/src/pages/Signin/SigninPushCodeConfirm/en.ftl | Removes page-specific localization strings |
| packages/fxa-settings/src/pages/Signin/SigninPushCodeConfirm/container.tsx | Deletes container that called push verification API |
| packages/fxa-settings/src/pages/Signin/SigninPushCodeConfirm/container.test.tsx | Removes unit tests for deleted container |
| packages/fxa-settings/src/pages/Signin/SigninPushCode/mocks.tsx | Removes story/test mock for deleted push code page |
| packages/fxa-settings/src/pages/Signin/SigninPushCode/interfaces.ts | Removes props interface for deleted push code component |
| packages/fxa-settings/src/pages/Signin/SigninPushCode/index.tsx | Deletes push code page UI |
| packages/fxa-settings/src/pages/Signin/SigninPushCode/index.test.tsx | Removes unit tests for deleted page |
| packages/fxa-settings/src/pages/Signin/SigninPushCode/index.stories.tsx | Removes Storybook stories for deleted page |
| packages/fxa-settings/src/pages/Signin/SigninPushCode/en.ftl | Removes page-specific localization strings |
| packages/fxa-settings/src/pages/Signin/SigninPushCode/container.tsx | Deletes push code container (push send + polling) |
| packages/fxa-settings/src/pages/Signin/SigninPushCode/container.test.tsx | Removes unit tests for deleted container |
| packages/fxa-settings/src/models/pages/signin/push-signin-query-params.ts | Removes query-param model used only by push confirm flow |
| packages/fxa-settings/src/components/images/index.tsx | Removes exported push-auth illustration component |
| packages/fxa-settings/src/components/images/graphic_push_factor_auth.svg | Deletes push-auth illustration asset |
| packages/fxa-settings/src/components/App/index.tsx | Unwires lazy imports and routes for push sign-in pages |
| packages/fxa-content-server/server/lib/routes/react-app/index.js | Removes push routes from React route groups |
| packages/fxa-content-server/server/lib/routes/react-app/content-server-routes.js | Removes push routes from frontend route allowlist |
| packages/fxa-content-server/app/tests/test_start.js | Stops loading deleted push view + experiment rule tests |
| packages/fxa-content-server/app/tests/spec/views/push/send_login.js | Deletes tests for legacy push “send login” view |
| packages/fxa-content-server/app/tests/spec/views/push/confirm_login.js | Deletes tests for legacy push “confirm login” view |
| packages/fxa-content-server/app/tests/spec/views/push/completed.js | Deletes tests for legacy push “completed” view |
| packages/fxa-content-server/app/tests/spec/views/mixins/signin-mixin.js | Removes test wiring for deleted push experiment mixin |
| packages/fxa-content-server/app/tests/spec/lib/experiments/grouping-rules/push.js | Deletes tests for removed push experiment grouping rule |
| packages/fxa-content-server/app/tests/spec/lib/experiment.js | Updates manual experiment name used by test after push experiment removal |
| packages/fxa-content-server/app/scripts/views/push/send_login.js | Deletes legacy push “send login” Backbone view |
| packages/fxa-content-server/app/scripts/views/push/confirm_login.js | Deletes legacy push “confirm login” Backbone view |
| packages/fxa-content-server/app/scripts/views/push/completed.js | Deletes legacy push “completed” Backbone view |
| packages/fxa-content-server/app/scripts/views/mixins/signin-mixin.js | Removes push-experiment dependency and navigation branch |
| packages/fxa-content-server/app/scripts/views/mixins/push-login-experiment-mixin.js | Deletes push experiment mixin |
| packages/fxa-content-server/app/scripts/templates/push/send_login.mustache | Deletes legacy push template |
| packages/fxa-content-server/app/scripts/templates/push/confirm_login.mustache | Deletes legacy push template |
| packages/fxa-content-server/app/scripts/templates/push/completed.mustache | Deletes legacy push template |
| packages/fxa-content-server/app/scripts/models/account.js | Removes account model method that triggered push login requests |
| packages/fxa-content-server/app/scripts/lib/router.js | Removes push and push-react routes from the client router |
| packages/fxa-content-server/app/scripts/lib/fxa-client.js | Removes client delegate for sending push login requests |
| packages/fxa-content-server/app/scripts/lib/experiments/grouping-rules/push.js | Deletes push experiment grouping rule implementation |
| packages/fxa-content-server/app/scripts/lib/experiment.js | Removes pushLogin from the manual experiments registry |
| packages/fxa-auth-server/test/mocks.js | Removes push mock method name for deleted push notifier |
| packages/fxa-auth-server/lib/routes/session.spec.ts | Deletes unit tests for removed session push endpoints |
| packages/fxa-auth-server/lib/routes/session.js | Removes /session/verify/send_push and /session/verify/verify_push routes and supporting logic |
| packages/fxa-auth-server/lib/push.js | Removes push command/reason and notifier for “verify login” push |
| packages/fxa-auth-server/lib/l10n/server.ftl | Removes orphaned FTL strings used only by removed push flow |
| packages/fxa-auth-server/docs/swagger/session-api.ts | Removes swagger docs entries for deleted endpoints |
| packages/fxa-auth-server/config/index.ts | Removes unused smtp push verification URL default |
| packages/fxa-auth-client/lib/client.ts | Removes auth-client methods that called deleted session push endpoints |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Because
This pull request
POST /session/verify/send_pushandPOST /session/verify/verify_pushfromfxa-auth-serveralong with their swagger docs and unit tests; dropsnotifyVerifyLoginRequest, theverifyLoginpush reason, andLOGIN_REQUESTpush command fromlib/push.js; cleans the test push mock and a deadsmtp.pushVerificationUrlconfig set.sendLoginPushRequest,verifyLoginPushRequest, andsendPushLoginRequestfromfxa-auth-client.SigninPushCode,SigninPushCodeConfirm,push-signin-query-params, andPushAuthImagefromfxa-settings; unwires the lazy routes inApp/index.tsx.views/push/*,templates/push/*,push-login-experiment-mixin, the push grouping rule, and thepushLoginexperiment registration infxa-content-server; updatessignin-mixin.jsto always navigate tosignin_token_codefor EMAIL_OTP; cleans router and react-app server route lists.session-verify-send-push-*strings fromlib/l10n/server.ftl.Issue that this pull request solves
Closes: https://mozilla-hub.atlassian.net/browse/FXA-13650
Checklist
Other information
This feature is over 2 years old but required client side changes to be useful. Lets remove it since no plans on prioritizing get clients updated.