diff --git a/.env.example b/.env.example index c76aa8e..4bebf30 100644 --- a/.env.example +++ b/.env.example @@ -53,9 +53,15 @@ NUXT_SESSION_PASSWORD=<32+ char random string> # - The webhook secret you set during creation. # - A generated private key (.pem). On Vercel, store with literal "\n" in # place of newlines; locally, keep the real newlines. +# - The Client ID and a generated client secret ("Client secrets" section). +# These drive the user-to-server OAuth that proves a connecting user +# actually administers the installation they're binding a tangled handle +# to. Distinct from the private key above. Required for the /connect flow. # --------------------------------------------------------------------------- NUXT_GITHUB_APP_ID= NUXT_GITHUB_WEBHOOK_SECRET= +NUXT_GITHUB_APP_CLIENT_ID= +NUXT_GITHUB_APP_CLIENT_SECRET= NUXT_GITHUB_APP_PRIVATE_KEY="-----BEGIN RSA PRIVATE KEY----- ... -----END RSA PRIVATE KEY----- diff --git a/README.md b/README.md index 2a52ebe..c22cb2c 100644 --- a/README.md +++ b/README.md @@ -45,7 +45,12 @@ The rest (`NUXT_DATABASE_URL`, the `NUXT_GITHUB_APP_*` values) come from your Neon dashboard and a [new GitHub App](https://github.com/settings/apps/new). The App needs `contents:read` and `metadata:read` permissions plus the `push`, `create`, `delete`, and `repository` events, with its webhook pointed at your -Smee URL. +Smee URL. Set the **Setup URL** to `/connect` (tick *Redirect on +update*) and the **Callback URL** to `/api/github/oauth/callback`; the +latter drives the user-OAuth that verifies a connecting user administers the +installation before any handle is bound. Copy the App's **Client ID** and a +generated **client secret** into `NUXT_GITHUB_APP_CLIENT_ID` / +`NUXT_GITHUB_APP_CLIENT_SECRET`. In separate terminals, proxy webhooks and drain the job queue: @@ -65,10 +70,13 @@ synchub.to runs on Vercel with a Neon Postgres database. 2. Import the repo into Vercel (the Nuxt preset is auto-detected) and set every variable from `.env.example` under **Settings > Environment Variables**. Mark the secrets (`NUXT_DATABASE_URL`, `NUXT_GITHUB_APP_PRIVATE_KEY`, - `NUXT_ATPROTO_PRIVATE_JWK`, `NUXT_ENCRYPTION_KEY`, `NUXT_SESSION_PASSWORD`, + `NUXT_GITHUB_APP_CLIENT_SECRET`, `NUXT_ATPROTO_PRIVATE_JWK`, + `NUXT_ENCRYPTION_KEY`, `NUXT_SESSION_PASSWORD`, `NUXT_GITHUB_WEBHOOK_SECRET`, `NUXT_CRON_SECRET`) as **Sensitive**. -3. Set `NUXT_PUBLIC_URL` to your real origin and point the GitHub App webhook at - `https:///api/github/webhook`. +3. Set `NUXT_PUBLIC_URL` to your real origin, point the GitHub App webhook at + `https:///api/github/webhook`, and set the App's Setup + + Callback URLs to `https:///connect` and + `https:///api/github/oauth/callback`. 4. Deploy. The worker runs on a Vercel Cron (declared in `nuxt.config.ts`, so no diff --git a/app/pages/connect.vue b/app/pages/connect.vue new file mode 100644 index 0000000..b361b0e --- /dev/null +++ b/app/pages/connect.vue @@ -0,0 +1,222 @@ + + + + + diff --git a/nuxt.config.ts b/nuxt.config.ts index 9d3f005..ad0ce07 100644 --- a/nuxt.config.ts +++ b/nuxt.config.ts @@ -23,6 +23,8 @@ export default defineNuxtConfig({ databaseUrl: '', githubAppId: '', githubAppPrivateKey: '', + githubAppClientId: '', + githubAppClientSecret: '', githubWebhookSecret: '', cronSecret: '', workerBudgetMs: '', @@ -58,6 +60,7 @@ export default defineNuxtConfig({ routeRules: { '/': { noScripts: true, prerender: true }, '/dashboard': { ssr: false, prerender: true }, + '/connect': { ssr: false }, }, nitro: { vercel: { diff --git a/server/api/atproto/callback.get.ts b/server/api/atproto/callback.get.ts index 11b24a8..73ecab0 100644 --- a/server/api/atproto/callback.get.ts +++ b/server/api/atproto/callback.get.ts @@ -1,6 +1,7 @@ import { and, eq, ne } from 'drizzle-orm' import { userIdentity } from '#server/db/schema' import { enqueue } from '#server/utils/queue' +import { clearInstallOwnership, hasVerifiedInstall } from '#server/utils/install-ownership' import { resolveHandle } from '#server/utils/resolve-handle' import { addAccount } from '#server/utils/server-session' import { generateAndPublishKey, revokeKeyForInstallationDid } from '#server/utils/tangled-pubkey' @@ -39,6 +40,18 @@ export default defineEventHandler(async event => { } installationId = parsed + // Bind-guard: the connecting user must have proven (via GitHub user-OAuth + // in the /connect flow) that they administer this installation. Without + // this check, anyone who completes a tangled OAuth carrying a victim's + // installation id in `state` could hijack the mirror. The proof is a + // sealed cookie set by /api/github/oauth/callback. + if (!(await hasVerifiedInstall(event, installationId))) { + throw createError({ + statusCode: 403, + statusMessage: 'installation ownership not verified; start from the connect page', + }) + } + // One installation maps to exactly one DID. If another DID is currently // bound to this installation, this connect displaces it: revoke that DID's // now-dead SSH key (PDS record + local row) and null its installationId so @@ -87,6 +100,9 @@ export default defineEventHandler(async event => { // and fans out per-repo enrolment. Doing this in the worker (rather than // inline here) keeps the OAuth callback fast regardless of repo count. await enqueue('tangled.backfill-installation', { installationId, page: 1 }) + + // Consume the ownership proof so it can't be replayed for another bind. + await clearInstallOwnership(event) } else { // Returning sign-in. Look up the installation we previously bound. diff --git a/server/api/connect/info.get.ts b/server/api/connect/info.get.ts new file mode 100644 index 0000000..0d54257 --- /dev/null +++ b/server/api/connect/info.get.ts @@ -0,0 +1,21 @@ +import { installationAccountLogin } from '#server/utils/github-app' + +export interface ConnectInfo { + installationId: number + login: string | null +} + +/** + * Public lookup of an installation's account login for the connect page. + * Returns only the login, which the App can already read; it is not a secret + * and reveals nothing the install screen didn't. The actual ownership gate is + * the GitHub user-OAuth step, not this endpoint. + */ +export default defineEventHandler(async (event): Promise => { + const raw = getQuery(event).installationId + if (typeof raw !== 'string' || !/^\d+$/.test(raw)) { + throw createError({ statusCode: 400, statusMessage: 'installationId is required and must be numeric' }) + } + const installationId = Number(raw) + return { installationId, login: await installationAccountLogin(installationId) } +}) diff --git a/server/api/github/oauth/callback.get.ts b/server/api/github/oauth/callback.get.ts new file mode 100644 index 0000000..55d11de --- /dev/null +++ b/server/api/github/oauth/callback.get.ts @@ -0,0 +1,55 @@ +import { installationAccountLogin, userAdministersInstallation, userOctokitFromCode } from '#server/utils/github-app' +import { markInstallOwned } from '#server/utils/install-ownership' +import { sessionConfig } from '#server/utils/server-session' + +interface OAuthFlowData { + state: string + installationId: number +} + +/** + * GitHub user-OAuth callback. Verifies the `state` against the sealed flow + * cookie, exchanges the code for a user token, and confirms the user + * administers the installation. On success, marks the install owned (a sealed + * cookie the atproto callback checks before binding) and sends the user into + * the tangled connect flow. + */ +export default defineEventHandler(async event => { + const query = getQuery(event) + const code = typeof query.code === 'string' ? query.code : null + const state = typeof query.state === 'string' ? query.state : null + if (!code || !state) { + throw createError({ statusCode: 400, statusMessage: 'missing code or state' }) + } + + const flow = await useSession(event, { + ...sessionConfig(), + name: 'synchub-gh-oauth', + maxAge: 10 * 60, + }) + const expectedState = flow.data.state + const installationId = flow.data.installationId + await flow.clear() + + if (!expectedState || expectedState !== state || typeof installationId !== 'number') { + throw createError({ statusCode: 400, statusMessage: 'invalid or expired oauth state' }) + } + + const userOctokit = await userOctokitFromCode(code) + const administers = await userAdministersInstallation(userOctokit, installationId) + if (!administers) { + throw createError({ + statusCode: 403, + statusMessage: 'your GitHub account does not administer this installation', + }) + } + + await markInstallOwned(event, installationId) + + // Ownership proven. Hand off to the connect page, which now lets the user + // pick the tangled handle to bind. Carry the account login for display. + const login = await installationAccountLogin(installationId) + const params = new URLSearchParams({ installation_id: String(installationId), verified: '1' }) + if (login) params.set('login', login) + await sendRedirect(event, `/connect?${params.toString()}`, 302) +}) diff --git a/server/api/github/oauth/start.get.ts b/server/api/github/oauth/start.get.ts new file mode 100644 index 0000000..1d8dc5c --- /dev/null +++ b/server/api/github/oauth/start.get.ts @@ -0,0 +1,34 @@ +import { randomBytes } from 'node:crypto' +import { githubOAuthUrl } from '#server/utils/github-app' +import { sessionConfig } from '#server/utils/server-session' + +interface OAuthFlowData { + state: string + installationId: number +} + +/** + * Begin GitHub user-to-server OAuth to prove the connecting user administers + * the installation they're about to bind a tangled handle to. The random + * `state` is sealed into a short-lived cookie and verified in the callback. + */ +export default defineEventHandler(async event => { + const query = getQuery(event) + const raw = query.installationId + if (typeof raw !== 'string' || !/^\d+$/.test(raw)) { + throw createError({ statusCode: 400, statusMessage: 'installationId is required and must be numeric' }) + } + const installationId = Number(raw) + + const state = randomBytes(16).toString('base64url') + const flow = await useSession(event, { + ...sessionConfig(), + name: 'synchub-gh-oauth', + maxAge: 10 * 60, + }) + await flow.update({ state, installationId }) + + const { url } = useRuntimeConfig().public + const redirectUri = `${url.replace(/\/$/, '')}/api/github/oauth/callback` + await sendRedirect(event, githubOAuthUrl({ state, redirectUri }), 302) +}) diff --git a/server/utils/github-app.ts b/server/utils/github-app.ts index 28acd21..f49c198 100644 --- a/server/utils/github-app.ts +++ b/server/utils/github-app.ts @@ -9,8 +9,14 @@ function useApp(): App { if (!appId || !privateKey) { throw new Error('NUXT_GITHUB_APP_ID and NUXT_GITHUB_APP_PRIVATE_KEY must be set') } + // The OAuth client id/secret are optional at construction time so the + // webhook + sync paths (server-to-server only) keep working without them; + // requireOAuthApp() throws explicitly when the /connect flow needs them. + const clientId = process.env.NUXT_GITHUB_APP_CLIENT_ID + const clientSecret = process.env.NUXT_GITHUB_APP_CLIENT_SECRET cachedApp = new App({ appId, + ...(clientId && clientSecret ? { oauth: { clientId, clientSecret } } : {}), // Vercel env vars escape newlines; restore them so PEM parsing works. privateKey: privateKey.replaceAll('\\n', '\n'), }) @@ -18,6 +24,7 @@ function useApp(): App { } export type InstallationOctokit = Awaited> +export type UserOctokit = Awaited> /** Get an Octokit pre-authed for a specific GitHub App installation. */ export async function installationOctokit(installationId: number): Promise { @@ -25,6 +32,71 @@ export async function installationOctokit(installationId: number): Promise { + const app = requireOAuthApp() + return app.oauth.getUserOctokit({ code }) +} + +/** + * True if the authenticated user administers `installationId`. The + * user-to-server `GET /user/installations` endpoint only returns installations + * the user can administer, so membership in the list is the ownership proof. + */ +export async function userAdministersInstallation( + userOctokit: UserOctokit, + installationId: number, +): Promise { + // A user with >100 app installations is implausible here, so one page is + // enough; `/user/installations` only lists installs the user administers. + const { data } = await userOctokit.request('GET /user/installations', { per_page: 100 }) + return data.installations.some(install => install.id === installationId) +} + +/** Resolve an installation's account login (for display on the connect page). */ +export async function installationAccountLogin(installationId: number): Promise { + const app = useApp() + try { + const { data } = await app.octokit.request('GET /app/installations/{installation_id}', { + installation_id: installationId, + }) + const account = data.account + if (account && 'login' in account) return account.login + return null + } + catch { + return null + } +} + /** Test hook. */ export function clearGitHubAppCache() { cachedApp = undefined diff --git a/server/utils/install-ownership.ts b/server/utils/install-ownership.ts new file mode 100644 index 0000000..2f7f4bf --- /dev/null +++ b/server/utils/install-ownership.ts @@ -0,0 +1,48 @@ +import type { H3Event } from 'h3' +import { sessionConfig } from './server-session' + +/** + * Short-lived sealed cookie proving the current browser completed GitHub + * user-OAuth and was confirmed as an administrator of a specific installation. + * + * The atproto callback checks this before binding a DID to an installation, so + * a connecting user can't claim an installation they don't actually control by + * crafting `/connect?installation_id=`. Reuses the session password + * for sealing; a distinct cookie name and a 15-minute TTL keep it scoped to a + * single connect flow. + */ +interface InstallOwnershipData { + installationId: number + verifiedAt: number +} + +const COOKIE_NAME = 'synchub-install-ownership' +const TTL_SECONDS = 15 * 60 + +function ownershipConfig() { + return { ...sessionConfig(), name: COOKIE_NAME, maxAge: TTL_SECONDS } +} + +export async function markInstallOwned(event: H3Event, installationId: number): Promise { + const session = await useSession(event, ownershipConfig()) + await session.update({ installationId, verifiedAt: Date.now() }) +} + +/** + * Return true if the current browser proved ownership of `installationId` + * within the TTL. Does not clear the cookie; the caller clears it after a + * successful bind so it can't be replayed. + */ +export async function hasVerifiedInstall(event: H3Event, installationId: number): Promise { + const session = await useSession(event, ownershipConfig()) + const { installationId: owned, verifiedAt } = session.data + if (typeof owned !== 'number' || typeof verifiedAt !== 'number') return false + if (owned !== installationId) return false + if (Date.now() - verifiedAt > TTL_SECONDS * 1000) return false + return true +} + +export async function clearInstallOwnership(event: H3Event): Promise { + const session = await useSession(event, ownershipConfig()) + await session.clear() +} diff --git a/test/unit/github-ownership.spec.ts b/test/unit/github-ownership.spec.ts new file mode 100644 index 0000000..2cf6d07 --- /dev/null +++ b/test/unit/github-ownership.spec.ts @@ -0,0 +1,26 @@ +import { describe, expect, it, vi } from 'vitest' +import { userAdministersInstallation } from '../../server/utils/github-app' +import type { UserOctokit } from '../../server/utils/github-app' + +function octokitReturning(ids: number[]): UserOctokit { + const request = vi.fn(async () => ({ data: { installations: ids.map(id => ({ id })) } })) + // eslint-disable-next-line ts/no-unsafe-type-assertion -- only `request` is exercised + return { request } as unknown as UserOctokit +} + +describe('userAdministersInstallation', () => { + it('returns true when the installation is in the user list', async () => { + const octokit = octokitReturning([10, 137556633, 42]) + expect(await userAdministersInstallation(octokit, 137556633)).toBe(true) + }) + + it('returns false when the installation is absent', async () => { + const octokit = octokitReturning([10, 42]) + expect(await userAdministersInstallation(octokit, 137556633)).toBe(false) + }) + + it('returns false for an empty installation list', async () => { + const octokit = octokitReturning([]) + expect(await userAdministersInstallation(octokit, 1)).toBe(false) + }) +}) diff --git a/test/unit/tangled-pubkey.spec.ts b/test/unit/tangled-pubkey.spec.ts index b0fced8..528a289 100644 --- a/test/unit/tangled-pubkey.spec.ts +++ b/test/unit/tangled-pubkey.spec.ts @@ -347,7 +347,7 @@ describe('revokeKeyForInstallationDid', () => { await revokeKeyForInstallationDid(1, 'did:plc:1') expect(restoreMock).toHaveBeenCalledWith('did:plc:1') - const del = deleteRecordMock.mock.calls[0]![0] + const del = deleteRecordMock.mock.calls[0][0] expect(del.repo).toBe('did:plc:1') expect(del.collection).toBe('sh.tangled.publicKey') expect(del.rkey).toBe('rkey-1') @@ -363,7 +363,7 @@ describe('revokeKeyForInstallationDid', () => { const db = useDb() const rows = await db.select().from(sshKey) expect(rows).toHaveLength(1) - expect(rows[0]!.did).toBe('did:plc:2') + expect(rows[0].did).toBe('did:plc:2') }) it('no-ops when no key exists for the pair', async () => { diff --git a/test/utils/git-wire.ts b/test/utils/git-wire.ts index eeb38d5..b80089d 100644 --- a/test/utils/git-wire.ts +++ b/test/utils/git-wire.ts @@ -71,7 +71,7 @@ export function fakeGithubFetch(repos: Map) { const url = typeof input === 'string' ? input : input.toString() const match = url.match(/github\.com\/(.+?)\.git\/(info\/refs|git-upload-pack)/) if (!match) throw new Error(`fakeGithubFetch: unexpected url ${url}`) - const repoPath = repos.get(match[1]!) + const repoPath = repos.get(match[1]) if (!repoPath) return new Response(null, { status: 404, statusText: 'Not Found' }) if (match[2] === 'info/refs') { diff --git a/vite.config.ts b/vite.config.ts index 4ec8f11..ad957ea 100644 --- a/vite.config.ts +++ b/vite.config.ts @@ -5,6 +5,11 @@ import { playwright } from 'vite-plus/test/browser-playwright' const rootDir = import.meta.dirname export default defineConfig({ + resolve: { + alias: { + '#server': `${rootDir}/server`, + }, + }, lint: { plugins: ['typescript', 'vue', 'oxc', 'unicorn', 'vitest'], jsPlugins: ['@stylistic/eslint-plugin'],