feat(content-server,settings): serve React for oauth/success route by vbudhram · Pull Request #21247 · mozilla/fxa

vbudhram · 2026-09-17T19:35:22Z

Because

A hard load of /oauth/success/:client_id was still served by Backbone.
The React page needs the client id, but that URL has no query parameters.

This pull request

Adds an oauthSuccessRoutes group in server/lib/routes/react-app/index.js, so Express sends the route to React. The group is hardcoded on, because Backbone no longer serves the route.
Removes the oauth/success/:client_id entry from app/scripts/lib/router.js. views/ready.js stays, because six other Backbone routes use it.
Adds getOAuthSuccessClientId in src/lib/oauth/success-route.ts. The integration factory, useClientInfoState, and OAuthWebIntegration read the client id from the pathname. The helper also accepts a trailing slash.
Sets isOAuthSuccessFlow on OAuthWebIntegration. An empty scope now gives an empty permission list instead of an error.
Renames getRoutesExcludingOAuthSuccessReact to getBackboneOAuthSuccessRouteNames in get-oauth-success.js, from review feedback. The old name described an exclusion, not the route names it returns. No behaviour change.

Issue that this pull request solves

Closes: FXA-6642

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: react-app/index.js, get-oauth-success.js, success-route.ts.
Suggested review order: the Express route group, then the Backbone router removal, then the client id lookup in fxa-settings.
Risky or complex parts: the router removal changes which app serves the URL. views/ready.js must stay for the other Backbone routes.

Screenshots (Optional)

None. The page renders the same React screen as before.

Other information (Optional)

The four fxa-settings suites this branch touches pass locally: 101 tests, 0 failures.
npx eslint app server in packages/fxa-content-server: exit 0.
npx nx lint fxa-content-server did not finish. Its lint-glean step builds a Python venv and the sandbox has no network. The eslint half ran directly, as shown above.
An inline node check of getBackboneOAuthSuccessRouteNames and the default export: 5 passed, 0 failed. No test file was added. fxa-content-server has only tests/functional and no unit test target.
The functional tests did not run in the sandbox. CI covers them.
get-frontend.js and get-frontend-pairing.js keep the older getRoutesExcluding* names. This pull request does not touch them.

Copilot

🟡 Changes recommended

Pathname-only hard loads still trigger the client-info error guard before PairSuccess renders.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Migrates /oauth/success/:clientId from Backbone to the React PairSuccess flow.

Changes:

Adds an unconditional React route group and removes the Backbone route.
Handles scope-less OAuth success integrations.
Adds trailing-slash and integration tests.

File summaries

File	Description
`oauth-web-integration.ts`	Bypasses permission parsing for success flows.
`oauth-web-integration.test.ts`	Tests empty permissions.
`integration-factory.ts`	Marks success-flow integrations.
`integration-factory.test.ts`	Tests factory behavior.
`integration-factory-flags.ts`	Extracts client IDs with trailing slashes.
`integration-factory-flags.test.ts`	Tests trailing-slash parsing.
`react-app/index.js`	Adds the React route group.
`get-oauth-success.js`	Disables the server Backbone fallback.
`router.js`	Removes the client Backbone route.

Review details

Files reviewed: 9/9 changed files
Comments generated: 1
Review effort level: Balanced

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

        throw new OAuthError('INVALID_PARAMETER');
      }
      integration.data.clientId = clientId;
+      integration.isOAuthSuccessFlow = true;


LZoog

I think we should probably update that one function name and can we add a Storybook page for this in React? Otherwise just want to spot check locally and can r+

LZoog · 2026-09-18T19:28:13Z


-function getRoutesExcludingOAuthSuccessReact({ oauthRoutes }, routeNames) {
-  return oauthRoutes.featureFlagOn
+function getRoutesExcludingOAuthSuccessReact(


This function is named getRoutesExcludingOAuthSuccessReact but is now getting oauthSuccessRoutes? Should this be updated to just getOAuthRoutes?

LZoog · 2026-09-18T19:40:29Z

  }>({ loading: true });

  const urlQueryData = new UrlQueryData(new ReachRouterWindow());
+  // A hard load of `/oauth/success/:clientId` carries no query params.


(Fine to ignore)
Was going to leave a nit about removing the comment but I guess even though the client ID is always in the URL, it probably had it as a query param at some point? So ehh I guess we can leave this comment (just thinking out loud)

LZoog · 2026-09-18T19:41:52Z

        }
      );
    },
-    'oauth/success/:client_id(/)': createViewHandler(ReadyView, {


(Fine to ignore)

We've been saving these for the removal issues but I see this:

Removes the oauth/success/:client_id(/) entry from app/scripts/lib/router.js. The views/ready.js import stays, because other routes still use it.

So 👍 seems fine

## Because - A hard load of `/oauth/success/:client_id` was still served by Backbone. - The React page needs the client id, but that URL has no query parameters. ## This pull request - Adds an `oauthSuccessRoutes` group in `server/lib/routes/react-app/index.js`, so Express sends the route to React. The group is hardcoded on, because Backbone no longer serves the route. - Removes the `oauth/success/:client_id` entry from `app/scripts/lib/router.js`. `views/ready.js` stays, because six other Backbone routes use it. - Adds `getOAuthSuccessClientId` in `src/lib/oauth/success-route.ts`. The integration factory, `useClientInfoState`, and `OAuthWebIntegration` read the client id from the pathname. The helper also accepts a trailing slash. - Sets `isOAuthSuccessFlow` on `OAuthWebIntegration`. An empty scope now gives an empty permission list instead of an error. - Renames `getRoutesExcludingOAuthSuccessReact` to `getBackboneOAuthSuccessRouteNames` in `get-oauth-success.js`, from review feedback. The old name described an exclusion, not the route names it returns. No behaviour change. ## Issue that this pull request solves Closes: FXA-6642

Copilot

Copilot review overview

🟢 Approval recommended

The routing migration and pathname-based client lookup are consistent and covered by focused tests.

Review effort: Balanced
Findings: 1

Open (1)

A bare hard load of /oauth/success/:clientId still cannot reach PairSuccess.…

vbudhram added the auto label Sep 17, 2026

Copilot AI balanced review requested due to automatic review settings September 17, 2026 19:35

vbudhram added the auto label Sep 17, 2026

vbudhram requested a review from a team September 17, 2026 19:35

vbudhram assigned LZoog Sep 17, 2026

Copilot started reviewing on behalf of vbudhram September 17, 2026 19:35 View session

Copilot AI reviewed Sep 17, 2026

View reviewed changes

Comment thread packages/fxa-settings/src/lib/integrations/integration-factory.ts

throw new OAuthError('INVALID_PARAMETER');

}

integration.data.clientId = clientId;

integration.isOAuthSuccessFlow = true;

vbudhram force-pushed the fxa-6642 branch from a1a0823 to cf6ee2e Compare September 17, 2026 20:30

LZoog reviewed Sep 18, 2026

View reviewed changes

vbudhram force-pushed the fxa-6642 branch from cf6ee2e to a748ba9 Compare September 21, 2026 16:41

vbudhram requested review from a team and a balanced review from Copilot September 21, 2026 16:41

Copilot started reviewing on behalf of vbudhram September 21, 2026 16:42 View session

Copilot AI reviewed Sep 21, 2026

View reviewed changes

Navigation Menu

feat(content-server,settings): serve React for oauth/success route - #21247

feat(content-server,settings): serve React for oauth/success route#21247
vbudhram wants to merge 1 commit into
mainfrom
fxa-6642

vbudhram commented Sep 17, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

LZoog left a comment

Uh oh!

LZoog Sep 18, 2026

Uh oh!

LZoog Sep 18, 2026 •

edited

Loading

Uh oh!

LZoog Sep 18, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

vbudhram commented Sep 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Because

This pull request

Issue that this pull request solves

Checklist

How to review (Optional)

Screenshots (Optional)

Other information (Optional)

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

LZoog left a comment

Choose a reason for hiding this comment

Uh oh!

LZoog Sep 18, 2026

Choose a reason for hiding this comment

Uh oh!

LZoog Sep 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LZoog Sep 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟢 Approval recommended

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

vbudhram commented Sep 17, 2026 •

edited

Loading

LZoog Sep 18, 2026 •

edited

Loading

LZoog Sep 18, 2026 •

edited

Loading