Skip to content

Commit 3cf6e54

Browse files
RhysSullivanRhysSullivan
authored
Test DCR registered-scope reuse and start narrowing (#2403)
* Test DCR registered-scope reuse and start narrowing * Test DCR scope recording against a server that narrows registrations
1 parent 56aec8b commit 3cf6e54

3 files changed

Lines changed: 283 additions & 1 deletion

File tree

‎packages/core/sdk/src/oauth-dcr-registered-scopes.test.ts‎

Lines changed: 246 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,246 @@
1+
import { describe, expect, it } from "@effect/vitest";
2+
import { Effect, Predicate } from "effect";
3+
4+
import {
5+
AuthTemplateSlug,
6+
ConnectionName,
7+
IntegrationSlug,
8+
OAuthClientSlug,
9+
ToolName,
10+
} from "./ids";
11+
import type { AuthMethodDescriptor } from "./integration";
12+
import type { OAuthStartError } from "./oauth-client";
13+
import { definePlugin } from "./plugin";
14+
import { makeTestWorkspaceHarness, memoryCredentialsPlugin } from "./test-config";
15+
import { scopesFromAuthorizeUrl, serveOAuthTestServer } from "./testing/oauth-test-server";
16+
17+
// A dynamically registered client may only be authorized for the scopes it
18+
// registered for: Mercury answers `invalid_scope` for any other. When the
19+
// server later advertises more scopes, reconnecting must register a client
20+
// that covers them rather than reuse the old registration, and a flow through
21+
// a client never asks for scopes outside its registration.
22+
23+
const INTEG = IntegrationSlug.make("acme");
24+
const TEMPLATE = AuthTemplateSlug.make("oauth");
25+
const FLOW_REDIRECT_URI = "https://localhost:5394/api/oauth/callback";
26+
27+
const registerRequestCount = (
28+
requests: readonly { readonly path: string; readonly method: string }[],
29+
): number => requests.filter((r) => r.path === "/register" && r.method === "POST").length;
30+
31+
/** An integration whose OAuth method either declares `scopes` or, like MCP,
32+
* discovers them from `discoveryUrl`. */
33+
const scopePlugin = (oauth: AuthMethodDescriptor["oauth"]) =>
34+
definePlugin(() => ({
35+
id: "acme" as const,
36+
storage: () => ({}),
37+
resolveTools: () =>
38+
Effect.succeed({
39+
tools: [{ name: ToolName.make("whoami"), description: "whoami" }],
40+
}),
41+
invokeTool: ({ credential }) => Effect.succeed({ token: credential.value }),
42+
describeAuthMethods: (): readonly AuthMethodDescriptor[] => [
43+
{ id: "oauth", label: "OAuth2", kind: "oauth", template: String(TEMPLATE), oauth },
44+
],
45+
extension: (ctx) => ({
46+
seed: () => ctx.core.integrations.register({ slug: INTEG, description: "Acme", config: {} }),
47+
}),
48+
}))();
49+
50+
const setup = (options: {
51+
readonly declaredScopes?: readonly string[];
52+
readonly registrableScopes?: readonly string[];
53+
}) =>
54+
Effect.gen(function* () {
55+
const server = yield* serveOAuthTestServer({
56+
scopes: ["read", "write"],
57+
enforceRegisteredScopes: true,
58+
...(options.registrableScopes === undefined
59+
? {}
60+
: { registrableScopes: options.registrableScopes }),
61+
});
62+
const plugins = [
63+
memoryCredentialsPlugin(),
64+
scopePlugin(
65+
options.declaredScopes === undefined
66+
? { discoveryUrl: server.mcpResourceUrl }
67+
: { scopes: options.declaredScopes },
68+
),
69+
] as const;
70+
const harness = yield* makeTestWorkspaceHarness({ plugins });
71+
yield* harness.executor.acme.seed();
72+
const probe = yield* harness.executor.oauth.probe({ url: server.mcpResourceUrl });
73+
const register = (scopes: readonly string[]) =>
74+
harness.executor.oauth.registerDynamicClient({
75+
owner: "org",
76+
slug: OAuthClientSlug.make("acme-dcr"),
77+
issuer: probe.issuer,
78+
registrationEndpoint: probe.registrationEndpoint!,
79+
authorizationUrl: probe.authorizationUrl,
80+
tokenUrl: probe.tokenUrl,
81+
resource: probe.resource,
82+
scopes,
83+
tokenEndpointAuthMethodsSupported: probe.tokenEndpointAuthMethodsSupported,
84+
clientName: "Executor",
85+
redirectUri: FLOW_REDIRECT_URI,
86+
originIntegration: INTEG,
87+
});
88+
const start = (client: OAuthClientSlug) =>
89+
harness.executor.oauth.start({
90+
owner: "org",
91+
client,
92+
clientOwner: "org",
93+
name: ConnectionName.make("main"),
94+
integration: INTEG,
95+
template: TEMPLATE,
96+
redirectUri: FLOW_REDIRECT_URI,
97+
});
98+
return { ...harness, server, probe, register, start };
99+
});
100+
101+
describe("DCR registered scopes", () => {
102+
it.effect("reconnecting after the server added scopes registers a client covering them", () =>
103+
Effect.scoped(
104+
Effect.gen(function* () {
105+
const { executor, server, probe, register, start } = yield* setup({});
106+
expect(probe.scopesSupported).toEqual(["read", "write"]);
107+
108+
// Registered while the server only offered `read`.
109+
const original = yield* register(["read"]);
110+
yield* server.clearRequests;
111+
112+
// Reconnect registers with the scopes the server now advertises.
113+
const reconnected = yield* register(probe.scopesSupported ?? []);
114+
expect(registerRequestCount(yield* server.requests)).toBe(1);
115+
expect(String(reconnected)).not.toBe(String(original));
116+
117+
const started = yield* start(reconnected);
118+
expect(started.status).toBe("redirect");
119+
if (started.status !== "redirect") return;
120+
expect(scopesFromAuthorizeUrl(started.authorizationUrl)).toEqual(["read", "write"]);
121+
const callback = yield* server.completeAuthorizationCodeFlow({
122+
authorizationUrl: started.authorizationUrl,
123+
});
124+
const connection = yield* executor.oauth.complete({
125+
state: started.state,
126+
code: callback.code,
127+
});
128+
expect(String(connection.address)).toBe("tools.acme.org.main");
129+
}),
130+
),
131+
);
132+
133+
it.effect("reuses a client whose registration covers the request", () =>
134+
Effect.scoped(
135+
Effect.gen(function* () {
136+
const { server, register } = yield* setup({});
137+
const original = yield* register(["read", "write"]);
138+
yield* server.clearRequests;
139+
140+
const reused = yield* register(["read"]);
141+
expect(registerRequestCount(yield* server.requests)).toBe(0);
142+
expect(String(reused)).toBe(String(original));
143+
}),
144+
),
145+
);
146+
147+
it.effect("does not reuse a client with no recorded registration scopes", () =>
148+
Effect.scoped(
149+
Effect.gen(function* () {
150+
const { config, server, register } = yield* setup({});
151+
const legacy = yield* register(["read"]);
152+
yield* Effect.promise(() =>
153+
config.db.updateMany("oauth_client", {
154+
where: (b) => b("slug", "=", String(legacy)),
155+
set: { origin_requested_scopes: null, origin_scopes: null },
156+
}),
157+
);
158+
yield* server.clearRequests;
159+
160+
const registered = yield* register(["read"]);
161+
expect(registerRequestCount(yield* server.requests)).toBe(1);
162+
expect(String(registered)).not.toBe(String(legacy));
163+
}),
164+
),
165+
);
166+
167+
it.effect("start narrows to the scopes the server registered, not the ones asked for", () =>
168+
Effect.scoped(
169+
Effect.gen(function* () {
170+
// The server registers only `read` though the client asked for both.
171+
const { executor, server, register, start } = yield* setup({
172+
registrableScopes: ["read"],
173+
});
174+
const client = yield* register(["read", "write"]);
175+
176+
const started = yield* start(client);
177+
expect(started.status).toBe("redirect");
178+
if (started.status !== "redirect") return;
179+
expect(scopesFromAuthorizeUrl(started.authorizationUrl)).toEqual(["read"]);
180+
const callback = yield* server.completeAuthorizationCodeFlow({
181+
authorizationUrl: started.authorizationUrl,
182+
});
183+
const connection = yield* executor.oauth.complete({
184+
state: started.state,
185+
code: callback.code,
186+
});
187+
expect(connection.oauthScope).toBe("read");
188+
}),
189+
),
190+
);
191+
192+
it.effect("reuses a narrowed registration instead of registering again on reconnect", () =>
193+
Effect.scoped(
194+
Effect.gen(function* () {
195+
const { server, register } = yield* setup({ registrableScopes: ["read"] });
196+
const original = yield* register(["read", "write"]);
197+
yield* server.clearRequests;
198+
199+
// Registering again would be narrowed the same way, so the client that
200+
// already asked for these scopes is reused.
201+
const reused = yield* register(["read", "write"]);
202+
expect(registerRequestCount(yield* server.requests)).toBe(0);
203+
expect(String(reused)).toBe(String(original));
204+
}),
205+
),
206+
);
207+
208+
it.effect("start narrows discovered scopes to the client's registration", () =>
209+
Effect.scoped(
210+
Effect.gen(function* () {
211+
const { executor, server, register, start } = yield* setup({});
212+
const client = yield* register(["read"]);
213+
214+
const started = yield* start(client);
215+
expect(started.status).toBe("redirect");
216+
if (started.status !== "redirect") return;
217+
expect(scopesFromAuthorizeUrl(started.authorizationUrl)).toEqual(["read"]);
218+
const callback = yield* server.completeAuthorizationCodeFlow({
219+
authorizationUrl: started.authorizationUrl,
220+
});
221+
const connection = yield* executor.oauth.complete({
222+
state: started.state,
223+
code: callback.code,
224+
});
225+
expect(connection.oauthScope).toBe("read");
226+
}),
227+
),
228+
);
229+
230+
it.effect("start refuses declared scopes outside the client's registration", () =>
231+
Effect.scoped(
232+
Effect.gen(function* () {
233+
const { server, register, start } = yield* setup({ declaredScopes: ["read", "write"] });
234+
const client = yield* register(["read"]);
235+
yield* server.clearRequests;
236+
237+
const error = yield* start(client).pipe(Effect.flip);
238+
expect(Predicate.isTagged("OAuthStartError")(error)).toBe(true);
239+
const startError = error as OAuthStartError;
240+
expect(startError.message).toContain("write");
241+
const requests = yield* server.requests;
242+
expect(requests.filter((r) => r.path === "/authorize")).toHaveLength(0);
243+
}),
244+
),
245+
);
246+
});

‎packages/core/sdk/src/oauth-register-dynamic.test.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -432,13 +432,17 @@ describe("oauth.registerDynamicClient", () => {
432432
clientSecret: "",
433433
});
434434
// Simulate the migration's backfill: legacy DCR stamp + issuer set.
435+
// Its registered scopes are recorded too; a row without them is
436+
// never reused (see oauth-dcr-registered-scopes.test.ts).
435437
yield* Effect.promise(() =>
436438
config.db.updateMany("oauth_client", {
437439
where: (b) => b("slug", "=", String(legacySlug)),
438440
set: {
439441
origin_kind: "dynamic_client_registration",
440442
origin_integration: null,
441443
origin_issuer: probe.issuer,
444+
origin_requested_scopes: "read",
445+
origin_scopes: "read",
442446
},
443447
}),
444448
);

‎packages/core/sdk/src/testing/oauth-test-server.ts‎

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,14 @@ export interface OAuthTestServerOptions {
7373
* back, so a client that re-sends its recorded grant is refused (issue
7474
* #1969). Omit to accept any `scope`, the default. */
7575
readonly refreshGrantScopes?: readonly string[];
76+
/** Refuse an authorize request for a scope outside the ones a dynamically
77+
* registered client registered for, answering `invalid_scope` the way
78+
* Mercury does. Omit to authorize any scope, the default. */
79+
readonly enforceRegisteredScopes?: boolean;
80+
/** Register a dynamically registered client only for the requested scopes
81+
* in this list, answering the narrowed set as the registration's `scope`
82+
* (RFC 7591 §3.2.1). Omit to register every requested scope, the default. */
83+
readonly registrableScopes?: readonly string[];
7684
readonly invalidRefreshTokenDescription?: string;
7785
/** RFC 6749 error code returned when a refresh-token grant is rejected.
7886
* Defaults to `invalid_grant`; set to e.g. `invalid_request` to mirror
@@ -191,6 +199,8 @@ interface ClientRecord {
191199
readonly clientSecret: string | null;
192200
readonly redirectUris: ReadonlySet<string>;
193201
readonly tokenEndpointAuthMethod: string;
202+
/** Scopes a dynamically registered client registered for. */
203+
readonly registeredScopes?: ReadonlySet<string>;
194204
}
195205

196206
interface AuthorizationTransaction {
@@ -707,10 +717,19 @@ export const serveOAuthTestServer = (
707717
"The provided redirect URIs are not approved for use by this authorization server.",
708718
);
709719
}
720+
const requestedScope = typeof json.scope === "string" ? json.scope : scopes.join(" ");
721+
const registeredScope =
722+
options.registrableScopes === undefined
723+
? requestedScope
724+
: requestedScope
725+
.split(/\s+/)
726+
.filter((scope) => options.registrableScopes!.includes(scope))
727+
.join(" ");
710728
clients.set(clientId, {
711729
clientSecret,
712730
redirectUris,
713731
tokenEndpointAuthMethod: requestedMethod,
732+
registeredScopes: new Set(registeredScope.split(/\s+/).filter(Boolean)),
714733
});
715734
return jsonResponse(
716735
201,
@@ -722,7 +741,7 @@ export const serveOAuthTestServer = (
722741
redirect_uris: [...redirectUris],
723742
grant_types: arrayOfStrings(json.grant_types),
724743
response_types: arrayOfStrings(json.response_types),
725-
scope: typeof json.scope === "string" ? json.scope : scopes.join(" "),
744+
scope: registeredScope,
726745
},
727746
{ "cache-control": "no-store" },
728747
);
@@ -741,6 +760,19 @@ export const serveOAuthTestServer = (
741760
if (client && client.redirectUris.size > 0 && !client.redirectUris.has(redirectUri)) {
742761
return oauthError(400, "invalid_request", "redirect_uri is not registered");
743762
}
763+
const registeredScopes = client?.registeredScopes;
764+
if (options.enforceRegisteredScopes && registeredScopes) {
765+
const unregistered = (requestUrl.searchParams.get("scope") ?? "")
766+
.split(/\s+/)
767+
.filter((scope) => scope.length > 0 && !registeredScopes.has(scope));
768+
if (unregistered.length > 0) {
769+
return oauthError(
770+
400,
771+
"invalid_scope",
772+
`The OAuth 2.0 Client is not allowed to request scope '${unregistered[0]}'.`,
773+
);
774+
}
775+
}
744776
if (!client) {
745777
clients.set(clientId, {
746778
clientSecret: null,

0 commit comments

Comments
 (0)