* Discover MCP OAuth scopes from server metadata at connect MCP-style integrations declare no OAuth scopes of their own; the scopes live on the server. Replace the declared/client scope union with an integration-driven scope policy: resolveOAuthScopePolicy returns either the template's declared scopes or a { kind: "discover" } marker. For the discover case, oauth.start reads the request scopes from the client's RFC 9728 protected-resource metadata, falling back to the RFC 8414 authorization-server metadata it names. Harden the transparent-DCR connect flow: carry the probe result through the bring-your-own-app fallback, match registered apps against discovered endpoints (requireEndpointMatch), and seed the register form with discovered scopes and token-endpoint auth methods via a shared registrationScopes helper. Add a self-host e2e scenario covering MCP OAuth scope discovery. Entire-Checkpoint: dc839672b8b0 * Bound MCP OAuth scope discovery against hostile or slow servers discoverScopesForResource takes its input entirely from a remote MCP server (the protected-resource metadata and the authorization servers it names). Add three bounds, each guarding a distinct failure: cap the authorization-server probe list at 3 (outbound request fan-out to server-named hosts), cap the discovered scope count at 100 (authorize-URL length), and wrap the whole sequence in a 30s timeout (total wall-clock; larger than the per-request timeout so it bounds the sequence, not a single slow-but-valid request). Tests: a 200-scope advertisement capped to 100, a four-AS list where the 3-server cap stops before the fourth (its scope never appears), and client_credentials reaching the discover path with the discovered scopes recorded on the connection. Entire-Checkpoint: f23d8ac3c4e2 * e2e: prove MCP OAuth scope discovery against a deployed emulate server Add an emulate-backed counterpart to the in-process scope-discovery selfhost test. It points at the @executor-js/emulate GitHub MCP `scope-discovery` instance, whose protected-resource metadata is silent on scopes so oauth.start must fall back to the authorization-server metadata it names, and asserts the authorize URL carries exactly the discovered scopes. Keeps the hermetic in-process test; adds real-deployment fidelity. * Format MCP OAuth scope discovery e2e Entire-Checkpoint: d3ea2960c9ec * Never seed RFC 8707 resource indicator from token endpoint DCR's resource fallback used the probe discovery URL, which collapses to the token endpoint for integrations without a real discovery URL. The token endpoint is not a resource identifier, so a non-MCP DCR client could register the token URL as its RFC 8707 resource indicator and have oauth.start probe it as a protected resource. Thread the genuine discovery URL (the MCP server URL) as a distinct resourceFallback; when absent, the indicator stays null as before. MCP flows are unchanged. Adds a regression test for the non-MCP case. ---------
Unified · Split
| @@ -0,0 +1,127 @@ | ||
| 1 | +// Selfhost-only, emulate-backed counterpart to mcp-oauth-scope-discovery.test.ts. | |
| 2 | +// Same claim — an MCP integration declares NO oauth scopes, so `oauth.start` | |
| 3 | +// DISCOVERS them from the server's metadata at connect — but proven against a | |
| 4 | +// REAL, deployed emulate MCP server instead of an in-process stub. | |
| 5 | +// | |
| 6 | +// The `@executor-js/emulate` GitHub MCP emulator exposes a `scope-discovery` | |
| 7 | +// instance whose RFC 9728 protected-resource metadata is deliberately SILENT on | |
| 8 | +// scopes, forcing a spec-faithful client to fall back to the RFC 8414 | |
| 9 | +// authorization-server metadata it names — which advertises the scopes below. | |
| 10 | +// The selfhost dev server fetches that live metadata over the network and the | |
| 11 | +// returned authorize URL carries exactly the discovered scopes. | |
| 12 | +import { randomBytes } from "node:crypto"; | |
| 13 | + | |
| 14 | +import { expect } from "@effect/vitest"; | |
| 15 | +import { Effect } from "effect"; | |
| 16 | +import { composePluginApi } from "@executor-js/api/server"; | |
| 17 | +import { mcpHttpPlugin } from "@executor-js/plugin-mcp/api"; | |
| 18 | +import { | |
| 19 | + AuthTemplateSlug, | |
| 20 | + ConnectionName, | |
| 21 | + IntegrationSlug, | |
| 22 | + OAuthClientSlug, | |
| 23 | +} from "@executor-js/sdk/shared"; | |
| 24 | +import { scopesFromAuthorizeUrl } from "@executor-js/sdk/testing"; | |
| 25 | + | |
| 26 | +import { scenario } from "../src/scenario"; | |
| 27 | +import { Api, Target } from "../src/services"; | |
| 28 | + | |
| 29 | +const api = composePluginApi([mcpHttpPlugin()] as const); | |
| 30 | + | |
| 31 | +// The deployed emulate instance whose protected-resource metadata is silent on | |
| 32 | +// scopes (it serves the MCP transport AND its own OAuth authorize/token + the | |
| 33 | +// RFC 9728/8414 metadata). The `scope-discovery` preset configures it; no seed | |
| 34 | +// call is needed. Use the path form — a two-label instance subdomain has no | |
| 35 | +// Universal SSL cert. | |
| 36 | +const EMULATOR_BASE = "https://emulators.dev/github/scope-discovery"; | |
| 37 | +const MCP_ENDPOINT = `${EMULATOR_BASE}/mcp`; | |
| 38 | + | |
| 39 | +// The scopes the emulate server advertises, only in its authorization-server | |
| 40 | +// metadata (the protected-resource metadata stays silent). Discovering them | |
| 41 | +// proves executor followed the RFC 8414 fallback, not a declared set. | |
| 42 | +const DISCOVERED_SCOPES = ["channels:history", "users:read"] as const; | |
| 43 | + | |
| 44 | +const freshSlug = (prefix: string): string => `${prefix}-${randomBytes(4).toString("hex")}`; | |
| 45 | + | |
| 46 | +scenario( | |
| 47 | + "MCP OAuth · oauth.start discovers scopes from a deployed emulate MCP server", | |
| 48 | + {}, | |
| 49 | + Effect.scoped( | |
| 50 | + Effect.gen(function* () { | |
| 51 | + const target = yield* Target; | |
| 52 | + const { client: makeApiClient } = yield* Api; | |
| 53 | + | |
| 54 | + const identity = yield* target.newIdentity(); | |
| 55 | + const client = yield* makeApiClient(api, identity); | |
| 56 | + const slug = freshSlug("mcp-scope-emulate"); | |
| 57 | + | |
| 58 | + // An MCP integration pointed at the deployed emulate server — no scopes | |
| 59 | + // declared on the integration; the server is the source. | |
| 60 | + yield* client.mcp.addServer({ | |
| 61 | + payload: { | |
| 62 | + transport: "remote", | |
| 63 | + name: "Scope-discovery MCP (emulate)", | |
| 64 | + endpoint: MCP_ENDPOINT, | |
| 65 | + slug, | |
| 66 | + authenticationTemplate: [{ kind: "oauth2" }], | |
| 67 | + }, | |
| 68 | + }); | |
| 69 | + yield* Effect.addFinalizer(() => | |
| 70 | + client.mcp | |
| 71 | + .removeServer({ params: { slug: IntegrationSlug.make(slug) } }) | |
| 72 | + .pipe(Effect.ignore), | |
| 73 | + ); | |
| 74 | + | |
| 75 | + // An OAuth app bound to the emulate MCP server as its resource (RFC 8707), | |
| 76 | + // using the emulate instance's own authorize/token endpoints. The app | |
| 77 | + // carries no scope set of its own. | |
| 78 | + const clientSlug = OAuthClientSlug.make(freshSlug("mcp-scope-emulate-app")); | |
| 79 | + yield* client.oauth.createClient({ | |
| 80 | + payload: { | |
| 81 | + owner: "org", | |
| 82 | + slug: clientSlug, | |
| 83 | + authorizationUrl: `${EMULATOR_BASE}/authorize`, | |
| 84 | + tokenUrl: `${EMULATOR_BASE}/token`, | |
| 85 | + grant: "authorization_code", | |
| 86 | + clientId: "test-client", | |
| 87 | + clientSecret: "test-secret", | |
| 88 | + resource: MCP_ENDPOINT, | |
| 89 | + }, | |
| 90 | + }); | |
| 91 | + // `removeServer` only reaps DCR-shaped clients, so this manually-created | |
| 92 | + // app must be removed explicitly. | |
| 93 | + yield* Effect.addFinalizer(() => | |
| 94 | + client.oauth | |
| 95 | + .removeClient({ params: { slug: clientSlug }, payload: { owner: "org" } }) | |
| 96 | + .pipe(Effect.ignore), | |
| 97 | + ); | |
| 98 | + | |
| 99 | + const started = yield* client.oauth.start({ | |
| 100 | + payload: { | |
| 101 | + owner: "org", | |
| 102 | + client: clientSlug, | |
| 103 | + clientOwner: "org", | |
| 104 | + name: ConnectionName.make("main"), | |
| 105 | + integration: IntegrationSlug.make(slug), | |
| 106 | + template: AuthTemplateSlug.make("oauth2"), | |
| 107 | + }, | |
| 108 | + }); | |
| 109 | + | |
| 110 | + expect(started.status, "start hands back an authorize redirect").toBe("redirect"); | |
| 111 | + if (started.status !== "redirect") return; | |
| 112 | + // The redirect opened an in-flight OAuth session; drop it so the run leaves | |
| 113 | + // no pending session behind. | |
| 114 | + yield* Effect.addFinalizer(() => | |
| 115 | + client.oauth.cancel({ payload: { state: started.state } }).pipe(Effect.ignore), | |
| 116 | + ); | |
| 117 | + | |
| 118 | + // The selfhost server fetched the deployed emulate server's metadata — | |
| 119 | + // silent protected resource, then its authorization server — and requested | |
| 120 | + // exactly the scopes it advertised. | |
| 121 | + expect( | |
| 122 | + scopesFromAuthorizeUrl(started.authorizationUrl), | |
| 123 | + "the authorize URL carries exactly the scopes the deployed emulate MCP server advertises", | |
| 124 | + ).toEqual([...DISCOVERED_SCOPES]); | |
| 125 | + }), | |
| 126 | + ), | |
| 127 | +); | |
| @@ -0,0 +1,123 @@ | ||
| 1 | +// Selfhost-only: an MCP integration declares NO oauth scopes — the server does. | |
| 2 | +// At connect, `oauth.start` DISCOVERS the request scopes from the MCP server's | |
| 3 | +// RFC 9728 protected-resource metadata and asks for exactly those on the | |
| 4 | +// authorize URL. Proven end-to-end: a real MCP server (in this process) | |
| 5 | +// publishes `scopes_supported`; the selfhost dev server fetches it over loopback | |
| 6 | +// and the returned authorize URL carries the discovered scopes — not a declared | |
| 7 | +// set and not the OAuth app's own scopes (the app declares none). | |
| 8 | +import { randomBytes } from "node:crypto"; | |
| 9 | + | |
| 10 | +import { expect } from "@effect/vitest"; | |
| 11 | +import { Effect } from "effect"; | |
| 12 | +import { composePluginApi } from "@executor-js/api/server"; | |
| 13 | +import { mcpHttpPlugin } from "@executor-js/plugin-mcp/api"; | |
| 14 | +import { makeGreetingMcpServer, serveMcpServer } from "@executor-js/plugin-mcp/testing"; | |
| 15 | +import { | |
| 16 | + AuthTemplateSlug, | |
| 17 | + ConnectionName, | |
| 18 | + IntegrationSlug, | |
| 19 | + OAuthClientSlug, | |
| 20 | +} from "@executor-js/sdk/shared"; | |
| 21 | +import { scopesFromAuthorizeUrl, serveOAuthTestServer } from "@executor-js/sdk/testing"; | |
| 22 | + | |
| 23 | +import { scenario } from "../src/scenario"; | |
| 24 | +import { Api, Target } from "../src/services"; | |
| 25 | + | |
| 26 | +const api = composePluginApi([mcpHttpPlugin()] as const); | |
| 27 | + | |
| 28 | +// The scopes the MCP server advertises in its protected-resource metadata. | |
| 29 | +const DISCOVERED_SCOPES = ["channels:history", "users:read"] as const; | |
| 30 | + | |
| 31 | +const freshSlug = (prefix: string): string => `${prefix}-${randomBytes(4).toString("hex")}`; | |
| 32 | + | |
| 33 | +scenario( | |
| 34 | + "MCP OAuth · oauth.start requests the scopes the server advertises, not a declared set", | |
| 35 | + {}, | |
| 36 | + Effect.scoped( | |
| 37 | + Effect.gen(function* () { | |
| 38 | + const target = yield* Target; | |
| 39 | + const { client: makeApiClient } = yield* Api; | |
| 40 | + | |
| 41 | + // A real authorization server (endpoints for the OAuth app) and a real MCP | |
| 42 | + // server whose RFC 9728 metadata advertises `scopes_supported`. The MCP | |
| 43 | + // integration declares NO template scopes — the server is the source. | |
| 44 | + const oauth = yield* serveOAuthTestServer(); | |
| 45 | + const mcp = yield* serveMcpServer(() => makeGreetingMcpServer(), { | |
| 46 | + auth: { | |
| 47 | + validateAuthorization: () => Effect.succeed(true), | |
| 48 | + authorizationServerUrls: [oauth.issuerUrl], | |
| 49 | + scopes: [...DISCOVERED_SCOPES], | |
| 50 | + }, | |
| 51 | + }); | |
| 52 | + | |
| 53 | + const identity = yield* target.newIdentity(); | |
| 54 | + const client = yield* makeApiClient(api, identity); | |
| 55 | + const slug = freshSlug("mcp-scope-discovery"); | |
| 56 | + | |
| 57 | + // An MCP integration with a single OAuth method — no scopes declared. | |
| 58 | + yield* client.mcp.addServer({ | |
| 59 | + payload: { | |
| 60 | + transport: "remote", | |
| 61 | + name: "Scope-discovery MCP", | |
| 62 | + endpoint: mcp.endpoint, | |
| 63 | + slug, | |
| 64 | + authenticationTemplate: [{ kind: "oauth2" }], | |
| 65 | + }, | |
| 66 | + }); | |
| 67 | + yield* Effect.addFinalizer(() => | |
| 68 | + client.mcp | |
| 69 | + .removeServer({ params: { slug: IntegrationSlug.make(slug) } }) | |
| 70 | + .pipe(Effect.ignore), | |
| 71 | + ); | |
| 72 | + | |
| 73 | + // An OAuth app bound to the MCP server as its resource (RFC 8707). The app | |
| 74 | + // carries no scope set of its own. | |
| 75 | + const clientSlug = OAuthClientSlug.make(freshSlug("mcp-scope-app")); | |
| 76 | + yield* client.oauth.createClient({ | |
| 77 | + payload: { | |
| 78 | + owner: "org", | |
| 79 | + slug: clientSlug, | |
| 80 | + authorizationUrl: oauth.authorizationEndpoint, | |
| 81 | + tokenUrl: oauth.tokenEndpoint, | |
| 82 | + grant: "authorization_code", | |
| 83 | + clientId: "test-client", | |
| 84 | + clientSecret: "test-secret", | |
| 85 | + resource: mcp.endpoint, | |
| 86 | + }, | |
| 87 | + }); | |
| 88 | + // Best-effort cleanup: `removeServer` only reaps DCR-shaped clients, so this | |
| 89 | + // manually-created app must be removed explicitly. | |
| 90 | + yield* Effect.addFinalizer(() => | |
| 91 | + client.oauth | |
| 92 | + .removeClient({ params: { slug: clientSlug }, payload: { owner: "org" } }) | |
| 93 | + .pipe(Effect.ignore), | |
| 94 | + ); | |
| 95 | + | |
| 96 | + const started = yield* client.oauth.start({ | |
| 97 | + payload: { | |
| 98 | + owner: "org", | |
| 99 | + client: clientSlug, | |
| 100 | + clientOwner: "org", | |
| 101 | + name: ConnectionName.make("main"), | |
| 102 | + integration: IntegrationSlug.make(slug), | |
| 103 | + template: AuthTemplateSlug.make("oauth2"), | |
| 104 | + }, | |
| 105 | + }); | |
| 106 | + | |
| 107 | + expect(started.status, "start hands back an authorize redirect").toBe("redirect"); | |
| 108 | + if (started.status !== "redirect") return; | |
| 109 | + // The redirect opened an in-flight OAuth session; drop it so the run leaves | |
| 110 | + // no pending session behind. | |
| 111 | + yield* Effect.addFinalizer(() => | |
| 112 | + client.oauth.cancel({ payload: { state: started.state } }).pipe(Effect.ignore), | |
| 113 | + ); | |
| 114 | + | |
| 115 | + // The selfhost server fetched the MCP server's protected-resource metadata | |
| 116 | + // and requested exactly the scopes it advertised. | |
| 117 | + expect( | |
| 118 | + scopesFromAuthorizeUrl(started.authorizationUrl), | |
| 119 | + "the authorize URL carries exactly the scopes the MCP server advertised", | |
| 120 | + ).toEqual([...DISCOVERED_SCOPES]); | |
| 121 | + }), | |
| 122 | + ), | |
| 123 | +); | |
| @@ -85,7 +85,11 @@ import type { · 84 unmodified lines | ||
| 85 | 85 | IntegrationConfig, |
| 86 | 86 | RegisterIntegrationInput, |
| 87 | 87 | } from "./integration"; |
| 88 | −import { makeOAuthService, type MintOAuthConnectionInput } from "./oauth-service"; | |
| 88 | +import { | |
| 89 | + makeOAuthService, | |
| 90 | + type MintOAuthConnectionInput, | |
| 91 | + type OAuthScopePolicy, | |
| 92 | +} from "./oauth-service"; | |
| 89 | 93 | import type { OAuthService } from "./oauth-client"; |
| 90 | 94 | import { |
| 91 | 95 | comparePolicyRow, |
| @@ -3192,23 +3196,27 @@ export const createExecutor = <const TPlugins extends readonly AnyPlugin[] = rea · 3100 unmodified lines | ||
| 3192 | 3196 | ownedKeys: (owner: Owner) => ownedKeys(owner), |
| 3193 | 3197 | defaultWritableProvider, |
| 3194 | 3198 | mintOAuthConnection: (input: MintOAuthConnectionInput) => mintOAuthConnection(input), |
| 3195 | − // Resolve the integration's DECLARED oauth scopes for a (integration, | |
| 3196 | − // template): load the row, run the owning plugin's auth-method projector, | |
| 3197 | − // and return the matching oauth method's declared `oauth.scopes`. Drives | |
| 3198 | − // the union-with-client-scopes request in `oauth.start` so reusing a narrow | |
| 3199 | − // client on a broad integration still requests the integration's full | |
| 3200 | − // scope set. Empty (no row / no oauth method / no declared scopes) ⇒ the | |
| 3201 | − // union collapses to the client's scopes (current behavior). | |
| 3202 | − resolveDeclaredOAuthScopes: (integration: IntegrationSlug, template: AuthTemplateSlug) => | |
| 3199 | + // One integration-row read + one projector run. Resolve the method this | |
| 3200 | + // template selects exactly as the runtime's `selectAuthMethod` does — | |
| 3201 | + // exact slug match, else the sole declared method (single-method | |
| 3202 | + // integrations accept any slug); an ambiguous miss selects nothing rather | |
| 3203 | + // than guessing across methods. The discover-vs-scopes choice then reads | |
| 3204 | + // off that method (MCP exposes `discoveryUrl`), so core needs no plugin-id | |
| 3205 | + // knowledge. | |
| 3206 | + resolveOAuthScopePolicy: (integration: IntegrationSlug, template: AuthTemplateSlug) => | |
| 3203 | 3207 | findIntegrationRow(integration).pipe( |
| 3204 | − Effect.map((row): readonly string[] => { | |
| 3205 | − if (!row) return []; | |
| 3206 | − const methods = describeAuthMethodsForRow(row); | |
| 3207 | − const match = | |
| 3208 | − methods.find( | |
| 3209 | − (m: AuthMethodDescriptor) => m.kind === "oauth" && m.template === String(template), | |
| 3210 | − ) ?? methods.find((m: AuthMethodDescriptor) => m.kind === "oauth"); | |
| 3211 | − return match?.oauth?.scopes ?? []; | |
| 3208 | + Effect.map((row): OAuthScopePolicy => { | |
| 3209 | + const methods = row ? describeAuthMethodsForRow(row) : []; | |
| 3210 | + const selected = | |
| 3211 | + methods.find((m: AuthMethodDescriptor) => m.template === String(template)) ?? | |
| 3212 | + (methods.length === 1 ? methods[0] : undefined); | |
| 3213 | + const oauth = selected?.kind === "oauth" ? selected.oauth : undefined; | |
| 3214 | + // Declared scopes win. Discover only when the selected method | |
| 3215 | + // declares none but names a source to discover them from (MCP). | |
| 3216 | + if (oauth?.scopes === undefined && oauth?.discoveryUrl !== undefined) { | |
| 3217 | + return { kind: "discover" }; | |
| 3218 | + } | |
| 3219 | + return { kind: "scopes", scopes: oauth?.scopes ?? [] }; | |
| 3212 | 3220 | }), |
| 3213 | 3221 | ), |
| 3214 | 3222 | httpClientLayer: config.httpClientLayer, |
| @@ -1,6 +1,6 @@ | ||
| 1 | 1 | import * as NodeHttpServer from "@effect/platform-node/NodeHttpServer"; |
| 2 | 2 | import { describe, expect, it } from "@effect/vitest"; |
| 3 | −import { Context, Effect, Layer, Predicate, type Scope } from "effect"; | |
| 3 | +import { Context, Effect, Exit, Layer, Predicate, type Scope } from "effect"; | |
| 4 | 4 | import { HttpServer, HttpServerRequest, HttpServerResponse } from "effect/unstable/http"; |
| 5 | 5 | |
| 6 | 6 | import { |
| @@ -13,12 +13,15 @@ import { · 6 unmodified lines | ||
| 13 | 13 | import type { AuthMethodDescriptor } from "./integration"; |
| 14 | 14 | import { definePlugin, type IntegrationRecord } from "./plugin"; |
| 15 | 15 | import { makeTestWorkspaceHarness, memoryCredentialsPlugin } from "./test-config"; |
| 16 | −import { serveOAuthTestServer } from "./testing/oauth-test-server"; | |
| 16 | +import { serveTestHttpApp } from "./testing"; | |
| 17 | +import { scopesFromAuthorizeUrl, serveOAuthTestServer } from "./testing/oauth-test-server"; | |
| 17 | 18 | |
| 18 | −// Integration-driven scopes: at connect, `oauth.start` requests EXACTLY the | |
| 19 | −// integration's DECLARED oauth scopes. The OAuth app carries no scope set of its | |
| 20 | −// own, so there is no union and no over-request — the integration is the sole | |
| 21 | −// source of what to request. (Replaces the earlier declared∪client union model.) | |
| 19 | +// Integration-driven scopes: at connect, `oauth.start` requests the integration's | |
| 20 | +// scopes — its DECLARED oauth scopes when it has any, otherwise the scopes | |
| 21 | +// DISCOVERED from the server's RFC 9728 / RFC 8414 metadata (MCP-style). Either | |
| 22 | +// way the OAuth app carries no scope set of its own, so there is no union and no | |
| 23 | +// over-request — the integration is the sole source of what to request. (Replaces | |
| 24 | +// the earlier declared∪client union model.) | |
| 22 | 25 | |
| 23 | 26 | const INTEG = IntegrationSlug.make("acme"); |
| 24 | 27 | const TEMPLATE = AuthTemplateSlug.make("oauth"); |
| @@ -29,11 +32,15 @@ const DECLARED_SCOPES = ["calendar", "gmail", "drive", "sheets"] as const; · 4 unmodified lines | ||
| 29 | 32 | |
| 30 | 33 | /** A plugin whose integration config carries declared oauth scopes, projected |
| 31 | 34 | * into an oauth `AuthMethodDescriptor` via `describeAuthMethods` — exactly the |
| 32 | − * shape `resolveDeclaredOAuthScopes` reads. `scopes: null` ⇒ no declared oauth | |
| 35 | + * shape `resolveOAuthScopePolicy` reads. `scopes: null` ⇒ no declared oauth | |
| 33 | 36 | * scopes (the MCP/no-template-scopes case). */ |
| 34 | −const makeScopePlugin = (config: { readonly scopes: readonly string[] | null }) => | |
| 37 | +const makeScopePluginWithId = <const TId extends string>( | |
| 38 | + id: TId, | |
| 39 | + config: { readonly scopes: readonly string[] | null }, | |
| 40 | + options: { readonly discoversScopes?: boolean } = {}, | |
| 41 | +) => | |
| 35 | 42 | definePlugin(() => ({ |
| 36 | − id: "acme" as const, | |
| 43 | + id, | |
| 37 | 44 | storage: () => ({}), |
| 38 | 45 | resolveTools: () => |
| 39 | 46 | Effect.succeed({ |
| @@ -44,9 +51,20 @@ const makeScopePlugin = (config: { readonly scopes: readonly string[] | null }) · 4 unmodified lines | ||
| 44 | 51 | const cfg = record.config as { readonly scopes?: readonly string[] | null } | null; |
| 45 | 52 | const scopes = cfg?.scopes; |
| 46 | 53 | if (scopes == null) { |
| 47 | − // No declared oauth scopes — the integration declares an oauth method | |
| 48 | − // with no template scopes (MCP/DCR-style). Declared scopes resolve to []. | |
| 49 | − return [{ id: "oauth", label: "OAuth2", kind: "oauth", template: String(TEMPLATE) }]; | |
| 54 | + // No declared oauth scopes. Server-targeting methods (MCP) expose a | |
| 55 | + // `discoveryUrl` and discover scopes from the server at connect; others | |
| 56 | + // declare none. Declared scopes resolve to []. | |
| 57 | + return [ | |
| 58 | + { | |
| 59 | + id: "oauth", | |
| 60 | + label: "OAuth2", | |
| 61 | + kind: "oauth", | |
| 62 | + template: String(TEMPLATE), | |
| 63 | + ...(options.discoversScopes | |
| 64 | + ? { oauth: { discoveryUrl: `https://${id}.example/mcp` } } | |
| 65 | + : {}), | |
| 66 | + }, | |
| 67 | + ]; | |
| 50 | 68 | } |
| 51 | 69 | return [ |
| 52 | 70 | { |
| @@ -68,11 +86,97 @@ const makeScopePlugin = (config: { readonly scopes: readonly string[] | null }) · 15 unmodified lines | ||
| 68 | 86 | }), |
| 69 | 87 | }))(); |
| 70 | 88 | |
| 71 | −/** Parse the `scope` query param from an authorize URL into an ordered list. */ | |
| 72 | −const scopesFromAuthorizeUrl = (authorizationUrl: string): readonly string[] => { | |
| 73 | − const raw = new URL(authorizationUrl).searchParams.get("scope"); | |
| 74 | − return raw == null || raw.length === 0 ? [] : raw.split(" "); | |
| 75 | −}; | |
| 89 | +const makeScopePlugin = (config: { readonly scopes: readonly string[] | null }) => | |
| 90 | + makeScopePluginWithId("acme", config); | |
| 91 | + | |
| 92 | +const makeMcpScopePlugin = (config: { readonly scopes: readonly string[] | null }) => | |
| 93 | + makeScopePluginWithId("mcp", config, { discoversScopes: true }); | |
| 94 | + | |
| 95 | +/** Serve RFC 9728 protected-resource metadata and RFC 8414 authorization-server | |
| 96 | + * metadata on one origin, with configurable scopes. `prm`: an object serves | |
| 97 | + * protected-resource metadata (omit `scopesSupported` to leave the resource | |
| 98 | + * SILENT on scopes; `[]` advertises an explicit empty set); `"error"` makes the | |
| 99 | + * PRM endpoint return 500; `null`/absent makes it 404. `authServerScopes`, when | |
| 100 | + * set, serves authorization-server metadata advertising those scopes. */ | |
| 101 | +const serveMetadataServer = (config: { | |
| 102 | + readonly prm?: | |
| 103 | + | { | |
| 104 | + readonly authorizationServers?: readonly string[]; | |
| 105 | + readonly scopesSupported?: readonly string[]; | |
| 106 | + } | |
| 107 | + | "error" | |
| 108 | + | null; | |
| 109 | + readonly authServerScopes?: readonly string[]; | |
| 110 | +}) => | |
| 111 | + Effect.gen(function* () { | |
| 112 | + const baseUrlRef = { value: "" }; | |
| 113 | + const respond = (path: string): HttpServerResponse.HttpServerResponse => { | |
| 114 | + const base = baseUrlRef.value; | |
| 115 | + if (path.startsWith("/.well-known/oauth-protected-resource")) { | |
| 116 | + if (config.prm === "error") | |
| 117 | + return HttpServerResponse.jsonUnsafe({ error: "boom" }, { status: 500 }); | |
| 118 | + if (config.prm == null) return HttpServerResponse.empty({ status: 404 }); | |
| 119 | + return HttpServerResponse.jsonUnsafe({ | |
| 120 | + resource: `${base}/mcp`, | |
| 121 | + bearer_methods_supported: ["header"], | |
| 122 | + ...(config.prm.authorizationServers | |
| 123 | + ? { authorization_servers: config.prm.authorizationServers } | |
| 124 | + : {}), | |
| 125 | + ...(config.prm.scopesSupported !== undefined | |
| 126 | + ? { scopes_supported: config.prm.scopesSupported } | |
| 127 | + : {}), | |
| 128 | + }); | |
| 129 | + } | |
| 130 | + if ( | |
| 131 | + path === "/.well-known/oauth-authorization-server" && | |
| 132 | + config.authServerScopes !== undefined | |
| 133 | + ) { | |
| 134 | + return HttpServerResponse.jsonUnsafe({ | |
| 135 | + issuer: base, | |
| 136 | + authorization_endpoint: `${base}/authorize`, | |
| 137 | + token_endpoint: `${base}/token`, | |
| 138 | + response_types_supported: ["code"], | |
| 139 | + code_challenge_methods_supported: ["S256"], | |
| 140 | + scopes_supported: config.authServerScopes, | |
| 141 | + }); | |
| 142 | + } | |
| 143 | + return HttpServerResponse.empty({ status: 404 }); | |
| 144 | + }; | |
| 145 | + const server = yield* serveTestHttpApp((request) => Effect.succeed(respond(request.url))); | |
| 146 | + baseUrlRef.value = server.baseUrl; | |
| 147 | + return { | |
| 148 | + baseUrl: server.baseUrl, | |
| 149 | + authorizationEndpoint: server.url("/authorize"), | |
| 150 | + tokenEndpoint: server.url("/token"), | |
| 151 | + mcpResourceUrl: server.url("/mcp"), | |
| 152 | + }; | |
| 153 | + }); | |
| 154 | + | |
| 155 | +/** Seed an MCP integration with no declared scopes and an OAuth app bound to the | |
| 156 | + * given server as its resource, returning the executor ready to `oauth.start`. | |
| 157 | + * The shared setup for the discovery cases below; case (h) inlines its own (no | |
| 158 | + * `resource`) because the absent resource IS the case under test. */ | |
| 159 | +const setupMcpScopeClient = (server: { | |
| 160 | + readonly authorizationEndpoint: string; | |
| 161 | + readonly tokenEndpoint: string; | |
| 162 | + readonly mcpResourceUrl: string; | |
| 163 | +}) => | |
| 164 | + Effect.gen(function* () { | |
| 165 | + const plugins = [memoryCredentialsPlugin(), makeMcpScopePlugin({ scopes: null })] as const; | |
| 166 | + const { executor } = yield* makeTestWorkspaceHarness({ plugins }); | |
| 167 | + yield* executor.mcp.seed(); | |
| 168 | + yield* executor.oauth.createClient({ | |
| 169 | + owner: "org", | |
| 170 | + slug: CLIENT, | |
| 171 | + authorizationUrl: server.authorizationEndpoint, | |
| 172 | + tokenUrl: server.tokenEndpoint, | |
| 173 | + grant: "authorization_code", | |
| 174 | + clientId: "test-client", | |
| 175 | + clientSecret: "test-secret", | |
| 176 | + resource: server.mcpResourceUrl, | |
| 177 | + }); | |
| 178 | + return executor; | |
| 179 | + }); | |
| 76 | 180 | |
| 77 | 181 | describe("oauth.start integration-driven scopes", () => { |
| 78 | 182 | it.effect( |
| @@ -151,10 +255,265 @@ describe("oauth.start integration-driven scopes", () => { · 72 unmodified lines | ||
| 151 | 255 | }), |
| 152 | 256 | ), |
| 153 | 257 | ); |
| 258 | + | |
| 259 | + it.effect( | |
| 260 | + "(c) for MCP with no protected-resource metadata, start requests no scopes (no arbitrary issuer probing)", | |
| 261 | + () => | |
| 262 | + Effect.scoped( | |
| 263 | + Effect.gen(function* () { | |
| 264 | + // The server advertises authorization-server scopes but publishes no | |
| 265 | + // RFC 9728 protected-resource metadata. We only follow the | |
| 266 | + // authorization servers a resource NAMES, so with no PRM there is | |
| 267 | + // nothing to follow — start requests no scopes rather than probing the | |
| 268 | + // resource URL or its origin. | |
| 269 | + const server = yield* serveMetadataServer({ | |
| 270 | + authServerScopes: ["channels:history", "users:read"], | |
| 271 | + }); | |
| 272 | + const executor = yield* setupMcpScopeClient(server); | |
| 273 | + | |
| 274 | + const started = yield* executor.oauth.start({ | |
| 275 | + owner: "org", | |
| 276 | + client: CLIENT, | |
| 277 | + clientOwner: "org", | |
| 278 | + name: ConnectionName.make("main"), | |
| 279 | + integration: INTEG, | |
| 280 | + template: TEMPLATE, | |
| 281 | + }); | |
| 282 | + expect(started.status).toBe("redirect"); | |
| 283 | + if (started.status !== "redirect") return; | |
| 284 | + | |
| 285 | + expect(scopesFromAuthorizeUrl(started.authorizationUrl)).toEqual([]); | |
| 286 | + }), | |
| 287 | + ), | |
| 288 | + ); | |
| 289 | + | |
| 290 | + it.effect( | |
| 291 | + "(d) for MCP, the resource's own scopes_supported wins over a divergent authorization server", | |
| 292 | + () => | |
| 293 | + Effect.scoped( | |
| 294 | + Effect.gen(function* () { | |
| 295 | + // The resource publishes its own RFC 9728 `scopes_supported` AND names | |
| 296 | + // an AS advertising a DIFFERENT set. The resource's list is | |
| 297 | + // authoritative (RFC 9728 §7.2): start requests exactly it and never | |
| 298 | + // consults the AS. This is the primary positive discovery path. | |
| 299 | + const server = yield* serveMetadataServer({ | |
| 300 | + prm: { scopesSupported: ["channels:history", "users:read"] }, | |
| 301 | + authServerScopes: ["should:not:appear"], | |
| 302 | + }); | |
| 303 | + const executor = yield* setupMcpScopeClient(server); | |
| 304 | + | |
| 305 | + const started = yield* executor.oauth.start({ | |
| 306 | + owner: "org", | |
| 307 | + client: CLIENT, | |
| 308 | + clientOwner: "org", | |
| 309 | + name: ConnectionName.make("main"), | |
| 310 | + integration: INTEG, | |
| 311 | + template: TEMPLATE, | |
| 312 | + }); | |
| 313 | + expect(started.status).toBe("redirect"); | |
| 314 | + if (started.status !== "redirect") return; | |
| 315 | + | |
| 316 | + expect(scopesFromAuthorizeUrl(started.authorizationUrl)).toEqual([ | |
| 317 | + "channels:history", | |
| 318 | + "users:read", | |
| 319 | + ]); | |
| 320 | + }), | |
| 321 | + ), | |
| 322 | + ); | |
| 323 | + | |
| 324 | + it.effect( | |
| 325 | + "(e) for MCP, discovers scopes from a cross-origin authorization server named in resource metadata", | |
| 326 | + () => | |
| 327 | + Effect.scoped( | |
| 328 | + Effect.gen(function* () { | |
| 329 | + // The resource's PRM is silent on scopes but names an AS on a DIFFERENT | |
| 330 | + // origin (RFC 9728). Discovery must follow `authorization_servers`, not | |
| 331 | + // just the resource URL/origin. | |
| 332 | + const authServer = yield* serveMetadataServer({ | |
| 333 | + authServerScopes: ["read:cross", "write:cross"], | |
| 334 | + }); | |
| 335 | + const resourceServer = yield* serveMetadataServer({ | |
| 336 | + prm: { authorizationServers: [authServer.baseUrl] }, | |
| 337 | + }); | |
| 338 | + const executor = yield* setupMcpScopeClient(resourceServer); | |
| 339 | + | |
| 340 | + const started = yield* executor.oauth.start({ | |
| 341 | + owner: "org", | |
| 342 | + client: CLIENT, | |
| 343 | + clientOwner: "org", | |
| 344 | + name: ConnectionName.make("main"), | |
| 345 | + integration: INTEG, | |
| 346 | + template: TEMPLATE, | |
| 347 | + }); | |
| 348 | + expect(started.status).toBe("redirect"); | |
| 349 | + if (started.status !== "redirect") return; | |
| 350 | + | |
| 351 | + expect(scopesFromAuthorizeUrl(started.authorizationUrl)).toEqual([ | |
| 352 | + "read:cross", | |
| 353 | + "write:cross", | |
| 354 | + ]); | |
| 355 | + }), | |
| 356 | + ), | |
| 357 | + ); | |
| 358 | + | |
| 359 | + it.effect( | |
| 360 | + "(f) for MCP, an explicit empty resource scopes_supported is honored (no AS fallback)", | |
| 361 | + () => | |
| 362 | + Effect.scoped( | |
| 363 | + Effect.gen(function* () { | |
| 364 | + // The resource explicitly advertises NO scopes; the AS would offer some, | |
| 365 | + // but the resource's explicit empty set is authoritative. | |
| 366 | + const server = yield* serveMetadataServer({ | |
| 367 | + prm: { scopesSupported: [] }, | |
| 368 | + authServerScopes: ["should:not:appear"], | |
| 369 | + }); | |
| 370 | + const executor = yield* setupMcpScopeClient(server); | |
| 371 | + | |
| 372 | + const started = yield* executor.oauth.start({ | |
| 373 | + owner: "org", | |
| 374 | + client: CLIENT, | |
| 375 | + clientOwner: "org", | |
| 376 | + name: ConnectionName.make("main"), | |
| 377 | + integration: INTEG, | |
| 378 | + template: TEMPLATE, | |
| 379 | + }); | |
| 380 | + expect(started.status).toBe("redirect"); | |
| 381 | + if (started.status !== "redirect") return; | |
| 382 | + | |
| 383 | + expect(scopesFromAuthorizeUrl(started.authorizationUrl)).toEqual([]); | |
| 384 | + }), | |
| 385 | + ), | |
| 386 | + ); | |
| 387 | + | |
| 388 | + it.effect( | |
| 389 | + "(g) for MCP, a resource-metadata discovery error fails start (no silent empty-scope fallback)", | |
| 390 | + () => | |
| 391 | + Effect.scoped( | |
| 392 | + Effect.gen(function* () { | |
| 393 | + // A transient 500 on the PRM endpoint must surface, not be swallowed | |
| 394 | + // into "request no scopes". | |
| 395 | + const server = yield* serveMetadataServer({ prm: "error" }); | |
| 396 | + const executor = yield* setupMcpScopeClient(server); | |
| 397 | + | |
| 398 | + const exit = yield* Effect.exit( | |
| 399 | + executor.oauth.start({ | |
| 400 | + owner: "org", | |
| 401 | + client: CLIENT, | |
| 402 | + clientOwner: "org", | |
| 403 | + name: ConnectionName.make("main"), | |
| 404 | + integration: INTEG, | |
| 405 | + template: TEMPLATE, | |
| 406 | + }), | |
| 407 | + ); | |
| 408 | + expect(Exit.isFailure(exit)).toBe(true); | |
| 409 | + }), | |
| 410 | + ), | |
| 411 | + ); | |
| 412 | + | |
| 413 | + it.effect( | |
| 414 | + "(h) for MCP, a client with no resource fails start (discovery cannot run without one)", | |
| 415 | + () => | |
| 416 | + Effect.scoped( | |
| 417 | + Effect.gen(function* () { | |
| 418 | + const server = yield* serveMetadataServer({ prm: { scopesSupported: ["read"] } }); | |
| 419 | + const plugins = [ | |
| 420 | + memoryCredentialsPlugin(), | |
| 421 | + makeMcpScopePlugin({ scopes: null }), | |
| 422 | + ] as const; | |
| 423 | + const { executor } = yield* makeTestWorkspaceHarness({ plugins }); | |
| 424 | + yield* executor.mcp.seed(); | |
| 425 | + | |
| 426 | + // No `resource` on the client — discovery has nothing to probe. | |
| 427 | + yield* executor.oauth.createClient({ | |
| 428 | + owner: "org", | |
| 429 | + slug: CLIENT, | |
| 430 | + authorizationUrl: server.authorizationEndpoint, | |
| 431 | + tokenUrl: server.tokenEndpoint, | |
| 432 | + grant: "authorization_code", | |
| 433 | + clientId: "test-client", | |
| 434 | + clientSecret: "test-secret", | |
| 435 | + }); | |
| 436 | + | |
| 437 | + const exit = yield* Effect.exit( | |
| 438 | + executor.oauth.start({ | |
| 439 | + owner: "org", | |
| 440 | + client: CLIENT, | |
| 441 | + clientOwner: "org", | |
| 442 | + name: ConnectionName.make("main"), | |
| 443 | + integration: INTEG, | |
| 444 | + template: TEMPLATE, | |
| 445 | + }), | |
| 446 | + ); | |
| 447 | + expect(Exit.isFailure(exit)).toBe(true); | |
| 448 | + }), | |
| 449 | + ), | |
| 450 | + ); | |
| 451 | + | |
| 452 | + it.effect("(j) caps server-advertised resource scopes so the authorize URL stays bounded", () => | |
| 453 | + Effect.scoped( | |
| 454 | + Effect.gen(function* () { | |
| 455 | + // A hostile/buggy server advertises far more scopes than any real | |
| 456 | + // template. Discovery caps the request at 100 so the authorize URL | |
| 457 | + // cannot be blown up. | |
| 458 | + const manyScopes = Array.from({ length: 200 }, (_, i) => `scope:${i}`); | |
| 459 | + const server = yield* serveMetadataServer({ prm: { scopesSupported: manyScopes } }); | |
| 460 | + const executor = yield* setupMcpScopeClient(server); | |
| 461 | + | |
| 462 | + const started = yield* executor.oauth.start({ | |
| 463 | + owner: "org", | |
| 464 | + client: CLIENT, | |
| 465 | + clientOwner: "org", | |
| 466 | + name: ConnectionName.make("main"), | |
| 467 | + integration: INTEG, | |
| 468 | + template: TEMPLATE, | |
| 469 | + }); | |
| 470 | + expect(started.status).toBe("redirect"); | |
| 471 | + if (started.status !== "redirect") return; | |
| 472 | + | |
| 473 | + const requested = scopesFromAuthorizeUrl(started.authorizationUrl); | |
| 474 | + expect(requested.length).toBe(100); | |
| 475 | + expect(requested).toEqual(manyScopes.slice(0, 100)); | |
| 476 | + }), | |
| 477 | + ), | |
| 478 | + ); | |
| 479 | + | |
| 480 | + it.effect( | |
| 481 | + "(k) caps the named authorization-server probe list at 3 (a 4th named AS is never reached)", | |
| 482 | + () => | |
| 483 | + Effect.scoped( | |
| 484 | + Effect.gen(function* () { | |
| 485 | + // The resource's PRM is silent on scopes and names four authorization | |
| 486 | + // servers. The first three contribute nothing; only the fourth would | |
| 487 | + // advertise scopes. The 3-server cap stops before it, so discovery | |
| 488 | + // yields no scopes — without the cap "capped:out" would appear. | |
| 489 | + const as1 = yield* serveMetadataServer({}); | |
| 490 | + const as2 = yield* serveMetadataServer({}); | |
| 491 | + const as3 = yield* serveMetadataServer({}); | |
| 492 | + const as4 = yield* serveMetadataServer({ authServerScopes: ["capped:out"] }); | |
| 493 | + const resourceServer = yield* serveMetadataServer({ | |
| 494 | + prm: { authorizationServers: [as1.baseUrl, as2.baseUrl, as3.baseUrl, as4.baseUrl] }, | |
| 495 | + }); | |
| 496 | + const executor = yield* setupMcpScopeClient(resourceServer); | |
| 497 | + | |
| 498 | + const started = yield* executor.oauth.start({ | |
| 499 | + owner: "org", | |
| 500 | + client: CLIENT, | |
| 501 | + clientOwner: "org", | |
| 502 | + name: ConnectionName.make("main"), | |
| 503 | + integration: INTEG, | |
| 504 | + template: TEMPLATE, | |
| 505 | + }); | |
| 506 | + expect(started.status).toBe("redirect"); | |
| 507 | + if (started.status !== "redirect") return; | |
| 508 | + | |
| 509 | + expect(scopesFromAuthorizeUrl(started.authorizationUrl)).toEqual([]); | |
| 510 | + }), | |
| 511 | + ), | |
| 512 | + ); | |
| 154 | 513 | }); |
| 155 | 514 | |
| 156 | 515 | // ----------------------------------------------------------------------------- |
| 157 | −// (c) The recorded `oauth_scope` reflects the requested (declared) scopes when | |
| 516 | +// (i) The recorded `oauth_scope` reflects the requested (declared) scopes when | |
| 158 | 517 | // the AS omits `scope`. A minimal inline token endpoint handles the |
| 159 | 518 | // client_credentials grant and deliberately OMITS `scope` from its response, |
| 160 | 519 | // forcing the recorded-scope fallback (`token.scope ?? requested.join(" ")`). |
| @@ -203,7 +562,7 @@ const serveScopelessTokenServer = (): Effect.Effect< · 42 unmodified lines | ||
| 203 | 562 | |
| 204 | 563 | describe("oauth.start recorded scope fallback", () => { |
| 205 | 564 | it.effect( |
| 206 | − "(c) records the requested (declared) scopes when the authorization server omits scope", | |
| 565 | + "(i) records the requested (declared) scopes when the authorization server omits scope", | |
| 207 | 566 | () => |
| 208 | 567 | Effect.scoped( |
| 209 | 568 | Effect.gen(function* () { |
| @@ -248,4 +607,57 @@ describe("oauth.start recorded scope fallback", () => { · 38 unmodified lines | ||
| 248 | 607 | }), |
| 249 | 608 | ), |
| 250 | 609 | ); |
| 610 | + | |
| 611 | + it.effect( | |
| 612 | + "(l) client_credentials discovers scopes and records them when the token endpoint omits scope", | |
| 613 | + () => | |
| 614 | + Effect.scoped( | |
| 615 | + Effect.gen(function* () { | |
| 616 | + // Discover policy (MCP, no declared scopes) reached through the | |
| 617 | + // client_credentials grant: scopes come from the resource's PRM, and | |
| 618 | + // the scopeless token endpoint forces the recorded-scope fallback to | |
| 619 | + // the discovered set. | |
| 620 | + const metadataServer = yield* serveMetadataServer({ | |
| 621 | + prm: { scopesSupported: ["mcp:read", "mcp:write"] }, | |
| 622 | + }); | |
| 623 | + const tokenServer = yield* serveScopelessTokenServer(); | |
| 624 | + const plugins = [ | |
| 625 | + memoryCredentialsPlugin(), | |
| 626 | + makeMcpScopePlugin({ scopes: null }), | |
| 627 | + ] as const; | |
| 628 | + const { executor, config } = yield* makeTestWorkspaceHarness({ plugins }); | |
| 629 | + yield* executor.mcp.seed(); | |
| 630 | + | |
| 631 | + yield* executor.oauth.createClient({ | |
| 632 | + owner: "org", | |
| 633 | + slug: CLIENT, | |
| 634 | + authorizationUrl: "http://127.0.0.1/authorize", | |
| 635 | + tokenUrl: tokenServer.tokenEndpoint, | |
| 636 | + grant: "client_credentials", | |
| 637 | + clientId: "test-client", | |
| 638 | + clientSecret: "test-secret", | |
| 639 | + resource: metadataServer.mcpResourceUrl, | |
| 640 | + }); | |
| 641 | + | |
| 642 | + const started = yield* executor.oauth.start({ | |
| 643 | + owner: "org", | |
| 644 | + client: CLIENT, | |
| 645 | + clientOwner: "org", | |
| 646 | + name: ConnectionName.make("cc"), | |
| 647 | + integration: INTEG, | |
| 648 | + template: TEMPLATE, | |
| 649 | + }); | |
| 650 | + expect(started.status).toBe("connected"); | |
| 651 | + | |
| 652 | + // The discovered scopes are requested and (the token endpoint omitting | |
| 653 | + // `scope`) recorded on the connection. | |
| 654 | + const row = yield* Effect.promise(() => | |
| 655 | + config.db.findFirst("connection", { | |
| 656 | + where: (b) => b("name", "=", "cc"), | |
| 657 | + }), | |
| 658 | + ); | |
| 659 | + expect(row?.oauth_scope).toBe("mcp:read mcp:write"); | |
| 660 | + }), | |
| 661 | + ), | |
| 662 | + ); | |
| 251 | 663 | }); |
| @@ -14,7 +14,7 @@ · 13 unmodified lines | ||
| 14 | 14 | // redeems the session, exchanges the code, and mints the connection. |
| 15 | 15 | // --------------------------------------------------------------------------- |
| 16 | 16 | |
| 17 | −import { Effect, Layer, Option, Schema } from "effect"; | |
| 17 | +import { Duration, Effect, Layer, Option, Schema } from "effect"; | |
| 18 | 18 | import { FetchHttpClient, type HttpClient } from "effect/unstable/http"; |
| 19 | 19 | |
| 20 | 20 | import type { Connection } from "./connection"; |
| @@ -52,6 +52,7 @@ import type { CredentialProvider } from "./provider"; · 31 unmodified lines | ||
| 52 | 52 | import { |
| 53 | 53 | discoverAuthorizationServerMetadata, |
| 54 | 54 | discoverProtectedResourceMetadata, |
| 55 | + OAuthDiscoveryError, | |
| 55 | 56 | registerDynamicClient as registerDynamicClientDcr, |
| 56 | 57 | } from "./oauth-discovery"; |
| 57 | 58 | import { |
| @@ -94,6 +95,15 @@ export interface MintOAuthConnectionInput { · 36 unmodified lines | ||
| 94 | 95 | readonly oauthTokenUrl?: string | null; |
| 95 | 96 | } |
| 96 | 97 | |
| 98 | +/** The OAuth scope policy for a `(integration, template)`. Either the | |
| 99 | + * integration declares the scopes to request (`scopes`, possibly empty — an | |
| 100 | + * empty set requests no scopes), or it declares none and the request scopes | |
| 101 | + * are discovered from the server's metadata at connect (`discover`, used by | |
| 102 | + * MCP). The two are mutually exclusive by construction. */ | |
| 103 | +export type OAuthScopePolicy = | |
| 104 | + | { readonly kind: "scopes"; readonly scopes: readonly string[] } | |
| 105 | + | { readonly kind: "discover" }; | |
| 106 | + | |
| 97 | 107 | /** Everything the OAuth service needs from the executor: fuma access for the |
| 98 | 108 | * owned `oauth_client` / `oauth_session` tables, the default credential |
| 99 | 109 | * provider for minted tokens, a `mintOAuthConnection` callback (writes the |
| @@ -114,23 +124,20 @@ export interface OAuthServiceDeps { · 14 unmodified lines | ||
| 114 | 124 | input: MintOAuthConnectionInput, |
| 115 | 125 | ) => Effect.Effect<Connection, StorageFailure>; |
| 116 | 126 | /** |
| 117 | − * Resolve the integration's DECLARED OAuth scopes for a given | |
| 118 | − * `(integration, template)` — the scopes the integration's auth template | |
| 119 | − * advertises (e.g. an OpenAPI bundle's full authentication-template scope | |
| 120 | − * union), NOT the scopes frozen on a specific `oauth_client` row. | |
| 121 | − * | |
| 122 | − * At connect (`start`) the requested scope set is the UNION of these declared | |
| 123 | − * scopes and the client's configured scopes, so reusing a narrow client on a | |
| 124 | − * broad integration still requests the integration's full scope set. When the | |
| 125 | − * integration declares no OAuth scopes (MCP / DCR integrations discover scopes | |
| 126 | − * from the server; integrations with no declared template scopes) this returns | |
| 127 | − * `[]` and the union collapses to the client's scopes — current behavior, | |
| 128 | − * unchanged. | |
| 127 | + * Resolve the OAuth scope policy for a `(integration, template)`: | |
| 128 | + * - `{ kind: "scopes", scopes }`: the scopes the integration's auth template | |
| 129 | + * DECLARES (e.g. an OpenAPI bundle's authentication-template scope union), | |
| 130 | + * NOT the scopes frozen on a specific `oauth_client` row. These are | |
| 131 | + * requested verbatim at connect (`start`); an empty set requests none. | |
| 132 | + * - `{ kind: "discover" }`: the integration declares no scopes, so `start` | |
| 133 | + * discovers the request scopes from the server's RFC 9728 / RFC 8414 | |
| 134 | + * metadata. Used by server-targeting integrations (MCP) whose scopes live | |
| 135 | + * on the server rather than in a template. | |
| 129 | 136 | */ |
| 130 | − readonly resolveDeclaredOAuthScopes: ( | |
| 137 | + readonly resolveOAuthScopePolicy: ( | |
| 131 | 138 | integration: IntegrationSlug, |
| 132 | 139 | template: AuthTemplateSlug, |
| 133 | − ) => Effect.Effect<readonly string[], StorageFailure>; | |
| 140 | + ) => Effect.Effect<OAuthScopePolicy, StorageFailure>; | |
| 134 | 141 | readonly httpClientLayer?: Layer.Layer<HttpClient.HttpClient>; |
| 135 | 142 | readonly fetch?: typeof globalThis.fetch; |
| 136 | 143 | readonly endpointUrlPolicy?: OAuthEndpointUrlPolicy; |
| @@ -169,9 +176,7 @@ const accessItemId = (owner: Owner, integration: IntegrationSlug, name: Connecti · 32 unmodified lines | ||
| 169 | 176 | `oauth:${owner}:${integration}:${name}`; |
| 170 | 177 | const refreshItemIdFor = (accessId: string): string => `${accessId}:refresh`; |
| 171 | 178 | |
| 172 | −/** Order-preserving de-duplication of a scope list. The requested scope set is | |
| 173 | − * the integration's DECLARED scopes — the integration is the sole source of what | |
| 174 | − * to request; the OAuth app no longer carries a scope set. */ | |
| 179 | +/** Order-preserving de-duplication of a scope list. */ | |
| 175 | 180 | const dedupeScopes = (scopes: readonly string[]): readonly string[] => [...new Set(scopes)]; |
| 176 | 181 | |
| 177 | 182 | const recordedOAuthScope = ( |
| @@ -349,6 +354,76 @@ export const makeOAuthService = (deps: OAuthServiceDeps): OAuthService => { · 171 unmodified lines | ||
| 349 | 354 | // callback; redirect-requiring flows fail loudly via `requireRedirectUri`. |
| 350 | 355 | const redirectUri = deps.redirectUri; |
| 351 | 356 | |
| 357 | + // Caps on server-controlled discovery input — a hostile or buggy server must | |
| 358 | + // not be able to hang `oauth.start` or overflow the authorize URL. | |
| 359 | + const MAX_DISCOVERY_AUTH_SERVERS = 3; // AS-failover lists are tiny in practice | |
| 360 | + const MAX_DISCOVERED_SCOPES = 100; // far beyond any realistic authorization template | |
| 361 | + const capScopes = (scopes: readonly string[]): readonly string[] => | |
| 362 | + dedupeScopes(scopes).slice(0, MAX_DISCOVERED_SCOPES); | |
| 363 | + | |
| 364 | + // Discover the scopes to request when the integration declares none — only | |
| 365 | + // reached for integrations that opt in (MCP-style). The resource's own RFC | |
| 366 | + // 9728 `scopes_supported` is authoritative when present, even when empty (§2 | |
| 367 | + // defines the field; §7.2 cautions against requesting more than it lists). | |
| 368 | + // Only when the resource is SILENT do we read the scopes advertised by the | |
| 369 | + // authorization servers it NAMES (RFC 8414) — we never probe arbitrary URLs. | |
| 370 | + const discoverScopesForResource = ( | |
| 371 | + resource: string | null, | |
| 372 | + ): Effect.Effect<readonly string[], OAuthDiscoveryError> => | |
| 373 | + Effect.gen(function* () { | |
| 374 | + if (resource == null) { | |
| 375 | + return yield* new OAuthDiscoveryError({ | |
| 376 | + message: "Cannot discover OAuth scopes: the client has no resource configured", | |
| 377 | + }); | |
| 378 | + } | |
| 379 | + // `httpClientLayer` flows through `options` so discovery uses the host's | |
| 380 | + // configured client (discovery self-provides from `options.httpClientLayer`). | |
| 381 | + const discoveryOptions = { endpointUrlPolicy: deps.endpointUrlPolicy, httpClientLayer }; | |
| 382 | + | |
| 383 | + const protectedResource = yield* discoverProtectedResourceMetadata( | |
| 384 | + resource, | |
| 385 | + discoveryOptions, | |
| 386 | + ); | |
| 387 | + const resourceScopes = protectedResource?.metadata.scopes_supported; | |
| 388 | + if (resourceScopes !== undefined) return capScopes(resourceScopes); | |
| 389 | + | |
| 390 | + // The resource is silent on scopes — read them from the authorization | |
| 391 | + // servers it names, in order. An advertised list is authoritative even | |
| 392 | + // when empty. Any AS we cannot read clean RFC 8414 metadata from — | |
| 393 | + // unreachable, 404, malformed, or issuer-mismatched — contributes nothing | |
| 394 | + // and we move on (mirroring the dynamic-registration discovery path); if | |
| 395 | + // none advertise scopes we request none and let the AS apply its defaults | |
| 396 | + // (RFC 8414 metadata is optional, so its absence is not a failure). The | |
| 397 | + // list is server-controlled, so cap how many of its hosts we probe. | |
| 398 | + for (const issuer of (protectedResource?.metadata.authorization_servers ?? []).slice( | |
| 399 | + 0, | |
| 400 | + MAX_DISCOVERY_AUTH_SERVERS, | |
| 401 | + )) { | |
| 402 | + const authServer = yield* discoverAuthorizationServerMetadata( | |
| 403 | + issuer, | |
| 404 | + discoveryOptions, | |
| 405 | + ).pipe(Effect.catchTag("OAuthDiscoveryError", () => Effect.succeed(null))); | |
| 406 | + const scopes = authServer?.metadata.scopes_supported; | |
| 407 | + if (scopes !== undefined) return capScopes(scopes); | |
| 408 | + } | |
| 409 | + | |
| 410 | + return []; | |
| 411 | + }).pipe( | |
| 412 | + // Bound the whole sequence (PRM + up to MAX_DISCOVERY_AUTH_SERVERS AS | |
| 413 | + // fetches, each with its own request timeout). 30s is larger than a single | |
| 414 | + // request timeout so it bounds the sequence, not a slow-but-valid request. | |
| 415 | + Effect.timeoutOrElse({ | |
| 416 | + duration: Duration.seconds(30), | |
| 417 | + orElse: () => | |
| 418 | + Effect.fail( | |
| 419 | + new OAuthDiscoveryError({ | |
| 420 | + message: "OAuth scope discovery timed out", | |
| 421 | + cause: "timeout", | |
| 422 | + }), | |
| 423 | + ), | |
| 424 | + }), | |
| 425 | + ); | |
| 426 | + | |
| 352 | 427 | // ----------------------------------------------------------------------- |
| 353 | 428 | // createClient — write the oauth_client row. |
| 354 | 429 | // ----------------------------------------------------------------------- |
| @@ -656,22 +731,32 @@ export const makeOAuthService = (deps: OAuthServiceDeps): OAuthService => { · 301 unmodified lines | ||
| 656 | 731 | }); |
| 657 | 732 | } |
| 658 | 733 | |
| 659 | − // The INTEGRATION is the sole source of what to request — its declared auth | |
| 660 | − // scopes (driven by the integration's tools, surfaced via the declared auth | |
| 661 | − // method). The OAuth app no longer carries a scope set, so there is no union | |
| 662 | − // to compute and no way to over-request from a stale client copy. | |
| 663 | − const declaredScopes = yield* deps | |
| 664 | − .resolveDeclaredOAuthScopes(input.integration, input.template) | |
| 734 | + // Declared scopes win (driven by the selected auth template). MCP-style | |
| 735 | + // integrations declare none and discover them from the client's protected | |
| 736 | + // resource / authorization server metadata at connect. | |
| 737 | + const scopePolicy = yield* deps | |
| 738 | + .resolveOAuthScopePolicy(input.integration, input.template) | |
| 665 | 739 | .pipe( |
| 666 | 740 | Effect.mapError( |
| 667 | 741 | (cause) => |
| 668 | 742 | new OAuthStartError({ |
| 669 | 743 | // oxlint-disable-next-line executor/no-unknown-error-message -- boundary: StorageFailure carries a typed `message` field |
| 670 | − message: `Failed to resolve declared OAuth scopes: ${cause.message}`, | |
| 744 | + message: `Failed to resolve OAuth scope policy: ${cause.message}`, | |
| 671 | 745 | }), |
| 672 | 746 | ), |
| 673 | 747 | ); |
| 674 | − const requestedScopes = dedupeScopes(declaredScopes); | |
| 748 | + const requestedScopes = | |
| 749 | + scopePolicy.kind === "discover" | |
| 750 | + ? yield* discoverScopesForResource(client.resource).pipe( | |
| 751 | + Effect.mapError( | |
| 752 | + (cause) => | |
| 753 | + new OAuthStartError({ | |
| 754 | + // oxlint-disable-next-line executor/no-unknown-error-message -- boundary: OAuthDiscoveryError carries a typed `message` field | |
| 755 | + message: `Failed to discover OAuth scopes: ${cause.message}`, | |
| 756 | + }), | |
| 757 | + ), | |
| 758 | + ) | |
| 759 | + : dedupeScopes(scopePolicy.scopes); | |
| 675 | 760 | |
| 676 | 761 | // client_credentials: exchange immediately and mint the connection. |
| 677 | 762 | if (client.grant === "client_credentials") { |
| @@ -746,10 +831,10 @@ export const makeOAuthService = (deps: OAuthServiceDeps): OAuthService => { · 68 unmodified lines | ||
| 746 | 831 | redirect_url: flowRedirectUri, |
| 747 | 832 | pkce_verifier: verifier, |
| 748 | 833 | identity_label: input.identityLabel ?? null, |
| 749 | − // Persist the requested scope set (declared ∪ client) so `complete`'s | |
| 750 | − // recorded-scope fallback reflects exactly what was requested when the | |
| 751 | − // AS omits `scope`, without re-resolving the integration's declared | |
| 752 | − // scopes at completion. | |
| 834 | + // Persist the requested scope set (the integration's declared or | |
| 835 | + // discovered scopes) so `complete`'s recorded-scope fallback reflects | |
| 836 | + // exactly what was requested when the AS omits `scope`, without | |
| 837 | + // re-resolving it at completion. | |
| 753 | 838 | payload: { owner: input.owner, clientOwner: input.clientOwner, requestedScopes }, |
| 754 | 839 | expires_at: expiresAt, |
| 755 | 840 | created_at: now, |
| @@ -807,9 +892,10 @@ export const makeOAuthService = (deps: OAuthServiceDeps): OAuthService => { · 51 unmodified lines | ||
| 807 | 892 | pkceVerifier: sessionRow.pkce_verifier == null ? null : String(sessionRow.pkce_verifier), |
| 808 | 893 | identityLabel: sessionRow.identity_label == null ? null : String(sessionRow.identity_label), |
| 809 | 894 | expiresAt: Number(sessionRow.expires_at), |
| 810 | − // The scope set `start` requested (declared ∪ client), persisted on the | |
| 811 | − // session payload. Drives the recorded-scope fallback when the AS omits | |
| 812 | − // `scope`. Missing/legacy payloads fall back to the client's scopes below. | |
| 895 | + // The scope set `start` requested (the integration's declared or | |
| 896 | + // discovered scopes), persisted on the session payload. Drives the | |
| 897 | + // recorded-scope fallback when the AS omits `scope`. Missing/legacy | |
| 898 | + // payloads fall back to the client's scopes below. | |
| 813 | 899 | requestedScopes: requestedScopesFromPayload(sessionRow.payload), |
| 814 | 900 | // The app's owner, recorded by `start` — reload the SAME app at |
| 815 | 901 | // completion by explicit owner (no derivation). Defaults to the session |
| @@ -922,8 +1008,9 @@ export const makeOAuthService = (deps: OAuthServiceDeps): OAuthService => { · 106 unmodified lines | ||
| 922 | 1008 | }, |
| 923 | 1009 | client: LoadedOAuthClient, |
| 924 | 1010 | token: OAuth2TokenResponse, |
| 925 | − /** The scope set requested at /authorize + /token (declared ∪ client) — | |
| 926 | − * the recorded-scope fallback when the AS omits `scope`. */ | |
| 1011 | + /** The scope set requested at /authorize + /token (the integration's | |
| 1012 | + * declared or discovered scopes) — the recorded-scope fallback when the AS | |
| 1013 | + * omits `scope`. */ | |
| 927 | 1014 | requestedScopes: readonly string[], |
| 928 | 1015 | /** The owner of `client` — persisted so refresh loads it by explicit owner. */ |
| 929 | 1016 | clientOwner: Owner, |
| @@ -1019,7 +1106,13 @@ export const makeOAuthService = (deps: OAuthServiceDeps): OAuthService => { · 89 unmodified lines | ||
| 1019 | 1106 | authorizationUrl: as.metadata.authorization_endpoint, |
| 1020 | 1107 | tokenUrl: as.metadata.token_endpoint, |
| 1021 | 1108 | resource: resource?.metadata.resource ?? null, |
| 1022 | − scopesSupported: as.metadata.scopes_supported, | |
| 1109 | + // Prefer the resource's own RFC 9728 scopes (authoritative, even when | |
| 1110 | + // empty); fall back to the authorization server's list only when PRM is | |
| 1111 | + // silent. For a spec-compliant MCP server (one that publishes PRM) this | |
| 1112 | + // matches what `oauth.start` discovers. The AS fallback is a best-effort | |
| 1113 | + // hint for the registration form on servers that omit PRM — where | |
| 1114 | + // `oauth.start` requests none — so the two can differ for those. | |
| 1115 | + scopesSupported: resource?.metadata.scopes_supported ?? as.metadata.scopes_supported, | |
| 1023 | 1116 | registrationEndpoint: as.metadata.registration_endpoint ?? null, |
| 1024 | 1117 | tokenEndpointAuthMethodsSupported: as.metadata.token_endpoint_auth_methods_supported, |
| 1025 | 1118 | } satisfies OAuthProbeResult; |
| @@ -23,6 +23,7 @@ export { · 22 unmodified lines | ||
| 23 | 23 | export { |
| 24 | 24 | OAuthTestServer, |
| 25 | 25 | serveOAuthTestServer, |
| 26 | + scopesFromAuthorizeUrl, | |
| 26 | 27 | OAuthTestServerAddressError, |
| 27 | 28 | OAuthTestServerFlowError, |
| 28 | 29 | type OAuthAuthorizationCompletion, |
| @@ -389,6 +389,13 @@ const completeAuthorizationCodeTokenFlow = · 388 unmodified lines | ||
| 389 | 389 | }; |
| 390 | 390 | }); |
| 391 | 391 | |
| 392 | +/** Parse the `scope` query param from an authorize URL into an ordered list | |
| 393 | + * (empty when the parameter is absent or blank). */ | |
| 394 | +export const scopesFromAuthorizeUrl = (authorizationUrl: string): readonly string[] => { | |
| 395 | + const raw = new URL(authorizationUrl).searchParams.get("scope"); | |
| 396 | + return raw == null || raw.length === 0 ? [] : raw.split(" "); | |
| 397 | +}; | |
| 398 | + | |
| 392 | 399 | export const serveOAuthTestServer = ( |
| 393 | 400 | options: OAuthTestServerOptions = {}, |
| 394 | 401 | ): Effect.Effect<OAuthTestServerShape, OAuthTestServerAddressError, Scope.Scope> => |
| @@ -13,7 +13,7 @@ describe("mcpAuthMethodInputFromEditorValue", () => { · 12 unmodified lines | ||
| 13 | 13 | expect(mcpAuthMethodInputFromEditorValue({ kind: "none" })).toEqual({ kind: "none" }); |
| 14 | 14 | }); |
| 15 | 15 | |
| 16 | − it("maps 'oauth' → { kind: 'oauth2' } (endpoints are resolved at connect time)", () => { | |
| 16 | + it("maps 'oauth' → { kind: 'oauth2' } (endpoints/scopes are resolved at connect time)", () => { | |
| 17 | 17 | const value: AuthTemplateEditorValue = { |
| 18 | 18 | kind: "oauth", |
| 19 | 19 | authorizationUrl: "https://a.example.com/auth", |
| @@ -108,8 +108,13 @@ describe("editorValueFromMcpAuthMethod", () => { · 88 unmodified lines | ||
| 108 | 108 | }); |
| 109 | 109 | }); |
| 110 | 110 | |
| 111 | − it("maps oauth2 to an oauth editor value with no endpoints (discovered at connect)", () => { | |
| 112 | − expect(editorValueFromMcpAuthMethod({ slug: "oauth2", kind: "oauth2" })).toEqual({ | |
| 111 | + it("maps oauth2 to an oauth editor value with no endpoints or scopes (discovered at connect)", () => { | |
| 112 | + expect( | |
| 113 | + editorValueFromMcpAuthMethod({ | |
| 114 | + slug: "oauth2", | |
| 115 | + kind: "oauth2", | |
| 116 | + }), | |
| 117 | + ).toEqual({ | |
| 113 | 118 | kind: "oauth", |
| 114 | 119 | authorizationUrl: "", |
| 115 | 120 | tokenUrl: "", |
| @@ -146,6 +151,7 @@ describe("authMethodsFromConfig", () => { · 30 unmodified lines | ||
| 146 | 151 | { id: "none", kind: "none", source: "spec", template: "none" }, |
| 147 | 152 | ]); |
| 148 | 153 | expect(methods[0]?.oauth?.discoveryUrl).toBe("https://mcp.example.com/mcp"); |
| 154 | + expect(methods[0]?.oauth?.scopes).toBeUndefined(); | |
| 149 | 155 | }); |
| 150 | 156 | |
| 151 | 157 | it("carries multi-placement methods through to the hub", () => { |
| @@ -18,6 +18,7 @@ import { · 17 unmodified lines | ||
| 18 | 18 | import { |
| 19 | 19 | makeTestConfig, |
| 20 | 20 | memoryCredentialsPlugin, |
| 21 | + scopesFromAuthorizeUrl, | |
| 21 | 22 | serveOAuthTestServer, |
| 22 | 23 | serveTestHttpApp, |
| 23 | 24 | } from "@executor-js/sdk/testing"; |
| @@ -502,6 +503,52 @@ describe("mcpPlugin", () => { · 478 unmodified lines | ||
| 502 | 503 | }), |
| 503 | 504 | ); |
| 504 | 505 | |
| 506 | + it.effect("oauth.start discovers scopes for an MCP oauth method", () => | |
| 507 | + Effect.scoped( | |
| 508 | + Effect.gen(function* () { | |
| 509 | + const server = yield* serveOAuthTestServer({ | |
| 510 | + scopes: ["channels:history", "users:read"], | |
| 511 | + }); | |
| 512 | + const executor = yield* createExecutor( | |
| 513 | + makeTestConfig({ plugins: [memoryCredentialsPlugin(), mcpPlugin()] as const }), | |
| 514 | + ); | |
| 515 | + | |
| 516 | + yield* executor.mcp.addServer({ | |
| 517 | + name: "Slack MCP", | |
| 518 | + endpoint: server.mcpResourceUrl, | |
| 519 | + slug: "slack_mcp", | |
| 520 | + authenticationTemplate: [{ kind: "oauth2" }], | |
| 521 | + }); | |
| 522 | + yield* executor.oauth.createClient({ | |
| 523 | + owner: "org", | |
| 524 | + slug: OAuthClientSlug.make("slack-app"), | |
| 525 | + authorizationUrl: server.authorizationEndpoint, | |
| 526 | + tokenUrl: server.tokenEndpoint, | |
| 527 | + grant: "authorization_code", | |
| 528 | + clientId: "test-client", | |
| 529 | + clientSecret: "test-secret", | |
| 530 | + resource: server.mcpResourceUrl, | |
| 531 | + }); | |
| 532 | + | |
| 533 | + const started = yield* executor.oauth.start({ | |
| 534 | + owner: "org", | |
| 535 | + client: OAuthClientSlug.make("slack-app"), | |
| 536 | + clientOwner: "org", | |
| 537 | + name: ConnectionName.make("main"), | |
| 538 | + integration: IntegrationSlug.make("slack_mcp"), | |
| 539 | + template: AuthTemplateSlug.make("oauth2"), | |
| 540 | + }); | |
| 541 | + | |
| 542 | + expect(started.status).toBe("redirect"); | |
| 543 | + if (started.status !== "redirect") return; | |
| 544 | + expect(scopesFromAuthorizeUrl(started.authorizationUrl)).toEqual([ | |
| 545 | + "channels:history", | |
| 546 | + "users:read", | |
| 547 | + ]); | |
| 548 | + }), | |
| 549 | + ), | |
| 550 | + ); | |
| 551 | + | |
| 505 | 552 | // When discovery fails (auth, network, etc.) the connection still lands with |
| 506 | 553 | // an empty tool set so the user can retry via `connections.refresh` once they |
| 507 | 554 | // fix the underlying problem. |
| @@ -527,9 +527,10 @@ const buildConnectorInput = ( · 526 unmodified lines | ||
| 527 | 527 | // stdio → [] (no remote connection to configure) |
| 528 | 528 | // apikey → carried placements (headers / query params) verbatim |
| 529 | 529 | // oauth2 → an oauth method carrying the MCP endpoint to probe |
| 530 | −// (`discoveryUrl`); endpoints are discovered live at | |
| 531 | −// connect time, so they are NOT pre-resolved here. We | |
| 532 | −// mark `supportsDynamicRegistration: true` because MCP | |
| 530 | +// (`discoveryUrl`). Endpoints/scopes are discovered | |
| 531 | +// live at connect time, so they are NOT pre-resolved | |
| 532 | +// here. We mark | |
| 533 | +// `supportsDynamicRegistration: true` because MCP | |
| 533 | 534 | // OAuth servers are expected to support RFC 7591 DCR; |
| 534 | 535 | // the connect flow probes to confirm and falls back. |
| 535 | 536 | // --------------------------------------------------------------------------- |
| @@ -278,6 +278,7 @@ describe("runDcrConnect", () => { · 277 unmodified lines | ||
| 278 | 278 | }, |
| 279 | 279 | { |
| 280 | 280 | discoveryUrl: "https://mcp.example.com/mcp", |
| 281 | + resourceFallback: "https://mcp.example.com/mcp", | |
| 281 | 282 | owner: "user", |
| 282 | 283 | integrationName: "App", |
| 283 | 284 | existingSlugs: [], |
| @@ -287,6 +288,42 @@ describe("runDcrConnect", () => { · 3 unmodified lines | ||
| 287 | 288 | ); |
| 288 | 289 | expect(outcome.kind).toBe("started"); |
| 289 | 290 | expect(registerArgs!.scopes).toEqual(["declared.scope"]); |
| 291 | + // PRM named no `resource`, so the genuine discovery URL (MCP) seeds the | |
| 292 | + // RFC 8707 resource indicator. | |
| 293 | + expect(registerArgs!.resource).toBe("https://mcp.example.com/mcp"); | |
| 294 | + }); | |
| 295 | + | |
| 296 | + it("never seeds the resource indicator from the token endpoint (non-MCP DCR)", async () => { | |
| 297 | + // A DCR integration with no discovery URL: `discoveryUrl` falls back to the | |
| 298 | + // token endpoint for probing, but the token endpoint is not an RFC 8707 | |
| 299 | + // resource identifier, so it must NOT become the resource. With no genuine | |
| 300 | + // discovery URL and no PRM-named resource, the indicator stays null. | |
| 301 | + let registerArgs: RegisterArgs | null = null; | |
| 302 | + const outcome = await runDcrConnect( | |
| 303 | + { | |
| 304 | + probe: (): Promise<ProbeResult | null> => | |
| 305 | + Promise.resolve({ | |
| 306 | + authorizationUrl: "https://auth.example.com/authorize", | |
| 307 | + tokenUrl: "https://auth.example.com/token", | |
| 308 | + registrationEndpoint: "https://auth.example.com/register", | |
| 309 | + }), | |
| 310 | + register: (args: RegisterArgs): Promise<OAuthClientSlug | null> => { | |
| 311 | + registerArgs = args; | |
| 312 | + return Promise.resolve(OAuthClientSlug.make("app")); | |
| 313 | + }, | |
| 314 | + start: (): void => {}, | |
| 315 | + }, | |
| 316 | + { | |
| 317 | + // discoveryUrl collapsed to the token endpoint (no real discovery URL). | |
| 318 | + discoveryUrl: "https://auth.example.com/token", | |
| 319 | + owner: "user", | |
| 320 | + integrationName: "App", | |
| 321 | + existingSlugs: [], | |
| 322 | + integration: TEST_INTEGRATION, | |
| 323 | + }, | |
| 324 | + ); | |
| 325 | + expect(outcome.kind).toBe("started"); | |
| 326 | + expect(registerArgs!.resource).toBeNull(); | |
| 290 | 327 | }); |
| 291 | 328 | |
| 292 | 329 | it("falls back to BYO when there is no registration endpoint (no register/start)", async () => { |
| @@ -317,9 +354,13 @@ describe("runDcrConnect", () => { · 24 unmodified lines | ||
| 317 | 354 | integration: TEST_INTEGRATION, |
| 318 | 355 | }, |
| 319 | 356 | ); |
| 320 | − expect(outcome).toEqual({ | |
| 357 | + expect(outcome).toMatchObject({ | |
| 321 | 358 | kind: "fallback", |
| 322 | 359 | reason: "no-registration-endpoint", |
| 360 | + probe: { | |
| 361 | + authorizationUrl: "https://auth.example.com/authorize", | |
| 362 | + tokenUrl: "https://auth.example.com/token", | |
| 363 | + }, | |
| 323 | 364 | }); |
| 324 | 365 | expect(calls).toEqual(["probe"]); |
| 325 | 366 | }); |
| @@ -378,7 +419,14 @@ describe("runDcrConnect", () => { · 52 unmodified lines | ||
| 378 | 419 | integration: TEST_INTEGRATION, |
| 379 | 420 | }, |
| 380 | 421 | ); |
| 381 | − expect(outcome.kind).toBe("fallback"); | |
| 422 | + expect(outcome).toMatchObject({ | |
| 423 | + kind: "fallback", | |
| 424 | + reason: "registration-failed", | |
| 425 | + probe: { | |
| 426 | + authorizationUrl: "https://auth.example.com/authorize", | |
| 427 | + tokenUrl: "https://auth.example.com/token", | |
| 428 | + }, | |
| 429 | + }); | |
| 382 | 430 | expect(calls).toEqual(["register"]); |
| 383 | 431 | }); |
| 384 | 432 | }); |
| @@ -48,7 +48,11 @@ import { · 47 unmodified lines | ||
| 48 | 48 | } from "../plugins/use-effective-oauth-client"; |
| 49 | 49 | import { cn } from "../lib/utils"; |
| 50 | 50 | import { buildUsageMap, connectionsUsingClient } from "../lib/oauth-client-usage"; |
| 51 | −import { OAuthClientForm, type OAuthClientFormPrefill } from "./oauth-client-form"; | |
| 51 | +import { | |
| 52 | + OAuthClientForm, | |
| 53 | + registrationScopes, | |
| 54 | + type OAuthClientFormPrefill, | |
| 55 | +} from "./oauth-client-form"; | |
| 52 | 56 | import { RemoveOAuthAppDialog } from "./remove-oauth-app-dialog"; |
| 53 | 57 | import { AddCustomMethodForm, type CreateCustomMethod } from "./add-custom-method-modal"; |
| 54 | 58 | import { PlacementLine, type AuthMethod } from "../lib/auth-placements"; |
| @@ -479,13 +483,17 @@ type DcrStartArgs = { · 424 unmodified lines | ||
| 479 | 483 | |
| 480 | 484 | /** Outcome of the DCR orchestration. `"started"` means the OAuth flow handed |
| 481 | 485 | * off (the popup/inline start ran); `"fallback"` means we could not auto-set-up |
| 482 | − * (probe failed, or no registration endpoint) and the caller should fall back | |
| 483 | − * to the bring-your-own-app picker. */ | |
| 486 | + * — a failed probe, no registration endpoint, or a failed registration — and | |
| 487 | + * the caller should fall back to the bring-your-own-app picker. A failed probe | |
| 488 | + * carries no probe result; the other two reasons always carry the probe that | |
| 489 | + * seeds the picker. */ | |
| 484 | 490 | type DcrOutcome = |
| 485 | 491 | | { readonly kind: "started" } |
| 492 | + | { readonly kind: "fallback"; readonly reason: "probe-failed" } | |
| 486 | 493 | | { |
| 487 | 494 | readonly kind: "fallback"; |
| 488 | − readonly reason: "probe-failed" | "no-registration-endpoint"; | |
| 495 | + readonly reason: "no-registration-endpoint" | "registration-failed"; | |
| 496 | + readonly probe: DcrProbeResult; | |
| 489 | 497 | }; |
| 490 | 498 | |
| 491 | 499 | type RunDcrConnectDeps = { |
| @@ -500,6 +508,13 @@ type RunDcrConnectDeps = { · 8 unmodified lines | ||
| 500 | 508 | |
| 501 | 509 | type RunDcrConnectInput = { |
| 502 | 510 | readonly discoveryUrl: string; |
| 511 | + /** The integration's genuine protected-resource URL (the MCP discovery URL), | |
| 512 | + * used as the RFC 8707 resource indicator when the server's PRM names no | |
| 513 | + * `resource`. Distinct from `discoveryUrl`, which falls back to the token | |
| 514 | + * endpoint for probing: the token endpoint is NOT a resource identifier, so | |
| 515 | + * it must never become the indicator. Undefined for integrations with no | |
| 516 | + * discovery URL (non-MCP DCR), collapsing the resource to null as before. */ | |
| 517 | + readonly resourceFallback?: string; | |
| 503 | 518 | readonly owner: Owner; |
| 504 | 519 | readonly integrationName: string; |
| 505 | 520 | /** The owner's existing client slugs, so the minted slug stays unique. */ |
| @@ -521,9 +536,8 @@ export const dcrClientNameForIntegration = (integrationName: string): string => · 15 unmodified lines | ||
| 521 | 536 | * Run the transparent DCR connect sequence: probe → register → start. |
| 522 | 537 | * |
| 523 | 538 | * - Probe failure → `{ kind: "fallback", reason: "probe-failed" }` (caller shows BYO). |
| 524 | − * - No registration endpoint → `{ kind: "fallback", reason: "no-registration-endpoint" }`. | |
| 525 | − * - Register failure → throws via the injected `register` rejecting; the caller | |
| 526 | − * treats a thrown/rejected register as fallback (kept out of the happy path). | |
| 539 | + * - No registration endpoint → `{ kind: "fallback", reason: "no-registration-endpoint", probe }`. | |
| 540 | + * - Registration failure → `{ kind: "fallback", reason: "registration-failed", probe }`. | |
| 527 | 541 | * - Success → registers, calls `start`, returns `{ kind: "started" }`. |
| 528 | 542 | */ |
| 529 | 543 | export async function runDcrConnect( |
| @@ -533,27 +547,24 @@ export async function runDcrConnect( · 3 unmodified lines | ||
| 533 | 547 | const probe = await deps.probe(input.discoveryUrl); |
| 534 | 548 | if (probe === null) return { kind: "fallback", reason: "probe-failed" }; |
| 535 | 549 | const registrationEndpoint = probe.registrationEndpoint; |
| 536 | − if (!registrationEndpoint) return { kind: "fallback", reason: "no-registration-endpoint" }; | |
| 550 | + if (!registrationEndpoint) return { kind: "fallback", reason: "no-registration-endpoint", probe }; | |
| 537 | 551 | |
| 538 | 552 | const slug = uniqueClientSlug(input.integrationName, input.existingSlugs); |
| 539 | − const scopes = | |
| 540 | − input.declaredScopes && input.declaredScopes.length > 0 | |
| 541 | − ? input.declaredScopes | |
| 542 | − : (probe.scopesSupported ?? []); | |
| 553 | + const scopes = registrationScopes(input.declaredScopes ?? [], probe.scopesSupported ?? []); | |
| 543 | 554 | const minted = await deps.register({ |
| 544 | 555 | owner: input.owner, |
| 545 | 556 | slug, |
| 546 | 557 | registrationEndpoint, |
| 547 | 558 | authorizationUrl: probe.authorizationUrl, |
| 548 | 559 | tokenUrl: probe.tokenUrl, |
| 549 | − resource: probe.resource ?? null, | |
| 560 | + resource: probe.resource ?? input.resourceFallback ?? null, | |
| 550 | 561 | scopes, |
| 551 | 562 | tokenEndpointAuthMethodsSupported: probe.tokenEndpointAuthMethodsSupported, |
| 552 | 563 | clientName: dcrClientNameForIntegration(input.integrationName), |
| 553 | 564 | redirectUri: input.redirectUri, |
| 554 | 565 | originIntegration: input.integration, |
| 555 | 566 | }); |
| 556 | − if (minted === null) return { kind: "fallback", reason: "probe-failed" }; | |
| 567 | + if (minted === null) return { kind: "fallback", reason: "registration-failed", probe }; | |
| 557 | 568 | deps.start({ client: minted, owner: input.owner }); |
| 558 | 569 | return { kind: "started" }; |
| 559 | 570 | } |
| @@ -704,6 +715,7 @@ function AddAccountModalView(props: AddAccountModalProps) { · 144 unmodified lines | ||
| 704 | 715 | // the modal to the bring-your-own-app picker if auto setup is unavailable. |
| 705 | 716 | const [dcrBusy, setDcrBusy] = useState(false); |
| 706 | 717 | const [dcrFailed, setDcrFailed] = useState(false); |
| 718 | + const [oauthFallbackProbe, setOAuthFallbackProbe] = useState<DcrProbeResult | null>(null); | |
| 707 | 719 | // FIX 3 escape hatch: when no registered app matched the integration's |
| 708 | 720 | // endpoints, the unmatched apps are collapsed behind an opt-in expander. |
| 709 | 721 | const [showOtherApps, setShowOtherApps] = useState(false); |
| @@ -768,6 +780,7 @@ function AddAccountModalView(props: AddAccountModalProps) { · 58 unmodified lines | ||
| 768 | 780 | setCredentialOrigin("paste"); |
| 769 | 781 | setOnePasswordItemId(""); |
| 770 | 782 | setPickedApp(null); |
| 783 | + setOAuthFallbackProbe(null); | |
| 771 | 784 | setDcrFailed(false); |
| 772 | 785 | }, [initialState, allMethods, defaultOwner, ownerOptions]); |
| 773 | 786 | |
| @@ -859,8 +872,14 @@ function AddAccountModalView(props: AddAccountModalProps) { · 85 unmodified lines | ||
| 859 | 872 | endpointMatched: oauthEndpointMatched, |
| 860 | 873 | displayRegisterCTA: oauthDisplayRegisterCTA, |
| 861 | 874 | } = useOAuthClientsForIntegration({ |
| 862 | − tokenUrl: method?.oauth?.tokenUrl, | |
| 863 | − authorizationUrl: method?.oauth?.authorizationUrl, | |
| 875 | + // MCP integrations declare no endpoints up front; once DCR has probed the | |
| 876 | + // server we match registered apps against the DISCOVERED endpoints. With no | |
| 877 | + // endpoints to match (a server-targeting MCP integration), require an | |
| 878 | + // explicit match so we surface the register CTA instead of auto-selecting an | |
| 879 | + // unrelated provider's app. | |
| 880 | + tokenUrl: method?.oauth?.tokenUrl ?? oauthFallbackProbe?.tokenUrl, | |
| 881 | + authorizationUrl: method?.oauth?.authorizationUrl ?? oauthFallbackProbe?.authorizationUrl, | |
| 882 | + requireEndpointMatch: isDcr, | |
| 864 | 883 | }); |
| 865 | 884 | const oauthPopup = useOAuthPopupFlow({ |
| 866 | 885 | popupName: "add-account-oauth", |
| @@ -941,6 +960,8 @@ function AddAccountModalView(props: AddAccountModalProps) { · 74 unmodified lines | ||
| 941 | 960 | setValues({}); |
| 942 | 961 | setCredentialOrigin("paste"); |
| 943 | 962 | setOnePasswordItemId(""); |
| 963 | + setDcrFailed(false); | |
| 964 | + setOAuthFallbackProbe(null); | |
| 944 | 965 | }; |
| 945 | 966 | |
| 946 | 967 | // A just-created custom method joins the in-session list and is auto-selected |
| @@ -1111,8 +1132,9 @@ function AddAccountModalView(props: AddAccountModalProps) { · 164 unmodified lines | ||
| 1111 | 1132 | }; |
| 1112 | 1133 | |
| 1113 | 1134 | // Transparent DCR connect: probe → register → start, no app picker. On any |
| 1114 | − // failure (probe error or no registration endpoint) we flip `dcrFailed` so the | |
| 1115 | − // bring-your-own-app picker renders as the recovery path with name/owner kept. | |
| 1135 | + // failure (probe error, no registration endpoint, or registration failure) we | |
| 1136 | + // flip `dcrFailed` so the bring-your-own-app picker renders as the recovery | |
| 1137 | + // path with name/owner kept. | |
| 1116 | 1138 | const handleDcrConnect = async () => { |
| 1117 | 1139 | const discoveryUrl = method?.oauth?.discoveryUrl ?? method?.oauth?.tokenUrl; |
| 1118 | 1140 | if (!method || !discoveryUrl) { |
| @@ -1172,6 +1194,10 @@ function AddAccountModalView(props: AddAccountModalProps) { · 53 unmodified lines | ||
| 1172 | 1194 | }, |
| 1173 | 1195 | { |
| 1174 | 1196 | discoveryUrl, |
| 1197 | + // Only a genuine discovery URL (MCP) seeds the RFC 8707 resource | |
| 1198 | + // indicator; the token-endpoint fallback baked into `discoveryUrl` must | |
| 1199 | + // not, so pass the un-collapsed method value here. | |
| 1200 | + resourceFallback: method.oauth?.discoveryUrl, | |
| 1175 | 1201 | owner: dcrOwner, |
| 1176 | 1202 | integrationName, |
| 1177 | 1203 | existingSlugs: [...oauthApps, ...oauthOtherApps].map((app: OAuthClientOption) => |
| @@ -1191,6 +1217,7 @@ function AddAccountModalView(props: AddAccountModalProps) { · 13 unmodified lines | ||
| 1191 | 1217 | ...(outcome.kind === "fallback" ? { dcr_fallback: true } : {}), |
| 1192 | 1218 | }); |
| 1193 | 1219 | if (outcome.kind === "fallback") { |
| 1220 | + setOAuthFallbackProbe("probe" in outcome ? outcome.probe : null); | |
| 1194 | 1221 | setDcrFailed(true); |
| 1195 | 1222 | toast.error("Automatic setup unavailable — register an app"); |
| 1196 | 1223 | } |
| @@ -1266,17 +1293,30 @@ function AddAccountModalView(props: AddAccountModalProps) { · 69 unmodified lines | ||
| 1266 | 1293 | )} |
| 1267 | 1294 | prefill={{ |
| 1268 | 1295 | authorizationUrl: |
| 1269 | − oauthHandoffPrefill?.authorizationUrl ?? method.oauth?.authorizationUrl, | |
| 1270 | − tokenUrl: oauthHandoffPrefill?.tokenUrl ?? method.oauth?.tokenUrl, | |
| 1296 | + oauthHandoffPrefill?.authorizationUrl ?? | |
| 1297 | + method.oauth?.authorizationUrl ?? | |
| 1298 | + oauthFallbackProbe?.authorizationUrl, | |
| 1299 | + tokenUrl: | |
| 1300 | + oauthHandoffPrefill?.tokenUrl ?? | |
| 1301 | + method.oauth?.tokenUrl ?? | |
| 1302 | + oauthFallbackProbe?.tokenUrl, | |
| 1303 | + resource: | |
| 1304 | + oauthHandoffPrefill?.resource ?? | |
| 1305 | + oauthFallbackProbe?.resource ?? | |
| 1306 | + method.oauth?.discoveryUrl ?? | |
| 1307 | + null, | |
| 1271 | 1308 | scopes: method.oauth?.scopes, |
| 1272 | − registrationEndpoint: method.oauth?.registrationEndpoint, | |
| 1309 | + discoveredScopes: oauthFallbackProbe?.scopesSupported, | |
| 1310 | + registrationEndpoint: | |
| 1311 | + method.oauth?.registrationEndpoint ?? | |
| 1312 | + oauthFallbackProbe?.registrationEndpoint ?? | |
| 1313 | + undefined, | |
| 1314 | + tokenEndpointAuthMethodsSupported: | |
| 1315 | + oauthFallbackProbe?.tokenEndpointAuthMethodsSupported, | |
| 1273 | 1316 | ...(oauthHandoffPrefill?.grant ? { grant: oauthHandoffPrefill.grant } : {}), |
| 1274 | 1317 | ...(oauthHandoffPrefill?.clientId |
| 1275 | 1318 | ? { clientId: oauthHandoffPrefill.clientId } |
| 1276 | 1319 | : {}), |
| 1277 | − ...(oauthHandoffPrefill?.resource != null | |
| 1278 | − ? { resource: oauthHandoffPrefill.resource } | |
| 1279 | − : {}), | |
| 1280 | 1320 | }} |
| 1281 | 1321 | fixedSlug={ |
| 1282 | 1322 | oauthClientHandoff?.slug != null && oauthClientHandoff.slug.length > 0 |
| @@ -89,8 +89,8 @@ export interface AuthTemplateEditorProps { · 88 unmodified lines | ||
| 89 | 89 | * tabs. Clicking a pill applies its value as an editable default. */ |
| 90 | 90 | readonly presets?: readonly AuthTemplateEditorPreset[]; |
| 91 | 91 | /** Some integrations, notably MCP, discover OAuth metadata from the server at |
| 92 | − * connection time. In that case the add form should declare OAuth but not | |
| 93 | − * expose editable provider URLs/scopes that are not persisted. */ | |
| 92 | + * connection time. In that case the add form should declare OAuth without | |
| 93 | + * editable provider URLs or scopes. */ | |
| 94 | 94 | readonly oauthMetadata?: "editable" | "discovered"; |
| 95 | 95 | } |
| 96 | 96 | |
| @@ -0,0 +1,28 @@ | ||
| 1 | +import { describe, expect, it } from "@effect/vitest"; | |
| 2 | + | |
| 3 | +import { registrationScopes } from "./oauth-client-form"; | |
| 4 | + | |
| 5 | +describe("registrationScopes", () => { | |
| 6 | + it("sends declared scopes and ignores discovered when declared scopes exist", () => { | |
| 7 | + // Declared (template) scopes are authoritative — a discovered set never wins. | |
| 8 | + expect(registrationScopes(["a", "b"], ["x"])).toEqual(["a", "b"]); | |
| 9 | + }); | |
| 10 | + | |
| 11 | + it("sends discovered scopes when none are declared", () => { | |
| 12 | + expect(registrationScopes([], ["x", "y"])).toEqual(["x", "y"]); | |
| 13 | + }); | |
| 14 | + | |
| 15 | + it("uses the current discovered set so a re-Discover replaces a prefilled probe", () => { | |
| 16 | + // Probe of server A seeds the discovered state; a later Discover of server B | |
| 17 | + // replaces it. With nothing declared, B's scopes register — not the stale A | |
| 18 | + // set that arrived via the DCR fallback prefill. | |
| 19 | + const seededFromProbeA = ["a:read"]; | |
| 20 | + const rediscoveredFromB = ["b:write"]; | |
| 21 | + expect(registrationScopes([], seededFromProbeA)).toEqual(["a:read"]); | |
| 22 | + expect(registrationScopes([], rediscoveredFromB)).toEqual(["b:write"]); | |
| 23 | + }); | |
| 24 | + | |
| 25 | + it("returns empty when nothing is declared or discovered", () => { | |
| 26 | + expect(registrationScopes([], [])).toEqual([]); | |
| 27 | + }); | |
| 28 | +}); | |
| @@ -39,7 +39,14 @@ export interface OAuthClientFormPrefill { · 38 unmodified lines | ||
| 39 | 39 | readonly authorizationUrl?: string; |
| 40 | 40 | readonly tokenUrl?: string; |
| 41 | 41 | readonly resource?: string | null; |
| 42 | + /** Template-DECLARED scopes (e.g. an OpenAPI bundle's scope union). Immutable | |
| 43 | + * in the form and authoritative — always sent at registration, surviving | |
| 44 | + * Discover. */ | |
| 42 | 45 | readonly scopes?: readonly string[]; |
| 46 | + /** Scopes already DISCOVERED from a prior server probe (e.g. a DCR fallback). | |
| 47 | + * Seed the form's discovered state; a later in-form Discover replaces them, | |
| 48 | + * and they are only sent when no declared scopes exist. */ | |
| 49 | + readonly discoveredScopes?: readonly string[]; | |
| 43 | 50 | readonly grant?: OAuthGrant; |
| 44 | 51 | /** Client id to seed (e.g. when editing an existing app). NOT a secret — the |
| 45 | 52 | * secret is never returned, so it is always re-entered. */ |
| @@ -48,8 +55,21 @@ export interface OAuthClientFormPrefill { · 2 unmodified lines | ||
| 48 | 55 | * method or surfaced by Discover), the form offers a one-click "Register |
| 49 | 56 | * automatically" path that needs no pasted client id/secret. */ |
| 50 | 57 | readonly registrationEndpoint?: string; |
| 58 | + /** Token-endpoint auth methods the server advertises (RFC 8414), so a | |
| 59 | + * prefilled "Register automatically" picks the right client-auth method | |
| 60 | + * instead of defaulting to public ("none"). */ | |
| 61 | + readonly tokenEndpointAuthMethodsSupported?: readonly string[]; | |
| 51 | 62 | } |
| 52 | 63 | |
| 64 | +/** The scopes to register via DCR. The integration's DECLARED (template) scopes | |
| 65 | + * are authoritative and immutable, so they win. Otherwise the DISCOVERED set is | |
| 66 | + * used — seeded from a prior server probe and replaced by any in-form Discover — | |
| 67 | + * so re-discovering a different issuer can't register a stale set. */ | |
| 68 | +export const registrationScopes = ( | |
| 69 | + declaredScopes: readonly string[], | |
| 70 | + discoveredScopes: readonly string[], | |
| 71 | +): readonly string[] => (declaredScopes.length > 0 ? declaredScopes : discoveredScopes); | |
| 72 | + | |
| 53 | 73 | export function OAuthClientForm(props: { |
| 54 | 74 | /** Human label for the integration this app backs (used in toasts + default name). */ |
| 55 | 75 | readonly integrationName: string; |
| @@ -108,6 +128,15 @@ export function OAuthClientForm(props: { · 52 unmodified lines | ||
| 108 | 128 | const [authorizationUrl, setAuthorizationUrl] = useState(prefill?.authorizationUrl ?? ""); |
| 109 | 129 | const [tokenUrl, setTokenUrl] = useState(prefill?.tokenUrl ?? ""); |
| 110 | 130 | const [resource, setResource] = useState(prefill?.resource ?? null); |
| 131 | + // Scopes to register with DCR. The form has no scopes input (DCR sends them | |
| 132 | + // verbatim). Declared scopes are the integration's TEMPLATE scopes — immutable | |
| 133 | + // and authoritative, so they survive Discover. Discovered scopes are seeded | |
| 134 | + // from a prior probe (e.g. a DCR fallback) and replaced by any in-form | |
| 135 | + // Discover; they are only sent when nothing is declared. | |
| 136 | + const declaredScopes = prefill?.scopes ?? []; | |
| 137 | + const [discoveredScopes, setDiscoveredScopes] = useState<readonly string[]>( | |
| 138 | + prefill?.discoveredScopes ?? [], | |
| 139 | + ); | |
| 111 | 140 | const [discovering, setDiscovering] = useState(false); |
| 112 | 141 | const [submitting, setSubmitting] = useState(false); |
| 113 | 142 | // DCR (RFC 7591): the registration endpoint + advertised auth methods. Seeded |
| @@ -116,7 +145,9 @@ export function OAuthClientForm(props: { · 2 unmodified lines | ||
| 116 | 145 | const [registrationEndpoint, setRegistrationEndpoint] = useState( |
| 117 | 146 | prefill?.registrationEndpoint ?? "", |
| 118 | 147 | ); |
| 119 | − const [authMethods, setAuthMethods] = useState<readonly string[] | undefined>(undefined); | |
| 148 | + const [authMethods, setAuthMethods] = useState<readonly string[] | undefined>( | |
| 149 | + prefill?.tokenEndpointAuthMethodsSupported, | |
| 150 | + ); | |
| 120 | 151 | const [registering, setRegistering] = useState(false); |
| 121 | 152 | |
| 122 | 153 | // Endpoints/scopes usually come prefilled from the integration's declared |
| @@ -166,6 +197,10 @@ export function OAuthClientForm(props: { · 43 unmodified lines | ||
| 166 | 197 | setAuthorizationUrl(result.authorizationUrl); |
| 167 | 198 | setTokenUrl(result.tokenUrl); |
| 168 | 199 | setResource(result.resource ?? null); |
| 200 | + // Record the discovered scopes. Declared scopes (if any) still take | |
| 201 | + // precedence at registration, so a re-Discover reflects the latest server | |
| 202 | + // without clobbering a declared set. | |
| 203 | + setDiscoveredScopes(result.scopesSupported ?? []); | |
| 169 | 204 | // Capture DCR availability so the "Register automatically" path shows for a |
| 170 | 205 | // pasted MCP/issuer URL without any client id/secret. |
| 171 | 206 | setRegistrationEndpoint(result.registrationEndpoint ?? ""); |
| @@ -189,9 +224,9 @@ export function OAuthClientForm(props: { · 17 unmodified lines | ||
| 189 | 224 | authorizationUrl: authorizationUrl.trim(), |
| 190 | 225 | tokenUrl: tokenUrl.trim(), |
| 191 | 226 | resource, |
| 192 | − // DCR sends the integration's declared scopes to the AS at registration | |
| 193 | − // (the app itself stores none). | |
| 194 | − scopes: [...(prefill?.scopes ?? [])], | |
| 227 | + // DCR sends the integration's declared scopes, or the discovered set when | |
| 228 | + // none are declared, to the AS at registration (the app stores none). | |
| 229 | + scopes: [...registrationScopes(declaredScopes, discoveredScopes)], | |
| 195 | 230 | tokenEndpointAuthMethodsSupported: authMethods, |
| 196 | 231 | clientName: name.trim(), |
| 197 | 232 | redirectUri: oauthCallbackUrl(), |
| @@ -96,6 +96,19 @@ describe("selectClientsForEndpoints", () => { · 95 unmodified lines | ||
| 96 | 96 | expect(result.unmatched).toEqual([]); |
| 97 | 97 | }); |
| 98 | 98 | |
| 99 | + it("with requireEndpointMatch and no endpoints, matches NOTHING (drives the register CTA)", () => { | |
| 100 | + // A server-targeting integration (MCP, endpoints discovered at connect) must | |
| 101 | + // not auto-select an unrelated provider's app just because nothing was | |
| 102 | + // declared to filter on. | |
| 103 | + const result = selectClientsForEndpoints([google, spotify], { requireEndpointMatch: true }); | |
| 104 | + expect(result.endpointMatched).toBe(false); | |
| 105 | + expect(result.matched).toEqual([]); | |
| 106 | + expect(result.unmatched.map((a: OAuthClientOption) => String(a.slug))).toEqual([ | |
| 107 | + "google-app", | |
| 108 | + "spotify-app", | |
| 109 | + ]); | |
| 110 | + }); | |
| 111 | + | |
| 99 | 112 | it("matches local-dev MCP by exact host when tldts cannot resolve a root domain", () => { |
| 100 | 113 | const local = app("local-mcp", { |
| 101 | 114 | authorizationUrl: "http://localhost:8787/authorize", |
| @@ -97,7 +97,15 @@ const sortUserFirst = (apps: readonly OAuthClientOption[]): readonly OAuthClient · 96 unmodified lines | ||
| 97 | 97 | */ |
| 98 | 98 | export function selectClientsForEndpoints( |
| 99 | 99 | all: readonly OAuthClientOption[], |
| 100 | − endpoints: { readonly tokenUrl?: string; readonly authorizationUrl?: string }, | |
| 100 | + endpoints: { | |
| 101 | + readonly tokenUrl?: string; | |
| 102 | + readonly authorizationUrl?: string; | |
| 103 | + /** When set, an integration that targets a SPECIFIC server (MCP, whose | |
| 104 | + * endpoints are discovered at connect) must match by endpoint — absent | |
| 105 | + * endpoints mean NO match (show the register CTA), never "every app | |
| 106 | + * matches". Prevents auto-selecting an unrelated provider's app. */ | |
| 107 | + readonly requireEndpointMatch?: boolean; | |
| 108 | + }, | |
| 101 | 109 | ): { |
| 102 | 110 | readonly matched: readonly OAuthClientOption[]; |
| 103 | 111 | readonly unmatched: readonly OAuthClientOption[]; |
| @@ -107,8 +115,13 @@ export function selectClientsForEndpoints( · 3 unmodified lines | ||
| 107 | 115 | const wantedAuthorizationRoot = endpoints.authorizationUrl |
| 108 | 116 | ? getRootDomain(endpoints.authorizationUrl) |
| 109 | 117 | : undefined; |
| 110 | − // No declared endpoints → no filter; every app is usable. | |
| 118 | + // No declared endpoints. A server-targeting integration must NOT match every | |
| 119 | + // app (it would auto-select an unrelated provider); surface the register CTA | |
| 120 | + // instead. Otherwise (no endpoint filter at all) every app is usable. | |
| 111 | 121 | if (!wantedTokenRoot && !wantedAuthorizationRoot) { |
| 122 | + if (endpoints.requireEndpointMatch) { | |
| 123 | + return { matched: [], unmatched: sortUserFirst(all), endpointMatched: false }; | |
| 124 | + } | |
| 112 | 125 | return { matched: sortUserFirst(all), unmatched: [], endpointMatched: true }; |
| 113 | 126 | } |
| 114 | 127 | const matched: OAuthClientOption[] = []; |
| @@ -133,6 +146,7 @@ export function selectClientsForEndpoints( · 18 unmodified lines | ||
| 133 | 146 | export function useOAuthClientsForIntegration(opts: { |
| 134 | 147 | readonly tokenUrl?: string; |
| 135 | 148 | readonly authorizationUrl?: string; |
| 149 | + readonly requireEndpointMatch?: boolean; | |
| 136 | 150 | }): UseOAuthClientsResult { |
| 137 | 151 | // Read the optimistic list so a just-registered/edited/removed app paints |
| 138 | 152 | // immediately, instead of flashing the stale server list until the refetch |
| @@ -153,6 +167,7 @@ export function useOAuthClientsForIntegration(opts: { · 14 unmodified lines | ||
| 153 | 167 | const { matched, unmatched, endpointMatched } = selectClientsForEndpoints(all, { |
| 154 | 168 | tokenUrl: opts.tokenUrl, |
| 155 | 169 | authorizationUrl: opts.authorizationUrl, |
| 170 | + requireEndpointMatch: opts.requireEndpointMatch, | |
| 156 | 171 | }); |
| 157 | 172 | // EXPLICIT outcome: when at least one app matched (or no endpoint was |
| 158 | 173 | // declared) we present the matched subset. When an endpoint was declared but |