Skip to content
Merged
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
27 changes: 24 additions & 3 deletions src/orb/oauth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,9 +68,25 @@ export async function verifyInstallationAdmin(
}

async function handleOrbEnrollment(c: Context<{ Bindings: Env }>, code: string, installationId: number): Promise<Response> {
const token = await exchangeOrbOAuthCode(c.env, code);
// A thrown network error (DNS failure, or a timeout past timeoutFetch's own retry budget) from any of the three
// GitHub calls below degrades to the same clean landing page a bad HTTP *response* already produces, instead of
// escaping handleOrbOAuthCallback as an uncaught framework 500. Mirrors the failure-doesn't-escape convention in
// webhook.ts/relay.ts/ingest.ts. The calls stay separate (not one body-wide wrapper) so a throw is never confused
// with a DB/broker fault after identity is established.
const identityError = () => c.html(landingPage(c.env, "Couldn't verify your GitHub identity", "We couldn't reach GitHub to verify your identity — try the install again."), 400);
let token: string | null;
try {
token = await exchangeOrbOAuthCode(c.env, code);
} catch {
return identityError();
}
if (!token) return c.html(landingPage(c.env, "Couldn't verify your GitHub identity", "The authorization didn't complete — re-run the install from GitHub and try again."), 400);
const user = await fetchOrbOAuthUser(token);
let user: GitHubUser | null;
try {
user = await fetchOrbOAuthUser(token);
} catch {
return identityError();
}
if (!user) return c.html(landingPage(c.env, "Couldn't verify your GitHub identity", "We couldn't read your GitHub account — try the install again."), 400);
const install = await c.env.DB.prepare("SELECT account_login, account_type, account_id, registered, self_enrollment_disabled, suspended_at, removed_at FROM orb_github_installations WHERE installation_id = ?")
.bind(installationId)
Expand All @@ -79,7 +95,12 @@ async function handleOrbEnrollment(c: Context<{ Bindings: Env }>, code: string,
// The admin-of-installation check is the authorization gate — it runs BEFORE we reveal or change any state, so a
// non-admin learns nothing about the install and can never enroll someone else's. It binds to the immutable
// GitHub account id (logins can be renamed/reused), so a stale account_login can never grant access.
const isAdmin = await verifyInstallationAdmin(token, user.login, user.id, install.account_login, install.account_type, install.account_id);
let isAdmin: boolean;
try {
isAdmin = await verifyInstallationAdmin(token, user.login, user.id, install.account_login, install.account_type, install.account_id);
} catch {
return identityError();
}
if (!isAdmin) return c.html(landingPage(c.env, "Admin access required", "You must be an admin of this installation's account to enroll it for self-host."), 403);
if (install.removed_at !== null || install.suspended_at !== null) return c.html(landingPage(c.env, "Installation not active", "This installation is suspended or uninstalled — re-install the Orb App, then retry."), 403);
if (install.self_enrollment_disabled === 1) return c.html(landingPage(c.env, "Installation disabled", "This installation was disabled by the operator — contact the operator to re-enable self-host enrollment."), 403);
Expand Down
52 changes: 52 additions & 0 deletions test/integration/orb-oauth.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -222,3 +222,55 @@ describe("maintainer self-enrollment via the OAuth callback", () => {
expect(await (await app.request("/v1/orb/oauth/callback?code=abc&installation_id=0", {}, brokeredEnv())).text()).toContain("LoopOver Orb connected"); // installationId > 0 false
});
});

describe("self-enrollment degrades a thrown GitHub network error to the clean identity page (not an uncaught 500)", () => {
const app = createApp();
const db = (e: Env) => e.DB as unknown as TestD1Database;
const brokeredEnv = () => createTestEnv({ ORB_BROKER_ENABLED: "true", ORB_GITHUB_CLIENT_ID: "id", ORB_GITHUB_CLIENT_SECRET: "sec" });
const seedInstall = (e: Env, cols: Record<string, string | number>) => {
const keys = Object.keys(cols);
return db(e).prepare(`INSERT INTO orb_github_installations (${keys.join(", ")}) VALUES (${keys.map(() => "?").join(", ")})`).bind(...keys.map((k) => cols[k] as string | number)).run();
};
// Each of the three network calls resolves through the module's default timeoutFetch, so spying it lets us reject
// exactly one URL while the earlier calls succeed — reaching the specific call under test before it throws.
const rejectAt = (throwUrlFragment: string) =>
vi.spyOn(githubClientModule, "timeoutFetch").mockImplementation((async (input: RequestInfo | URL) => {
const url = String(input);
if (url.includes(throwUrlFragment)) throw new Error("network down");
if (url.includes("/login/oauth/access_token")) return Response.json({ access_token: "ghu_x" });
if (url.includes("api.github.com/user/memberships/orgs/")) return Response.json({ role: "admin", state: "active", organization: { id: 20 } });
if (url.endsWith("api.github.com/user")) return Response.json({ login: "alice", id: 7 });
return new Response("nf", { status: 404 });
}) as typeof githubClientModule.timeoutFetch);
afterEach(() => vi.restoreAllMocks());

it("a thrown code-exchange fetch degrades to the identity page (400), not an uncaught exception", async () => {
const e = brokeredEnv();
await seedInstall(e, { installation_id: 600, account_login: "acme", account_type: "Organization", account_id: 20, registered: 1 });
rejectAt("/login/oauth/access_token");
const res = await app.request("/v1/orb/oauth/callback?code=abc&installation_id=600", {}, e);
expect(res.status).toBe(400);
expect(await res.text()).toContain("Couldn't verify your GitHub identity");
});

it("a thrown /user read degrades to the identity page (400)", async () => {
const e = brokeredEnv();
await seedInstall(e, { installation_id: 601, account_login: "acme", account_type: "Organization", account_id: 20, registered: 1 });
rejectAt("api.github.com/user"); // exchange succeeds first, then the /user read throws
const res = await app.request("/v1/orb/oauth/callback?code=abc&installation_id=601", {}, e);
expect(res.status).toBe(400);
expect(await res.text()).toContain("Couldn't verify your GitHub identity");
expect(await db(e).prepare("SELECT 1 AS x FROM orb_enrollments WHERE installation_id=601").first()).toBeUndefined(); // never reached enrollment
});

it("a thrown org-membership check degrades to the identity page (400), leaving the install unregistered", async () => {
const e = brokeredEnv();
await seedInstall(e, { installation_id: 602, account_login: "acme", account_type: "Organization", account_id: 20, registered: 0 });
rejectAt("/user/memberships/orgs/"); // exchange + /user succeed, then the admin check throws
const res = await app.request("/v1/orb/oauth/callback?code=abc&installation_id=602", {}, e);
expect(res.status).toBe(400);
expect(await res.text()).toContain("Couldn't verify your GitHub identity");
const row = await db(e).prepare("SELECT registered FROM orb_github_installations WHERE installation_id=602").first<{ registered: number }>();
expect(row?.registered).toBe(0); // a throw at the gate never auto-registers
});
});