You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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)thrownewIssuerMismatchError(...);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.
❌ 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.
Follow-up to the RFC 9207 regression fixed in #1688 (commit
f4fb4b50). That bug — a dropped callbackissbreaking the authorization-code exchange against any RFC 9207 server — passed a fully greennpm 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:Real IdPs (Okta / xaa.dev) advertise it. Our mocks did not, so the SDK silently took the lenient branch and a dropped
isswas invisible to CI.clients/web/src/test/integration/mcp/ema-mock-servers.ts). Flipping that one flag made three existing tests immediately reproduce the live failure.test-servers/src/test-server-oauth.ts): it neither advertisesauthorization_response_iss_parameter_supportednor emitsisson the authorization redirect.Consequence: the
mcpAuth.ts→ SDKauth()issforwarding 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.tsadvertiseauthorization_response_iss_parameter_supported: trueand includeisson the redirect (both are what real servers do). Expect this to surface failures in any standard-OAuth test that dropsiss— that is the point.2. EMA e2e never drives leg 1
clients/web/src/test/integration/mcp/inspectorClient-ema-e2e.test.tshas 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 connectpersists EMA resource tokens tagged enterpriseManaged in storageNone 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.