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
This commit is contained in:
parent
dbb8c40245
commit
f4fb4b50c9
16 changed files with 125 additions and 17 deletions
|
|
@ -983,9 +983,12 @@ describe("App (OAuth flows)", () => {
|
|||
const r = await mount(oneHttp());
|
||||
await press(r, ["c"]);
|
||||
await waitUntil(() => h.cb.opts !== null);
|
||||
await h.cb.opts!.onCallback({ code: "abc" });
|
||||
await h.cb.opts!.onCallback({ code: "abc", iss: "https://as.example" });
|
||||
await tick();
|
||||
expect(h.clientSpies.completeOAuthFlow).toHaveBeenCalledWith("abc");
|
||||
expect(h.clientSpies.completeOAuthFlow).toHaveBeenCalledWith(
|
||||
"abc",
|
||||
"https://as.example",
|
||||
);
|
||||
});
|
||||
|
||||
it("reports an OAuth callback error during a 401 connect", async () => {
|
||||
|
|
|
|||
|
|
@ -2326,6 +2326,7 @@ function App() {
|
|||
connectStartRef.current = Date.now();
|
||||
await client.resumeAfterOAuth(params.code, {
|
||||
remoteSessionId: resumeSnapshot?.remoteSessionId,
|
||||
iss: params.iss,
|
||||
});
|
||||
} catch (err) {
|
||||
connectStartRef.current = undefined;
|
||||
|
|
|
|||
|
|
@ -381,6 +381,7 @@ describe("emaFlow", () => {
|
|||
const tokens = await completeEmaIdpAuthorizationAndMint(
|
||||
config,
|
||||
"auth-code",
|
||||
IDP_ISSUER,
|
||||
);
|
||||
|
||||
expect(tokens.access_token).toBe("resource-access-token");
|
||||
|
|
@ -415,7 +416,7 @@ describe("emaFlow", () => {
|
|||
await startEmaIdpAuthorization(config);
|
||||
|
||||
await expect(
|
||||
completeEmaIdpAuthorizationAndMint(config, "auth-code"),
|
||||
completeEmaIdpAuthorizationAndMint(config, "auth-code", IDP_ISSUER),
|
||||
).rejects.toThrow(/EMA legs 2–3 \(resource token mint\)/);
|
||||
});
|
||||
|
||||
|
|
@ -480,7 +481,7 @@ describe("emaFlow", () => {
|
|||
});
|
||||
|
||||
await expect(
|
||||
completeEmaIdpAuthorizationAndMint(config, "auth-code"),
|
||||
completeEmaIdpAuthorizationAndMint(config, "auth-code", IDP_ISSUER),
|
||||
).rejects.toThrow(
|
||||
"EMA leg 1 (IdP authorization code exchange): leg1-non-error",
|
||||
);
|
||||
|
|
|
|||
|
|
@ -390,6 +390,7 @@ describe("completeIdpOidcAuthorization", () => {
|
|||
const result = await completeIdpOidcAuthorization({
|
||||
idp,
|
||||
authorizationCode: "auth-code-xyz",
|
||||
iss: IDP_ISSUER,
|
||||
redirectUrl: "http://127.0.0.1:3000/oauth/callback",
|
||||
storage,
|
||||
fetchFn,
|
||||
|
|
@ -465,6 +466,7 @@ describe("completeIdpOidcAuthorization", () => {
|
|||
const result = await completeIdpOidcAuthorization({
|
||||
idp,
|
||||
authorizationCode: "code",
|
||||
iss: IDP_ISSUER,
|
||||
redirectUrl: "http://127.0.0.1:3000/oauth/callback",
|
||||
storage,
|
||||
fetchFn,
|
||||
|
|
@ -498,10 +500,67 @@ describe("completeIdpOidcAuthorization", () => {
|
|||
completeIdpOidcAuthorization({
|
||||
idp,
|
||||
authorizationCode: "code",
|
||||
iss: IDP_ISSUER,
|
||||
redirectUrl: "http://127.0.0.1:3000/oauth/callback",
|
||||
storage,
|
||||
fetchFn,
|
||||
}),
|
||||
).rejects.toThrow("IdP token response did not include an ID Token");
|
||||
});
|
||||
|
||||
// Regression: the IdP metadata advertises
|
||||
// `authorization_response_iss_parameter_supported`, so the SDK enforces
|
||||
// RFC 9207 §2.4 and rejects the exchange unless the callback `iss` is
|
||||
// forwarded. Dropping `iss` anywhere between the callback and
|
||||
// `exchangeAuthorization` breaks EMA leg 1 against every real IdP.
|
||||
it("forwards the callback iss to the token exchange (RFC 9207)", async () => {
|
||||
storage = buildStorage();
|
||||
const idToken = jwtWithExp(Math.floor(Date.now() / 1000) + 3600);
|
||||
const fetchFn = vi.fn(
|
||||
async () =>
|
||||
new Response(
|
||||
JSON.stringify({
|
||||
id_token: idToken,
|
||||
access_token: "at",
|
||||
token_type: "Bearer",
|
||||
}),
|
||||
),
|
||||
);
|
||||
|
||||
await expect(
|
||||
completeIdpOidcAuthorization({
|
||||
idp,
|
||||
authorizationCode: "code",
|
||||
redirectUrl: "http://127.0.0.1:3000/oauth/callback",
|
||||
storage,
|
||||
fetchFn,
|
||||
}),
|
||||
).rejects.toThrow(/Issuer mismatch/);
|
||||
|
||||
const result = await completeIdpOidcAuthorization({
|
||||
idp,
|
||||
authorizationCode: "code",
|
||||
iss: IDP_ISSUER,
|
||||
redirectUrl: "http://127.0.0.1:3000/oauth/callback",
|
||||
storage,
|
||||
fetchFn,
|
||||
});
|
||||
expect(result.idToken).toBe(idToken);
|
||||
});
|
||||
|
||||
it("rejects a callback iss from a different issuer (mix-up defense)", async () => {
|
||||
storage = buildStorage();
|
||||
const fetchFn = vi.fn();
|
||||
await expect(
|
||||
completeIdpOidcAuthorization({
|
||||
idp,
|
||||
authorizationCode: "code",
|
||||
iss: "https://evil.example.com",
|
||||
redirectUrl: "http://127.0.0.1:3000/oauth/callback",
|
||||
storage,
|
||||
fetchFn,
|
||||
}),
|
||||
).rejects.toThrow(/Issuer mismatch/);
|
||||
expect(fetchFn).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -56,15 +56,21 @@ function createMockCallbackServer(handlers: {
|
|||
} as unknown as OAuthCallbackServer;
|
||||
}
|
||||
|
||||
/** Drive the loopback callback the way a browser redirect would. */
|
||||
const CALLBACK_ISS = "https://as.example";
|
||||
|
||||
/**
|
||||
* Drive the loopback callback the way a browser redirect would, including the
|
||||
* RFC 9207 `iss` that real authorization servers send.
|
||||
*/
|
||||
async function simulateCallback(
|
||||
handlers: MockCallbackHandlers,
|
||||
code = "auth-code-123",
|
||||
iss: string | undefined = CALLBACK_ISS,
|
||||
): Promise<void> {
|
||||
if (!handlers.onCallback) {
|
||||
throw new Error("onCallback not registered");
|
||||
}
|
||||
await handlers.onCallback({ code });
|
||||
await handlers.onCallback({ code, iss });
|
||||
}
|
||||
|
||||
describe("runRunnerInteractiveOAuth", () => {
|
||||
|
|
@ -145,7 +151,10 @@ describe("runRunnerInteractiveOAuth", () => {
|
|||
});
|
||||
|
||||
expect(result).toEqual({ kind: "success" });
|
||||
expect(client.completeOAuthFlow).toHaveBeenCalledWith("auth-code-123");
|
||||
expect(client.completeOAuthFlow).toHaveBeenCalledWith(
|
||||
"auth-code-123",
|
||||
CALLBACK_ISS,
|
||||
);
|
||||
expect(redirectUrlProvider.redirectUrl).toBe(
|
||||
"http://127.0.0.1:6276/oauth/callback",
|
||||
);
|
||||
|
|
@ -179,7 +188,10 @@ describe("runRunnerInteractiveOAuth", () => {
|
|||
authorizationUrl,
|
||||
);
|
||||
expect(client.authenticate).not.toHaveBeenCalled();
|
||||
expect(client.completeOAuthFlow).toHaveBeenCalledWith("step-up-code");
|
||||
expect(client.completeOAuthFlow).toHaveBeenCalledWith(
|
||||
"step-up-code",
|
||||
CALLBACK_ISS,
|
||||
);
|
||||
});
|
||||
|
||||
it("returns insufficient_scope when post-step-up check fails", async () => {
|
||||
|
|
|
|||
|
|
@ -1525,11 +1525,12 @@ describe("OAuthManager", () => {
|
|||
);
|
||||
expect(outcome.kind).toBe("interactive");
|
||||
|
||||
await manager.completeOAuthFlow("auth-code");
|
||||
await manager.completeOAuthFlow("auth-code", "https://idp.example.com");
|
||||
|
||||
expect(mintSpy).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ scope: "mcp tools:read weather:read" }),
|
||||
"auth-code",
|
||||
"https://idp.example.com",
|
||||
);
|
||||
expect(storageOf(params).saveScope).toHaveBeenCalledWith(
|
||||
SERVER_URL,
|
||||
|
|
|
|||
|
|
@ -21,7 +21,15 @@ export const EMA_MOCK_IDP_CLIENT_SECRET = "ema-mock-idp-secret";
|
|||
export const EMA_MOCK_RESOURCE_CLIENT_ID = "ema-mock-resource-client";
|
||||
export const EMA_MOCK_RESOURCE_CLIENT_SECRET = "ema-mock-resource-secret";
|
||||
|
||||
/** Fields required by SDK `OAuthMetadataSchema` for discovery in tests. */
|
||||
/**
|
||||
* Fields required by SDK `OAuthMetadataSchema` for discovery in tests.
|
||||
*
|
||||
* `authorization_response_iss_parameter_supported` mirrors real-world IdPs
|
||||
* (e.g. Okta/xaa.dev): once advertised, the SDK enforces RFC 9207 §2.4 and
|
||||
* rejects a callback whose `iss` is missing. Keep it on so the mock stays at
|
||||
* least as strict as production servers — with it absent the SDK silently takes
|
||||
* its lenient path and a dropped `iss` goes unnoticed.
|
||||
*/
|
||||
export function minimalOAuthAsMetadata(
|
||||
baseUrl: string,
|
||||
authorizationEndpoint = `${baseUrl}/authorize`,
|
||||
|
|
@ -32,6 +40,7 @@ export function minimalOAuthAsMetadata(
|
|||
token_endpoint: `${baseUrl}/token`,
|
||||
response_types_supported: ["code"],
|
||||
token_endpoint_auth_methods_supported: ["client_secret_post"],
|
||||
authorization_response_iss_parameter_supported: true,
|
||||
};
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -125,11 +125,13 @@ export async function startEmaIdpAuthorization(
|
|||
export async function completeEmaIdpAuthorizationAndMint(
|
||||
config: EmaFlowConfig,
|
||||
authorizationCode: string,
|
||||
iss?: string,
|
||||
): Promise<OAuthTokens> {
|
||||
try {
|
||||
await completeIdpOidcAuthorization({
|
||||
idp: config.idp,
|
||||
authorizationCode,
|
||||
iss,
|
||||
redirectUrl: config.redirectUrl,
|
||||
storage: config.storage,
|
||||
fetchFn: config.fetchFn,
|
||||
|
|
|
|||
|
|
@ -97,6 +97,12 @@ export async function startIdpOidcAuthorization(params: {
|
|||
export async function completeIdpOidcAuthorization(params: {
|
||||
idp: EnterpriseManagedAuthIdpConfig;
|
||||
authorizationCode: string;
|
||||
/**
|
||||
* RFC 9207 `iss` from the IdP callback. Forwarded to the SDK, which rejects
|
||||
* the exchange when the IdP metadata advertises
|
||||
* `authorization_response_iss_parameter_supported` but no `iss` arrives.
|
||||
*/
|
||||
iss?: string;
|
||||
redirectUrl: string;
|
||||
storage: OAuthStorage;
|
||||
fetchFn?: typeof fetch;
|
||||
|
|
@ -127,6 +133,7 @@ export async function completeIdpOidcAuthorization(params: {
|
|||
metadata,
|
||||
clientInformation,
|
||||
authorizationCode: params.authorizationCode,
|
||||
iss: params.iss,
|
||||
codeVerifier,
|
||||
redirectUri: params.redirectUrl,
|
||||
fetchFn: params.fetchFn,
|
||||
|
|
|
|||
|
|
@ -44,7 +44,7 @@ export type McpAuthResult = "AUTHORIZED" | "REDIRECT";
|
|||
export interface McpAuthOptions {
|
||||
serverUrl: string | URL;
|
||||
authorizationCode?: string;
|
||||
/** RFC 9207 callback `iss` — forwarded on v2 upgrade; ignored by v1 SDK `auth()`. */
|
||||
/** RFC 9207 callback `iss`, validated by the SDK against the metadata issuer. */
|
||||
iss?: string;
|
||||
scope?: string;
|
||||
resourceMetadataUrl?: URL;
|
||||
|
|
@ -74,6 +74,7 @@ export async function mcpAuth(
|
|||
return sdkAuth(provider, {
|
||||
serverUrl: options.serverUrl,
|
||||
authorizationCode: options.authorizationCode,
|
||||
iss: options.iss,
|
||||
scope: options.scope,
|
||||
resourceMetadataUrl: options.resourceMetadataUrl,
|
||||
fetchFn: options.fetchFn,
|
||||
|
|
|
|||
|
|
@ -30,6 +30,8 @@ function escapeHtml(s: string): string {
|
|||
export type OAuthCallbackHandler = (params: {
|
||||
code: string;
|
||||
state?: string;
|
||||
/** RFC 9207 `iss`, when the authorization server sends it. */
|
||||
iss?: string;
|
||||
}) => Promise<void>;
|
||||
|
||||
export type OAuthErrorHandler = (params: {
|
||||
|
|
@ -181,7 +183,7 @@ export class OAuthCallbackServer {
|
|||
this.handled = true;
|
||||
const cb = this.onCallback;
|
||||
if (cb) {
|
||||
cb({ code: params.code, state })
|
||||
cb({ code: params.code, state, iss: params.iss })
|
||||
.then(() => {
|
||||
send(200, SUCCESS_HTML);
|
||||
void this.stop();
|
||||
|
|
|
|||
|
|
@ -9,7 +9,7 @@ import type { RunnerOAuthCallbackConfig } from "./runner-oauth-callback.js";
|
|||
export interface RunnerInteractiveOAuthClient {
|
||||
authenticate(): Promise<URL | undefined>;
|
||||
beginInteractiveAuthorization(authorizationUrl: URL): Promise<void>;
|
||||
completeOAuthFlow(authorizationCode: string): Promise<void>;
|
||||
completeOAuthFlow(authorizationCode: string, iss?: string): Promise<void>;
|
||||
checkAuthChallengeSatisfied(challenge: AuthChallenge): Promise<boolean>;
|
||||
}
|
||||
|
||||
|
|
@ -67,7 +67,7 @@ export async function runRunnerInteractiveOAuth(
|
|||
path: options.callbackListen.pathname,
|
||||
onCallback: async (params) => {
|
||||
try {
|
||||
await options.client.completeOAuthFlow(params.code);
|
||||
await options.client.completeOAuthFlow(params.code, params.iss);
|
||||
flowResolve();
|
||||
} catch (err) {
|
||||
flowReject(err instanceof Error ? err : new Error(String(err)));
|
||||
|
|
|
|||
|
|
@ -105,6 +105,12 @@ export type CallbackParams =
|
|||
successful: true;
|
||||
// The authorization code is generated by the authorization server.
|
||||
code: string;
|
||||
// RFC 9207 `iss`. Present only when the authorization server includes it.
|
||||
// Forwarded to the SDK, which validates it against the issuer in the
|
||||
// validated metadata (RFC 9207 §2.4) to detect mix-up attacks. Servers
|
||||
// advertising `authorization_response_iss_parameter_supported: true` MUST
|
||||
// send it, and the SDK rejects the callback when it is missing.
|
||||
iss?: string;
|
||||
}
|
||||
| {
|
||||
successful: false;
|
||||
|
|
|
|||
|
|
@ -89,7 +89,10 @@ export const parseOAuthCallbackParams = (location: string): CallbackParams => {
|
|||
|
||||
const code = params.get("code");
|
||||
if (code) {
|
||||
return { successful: true, code };
|
||||
const iss = params.get("iss");
|
||||
return iss === null
|
||||
? { successful: true, code }
|
||||
: { successful: true, code, iss };
|
||||
}
|
||||
|
||||
const error = params.get("error");
|
||||
|
|
|
|||
|
|
@ -3360,9 +3360,9 @@ export class InspectorClient extends InspectorClientEventTarget {
|
|||
*/
|
||||
async resumeAfterOAuth(
|
||||
authorizationCode: string,
|
||||
options?: { remoteSessionId?: string },
|
||||
options?: { remoteSessionId?: string; iss?: string },
|
||||
): Promise<void> {
|
||||
await this.completeOAuthFlow(authorizationCode);
|
||||
await this.completeOAuthFlow(authorizationCode, options?.iss);
|
||||
|
||||
const remoteSessionId = options?.remoteSessionId;
|
||||
const transport = this.baseTransport;
|
||||
|
|
|
|||
|
|
@ -293,6 +293,7 @@ export class OAuthManager {
|
|||
const tokens = await completeEmaIdpAuthorizationAndMint(
|
||||
config,
|
||||
authorizationCode,
|
||||
iss,
|
||||
);
|
||||
const requestedScope = this.pendingAuthorizationScope;
|
||||
const scopeToPersist = resolvePersistedScopeAfterGrant(
|
||||
|
|
|
|||
Loading…
Reference in a new issue