Skip to content
Draft
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
6 changes: 6 additions & 0 deletions .changeset/github-app-installation-setup.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@executor-js/sdk": patch
"@executor-js/api": patch
---

Guide first-party GitHub connections through app installation and repository selection before user authorization. Keep the original authorization URL in an expiring, owner-scoped session so web, MCP, and reconnect flows share the same setup step.
4 changes: 4 additions & 0 deletions apps/cloud/src/app-paths.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,10 @@ describe("app-plane dispatch", () => {
expect(servedByAppPlane("/api/oauth/callback", "POST")).toBe(false);
});

it("leaves OAuth setup to Start for sign-in and organization selection", () => {
expect(servedByAppPlane("/api/oauth/setup", "GET")).toBe(false);
});

const appPlane = [
"/api/connections",
"/api/tools",
Expand Down
7 changes: 5 additions & 2 deletions apps/cloud/src/app-paths.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ export const isAppOwnedPath = (pathname: string) =>
//
// POST /api/sentry-tunnel - `sentryTunnelMiddleware` forwards the envelope
// to Sentry; the app has no such route.
// /api/oauth/callback - `oauthCallbackSignInMiddleware` redirects a
// /api/oauth/{callback,setup} - `oauthBrowserSignInMiddleware` redirects a
// signed-out visitor to /login, and start.ts
// rewrites the org-scoped `state` before handing
// off. Routing it early would drop both.
Expand All @@ -41,7 +41,10 @@ export const isAppOwnedPath = (pathname: string) =>
// ---------------------------------------------------------------------------

export const isStartOwnedApiPath = (pathname: string, method: string): boolean =>
(pathname === "/api/sentry-tunnel" && method === "POST") || pathname === "/api/oauth/callback";
(pathname === "/api/sentry-tunnel" && method === "POST") || isOAuthBrowserPath(pathname);

export const isOAuthBrowserPath = (pathname: string): boolean =>
pathname === "/api/oauth/callback" || pathname === "/api/oauth/setup";

export const servedByAppPlane = (pathname: string, method: string): boolean =>
isApiPath(pathname) && !isStartOwnedApiPath(pathname, method);
14 changes: 14 additions & 0 deletions apps/cloud/src/engine/first-party-oauth-clients.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,10 @@ describe("cloud first-party OAuth clients", () => {
optional_scope: "content crm.objects.custom.read crm.schemas.custom.read",
},
});
expect(byName.get("github")).toMatchObject({
authorizationScopes: [],
authorizationSetup: { actionUrl: "https://github.com/apps/executor-sh/installations/new" },
});
expect(byName.get("linear")).toMatchObject({ authorizationScopeSeparator: "," });
expect(byName.get("microsoft")).toMatchObject({
additionalAuthorizationScopes: ["offline_access"],
Expand Down Expand Up @@ -179,3 +183,13 @@ describe("cloud first-party Google app", () => {
}
});
});

it("uses the installation page for the deployment's GitHub App", () => {
const github = firstPartyOAuthClientsFor({
...completeEnv,
FIRST_PARTY_GITHUB_INSTALLATION_URL: "https://github.com/apps/custom-app/installations/new",
}).find((client) => client.name === "github");
expect(github?.authorizationSetup?.actionUrl).toBe(
"https://github.com/apps/custom-app/installations/new",
);
});
10 changes: 10 additions & 0 deletions apps/cloud/src/engine/first-party-oauth-clients.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ export interface FirstPartyOAuthClientEnv {
readonly FIRST_PARTY_GITHUB_CLIENT_SECRET?: string;
readonly FIRST_PARTY_GITHUB_AUTHORIZE_URL?: string;
readonly FIRST_PARTY_GITHUB_TOKEN_URL?: string;
readonly FIRST_PARTY_GITHUB_INSTALLATION_URL?: string;
readonly FIRST_PARTY_GITLAB_CLIENT_ID?: string;
readonly FIRST_PARTY_GITLAB_CLIENT_SECRET?: string;
readonly FIRST_PARTY_GOOGLE_CLIENT_ID?: string;
Expand Down Expand Up @@ -269,6 +270,15 @@ export const firstPartyOAuthClientsFor = (
authorizationUrl:
env.FIRST_PARTY_GITHUB_AUTHORIZE_URL ?? "https://github.com/login/oauth/authorize",
tokenUrl: env.FIRST_PARTY_GITHUB_TOKEN_URL ?? "https://github.com/login/oauth/access_token",
authorizationSetup: {
title: "Connect GitHub",
description:
"Install the GitHub App on your account or organization and choose the repositories Executor can access. Authorizing your account alone does not grant private repository access. Organization access may require an owner's approval.",
actionLabel: "Install or configure GitHub App",
actionUrl:
env.FIRST_PARTY_GITHUB_INSTALLATION_URL ??
"https://github.com/apps/executor-sh/installations/new",
},
integrations: [IntegrationSlug.make("github_rest")],
// GitHub App user access tokens do not use classic OAuth scopes; their
// capabilities come from the app's registered permissions.
Expand Down
2 changes: 2 additions & 0 deletions apps/cloud/src/env-augment.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,8 @@ declare global {
// production (the real github.com endpoints are the defaults).
FIRST_PARTY_GITHUB_AUTHORIZE_URL?: string;
FIRST_PARTY_GITHUB_TOKEN_URL?: string;
// Override when deploying with a different GitHub App registration.
FIRST_PARTY_GITHUB_INSTALLATION_URL?: string;
FIRST_PARTY_GITLAB_CLIENT_ID?: string;
FIRST_PARTY_GITLAB_CLIENT_SECRET?: string;
FIRST_PARTY_GOOGLE_CLIENT_ID?: string;
Expand Down
19 changes: 8 additions & 11 deletions apps/cloud/src/start.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { createMiddleware, createStart } from "@tanstack/react-start";
import { decodeOAuthCallbackState } from "@executor-js/sdk/shared";

import { isAppOwnedPath } from "./app-paths";
import { isAppOwnedPath, isOAuthBrowserPath } from "./app-paths";
import { authGateMiddleware } from "./auth/doc-gate";
import { parseCookie } from "./auth/cookies";
import { ORG_SELECTOR_HEADER } from "./auth/organization";
Expand Down Expand Up @@ -48,9 +48,8 @@ const getApp = async (): Promise<NonNullable<typeof app>> =>
(app ??= (await import("./app")).cloudApiHandler());

const SESSION_COOKIE = "wos-session";
const OAUTH_CALLBACK_PATH = "/api/oauth/callback";

const oauthCallbackOrgScopedRequest = (request: Request): Request => {
const oauthBrowserOrgScopedRequest = (request: Request): Request => {
const url = new URL(request.url);
const callbackState = decodeOAuthCallbackState(url.searchParams.get("state"));
if (callbackState === null) return request;
Expand All @@ -61,12 +60,9 @@ const oauthCallbackOrgScopedRequest = (request: Request): Request => {
return new Request(rewritten, { headers });
};

const oauthCallbackSignInMiddleware = createMiddleware({ type: "request" }).server(
const oauthBrowserSignInMiddleware = createMiddleware({ type: "request" }).server(
({ pathname, request, next }) => {
if (
pathname !== OAUTH_CALLBACK_PATH ||
(request.method !== "GET" && request.method !== "HEAD")
) {
if (!isOAuthBrowserPath(pathname) || (request.method !== "GET" && request.method !== "HEAD")) {
return next();
}
const sealed = parseCookie(request.headers.get("cookie"), SESSION_COOKIE);
Expand All @@ -89,8 +85,9 @@ const oauthCallbackSignInMiddleware = createMiddleware({ type: "request" }).serv
const appRequestMiddleware = createMiddleware({ type: "request" }).server(
async ({ pathname, request, next }) => {
if (isAppOwnedPath(pathname)) {
const scopedRequest =
pathname === OAUTH_CALLBACK_PATH ? oauthCallbackOrgScopedRequest(request) : request;
const scopedRequest = isOAuthBrowserPath(pathname)
? oauthBrowserOrgScopedRequest(request)
: request;
return (await getApp()).handler(prepareMcpOrgScope(scopedRequest));
}
return next();
Expand All @@ -113,7 +110,7 @@ export const startInstance = createStart(() => ({
docsProxyMiddleware,
sentryTunnelMiddleware,
posthogProxyMiddleware,
oauthCallbackSignInMiddleware,
oauthBrowserSignInMiddleware,
appRequestMiddleware,
authGateMiddleware,
],
Expand Down
81 changes: 67 additions & 14 deletions e2e/scenarios/first-party-oauth.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,9 @@
//
// 1. Listing: `oauth.listClients` surfaces `first-party:github` with a
// `first_party` origin and its public client id — no create call ever ran.
// 2. Flow: `oauth.start` through the first-party slug redirects to the
// provider's authorize endpoint carrying the env-configured client id and
// this platform's `/api/oauth/callback` — proof the config-resolved
// identity (not a stored row) drives the flow. The redirect is asserted,
// never followed: github.com is not visited.
// 2. Flow: the returned URL opens installation guidance, then preserves the
// original provider authorization URL, PKCE and organization routing.
// GitHub's links are checked without contacting the live provider.
// 3. Guardrails: the reserved `first-party:` namespace is rejected by
// createClient, so no org can shadow the host's app with its own row.
import { randomBytes } from "node:crypto";
Expand Down Expand Up @@ -93,7 +91,7 @@ const googleShapedIntegrationSpec = (scopes: readonly string[]) => ({
});

scenario(
"First-party OAuth · the host-declared GitHub app is listed and drives the authorize redirect",
"First-party OAuth · GitHub setup includes installation before authorization",
{},
Effect.scoped(
Effect.gen(function* () {
Expand All @@ -103,6 +101,7 @@ scenario(
// their own OAuth apps through its existing registration flow.
if (target.name !== "cloud") return;
const { client: makeApiClient } = yield* Api;
const browser = yield* Browser;
const identity = yield* target.newIdentity();
const client = yield* makeApiClient(api, identity);

Expand All @@ -116,6 +115,9 @@ scenario(
// 2. A start through the first-party slug builds GitHub's authorize URL
// from the config identity and this platform's served callback.
const integration = IntegrationSlug.make(unique("fpgh"));
yield* Effect.addFinalizer(() =>
client.openapi.removeSpec({ params: { slug: integration } }).pipe(Effect.ignore),
);
yield* client.openapi.addSpec({
payload: { ...githubShapedIntegrationSpec, slug: integration },
});
Expand All @@ -129,16 +131,67 @@ scenario(
template: AuthTemplateSlug.make("oauth"),
},
});
expect(started.status, "oauth.start redirects to the provider").toBe("redirect");
const authorizationUrl = started.status === "redirect" ? started.authorizationUrl : "";
const authorize = new URL(authorizationUrl);
expect(authorize.origin + authorize.pathname).toBe(
"https://github.com/login/oauth/authorize",
expect(started.status, "oauth.start opens setup").toBe("redirect");
if (started.status !== "redirect") return yield* Effect.die("expected redirect");
yield* Effect.addFinalizer(() =>
client.oauth.cancel({ payload: { state: started.state } }).pipe(Effect.ignore),
);
expect(authorize.searchParams.get("client_id")).toBe("e2e-first-party-github");
expect(authorize.searchParams.get("redirect_uri")).toBe(
new URL("/api/oauth/callback", target.baseUrl).toString(),
const setupUrl = new URL(started.authorizationUrl);
expect(setupUrl.origin + setupUrl.pathname).toBe(
new URL("/api/oauth/setup", target.baseUrl).toString(),
);
// A URL supplied by a caller must never replace the saved continuation.
setupUrl.searchParams.set("authorization_url", "https://example.invalid/phishing");
yield* browser.session(identity, async ({ page, step }) => {
await step("Open GitHub setup and find repository installation", async () => {
const response = await page.goto(setupUrl.toString());
expect(response?.status()).toBe(200);
expect(response?.headers()["cache-control"]).toBe("no-store");
await page.getByRole("heading", { name: "Connect GitHub", exact: true }).waitFor();
const install = page.getByRole("link", { name: "Install or configure GitHub App" });
expect(await install.getAttribute("href")).toBe(
"https://github.com/apps/executor-sh/installations/new",
);
expect(await install.getAttribute("target")).toBe("_blank");
const authorize = new URL(
(await page
.getByRole("link", { name: "Continue to authorization" })
.getAttribute("href"))!,
);
expect(authorize.origin + authorize.pathname).toBe(
"https://github.com/login/oauth/authorize",
);
expect(authorize.searchParams.get("client_id")).toBe("e2e-first-party-github");
expect(authorize.searchParams.get("redirect_uri")).toBe(
new URL("/api/oauth/callback", target.baseUrl).toString(),
);
expect(authorize.searchParams.get("state")).toBe(setupUrl.searchParams.get("state"));
expect(authorize.searchParams.get("code_challenge_method")).toBe("S256");
expect(authorize.searchParams.get("code_challenge")).toMatch(/^[A-Za-z0-9_-]{43}$/);
expect(authorize.searchParams.has("scope")).toBe(false);
});
await step("Read setup in light mode", async () => {
await page.emulateMedia({ colorScheme: "light" });
});
await step("Choose access on a narrow screen", async () => {
await page.setViewportSize({ width: 390, height: 844 });
expect(
await page.locator("body").evaluate((body) => body.scrollWidth <= window.innerWidth),
).toBe(true);
});
await step("Read setup in dark mode on a narrow screen", async () => {
await page.emulateMedia({ colorScheme: "dark" });
});
await step("Cancel setup and reopen the expired link", async () => {
await Effect.runPromise(client.oauth.cancel({ payload: { state: started.state } }));
const response = await page.goto(setupUrl.toString());
expect(response?.status()).toBe(410);
await page.getByRole("heading", { name: "Connection setup unavailable" }).waitFor();
expect(await page.getByRole("link", { name: "Continue to authorization" }).count()).toBe(
0,
);
});
});

// 3. The reserved namespace cannot be shadowed by a stored row. The
// server rejects with a StorageError, which the HTTP edge scrubs to an
Expand Down
25 changes: 25 additions & 0 deletions packages/core/api/src/handlers/oauth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { HttpServerResponse } from "effect/unstable/http";
import { Effect, Option, Schema } from "effect";

import { runOAuthCallback, type PopupErrorMessage } from "../oauth-popup";
import { oauthSetupDocument, oauthSetupUnavailableDocument } from "../oauth-setup";
import {
OAUTH_POPUP_MESSAGE_TYPE,
OAuthCompleteError,
Expand All @@ -20,6 +21,7 @@ import {
OAuthState,
type Connection,
type ConnectResult,
decodeOAuthCallbackState,
} from "@executor-js/sdk";

import { ExecutorApi } from "../api";
Expand Down Expand Up @@ -173,6 +175,29 @@ export const OAuthHandlers = HttpApiBuilder.group(ExecutorApi, "oauth", (handler
}),
),
)
.handle("setup", ({ query }) =>
capture(
Effect.gen(function* () {
const executor = yield* ExecutorService;
const state = decodeOAuthCallbackState(query.state)?.state ?? query.state;
const setup = yield* executor.oauth.getSetup(OAuthState.make(state));
return HttpServerResponse.text(oauthSetupDocument(setup), {
contentType: "text/html; charset=utf-8",
headers: { "cache-control": "no-store", "referrer-policy": "no-referrer" },
});
}).pipe(
Effect.catchTag("OAuthSessionNotFoundError", () =>
Effect.succeed(
HttpServerResponse.text(oauthSetupUnavailableDocument(), {
contentType: "text/html; charset=utf-8",
status: 410,
headers: { "cache-control": "no-store" },
}),
),
),
),
),
)
.handle("complete", ({ payload }) =>
capture(
Effect.gen(function* () {
Expand Down
8 changes: 8 additions & 0 deletions packages/core/api/src/html-escape.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
/** Escape text and quoted attribute values in server-rendered HTML. */
export const escapeHtml = (value: string): string =>
value
.replaceAll("&", "&amp;")
.replaceAll("<", "&lt;")
.replaceAll(">", "&gt;")
.replaceAll('"', "&quot;")
.replaceAll("'", "&#39;");
10 changes: 2 additions & 8 deletions packages/core/api/src/oauth-popup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@

import { Cause, Effect } from "effect";

import { escapeHtml } from "./html-escape";

import {
decodeOAuthCallbackState,
OAUTH_POPUP_MESSAGE_TYPE,
Expand Down Expand Up @@ -48,14 +50,6 @@ export const setOAuthCompletionListener = (listener: OAuthCompletionListener | n
// HTML generation
// ---------------------------------------------------------------------------

const escapeHtml = (value: string): string =>
value
.replaceAll("&", "&amp;")
.replaceAll("<", "&lt;")
.replaceAll(">", "&gt;")
.replaceAll('"', "&quot;")
.replaceAll("'", "&#39;");

/**
* Serialize for embedding inside a `<script>` tag. Escapes the characters
* that could prematurely terminate the script or mislead an HTML parser
Expand Down
39 changes: 39 additions & 0 deletions packages/core/api/src/oauth-setup.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
import { describe, expect, it } from "@effect/vitest";

import { oauthSetupDocument } from "./oauth-setup";

const setup = {
title: "Connect GitHub",
description: "Choose repositories.",
actionLabel: "Install GitHub App",
actionUrl: "https://github.com/apps/example/installations/new",
};

describe("OAuth setup document", () => {
it("escapes host text and query parameters without exposing markup or a referrer", () => {
const html = oauthSetupDocument({
setup: { ...setup, title: '<script>alert("title")</script>', description: "<img src=x>" },
authorizationUrl: "https://github.com/login/oauth/authorize?state=a&client_id=b",
});
expect(html).not.toContain("<script>");
expect(html).not.toContain("<img");
expect(html).toContain("&lt;script&gt;");
expect(html).toContain("?state=a&amp;client_id=b");
expect(html).toContain('name="referrer" content="no-referrer"');
expect(html).toMatch(/target="_blank"\s+rel="noopener noreferrer"/);
});

it("refuses executable or malformed link targets", () => {
for (const url of ["javascript:alert(1)", "data:text/html,hello", "not a URL"]) {
expect(
oauthSetupDocument({
setup: { ...setup, actionUrl: url },
authorizationUrl: "https://github.com/",
}),
).toContain("Connection setup unavailable");
expect(oauthSetupDocument({ setup, authorizationUrl: url })).toContain(
"Connection setup unavailable",
);
}
});
});
Loading
Loading