Commit graph inspector/core/auth/challenge.ts
Author SHA1 Message Date
cliffhall
f8a46726b7 fix(auth): pass the challenge's resource_metadata URL to SDK auth()
An RFC 9728 `WWW-Authenticate: Bearer resource_metadata="…"` challenge was
parsed and then dropped by the normalized `AuthChallenge`, so every
challenge-driven `mcpAuth()` call ran discovery without it and probed
locations derived from the MCP server URL instead. OAuth failed outright
against any server whose metadata document lives off the well-known path.

The SDK handles this correctly inside its own transports, but the Inspector
never reaches that path — it intercepts the 401 and drives `auth()` itself
to capture the authorization URL and run the step-up scope union.

Carry the advertised URL on `AuthChallenge` as a string (it crosses the web
client's remote-backend boundary as JSON), convert it at the OAuth boundary,
and pass it to the standard challenge leg, the forced step-up leg, and CIMD
pre-registration — which runs before `auth()` and did its own discovery. A
malformed value is ignored rather than surfaced, matching the SDK's parser.
The callback leg needs nothing: SDK `auth()` persists the URL in its
discovery state.

Adds an `oauth.resourceMetadataPath` test-server mode that moves the
document off the well-known path entirely, so the integration test proves
discovery genuinely fails without the fix.

Closes #2071

Signed-off-by: cliffhall <cliff@futurescale.com>
2026-08-24 19:48:40 -04:00
Cliff Hall
01df9d81bb
fix(core): start OAuth on a modern/auto-era connect (#1805) (#1809)
* fix(core): start OAuth on a modern/auto-era connect (#1805)

The SDK's `server/discover` negotiation probe runs before anything else on
`protocolEra: "auto" | "modern"`, and its classifier reports whatever the
transport threw as `SdkError(ERA_NEGOTIATION_FAILED)` with the real error moved
to `data.cause`. That buried both connect-time auth signals: the remote path's
`AuthRecoveryRequiredError` (matched with `instanceof`, and carrying the
authorization URL) and a direct transport's `AuthChallengeError`. An
OAuth-protected server that authorized fine on the legacy era instead produced a
dead-end "Version negotiation probe failed" — no redirect, and the connection
status flipped to `error` despite `isConnectAuthRecoveryError` existing to
prevent exactly that.

- `findNestedAuthError()` walks `cause` / `data.cause` for either typed error.
  Not gated on the SDK's error code or message, so it survives rewording.
- `connect()` unwraps the rejection at the earliest point, ahead of
  `withDirectAuthRecovery` (whose `isAuthChallengeError` check is shallow), the
  outer status guard, and every client's connect error handling — so web, CLI,
  and TUI are all fixed without a client-side change.
- On the direct path, enable `interceptAuthChallenges` for the probing eras even
  with no stored tokens. Without an authProvider the 401 reaches the SDK as a raw
  `SdkHttpError`, and the classifier ignores the HTTP status (it only looks for a
  JSON-RPC error body) — so pin mode rethrew it with the 401 discarded entirely,
  no status and not even a cause for the walk to find.

Verified against a live dual-era server that answers 401 on both `initialize`
and `server/discover`: `auto` and `modern` now reject with
`AuthRecoveryRequiredError` carrying a real authorization URL. Each half of the
fix is load-bearing — reverting either fails the new integration tests.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013SNjAqoEhUVVkr3UStR6om

* docs(core): mark the era-probe challenge interception as a removable workaround

Per review on #1806: the `|| this.probesProtocolEra()` clause compensates for
an upstream SDK gap (modelcontextprotocol/typescript-sdk#2561, tracked as
#1807). Make its lifespan explicit so whoever picks up #1807 knows this block
is the thing to delete, rather than having to re-derive it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016S7h83UKNQ7kvSiFDNksCt

* fix(core): resolve connect() when auth recovery is satisfied silently

Review findings on #1809:

- connect() built its handshake promise once, outside the runConnect closure
  withDirectAuthRecovery retries. On a "satisfied" outcome the recovery runs a
  complete nested connect() via reconnectAfterAuthRecovery(), so the retry leg
  re-awaited the already-rejected promise and rejected a connect() whose client
  was in fact connected. Short-circuit the retry when already connected, and
  skip the post-connect block the nested connect already ran (a duplicate
  `connect` event re-triggers every list-state manager's refresh).
- Document the accepted 403 side effect of enabling interceptAuthChallenges
  with no stored tokens, and why every `{ pin }` counts as probing.
- Replace the private-method double cast in the era-probe test with an
  observable assertion on the transport options, and split the non-auth
  failure case per era instead of a message alternation that passed either way.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016S7h83UKNQ7kvSiFDNksCt

* fix(core): key the connect retry leg off attempt, bound nested recoveries

Second-pass review findings on #1809:

- The satisfied-recovery short-circuit keyed off live connection status, so an
  onclose landing between the nested connect() and the retry leg fell through
  to the rejected handshake promise and reported the stale 401 as the reason
  the session died. withDirectAuthRecovery now passes `attempt` to its
  operation, so connect() identifies the retry leg structurally and reports the
  teardown when the recovered session is already gone.
- Bound nested silent recoveries. The `attempt >= 1` guard is per-call and a
  satisfied outcome recovers via a nested connect() with a fresh counter, so a
  server that keeps rejecting freshly refreshed tokens recursed without bound.
  Counted across the nesting boundary, capped at 3.
- Strengthen the regression test (server info populated by the nested connect,
  no duplicate statusChange) and add the two missing cases: a failed reconnect
  still rejects, and the recursion bound terminates.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016S7h83UKNQ7kvSiFDNksCt

* test(core): cover the recovery teardown branch; fix a duplicated case

Third-pass review findings on #1809, both about tests added in the previous
commit:

- The retry leg's teardown error had no coverage, so a regression to the old
  status fall-through would have stayed green. Covered deterministically: the
  nested connect dispatches `connect` after setting "connected", so closing the
  live transport from that listener flips the status before the retry leg runs.
  Asserts the error identity, which is the point of the change.
- "still rejects when the reconnect underneath the recovery fails" used a
  challenging transport for every connect, so it walked the same bounded chain
  as the recursion-bound test rather than covering a non-auth reconnect
  failure, and its comment claimed an "error" status the auth path never
  reaches. Now fails the reconnect with a plain error and asserts both.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016S7h83UKNQ7kvSiFDNksCt

---------

Co-authored-by: rinormaloku <rinormaloku37@gmail.com>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-07-27 15:06:01 -04:00
Cliff Hall
373f346e5f
chore: close quality-gate coverage gaps (#1689) (#1766)
Some checks failed
Run install, format, lint, build, and test on every push / build (push) Has been cancelled
Run install, format, lint, build, and test on every push / publish (push) Has been cancelled
Run install, format, lint, build, and test on every push / publish-github-container-registry (push) Has been cancelled
Closes #1689. Typecheck cli/tui src, add a core/ format+lint gate, and gate core/json + core/client coverage. See PR #1766 for the full review history.
2026-07-24 17:20:12 -04:00
Bob Dickinson
562bdd00f1 All of the mid-session and step-up auth work 2026-07-02 16:35:31 -07:00