History

Preserve OAuth scopes across unrelated metadata (#1425)

Rhys Sullivan committed 3 months ago

Commit 0c4e9b41 parent e2712db3 files · +63 −3Browse files

Unified · Split

3 files
  1. .changeset/preserve-oauth-scope-surface.md +5 −0
  2. packages/core/sdk/src/oauth-scope-union.test.ts +47 −2
  3. packages/core/sdk/src/oauth-service.ts +11 −1
.changeset/preserve-oauth-scope-surface.md+5−0
@@ -0,0 +1,5 @@
1+---
2+"@executor-js/sdk": patch
3+---
4+
5+Preserve an integration's declared OAuth scopes when same-origin authorization-server metadata describes a different authorization or token endpoint.
packages/core/sdk/src/oauth-scope-union.test.ts+47−2
@@ -107,6 +107,8 @@ const serveMetadataServer = (config: { · 106 unmodified lines
107107 | "error"
108108 | null;
109109 readonly authServerScopes?: readonly string[];
110+ readonly authServerAuthorizationPath?: string;
111+ readonly authServerTokenPath?: string;
110112 }) =>
111113 Effect.gen(function* () {
112114 const baseUrlRef = { value: "" };
@@ -133,8 +135,8 @@ const serveMetadataServer = (config: { · 20 unmodified lines
133135 ) {
134136 return HttpServerResponse.jsonUnsafe({
135137 issuer: base,
136− authorization_endpoint: `${base}/authorize`,
137− token_endpoint: `${base}/token`,
138+ authorization_endpoint: `${base}${config.authServerAuthorizationPath ?? "/authorize"}`,
139+ token_endpoint: `${base}${config.authServerTokenPath ?? "/token"}`,
138140 response_types_supported: ["code"],
139141 code_challenge_methods_supported: ["S256"],
140142 scopes_supported: config.authServerScopes,
@@ -325,6 +327,49 @@ describe("oauth.start integration-driven scopes", () => { · 184 unmodified lines
325327 ),
326328 );
327329
330+ it.effect("keeps declared scopes when same-origin metadata describes another OAuth surface", () =>
331+ Effect.scoped(
332+ Effect.gen(function* () {
333+ const server = yield* serveMetadataServer({
334+ authServerScopes: ["mcp:connect"],
335+ authServerAuthorizationPath: "/oauth/mcp",
336+ });
337+ const plugins = [
338+ memoryCredentialsPlugin(),
339+ makeScopePlugin({ scopes: ["file_content:read", "file_comments:write"] }),
340+ ] as const;
341+ const { executor } = yield* makeTestWorkspaceHarness({ plugins });
342+ yield* executor.acme.seed();
343+
344+ yield* executor.oauth.createClient({
345+ owner: "org",
346+ slug: CLIENT,
347+ authorizationUrl: server.authorizationEndpoint,
348+ tokenUrl: server.tokenEndpoint,
349+ grant: "authorization_code",
350+ clientId: "test-client",
351+ clientSecret: "test-secret",
352+ });
353+
354+ const started = yield* executor.oauth.start({
355+ owner: "org",
356+ client: CLIENT,
357+ clientOwner: "org",
358+ name: ConnectionName.make("main"),
359+ integration: INTEG,
360+ template: TEMPLATE,
361+ });
362+ expect(started.status).toBe("redirect");
363+ if (started.status !== "redirect") return;
364+
365+ expect(scopesFromAuthorizeUrl(started.authorizationUrl)).toEqual([
366+ "file_content:read",
367+ "file_comments:write",
368+ ]);
369+ }),
370+ ),
371+ );
372+
328373 it.effect(
329374 "(d) for MCP, the resource's own scopes_supported wins over a divergent authorization server",
330375 () =>
packages/core/sdk/src/oauth-service.ts+11−1
@@ -54,6 +54,7 @@ import { · 53 unmodified lines
5454 discoverProtectedResourceMetadata,
5555 OAuthDiscoveryError,
5656 registerDynamicClient as registerDynamicClientDcr,
57+ type OAuthAuthorizationServerMetadata,
5758 } from "./oauth-discovery";
5859 import {
5960 assertSupportedOAuthEndpointUrl,
@@ -414,6 +415,14 @@ const canonicalUrlString = (value: string): string => { · 354 unmodified lines
414415 return url.toString();
415416 };
416417
418+const oauthMetadataMatchesClient = (
419+ client: Pick<LoadedOAuthClient, "authorizationUrl" | "tokenUrl">,
420+ metadata: OAuthAuthorizationServerMetadata,
421+): boolean =>
422+ canonicalUrlString(metadata.authorization_endpoint) ===
423+ canonicalUrlString(client.authorizationUrl) &&
424+ canonicalUrlString(metadata.token_endpoint) === canonicalUrlString(client.tokenUrl);
425+
417426 const isWellKnownOAuthMetadataUrl = (value: string): boolean => {
418427 const path = new URL(value.trim()).pathname.toLowerCase();
419428 return (
@@ -494,7 +503,8 @@ export const makeOAuthService = (deps: OAuthServiceDeps): OAuthService => { · 74 unmodified lines
494503 Effect.catch(() => Effect.succeed(null)),
495504 Effect.provide(httpClientLayer),
496505 );
497− return intersectScopes(requestedScopes, as?.metadata.scopes_supported);
506+ if (!as || !oauthMetadataMatchesClient(client, as.metadata)) return requestedScopes;
507+ return intersectScopes(requestedScopes, as.metadata.scopes_supported);
498508 }).pipe(Effect.catch(() => Effect.succeed(requestedScopes)));
499509
500510 // Caps on server-controlled discovery input — a hostile or buggy server must