* fix(auth): offer explicit re-authorize when callback state is lost (#1808) A `/oauth/callback` that arrives with no recorded SEP-2352 discovery state made the SDK throw `AuthorizationServerMismatchError` and the Inspector dead-ended, surfacing the raw SDK text with no path forward. The SDK folds two very different failures into that one error class, so `core/auth/issuerBinding.ts` classifies them structurally: a recoverable "lost authorization state" (the SDK puts prose, not an issuer URL, in the `recordedIssuer` slot) versus a genuine cross-AS `issuer_mismatch`. Only the first gets a recovery affordance. Web: the callback failure now raises a dedicated banner ("Authorization state was lost" / "Authorize again") whose action clears the stale OAuth state for that server and starts a fresh authorization. A genuine mismatch stays a red, non-dismissible security notification with both issuers named and no one-click retry. Node runners (CLI/TUI): the same classification rewrites the callback rejection into the shared actionable copy instead of the SDK text. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016S7h83UKNQ7kvSiFDNksCt * fix(auth): detect the SDK mismatch error via isInstance (#1808) The callback-leg classifier gated on `err.mcpBrand`, but the SDK declares that brand in a `static {}` block, so it lives on the constructor and is never reachable from an instance. The predicate always returned false, so `findIssuerBindingFailure()` never matched and the #1808 recovery banner was inert in production — the callback dead-ended exactly as before. Use the SDK's own `AuthorizationServerMismatchError.isInstance()`, which is cross-copy safe by construction (it consults a `Symbol.for()`-keyed brand set), with a `name` fallback for a serialization boundary. Correct the module doc, which cited cross-bundle safety as the reason to avoid `instanceof` — the very thing the SDK's branded `hasInstance` provides. Every fixture now constructs a real `AuthorizationServerMismatchError` instead of a hand-rolled object with an own `mcpBrand`, which is why a green `npm run ci` hid this. Adds a test pinning the brand's placement so the feature cannot silently go inert again. Review follow-ups: drop the unused `isLostAuthorizationStateError`, remove the duplicated banner title default, resolve the banner title/action label where the banner is raised, keep the clear-failure notification open, and bound remote-supplied issuers in user-facing copy. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016S7h83UKNQ7kvSiFDNksCt --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
167 lines
5.9 KiB
TypeScript
167 lines
5.9 KiB
TypeScript
/**
|
|
* SEP-2352 issuer-binding failure classification.
|
|
*
|
|
* On the authorization-code callback leg the SDK compares the authorization
|
|
* server resolved by discovery against the one recorded in `discoveryState()`
|
|
* at redirect time, and throws `AuthorizationServerMismatchError` when the two
|
|
* cannot be reconciled. That single error class covers two very different
|
|
* situations:
|
|
*
|
|
* 1. **Lost authorization state** — no discovery state was recorded at all
|
|
* (`recordedIssuer === undefined` inside the SDK). This is a *recoverable*
|
|
* bookkeeping failure: the stored authorization state for the server was
|
|
* dropped between the redirect and the callback (a new browser session, a
|
|
* partially cleared store, a callback resumed in another tab). Nothing
|
|
* suspicious happened — the user just has to authorize again.
|
|
* 2. **Genuine issuer mismatch** — a real issuer *was* recorded and the callback
|
|
* resolved a different one. This is a security signal (RFC 7636: the
|
|
* `authorization_code` and `code_verifier` are bound to the AS that minted
|
|
* them; replaying them at another AS's token endpoint is a credential
|
|
* exfiltration vector). It must never be papered over with a friendly
|
|
* "just re-authorize" affordance.
|
|
*
|
|
* The SDK does not expose a discriminator, so we key off the shape of
|
|
* `recordedIssuer`: in the "lost state" case the SDK passes a human-readable
|
|
* sentence in the `recordedIssuer` slot ("discoveryState was not available on
|
|
* the callback leg; …"); a genuine mismatch always carries an absolute
|
|
* `http(s)` issuer URL there. We accept either signal — the sentinel phrase or
|
|
* "not a URL" — so a reworded SDK sentence still classifies correctly.
|
|
*/
|
|
|
|
import { AuthorizationServerMismatchError } from "@modelcontextprotocol/client";
|
|
|
|
/** Phrase the SDK puts in the `recordedIssuer` slot when nothing was recorded. */
|
|
const MISSING_DISCOVERY_STATE_PHRASE = "discoveryState was not available";
|
|
|
|
/** Outcome of classifying a callback-leg issuer-binding failure. */
|
|
export type IssuerBindingFailure =
|
|
| {
|
|
/** No discovery state was recorded — recoverable by authorizing again. */
|
|
kind: "lost_authorization_state";
|
|
/** Issuer resolved by discovery on the callback leg. */
|
|
currentIssuer: string;
|
|
}
|
|
| {
|
|
/** A different authorization server answered the callback — security signal. */
|
|
kind: "issuer_mismatch";
|
|
/** Issuer recorded at redirect time. */
|
|
recordedIssuer: string;
|
|
/** Issuer resolved by discovery on the callback leg. */
|
|
currentIssuer: string;
|
|
};
|
|
|
|
interface AuthorizationServerMismatchShape {
|
|
recordedIssuer: string;
|
|
currentIssuer: string;
|
|
}
|
|
|
|
/**
|
|
* Recognize the SDK's `AuthorizationServerMismatchError`.
|
|
*
|
|
* Uses the SDK's own `isInstance` predicate, which is **cross-copy safe by
|
|
* construction**: the SDK stamps each instance with a brand set keyed by
|
|
* `Symbol.for("mcp.sdk.errorBrands")` and overrides `Symbol.hasInstance` to
|
|
* consult it, so an error thrown by a *different* bundled copy of the SDK still
|
|
* matches. (Note the brand constant itself is declared `static`, so it lives on
|
|
* the class and is never reachable as `err.mcpBrand` — checking that property on
|
|
* an instance always fails.)
|
|
*
|
|
* The `name` comparison is a deliberate fallback for a serialization boundary,
|
|
* where neither the brand set nor the prototype survives but `name` does.
|
|
*/
|
|
function isAuthorizationServerMismatchShape(
|
|
err: unknown,
|
|
): err is AuthorizationServerMismatchShape {
|
|
if (err === null || typeof err !== "object") {
|
|
return false;
|
|
}
|
|
const candidate = err as {
|
|
recordedIssuer?: unknown;
|
|
currentIssuer?: unknown;
|
|
name?: unknown;
|
|
};
|
|
if (
|
|
typeof candidate.recordedIssuer !== "string" ||
|
|
typeof candidate.currentIssuer !== "string"
|
|
) {
|
|
return false;
|
|
}
|
|
return (
|
|
AuthorizationServerMismatchError.isInstance(err) ||
|
|
candidate.name === "AuthorizationServerMismatchError"
|
|
);
|
|
}
|
|
|
|
/** True when `recordedIssuer` is the SDK's "nothing was recorded" placeholder. */
|
|
function isMissingDiscoveryStatePlaceholder(recordedIssuer: string): boolean {
|
|
if (recordedIssuer.includes(MISSING_DISCOVERY_STATE_PHRASE)) {
|
|
return true;
|
|
}
|
|
// A genuine recorded issuer is always an absolute http(s) URL, so anything
|
|
// else in that slot is prose the SDK put there in place of an issuer.
|
|
try {
|
|
const url = new URL(recordedIssuer);
|
|
return url.protocol !== "http:" && url.protocol !== "https:";
|
|
} catch {
|
|
return true;
|
|
}
|
|
}
|
|
|
|
/**
|
|
* Find and classify a SEP-2352 issuer-binding failure anywhere in an error's
|
|
* `cause` / `data.cause` chain (era-negotiation and transport wrappers bury the
|
|
* original rejection, see `findNestedAuthError`).
|
|
*/
|
|
export function findIssuerBindingFailure(
|
|
err: unknown,
|
|
): IssuerBindingFailure | undefined {
|
|
return findIssuerBindingFailureDeep(err, new Set());
|
|
}
|
|
|
|
function findIssuerBindingFailureDeep(
|
|
err: unknown,
|
|
seen: Set<unknown>,
|
|
): IssuerBindingFailure | undefined {
|
|
if (err === null || typeof err !== "object" || seen.has(err)) {
|
|
return undefined;
|
|
}
|
|
seen.add(err);
|
|
|
|
if (isAuthorizationServerMismatchShape(err)) {
|
|
return classifyAuthorizationServerMismatch(err);
|
|
}
|
|
|
|
const nested = findIssuerBindingFailureDeep(
|
|
(err as { cause?: unknown }).cause,
|
|
seen,
|
|
);
|
|
if (nested) {
|
|
return nested;
|
|
}
|
|
|
|
const data = (err as { data?: unknown }).data;
|
|
if (data !== null && typeof data === "object") {
|
|
return findIssuerBindingFailureDeep(
|
|
(data as { cause?: unknown }).cause,
|
|
seen,
|
|
);
|
|
}
|
|
|
|
return undefined;
|
|
}
|
|
|
|
function classifyAuthorizationServerMismatch(
|
|
err: AuthorizationServerMismatchShape,
|
|
): IssuerBindingFailure {
|
|
if (isMissingDiscoveryStatePlaceholder(err.recordedIssuer)) {
|
|
return {
|
|
kind: "lost_authorization_state",
|
|
currentIssuer: err.currentIssuer,
|
|
};
|
|
}
|
|
return {
|
|
kind: "issuer_mismatch",
|
|
recordedIssuer: err.recordedIssuer,
|
|
currentIssuer: err.currentIssuer,
|
|
};
|
|
}
|