Commit graph inspector/core/auth/node/runner-interactive-oauth.ts
Author SHA1 Message Date
Cliff Hall
7784553674
fix(auth): explicit re-authorize when the OAuth callback state was lost (#1808) (#1810)
* 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>
2026-07-27 18:08:31 -04:00
cliffhall
f4fb4b50c9 Forward RFC 9207 iss to the v2 SDK (fixes OAuth + EMA leg 1)
SDK v2 added RFC 9207 §2.4 authorization-response issuer validation
(`validateAuthorizationResponseIssuer` / `IssuerMismatchError`) as a mix-up
attack defense. It is strict: when AS metadata advertises
`authorization_response_iss_parameter_supported: true` and no `iss` reaches
`exchangeAuthorization`/`auth()`, the exchange throws.

v1.29.0 had no `iss` parameter and no RFC 9207 validation at all, so dropping
the callback `iss` was harmless. The migration swapped in the stricter SDK
without feeding it the new input, breaking the authorization-code exchange
against any RFC 9207 authorization server (Okta/xaa.dev advertise it):

    Issuer mismatch in authorization response (RFC 9207):
    expected "https://idp.xaa.dev", received undefined

Found by smoke-testing EMA against live xaa.dev staging; not caught by CI
because the mock IdP never advertised the parameter, so the SDK took its
lenient path (a missing `iss` is only fatal when advertised).

`iss` was dropped in three places, with the plumbing half-built —
`oauthManager`/`inspectorClient` already accepted `iss`, but nothing supplied
or forwarded it:

- `parseOAuthCallbackParams` never extracted `iss` from the callback query
- `mcpAuth` never forwarded `options.iss` to SDK `auth()` — its own comment
  ("forwarded on v2 upgrade; ignored by v1 SDK auth()") predicted this exact
  gap, so standard OAuth was broken identically, not just EMA
- EMA leg 1 (`completeIdpOidcAuthorization`) never passed `iss` to
  `exchangeAuthorization`

Thread `iss` end to end: callback query -> CallbackParams -> resumeAfterOAuth
-> completeOAuthFlow -> {mcpAuth | EMA leg 1} -> SDK. The Node callback server
and runner carry it for CLI/TUI.

Tests: `ema-mock-servers.ts` now advertises
`authorization_response_iss_parameter_supported: true` so the mock is at least
as strict as real IdPs — with it absent the SDK silently takes the lenient path
and a dropped `iss` goes unnoticed. Flipping it made three existing tests
reproduce the failure. Adds regression coverage for forwarding and for
rejecting a mismatched `iss` before any network call.

Verified end to end against live xaa.dev staging: EMA legs 1-3 (IdP SSO ->
ID-JAG mint -> resource token) now complete and the MCP session initializes.

web 3099, cli 212, tui 274 green.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VjMemXZohSR2EndBQYif2b
2026-07-15 20:16:43 -04:00
cliffhall
f4b6095719 test: close ≥90% per-file coverage gaps for mid-session auth
The mid-session auth work passed `validate`/CI but failed the local-only
`npm run test:coverage` per-file gate (≥90% lines/statements/functions/
branches) on 12 files. Add targeted unit/integration tests so every changed
file clears the gate; annotate provably-dead defensive branches with justified
`/* v8 ignore */` comments rather than relaxing the gate.

Files brought to ≥90 on all four dimensions:
- core/auth: mcpAuth, oauthUx, utils, challenge, node/runner-interactive-oauth
- core/mcp: oauthManager, inspectorClient, node/authChallengeFetch,
  remote/remoteClientTransport, remote/node/{remote-session,server}
- clients/web/src/utils: oauthResume, browserTabVisibility

`npm run test:coverage` now passes (3575 tests, 0 threshold failures);
`npm run validate` green across all four clients.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DZyeWManYTvcXxjcrhxSQc
2026-07-03 12:54:37 -04:00
Bob Dickinson
52023ff1c3 fix(auth): clear OAuth callback timer and reorder EMA scope persist
Clear the loopback callback timeout in runner-interactive-oauth finally
so already_authorized and error paths do not hang the CLI/TUI. Check EMA
step-up satisfaction against minted token scope before persisting the
requested union, matching the standard OAuth path.

Co-authored-by: Cursor <cursoragent@cursor.com>
2026-07-02 17:55:34 -07:00
Bob Dickinson
562bdd00f1 All of the mid-session and step-up auth work 2026-07-02 16:35:31 -07:00