From fb952d8a154ffe4832456bf296586179214d875e Mon Sep 17 00:00:00 2001 From: Rhys Sullivan <39114868+RhysSullivan@users.noreply.github.com> Date: Fri, 28 Aug 2026 02:14:36 -0700 Subject: [PATCH 1/3] Gate the RFC 8707 resource parameter on advertised authorization-server support --- ...auth-resource-indicator-capability-gate.md | 19 ++ packages/core/sdk/src/oauth-discovery.test.ts | 59 +++++ packages/core/sdk/src/oauth-discovery.ts | 12 + packages/core/sdk/src/oauth-helpers.test.ts | 230 ++++++++++++++++++ packages/core/sdk/src/oauth-helpers.ts | 110 +++++++-- 5 files changed, 409 insertions(+), 21 deletions(-) create mode 100644 .changeset/oauth-resource-indicator-capability-gate.md diff --git a/.changeset/oauth-resource-indicator-capability-gate.md b/.changeset/oauth-resource-indicator-capability-gate.md new file mode 100644 index 000000000..169038057 --- /dev/null +++ b/.changeset/oauth-resource-indicator-capability-gate.md @@ -0,0 +1,19 @@ +--- +"executor": patch +--- + +**The RFC 8707 `resource` parameter is now gated on what the authorization server advertises** + +Executor sent `resource` unconditionally on every authorization, code exchange, refresh, and client-credentials request whenever an OAuth app had a resource configured. Microsoft Entra v2 rejects that: a request carrying both `resource` and a v2 `scope` such as `https://api.fabric.microsoft.com/.default` fails with `AADSTS9010010` before the consent screen, so the Microsoft Fabric Core MCP server could not be connected at all. + +The rule now applied on all four grants, in full: + +- **No authorization-server metadata was discovered → send `resource`.** Nothing is known about the server, and the MCP authorization spec expects resource indicators. This is the previous behavior, unchanged, and it covers every manually configured provider. +- **Metadata was discovered and advertises `resource_indicators_supported: true` → send `resource`.** +- **Metadata was discovered and the flag is absent or `false` → omit `resource`.** RFC 8414 §2 makes an omitted metadata field mean "not advertised", and RFC 8707 §2 makes `resource` optional for clients, so omitting it is conformant. + +The decision reads only what a server publishes about itself — there is no Microsoft special case and no host allowlist. `resource_indicators_supported` is not in the IANA authorization-server metadata registry (RFC 8707 registered no discovery parameter), but it is the only machine-readable signal a server gives, so discovery now parses it and threads it to the grant helpers. + +The protected resource is still discovered, validated against the requested endpoint, and retained in the flow state for MCP binding when the parameter itself is withheld. The `executor.oauth.has_resource` span attribute now reports what actually went on the wire rather than what was configured. + +Providers whose authorization server advertises the capability, and providers reached without any metadata discovery, are unaffected. diff --git a/packages/core/sdk/src/oauth-discovery.test.ts b/packages/core/sdk/src/oauth-discovery.test.ts index 4562e9188..84a664fde 100644 --- a/packages/core/sdk/src/oauth-discovery.test.ts +++ b/packages/core/sdk/src/oauth-discovery.test.ts @@ -369,6 +369,9 @@ describe("beginDynamicAuthorization", () => { scopes_supported: ["openid", "profile", "email", "offline_access", "workspace:member"], response_types_supported: ["code"], code_challenge_methods_supported: ["S256"], + // An MCP-profile server implements RFC 8707, so it advertises it — + // that is what earns the `resource` parameter asserted below. + resource_indicators_supported: true, }); } if (request.url === "/oauth/register") { @@ -624,6 +627,7 @@ describe("beginDynamicAuthorization", () => { registration_endpoint: `${baseUrl}/register`, response_types_supported: ["code"], code_challenge_methods_supported: ["S256"], + resource_indicators_supported: true, }); } if (request.url === "/register") { @@ -652,6 +656,61 @@ describe("beginDynamicAuthorization", () => { ), ); + // #1789 — Microsoft Entra v2 publishes authorization-server metadata, does + // NOT advertise RFC 8707, and rejects `resource` alongside a v2 `scope` with + // AADSTS9010010. Metadata that stays silent on the capability means "not + // advertised" (RFC 8414 §2), so the resource indicator is withheld — while + // the protected resource itself is still discovered and kept in the state + // for MCP binding. + it.effect("omits the resource parameter when the AS metadata does not advertise RFC 8707", () => + withOAuthFixture( + (request, baseUrl) => { + if (request.url === "/.well-known/oauth-protected-resource/v1/mcp/core") { + return sendJson({ + resource: baseUrl, + authorization_servers: [baseUrl], + scopes_supported: [`${baseUrl}/.default`], + }); + } + if (request.url === "/.well-known/oauth-authorization-server") { + return sendJson({ + issuer: baseUrl, + authorization_endpoint: `${baseUrl}/oauth2/v2.0/authorize`, + token_endpoint: `${baseUrl}/oauth2/v2.0/token`, + registration_endpoint: `${baseUrl}/register`, + response_types_supported: ["code"], + code_challenge_methods_supported: ["S256"], + }); + } + if (request.url === "/register") { + return sendJson( + { + client_id: "entra-client", + redirect_uris: ["https://app/cb"], + token_endpoint_auth_method: "none", + }, + 201, + ); + } + return notFound(); + }, + ({ baseUrl }) => + Effect.gen(function* () { + const result = yield* beginDynamicAuthorization({ + endpoint: `${baseUrl}/v1/mcp/core`, + redirectUrl: "https://app/cb", + state: "s", + }); + + const authUrl = new URL(result.authorizationUrl); + expect(authUrl.searchParams.has("resource")).toBe(false); + expect(authUrl.searchParams.get("scope")).toBe(`${baseUrl}/.default`); + // Withheld from the wire, still retained for MCP discovery/binding. + expect(result.state.resource).toBe(baseUrl); + }), + ), + ); + it.effect("includes client_uri in the DCR body", () => withOAuthFixture( (request, baseUrl) => { diff --git a/packages/core/sdk/src/oauth-discovery.ts b/packages/core/sdk/src/oauth-discovery.ts index bb30fec79..901e954c5 100644 --- a/packages/core/sdk/src/oauth-discovery.ts +++ b/packages/core/sdk/src/oauth-discovery.ts @@ -82,6 +82,14 @@ export const OAuthAuthorizationServerMetadataSchema = Schema.Struct({ introspection_endpoint: Schema.optional(Schema.String), userinfo_endpoint: Schema.optional(Schema.String), id_token_signing_alg_values_supported: Schema.optional(StringArray), + /** Whether this server implements RFC 8707 resource indicators. RFC 8707 + * registered no metadata parameter, so this is a de-facto field rather than + * an IANA-registered one — but it is the only machine-readable signal a + * server gives, and `shouldSendResourceIndicator` reads it to decide whether + * `resource` goes on authorization and token requests. An absent field means + * "not advertised" (RFC 8414 §2), NOT "unknown": a server that publishes + * metadata and stays silent here gets no `resource`. */ + resource_indicators_supported: Schema.optional(Schema.Boolean), /** draft-ietf-oauth-identity-assertion-authz-grant-04 §7.2 — the * authorization grant profiles this Resource Authorization Server * implements. Advertising a profile says only that the server implements @@ -898,6 +906,10 @@ export const beginDynamicAuthorization = ( state: input.state, codeChallenge, resource: resourceValue, + // The AS metadata was just discovered, so its RFC 8707 capability is + // known: send `resource` only if it advertises support. Entra v2 rejects + // `resource` alongside a v2 `scope` (AADSTS9010010). + authorizationServerMetadata: authServer.metadata, endpointUrlPolicy: options.endpointUrlPolicy, }); diff --git a/packages/core/sdk/src/oauth-helpers.test.ts b/packages/core/sdk/src/oauth-helpers.test.ts index ab5dcb903..12407f70c 100644 --- a/packages/core/sdk/src/oauth-helpers.test.ts +++ b/packages/core/sdk/src/oauth-helpers.test.ts @@ -26,7 +26,10 @@ import { isUnusableSuccessTokenResponse, refreshAccessToken, shouldRefreshToken, + shouldSendResourceIndicator, + type ResourceIndicatorSupport, } from "./oauth-helpers"; +import type { OAuthAuthorizationServerMetadata } from "./oauth-discovery"; import { serveTestHttpApp } from "./testing"; interface TokenCall { @@ -1423,6 +1426,233 @@ describe("refreshAccessToken", () => { ); }); +// --------------------------------------------------------------------------- +// RFC 8707 resource indicators — the AS capability gate (#1789) +// +// THE RULE under test, on every grant that can carry `resource`: +// no metadata → send (unchanged; MCP's default expectation) +// metadata + flag true → send +// metadata + flag absent → omit (RFC 8414 §2: not advertised) +// metadata + flag false → omit +// +// The "metadata + flag absent" row is the bug: Microsoft Entra v2 publishes +// metadata, does not advertise resource indicators, and rejects `resource` +// alongside a v2 `scope` with AADSTS9010010. +// --------------------------------------------------------------------------- + +const RESOURCE = "https://api.example.com/v1/mcp"; +const AS_SUPPORTED: ResourceIndicatorSupport = { resource_indicators_supported: true }; +const AS_UNSUPPORTED: ResourceIndicatorSupport = { resource_indicators_supported: false }; +/** Metadata that exists but never mentions resource indicators — the Entra + * shape. Typed as the whole discovered document, not just the one flag, so + * this fixture stays honest about what a real caller threads through. */ +const AS_SILENT: OAuthAuthorizationServerMetadata = { + issuer: "https://login.microsoftonline.com/tenant/v2.0", + authorization_endpoint: "https://login.microsoftonline.com/tenant/oauth2/v2.0/authorize", + token_endpoint: "https://login.microsoftonline.com/tenant/oauth2/v2.0/token", +}; + +describe("shouldSendResourceIndicator", () => { + it("sends when no metadata was discovered", () => { + expect(shouldSendResourceIndicator()).toBe(true); + expect(shouldSendResourceIndicator(undefined)).toBe(true); + expect(shouldSendResourceIndicator(null)).toBe(true); + }); + + it("sends only when the server advertises support", () => { + expect(shouldSendResourceIndicator(AS_SUPPORTED)).toBe(true); + expect(shouldSendResourceIndicator(AS_UNSUPPORTED)).toBe(false); + expect(shouldSendResourceIndicator(AS_SILENT)).toBe(false); + expect(shouldSendResourceIndicator({})).toBe(false); + }); +}); + +describe("buildAuthorizationUrl resource-indicator gating", () => { + const baseInput = { + authorizationUrl: "https://example.com/authorize", + clientId: "client-123", + redirectUrl: "https://app.example.com/callback", + scopes: ["https://api.fabric.microsoft.com/.default"] as const, + state: "state-abc", + codeChallenge: "E9Melhoa2OwvFrEMTJguCHaoeK1t8URWbuGJSstw-cM", + resource: RESOURCE, + }; + + it("sends resource when no authorization-server metadata is known", () => { + const url = new URL(buildAuthorizationUrl(baseInput)); + expect(url.searchParams.get("resource")).toBe(RESOURCE); + }); + + it("sends resource when the server advertises resource_indicators_supported", () => { + const url = new URL( + buildAuthorizationUrl({ ...baseInput, authorizationServerMetadata: AS_SUPPORTED }), + ); + expect(url.searchParams.get("resource")).toBe(RESOURCE); + }); + + it("omits resource when the server publishes metadata without the capability", () => { + for (const metadata of [AS_UNSUPPORTED, AS_SILENT]) { + const url = new URL( + buildAuthorizationUrl({ ...baseInput, authorizationServerMetadata: metadata }), + ); + expect(url.searchParams.has("resource")).toBe(false); + // The scope the AS DOES accept must survive the veto untouched. + expect(url.searchParams.get("scope")).toBe("https://api.fabric.microsoft.com/.default"); + } + }); +}); + +describe("exchangeAuthorizationCode resource-indicator gating", () => { + const exchange = ( + tokenUrl: string, + authorizationServerMetadata?: ResourceIndicatorSupport | null, + ) => + exchangeAuthorizationCode({ + tokenUrl, + clientId: "cid", + redirectUrl: "https://app.example.com/cb", + codeVerifier: "verifier", + code: "abc", + resource: RESOURCE, + authorizationServerMetadata, + }); + + it.effect("sends resource when no authorization-server metadata is known", () => + withTokenEndpoint(tokenResponse(validCodeBody), ({ tokenUrl, calls }) => + Effect.gen(function* () { + yield* exchange(tokenUrl); + expect((yield* calls)[0]!.body.get("resource")).toBe(RESOURCE); + }), + ), + ); + + it.effect("sends resource when the server advertises support", () => + withTokenEndpoint(tokenResponse(validCodeBody), ({ tokenUrl, calls }) => + Effect.gen(function* () { + yield* exchange(tokenUrl, AS_SUPPORTED); + expect((yield* calls)[0]!.body.get("resource")).toBe(RESOURCE); + }), + ), + ); + + it.effect("omits resource when the server publishes metadata without the capability", () => + withTokenEndpoint(tokenResponse(validCodeBody), ({ tokenUrl, calls }) => + Effect.gen(function* () { + yield* exchange(tokenUrl, AS_SILENT); + yield* exchange(tokenUrl, AS_UNSUPPORTED); + const bodies = (yield* calls).map((call) => call.body); + expect(bodies).toHaveLength(2); + for (const body of bodies) { + expect(body.has("resource")).toBe(false); + // The grant itself is untouched — only `resource` is withheld. + expect(body.get("grant_type")).toBe("authorization_code"); + expect(body.get("code_verifier")).toBe("verifier"); + } + }), + ), + ); +}); + +describe("exchangeClientCredentials resource-indicator gating", () => { + const exchange = ( + tokenUrl: string, + authorizationServerMetadata?: ResourceIndicatorSupport | null, + ) => + exchangeClientCredentials({ + tokenUrl, + clientId: "cid", + clientSecret: "secret", + scopes: ["https://api.fabric.microsoft.com/.default"], + resource: RESOURCE, + authorizationServerMetadata, + }); + + it.effect("sends resource when no authorization-server metadata is known", () => + withTokenEndpoint(tokenResponse(validRefreshBody), ({ tokenUrl, calls }) => + Effect.gen(function* () { + yield* exchange(tokenUrl); + expect((yield* calls)[0]!.body.get("resource")).toBe(RESOURCE); + }), + ), + ); + + it.effect("sends resource when the server advertises support", () => + withTokenEndpoint(tokenResponse(validRefreshBody), ({ tokenUrl, calls }) => + Effect.gen(function* () { + yield* exchange(tokenUrl, AS_SUPPORTED); + expect((yield* calls)[0]!.body.get("resource")).toBe(RESOURCE); + }), + ), + ); + + it.effect("omits resource when the server publishes metadata without the capability", () => + withTokenEndpoint(tokenResponse(validRefreshBody), ({ tokenUrl, calls }) => + Effect.gen(function* () { + yield* exchange(tokenUrl, AS_SILENT); + yield* exchange(tokenUrl, AS_UNSUPPORTED); + const bodies = (yield* calls).map((call) => call.body); + expect(bodies).toHaveLength(2); + for (const body of bodies) { + expect(body.has("resource")).toBe(false); + expect(body.get("grant_type")).toBe("client_credentials"); + expect(body.get("scope")).toBe("https://api.fabric.microsoft.com/.default"); + } + }), + ), + ); +}); + +describe("refreshAccessToken resource-indicator gating", () => { + const refresh = ( + tokenUrl: string, + authorizationServerMetadata?: ResourceIndicatorSupport | null, + ) => + refreshAccessToken({ + tokenUrl, + clientId: "cid", + refreshToken: "old", + resource: RESOURCE, + authorizationServerMetadata, + }); + + it.effect("sends resource when no authorization-server metadata is known", () => + withTokenEndpoint(tokenResponse(validRefreshBody), ({ tokenUrl, calls }) => + Effect.gen(function* () { + yield* refresh(tokenUrl); + expect((yield* calls)[0]!.body.get("resource")).toBe(RESOURCE); + }), + ), + ); + + it.effect("sends resource when the server advertises support", () => + withTokenEndpoint(tokenResponse(validRefreshBody), ({ tokenUrl, calls }) => + Effect.gen(function* () { + yield* refresh(tokenUrl, AS_SUPPORTED); + expect((yield* calls)[0]!.body.get("resource")).toBe(RESOURCE); + }), + ), + ); + + // Refresh builds its extra params conditionally and passes `undefined` when + // the set is empty, so the veto must leave a well-formed refresh request + // rather than an empty `additionalParameters` bag. + it.effect("omits resource when the server publishes metadata without the capability", () => + withTokenEndpoint(tokenResponse(validRefreshBody), ({ tokenUrl, calls }) => + Effect.gen(function* () { + yield* refresh(tokenUrl, AS_SILENT); + yield* refresh(tokenUrl, AS_UNSUPPORTED); + const bodies = (yield* calls).map((call) => call.body); + expect(bodies).toHaveLength(2); + for (const body of bodies) { + expect(body.has("resource")).toBe(false); + expect(body.get("grant_type")).toBe("refresh_token"); + expect(body.get("refresh_token")).toBe("old"); + } + }), + ), + ); +}); + describe("shouldRefreshToken", () => { it("never refreshes when expiresAt is null", () => { expect(shouldRefreshToken({ expiresAt: null })).toBe(false); diff --git a/packages/core/sdk/src/oauth-helpers.ts b/packages/core/sdk/src/oauth-helpers.ts index 53a4ce688..d4ad38ba6 100644 --- a/packages/core/sdk/src/oauth-helpers.ts +++ b/packages/core/sdk/src/oauth-helpers.ts @@ -181,6 +181,57 @@ export const createPkceCodeChallenge = (verifier: string): Promise => * and redeemed by `oauth.complete`. */ export const createOAuthState = (): string => oauth.generateRandomState(); +// --------------------------------------------------------------------------- +// RFC 8707 resource indicators — when `resource` may be sent +// --------------------------------------------------------------------------- + +/** The authorization server's discovered RFC 8414 metadata, narrowed to the one + * field the resource-indicator decision reads. + * + * `resource_indicators_supported` is NOT in the IANA "OAuth Authorization + * Server Metadata" registry — RFC 8707 registered no discovery parameter at + * all. It is the de-facto flag servers that do implement resource indicators + * advertise, so it is the only machine-readable signal available. */ +export type ResourceIndicatorSupport = { + readonly resource_indicators_supported?: boolean; +}; + +/** Whether the RFC 8707 `resource` parameter may be sent to this authorization + * server. THE RULE, in full: + * + * - No metadata was discovered (`undefined` / `null`) → SEND. Nothing is known + * about the server, and MCP Authorization 2025-06-18 tells clients to send + * resource indicators. This is the unchanged, pre-existing behavior and it + * covers every manually configured provider. + * - Metadata was discovered and `resource_indicators_supported` is `true` + * → SEND. The server says it implements RFC 8707. + * - Metadata was discovered and the flag is absent or `false` → OMIT. Per + * RFC 8414 §2 an omitted metadata field means the capability is not + * advertised, and RFC 8707 §2 makes `resource` OPTIONAL for clients, so + * omitting it is conformant. Sending it anyway is what breaks Microsoft + * Entra v2, which rejects `resource` alongside a v2 `scope` with + * AADSTS9010010. + * + * Deliberately NOT a per-provider special case: the decision reads only what + * the server advertises about itself. */ +export const shouldSendResourceIndicator = ( + authorizationServerMetadata?: ResourceIndicatorSupport | null, +): boolean => + authorizationServerMetadata == null + ? true + : authorizationServerMetadata.resource_indicators_supported === true; + +/** The `resource` value to actually put on the wire — `undefined` when the + * caller has none, or when the authorization server does not advertise + * RFC 8707 support. */ +const resourceParamFor = (input: { + readonly resource?: string | null; + readonly authorizationServerMetadata?: ResourceIndicatorSupport | null; +}): string | undefined => + input.resource && shouldSendResourceIndicator(input.authorizationServerMetadata) + ? input.resource + : undefined; + // --------------------------------------------------------------------------- // Authorization URL builder // --------------------------------------------------------------------------- @@ -196,9 +247,12 @@ export type BuildAuthorizationUrlInput = { /** Separator between scopes. RFC 6749 says space; some providers use comma. */ readonly scopeSeparator?: string; /** RFC 8707 Resource Indicator. MCP Authorization 2025-06-18 §"Resource - * Parameter Implementation" requires clients to send this on every - * authorization request, regardless of AS support. */ + * Parameter Implementation" asks clients to send this on every authorization + * request; `authorizationServerMetadata` can veto it. */ readonly resource?: string; + /** The authorization server's discovered metadata. Gates `resource` — see + * `shouldSendResourceIndicator`. Omit when nothing was discovered. */ + readonly authorizationServerMetadata?: ResourceIndicatorSupport | null; /** Provider-specific extras (e.g. Google's `access_type=offline`). */ readonly extraParams?: Readonly>; readonly endpointUrlPolicy?: OAuthEndpointUrlPolicy; @@ -227,8 +281,9 @@ export const buildAuthorizationUrl = (input: BuildAuthorizationUrlInput): string url.searchParams.set("state", input.state); url.searchParams.set("code_challenge_method", "S256"); url.searchParams.set("code_challenge", input.codeChallenge); - if (input.resource) { - url.searchParams.set("resource", input.resource); + const resource = resourceParamFor(input); + if (resource) { + url.searchParams.set("resource", resource); } if (input.extraParams) { for (const [k, v] of Object.entries(input.extraParams)) { @@ -1070,10 +1125,13 @@ export type ExchangeAuthorizationCodeInput = { readonly code: string; readonly clientAuth?: ClientAuthMethod; readonly idTokenSigningAlgValuesSupported?: readonly string[]; - /** RFC 8707 Resource Indicator. MCP Auth spec MUST-requires this on - * the token request when the client knows the resource it intends - * to call. */ + /** RFC 8707 Resource Indicator. The MCP Auth spec asks for this on the token + * request when the client knows the resource it intends to call; + * `authorizationServerMetadata` can veto it. */ readonly resource?: string; + /** The authorization server's discovered metadata. Gates `resource` — see + * `shouldSendResourceIndicator`. Omit when nothing was discovered. */ + readonly authorizationServerMetadata?: ResourceIndicatorSupport | null; readonly timeoutMs?: number; readonly endpointUrlPolicy?: OAuthEndpointUrlPolicy; readonly fetch?: typeof globalThis.fetch; @@ -1103,8 +1161,9 @@ export const exchangeAuthorizationCode = ( redirect_uri: input.redirectUrl, code_verifier: input.codeVerifier, }); - if (input.resource) { - params.set("resource", input.resource); + const resource = resourceParamFor(input); + if (resource) { + params.set("resource", resource); } const response = await oauth.genericTokenEndpointRequest( as, @@ -1128,7 +1187,7 @@ export const exchangeAuthorizationCode = ( grantType: "authorization_code", tokenUrl: input.tokenUrl, clientAuth: input.clientAuth, - hasResource: input.resource !== undefined, + hasResource: resourceParamFor(input) !== undefined, }), ); @@ -1143,9 +1202,13 @@ export type ExchangeClientCredentialsInput = { readonly scopes?: readonly string[]; readonly scopeSeparator?: string; readonly clientAuth?: ClientAuthMethod; - /** RFC 8707 Resource Indicator. MCP Authorization 2025-06-18 requires this - * on token requests when the client knows the protected resource. */ + /** RFC 8707 Resource Indicator. MCP Authorization 2025-06-18 asks for this on + * token requests when the client knows the protected resource; + * `authorizationServerMetadata` can veto it. */ readonly resource?: string; + /** The authorization server's discovered metadata. Gates `resource` — see + * `shouldSendResourceIndicator`. Omit when nothing was discovered. */ + readonly authorizationServerMetadata?: ResourceIndicatorSupport | null; readonly timeoutMs?: number; readonly endpointUrlPolicy?: OAuthEndpointUrlPolicy; readonly fetch?: typeof globalThis.fetch; @@ -1166,8 +1229,9 @@ export const exchangeClientCredentials = ( if (input.scopes && input.scopes.length > 0) { params.set("scope", input.scopes.join(input.scopeSeparator ?? " ")); } - if (input.resource) { - params.set("resource", input.resource); + const resource = resourceParamFor(input); + if (resource) { + params.set("resource", resource); } const response = await oauth.clientCredentialsGrantRequest( as, @@ -1191,7 +1255,7 @@ export const exchangeClientCredentials = ( grantType: "client_credentials", tokenUrl: input.tokenUrl, clientAuth: input.clientAuth, - hasResource: input.resource !== undefined, + hasResource: resourceParamFor(input) !== undefined, }), ); @@ -1209,10 +1273,13 @@ export type RefreshAccessTokenInput = { readonly scopeSeparator?: string; readonly clientAuth?: ClientAuthMethod; readonly idTokenSigningAlgValuesSupported?: readonly string[]; - /** RFC 8707 Resource Indicator — MCP spec MUST-requires this on - * refresh requests so the new access token's audience is bound to - * the same resource. */ + /** RFC 8707 Resource Indicator — the MCP spec asks for this on refresh + * requests so the new access token's audience stays bound to the same + * resource; `authorizationServerMetadata` can veto it. */ readonly resource?: string; + /** The authorization server's discovered metadata. Gates `resource` — see + * `shouldSendResourceIndicator`. Omit when nothing was discovered. */ + readonly authorizationServerMetadata?: ResourceIndicatorSupport | null; readonly timeoutMs?: number; readonly endpointUrlPolicy?: OAuthEndpointUrlPolicy; readonly fetch?: typeof globalThis.fetch; @@ -1236,8 +1303,9 @@ export const refreshAccessToken = ( if (input.scopes && input.scopes.length > 0) { extraParams.set("scope", input.scopes.join(input.scopeSeparator ?? " ")); } - if (input.resource) { - extraParams.set("resource", input.resource); + const resource = resourceParamFor(input); + if (resource) { + extraParams.set("resource", resource); } const additionalParameters = Array.from(extraParams.keys()).length > 0 ? extraParams : undefined; @@ -1270,7 +1338,7 @@ export const refreshAccessToken = ( grantType: "refresh_token", tokenUrl: input.tokenUrl, clientAuth: input.clientAuth, - hasResource: input.resource !== undefined, + hasResource: resourceParamFor(input) !== undefined, }), ); From 32c702bd951292264fe3db9c5a5c239b65d9558c Mon Sep 17 00:00:00 2001 From: Rhys Sullivan <39114868+RhysSullivan@users.noreply.github.com> Date: Fri, 28 Aug 2026 11:53:20 -0700 Subject: [PATCH 2/3] Preserve an explicitly absent OAuth resource instead of gating on discovery metadata --- .changeset/oauth-resource-explicit-absence.md | 17 ++ ...auth-resource-indicator-capability-gate.md | 19 -- packages/core/sdk/src/executor.ts | 10 +- packages/core/sdk/src/oauth-discovery.test.ts | 59 ----- packages/core/sdk/src/oauth-discovery.ts | 12 - packages/core/sdk/src/oauth-flow.test.ts | 157 ++++++++++++ packages/core/sdk/src/oauth-helpers.test.ts | 230 ------------------ packages/core/sdk/src/oauth-helpers.ts | 110 ++------- .../core/sdk/src/oauth-scope-union.test.ts | 44 ++-- packages/core/sdk/src/oauth-service.ts | 28 ++- .../src/components/oauth-client-form.tsx | 33 ++- 11 files changed, 282 insertions(+), 437 deletions(-) create mode 100644 .changeset/oauth-resource-explicit-absence.md delete mode 100644 .changeset/oauth-resource-indicator-capability-gate.md diff --git a/.changeset/oauth-resource-explicit-absence.md b/.changeset/oauth-resource-explicit-absence.md new file mode 100644 index 000000000..8d556a88c --- /dev/null +++ b/.changeset/oauth-resource-explicit-absence.md @@ -0,0 +1,17 @@ +--- +"executor": patch +"@executor-js/react": patch +--- + +**An OAuth app can now be registered without an RFC 8707 resource, and that absence holds on every request** + +Microsoft Entra v2 rejects any authorization request that carries both a v2 `scope` (such as `https://api.fabric.microsoft.com/.default`) and the RFC 8707 `resource` parameter, failing with `AADSTS9010010` before the consent screen. Executor made that unavoidable for MCP servers behind Entra: registering an app for an MCP integration always derived the MCP endpoint as the resource, the form had no field to change it, and so every request carried the parameter Entra rejects. + +The register/edit OAuth app form now shows the resource indicator. It is still prefilled for MCP servers — nothing changes for providers that accept the parameter — but it can be cleared, and a cleared value persists as "no resource". A resource-less app then omits `resource` on all four grants alike: the authorization request, the code exchange, token refresh, and client-credentials. Symmetry matters here — sending `resource` on authorize but not on the token request (or the reverse) would bind the two tokens to different audiences. + +Two adjacent gaps closed with it: + +- MCP scope discovery no longer depends on the app's resource. It now falls back to the integration's own discovery URL (the MCP endpoint), so clearing the resource does not break connecting. +- Token refresh for a first-party OAuth app dropped the app's configured resource, refreshing to a different audience than the original grant. It now sends the same resource the authorization request sent. + +Apps that keep their resource — the default for every discovered MCP server — behave exactly as before: the parameter is sent on every grant, as the MCP authorization spec expects. diff --git a/.changeset/oauth-resource-indicator-capability-gate.md b/.changeset/oauth-resource-indicator-capability-gate.md deleted file mode 100644 index 169038057..000000000 --- a/.changeset/oauth-resource-indicator-capability-gate.md +++ /dev/null @@ -1,19 +0,0 @@ ---- -"executor": patch ---- - -**The RFC 8707 `resource` parameter is now gated on what the authorization server advertises** - -Executor sent `resource` unconditionally on every authorization, code exchange, refresh, and client-credentials request whenever an OAuth app had a resource configured. Microsoft Entra v2 rejects that: a request carrying both `resource` and a v2 `scope` such as `https://api.fabric.microsoft.com/.default` fails with `AADSTS9010010` before the consent screen, so the Microsoft Fabric Core MCP server could not be connected at all. - -The rule now applied on all four grants, in full: - -- **No authorization-server metadata was discovered → send `resource`.** Nothing is known about the server, and the MCP authorization spec expects resource indicators. This is the previous behavior, unchanged, and it covers every manually configured provider. -- **Metadata was discovered and advertises `resource_indicators_supported: true` → send `resource`.** -- **Metadata was discovered and the flag is absent or `false` → omit `resource`.** RFC 8414 §2 makes an omitted metadata field mean "not advertised", and RFC 8707 §2 makes `resource` optional for clients, so omitting it is conformant. - -The decision reads only what a server publishes about itself — there is no Microsoft special case and no host allowlist. `resource_indicators_supported` is not in the IANA authorization-server metadata registry (RFC 8707 registered no discovery parameter), but it is the only machine-readable signal a server gives, so discovery now parses it and threads it to the grant helpers. - -The protected resource is still discovered, validated against the requested endpoint, and retained in the flow state for MCP binding when the parameter itself is withheld. The `executor.oauth.has_resource` span attribute now reports what actually went on the wire rather than what was configured. - -Providers whose authorization server advertises the capability, and providers reached without any metadata discovery, are unaffected. diff --git a/packages/core/sdk/src/executor.ts b/packages/core/sdk/src/executor.ts index 7c3c25929..0fb526a3e 100644 --- a/packages/core/sdk/src/executor.ts +++ b/packages/core/sdk/src/executor.ts @@ -2262,7 +2262,11 @@ export const createExecutor = { scopes_supported: ["openid", "profile", "email", "offline_access", "workspace:member"], response_types_supported: ["code"], code_challenge_methods_supported: ["S256"], - // An MCP-profile server implements RFC 8707, so it advertises it — - // that is what earns the `resource` parameter asserted below. - resource_indicators_supported: true, }); } if (request.url === "/oauth/register") { @@ -627,7 +624,6 @@ describe("beginDynamicAuthorization", () => { registration_endpoint: `${baseUrl}/register`, response_types_supported: ["code"], code_challenge_methods_supported: ["S256"], - resource_indicators_supported: true, }); } if (request.url === "/register") { @@ -656,61 +652,6 @@ describe("beginDynamicAuthorization", () => { ), ); - // #1789 — Microsoft Entra v2 publishes authorization-server metadata, does - // NOT advertise RFC 8707, and rejects `resource` alongside a v2 `scope` with - // AADSTS9010010. Metadata that stays silent on the capability means "not - // advertised" (RFC 8414 §2), so the resource indicator is withheld — while - // the protected resource itself is still discovered and kept in the state - // for MCP binding. - it.effect("omits the resource parameter when the AS metadata does not advertise RFC 8707", () => - withOAuthFixture( - (request, baseUrl) => { - if (request.url === "/.well-known/oauth-protected-resource/v1/mcp/core") { - return sendJson({ - resource: baseUrl, - authorization_servers: [baseUrl], - scopes_supported: [`${baseUrl}/.default`], - }); - } - if (request.url === "/.well-known/oauth-authorization-server") { - return sendJson({ - issuer: baseUrl, - authorization_endpoint: `${baseUrl}/oauth2/v2.0/authorize`, - token_endpoint: `${baseUrl}/oauth2/v2.0/token`, - registration_endpoint: `${baseUrl}/register`, - response_types_supported: ["code"], - code_challenge_methods_supported: ["S256"], - }); - } - if (request.url === "/register") { - return sendJson( - { - client_id: "entra-client", - redirect_uris: ["https://app/cb"], - token_endpoint_auth_method: "none", - }, - 201, - ); - } - return notFound(); - }, - ({ baseUrl }) => - Effect.gen(function* () { - const result = yield* beginDynamicAuthorization({ - endpoint: `${baseUrl}/v1/mcp/core`, - redirectUrl: "https://app/cb", - state: "s", - }); - - const authUrl = new URL(result.authorizationUrl); - expect(authUrl.searchParams.has("resource")).toBe(false); - expect(authUrl.searchParams.get("scope")).toBe(`${baseUrl}/.default`); - // Withheld from the wire, still retained for MCP discovery/binding. - expect(result.state.resource).toBe(baseUrl); - }), - ), - ); - it.effect("includes client_uri in the DCR body", () => withOAuthFixture( (request, baseUrl) => { diff --git a/packages/core/sdk/src/oauth-discovery.ts b/packages/core/sdk/src/oauth-discovery.ts index 901e954c5..bb30fec79 100644 --- a/packages/core/sdk/src/oauth-discovery.ts +++ b/packages/core/sdk/src/oauth-discovery.ts @@ -82,14 +82,6 @@ export const OAuthAuthorizationServerMetadataSchema = Schema.Struct({ introspection_endpoint: Schema.optional(Schema.String), userinfo_endpoint: Schema.optional(Schema.String), id_token_signing_alg_values_supported: Schema.optional(StringArray), - /** Whether this server implements RFC 8707 resource indicators. RFC 8707 - * registered no metadata parameter, so this is a de-facto field rather than - * an IANA-registered one — but it is the only machine-readable signal a - * server gives, and `shouldSendResourceIndicator` reads it to decide whether - * `resource` goes on authorization and token requests. An absent field means - * "not advertised" (RFC 8414 §2), NOT "unknown": a server that publishes - * metadata and stays silent here gets no `resource`. */ - resource_indicators_supported: Schema.optional(Schema.Boolean), /** draft-ietf-oauth-identity-assertion-authz-grant-04 §7.2 — the * authorization grant profiles this Resource Authorization Server * implements. Advertising a profile says only that the server implements @@ -906,10 +898,6 @@ export const beginDynamicAuthorization = ( state: input.state, codeChallenge, resource: resourceValue, - // The AS metadata was just discovered, so its RFC 8707 capability is - // known: send `resource` only if it advertises support. Entra v2 rejects - // `resource` alongside a v2 `scope` (AADSTS9010010). - authorizationServerMetadata: authServer.metadata, endpointUrlPolicy: options.endpointUrlPolicy, }); diff --git a/packages/core/sdk/src/oauth-flow.test.ts b/packages/core/sdk/src/oauth-flow.test.ts index e7a8e6d65..52176d7b9 100644 --- a/packages/core/sdk/src/oauth-flow.test.ts +++ b/packages/core/sdk/src/oauth-flow.test.ts @@ -2005,3 +2005,160 @@ describe("reactive OAuth refresh on upstream 401", () => { ), ); }); + +// --------------------------------------------------------------------------- +// RFC 8707 resource omission for a resource-less client (#1789) +// +// A client persisted with NO resource sends no `resource` parameter on ANY +// request — authorize, code exchange, refresh, client-credentials. Microsoft +// Entra v2 rejects requests that carry `resource` next to a v2 `scope` +// (AADSTS9010010), and the way out is a client whose resource is absent; that +// absence must hold on every grant, or the token audience diverges between +// authorize and token. The mirror-image assertions — a client WITH a resource +// sends it on authorize + exchange + refresh — live in the tests above. +// --------------------------------------------------------------------------- +describe("resource-less client sends no resource parameter (#1789)", () => { + it.effect("authorize, code exchange, and refresh all omit `resource`", () => + Effect.scoped( + Effect.gen(function* () { + const server = yield* serveOAuthTestServer({ scopes: ["read"] }); + const { executor, config } = yield* makeTestWorkspaceHarness({ plugins }); + yield* executor.acme.seed(); + + // `resource: null` — explicitly none, not merely unset. + yield* executor.oauth.createClient({ + owner: "org", + slug: CLIENT, + authorizationUrl: server.authorizationEndpoint, + tokenUrl: server.tokenEndpoint, + grant: "authorization_code", + clientId: "test-client", + clientSecret: "test-secret", + resource: null, + }); + + const started = yield* executor.oauth.start({ + owner: "org", + client: CLIENT, + clientOwner: "org", + name: ConnectionName.make("main"), + integration: INTEG, + template: TEMPLATE, + }); + expect(started.status).toBe("redirect"); + if (started.status !== "redirect") return; + expect(new URL(started.authorizationUrl).searchParams.has("resource")).toBe(false); + + const callback = yield* server.completeAuthorizationCodeFlow({ + authorizationUrl: started.authorizationUrl, + }); + yield* executor.oauth.complete({ state: started.state, code: callback.code }); + + // Force expiry so the next execute refreshes. + yield* Effect.promise(() => + config.db.updateMany("connection", { + where: (b) => b("name", "=", "main"), + set: { expires_at: Date.now() - 60_000 }, + }), + ); + const refreshed = (yield* executor.execute( + ToolAddress.make("tools.acme.org.main.whoami"), + {}, + )) as { token: string }; + expect(refreshed.token).toMatch(/^at_/); + + // What the authorization server actually SAW: the authorize request, + // the code exchange, and the refresh each carried no `resource`. + const requests = yield* server.requests; + const authorize = requests.find((r) => r.path === "/authorize" && r.method === "GET"); + expect(authorize).toBeDefined(); + expect(authorize?.query.resource ?? null).toBeNull(); + const exchange = requests.find( + (r) => r.path === "/token" && r.body.includes("grant_type=authorization_code"), + ); + expect(exchange).toBeDefined(); + expect(exchange?.body ?? "").not.toContain("resource="); + const refresh = requests.find( + (r) => r.path === "/token" && r.body.includes("grant_type=refresh_token"), + ); + expect(refresh).toBeDefined(); + expect(refresh?.body ?? "").not.toContain("resource="); + }), + ), + ); + + it.effect("client_credentials omits `resource` for a resource-less client", () => + Effect.scoped( + Effect.gen(function* () { + const server = yield* serveOAuthTestServer({ scopes: ["read"] }); + const { executor } = yield* makeTestWorkspaceHarness({ plugins }); + yield* executor.acme.seed(); + + yield* executor.oauth.createClient({ + owner: "org", + slug: CLIENT, + authorizationUrl: server.authorizationEndpoint, + tokenUrl: server.tokenEndpoint, + grant: "client_credentials", + clientId: "test-client", + clientSecret: "test-secret", + resource: null, + }); + + const started = yield* executor.oauth.start({ + owner: "org", + client: CLIENT, + clientOwner: "org", + name: ConnectionName.make("cc"), + integration: INTEG, + template: TEMPLATE, + }); + expect(started.status).toBe("connected"); + + const requests = yield* server.requests; + const grant = requests.find( + (r) => r.path === "/token" && r.body.includes("grant_type=client_credentials"), + ); + expect(grant).toBeDefined(); + expect(grant?.body ?? "").not.toContain("resource="); + }), + ), + ); + + it.effect("client_credentials sends `resource` when the client has one", () => + Effect.scoped( + Effect.gen(function* () { + const server = yield* serveOAuthTestServer({ scopes: ["read"] }); + const { executor } = yield* makeTestWorkspaceHarness({ plugins }); + yield* executor.acme.seed(); + + yield* executor.oauth.createClient({ + owner: "org", + slug: CLIENT, + authorizationUrl: server.authorizationEndpoint, + tokenUrl: server.tokenEndpoint, + grant: "client_credentials", + clientId: "test-client", + clientSecret: "test-secret", + resource: server.mcpResourceUrl, + }); + + const started = yield* executor.oauth.start({ + owner: "org", + client: CLIENT, + clientOwner: "org", + name: ConnectionName.make("cc"), + integration: INTEG, + template: TEMPLATE, + }); + expect(started.status).toBe("connected"); + + const requests = yield* server.requests; + const grant = requests.find( + (r) => r.path === "/token" && r.body.includes("grant_type=client_credentials"), + ); + expect(grant?.body).toContain(`resource=${encodeURIComponent(server.mcpResourceUrl)}`); + }), + ), + ); +}); diff --git a/packages/core/sdk/src/oauth-helpers.test.ts b/packages/core/sdk/src/oauth-helpers.test.ts index 12407f70c..ab5dcb903 100644 --- a/packages/core/sdk/src/oauth-helpers.test.ts +++ b/packages/core/sdk/src/oauth-helpers.test.ts @@ -26,10 +26,7 @@ import { isUnusableSuccessTokenResponse, refreshAccessToken, shouldRefreshToken, - shouldSendResourceIndicator, - type ResourceIndicatorSupport, } from "./oauth-helpers"; -import type { OAuthAuthorizationServerMetadata } from "./oauth-discovery"; import { serveTestHttpApp } from "./testing"; interface TokenCall { @@ -1426,233 +1423,6 @@ describe("refreshAccessToken", () => { ); }); -// --------------------------------------------------------------------------- -// RFC 8707 resource indicators — the AS capability gate (#1789) -// -// THE RULE under test, on every grant that can carry `resource`: -// no metadata → send (unchanged; MCP's default expectation) -// metadata + flag true → send -// metadata + flag absent → omit (RFC 8414 §2: not advertised) -// metadata + flag false → omit -// -// The "metadata + flag absent" row is the bug: Microsoft Entra v2 publishes -// metadata, does not advertise resource indicators, and rejects `resource` -// alongside a v2 `scope` with AADSTS9010010. -// --------------------------------------------------------------------------- - -const RESOURCE = "https://api.example.com/v1/mcp"; -const AS_SUPPORTED: ResourceIndicatorSupport = { resource_indicators_supported: true }; -const AS_UNSUPPORTED: ResourceIndicatorSupport = { resource_indicators_supported: false }; -/** Metadata that exists but never mentions resource indicators — the Entra - * shape. Typed as the whole discovered document, not just the one flag, so - * this fixture stays honest about what a real caller threads through. */ -const AS_SILENT: OAuthAuthorizationServerMetadata = { - issuer: "https://login.microsoftonline.com/tenant/v2.0", - authorization_endpoint: "https://login.microsoftonline.com/tenant/oauth2/v2.0/authorize", - token_endpoint: "https://login.microsoftonline.com/tenant/oauth2/v2.0/token", -}; - -describe("shouldSendResourceIndicator", () => { - it("sends when no metadata was discovered", () => { - expect(shouldSendResourceIndicator()).toBe(true); - expect(shouldSendResourceIndicator(undefined)).toBe(true); - expect(shouldSendResourceIndicator(null)).toBe(true); - }); - - it("sends only when the server advertises support", () => { - expect(shouldSendResourceIndicator(AS_SUPPORTED)).toBe(true); - expect(shouldSendResourceIndicator(AS_UNSUPPORTED)).toBe(false); - expect(shouldSendResourceIndicator(AS_SILENT)).toBe(false); - expect(shouldSendResourceIndicator({})).toBe(false); - }); -}); - -describe("buildAuthorizationUrl resource-indicator gating", () => { - const baseInput = { - authorizationUrl: "https://example.com/authorize", - clientId: "client-123", - redirectUrl: "https://app.example.com/callback", - scopes: ["https://api.fabric.microsoft.com/.default"] as const, - state: "state-abc", - codeChallenge: "E9Melhoa2OwvFrEMTJguCHaoeK1t8URWbuGJSstw-cM", - resource: RESOURCE, - }; - - it("sends resource when no authorization-server metadata is known", () => { - const url = new URL(buildAuthorizationUrl(baseInput)); - expect(url.searchParams.get("resource")).toBe(RESOURCE); - }); - - it("sends resource when the server advertises resource_indicators_supported", () => { - const url = new URL( - buildAuthorizationUrl({ ...baseInput, authorizationServerMetadata: AS_SUPPORTED }), - ); - expect(url.searchParams.get("resource")).toBe(RESOURCE); - }); - - it("omits resource when the server publishes metadata without the capability", () => { - for (const metadata of [AS_UNSUPPORTED, AS_SILENT]) { - const url = new URL( - buildAuthorizationUrl({ ...baseInput, authorizationServerMetadata: metadata }), - ); - expect(url.searchParams.has("resource")).toBe(false); - // The scope the AS DOES accept must survive the veto untouched. - expect(url.searchParams.get("scope")).toBe("https://api.fabric.microsoft.com/.default"); - } - }); -}); - -describe("exchangeAuthorizationCode resource-indicator gating", () => { - const exchange = ( - tokenUrl: string, - authorizationServerMetadata?: ResourceIndicatorSupport | null, - ) => - exchangeAuthorizationCode({ - tokenUrl, - clientId: "cid", - redirectUrl: "https://app.example.com/cb", - codeVerifier: "verifier", - code: "abc", - resource: RESOURCE, - authorizationServerMetadata, - }); - - it.effect("sends resource when no authorization-server metadata is known", () => - withTokenEndpoint(tokenResponse(validCodeBody), ({ tokenUrl, calls }) => - Effect.gen(function* () { - yield* exchange(tokenUrl); - expect((yield* calls)[0]!.body.get("resource")).toBe(RESOURCE); - }), - ), - ); - - it.effect("sends resource when the server advertises support", () => - withTokenEndpoint(tokenResponse(validCodeBody), ({ tokenUrl, calls }) => - Effect.gen(function* () { - yield* exchange(tokenUrl, AS_SUPPORTED); - expect((yield* calls)[0]!.body.get("resource")).toBe(RESOURCE); - }), - ), - ); - - it.effect("omits resource when the server publishes metadata without the capability", () => - withTokenEndpoint(tokenResponse(validCodeBody), ({ tokenUrl, calls }) => - Effect.gen(function* () { - yield* exchange(tokenUrl, AS_SILENT); - yield* exchange(tokenUrl, AS_UNSUPPORTED); - const bodies = (yield* calls).map((call) => call.body); - expect(bodies).toHaveLength(2); - for (const body of bodies) { - expect(body.has("resource")).toBe(false); - // The grant itself is untouched — only `resource` is withheld. - expect(body.get("grant_type")).toBe("authorization_code"); - expect(body.get("code_verifier")).toBe("verifier"); - } - }), - ), - ); -}); - -describe("exchangeClientCredentials resource-indicator gating", () => { - const exchange = ( - tokenUrl: string, - authorizationServerMetadata?: ResourceIndicatorSupport | null, - ) => - exchangeClientCredentials({ - tokenUrl, - clientId: "cid", - clientSecret: "secret", - scopes: ["https://api.fabric.microsoft.com/.default"], - resource: RESOURCE, - authorizationServerMetadata, - }); - - it.effect("sends resource when no authorization-server metadata is known", () => - withTokenEndpoint(tokenResponse(validRefreshBody), ({ tokenUrl, calls }) => - Effect.gen(function* () { - yield* exchange(tokenUrl); - expect((yield* calls)[0]!.body.get("resource")).toBe(RESOURCE); - }), - ), - ); - - it.effect("sends resource when the server advertises support", () => - withTokenEndpoint(tokenResponse(validRefreshBody), ({ tokenUrl, calls }) => - Effect.gen(function* () { - yield* exchange(tokenUrl, AS_SUPPORTED); - expect((yield* calls)[0]!.body.get("resource")).toBe(RESOURCE); - }), - ), - ); - - it.effect("omits resource when the server publishes metadata without the capability", () => - withTokenEndpoint(tokenResponse(validRefreshBody), ({ tokenUrl, calls }) => - Effect.gen(function* () { - yield* exchange(tokenUrl, AS_SILENT); - yield* exchange(tokenUrl, AS_UNSUPPORTED); - const bodies = (yield* calls).map((call) => call.body); - expect(bodies).toHaveLength(2); - for (const body of bodies) { - expect(body.has("resource")).toBe(false); - expect(body.get("grant_type")).toBe("client_credentials"); - expect(body.get("scope")).toBe("https://api.fabric.microsoft.com/.default"); - } - }), - ), - ); -}); - -describe("refreshAccessToken resource-indicator gating", () => { - const refresh = ( - tokenUrl: string, - authorizationServerMetadata?: ResourceIndicatorSupport | null, - ) => - refreshAccessToken({ - tokenUrl, - clientId: "cid", - refreshToken: "old", - resource: RESOURCE, - authorizationServerMetadata, - }); - - it.effect("sends resource when no authorization-server metadata is known", () => - withTokenEndpoint(tokenResponse(validRefreshBody), ({ tokenUrl, calls }) => - Effect.gen(function* () { - yield* refresh(tokenUrl); - expect((yield* calls)[0]!.body.get("resource")).toBe(RESOURCE); - }), - ), - ); - - it.effect("sends resource when the server advertises support", () => - withTokenEndpoint(tokenResponse(validRefreshBody), ({ tokenUrl, calls }) => - Effect.gen(function* () { - yield* refresh(tokenUrl, AS_SUPPORTED); - expect((yield* calls)[0]!.body.get("resource")).toBe(RESOURCE); - }), - ), - ); - - // Refresh builds its extra params conditionally and passes `undefined` when - // the set is empty, so the veto must leave a well-formed refresh request - // rather than an empty `additionalParameters` bag. - it.effect("omits resource when the server publishes metadata without the capability", () => - withTokenEndpoint(tokenResponse(validRefreshBody), ({ tokenUrl, calls }) => - Effect.gen(function* () { - yield* refresh(tokenUrl, AS_SILENT); - yield* refresh(tokenUrl, AS_UNSUPPORTED); - const bodies = (yield* calls).map((call) => call.body); - expect(bodies).toHaveLength(2); - for (const body of bodies) { - expect(body.has("resource")).toBe(false); - expect(body.get("grant_type")).toBe("refresh_token"); - expect(body.get("refresh_token")).toBe("old"); - } - }), - ), - ); -}); - describe("shouldRefreshToken", () => { it("never refreshes when expiresAt is null", () => { expect(shouldRefreshToken({ expiresAt: null })).toBe(false); diff --git a/packages/core/sdk/src/oauth-helpers.ts b/packages/core/sdk/src/oauth-helpers.ts index d4ad38ba6..53a4ce688 100644 --- a/packages/core/sdk/src/oauth-helpers.ts +++ b/packages/core/sdk/src/oauth-helpers.ts @@ -181,57 +181,6 @@ export const createPkceCodeChallenge = (verifier: string): Promise => * and redeemed by `oauth.complete`. */ export const createOAuthState = (): string => oauth.generateRandomState(); -// --------------------------------------------------------------------------- -// RFC 8707 resource indicators — when `resource` may be sent -// --------------------------------------------------------------------------- - -/** The authorization server's discovered RFC 8414 metadata, narrowed to the one - * field the resource-indicator decision reads. - * - * `resource_indicators_supported` is NOT in the IANA "OAuth Authorization - * Server Metadata" registry — RFC 8707 registered no discovery parameter at - * all. It is the de-facto flag servers that do implement resource indicators - * advertise, so it is the only machine-readable signal available. */ -export type ResourceIndicatorSupport = { - readonly resource_indicators_supported?: boolean; -}; - -/** Whether the RFC 8707 `resource` parameter may be sent to this authorization - * server. THE RULE, in full: - * - * - No metadata was discovered (`undefined` / `null`) → SEND. Nothing is known - * about the server, and MCP Authorization 2025-06-18 tells clients to send - * resource indicators. This is the unchanged, pre-existing behavior and it - * covers every manually configured provider. - * - Metadata was discovered and `resource_indicators_supported` is `true` - * → SEND. The server says it implements RFC 8707. - * - Metadata was discovered and the flag is absent or `false` → OMIT. Per - * RFC 8414 §2 an omitted metadata field means the capability is not - * advertised, and RFC 8707 §2 makes `resource` OPTIONAL for clients, so - * omitting it is conformant. Sending it anyway is what breaks Microsoft - * Entra v2, which rejects `resource` alongside a v2 `scope` with - * AADSTS9010010. - * - * Deliberately NOT a per-provider special case: the decision reads only what - * the server advertises about itself. */ -export const shouldSendResourceIndicator = ( - authorizationServerMetadata?: ResourceIndicatorSupport | null, -): boolean => - authorizationServerMetadata == null - ? true - : authorizationServerMetadata.resource_indicators_supported === true; - -/** The `resource` value to actually put on the wire — `undefined` when the - * caller has none, or when the authorization server does not advertise - * RFC 8707 support. */ -const resourceParamFor = (input: { - readonly resource?: string | null; - readonly authorizationServerMetadata?: ResourceIndicatorSupport | null; -}): string | undefined => - input.resource && shouldSendResourceIndicator(input.authorizationServerMetadata) - ? input.resource - : undefined; - // --------------------------------------------------------------------------- // Authorization URL builder // --------------------------------------------------------------------------- @@ -247,12 +196,9 @@ export type BuildAuthorizationUrlInput = { /** Separator between scopes. RFC 6749 says space; some providers use comma. */ readonly scopeSeparator?: string; /** RFC 8707 Resource Indicator. MCP Authorization 2025-06-18 §"Resource - * Parameter Implementation" asks clients to send this on every authorization - * request; `authorizationServerMetadata` can veto it. */ + * Parameter Implementation" requires clients to send this on every + * authorization request, regardless of AS support. */ readonly resource?: string; - /** The authorization server's discovered metadata. Gates `resource` — see - * `shouldSendResourceIndicator`. Omit when nothing was discovered. */ - readonly authorizationServerMetadata?: ResourceIndicatorSupport | null; /** Provider-specific extras (e.g. Google's `access_type=offline`). */ readonly extraParams?: Readonly>; readonly endpointUrlPolicy?: OAuthEndpointUrlPolicy; @@ -281,9 +227,8 @@ export const buildAuthorizationUrl = (input: BuildAuthorizationUrlInput): string url.searchParams.set("state", input.state); url.searchParams.set("code_challenge_method", "S256"); url.searchParams.set("code_challenge", input.codeChallenge); - const resource = resourceParamFor(input); - if (resource) { - url.searchParams.set("resource", resource); + if (input.resource) { + url.searchParams.set("resource", input.resource); } if (input.extraParams) { for (const [k, v] of Object.entries(input.extraParams)) { @@ -1125,13 +1070,10 @@ export type ExchangeAuthorizationCodeInput = { readonly code: string; readonly clientAuth?: ClientAuthMethod; readonly idTokenSigningAlgValuesSupported?: readonly string[]; - /** RFC 8707 Resource Indicator. The MCP Auth spec asks for this on the token - * request when the client knows the resource it intends to call; - * `authorizationServerMetadata` can veto it. */ + /** RFC 8707 Resource Indicator. MCP Auth spec MUST-requires this on + * the token request when the client knows the resource it intends + * to call. */ readonly resource?: string; - /** The authorization server's discovered metadata. Gates `resource` — see - * `shouldSendResourceIndicator`. Omit when nothing was discovered. */ - readonly authorizationServerMetadata?: ResourceIndicatorSupport | null; readonly timeoutMs?: number; readonly endpointUrlPolicy?: OAuthEndpointUrlPolicy; readonly fetch?: typeof globalThis.fetch; @@ -1161,9 +1103,8 @@ export const exchangeAuthorizationCode = ( redirect_uri: input.redirectUrl, code_verifier: input.codeVerifier, }); - const resource = resourceParamFor(input); - if (resource) { - params.set("resource", resource); + if (input.resource) { + params.set("resource", input.resource); } const response = await oauth.genericTokenEndpointRequest( as, @@ -1187,7 +1128,7 @@ export const exchangeAuthorizationCode = ( grantType: "authorization_code", tokenUrl: input.tokenUrl, clientAuth: input.clientAuth, - hasResource: resourceParamFor(input) !== undefined, + hasResource: input.resource !== undefined, }), ); @@ -1202,13 +1143,9 @@ export type ExchangeClientCredentialsInput = { readonly scopes?: readonly string[]; readonly scopeSeparator?: string; readonly clientAuth?: ClientAuthMethod; - /** RFC 8707 Resource Indicator. MCP Authorization 2025-06-18 asks for this on - * token requests when the client knows the protected resource; - * `authorizationServerMetadata` can veto it. */ + /** RFC 8707 Resource Indicator. MCP Authorization 2025-06-18 requires this + * on token requests when the client knows the protected resource. */ readonly resource?: string; - /** The authorization server's discovered metadata. Gates `resource` — see - * `shouldSendResourceIndicator`. Omit when nothing was discovered. */ - readonly authorizationServerMetadata?: ResourceIndicatorSupport | null; readonly timeoutMs?: number; readonly endpointUrlPolicy?: OAuthEndpointUrlPolicy; readonly fetch?: typeof globalThis.fetch; @@ -1229,9 +1166,8 @@ export const exchangeClientCredentials = ( if (input.scopes && input.scopes.length > 0) { params.set("scope", input.scopes.join(input.scopeSeparator ?? " ")); } - const resource = resourceParamFor(input); - if (resource) { - params.set("resource", resource); + if (input.resource) { + params.set("resource", input.resource); } const response = await oauth.clientCredentialsGrantRequest( as, @@ -1255,7 +1191,7 @@ export const exchangeClientCredentials = ( grantType: "client_credentials", tokenUrl: input.tokenUrl, clientAuth: input.clientAuth, - hasResource: resourceParamFor(input) !== undefined, + hasResource: input.resource !== undefined, }), ); @@ -1273,13 +1209,10 @@ export type RefreshAccessTokenInput = { readonly scopeSeparator?: string; readonly clientAuth?: ClientAuthMethod; readonly idTokenSigningAlgValuesSupported?: readonly string[]; - /** RFC 8707 Resource Indicator — the MCP spec asks for this on refresh - * requests so the new access token's audience stays bound to the same - * resource; `authorizationServerMetadata` can veto it. */ + /** RFC 8707 Resource Indicator — MCP spec MUST-requires this on + * refresh requests so the new access token's audience is bound to + * the same resource. */ readonly resource?: string; - /** The authorization server's discovered metadata. Gates `resource` — see - * `shouldSendResourceIndicator`. Omit when nothing was discovered. */ - readonly authorizationServerMetadata?: ResourceIndicatorSupport | null; readonly timeoutMs?: number; readonly endpointUrlPolicy?: OAuthEndpointUrlPolicy; readonly fetch?: typeof globalThis.fetch; @@ -1303,9 +1236,8 @@ export const refreshAccessToken = ( if (input.scopes && input.scopes.length > 0) { extraParams.set("scope", input.scopes.join(input.scopeSeparator ?? " ")); } - const resource = resourceParamFor(input); - if (resource) { - extraParams.set("resource", resource); + if (input.resource) { + extraParams.set("resource", input.resource); } const additionalParameters = Array.from(extraParams.keys()).length > 0 ? extraParams : undefined; @@ -1338,7 +1270,7 @@ export const refreshAccessToken = ( grantType: "refresh_token", tokenUrl: input.tokenUrl, clientAuth: input.clientAuth, - hasResource: resourceParamFor(input) !== undefined, + hasResource: input.resource !== undefined, }), ); diff --git a/packages/core/sdk/src/oauth-scope-union.test.ts b/packages/core/sdk/src/oauth-scope-union.test.ts index 65db26ddb..6e7aa4166 100644 --- a/packages/core/sdk/src/oauth-scope-union.test.ts +++ b/packages/core/sdk/src/oauth-scope-union.test.ts @@ -38,7 +38,7 @@ const DECLARED_SCOPES = ["calendar", "gmail", "drive", "sheets"] as const; const makeScopePluginWithId = ( id: TId, config: { readonly scopes: readonly string[] | null }, - options: { readonly discoversScopes?: boolean } = {}, + options: { readonly discoversScopes?: boolean; readonly discoveryUrl?: string } = {}, ) => definePlugin(() => ({ id, @@ -62,7 +62,7 @@ const makeScopePluginWithId = ( kind: "oauth", template: String(TEMPLATE), ...(options.discoversScopes - ? { oauth: { discoveryUrl: `https://${id}.example/mcp` } } + ? { oauth: { discoveryUrl: options.discoveryUrl ?? `https://${id}.example/mcp` } } : {}), }, ]; @@ -495,19 +495,28 @@ describe("oauth.start integration-driven scopes", () => { ); it.effect( - "(h) for MCP, a client with no resource fails start (discovery cannot run without one)", + "(h) for MCP, a client with no resource still discovers scopes from the integration's discovery URL", () => Effect.scoped( Effect.gen(function* () { + // #1789 — a user may CLEAR the client's RFC 8707 resource (Entra v2 + // rejects the parameter). Scope discovery must not die with it: the + // integration's own discovery URL (the MCP endpoint) is probed + // instead, and the authorize request carries no `resource`. const server = yield* serveMetadataServer({ prm: { scopesSupported: ["read"] } }); const plugins = [ memoryCredentialsPlugin(), - makeMcpScopePlugin({ scopes: null }), + makeScopePluginWithId( + "mcp", + { scopes: null }, + { discoversScopes: true, discoveryUrl: server.mcpResourceUrl }, + ), ] as const; const { executor } = yield* makeTestWorkspaceHarness({ plugins }); yield* executor.mcp.seed(); - // No `resource` on the client — discovery has nothing to probe. + // No `resource` on the client — the wire parameter is absent by + // choice, while discovery still has the integration's URL. yield* executor.oauth.createClient({ owner: "org", slug: CLIENT, @@ -518,17 +527,20 @@ describe("oauth.start integration-driven scopes", () => { clientSecret: "test-secret", }); - const exit = yield* Effect.exit( - executor.oauth.start({ - owner: "org", - client: CLIENT, - clientOwner: "org", - name: ConnectionName.make("main"), - integration: INTEG, - template: TEMPLATE, - }), - ); - expect(Exit.isFailure(exit)).toBe(true); + const started = yield* executor.oauth.start({ + owner: "org", + client: CLIENT, + clientOwner: "org", + name: ConnectionName.make("main"), + integration: INTEG, + template: TEMPLATE, + }); + expect(started.status).toBe("redirect"); + if (started.status !== "redirect") return; + + expect(scopesFromAuthorizeUrl(started.authorizationUrl)).toEqual(["read"]); + // The cleared resource stays cleared on the wire. + expect(new URL(started.authorizationUrl).searchParams.has("resource")).toBe(false); }), ), ); diff --git a/packages/core/sdk/src/oauth-service.ts b/packages/core/sdk/src/oauth-service.ts index 9feb2bcbd..ab62cb958 100644 --- a/packages/core/sdk/src/oauth-service.ts +++ b/packages/core/sdk/src/oauth-service.ts @@ -167,10 +167,15 @@ const startErrorFromEnterpriseManaged = (cause: EnterpriseManagedMintError): OAu * integration declares the scopes to request (`scopes`, possibly empty — an * empty set requests no scopes), or it declares none and the request scopes * are discovered from the server's metadata at connect (`discover`, used by - * MCP). The two are mutually exclusive by construction. */ + * MCP). The two are mutually exclusive by construction. + * + * `discover` carries the integration's own discovery URL (the MCP endpoint) + * so scope discovery does not depend on the CLIENT having a persisted RFC + * 8707 resource: a user may clear the client's resource (Entra v2 rejects + * the parameter, #1789) without losing scope discovery. */ export type OAuthScopePolicy = | { readonly kind: "scopes"; readonly scopes: readonly string[] } - | { readonly kind: "discover" }; + | { readonly kind: "discover"; readonly discoveryUrl: string }; /** Everything the OAuth service needs from the executor: fuma access for the * owned `oauth_client` / `oauth_session` tables, the default credential @@ -205,10 +210,12 @@ export interface OAuthServiceDeps { * DECLARES (e.g. an OpenAPI bundle's authentication-template scope union), * NOT the scopes frozen on a specific `oauth_client` row. These are * requested verbatim at connect (`start`); an empty set requests none. - * - `{ kind: "discover" }`: the integration declares no scopes, so `start` - * discovers the request scopes from the server's RFC 9728 / RFC 8414 - * metadata. Used by server-targeting integrations (MCP) whose scopes live - * on the server rather than in a template. + * - `{ kind: "discover", discoveryUrl }`: the integration declares no + * scopes, so `start` discovers the request scopes from the server's RFC + * 9728 / RFC 8414 metadata. Used by server-targeting integrations (MCP) + * whose scopes live on the server rather than in a template. + * `discoveryUrl` is the integration's protected-resource URL (the MCP + * endpoint), used when the client persists no resource. */ readonly resolveOAuthScopePolicy: ( integration: IntegrationSlug, @@ -1453,7 +1460,14 @@ export const makeOAuthService = (deps: OAuthServiceDeps): OAuthService => { const requestedScopes = scopePolicy.kind === "discover" ? yield* (() => { - const discovered = discoverScopesForResource(client.resource).pipe( + // Scope discovery reads protected-resource metadata. The client's + // persisted resource is the historical source and stays primary, + // but it is a WIRE parameter the user may clear (Entra v2 rejects + // `resource`, #1789) — the integration's own discovery URL then + // keeps scope discovery working for a resource-less client. + const discovered = discoverScopesForResource( + client.resource ?? scopePolicy.discoveryUrl, + ).pipe( Effect.mapError( (cause) => new OAuthStartError({ diff --git a/packages/react/src/components/oauth-client-form.tsx b/packages/react/src/components/oauth-client-form.tsx index 9d73c881d..c9b65d2a2 100644 --- a/packages/react/src/components/oauth-client-form.tsx +++ b/packages/react/src/components/oauth-client-form.tsx @@ -224,6 +224,13 @@ export function OAuthClientForm(props: { mode: "promiseExit", }); + // Blank means "send no RFC 8707 resource": persist null so every OAuth + // request (authorize, exchange, refresh, client-credentials) omits the + // parameter. Clearing must stick — no endpoint is re-derived over an + // intentional absence (Entra v2 rejects `resource`, #1789). + const normalizedResource = + resource == null || resource.trim().length === 0 ? null : resource.trim(); + const canSubmit = canSubmitOAuthClientForm({ submitting, name, @@ -250,7 +257,7 @@ export function OAuthClientForm(props: { authorizationUrl, tokenUrl, issuer: discoveredIssuer, - resource, + resource: normalizedResource, }); const showAppSetup = appSetup !== undefined && grant === "authorization_code" && !showAutoRegister; @@ -303,7 +310,7 @@ export function OAuthClientForm(props: { registrationEndpoint: registrationEndpoint.trim(), authorizationUrl: authorizationUrl.trim(), tokenUrl: tokenUrl.trim(), - resource, + resource: normalizedResource, // DCR sends the integration's declared scopes, or the discovered set when // none are declared, to the AS at registration (the app stores none). scopes: [...registrationScopes(declaredScopes, discoveredScopes)], @@ -337,7 +344,7 @@ export function OAuthClientForm(props: { grant, clientId: clientId.trim(), clientSecret: clientSecret.trim(), - resource, + resource: normalizedResource, // Editing preserves the app's already-recorded origin (via // `intentIntegration`, passed verbatim by the caller); a fresh // registration from an integration's dialog stamps recorded intent. @@ -613,6 +620,26 @@ export function OAuthClientForm(props: { /> + {/* RFC 8707 resource indicator. Prefilled for MCP servers; the field + exists so a user can CLEAR it — some authorization servers + (Microsoft Entra v2) reject requests that carry `resource`, and a + cleared value persists as "no resource" on every OAuth request. */} +
+ + ) => setResource(e.target.value)} + className="font-mono" + /> +
+ {endpointsKnown ? (