Skip to content

OAuth test mocks are more permissive than real servers: no RFC 9207 iss coverage (EMA leg 1 + standard auth) #1693

Description

@cliffhall

Follow-up to the RFC 9207 regression fixed in #1688 (commit f4fb4b50). That bug — a dropped callback iss breaking the authorization-code exchange against any RFC 9207 server — passed a fully green npm run ci (3087 tests) and was only caught by manually smoke-testing EMA against live xaa.dev staging. Two coverage gaps let it through, and one is still open.

1. The mock authorization servers are more permissive than real ones

SDK v2 only enforces RFC 9207 §2.4 when the AS metadata advertises authorization_response_iss_parameter_supported: true:

if (iss === undefined) {
  if (issParameterSupported) throw new IssuerMismatchError(...);
  return; // lenient when not advertised
}

Real IdPs (Okta / xaa.dev) advertise it. Our mocks did not, so the SDK silently took the lenient branch and a dropped iss was invisible to CI.

  • Fixed in SDK v2 migration (behavior-neutral, versionNegotiation: 'legacy') (#1624) #1688 for the EMA mock IdP (clients/web/src/test/integration/mcp/ema-mock-servers.ts). Flipping that one flag made three existing tests immediately reproduce the live failure.
  • Still open for the composable test server AS (test-servers/src/test-server-oauth.ts): it neither advertises authorization_response_iss_parameter_supported nor emits iss on the authorization redirect.

Consequence: the mcpAuth.ts → SDK auth() iss forwarding added in #1688 has no test coverage. Standard OAuth had the identical bug as EMA, and a regression there would still slip through today.

Proposed: have test-server-oauth.ts advertise authorization_response_iss_parameter_supported: true and include iss on the redirect (both are what real servers do). Expect this to surface failures in any standard-OAuth test that drops iss — that is the point.

2. EMA e2e never drives leg 1

clients/web/src/test/integration/mcp/inspectorClient-ema-e2e.test.ts has three tests, all silent EMA (seeded/cached IdP session → legs 2–3):

  • connects via silent EMA (cached IdP session + legs 2–3)
  • reuses silent EMA on a second connect
  • persists EMA resource tokens tagged enterpriseManaged in storage

None exercise the interactive leg 1 authorization-code exchange — the exact path that broke. Coverage for it is unit-level only (idpOidc.test.ts, incl. the regressions added in #1688).

Proposed: add an e2e that drives leg 1 through the mock IdP end to end (authorize → callback with iss → exchange → ID-JAG mint → resource token), so the full EMA ladder is covered without needing live xaa.dev.

Why this matters

The general lesson is broader than iss: a mock that is laxer than production turns a green suite into false assurance. Where our mocks model a spec-defined server, they should be at least as strict as the real thing.

Reported by @cliffhall.

Metadata

Metadata

Assignees

Labels

v2Issues and PRs for v2

Type

No type

Fields

No fields configured for issues without a type.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions