fix(auth): the data lane honors set-auth-token — impersonation takes effect and is visible (#4467) - #4511
Merged
Merged
Conversation
…s effect (#4467) The console injects the same localStorage bearer from two lanes: the AUTH lane (`createBearerFetch` inside `createAuthClient`) and the DATA lane (`createAuthenticatedFetch` — the adapter, `provider: 'api'` data sources, and every metadata `type: 'api'` action). better-auth's server-side bearer plugin hands a ROTATED session token back in the `set-auth-token` response header on whichever lane the call arrived over, and only the auth lane read it. `POST /auth/admin/impersonate-user` is exactly such a call — an ordinary metadata action — so the impersonated session token was dropped on the floor while the server's bearer plugin kept overwriting the impersonation cookie with the admin bearer the console kept sending. Impersonation was a complete no-op in the console, not merely an invisible one: support staff believed they were seeing a user's view while acting entirely as themselves. This mirrors the auth lane's three-line capture into the data lane — one contract, one answer, with no impersonation-specific logic in a generic lane. Gated on `isApiCall`, the same condition that decided we authenticated the request at all; untrusted targets remain the `sameOriginOnly` option's job. Riding along, because identity has to follow the rotation: - `AuthContextValue.refreshSession()` re-resolves user/session in place, without raising `isLoading`. - `TokenStorage.subscribeRotation()` notifies when a token already in hand is replaced by a different one. First store, `clear()` and re-storing the same value stay silent — those transitions have an owner that updates identity itself. An ownerless rotation is what starting impersonation produces. - `AuthClientSession.impersonatedBy?` — interface widening (#4495 precedent). `external/api.test.ts` builds a hand-rolled `Response` fake behind a cast; it omitted `headers`, which this lane now reads on every API response. The fake is made honest rather than the lane made lenient — the auth lane has always read `headers` unguarded.
…ails loudly (#4467) While `session.impersonatedBy` is present, `ConsoleShell` renders a banner naming BOTH parties — the impersonated user, whose name every write is recorded under, and the administrator who started it — plus a stop affordance. It derives from the session rather than from client memory of the click, so it survives a full SPA reboot, a new tab and a browser restart, and it cannot disagree with who the server thinks is acting. An ordinary session renders `null` and its chrome is unchanged. It mounts in `ConsoleShell` — the one provider stack every console route passes through — beside the other global surfaces with a single home (`RemediationOverlay`, `NotificationSnackbar`). The page-level bars it resembles visually (`DraftPreviewBar`, `UnpublishedAppBar`) mount inside `ConsoleLayout`, which only wraps `/apps/*`; the card was filed on a console whose `/home` showed no sign of impersonation at all, so a home that could not carry the indicator would have reproduced the bug. The exit calls `POST /auth/admin/stop-impersonating` over the same data lane (so the restored administrator token is adopted) and then awaits a refresh. The server resolves the administrator from the `admin_session` COOKIE, so a deployment that blocks cookies cannot exit this way — the banner says so and stays up instead of appearing to succeed, which would leave the operator doing ordinary work under someone else's identity. Admin-only navigation needs no separate change: it is gated on `user.isPlatformAdmin`, which `AppContent` reads from `useAuth()` — so once identity re-resolves to the impersonated user, the admin surfaces follow. Ten locale packs carry the banner's copy.
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
yinlianghui
marked this pull request as ready for review
August 13, 2026 02:22
Collaborator
Author
|
ACCEPT — step-7 复核 by PM session
Flipping ready + arming auto-merge. objectstack#8243 (server-side defence-in-depth) remains the upstream companion. Generated by Claude Code Generated by Claude Code |
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
2 participants
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.
Closes #4467
The mechanism: two lanes inject one credential, only one honored the rotation
The console injects the same localStorage bearer from two places:
createBearerFetchinsidecreateAuthClient.tsget-session, the auth endpointscreateAuthenticatedFetchprovider: 'api'data sources, every metadatatype: 'api'actionbetter-auth's server-side bearer plugin hands a rotated session token back in the
set-auth-tokenresponse header on whichever lane the call arrived over. Only the authlane read it — it was the repo's single reader. A rotation issued to a data-lane call was
discarded, and the browser kept sending the OLD token.
POST /auth/admin/impersonate-useris exactly such a call: an ordinary metadata action,so it runs over the data lane. The impersonated session token arrived in
set-auth-tokenand was dropped on the floor, while the server's bearer plugin kept overwriting the
impersonation cookie with the admin bearer the console kept sending.
Impersonation was therefore a complete no-op in the console, not merely an invisible
one — and the audit risk is the inverse of the card's original framing: support staff
believed they were seeing a user's view while acting entirely as themselves. This is the
measurement that falsified the card's stated premise (client identity going stale);
identity was never stale, it re-resolved correctly every SPA boot, to the admin, because
the admin is who the session still was.
The fix
createAuthenticatedFetchmirrors the auth lane's three-line capture. Onecontract, one answer, on both lanes — no impersonation-specific logic in a generic
lane, and no knowledge of which endpoint rotated. Gated on
isApiCall, the samecondition that decided we authenticated the request at all; untrusted targets remain
the
sameOriginOnlyoption's job, which short-circuits before any header work.AuthProvidergrowsrefreshSession()— re-resolves user/session in place,deliberately without raising
isLoading(a refresh must not blank the console it runsunder).
TokenStorage.subscribeRotation()notifies when a token already in hand isreplaced by a different one. First store,
clear(), and re-storing the same value staysilent: those transitions have an owner that updates identity itself. An ownerless
rotation is precisely what starting impersonation produces — the action runtime
executes
type: 'api'actions generically and cannot know one endpoint changed who theuser is without hard-coding that endpoint into a generic runtime. The rotation is the
signal, and it is the server's own declaration rather than our guess about a URL.
AuthClientSession.impersonatedBy?— interface widening, per the fix(list): OBJECT_API_DISABLED renders an honest cannot-work state instead of the empty state (#4408) #4495 precedent.ConsoleShell, the one provider stack every console routepasses through, naming BOTH parties plus a stop affordance. It derives from the
session, not from client memory of the click, so it survives a full SPA reboot, a new
tab and a browser restart, and it cannot disagree with who the server thinks is acting.
stop-impersonatingrestores the administrator from theadmin_sessioncookie, so a cookie-blocked deployment cannot exit this way. A stopthat resolves but leaves the session impersonated keeps the banner up and says so —
silently appearing to succeed would be strictly worse than the original defect.
Admin-only navigation needed no separate change: it is gated on
user.isPlatformAdmin,which
AppContentreads fromuseAuth(), so once identity re-resolves to theimpersonated user the admin surfaces follow structurally.
Red-first
The card's original harness — a mocked session carrying
impersonatedBy— would have beena phantom pin: production could not reach that state at all, because the token that
produces it was being thrown away. The primary pin is therefore lane-level. The lane fix
was taken out with
git checkoutof the pre-fix file (nevergit stash), the suites run,and the file restored and verified identical by sha256.
Verbatim, with the fix removed:
expected 'Dev Admin' to be 'RT8 Dave'is the card's own symptom reproduced in a unittest — the console greeting the admin over a session that is someone else.
The 13 green beside those 3 reds are the point: the auth lane's identical capture
stays green throughout, which is the asymmetry stated as a test rather than as prose.
Restored, all 16 pass.
The banner cases ride the real identity flow —
AuthProviderresolving from a clientwhose
getSessionanswers as a function of the token inTokenStorage, exactly as theserver does. Nothing injects context state and nothing mocks
impersonatedByinto asession the console could not otherwise reach, which is what makes the exit case a live
check on the lane fix too.
Must-not-change, green on both sides: an ordinary session renders zero banner and
identical chrome; a data-lane response without the header leaves the stored token alone; a
sameOriginOnlycross-origin response cannot rotate anything; re-storing the same tokenneither notifies nor re-resolves identity.
What the harness cannot drive, stated rather than faked (recorded in-file): jsdom has
no server, so the cookie half of the mechanism is out of reach — the
admin_sessioncookie, the bearer plugin overwriting the request's session cookie, and the signed-cookie
round trip. Those were measured live against a running stack on the card.
Honest costs, recorded
the duration.
exit states its own failure instead of appearing to succeed.
Accepted on the card as the price of the feature actually functioning. Defence-in-depth on
the server side — refusing bearer-shadowed impersonation — is filed upstream as
objectstack#8243.
Consumption-radius note
external/api.test.tsbuilds a hand-rolledResponsefake behind a cast, and omittedheaders, which this lane now reads on every API response. The fake was made honestrather than the lane made lenient — the auth lane has always read
headersunguarded,and a tolerant
?.here would have re-introduced the asymmetry this PR removes. Therepo-root run over 377 files is the sweep that found it; the other eight
Responsefakesin the tree either use a real
Responseor never route through this lane.Verification
type-check(bothtsc --noEmitandtsconfig.test.json) for auth, app-shell, i18n — green.check:i18n-keys,check:i18n-drift,changeset:check,check:control-bytesall green.AuthProvider.tsxcarries the same 3 pre-existingset-state-in-effectwarnings asorigin/main(lines 99/506/513 there, 177/565/572here) — no new findings; the banner and both new test files produce zero.
Changesets
@object-ui/authminor — published transport behavior moves (a data-lane response nowreplaces the stored session token) plus three additive surfaces.
@object-ui/app-shellpatch,@object-ui/i18npatch.Generated by Claude Code