Skip to content
Open
5 changes: 5 additions & 0 deletions .changeset/oauth-local-reject-user-client.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@executor-js/sdk": patch
---

Reject user-owned OAuth clients when the subject is local, including before DCR.
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
156 changes: 156 additions & 0 deletions e2e/local/oauth-client-ownership.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,156 @@
import { expect } from "@effect/vitest";
import { connectEmulator } from "@executor-js/emulate";
import { Effect } from "effect";
import { HttpApiClient } from "effect/unstable/httpapi";
import { FetchHttpClient, HttpClient, HttpClientRequest } from "effect/unstable/http";
import { composePluginApi } from "@executor-js/api/server";
import { mcpHttpPlugin } from "@executor-js/plugin-mcp/api";
import {
AuthTemplateSlug,
ConnectionName,
IntegrationSlug,
OAuthClientSlug,
} from "@executor-js/sdk/shared";

import { createEmulatorInstance } from "../src/emulator-instance";
import { scenario } from "../src/scenario";
import { Browser, Cli, RunDir } from "../src/services";
import { withLocalServer } from "./local-server";

const api = composePluginApi([mcpHttpPlugin()] as const);

scenario(
"Local OAuth · reject user clients before registration while org clients connect",
{ timeout: 180_000 },
Effect.scoped(
Effect.gen(function* () {
const cli = yield* Cli;
const runDir = yield* RunDir;
const browser = yield* Browser;
const base = yield* createEmulatorInstance("mcp", "local-ownership");
const emulator = yield* Effect.promise(() =>
connectEmulator({ baseUrl: base, service: "mcp" }),
);
yield* Effect.promise(() => emulator.seed({ users: [{ login: "local-oauth-user" }] }));
yield* withLocalServer(cli, runDir, (server) =>
Effect.gen(function* () {
const client = yield* HttpApiClient.make(api, {
baseUrl: new URL("/api", server.origin).toString(),
transformClient: HttpClient.mapRequest((request) =>
HttpClientRequest.setHeader(request, "authorization", `Bearer ${server.token}`),
),
}).pipe(Effect.provide(FetchHttpClient.layer));
// The CLI fixture binds port0; pass its actual callback through the
// public override instead of its pre-bind default port.
const redirectUri = new URL("/api/oauth/callback", server.origin).toString();
const endpoints = { authorizationUrl: `${base}/authorize`, tokenUrl: `${base}/token` };
const slug = IntegrationSlug.make("local-oauth-owner");
const registration = {
slug: OAuthClientSlug.make("local-oauth-app"),
issuer: base,
registrationEndpoint: `${base}/register`,
...endpoints,
resource: `${base}/mcp`,
scopes: ["repo", "read:user"],
tokenEndpointAuthMethodsSupported: ["none"],
redirectUri,
originIntegration: slug,
};
const rejected = yield* client.oauth
.createClient({
payload: {
owner: "user",
slug: OAuthClientSlug.make("forbidden-personal"),
grant: "authorization_code",
...endpoints,
clientId: "unused-client",
clientSecret: "unused-secret",
},
})
.pipe(Effect.flip);
expect(rejected._tag).toBe("InternalError");
const deniedDcr = yield* client.oauth
.registerDynamic({ payload: { ...registration, owner: "user" } })
.pipe(Effect.flip);
expect(deniedDcr._tag).toBe("InternalError");
const before = yield* Effect.promise(() => emulator.ledger.list());
expect(before.some((entry) => entry.path === "/register")).toBe(false);
expect((yield* client.oauth.listClients()).some((entry) => entry.owner === "user")).toBe(
false,
);
yield* client.mcp.addServer({
payload: {
slug,
name: "Local OAuth",
endpoint: `${base}/mcp`,
transport: "remote",
authenticationTemplate: [{ kind: "oauth2" }],
},
});
yield* Effect.addFinalizer(() =>
client.mcp.removeServer({ params: { slug } }).pipe(Effect.ignore),
);
const registered = yield* client.oauth.registerDynamic({
payload: { ...registration, owner: "org" },
});
yield* Effect.addFinalizer(() =>
client.oauth
.removeClient({ params: { slug: registered.client }, payload: { owner: "org" } })
.pipe(Effect.ignore),
);
const started = yield* client.oauth.start({
payload: {
owner: "org",
client: registered.client,
clientOwner: "org",
name: ConnectionName.make("main"),
integration: slug,
template: AuthTemplateSlug.make("oauth2"),
redirectUri,
},
});
if (started.status !== "redirect") return yield* Effect.die("Expected OAuth redirect");
yield* browser.session({ label: "local" }, async ({ page, step }) => {
await step("Sign in to the local console", async () => {
await page.goto(server.url);
await page
.getByTestId("integration-entry-executor")
.first()
.waitFor({ timeout: 30_000 });
});
await step("Authorize an org-owned OAuth client", async () => {
await page.goto(started.authorizationUrl);
await page.getByText("Authorize MCP client", { exact: true }).waitFor();
const authorize = new URL(started.authorizationUrl);
const approved = await page.request.post(`${base}/authorize/approve`, {
form: { ...Object.fromEntries(authorize.searchParams), login: "local-oauth-user" },
maxRedirects: 0,
});
expect(approved.status()).toBe(302);
const callback = approved.headers().location;
if (!callback) throw new Error("Missing OAuth callback");
await page.goto(callback);
await page.getByText("Connected", { exact: true }).waitFor({ timeout: 30_000 });
});
});
const catalog = yield* client.tools.list({ query: { integration: slug } });
expect(catalog.some((entry) => entry.name === "get_me" && entry.owner === "org")).toBe(
true,
);
const ledger = yield* Effect.promise(() => emulator.ledger.list());
expect(
ledger.some((entry) => entry.path === "/token" && entry.response.status === 200),
).toBe(true);
expect(
ledger.some(
(entry) =>
entry.path === "/mcp" &&
entry.response.status === 200 &&
entry.identity.user?.login === "local-oauth-user",
),
).toBe(true);
}),
);
}),
),
);
50 changes: 49 additions & 1 deletion packages/core/sdk/src/oauth-flow.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,54 @@ describe("oauth.start / oauth.complete", () => {
}),
),
);
it.effect("createClient rejects owner user when subject is local", () =>
Effect.gen(function* () {
const executor = yield* createExecutor(makeTestConfig({ plugins, subject: "local" }));
let seen = false;
yield* executor.oauth
.createClient({
owner: "user",
slug: OAuthClientSlug.make("personal"),
authorizationUrl: "https://example.com/authorize",
tokenUrl: "https://example.com/token",
grant: "authorization_code",
clientId: "id",
clientSecret: "secret",
})
.pipe(
Effect.catchTag("StorageError", (err) => {
seen = true;
expect(err.message).toContain("User-owned OAuth clients are not supported");
return Effect.void;
}),
);
expect(seen).toBe(true);
}),
);

it.effect("local user DCR is rejected before contacting the provider", () =>
Effect.scoped(
Effect.gen(function* () {
const server = yield* serveOAuthTestServer();
const executor = yield* createExecutor(makeTestConfig({ plugins, subject: "local" }));
const error = yield* executor.oauth
.registerDynamicClient({
owner: "user",
slug: CLIENT,
issuer: server.issuerUrl,
registrationEndpoint: server.registrationEndpoint,
authorizationUrl: server.authorizationEndpoint,
tokenUrl: server.tokenEndpoint,
scopes: ["read"],
redirectUri: "http://localhost/callback",
})
.pipe(Effect.flip);
expect(Predicate.isTagged("StorageError")(error)).toBe(true);
expect(yield* server.requests).toEqual([]);
expect(yield* executor.oauth.listClients()).toEqual([]);
}),
),
);

it.effect("persists HTTP Basic client auth for code exchange and refresh", () =>
Effect.scoped(
Expand Down Expand Up @@ -999,7 +1047,7 @@ describe("oauth.start / oauth.complete", () => {
);
expect(Predicate.isTagged("OAuthStartError")(error)).toBe(true);
const startError = error as OAuthStartError;
expect(startError.message).toContain("must use a Workspace app");
expect(startError.message).toContain("must use an org-owned OAuth client");
}),
),
);
Expand Down
16 changes: 15 additions & 1 deletion packages/core/sdk/src/oauth-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -878,6 +878,13 @@ export const makeOAuthService = (deps: OAuthServiceDeps): OAuthService => {
input: CreateOAuthClientInput,
): Effect.Effect<OAuthClientSlug, OrgWriteDeniedError | StorageFailure> =>
Effect.gen(function* () {
if (input.owner === "user" && deps.subject === "local") {
return yield* new StorageError({
message:
'User-owned OAuth clients are not supported on single-workspace hosts. Use owner "org" instead.',
cause: undefined,
});
}
// The `first-party:` namespace is reserved for config-declared apps — a
// stored row under it would be shadowed by (or worse, impersonate) the
// host's own app.
Expand Down Expand Up @@ -1393,6 +1400,13 @@ export const makeOAuthService = (deps: OAuthServiceDeps): OAuthService => {
OAuthRegisterDynamicError | OrgWriteDeniedError | StorageFailure
> =>
Effect.gen(function* () {
if (input.owner === "user" && deps.subject === "local") {
return yield* new StorageError({
message:
'User-owned OAuth clients are not supported on single-workspace hosts. Use owner "org" instead.',
cause: undefined,
});
}
yield* deps.guardOrgWrite(input.owner);
const issuer = canonicalDcrIssuer(input.issuer, input.registrationEndpoint);
// Resolved before the reuse decision: a persisted client registered with
Expand Down Expand Up @@ -1671,7 +1685,7 @@ export const makeOAuthService = (deps: OAuthServiceDeps): OAuthService => {
});
if (!firstPartyFlow && input.owner === "org" && input.clientOwner === "user") {
return yield* new OAuthStartError({
message: "A Workspace connection must use a Workspace app.",
message: "An org connection must use an org-owned OAuth client.",
});
}
// Load the app by its EXPLICIT owner (the caller knows it — no derivation).
Expand Down
Loading