ERA-13478: Post-PKCE account-linking gate consumption + finalizing flag

[StephenWithPH]

What does this PR do?

Implements the web side of the Phase 2 account-linking gate (subtask of ERA-13475). After a successful Auth0 PKCE round-trip on a common-DB site (require_idp && idp_org_id == null), the SPA consults GET /api/v1.0/user/linked before entering its authenticated state and routes the user accordingly:

• 204 → proceed into the app (token cookie + POST_AUTH_SUCCESS + navigate)
• 200 (text/plain URL in body) → full-page navigation to the server-supplied Link Accounts URL (the SPA does not hardcode /auth/link-accounts/)
• 400 → clear the Auth0 SDK session (logout({ openUrl: false })) + SPA token state, return to /login
• network / 5xx / timeout / cancellation → leave the user at /login with a retryable error, token untouched

During the gate's in-flight window the user stays on the loading overlay rather than being bounced to /login: RequireAccessToken keeps showing it because ?code&state are still present in react-router's location (react-router doesn't observe the Auth0 SDK's raw history.replaceState, so the params persist in useLocation() until the SPA itself navigates). Org-scoped (rcuksa) logins skip the gate entirely.

Evidence

• yarn test --watchAll=false — 278 suites, 2461 passed, 1 skipped, 0 failures
• yarn lint clean on all changed files
• New/extended tests: the account-linking util (204 / 200-with-URL-validation / 400 / transient classification), Auth0TokenManager (204 / 200 / 400 / transient / org-scoped), an integration test (real router + store) proving no /login bounce or deep-link clobber across the gate window, and a unit test pinning the react-router-vs-replaceState behavior the guarantee rests on.

non-admin-linking.mov

Relevant link(s)

• ERA-13478 (subtask of ERA-13475)
• Branch Environment

Notes

• Server dependency: the /user/linked endpoint (tracked under parent ERA-13475) must be deployed to the target env for end-to-end behavior. This web change is self-contained and inert on non-common-DB flows until then.
• Gate URL is built as ${API_URL}user/linked (no trailing slash, mirroring CURRENT_USER_API_URL); the server route accepts the no-slash form.
• Changes since review (core-reviewer + @luixlive): an early view.auth0CallbackInProgress Redux flag was found redundant — hasAuth0Params already holds the overlay through the gate window — and was removed; the investigation is captured as the integration + react-router tests above. The gate client also moved from ducks/ to utils/ (it's not Redux), the unreachable missing-URL branch was dropped, the login error now initializes from router state (no effect), and Auth0TokenManager imports/comments were tidied.

[github-actions]

🚀 PR Environment Deployed

App	Sync	Health	Image
pr-web-era-13478	✅ Synced	✅ Healthy	5676d78759fc08fe3938ece6b6f12c0951dc0ecc

Access: https://era-13478.dev.pamdas.org

View in ArgoCD

[Copilot]

Pull request overview

Implements the client-side “Phase 2” account-linking gate for Auth0 PKCE logins on common-DB (require_idp && !idp_org_id) sites, including a short-lived view.auth0CallbackInProgress flag so route-guards show a loading overlay during post-callback finalization instead of bouncing to /login.

Changes:

• Added an account-linking gate client (GET /api/v1.0/user/linked) with result classification and a reducer-backed “callback in progress” flag.
• Updated Auth0 PKCE callback handling to consult the gate before entering authenticated SPA state, including handoff to a server-supplied link-accounts URL and retryable login error surfacing.
• Updated RequireAccessToken and tests to respect the finalization flag and show the loading overlay during the in-flight window; added new i18n string across all locales.

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
src/utils/navigation.js	Adds a wrapper for full-page redirects to support unit testing.
src/RequireAccessToken/index.js	Uses auth0CallbackInProgress to avoid deep-link clobber and show loading overlay during finalization.
src/RequireAccessToken/index.test.js	Adds coverage ensuring the overlay is shown while the gate is in-flight.
src/reducers/index.js	Wires view.auth0CallbackInProgress into the root reducer (non-persisted).
src/Login/index.js	Surfaces a retryable “sign-in incomplete” alert via router state.
src/ducks/account-linking.js	Introduces gate request/classification logic plus auth0CallbackInProgress action+reducer.
src/ducks/account-linking.test.js	Tests gate classification behavior and reducer toggling.
src/Auth0TokenManager/index.js	Orchestrates post-PKCE token acquisition + gate consult + redirect/login outcomes, managing the finalizing flag lifecycle.
src/Auth0TokenManager/index.test.js	Adds tests for 204/200/400/transient/org-scoped behaviors and flag lifecycle.
public/locales/en-US/login.json	Adds errorAlert.signInIncomplete.
public/locales/es/login.json	Adds errorAlert.signInIncomplete.
public/locales/fr/login.json	Adds errorAlert.signInIncomplete.
public/locales/ne-NP/login.json	Adds errorAlert.signInIncomplete.
public/locales/pt/login.json	Adds errorAlert.signInIncomplete.
public/locales/sw/login.json	Adds errorAlert.signInIncomplete.

[Copilot]

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated no new comments.

[luixlive]

A bunch of comments, some of them are nits and very simple. But some require a second pass. Let's reduce the amount of comments. They're a lot 😅 Was even complicated to follow the code flow without getting distracted by them.

[luixlive]

nit: Imports and hooks in this file have grown, and they have became a big block. We can organize them a bit better by separating external from internal dependencies and sorting alphabetically:

import { useEffect, useRef } from 'react';
import { useAuth0 } from '@auth0/auth0-react';
import { useDispatch, useSelector } from 'react-redux';
import { useLocation } from 'react-router';

import appConfig from '../config';
import { checkAccountLinked, GATE_RESULT, setAuth0CallbackInProgress } from '../ducks/account-linking';
import { clearAuth, POST_AUTH_SUCCESS } from '../ducks/auth';
import {
  clearIntendedPostAuth0SuccessRoute,
  getIntendedPostAuth0SuccessRoute,
  isValidTokenFormat,
  stripAuth0Params,
} from '../utils/auth';
import { hasAuth0CallbackParams } from '../utils/auth0';
import { REACT_APP_ROUTE_PREFIX } from '../constants';
import { redirectToExternalUrl } from '../utils/navigation';
import useNavigate from '../hooks/useNavigate';

Same with hooks, separating by responsibilities:

  const dispatch = useDispatch();
  const location = useLocation();
  const navigate = useNavigate();

  const existingToken = useSelector((state) => state.data.token?.access_token);
  const idpOrgId = useSelector((state) => state.view.systemConfig?.idp_org_id);
  const requireIdp = useSelector((state) => !!state.view.systemConfig?.require_idp);

  const { isAuthenticated, getAccessTokenSilently, logout } = useAuth0();

  const hasHandledCallback = useRef(false);
  const sawAuth0Params = useRef(false);

[StephenWithPH]

Done in d0ecd9b3 — imports grouped external-then-internal (alphabetical by path), and the hooks grouped by responsibility per your suggestion.

[luixlive]

Commented code 👀

[StephenWithPH]

Good eye — it reads like dead code, but it's clarifying the LINKED fall-through (none of the gate branches return, so control falls through to the sign-in below). Reworded to prose in d0ecd9b3 so it no longer looks like a commented-out expression.

[luixlive]

Wrong test: UNLINKED is only returned when isHttpUrl passes. We are testing an impossible path using mocks, which can lead to confusion.

[StephenWithPH]

Right. Once the gate validates the URL, UNLINKED only ever carries a usable http(s) URL, so that else branch and its test were unreachable. Removed both in 2b9b38e5 — a bad/absent URL now lands at /login via the TRANSIENT path.

[luixlive]

Hmm this file has some anti-patterns. It's a duck file, which is supposed to hold a piece of Redux state and its handling, but we have this method here checkAccountLinked that is totally unrelated to the store. It doesn't read redux, it doesn't dispatch actions. The response of the axios request is just returned, not even stored. This should either be an independent utility, or properly store the retrieved data in the store, but only if it's truly necessary.

All the reducer does is handle a boolean inProgress, which I'm not sure if it really needs to be handled through Redux. If it does, it probably belongs in src/ducks/auth.js.

[StephenWithPH]

Agreed. The finalizing-flag reducer is gone now (3c426a6c — see the RequireAccessToken thread), so there's no Redux left in the module; moved checkAccountLinked to src/utils/account-linking.js in de268489.

[luixlive]

Right now the dependency array of this useEffect listens to all changes to location.state. We should scope it only to what we need:

const authLinkingError = location.state?.authLinkingError;
useEffect(() => {
  if (authLinkingError) {
    // eslint-disable-next-line react-hooks/set-state-in-effect
    setAlertMessage(t('errorAlert.signInIncomplete'));
  }
}, [authLinkingError, t]);

Now, the eslint warning tells us that probably we don't need a useEffect for this. We could simply initialize the error message from the location state and completely remove this useEffect:

const [alertMessage, setAlertMessage] = useState(() =>
  location.state?.authLinkingError ? t('errorAlert.signInIncomplete') : null
);

This code reads better and is more performant because it doesn't cause rerenders or flickering from the alert message since it's set immediately.

[StephenWithPH]

Done in 610ba9e8 — alertMessage now initializes from location.state via a useState initializer, so the effect (and its react-hooks/set-state-in-effect disable) are gone and there's no extra render / flicker.

[luixlive]

Probably this is a good time to get rid of mapStateToProps. Let's use useSelector instead for all these variables. If you ask Claude it will know what to do 😄

[StephenWithPH]

Removing the finalizing flag reverted this file to its original develop form, so this PR no longer touches it. I'd rather not pull an untouched component into the PR as a pure refactor, so I'm leaving the connect→useSelector conversion out of scope here.

[luixlive]

We should add a simple unit test to avoid breaking the simple functionality of this function 👍

[StephenWithPH]

Added in 0411a49d. (jsdom locks down window.location — it's non-configurable and assign isn't spyable — so the test asserts the href assignment via an observable hash fragment.)

[luixlive]

From a Claude review:
The dispatch(setAuth0CallbackInProgress(true)) runs in a useEffect (after paint), so there is one committed render where auth0Loading, hasAuth0Params, and auth0CallbackInProgress are all false simultaneously — RequireAccessToken renders in that frame, and the setIntendedPostAuth0SuccessRoute effect fires with the cleaned callback URL, overwriting the original deep link in localStorage before the gate resolves.

Seems like a possible bug that will navigate the user to login unexpectedly. Not sure if it's a real bug, but worth asking an agent to investigate and fix if necessary.

[StephenWithPH]

Investigated this closely — it's not a real bounce, and the reason is subtle.

The premise is that hasAuth0Params is false during the gate's in-flight window. It isn't: react-router's useLocation never observes the Auth0 SDK's param strip. The SDK clears ?code&state with a raw window.history.replaceState (no popstate), and react-router only re-syncs on popstate / its own navigate() — so the params persist in useLocation() until we navigate ourselves. That keeps RequireAccessToken on the LoadingOverlay (the || hasAuth0Params branch) for the whole round-trip: no <Navigate to="/login">, no intended-route clobber.

That said, your instinct was half-right — a flag set in an effect genuinely can't cover the first post-auth render, so auth0CallbackInProgress was redundant (hasAuth0Params was already doing the work). Removed it entirely in 3c426a6c (which also addresses the "does this need Redux" thread). Added two tests: a unit test pinning the react-router-vs-replaceState behavior the guarantee rests on, and an integration test (real router + store) proving no /login bounce or deep-link clobber across the gate window — verified falsifiable (it fails if the overlay guard regresses).

[Copilot]

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.

[StephenWithPH]

Not an issue on the installed version: @auth0/auth0-react@2.17.0 types logout as (options?: LogoutOptions) => Promise<void> (dist/auth0-context.d.ts), so .catch() is valid here. (v1's logout returned void — that's likely the mismatch.) Leaving the code as-is.

[StephenWithPH]

Good catch — the description was stale. Updated it. The view.auth0CallbackInProgress flag was removed in 3c426a6c; as you note, the no-bounce behavior relies on hasAuth0Params staying truthy because react-router never observes the SDK's raw history.replaceState.

[StephenWithPH]

Thanks for the thorough review @luixlive (and Copilot) 🙏 — recap of what changed since the review. All pushed; yarn test green (278 suites / 2461 passed).

The substantive one: the auth0CallbackInProgress flag is gone (3c426a6c)

Digging into the possible-bounce concern showed the post-callback overlay is actually held by hasAuth0Params the whole time: react-router never observes the Auth0 SDK's raw history.replaceState, so ?code&state persist in useLocation() until the SPA itself navigates. The flag (set in an effect) was redundant, so it's removed end-to-end. The reasoning is captured as two tests — an integration test (real router + store) proving no /login bounce or deep-link clobber across the gate window, and a unit test pinning the react-router-vs-replaceState behavior the guarantee rests on.

Per comment

• Duck anti-pattern → gate client moved ducks/ → src/utils/account-linking.js (no Redux left in it) — de268489
• Impossible test / dead branch → the gate validates the URL, so UNLINKED always carries a usable one; dropped the unreachable missing-URL branch and its test — 2b9b38e5
• Imports/hooks ordering + "commented code" → reordered (external→internal, alphabetical); the flagged line wasn't dead code (it clarifies the LINKED fall-through) so it's reworded to prose, and comments trimmed — d0ecd9b3
• Login useEffect → alertMessage now initializes from location.state via a useState initializer; effect + set-state-in-effect disable gone, no flicker — 610ba9e8
• redirectToExternalUrl unit test → added — 0411a49d
• Copilot → logout().catch() is fine on @auth0/auth0-react@2.17.0 (logout returns Promise<void>); PR description updated to match the final design.

Deferred

• RequireAccessToken connect → useSelector — removing the flag reverted that file to its develop form, so this PR no longer touches it. Left out of scope rather than pulling an untouched component in as a pure refactor.

Ready for another look 🙏

behavior with these changes:

Screen.Recording.2026-06-17.at.20.07.33.mov

[luixlive]

A couple more comments, but nothing to block since are suppositions by just reading the code. I trust on your own testing of the implementation. But worth taking a look. Approved 👍

[luixlive]

I think we could leave the user in a locked in certain scenarios.

We set hasHandledCallback.current = true;, but then we have some scenarios like TRANSIENT gate result, isValidTokenFormat returning false, getAccessTokenSilently throwing, where we expect a retry, but it would be impossible since hasHandledCallback.current is already true. Am I missing something?

[StephenWithPH]

Not a lock-in — the only retry from those failure paths is the /login "Sign in" button → loginWithRedirect, which is a full-page navigation to Auth0 and back. That reloads the SPA and remounts Auth0TokenManager, so hasHandledCallback (a useRef) reinitializes to false and the callback is processed fresh.

The ref intentionally stays true on the in-SPA failure paths: if we reset it instead, the effect would re-enter immediately on the /login navigation (sawAuth0Params and isAuthenticated are both still true) → re-run the gate → fail → re-navigate → loop. So keeping it true is the correct behavior; the remount is what enables the retry.

[luixlive]

Shouldn't we await for clearAuth to finish before navigating away?

[StephenWithPH]

Left as-is here, on purpose: this branch runs before we enter the authenticated state (the gate is consulted before any POST_AUTH_SUCCESS), so there's no redux/token state to flush — clearAuth is purely defensive and its cookie delete is synchronous, and /login re-clears on mount regardless. handle401Errors awaits because it fires after a genuine authenticated session, which is a different situation. So the logout() / clearAuth() cleanup can run in the background; the redirect doesn't need to block on it.

[luixlive]

Claude detected this method to be a re-implementation of getIsValidWebUrl from src/utils/string.js.

Also, I still see a lot of comments in this file.

[StephenWithPH]

Good catch — isHttpUrl was byte-for-byte the same as getIsValidWebUrl (src/utils/string.js, already used by the SchemaForm Text element), so I dropped it and reused the shared util (and trimmed this file's comments) in 5676d78. Coverage is unchanged: the account-linking tests already exercise the validation (200 + non-http / non-URL → TRANSIENT), and getIsValidWebUrl has its own unit tests.