The /api/fetch route classifies an event-stream or NDJSON response as a
stream and then answers with JSON and no body — so nobody downstream
owns the upstream stream and nothing will ever read it, while the
`finally` clears the only deadline on the way out. It is now cancelled
explicitly, best-effort, the way `releaseBody` in oidcDiscoveryCompat
already does. Measured, undici reclaims an unread body here on its own,
so this is belt and braces rather than a demonstrated leak — kept
because relying on that is an implementation detail of the fetch
beneath us, not a contract. The route test and the source comment both
say so rather than implying a stronger claim.
Forwarding the signal also made revocation.ts's rationale stale: it
still described a proxy that drops `init.signal` and a backend that
cannot cancel. Both comments there, and the matching one in
revocation.test.ts, now describe the post-#2319 path — the signal
cancels, the race guarantees, and what the race is still for.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YFnM6RqB4nXqNysgUYmjmq
Signed-off-by: cliffhall <cliff@futurescale.com>
The shared teardown deadline was decided on `remainingMs > 0` against
`Date.now()`. Both halves of that are noise-sensitive: the deadline is
enforced by a `setTimeout`, and a wall clock at millisecond resolution
can still read a hair short of it when the loop comes round, so the next
grant inherits a fractional budget and spends it on a request that cannot
possibly complete. The same run then issues one request or two depending
on scheduling — which is what makes the "shares one deadline across
grants" test intermittent on CI (#2252).
Measure the deadline with `performance.now()` (monotonic, so an NTP step
or a suspend cannot expire or extend the budget, and sub-millisecond, so
none is lost to rounding), and skip any grant reaching the loop with less
than MIN_REVOCATION_REQUEST_BUDGET_MS left. The skipped grant is already
reported as `failed`, so nothing is silently dropped — the needless call
to the authorization server is.
The CLI's outer per-plan budget in `sendPlans` shares both the shape and
the defect, so it gets the same treatment.
One consequence worth naming: a monotonic clock hands `revokeToken` a
fractional budget, and Node's `AbortSignal.timeout` throws
`ERR_OUT_OF_RANGE` on a non-integer delay — before the fetch, so the
request would never be sent and the revocation would report that as its
failure. `revokeToken` now rounds its own budget to whole milliseconds,
which also covers any other caller passing a fractional one.
Tests: the epsilon skip is pinned by a grant that lands inside the floor
but above zero (removing the floor fails it, verified); the fractional
budget by a `revokeToken` call at 19.996ms.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017wEXtbs8UEHUMxDEAxbs99
Signed-off-by: cliffhall <cliff@futurescale.com>
#2222 reported that `core/auth/revocation.ts` breaks a client secret
containing `+`, on the basis that `encodeURIComponent` leaves `+` bare.
It does not — it escapes it as `%2B`. The characters it leaves bare are
exactly `!'()*-._~` and alphanumerics, every one of which a form-
urldecoder passes through unchanged, so its output decodes identically
under both algorithms; verified over every code point up to U+2FFF.
Switching to `URLSearchParams` would be the literal algorithm §2.3.1
names and a small regression: it encodes a space as `+`, which a lenient
server decoding with `decodeURIComponent` alone reads as a literal `+`,
while `%20` is understood by both. The encoder is therefore unchanged
and the reasoning is recorded on it.
The report's second half stands and is the real defect: the fixture
decoded with `decodeURIComponent`, the encoder's own inverse, so the
round trip succeeded for every input by construction and no test could
have failed on an encoding mistake. `/oauth/revoke` now runs a genuine
RFC 6749 Appendix B decode, with unit cases for `+`, a space, `:` and
`%` (asserted on the wire and through an independent decoder), an
end-to-end client whose secret is base64 with a `+` in it, and a guard
that an unencoded credential is refused — the case that separates the
two decoders.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BhVaufrEZAfFAXcg6456hK
Signed-off-by: cliffhall <cliff@futurescale.com>
Copilot reported "no new comments" again, with four real findings in its
suppressed block.
- The web in-flight lock was global, so a pending clear for server A silently
swallowed a click on server B's — even though the callback explicitly
supports clearing a non-active server. Keyed by server id now.
- `else if (!isActive)` skipped the resume-snapshot cleanup when the target IS
active but has no client yet, which is reachable while a session is being
built or torn down. The clear then emptied storage and left stale OAuth
recovery state pointing at credentials that no longer existed.
- `revocationAuthMethods` returned `[]` for an omitted
`revocation_endpoint_auth_methods_supported`, which does not enforce RFC
8414's default: an empty list means "the metadata said nothing" to
`selectClientAuthMethod`, which then honors the registration's own
`token_endpoint_auth_method`. The default is named literally now.
- The `v8 ignore` on the CLI budget branch was wrong, and the challenge to it
was right: an injected `budgetMs` of zero reaches it directly, through a seam
I had added myself the round before. Removed, and covered by a test that
asserts no request is made and the unattempted key is reported.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0177mPHAdECD18nLLCTwR5rh
Signed-off-by: cliffhall <cliff@futurescale.com>
Copilot review round 19.
- Parsing the preconfigured client registration sat outside the per-slot
salvage, so one malformed fallback aborted the whole plan and grants carrying
their own valid credentials were never revoked. It is a *fallback*; a bad one
is now recorded and skipped, matching every other slot.
- `sendPlans` preserved an earlier success when a later key was never
attempted, contradicting the failure-first rule two lines below — the
unattempted key's grant may still be live. That branch turns out to be a
bound rather than a path (`withDeadline` fails any plan that reaches the
deadline, so a preceding plan cannot both succeed and leave zero budget), so
it carries a justified `v8 ignore` saying exactly that.
- `clearStoredAuthForRelogin` takes an optional `budgetMs`, which is what lets
the shared-budget behavior be tested without a five-second test.
- Three stale references to `planOAuthRevocation`, which no longer exists —
two JSDoc links and the `AGENTS.md` entry, which additionally still told
callers to make a separate `storage.clear()` call. Following that text would
have reintroduced the second clear the atomic step exists to remove.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0177mPHAdECD18nLLCTwR5rh
Signed-off-by: cliffhall <cliff@futurescale.com>
Copilot review round 18. Three of its findings were the same check-then-act:
`planOAuthRevocation` is asynchronous, so the gap between its reads and the
following `storage.clear()` is still a window — shorter than the network one
closed in round 13, but the same shape. An OAuth completion landing at any of
those awaits saved a grant the clear then destroyed.
`OAuthStorage.takeRevocationSnapshot(serverUrl)` reads what revocation needs
and clears the server in ONE synchronous pass over the in-memory state, with
the persist after the mutation. `clearAndPlanRevocation` replaces
`planOAuthRevocation`, and there is no separate `clear` call left in any
caller — the clear cannot be forgotten or reordered.
The snapshot returns values unparsed, deliberately: schema validation is pure
and belongs after the mutation, since running it inside would reintroduce the
`await` this exists to remove.
Two consequences worth naming. A failure of the take-and-clear now REJECTS
rather than returning an outcome, because the state was not cleared and the
caller's contract is that it was; both clients already have a rejection path.
And cross-process atomicity is explicitly not claimed — nothing in this store
takes a lock, every mutation is a read-modify-write over a loaded snapshot, and
`clear` alone always had that property.
Also from round 18:
- The CLI's budget check ran before considering `plan.outcome`, so a key that
needed no network was reported as budget-exhausted — warning that a grant may
be live when that key held none.
- A disconnect failure after a successful clear was reported as "could not
clear OAuth state" in both clients. It now goes to the disconnect channel,
and the success is still reported.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0177mPHAdECD18nLLCTwR5rh
Signed-off-by: cliffhall <cliff@futurescale.com>
Copilot review round 17.
`resolveClientInformation` mirrored `BaseOAuthClientProvider`, preferring the
preregistered entry. That order answers "who should I authenticate as now";
revocation asks "who minted this token", and the two diverge — the store lets a
preregistered client and an issuer-bound dynamic registration coexist, so after
a server is switched from DCR to a configured `oauth.clientId` an older DCR
grant was revoked with the configured client's credentials. RFC 7009 §2.2
answers 200 for a token the server does not recognise as the caller's, so that
reported `revoked` while the grant stayed live and the local record was already
gone — the worst available outcome.
The registration bound to the grant's issuer now wins, with the preregistered
entry as the fallback, which covers both directions: a token minted with the
configured client leaves the issuer slot empty, since the SDK writes it only
after DCR.
Stated in the comment rather than implied: this is best available evidence, not
proof. The store records client information per issuer, not per token, so
binding identity at save time remains the real fix and is a storage-shape
change beyond this PR.
Also fixed a CLI test comment still describing the pre-round-13 ordering.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0177mPHAdECD18nLLCTwR5rh
Signed-off-by: cliffhall <cliff@futurescale.com>
Copilot review round 15.
The issuer check still failed open for the legacy unkeyed grant. "Unkeyed"
means its authorization server is UNKNOWN, not that the cached endpoint is
proven to own it: server metadata is a single slot a later discovery
overwrites, while a pre-SEP-2352 token survives until the first issuer-stamped
save. So a clear could post an old bearer credential to an authorization server
that never minted it.
Such a grant is now cleared without being revoked, and reported as such with a
message saying re-authorizing makes future clears revocable. The cost is that
a pre-issuer-binding entry gets no revocation — which is exactly the behavior
it had before this feature existed, so nothing regresses — and it earns
revocation back on the next authorization. Not sending is the only answer that
cannot be wrong.
Every seed in the suite is now issuer-bound, which is what a real flow writes;
the two tests that were about the legacy grant now assert the refusal, since
that is the behavior worth pinning.
Also renamed the CLI test that still described the pre-round-13 ordering.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0177mPHAdECD18nLLCTwR5rh
Signed-off-by: cliffhall <cliff@futurescale.com>
Copilot review round 14. Two code fixes and a documentation sweep.
- `buildRevocationRequest` ran outside `revokeToken`'s best-effort try.
`encodeURIComponent` throws on a lone UTF-16 surrogate — valid JSON, so it
can reach there from a persisted client id or secret — and every caller has
already cleared its local state by then, so a rejection instead of a `failed`
outcome would break the documented guarantee. Construction is inside the try
now, and the catch reports `params.endpoint` rather than a `url` that may not
exist on that path.
- The fixture's Basic decode turned a malformed percent escape into an Express
500, reporting a bad credential as a server fault rather than as the
`invalid_client` 401 the endpoint means. Decoding failure is failed
authentication.
- Ten places still said revocation happens *before* the clear, which the
reorder in the previous commit made false: the settings-form description, the
`--no-revoke` help text, both READMEs, `docs/mcp-server-configuration.md`,
three JSDoc blocks, and the PR body. Swept, and the ordering is stated where
it is load-bearing rather than asserted in passing. Also merged the two
adjacent JSDoc blocks on `InspectorClient.clearOAuthTokens`.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0177mPHAdECD18nLLCTwR5rh
Signed-off-by: cliffhall <cliff@futurescale.com>
Copilot review round 13. Three of its four findings were one root cause, and
it is a design flaw in the premise this PR started from.
Revoking *before* the clear meant the clear waited up to five seconds on the
network — and anything that wrote OAuth state in that window was then deleted
by a clear reasoning about the grant it replaced. A same-server reauthorization
in the TUI (whose Disconnect/Connect accelerators stay live), a stored-only web
clear for a server that becomes active mid-flight, and another CLI/TUI process
writing to the shared file-backed store all hit it.
`revokeStoredOAuthTokens` is therefore split into `planOAuthRevocation` and
`executeOAuthRevocation`, and every caller runs plan → `storage.clear()` →
execute. The snapshot still precedes the clear, because the store is where the
token, the credentials and the endpoint live; but the clear no longer waits on
anything. There is deliberately no single-call wrapper left, so no caller can
reintroduce the racy order by accident.
Fourth finding, same file: `handleConnect` now retires `clearOAuthAttemptRef`
alongside `disconnectAttemptRef`. The server-name check does not cover
disconnect/reconnect to the SAME server — the name still matches on the other
side of it — so a clear still in flight could tear down the session the connect
had just established.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0177mPHAdECD18nLLCTwR5rh
Signed-off-by: cliffhall <cliff@futurescale.com>
Copilot review rounds 8 and 9.
- A cached metadata document with no `issuer` was letting every issuer-bound
token go to its single revocation endpoint. Absence establishes nothing, so
that is now a mismatch: an issuer-bound grant must match, and only a legacy
unkeyed grant — which nothing binds to a different authorization server —
proceeds without the comparison. This was a credential-disclosure risk, not
just a correctness one.
- The ctx-less grant read was skipped whenever it carried an issuer stamp, on
the assumption that slot had already been enumerated. If `listIssuers` fails
there is no enumerated slot behind the stamp, so the one readable grant went
unrevoked while `clear` deleted it. Both reads now share one issuer+token
key, and the stamped issuer is retained rather than discarded.
- `AuthTab`'s repeat guard read `clearState`, which is not updated until React
re-renders, so two `s` events in one input turn both started a clear against
the same store entry. It is a ref now, set before the call.
- A rejected clear left `ownClearRef` set — `handleClearOAuth` never reached
its `oauthRevision` bump — so the next unrelated revision was swallowed and
the failure banner stranded. Cleared on that path.
- The web `clearServerOAuthAndDisconnect` had the staleness the TUI path was
already fixed for: `isActive`/`inspectorClient` are captured before a
five-second await, so a switch during it disconnected the session the user
had just moved to and ran the session-wide cleanup against it. Revalidated
against `sessionRef` after each await.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0177mPHAdECD18nLLCTwR5rh
Signed-off-by: cliffhall <cliff@futurescale.com>
Copilot review round 7.
- The five-second timeout was per request while grants are revoked
sequentially, so a server with N issuer slots could block the disconnect for
N x 5s — the "short timeout" bounded a request rather than anything the user
feels. It is now one budget for the whole call: each grant gets the remaining
time, and a grant reached after it is exhausted is reported as never
attempted rather than starting a fresh five seconds. `--relogin` shares one
budget across its two key spellings for the same reason.
- `AuthTab` called the same settle handler on rejection, so a failed local
clear or disconnect announced "OAuth state cleared". A rejection is not a
revocation failure — those come back as outcomes — so it now has its own
path and reports the real error. The completion is also scoped: a clear
carries the server it started on and an attempt number, and a switch retires
it, so server A's confirmation cannot appear under B.
- `clearOAuthAttemptRef` is now retired by the server-switch effect alongside
`disconnectAttemptRef`; without that an A -> B -> A round trip left both the
token and the captured name matching again. `handleClearOAuth` also
revalidates after `await disconnectInspector()`, since a switch during that
second await would have put the revision bump on the new selection.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0177mPHAdECD18nLLCTwR5rh
Signed-off-by: cliffhall <cliff@futurescale.com>
Copilot review round 6.
- The Basic credential is now form-urlencoded per RFC 6749 §2.3.1. Round 1
matched the SDK's raw `id:secret` on the theory that a credential should be
presented the same way here as at the token endpoint; that does not hold — a
client authenticated with `client_secret_post` never exercised the SDK's
Basic path, so there is no precedent to match, and the raw form is ambiguous
for an id containing `:` and makes `btoa` throw on a non-Latin-1 secret. The
fixture decodes each half.
- The ctx-less grant read is suppressed on the store's issuer *stamp* rather
than on the token's value. The active slot can hold client information
without a token, in which case the read falls back to the legacy grant — and
a value collision with any other issuer would have dropped that grant
unrevoked and unreported.
- The TUI announced "OAuth state cleared" on the keypress, so with a bounded
revocation request it said so while the work was still in flight and then
removed it when the work finished. `onClearOAuth` returns a promise;
`AuthTab` shows "Clearing OAuth state…" until it settles, ignores repeats
while one is in flight, and skips exactly the revision bump its own clear
caused so the confirmation is not immediately reset by it.
- `handleClearOAuth` now claims an attempt and retires a superseded one, the
way `handleDisconnect` does: the await can keep it suspended for seconds,
long enough for the user to switch servers and have server A's outcome —
including its disconnect — land on server B.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0177mPHAdECD18nLLCTwR5rh
Signed-off-by: cliffhall <cliff@futurescale.com>
Copilot review round 5.
- Deduplicating grants by token string alone could conflate two grants: a token
is only meaningful to the authorization server that minted it, so two issuers
minting the same opaque value are two grants. The second was dropped before
the issuer-mismatch check could report it, and `clear` then deleted it. The
key is now issuer+token; the ctx-less read stays suppressed only when its
token already came from a slot, which is exactly the active-issuer duplicate
and never the legacy one.
- A read failure in one issuer slot aborted `collectGrants`, so one corrupt
slot left every other revocable grant untouched while the clear deleted them
all. Each slot is now read independently and its failure carried as an
outcome alongside the grants that could still be revoked; the no-metadata and
no-endpoint returns carry them too, so a failure is never swallowed by a skip.
- The CLI no longer deduplicates its two key spellings. That could only be done
by pre-reading a single token, and `revokeStoredOAuthTokens` now enumerates
every issuer slot under a key — so a shared active token skipped the second
key entirely, taking any other issuer-bound grant under it with the local
delete. A duplicate RFC 7009 request is harmless (§2.2 makes an unknown token
a success); a missed one is the leak.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0177mPHAdECD18nLLCTwR5rh
Signed-off-by: cliffhall <cliff@futurescale.com>
Copilot review round 4.
- `getTokens(url, issuer)` falls back to the legacy unkeyed slot when that
issuer holds none — right for a connect, wrong for `collectGrants`, which
labelled the fallback with the issuer it happened to be iterating and paired
it with that issuer's credentials. During a partially migrated flow the clear
could have sent an old, unbound token to a newly discovered authorization
server. `OAuthStorage.getIssuerTokens` is the exact, no-fallback read; only
the ctx-less read still falls back, which is the one place it belongs.
- The TUI's warning tone was sticky. It lived in its own state while ~30 call
sites set ordinary OAuth notes without resetting it, so one revocation
failure left every later "Authorization updated" yellow, across server
switches. It is now DERIVED — `oauthMessageToneFor(message, warningText)` —
so it cannot outlive the message that earned it, with nothing to remember to
clear. Both setters stay plain `useState` setters: a `useCallback` wrapper
would be a value `react-hooks/exhaustive-deps` demands in seven dependency
arrays for an identity that never changes.
- Hook-level tests for the web wiring (previously missed): the per-server
opt-out and the backend-proxied fetch are now asserted to reach
`clearServerOAuthState`, so handing it the page-origin fetch — which a real
authorization server's missing CORS headers would reject — fails a test.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0177mPHAdECD18nLLCTwR5rh
Signed-off-by: cliffhall <cliff@futurescale.com>
Copilot review round 3.
`clear(serverUrl)` deletes EVERY `byIssuer` slot, but revocation read only the
context-free (active-issuer) token. A server that had authorized against
issuers A and B therefore revoked B, then destroyed the local record of A while
A's grant stayed live at its authorization server — the same leak this feature
exists to close, one level down.
- `OAuthStorage.listIssuers(serverUrl)` is the new seam (one implementation:
every backend extends `OAuthStorageBase`). `collectGrants` walks it plus the
ctx-less read, deduping by token, so every grant the clear will delete is
covered.
- Metadata is cached once per server, not per issuer, so a grant bound to an
issuer the cached document does not describe cannot be revoked: sending its
token to that endpoint would hand a credential to a server that never minted
it. That grant is reported as failed — dropped unrevoked — rather than
silently discarded or misdirected.
- `aggregateOutcomes` gives a failure precedence over another grant's success,
so one live grant cannot hide behind another's 200. Exported for its own
test, since `computeOutcome` returns early on the empty case.
Also from that round:
- The fixture's /oauth/revoke now verifies token ownership (RFC 7009 §2.1).
Access tokens record their owning `client_id`; a token belonging to another
client is left alone and still answered 200 (§2.2 — the response must not
tell one client whether another's token exists). New e2e case proves a
second registered client cannot revoke the first's grant.
- The TUI rendered the revocation-failure note in cyan, the informational
tone. It is a partial success — the local clear really happened — so it is
not an `error` status, but the grant may still be live and cyan understated
that. `AuthTab` takes an `oauthMessageTone`; the message says so explicitly.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0177mPHAdECD18nLLCTwR5rh
Signed-off-by: cliffhall <cliff@futurescale.com>
- RFC 8414 §2 defaults an omitted `revocation_endpoint_auth_methods_supported`
to `client_secret_basic`; it does not inherit the token endpoint's list.
Inheriting made metadata advertising only `client_secret_post` there send
POST credentials to an endpoint that never advertised the method.
- The 5s timeout was inert on the web paths: `createRemoteFetch` re-issues the
call as a POST to `/api/fetch` and drops `init.signal`, and the backend's
outbound fetch gets none either, so a wedged AS could hold the teardown open
indefinitely. A wall-clock race now enforces the deadline; the signal is kept
because it genuinely cancels the direct-fetch paths.
- The settings-modal Clear read `settingsModalTarget`, which comes from the
persisted `servers` list, while edits live in `settingsDraft` until the save
debounce. Toggling the checkbox and clearing immediately used the previous
value. It now merges the draft.
- `--relogin` clears both key spellings, so it now revokes from both rather
than stopping at the first populated one — a stale entry under the other
spelling is a live grant. The normalised key goes first, matching
`findStoredServerState`, and a second key holding the same token is skipped.
- The fixture's /oauth/revoke now ENFORCES RFC 7009 §2.1 client
authentication (both RFC 6749 §2.3.1 forms), so the e2e's claim that the
Inspector authenticates correctly is actually tested; two negative cases
assert a wrong secret and an unidentified client are refused.
- Dropped the `as unknown as` casts from the tests: a typed `InspectorLogger`
double, and a spy on the real storage instead of a spread-and-cast.
- Added CLI-level tests through `runCli` for `--no-revoke`, the per-server
`oauth.revokeOnClear`, and the stderr warning, and TUI tests for the
forwarded opt-out and the rendered failure.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0177mPHAdECD18nLLCTwR5rh
Signed-off-by: cliffhall <cliff@futurescale.com>
Clearing the Inspector's OAuth state deleted the local copy of the tokens
and stopped there. The access token — and the refresh token, which is
long-lived by design — stayed valid at the authorization server until they
expired on their own, so a day of connect/disconnect iteration left it
holding grants for sessions that ended hours ago. RFC 7009 §1 describes
exactly that case, and nothing in the repo constructed a revocation
request, so a server author implementing `revocation_endpoint` had no way
to exercise it with the tool that exists to exercise this.
core/auth/revocation.ts builds and sends the request; the three clear paths
call it, so the behavior is written once — web's "Clear OAuth state and
disconnect" (both the active and stored-only paths), the TUI's Auth tab,
and the CLI's --relogin. It runs BEFORE the local clear, because the token,
the client credentials and the discovered `revocation_endpoint` all live in
the store the clear is about to empty. The request names the refresh token
when there is one: §2.1 asks the AS to also invalidate the access tokens
issued under the same grant, so one request covers both halves.
Best-effort by construction. Every path returns a TokenRevocationOutcome
rather than throwing: no advertised endpoint means nothing is sent and that
authorization server behaves exactly as before, and a network error, a
non-2xx, a 5s timeout or an unreadable store is reported and nothing more.
Forgetting the tokens is what the user asked for. `lost_authorization_state`
recovery deliberately skips it — it clears a half-finished flow to retry it,
and an authorization that never completed has no grant to revoke.
Client authentication resolves the preregistered slot first, mirroring
BaseOAuthClientProvider.clientInformation; reading only the dynamic slot
would send no authentication for a server configured with `oauth.clientId`.
The Basic credential is byte-identical to the SDK's applyBasicAuth so this
request and the token request cannot present the same secret differently.
Opt out per server with `oauth.revokeOnClear` (default on, only `false`
written to disk) or per CLI run with --no-revoke. Read at clear time rather
than connect time, so toggling needs no reconnect. Turning it off is a
testing affordance: a client that walks away still holding live tokens is a
case a server author may want to reproduce.
test-servers now advertises `revocation_endpoint` and serves POST
/oauth/revoke, faithful on §2.2 (an unknown token is a success) and §2.1
(revoking a refresh token invalidates the access tokens under the same
grant), with oauth-revocation-http.json / oauth-no-revocation-http.json as
showcase configs.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0177mPHAdECD18nLLCTwR5rh
Signed-off-by: cliffhall <cliff@futurescale.com>
