History

Fix self-host OAuth callback recovery

Rhys Sullivan committed 4 months ago

Commit 1630b111 parent 638c90b8 files · +331 −19Browse files

Unified · Split

8 files
  1. apps/host-selfhost/src/app.ts +8 −2
  2. apps/host-selfhost/src/auth/oauth-callback-login.test.ts +44 −0
  3. apps/host-selfhost/src/auth/oauth-callback-login.ts +23 −0
  4. apps/host-selfhost/src/auth/return-to.test.ts +58 −0
  5. apps/host-selfhost/src/auth/return-to.ts +14 −0
  6. apps/host-selfhost/src/serve.ts +35 −16
  7. apps/host-selfhost/web/login.tsx +3 −1
  8. e2e/selfhost/oauth-callback-unauthenticated.test.ts +146 −0
apps/host-selfhost/src/app.ts+8−2
@@ -21,6 +21,7 @@ import { · 20 unmodified lines
21import { makeSelfHostMcpSeams } from "./mcp";21import { makeSelfHostMcpSeams } from "./mcp";
22import { selfHostPlugins } from "./plugins";22import { selfHostPlugins } from "./plugins";
23import { ErrorCaptureLive } from "./observability";23import { ErrorCaptureLive } from "./observability";
24import { oauthCallbackSignInRedirectLocation } from "./auth/oauth-callback-login";
24 25
25// ===========================================================================26// ===========================================================================
26// The self-hosted Executor app, as ONE `ExecutorApp.make` call.27// The self-hosted Executor app, as ONE `ExecutorApp.make` call.
@@ -113,6 +114,7 @@ export const makeSelfHostApp = async (options: MakeSelfHostAppOptions = {}) => { · 86 unmodified lines
113 // (it can't prove each host's resolution); self-host narrows it here.114 // (it can't prove each host's resolution); self-host narrows it here.
114 AppLayer: appLayer as Layer.Layer<never>,115 AppLayer: appLayer as Layer.Layer<never>,
115 toWebHandler,116 toWebHandler,
117 betterAuth,
116 closeDb: async () => {118 closeDb: async () => {
117 await mcp.close();119 await mcp.close();
118 await dbHandle.close();120 await dbHandle.close();
@@ -132,10 +134,14 @@ export interface SelfHostApiHandler { · 13 unmodified lines
132export const makeSelfHostApiHandler = async (134export const makeSelfHostApiHandler = async (
133 options: MakeSelfHostAppOptions = {},135 options: MakeSelfHostAppOptions = {},
134): Promise<SelfHostApiHandler> => {136): Promise<SelfHostApiHandler> => {
135 const { toWebHandler, closeDb } = await makeSelfHostApp(options);137 const { toWebHandler, betterAuth, closeDb } = await makeSelfHostApp(options);
136 const web = toWebHandler();138 const web = toWebHandler();
137 return {139 return {
138 handler: web.handler,140 handler: async (request) => {
141 const location = await oauthCallbackSignInRedirectLocation(request, betterAuth.auth);
142 if (location) return new Response(null, { status: 302, headers: { location } });
143 return web.handler(request);
144 },
139 dispose: async () => {145 dispose: async () => {
140 await web.dispose();146 await web.dispose();
141 await closeDb();147 await closeDb();
apps/host-selfhost/src/auth/oauth-callback-login.test.ts+44−0
@@ -0,0 +1,44 @@
1import { describe, expect, it } from "@effect/vitest";
2
3import { oauthCallbackSignInRedirectLocation } from "./oauth-callback-login";
4
5const auth = (session: unknown | null) => ({
6 api: {
7 getSession: async () => session,
8 },
9});
10
11describe("oauthCallbackSignInRedirectLocation", () => {
12 it("redirects a signed-out OAuth callback to login with returnTo", async () => {
13 await expect(
14 oauthCallbackSignInRedirectLocation(
15 new Request("http://selfhost.test/api/oauth/callback?state=s1&code=c1"),
16 auth(null),
17 ),
18 ).resolves.toBe("/login?returnTo=%2Fapi%2Foauth%2Fcallback%3Fstate%3Ds1%26code%3Dc1");
19 });
20
21 it("does not redirect a signed-in OAuth callback", async () => {
22 await expect(
23 oauthCallbackSignInRedirectLocation(
24 new Request("http://selfhost.test/api/oauth/callback?state=s1&code=c1"),
25 auth({ user: { id: "user_1" } }),
26 ),
27 ).resolves.toBeNull();
28 });
29
30 it("ignores other paths and non-browser methods", async () => {
31 await expect(
32 oauthCallbackSignInRedirectLocation(
33 new Request("http://selfhost.test/api/oauth/callback?state=s1", { method: "POST" }),
34 auth(null),
35 ),
36 ).resolves.toBeNull();
37 await expect(
38 oauthCallbackSignInRedirectLocation(
39 new Request("http://selfhost.test/api/oauth/callback/extra?state=s1"),
40 auth(null),
41 ),
42 ).resolves.toBeNull();
43 });
44});
apps/host-selfhost/src/auth/oauth-callback-login.ts+23−0
@@ -0,0 +1,23 @@
1import { loginPath } from "./return-to";
2
3export const OAUTH_CALLBACK_PATH = "/api/oauth/callback";
4
5interface SessionAuth {
6 readonly api: {
7 readonly getSession: (input: { readonly headers: Headers }) => Promise<unknown | null>;
8 };
9}
10
11export const oauthCallbackSignInRedirectLocation = async (
12 request: Request,
13 auth: SessionAuth,
14): Promise<string | null> => {
15 if (request.method !== "GET" && request.method !== "HEAD") return null;
16 const url = new URL(request.url);
17 if (url.pathname !== OAUTH_CALLBACK_PATH) return null;
18
19 const session = await auth.api.getSession({ headers: request.headers });
20 if (session) return null;
21
22 return loginPath(`${url.pathname}${url.search}`);
23};
apps/host-selfhost/src/auth/return-to.test.ts+58−0
@@ -0,0 +1,58 @@
1import { describe, expect, it } from "@effect/vitest";
2
3import { isSafeReturnTo, loginPath, safeReturnTo } from "./return-to";
4
5describe("isSafeReturnTo", () => {
6 const safe = [
7 "/",
8 "/tools",
9 "/integrations/sentry?addAccount=1",
10 "/api-keys",
11 "/api/oauth/callback?state=oauth-state&code=provider-code",
12 ];
13 for (const path of safe) {
14 it(`allows ${path}`, () => {
15 expect(isSafeReturnTo(path)).toBe(true);
16 });
17 }
18
19 const unsafe = [
20 "https://evil.example",
21 "//evil.example",
22 "/api/auth/logout",
23 "/api/oauth/callback/extra?state=oauth-state",
24 "/api",
25 "javascript:alert(1)",
26 "tools",
27 "",
28 ];
29 for (const path of unsafe) {
30 it(`rejects ${JSON.stringify(path)}`, () => {
31 expect(isSafeReturnTo(path)).toBe(false);
32 });
33 }
34});
35
36describe("safeReturnTo", () => {
37 it("passes a safe path through", () => {
38 expect(safeReturnTo("/tools")).toBe("/tools");
39 });
40
41 it("nulls unsafe and absent values", () => {
42 expect(safeReturnTo("https://evil.example")).toBeNull();
43 expect(safeReturnTo(null)).toBeNull();
44 expect(safeReturnTo(undefined)).toBeNull();
45 });
46});
47
48describe("loginPath", () => {
49 it("omits returnTo for the root", () => {
50 expect(loginPath("/")).toBe("/login");
51 });
52
53 it("carries OAuth callback resumes URI-encoded", () => {
54 expect(loginPath("/api/oauth/callback?state=oauth-state&code=provider-code")).toBe(
55 "/login?returnTo=%2Fapi%2Foauth%2Fcallback%3Fstate%3Doauth-state%26code%3Dprovider-code",
56 );
57 });
58});
apps/host-selfhost/src/auth/return-to.ts+14−0
@@ -0,0 +1,14 @@
1const pathPart = (path: string): string => path.split(/[?#]/, 1)[0] ?? "";
2
3const isOAuthCallbackReturnTo = (path: string): boolean => pathPart(path) === "/api/oauth/callback";
4
5export const isSafeReturnTo = (path: string): boolean =>
6 path.startsWith("/") &&
7 !path.startsWith("//") &&
8 (!/^\/api(\/|$)/.test(path) || isOAuthCallbackReturnTo(path));
9
10export const safeReturnTo = (path: string | null | undefined): string | null =>
11 path && isSafeReturnTo(path) ? path : null;
12
13export const loginPath = (returnTo: string): string =>
14 returnTo === "/" ? "/login" : `/login?returnTo=${encodeURIComponent(returnTo)}`;
apps/host-selfhost/src/serve.ts+35−16
@@ -19,6 +19,7 @@ import { · 18 unmodified lines
19 HttpMiddleware,19 HttpMiddleware,
20 HttpRouter,20 HttpRouter,
21 HttpServerRequest,21 HttpServerRequest,
22 HttpServerResponse,
22 HttpStaticServer,23 HttpStaticServer,
23} from "effect/unstable/http";24} from "effect/unstable/http";
24import { BunFileSystem, BunHttpServer, BunPath, BunRuntime } from "@effect/platform-bun";25import { BunFileSystem, BunHttpServer, BunPath, BunRuntime } from "@effect/platform-bun";
@@ -26,6 +27,11 @@ import { Effect, Layer } from "effect"; · 1 unmodified line
26 27
27import { makeSelfHostApp } from "./app";28import { makeSelfHostApp } from "./app";
28import { loadConfig } from "./config";29import { loadConfig } from "./config";
30import type { BetterAuthHandle } from "./auth";
31import {
32 OAUTH_CALLBACK_PATH,
33 oauthCallbackSignInRedirectLocation,
34} from "./auth/oauth-callback-login";
29import { stripMcpOrgSegment } from "./mcp/org-path";35import { stripMcpOrgSegment } from "./mcp/org-path";
30 36
31const distDir = fileURLToPath(new URL("../dist/", import.meta.url));37const distDir = fileURLToPath(new URL("../dist/", import.meta.url));
@@ -33,24 +39,37 @@ const distDir = fileURLToPath(new URL("../dist/", import.meta.url)); · 1 unmodified line
33// Rewrite `/<org>/mcp` (and its OAuth discovery path) to the bare path before39// Rewrite `/<org>/mcp` (and its OAuth discovery path) to the bare path before
34// routing, so the "Connect an agent" card's org-pinned URL reaches the real40// routing, so the "Connect an agent" card's org-pinned URL reaches the real
35// `/mcp` route — see ./mcp/org-path. A no-op for every other request.41// `/mcp` route — see ./mcp/org-path. A no-op for every other request.
36const mcpOrgPathRewrite = HttpMiddleware.make((httpApp) =>42const selfHostHttpMiddleware = (betterAuth: BetterAuthHandle) =>
37 Effect.gen(function* () {43 HttpMiddleware.make((httpApp) =>
38 const request = yield* HttpServerRequest.HttpServerRequest;44 Effect.gen(function* () {
39 const url = new URL(request.url, "http://host.internal");45 const request = yield* HttpServerRequest.HttpServerRequest;
40 const rewritten = stripMcpOrgSegment(url.pathname);46 const url = new URL(request.url, "http://host.internal");
41 if (rewritten === null) return yield* httpApp;47 if (
42 return yield* httpApp.pipe(48 url.pathname === OAUTH_CALLBACK_PATH &&
43 Effect.provideService(49 (request.method === "GET" || request.method === "HEAD")
44 HttpServerRequest.HttpServerRequest,50 ) {
45 request.modify({ url: `${rewritten}${url.search}` }),51 const headers = new Headers(request.headers as Record<string, string>);
46 ),52 const webRequest = new Request(url, { method: request.method, headers });
47 );53 const location = yield* Effect.promise(() =>
48 }),54 oauthCallbackSignInRedirectLocation(webRequest, betterAuth.auth),
49);55 );
56 if (location) return HttpServerResponse.redirect(location, { status: 302 });
57 }
58
59 const rewritten = stripMcpOrgSegment(url.pathname);
60 if (rewritten === null) return yield* httpApp;
61 return yield* httpApp.pipe(
62 Effect.provideService(
63 HttpServerRequest.HttpServerRequest,
64 request.modify({ url: `${rewritten}${url.search}` }),
65 ),
66 );
67 }),
68 );
50 69
51export const startServer = async (): Promise<void> => {70export const startServer = async (): Promise<void> => {
52 const config = loadConfig();71 const config = loadConfig();
53 const { AppLayer } = await makeSelfHostApp();72 const { AppLayer, betterAuth } = await makeSelfHostApp();
54 73
55 // Serve the built SPA. Specific API/docs/auth/mcp routes take precedence;74 // Serve the built SPA. Specific API/docs/auth/mcp routes take precedence;
56 // `spa: true` falls back to index.html for any other path (client routing).75 // `spa: true` falls back to index.html for any other path (client routing).
@@ -60,7 +79,7 @@ export const startServer = async (): Promise<void> => { · 3 unmodified lines
60 );79 );
61 80
62 const ServerLive = HttpRouter.serve(Layer.mergeAll(AppLayer, StaticLive), {81 const ServerLive = HttpRouter.serve(Layer.mergeAll(AppLayer, StaticLive), {
63 middleware: mcpOrgPathRewrite,82 middleware: selfHostHttpMiddleware(betterAuth),
64 }).pipe(83 }).pipe(
65 Layer.provide(84 Layer.provide(
66 BunHttpServer.layer({ hostname: config.host, port: config.port, idleTimeout: 0 }),85 BunHttpServer.layer({ hostname: config.host, port: config.port, idleTimeout: 0 }),
apps/host-selfhost/web/login.tsx+3−1
@@ -5,6 +5,7 @@ import { Input } from "@executor-js/react/components/input"; · 4 unmodified lines
5import { Label } from "@executor-js/react/components/label";5import { Label } from "@executor-js/react/components/label";
6 6
7import { authClient } from "./auth-client";7import { authClient } from "./auth-client";
8import { safeReturnTo } from "../src/auth/return-to";
8 9
9// Self-host login: email + password sign-in via Better Auth. On success we10// Self-host login: email + password sign-in via Better Auth. On success we
10// reload so the shared AuthProvider re-reads /account/me and the AuthGate swaps11// reload so the shared AuthProvider re-reads /account/me and the AuthGate swaps
@@ -15,6 +16,7 @@ import { authClient } from "./auth-client"; · 4 unmodified lines
15// redeeming an invite — either the full /join/<code> link, or by entering the16// redeeming an invite — either the full /join/<code> link, or by entering the
16// code here ("Have an invite code?"), which forwards to the same join page.17// code here ("Have an invite code?"), which forwards to the same join page.
17export const LoginPage = () => {18export const LoginPage = () => {
19 const returnTo = safeReturnTo(new URLSearchParams(window.location.search).get("returnTo")) ?? "/";
18 const [mode, setMode] = useState<"signin" | "code">("signin");20 const [mode, setMode] = useState<"signin" | "code">("signin");
19 const [email, setEmail] = useState("");21 const [email, setEmail] = useState("");
20 const [password, setPassword] = useState("");22 const [password, setPassword] = useState("");
@@ -32,7 +34,7 @@ export const LoginPage = () => { · 11 unmodified lines
32 setError(result.error.message ?? "Sign in failed");34 setError(result.error.message ?? "Sign in failed");
33 return;35 return;
34 }36 }
35 window.location.href = "/";37 window.location.href = returnTo;
36 };38 };
37 39
38 const redeem = (event: FormEvent) => {40 const redeem = (event: FormEvent) => {
e2e/selfhost/oauth-callback-unauthenticated.test.ts+146−0
@@ -0,0 +1,146 @@
1import { randomBytes } from "node:crypto";
2
3import { expect } from "@effect/vitest";
4import { Effect } from "effect";
5import { composePluginApi } from "@executor-js/api/server";
6import { openApiHttpPlugin } from "@executor-js/plugin-openapi/api";
7import {
8 AuthTemplateSlug,
9 ConnectionName,
10 IntegrationSlug,
11 OAuthClientSlug,
12} from "@executor-js/sdk/shared";
13import { serveOAuthTestServer } from "@executor-js/sdk/testing";
14
15import { scenario } from "../src/scenario";
16import { Api, Browser, Target } from "../src/services";
17
18const api = composePluginApi([openApiHttpPlugin()] as const);
19
20const unique = (prefix: string) => `${prefix}_${randomBytes(4).toString("hex")}`;
21
22const oauthIntegrationSpec = (oauth: {
23 readonly authorizationEndpoint: string;
24 readonly tokenEndpoint: string;
25}) =>
26 ({
27 spec: {
28 kind: "blob" as const,
29 value: JSON.stringify({
30 openapi: "3.0.3",
31 info: { title: "OAuth-protected API", version: "1.0.0" },
32 paths: {
33 "/me": {
34 get: {
35 operationId: "getMe",
36 tags: ["default"],
37 responses: { "200": { description: "the caller" } },
38 },
39 },
40 },
41 }),
42 },
43 baseUrl: "http://127.0.0.1:59999",
44 authenticationTemplate: [
45 {
46 slug: "oauth",
47 kind: "oauth2" as const,
48 authorizationUrl: oauth.authorizationEndpoint,
49 tokenUrl: oauth.tokenEndpoint,
50 scopes: ["read"],
51 },
52 ],
53 }) as const;
54
55scenario(
56 "OAuth callback · a signed-out self-host callback uses login returnTo and resumes the connection",
57 {},
58 Effect.gen(function* () {
59 const target = yield* Target;
60 const { client: makeApiClient } = yield* Api;
61 const browser = yield* Browser;
62 const oauth = yield* serveOAuthTestServer();
63 const identity = yield* target.newIdentity();
64 const client = yield* makeApiClient(api, identity);
65
66 const integration = IntegrationSlug.make(unique("selfhostsignedoutcb"));
67 yield* client.openapi.addSpec({
68 payload: { ...oauthIntegrationSpec(oauth), slug: integration },
69 });
70
71 const clientSlug = OAuthClientSlug.make(unique("selfhostsignedoutc"));
72 yield* client.oauth.createClient({
73 payload: {
74 owner: "org",
75 slug: clientSlug,
76 authorizationUrl: oauth.authorizationEndpoint,
77 tokenUrl: oauth.tokenEndpoint,
78 grant: "authorization_code",
79 clientId: "test-client",
80 clientSecret: "test-secret",
81 },
82 });
83
84 const started = yield* client.oauth.start({
85 payload: {
86 client: clientSlug,
87 clientOwner: "org",
88 owner: "org",
89 name: ConnectionName.make("main"),
90 integration,
91 template: AuthTemplateSlug.make("oauth"),
92 },
93 });
94 expect(started.status, "oauth.start begins at the provider").toBe("redirect");
95 const authorizationUrl = started.status === "redirect" ? started.authorizationUrl : "";
96
97 const authorize = yield* Effect.promise(() => fetch(authorizationUrl, { redirect: "manual" }));
98 expect(authorize.status, "the provider asks the user to log in").toBe(302);
99 const consent = yield* Effect.promise(() =>
100 fetch(authorize.headers.get("location") ?? "", {
101 method: "POST",
102 redirect: "manual",
103 headers: {
104 authorization: `Basic ${Buffer.from("alice:password").toString("base64")}`,
105 },
106 }),
107 );
108 expect(consent.status, "provider consent redirects back to Executor").toBe(302);
109 const callback = new URL(consent.headers.get("location") ?? "");
110 const callbackPath = `${callback.pathname}${callback.search}`;
111
112 yield* browser.session({ label: "anonymous" }, async ({ page, step }) => {
113 await step("Provider sends a signed-out browser to the OAuth callback", async () => {
114 const response = await page.goto(callbackPath, { waitUntil: "networkidle" });
115 expect(response?.status(), "the callback redirects into the login flow").toBe(200);
116 await page.getByText("Sign in to your instance").waitFor();
117 });
118
119 const loginUrl = new URL(page.url());
120 expect(loginUrl.pathname, "the signed-out callback lands on the sign-in page").toBe("/login");
121 expect(
122 loginUrl.searchParams.get("returnTo"),
123 "login preserves the callback so it can resume after sign-in",
124 ).toBe(callbackPath);
125
126 await step("Sign in resumes the original OAuth callback", async () => {
127 await page.getByLabel("Email").fill(identity.credentials!.email);
128 await page.getByLabel("Password").fill(identity.credentials!.password);
129 await page.getByRole("button", { name: "Sign in" }).click();
130 await page.waitForURL((url) => url.pathname === "/api/oauth/callback", {
131 timeout: 30_000,
132 });
133 await page.waitForFunction(() => document.body.innerText.includes("Connected"), null, {
134 timeout: 30_000,
135 });
136 });
137
138 const body = (await page.locator("body").textContent())?.trim() ?? "";
139 expect(new URL(page.url()).pathname, "the login returnTo lands back on the callback").toBe(
140 "/api/oauth/callback",
141 );
142 expect(body, "the callback completes after the sign-in recovery").toContain("Connected");
143 expect(body, "the raw protected API response is not shown").not.toContain("Unauthorized");
144 });
145 }).pipe(Effect.scoped),
146);