ERA-13805 Resolve authorization from the site, not from a status flag - #1696
Draft
StephenWithPH wants to merge 11 commits into
Draft
ERA-13805 Resolve authorization from the site, not from a status flag#1696StephenWithPH wants to merge 11 commits into
StephenWithPH wants to merge 11 commits into
Conversation
authorizationServers is keyed by the RFC 8414 issuer identifier a site advertises in its RFC 9728 metadata, and each entry names the grant it uses along with the client registration for it. It is also this build's trust policy: an advertised issuer the map does not name cannot be authenticated against, so an override replaces the map wholesale rather than merging -- a non-production build must not go on trusting the production tenant. $self is the reserved key for the site's own authorization server, django-oauth-toolkit serving /oauth2/token as das_web_client. That server is not a legacy side-channel to be branched around; it is an OAuth 2.0 registration like any other, differing only in grant type. Its issuer is the site origin, so it cannot be a literal key and is matched by predicate. Omitting it builds a client that holds no password registration at all. The map sits at the top level rather than under auth0, which now holds only the scalars the Auth0 provider and login paths still read. Those go once those paths read the registration discovery resolves; until then the example override has to supply both and keep them in step. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Fetches the site's protected-resource document and answers with a registration rather than a routing decision: one pass over what the server advertised, in the server's order, taking the first issuer this build holds a registration for. The registry is the trust policy, so there is no Auth0-versus-legacy branch -- the grant is the only thing that differs, and it is what callers dispatch on. Falling past an unrecognized issuer is deliberate. Server emits Auth0 first, so a client that can use Auth0 always does; reaching a later entry means the server named a server we cannot use while also naming one we can, and refusing then would decline a login the server sanctioned. `skipped` keeps that mismatch visible without making it a failure. The probe answers before any token exists, so it runs on a bare axios client rather than inheriting the app's Authorization header, cancel token, or 401-to-login handling, and an abort deadline keeps startup from waiting on a server that never answers. A document is trusted only when its resource is the origin it came from: an ingress that does not route the well-known path serves the SPA shell instead, so a 200 alone proves nothing. Same-origin is matched on the origin boundary, so a host merely beginning with this one cannot pass. Failure is one of three reasons, reported to the console rather than the screen: a site that answered but not usefully is an administrator's problem, one that did not answer may just be a bad moment, and one advertising nothing we hold is the wrong build for that site. Whoever debugs it gets the reason, the URL probed, the status, and both the advertised and registered issuers. It also reads back an issuer stashed across the Auth0 redirect, running it through the same registry, so the callback leg never has to depend on a live probe. Nothing dispatches this yet. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Renders Auth0Provider with the client registration discovery resolved, runs a site whose grant is password with no provider above it at all, and withholds the app until the probe settles. Auth0Provider wants a host while discovery names the server by issuer, so the host is derived from the matched registry key rather than stored beside it. One message covers every failure, because refreshing or finding an administrator is the whole of what the reader can do; which reason it was goes to the console. The string lives in the login namespace, which is preloaded -- this screen is the app's first render, and a namespace fetched on demand would show the fallback about as often as the translation. It also fires the system-status fetch alongside the probe. Neither answer depends on the other and startup waits on both, so leaving that to a component rendered underneath would put two round trips in series on every cold load. Its tests derive state by running the reducers over real actions. Hand-written store literals cannot see a change to a slice shape, and would go on asserting against a shape production no longer has. Nothing renders this yet -- index.js still builds Auth0Provider statically. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The site issues a token whenever the credentials are right. Whether this application may present it is a separate question, enforced per request in the authenticator rather than at the token endpoint, so a client that is not permitted still receives a 200 and a valid token. Adopting it took the user into the app, 401'd every call, failed to renew (no Auth0 provider is mounted on a password-grant site), and returned them to a login form that had just reported success -- with nothing said about why, and no way out, since the resolution is deterministic and every retry repeats it. So postAuth hands the token back instead of adopting it, and adoption waits on one call to /api/v1.0/user/me. A 401 there means the site refused the token and says so, naming the cause rather than the credentials, which were correct. A transport failure adopts anyway: nothing was learned, and refusing on a blip would invent a failure the server never gave. The token is attached per-call and the request opts out of the global 401 recovery, the same arrangement checkAccountLinked uses on the Auth0 path -- the shared header is not installed until adoption, and without skipAuth this check's own 401 would sign the user out mid-check. Costs no extra round trip in practice: Nav already calls user/me on mount right after login. The call moves earlier and its answer is acted on. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The gate replaces the statically-configured Auth0Provider, so the tenant the SDK talks to now comes from what the site advertised rather than from build config. This is the commit where the probe starts running. RootApp was already a startup gate -- it fetched system config, blocked on it, and rendered its own error. Leaving that inside a second gate would have put the two round trips in series on every cold load, since RootApp cannot mount until discovery settles. So the two are one gate now, firing both fetches together and waiting on both. A system config that never arrives still holds the overlay indefinitely, exactly as before: fetchSystemStatus swallows its errors and resolves undefined, so no caller can tell failure from slowness. That wants fixing, but not from here -- App.js consumes the same thunk's resolved value. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
isSystemConfigLoaded inferred readiness from require_idp being non-null, so the field carried two unrelated meanings: how the site authorizes, and whether the status response had arrived at all. Removing it would have broken the startup gate silently -- every site would have looked permanently unloaded. The gate's system-config fixture now comes from the real reducer too, like the discovery one. As a hand-written literal it went stale the moment the predicate changed; deriving it meant this change failed loudly instead. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Login picks its form from the grant, Auth0TokenManager and useAuthRecovery take the audience from the registration discovery resolved, and RequireAccessToken and Nav ask the same question of the same source. require_idp now has no readers outside the duck that stores it, and appConfig is no longer imported by any login path -- which is what lets the scalars go. Fixes the gate installed in the previous commit: it reads state.view.authDiscovery and the reducer was never added to the root reducer, so that slice was undefined and destructuring it would have thrown on first render. Its own tests could not see this, because they build a store that already contains the slice. store.test.js asserts the wiring against the real store instead, and fails with `undefined` when the reducer is removed. The probe's axios client is now built on first use. Importing this duck used to call axios.create() as a side effect, which broke any suite mocking axios without a create -- RequestConfigManager's, once useAuthRecovery pulled the duck in. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Auth0Provider has to be mounted to exchange ?code&state, so building it from discovery made the callback leg depend on a live probe -- and a probe that failed there would spend the code and return the user to the login form having said nothing, after they had already authenticated. The issuer resolved on the way out is stashed and read back on the way in, then run through the same registry the probe uses. A key is stored, never a registration: a tampered value can name an issuer but cannot supply a client ID, and an issuer this build holds no registration for resolves to no_usable_as rather than a login attempt. It is consumed once, and only on a callback leg -- a normal load probes even if a stash is lying around, so a stale resolution cannot outlive the redirect it belonged to. A successful restore skips the probe rather than racing it. Firing one anyway would let a failure overwrite a working resolution mid-callback, which is the failure this exists to prevent. sessionStorage, beside the intended-route stash that already crosses this same redirect, scopes it to one tab and one attempt. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Nothing reads it: the login paths ask discovery which authorization server the site named, and readiness is its own flag now. The status response goes on sending the field, so the duck drops it on ingest rather than storing a second answer to a question already answered elsewhere. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The tenant was named twice: once as audience/clientId/domain read directly by whoever needed it, and once as a registry entry keyed by issuer. Nothing reads the triple now -- the Auth0 provider is built from the registration discovery resolved, and so are the audiences the login and step-up paths send -- so the registry is the only auth config left, and there is one place a tenant is described rather than two that could disagree. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The registry named das_web_client for the site's own authorization server while postAuth embedded the same string in the token request, so the two agreed only by coincidence and the $self entry was dead data -- editing it changed nothing. Only the Auth0 path was reading a resolved clientId, because it is the one the provider is built from. postAuth now takes the client ID from its caller, which reads it off the resolution. That completes the substitution begun in "Read the resolution rather than the status flag", where the grant and audience moved but this did not, and it makes the registry the single place a client is described. It also gives the $self entry teeth: omitting it from a build's registry now withholds the credential the password grant needs, rather than leaving a hard-coded one behind that would have worked anyway. postAuth had no test at all, so the hard-coded value was never asserted either way. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Implements RFC 9728-based authorization discovery so the web client determines the login mechanism (Auth0 redirect vs. password grant) from the site’s advertised OAuth protected-resource metadata, removing reliance on feature_flags.require_idp and building Auth0 configuration from a client-registration registry.
Changes:
- Add
ducks/auth-discovery+AuthDiscoveryGateto probe/.well-known/oauth-protected-resource, resolve a trusted client registration, and gate app startup accordingly. - Refactor login/token flows to use the resolved registration (audience/clientId/grant/issuer), including stashing/restoring the issuer across Auth0 redirects.
- Add password-grant token usability validation against
/api/v1.0/user/me, and migrate config + tests to the new registry shape andsystemConfig.loadedreadiness flag.
Reviewed changes
Copilot reviewed 34 out of 34 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/utils/token-usability.test.js | Adds tests for token usability probing behavior and outcomes. |
| src/utils/token-usability.js | Adds token usability probe to distinguish “issued” vs “usable” password-grant tokens. |
| src/utils/auth.test.js | Updates readiness sentinel tests and adds sessionStorage issuer round-trip coverage. |
| src/utils/auth.js | Adds resolved-issuer sessionStorage helpers and switches system-config readiness to loaded. |
| src/store.test.js | Ensures authDiscovery slice is wired and that auth discovery/system config aren’t persisted. |
| src/RequireAccessToken/index.test.js | Updates tests to use auth-discovery grant state instead of require_idp. |
| src/RequireAccessToken/index.js | Switches redirect-grant checks to selectUsesRedirectGrant. |
| src/reducers/index.js | Wires the new authDiscovery reducer into the root reducer. |
| src/Nav/index.js | Switches logout decision from require_idp to redirect-grant selection. |
| src/Login/index.test.js | Updates Auth0 vs password-grant login tests; adds token adoption/usability checks and issuer stashing coverage. |
| src/Login/index.js | Uses resolved registration for Auth0 audience/clientId and password-grant postAuth; validates issued token before adoption. |
| src/index.js | Replaces static Auth0Provider setup with AuthDiscoveryGate and removes the old system-config startup gate. |
| src/hooks/useAuthRecovery.test.js | Updates tests to use resolved registration and verifies issuer is stashed for step-up redirects. |
| src/hooks/useAuthRecovery.js | Uses resolved registration for step-up redirect and stashes issuer for callback leg restore. |
| src/ducks/system-config/index.test.js | Updates tests for new loaded readiness flag and ignoring require_idp ingest. |
| src/ducks/system-config/index.js | Stops ingesting require_idp and adds explicit loaded readiness flag. |
| src/ducks/auth.test.js | Adds MSW-backed tests verifying postAuth sends the provided client ID and returns the token. |
| src/ducks/auth.js | Makes postAuth return the issued token and accept a resolved client ID (no longer hard-codes das_web_client). |
| src/ducks/auth-discovery/index.test.js | Adds comprehensive tests for RFC 9728 probing, issuer normalization/matching, restore behavior, and selectors. |
| src/ducks/auth-discovery/index.js | Adds RFC 9728 probe + registry-based resolution and redirect-leg restore. |
| src/config.test.js | Updates config tests to validate the new authorizationServers registry and replacement semantics. |
| src/config.js | Replaces flat Auth0 config with authorizationServers client registry. |
| src/AuthDiscoveryGate/index.test.js | Adds tests for startup gating, callback-leg restore vs probe, provider construction, and failure surface. |
| src/AuthDiscoveryGate/index.js | Introduces discovery + system-status startup gate and dynamic Auth0Provider mounting based on resolved grant. |
| src/Auth0TokenManager/index.test.js | Updates token-manager tests to use resolved discovery audience and grant selection. |
| src/Auth0TokenManager/index.js | Switches Auth0 audience and “IDP mode” gating from require_idp to resolved grant. |
| src/Auth0TokenManager/accountLinkingGate.integration.test.js | Updates integration harness to provide auth-discovery state in the Redux tree. |
| public/locales/sw/login.json | Adds new login failure strings for discovery failure + token-not-accepted cases. |
| public/locales/pt/login.json | Adds new login failure strings for discovery failure + token-not-accepted cases. |
| public/locales/ne-NP/login.json | Adds new login failure strings for discovery failure + token-not-accepted cases. |
| public/locales/fr/login.json | Adds new login failure strings for discovery failure + token-not-accepted cases. |
| public/locales/es/login.json | Adds new login failure strings for discovery failure + token-not-accepted cases. |
| public/locales/en-US/login.json | Adds new login failure strings for discovery failure + token-not-accepted cases. |
| public/config.js.example | Updates local-dev config example to use the authorization server registry. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+33
to
+46
| useEffect(() => { | ||
| // Returning from the Auth0 redirect, prefer the resolution stashed on the way out. The SDK | ||
| // needs its provider mounted to exchange ?code&state, and a probe that failed here would | ||
| // spend the code for nothing. Only when there is no stash does this leg probe. | ||
| const resolveDiscovery = async () => { | ||
| const restored = hasAuth0CallbackParams(window.location.search) | ||
| && await dispatch(restoreAuthDiscovery()); | ||
|
|
||
| if (!restored) dispatch(fetchAuthDiscovery()); | ||
| }; | ||
|
|
||
| resolveDiscovery(); | ||
| dispatch(fetchSystemStatus()); | ||
| }, [dispatch]); |
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.
What this does
EarthRanger Web asks the site how to authorize and acts on that answer alone. One unauthenticated request per site —
GET {siteOrigin}/.well-known/oauth-protected-resource(RFC 9728) — resolves an authorization server to a client registration held in this build. No flow taxonomy, no Auth0-versus-legacy branch, no special case for DAS: both authorization servers are OAuth 2.0 and differ only in grant type.feature_flags.require_idpis gone from the login path and from the store.das_web_clientis no longer hard-coded at the call site — it is a registry entry like any other.Stacked on ERA-13429 (#1689). Base is that branch, so the diff here is the discovery work alone. It retargets to
developwhen #1689 merges.Changes
ducks/auth-discovery(new)Authorizationheader, the master cancel token, nor the 401-to-login interceptor.AuthDiscoveryGate(new)Auth0Providerinsrc/index.js. Builds the provider from the resolution, renders children bare on apasswordsite, or renders the failure surface. Fires the probe andfetchSystemStatustogether so installing the gate does not put them in series.config.jsauthorizationServers, keyed by RFC 8414 issuer identifier, replaces the flat{ audience, clientId, domain }triple. An override replaces the map rather than merging into it.utils/token-usability.js(new)checkTokenUsable— a password-grant token is validated against/api/v1.0/user/mebefore the app is entered.ducks/auth.jspostAuthreturns the token instead of adopting it, and takes the resolved client ID.RequireAccessToken,Auth0TokenManager,Nav,Login,utils/auth.jsrequire_idp.utils/auth.jsisSystemConfigLoadedflag — it previously keyed onrequire_idp !== null, which was doing double duty as "config has loaded".ducks/system-configrequire_idp.The registry is the trust policy
An advertised issuer this build does not name cannot be authenticated against. That is why an override replaces the map wholesale: a non-production build must not go on trusting the production tenant.
$selfis the reserved key for the site's own authorization server — django-oauth-toolkit serving/oauth2/token. Its issuer is the site origin, which differs per site and so cannot be a literal, so it is matched by predicate on the origin boundary. Dropping$selffrom an override is a supported way to build a client that refuses the password grant outright.Issuer comparison parses rather than does string surgery:
origin + pathname, trailing slashes stripped, default port and scheme/host case folded, path case significant (RFC 3986 makes only scheme and host case-insensitive), query or fragment refused (RFC 8414 §2). Anything unparseable has no identity rather than a string that might collide with one. The issuer that travels with a resolution is the registered key, not the advertised string that matched it — so what reaches the Auth0 SDK as its domain is a value this build holds.A token being issued is not a token being usable
bypass_auth0is enforced in the authenticator, not at the token endpoint. On a site whosedas_web_clientrow isbypass_auth0 = False,/oauth2/token/still returns 200 with a valid token and the refusal only appears on the first API call.Left alone that is a bounce loop with nothing reported: the form succeeds, the app is entered, every call 401s, renewal fails (no
Auth0Provideris mounted on apasswordsite, sogetAccessTokenSilentlyis auth0-react's throwing stub), and the user lands back on the login form. It never self-heals, because the resolution is deterministic and every retry repeats it.So issuance is separated from adoption. A 401 on validation blocks adoption and says so — specifically, not "invalid credentials", because the credentials were right. A network error or 5xx adopts anyway: nothing was learned, and refusing on a blip would invent a new failure mode. This costs no extra round trip in practice, since
Navalready calls/user/meon mount immediately after login.Failure surface
One message for every reason, naming the site. Refreshing or finding an administrator is the whole of what a reader can do, so a taxonomy on screen would ask them to act on a distinction that does not change their action. The reason and its detail — probed URL, advertised versus registered issuers,
resource, status — go toconsole.warn, where whoever debugs it still has them.The string is
signInUnavailableinlogin.jsonrather thanerrors.json, becauseloginis preloaded anderrorsis not; this screen is the app's first render, and an on-demand namespace would show the key about as often as the translation.What reviewers should know
The well-known endpoint is load-bearing for every login. There is no
/api/v1.0/statusfallback and no silent degradation — deliberately, so there is one source of truth. Every failure reason is therefore a login outage for that site until the cause is fixed. The endpoint must be routed to the backend in every environment.A site advertising
[unregistered-foreign, DAS-as-AS]falls through to the password grant rather than refusing, with the foreign issuer recorded inskipped. This inverts an earlier "must not fall through to legacy" rule, deliberately: Server emits Auth0 first, so a client that can use Auth0 always does, and reaching a later entry means the server named an authorization server this build cannot use while also naming one it can. Declining then would refuse a login the server just sanctioned.The callback leg does not probe.
Auth0Providermust be mounted to exchange?code&state, so a probe that failed there would spend the code and bounce the user with nothing said. The issuer resolved before the redirect is stashed insessionStorageand re-run through the same registry — the key only, never the registration, so the trust policy gates the restored path exactly as it gates a probe result.src/store.test.jsis new and load-bearing. The gate readsstate.view.authDiscovery; an unwired reducer would have made it throw on first render. The test was verified to fail without the wiring.Testing
Full suite green: 3112 tests / 318 suites. Written test-first, one behaviour per commit; each commit passes on its own, verified in isolation.
Commits
Links