Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions .changeset/cimd-refresh-token.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
---
"@executor-js/api": patch
---

Advertise refresh-token support in OAuth client ID metadata documents.

OAuth providers may reject the `offline_access` scope when the client's
metadata declares only the authorization-code grant. Hosted and local client
metadata now declare both `authorization_code` and `refresh_token`, matching
Executor's dynamic client registration behavior.
11 changes: 10 additions & 1 deletion apps/cloud/src/mcp/session-build-semaphore.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { describe, expect, it, beforeEach } from "@effect/vitest";
import { describe, expect, it, beforeEach, afterEach, vi } from "@effect/vitest";

import {
acquireBuildSlot,
Expand All @@ -13,6 +13,10 @@ describe("session-build-semaphore", () => {
resetBuildSlotsForTest();
});

afterEach(() => {
vi.useRealTimers();
});

it("grants up to the cap immediately, with no wait", async () => {
const results = await Promise.all([
acquireBuildSlot().promise,
Expand Down Expand Up @@ -214,6 +218,7 @@ describe("session-build-semaphore", () => {
});

it("proceeds without a slot when the queue wait exceeds the timeout, and does not count it as active", async () => {
vi.useFakeTimers();
await Promise.all([
acquireBuildSlot().promise,
acquireBuildSlot().promise,
Expand All @@ -223,6 +228,10 @@ describe("session-build-semaphore", () => {
expect(currentActiveBuildsForTest()).toBe(4);

const timedOutHandle = acquireBuildSlot(10);
await vi.advanceTimersByTimeAsync(9);
expect(currentQueueLengthForTest()).toBe(1);
expect(currentActiveBuildsForTest()).toBe(4);
await vi.advanceTimersByTimeAsync(1);
const result = await timedOutHandle.promise;

expect(result).toEqual({ acquired: false, waitMs: expect.any(Number), timedOut: true });
Expand Down
70 changes: 64 additions & 6 deletions e2e/selfhost/mcp-oauth-cimd-connect.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import { visit } from "../src/surfaces/browser";
const api = composePluginApi([mcpHttpPlugin()] as const);

scenario(
"MCP OAuth · advertised CIMD starts authorization without dynamic registration",
"MCP OAuth · CIMD advertises refresh support and completes connection without dynamic registration",
{ timeout: 180_000 },
Effect.scoped(
Effect.gen(function* () {
Expand All @@ -30,7 +30,7 @@ scenario(
const oauth = yield* OAuthTestServer;
const server = yield* serveMcpServerWithOAuth(
() => makeGreetingMcpServer({ name: "cimd-connect-mcp" }),
{ path: "/mcp" },
{ path: "/mcp", scopes: ["read", "offline_access"] },
);
const identity = yield* target.newIdentity();
const client = yield* makeApiClient(api, identity);
Expand Down Expand Up @@ -69,17 +69,68 @@ scenario(
authorize,
"the popup reached the discovered authorization endpoint",
).toBeDefined();
const clientId = authorize?.query["client_id"];
createdClientId = clientId;
expect(
(authorize?.query["scope"] ?? "").split(" "),
"authorization requests the resource's offline access scope",
).toContain("offline_access");
const clientId = authorize?.query["client_id"] ?? "";
createdClientId = clientId || undefined;
expect(
clientId,
"authorization uses Executor's metadata document as client_id",
).toMatch(/^https?:\/\/[^/]+\/api\/oauth\/client-id-metadata\/.+\.json$/);
await popup.close();
const metadataResponse = await page.request.get(clientId);
expect(metadataResponse.status(), "the client metadata document is reachable").toBe(
200,
);
expect(
await metadataResponse.json(),
"the client declares the grant required by offline_access",
).toMatchObject({
grant_types: ["authorization_code", "refresh_token"],
});
expect(authorize).toBeDefined();
// oxlint-disable-next-line executor/no-try-catch-or-throw, executor/no-error-constructor -- test boundary: authorization must exist before completing the flow
if (authorize === undefined) throw new Error("Missing authorization request");
const completed = await Effect.runPromise(
oauth.completeAuthorizationCodeFlow({ authorizationUrl: authorize.url }),
);
await popup.goto(completed.callbackUrl);
await page
.getByRole("heading", { name: /Add connection/ })
.waitFor({ state: "hidden" });
await popup.close().catch(() => undefined);
});
});

const connections = yield* client.connections.list({ query: { integration: slug } });
expect(connections, "the OAuth callback saved the connection").toHaveLength(1);
const tools = yield* client.tools.list({ query: { integration: slug } });
expect(
tools.some((tool) => tool.name === "simple_echo"),
"authenticated discovery finds the upstream tool",
).toBe(true);

const invoked = yield* client.executions.execute({
payload: {
code: `return await ${tools[0]?.address}({});`,
autoApprove: true,
},
});
expect(invoked.status).toBe("completed");
expect(invoked.text, "the connected tool runs through authenticated MCP").toContain(
"mcp-ok",
);

const requests = yield* oauth.requests;
expect(
requests.some(
(request) =>
request.path === "/token" &&
new URLSearchParams(request.body).get("grant_type") === "authorization_code",
),
"the callback exchanged the code using the advertised client",
).toBe(true);
expect(
requests.filter((request) => request.method === "POST" && request.path === "/register"),
"CIMD wins when the server also advertises DCR",
Expand All @@ -105,5 +156,12 @@ scenario(
),
);
}),
).pipe(Effect.provide(OAuthTestServer.layer({ clientIdMetadataDocumentSupported: true }))),
).pipe(
Effect.provide(
OAuthTestServer.layer({
clientIdMetadataDocumentSupported: true,
scopes: ["read", "offline_access"],
}),
),
),
);
2 changes: 2 additions & 0 deletions packages/core/api/src/server/oauth-client-metadata.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ describe("OAuth client ID metadata document", () => {
"http://100.81.219.45:42384/api/oauth/client-id-metadata/acme.json",
);
expect(metadata.redirect_uris).toEqual(["http://100.81.219.45:42384/api/oauth/callback"]);
expect(metadata.grant_types).toEqual(["authorization_code", "refresh_token"]);
expect(metadata.token_endpoint_auth_method).toBe("none");
expect(metadata.application_type).toBe("web");
});
Expand Down Expand Up @@ -63,6 +64,7 @@ describe("OAuth client ID metadata document", () => {
"http://localhost/api/oauth/callback",
"http://[::1]/api/oauth/callback",
]);
expect(metadata.grant_types).toEqual(["authorization_code", "refresh_token"]);
expect(metadata.application_type).toBe("native");
});

Expand Down
10 changes: 7 additions & 3 deletions packages/core/api/src/server/oauth-client-metadata.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@ export const OAUTH_CLIENT_ID_METADATA_DOCUMENT_TARGET_PATH_PREFIX =
export const OAUTH_CLIENT_ID_METADATA_DOCUMENT_DEFAULT_TARGET = "default" as const;
export const OAUTH_CLIENT_ID_METADATA_DOCUMENT_LOCAL_TARGET = "local" as const;

// Keep CIMD aligned with DCR: providers may reject `offline_access` unless the
// client declares that it can use the refresh-token grant.
const OAUTH_CLIENT_GRANT_TYPES = ["authorization_code", "refresh_token"] as const;

type MetadataTarget =
| typeof OAUTH_CLIENT_ID_METADATA_DOCUMENT_DEFAULT_TARGET
| typeof OAUTH_CLIENT_ID_METADATA_DOCUMENT_LOCAL_TARGET
Expand All @@ -19,7 +23,7 @@ interface OAuthClientIdMetadataDocument {
readonly client_name: string;
readonly client_uri: string;
readonly redirect_uris: readonly string[];
readonly grant_types: readonly ["authorization_code"];
readonly grant_types: typeof OAUTH_CLIENT_GRANT_TYPES;
readonly response_types: readonly ["code"];
readonly token_endpoint_auth_method: "none";
readonly application_type: "web" | "native";
Expand Down Expand Up @@ -129,7 +133,7 @@ export const oauthClientIdMetadataDocumentFromRequest = ({
client_name: "Executor Local",
client_uri: url.origin,
redirect_uris: localLoopbackRedirectUris(mountPrefix),
grant_types: ["authorization_code"],
grant_types: OAUTH_CLIENT_GRANT_TYPES,
response_types: ["code"],
token_endpoint_auth_method: "none",
application_type: "native",
Expand All @@ -150,7 +154,7 @@ export const oauthClientIdMetadataDocumentFromRequest = ({
client_name: "Executor",
client_uri: url.origin,
redirect_uris: [redirectUri.toString()],
grant_types: ["authorization_code"],
grant_types: OAUTH_CLIENT_GRANT_TYPES,
response_types: ["code"],
token_endpoint_auth_method: "none",
application_type: "web",
Expand Down
Loading