diff --git a/apps/hub/src/index.ts b/apps/hub/src/index.ts index 8a042e953..6cb39370b 100644 --- a/apps/hub/src/index.ts +++ b/apps/hub/src/index.ts @@ -253,7 +253,10 @@ import { createConnectionRoutes, createMcpOAuthRoutes, createMcpServerRoutes, + createOAuthConnectRoutes, + createTenantConnectCredential, createWorkflowConnectionRoutes, + DEFAULT_RETURN_PATH_ALLOWLIST, listMcpServerConnections, } from "@workbench/connections"; import { CONNECTOR_REGISTRY } from "@workbench/connections/registry"; @@ -1753,6 +1756,36 @@ export async function createHub(config: HubConfig) { listConnectedProviders(db, tenantId), }), ); + // Connections' own OAuth connect flow (CL-6389): `createOAuthConnectRoutes` + // (`@workbench/connections`) was exported but never mounted here — every + // provider whose descriptor sets `oauth` (OpenRouter, Hugging Face, and + // the GitHub App path) needs this to complete a one-click connect from + // the settings surface above. Follows #115's `mcp-servers/oauth` mount + // just below: state-param CSRF (real `state()` + exact-match callback + // validation) lives entirely inside the factory; this mount only wires + // the tenant already resolved by the platform's tenant middleware + // through to `createTenantConnectCredential`. + app.route( + `${TENANT_PREFIX}/connections/oauth`, + createOAuthConnectRoutes({ + hubUrl: config.baseUrl, + log: (line) => log.info`${line}`, + credentialCipher, + // Same env bag `GET .../connections/oauth-configured` reads above. + oauthEnv: { + huggingfaceClientId: config.huggingfaceOAuthClientId, + githubAppClientId: config.githubAppClientId, + githubAppClientSecret: config.githubAppClientSecret, + }, + connectCredential: createTenantConnectCredential({ + hubUrl: config.baseUrl, + log: (line) => log.info`${line}`, + providerHealth: providerHealthStore, + }), + defaultReturnPath: "/settings/connections", + returnPathAllowlist: [...DEFAULT_RETURN_PATH_ALLOWLIST, "/plugins"], + }), + ); // GitHub connect card (CL-6344): the code-review template's inline // room card reads its live state and starts reviews through here. // Connecting the PAT itself stays on `connections` above (`github` is @@ -2990,6 +3023,10 @@ export async function createHub(config: HubConfig) { onboardingDeps.seedModel = config.seedModel; if (config.huggingfaceOAuthClientId !== undefined) onboardingDeps.huggingfaceClientId = config.huggingfaceOAuthClientId; + if (config.githubAppClientId !== undefined) + onboardingDeps.githubAppClientId = config.githubAppClientId; + if (config.githubAppClientSecret !== undefined) + onboardingDeps.githubAppClientSecret = config.githubAppClientSecret; app.route("/api/onboarding", createOnboardingRoutes(onboardingDeps)); diff --git a/packages/connections/src/index.ts b/packages/connections/src/index.ts index 7f573b6b9..846f5ca83 100644 --- a/packages/connections/src/index.ts +++ b/packages/connections/src/index.ts @@ -75,6 +75,10 @@ export { type CreateOAuthConnectRoutesDeps, type OAuthStoreOutcome, } from "./oauth-routes"; +export { + createTenantConnectCredential, + type CreateTenantConnectCredentialDeps, +} from "./oauth-tenant-connect"; export { CONNECT_STATE_TTL_MS as OPENROUTER_CONNECT_STATE_TTL_MS, exchangeCodeForKey, diff --git a/packages/connections/src/oauth-routes.ts b/packages/connections/src/oauth-routes.ts index 973532a8d..eaf1cdbca 100644 --- a/packages/connections/src/oauth-routes.ts +++ b/packages/connections/src/oauth-routes.ts @@ -156,6 +156,13 @@ export type CreateOAuthConnectRoutesDeps = { * Required: without a caller-supplied store step, a successful * exchange would have nowhere to land. */ readonly connectCredential: (args: { + /** Given so a tenant-scoped caller (mounted inside the platform's + * tenant middleware, same as `afterConnected` below) can read + * `c.get("tenant")`/`c.get("principal")` directly instead of + * re-deriving them — see `createTenantConnectCredential` in + * `./oauth-tenant-connect.ts`. A caller with no tenant middleware + * (`packages/onboarding`'s own mount) is free to ignore it. */ + c: Context; connectorId: string; userId: string; userEmail: string; @@ -518,6 +525,7 @@ export function createOAuthConnectRoutes( const connectCredentialArgs: Parameters< typeof deps.connectCredential >[0] = { + c, connectorId, userId: user.id, userEmail: user.email, diff --git a/packages/connections/src/oauth-tenant-connect.test.ts b/packages/connections/src/oauth-tenant-connect.test.ts new file mode 100644 index 000000000..2d4539b3f --- /dev/null +++ b/packages/connections/src/oauth-tenant-connect.test.ts @@ -0,0 +1,234 @@ +// Proves `createTenantConnectCredential` end to end through the actual +// `createOAuthConnectRoutes` mount apps/hub uses (CL-6389): a full +// authorize -> callback -> credential-stored round trip against a fake +// provider (mirroring `oauth-routes.test.ts`'s `fakeDescriptor`), and a +// mismatched-state callback that must never reach persistence at all. +import { describe, expect, test } from "bun:test"; +import { Hono } from "hono"; +import type { MiddlewareHandler } from "hono"; +import type { TenantEnv } from "@intx/hub-api"; +import { createNoopCredentialCipher } from "@intx/crypto"; +import type { ConnectorDescriptor } from "./descriptor"; +import { createOAuthConnectRoutes } from "./oauth-routes"; +import { createTenantConnectCredential } from "./oauth-tenant-connect"; +import { createProviderHealthStore } from "./provider-health"; + +const TENANT = { + id: "tnt_1", + name: "Acme", + slug: "acme", + domain: "acme.example", + parentId: null, + config: null, + createdAt: new Date(), + updatedAt: new Date(), +}; + +const PRINCIPAL = { + id: "prn_alice", + tenantId: TENANT.id, + kind: "user" as const, + refId: "prn_alice", + status: "active" as const, + createdAt: new Date(), + updatedAt: new Date(), +}; + +const WIDGET_CONNECTOR: ConnectorDescriptor = { + id: "widget", + displayName: "Widget", + authKind: "oauth-pkce", + docsUrl: "https://widget.example.com", + credentialPlugin: "http", + feedsTools: [], + oauth: { + authorizeUrl: "https://widget.example.com/authorize", + usesPKCE: true, + echoesState: false, + deploysDefaultWorkflows: false, + buildAuthorizeUrl: ({ callbackUrl, codeChallenge }) => { + const url = new URL("https://widget.example.com/authorize"); + url.searchParams.set("redirect_uri", callbackUrl); + if (codeChallenge !== undefined) { + url.searchParams.set("code_challenge", codeChallenge); + } + return url; + }, + // Stands in for the fake provider's own token endpoint. + exchange: async ({ code }) => ({ ok: true, apiKey: `key-for-${code}` }), + }, +}; + +function mountTenantScoped( + overrides: Parameters[0] = { + hubUrl: "https://bench.example.com", + log: () => undefined, + }, +): { + app: Hono; + providers: { tenantId: string; name: string; plugin: string }[]; + credentials: { tenantId: string; providerId: string; secret: string }[]; +} { + const providers: { tenantId: string; name: string; plugin: string }[] = []; + const credentials: { + tenantId: string; + providerId: string; + secret: string; + }[] = []; + + const connectCredential = createTenantConnectCredential({ + ...overrides, + registry: { widget: WIDGET_CONNECTOR }, + ensureProviderFn: async (_api, _cookies, args) => { + providers.push({ + tenantId: args.tenantId, + name: args.name, + plugin: args.plugin, + }); + return `prv_${args.name}`; + }, + ensureCredentialFn: async (_api, _cookies, args) => { + credentials.push({ + tenantId: args.tenantId, + providerId: args.providerId, + secret: args.secret, + }); + return `cred_${args.providerId}`; + }, + }); + + const routes = createOAuthConnectRoutes({ + hubUrl: "https://bench.example.com", + log: () => undefined, + credentialCipher: createNoopCredentialCipher(), + registry: { widget: WIDGET_CONNECTOR }, + connectCredential, + }); + + const asTenant: MiddlewareHandler = async (c, next) => { + c.set("user", { + id: "user_1", + email: "user_1@example.com", + } as never); + c.set("tenant", TENANT); + c.set("principal", PRINCIPAL); + await next(); + }; + const app = new Hono(); + app.use("*", asTenant); + app.route("/api/tenants/tnt_1/connections/oauth", routes); + return { app, providers, credentials }; +} + +function cookieHeaderFrom(response: Response): string { + return response.headers + .getSetCookie() + .map((sc) => sc.split(";")[0]) + .join("; "); +} + +describe("createTenantConnectCredential, mounted through createOAuthConnectRoutes", () => { + test("full authorize -> callback -> credential-stored round trip", async () => { + const { app, providers, credentials } = mountTenantScoped(); + + const started = await app.request( + "/api/tenants/tnt_1/connections/oauth/widget/start", + ); + expect(started.status).toBe(302); + const authorizeUrl = new URL(started.headers.get("location") ?? ""); + expect(authorizeUrl.origin).toBe("https://widget.example.com"); + const cookie = cookieHeaderFrom(started); + + const callback = await app.request( + "/api/tenants/tnt_1/connections/oauth/widget/callback?code=abc123", + { headers: { cookie } }, + ); + expect(callback.status).toBe(302); + const redirect = new URL( + callback.headers.get("location") ?? "", + "https://x", + ); + expect(redirect.searchParams.get("outcome")).toBe("connected"); + expect(redirect.searchParams.get("tenantSlug")).toBe(TENANT.slug); + + expect(providers).toEqual([ + { tenantId: TENANT.id, name: "widget", plugin: "http" }, + ]); + expect(credentials).toEqual([ + { + tenantId: TENANT.id, + providerId: "prv_widget", + secret: "key-for-abc123", + }, + ]); + }); + + test("a callback with no matching state never persists a credential", async () => { + const { app, providers, credentials } = mountTenantScoped(); + + const callback = await app.request( + "/api/tenants/tnt_1/connections/oauth/widget/callback?code=abc123&state=forged", + ); + expect(callback.status).toBe(302); + const redirect = new URL( + callback.headers.get("location") ?? "", + "https://x", + ); + expect(redirect.searchParams.get("outcome")).toBe("error"); + expect(redirect.searchParams.get("code")).toBe("state_expired"); + expect(providers).toEqual([]); + expect(credentials).toEqual([]); + }); + + test("a tampered state cookie is rejected before anything is persisted", async () => { + const { app, providers, credentials } = mountTenantScoped(); + + const started = await app.request( + "/api/tenants/tnt_1/connections/oauth/widget/start", + ); + // Same cookie *name*, garbage value: fails the sealed-state cipher + // check the same way a forged or replayed cookie would, matching + // the factory's own cross-user regression coverage in + // oauth-routes.test.ts. + const cookie = cookieHeaderFrom(started).replace( + /workbench_widget_connect=[^;]+/, + "workbench_widget_connect=not-a-real-sealed-state", + ); + + const callback = await app.request( + "/api/tenants/tnt_1/connections/oauth/widget/callback?code=abc123", + { headers: { cookie } }, + ); + expect(callback.status).toBe(302); + const redirect = new URL( + callback.headers.get("location") ?? "", + "https://x", + ); + expect(redirect.searchParams.get("code")).toBe("state_expired"); + expect(providers).toEqual([]); + expect(credentials).toEqual([]); + }); + + test("provider-health clears on a successful connect", async () => { + const providerHealth = createProviderHealthStore(); + providerHealth.report(TENANT.id, "widget", "credential_failure"); + expect(providerHealth.listForTenant(TENANT.id)["widget"]).toBeDefined(); + + const { app } = mountTenantScoped({ + hubUrl: "https://bench.example.com", + log: () => undefined, + providerHealth, + }); + + const started = await app.request( + "/api/tenants/tnt_1/connections/oauth/widget/start", + ); + const cookie = cookieHeaderFrom(started); + await app.request( + "/api/tenants/tnt_1/connections/oauth/widget/callback?code=abc123", + { headers: { cookie } }, + ); + + expect(providerHealth.listForTenant(TENANT.id)["widget"]).toBeUndefined(); + }); +}); diff --git a/packages/connections/src/oauth-tenant-connect.ts b/packages/connections/src/oauth-tenant-connect.ts new file mode 100644 index 000000000..c27b5e1b5 --- /dev/null +++ b/packages/connections/src/oauth-tenant-connect.ts @@ -0,0 +1,150 @@ +// The `connectCredential` wiring `apps/hub` needs to mount +// `createOAuthConnectRoutes` (`./oauth-routes.ts`) directly beside its +// tenant-scoped `createConnectionRoutes` (`./routes.ts`), rather than +// only reachable through `packages/onboarding`'s own first-login mount. +// Where onboarding resolves "the user's personal tenant" from scratch +// (no tenant exists yet at first login), this caller already runs +// inside the platform's tenant middleware — the same one +// `createConnectionRoutes` and `createMcpOAuthRoutes` (#115) run +// inside — so `c.get("tenant")`/`c.get("principal")` are already +// resolved and this never re-derives a tenant of its own. +// +// Persists exactly the way `routes.ts`'s `POST /:connectorId/complete` +// does: `ensureProvider` + `ensureCredential`, then `seedCatalog` for an +// inference connector so a just-connected provider's models are +// launchable immediately, not just stored. The credential is already +// proven by the OAuth exchange itself, so there is no separate probe +// step here (unlike `/complete`'s pasted-key path, which has nothing +// else vouching for the secret). +import type { Context } from "hono"; +import type { TenantEnv } from "@intx/hub-api"; +import { + createHubAPI, + ensureCredential, + ensureProvider, + PROVIDER_TEST_CONFIG, + seedCatalog, + type ApiCall, + type EnsureCredentialArgs, + type EnsureProviderArgs, + type SeedCatalogArgs, + type SupportedCredentialProvider, +} from "@workbench/hub-client"; +import type { ConnectorDescriptor } from "./descriptor"; +import type { ProviderHealthStore } from "./provider-health"; +import { CONNECTOR_REGISTRY } from "./registry"; +import type { CreateOAuthConnectRoutesDeps } from "./oauth-routes"; + +export type CreateTenantConnectCredentialDeps = { + readonly hubUrl: string; + readonly log: (line: string) => void; + /** Test-only override, matching every other route factory here. */ + readonly registry?: Readonly>; + /** Cleared on a successful connect, same store `createConnectionRoutes`' + * `/complete` and `GET /provider-health` share (CL-6092). */ + readonly providerHealth?: ProviderHealthStore; + /** Test-only override, matching `routes.ts`'s own seam — lets this + * module's own test prove the persist/seed sequencing without + * reaching for module mocking or a real hub HTTP server. */ + readonly ensureProviderFn?: ( + api: ApiCall, + cookies: string[], + args: EnsureProviderArgs, + log: (line: string) => void, + ) => ReturnType; + readonly ensureCredentialFn?: ( + api: ApiCall, + cookies: string[], + args: EnsureCredentialArgs, + log: (line: string) => void, + ) => ReturnType; + readonly seedCatalogFn?: ( + args: SeedCatalogArgs, + ) => ReturnType; +}; + +function isInferenceProvider(id: string): id is SupportedCredentialProvider { + return Object.hasOwn(PROVIDER_TEST_CONFIG, id); +} + +/** + * Builds the `connectCredential` dep `createOAuthConnectRoutes` needs, + * scoped to whatever tenant the request's own middleware already + * resolved. `args.c` is cast to `Context` — safe only + * because this is wired exclusively into a mount reached through the + * platform's tenant middleware (see this module's own header); a caller + * mounting outside that middleware must not use this. + */ +export function createTenantConnectCredential( + deps: CreateTenantConnectCredentialDeps, +): CreateOAuthConnectRoutesDeps["connectCredential"] { + const api = createHubAPI(deps.hubUrl); + const registry = deps.registry ?? CONNECTOR_REGISTRY; + const runEnsureProvider = deps.ensureProviderFn ?? ensureProvider; + const runEnsureCredential = deps.ensureCredentialFn ?? ensureCredential; + const runSeedCatalog = deps.seedCatalogFn ?? seedCatalog; + + return async (args) => { + const c = args.c as Context; + const tenant = c.get("tenant"); + const principal = c.get("principal"); + const descriptor = registry[args.connectorId]; + if (descriptor === undefined) { + return { + kind: "invalid-credential", + message: `Unknown connector: ${args.connectorId}`, + }; + } + + try { + const providerId = await runEnsureProvider( + api, + args.cookies, + { + tenantId: tenant.id, + name: descriptor.id, + plugin: descriptor.credentialPlugin, + }, + deps.log, + ); + await runEnsureCredential( + api, + args.cookies, + { + tenantId: tenant.id, + providerId, + name: descriptor.displayName, + secret: args.apiKey, + type: "api_key", + verified: true, + }, + deps.log, + ); + if (isInferenceProvider(descriptor.id)) { + await runSeedCatalog({ + api, + cookies: args.cookies, + tenantId: tenant.id, + log: deps.log, + provider: descriptor.id, + apiKey: args.apiKey, + credentialVerified: true, + }); + } + deps.providerHealth?.clear(tenant.id, descriptor.id); + return { + kind: "connected", + tenantId: tenant.id, + tenantSlug: tenant.slug, + principalId: principal.id, + tenantDomain: tenant.domain, + }; + } catch (cause) { + const message = cause instanceof Error ? cause.message : String(cause); + deps.log( + `oauth connect for ${args.connectorId} on tenant ${tenant.id} failed to persist: ${message}`, + ); + return { kind: "invalid-credential", message }; + } + }; +} diff --git a/packages/onboarding/src/routes.ts b/packages/onboarding/src/routes.ts index 957a168d5..ebb4c8bb6 100644 --- a/packages/onboarding/src/routes.ts +++ b/packages/onboarding/src/routes.ts @@ -134,6 +134,14 @@ export type CreateOnboardingRoutesDeps = { exchange?: typeof exchangeHuggingFaceCodeForToken; connectCredential?: typeof testAndPersistCredential; }; + /** The GitHub OAuth App id/secret from github.com/settings/apps (see + * .env.example). Absent leaves the one-click GitHub connect path + * reporting `not_configured` — the PAT paste form stays available + * either way (CL-6386). Read here so this package's own `/oauth` + * mount decides the same thing `@workbench/connections`' tenant- + * scoped `GET .../oauth-configured` route already reports. */ + githubAppClientId?: string; + githubAppClientSecret?: string; /** Test seam for `POST /complete-setup`'s slow-path deploy step. */ ensureSeededFn?: typeof ensureSeeded; /** Test seam for `POST /complete`'s own success path — defaults to the @@ -655,7 +663,11 @@ export function createOnboardingRoutes( log: deps.log, credentialCipher, registry: oauthRegistry, - oauthEnv: { huggingfaceClientId: deps.huggingfaceClientId }, + oauthEnv: { + huggingfaceClientId: deps.huggingfaceClientId, + githubAppClientId: deps.githubAppClientId, + githubAppClientSecret: deps.githubAppClientSecret, + }, connectCredential, recentlyConnected, afterConnected,