diff --git a/cloud/apps/sandbox-control/migrations/0028_mcp_grants.sql b/cloud/apps/sandbox-control/migrations/0028_mcp_grants.sql new file mode 100644 index 00000000000..c2a5db2f49e --- /dev/null +++ b/cloud/apps/sandbox-control/migrations/0028_mcp_grants.sql @@ -0,0 +1,23 @@ +-- MCP OAuth grants (src/mcp/oauth.ts): tie grants to the credential that approved them, bound their +-- lifetime, remember revoked families and let housekeeping purge abandoned registrations. Additive. + +-- The orca_sk_ key that approved the grant (NULL: a desktop approval code). Revoking that key +-- revokes every token of the grant. +ALTER TABLE oauth_codes ADD COLUMN grantor_key_id TEXT; +-- Set when a used code is presented again (replay), so a racing exchange revokes what it minted. +ALTER TABLE oauth_codes ADD COLUMN replayed_at INTEGER; +ALTER TABLE api_keys ADD COLUMN grantor_key_id TEXT; +-- Absolute end of a grant (90 days from the code exchange): refresh is refused after it. +ALTER TABLE api_keys ADD COLUMN family_expires_at INTEGER; +CREATE INDEX api_keys_grantor ON api_keys (grantor_key_id) WHERE grantor_key_id IS NOT NULL; + +-- Revoked grant families. Checked after minting, so a token minted concurrently with a revocation +-- or replay never survives it. +CREATE TABLE oauth_revoked_families ( + family_id TEXT PRIMARY KEY, + reason TEXT NOT NULL, + revoked_at INTEGER NOT NULL +); + +-- Last successful code exchange; registrations that never complete one are purged after a day. +ALTER TABLE oauth_clients ADD COLUMN last_granted_at INTEGER; diff --git a/cloud/apps/sandbox-control/src/api/keys.ts b/cloud/apps/sandbox-control/src/api/keys.ts index fa16402c3f1..27cb848feb4 100644 --- a/cloud/apps/sandbox-control/src/api/keys.ts +++ b/cloud/apps/sandbox-control/src/api/keys.ts @@ -43,6 +43,9 @@ export type ApiKeyRow = { client_id?: string | null audience?: string | null family_id?: string | null + /** Migration 0028: the orca_sk_ key that approved the grant, and the grant's absolute end. */ + grantor_key_id?: string | null + family_expires_at?: number | null } const TAG: Record = { user: 'sk', workspace: 'ws', oauth: 'oat', oauth_refresh: 'ort' } @@ -95,7 +98,7 @@ export async function mintApiKey( kind?: ApiKeyKind sandboxId?: string | null expiresAt?: number | null - oauth?: { clientId: string; audience: string; familyId: string } + oauth?: { clientId: string; audience: string; familyId: string; grantorKeyId: string | null; familyExpiresAt: number } }, now: number ): Promise { @@ -120,17 +123,19 @@ export async function mintApiKey( revoked_by: null, client_id: input.oauth?.clientId ?? null, audience: input.oauth?.audience ?? null, - family_id: input.oauth?.familyId ?? null + family_id: input.oauth?.familyId ?? null, + grantor_key_id: input.oauth?.grantorKeyId ?? null, + family_expires_at: input.oauth?.familyExpiresAt ?? null } await db .prepare( `INSERT INTO api_keys (id, account_id, created_by, creator_role, name, prefix, secret_hash, scopes, kind, - sandbox_id, expires_at, created_at${row.client_id ? ', client_id, audience, family_id' : ''}) - VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?${row.client_id ? ', ?, ?, ?' : ''})` + sandbox_id, expires_at, created_at${row.client_id ? ', client_id, audience, family_id, grantor_key_id, family_expires_at' : ''}) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?${row.client_id ? ', ?, ?, ?, ?, ?' : ''})` ) .bind(row.id, row.account_id, row.created_by, row.creator_role, row.name, row.prefix, row.secret_hash, row.scopes, row.kind, row.sandbox_id, row.expires_at, row.created_at, - ...(row.client_id ? [row.client_id, row.audience, row.family_id] : [])) + ...(row.client_id ? [row.client_id, row.audience, row.family_id, row.grantor_key_id, row.family_expires_at] : [])) .run() return { row, secret } } @@ -173,11 +178,16 @@ export function getApiKey(db: D1Database, id: string): Promise } export async function revokeApiKey(db: D1Database, id: string, by: string, now: number): Promise { - const result = await db - .prepare('UPDATE api_keys SET revoked_at = ?, revoked_by = ? WHERE id = ? AND revoked_at IS NULL') - .bind(now, by, id) - .run() - return result.meta.changes === 1 + const [result] = await db.batch([ + db.prepare('UPDATE api_keys SET revoked_at = ?, revoked_by = ? WHERE id = ? AND revoked_at IS NULL').bind(now, by, id), + // MCP grants approved with this key die with it (their families stay revoked, see oauth.ts). + db + .prepare(`INSERT OR IGNORE INTO oauth_revoked_families (family_id, reason, revoked_at) + SELECT DISTINCT family_id, 'grantor_revoked', ? FROM api_keys WHERE grantor_key_id = ? AND family_id IS NOT NULL`) + .bind(now, id), + db.prepare(`UPDATE api_keys SET revoked_at = ?, revoked_by = 'system:grantor_revoked' WHERE grantor_key_id = ? AND revoked_at IS NULL`).bind(now, id) + ]) + return result!.meta.changes === 1 } /** Workspace tokens die with their session: revoked on every new boot, suspend and destroy. */ diff --git a/cloud/apps/sandbox-control/src/api/openapi.ts b/cloud/apps/sandbox-control/src/api/openapi.ts index 3d2eae68362..ebe90a3a5ba 100644 --- a/cloud/apps/sandbox-control/src/api/openapi.ts +++ b/cloud/apps/sandbox-control/src/api/openapi.ts @@ -196,6 +196,38 @@ export const OPERATIONS: Record> = { responses: { 201: { description: 'Created', schema: obj({ code: { type: 'string' }, expiresAt: { type: 'integer' }, scopes: { type: 'array', items: { type: 'string' } } }) } } } }, + '/v1/oauth-grants': { + get: { + tags: ['API keys'], + sessionOnly: true, + summary: 'List connected MCP apps', + description: 'Live OAuth grants: members see their own, owners/admins the whole account. A grant ends 90 days after approval, when revoked here, or when the API key that approved it is revoked.', + responses: { + 200: { + description: 'Grants', + schema: obj({ + grants: { + type: 'array', + items: obj({ + id: { type: 'string' }, + clientId: { type: 'string' }, + clientName: { type: ['string', 'null'] }, + userId: { type: 'string' }, + scopes: { type: 'array', items: { type: 'string' } }, + approvedWith: { type: 'object' }, + grantedAt: { type: 'integer' }, + lastUsedAt: { type: ['integer', 'null'] }, + expiresAt: { type: ['integer', 'null'] } + }) + } + }) + } + } + } + }, + '/v1/oauth-grants/{id}': { + delete: { tags: ['API keys'], sessionOnly: true, summary: 'Disconnect an MCP app (revoke every token of the grant)', params: [idParam('id', 'Grant id (fam_…)')], responses: { 200: 'Revoked' } } + }, '/v1/settings': { get: { tags: ['Account'], summary: 'Programmatic-access settings', responses: { 200: { description: 'Settings', schema: obj({ settings: obj({ vmCliEnabled: { type: 'boolean' } }) }) } } }, put: { tags: ['Account'], sessionOnly: true, summary: 'Update settings (owner/admin)', body: obj({ vmCliEnabled: { type: 'boolean' } }, ['vmCliEnabled']), responses: { 200: 'Updated' } } diff --git a/cloud/apps/sandbox-control/src/api/router.ts b/cloud/apps/sandbox-control/src/api/router.ts index ee3df5d2a78..c520e321703 100644 --- a/cloud/apps/sandbox-control/src/api/router.ts +++ b/cloud/apps/sandbox-control/src/api/router.ts @@ -130,7 +130,7 @@ function messageBucket(caller: ApiCaller): string { } export function isApiPath(pathname: string): boolean { - return /^\/v1\/(me|api-keys|settings|audit|projects|workspaces|routines|oauth-approvals)(\/|$)/.test(pathname) + return /^\/v1\/(me|api-keys|settings|audit|projects|workspaces|routines|oauth-approvals|oauth-grants)(\/|$)/.test(pathname) } export async function handleApi(request: Request, url: URL, env: Parameters[0], ctx: ApiContext): Promise { diff --git a/cloud/apps/sandbox-control/src/index.ts b/cloud/apps/sandbox-control/src/index.ts index d998f59d227..1991aaaf6f1 100644 --- a/cloud/apps/sandbox-control/src/index.ts +++ b/cloud/apps/sandbox-control/src/index.ts @@ -16,7 +16,8 @@ import { maintainPool, poolConfigFromEnv } from './pool' import { authenticateDesktop } from './auth' import { authenticateApi } from './api/caller' import { handleApi, handleApiAs, isApiPath, type ApiContext, type ApiExtension } from './api/router' -import { handleMcpRoutes, isMcpPath, oauthApprovalsApi } from './mcp' +import { handleMcpRoutes, isMcpPath, oauthApprovalsApi, oauthGrantsApi } from './mcp' +import { purgeOAuth } from './mcp/oauth' import { retryPendingMessages } from './api/messages' import { revokeWorkspaceTokens } from './api/keys' import { provisionWorkspaceToken } from './api/settings' @@ -301,7 +302,7 @@ async function apiContext(env: Env, deps: Deps, request: Request, life?: Promise life: await (life ?? lifecycleDeps(env, deps)), waitUntil: deps.waitUntil ?? ((promise) => void promise.catch(() => undefined)), readJson: () => readJson(request), - extensions: [oauthApprovalsApi, ...(deps.apiExtensions ?? [])] + extensions: [oauthApprovalsApi, oauthGrantsApi,...(deps.apiExtensions ?? [])] } } @@ -691,6 +692,11 @@ export default { console.log(JSON.stringify({ event: 'message_retry', retried })) } } + // Abandoned MCP client registrations and dead OAuth codes/approvals. + const purged = await purgeOAuth(env.SANDBOX_DB, Date.now()).catch(() => -1) + if (purged !== 0) { + console.log(JSON.stringify({ event: 'oauth_purge', purged })) + } await advanceGoldenImagesFromEnv(env).catch((error: unknown) => console.log(JSON.stringify({ event: 'golden_image_tick_failed', error: String(error).slice(0, 200) })) ) diff --git a/cloud/apps/sandbox-control/src/mcp/index.ts b/cloud/apps/sandbox-control/src/mcp/index.ts index 38f413ea7c6..592429a5ba0 100644 --- a/cloud/apps/sandbox-control/src/mcp/index.ts +++ b/cloud/apps/sandbox-control/src/mcp/index.ts @@ -2,7 +2,7 @@ // /v1/oauth-approvals API route the desktop uses to approve an MCP client. import { z } from 'zod' import { auditCaller } from '../api/audit' -import { ApiError, scopesForRole, type ApiCaller } from '../api/caller' +import { ApiError, isAdmin, scopesForRole, type ApiCaller } from '../api/caller' import { API_SCOPES, type ApiScope } from '../api/keys' import type { ApiExtension } from '../api/router' import { @@ -15,6 +15,10 @@ import { protectedResourceMetadataUrl, registerClient, revoke, + revokeFamily, + grantOwner, + grantView, + listGrants, token } from './oauth' import { handleMcpPost } from './server' @@ -170,3 +174,36 @@ export const oauthApprovalsApi: ApiExtension = async (request, url, caller, ctx) await auditCaller(ctx.db, caller, { action: 'mcp.approval_create', targetType: 'account', targetId: caller.accountId, detail: { scopes: requested } }, now) return Response.json(approval, { status: 201, headers: { 'cache-control': 'no-store' } }) } + +/** + * Connected MCP apps (signed-in desktop only): GET /v1/oauth-grants lists live grants (members see + * their own, owners/admins the account's); DELETE /v1/oauth-grants/:id revokes one for good. + */ +export const oauthGrantsApi: ApiExtension = async (request, url, caller, ctx) => { + if (url.pathname !== '/v1/oauth-grants' && !url.pathname.startsWith('/v1/oauth-grants/')) { + return null + } + const match = /^\/v1\/oauth-grants(?:\/(fam_[A-Za-z0-9]{20}))?$/.exec(url.pathname) + if (!match) { + throw new ApiError(404, 'not_found') + } + if (caller.via !== 'session') { + throw new ApiError(403, 'forbidden', 'session_required') + } + const now = ctx.now() + const familyId = match[1] + if (!familyId && request.method === 'GET') { + const rows = await listGrants(ctx.db, caller.accountId, now, isAdmin(caller) ? undefined : caller.userId) + return Response.json({ grants: rows.map(grantView) }, { headers: { 'cache-control': 'no-store' } }) + } + if (familyId && request.method === 'DELETE') { + const owner = await grantOwner(ctx.db, caller.accountId, familyId) + if (!owner || (owner.created_by !== caller.userId && !isAdmin(caller))) { + throw new ApiError(404, 'not_found') + } + const revoked = await revokeFamily(ctx.db, familyId, 'user_revoked', now) + await auditCaller(ctx.db, caller, { action: 'mcp.grant_revoke', targetType: 'oauth_grant', targetId: familyId, detail: { clientId: owner.client_id, tokensRevoked: revoked } }, now) + return Response.json({ revoked: true }, { headers: { 'cache-control': 'no-store' } }) + } + throw new ApiError(405, 'method_not_allowed') +} diff --git a/cloud/apps/sandbox-control/src/mcp/oauth.ts b/cloud/apps/sandbox-control/src/mcp/oauth.ts index 62af6710b6b..8a2c8078d33 100644 --- a/cloud/apps/sandbox-control/src/mcp/oauth.ts +++ b/cloud/apps/sandbox-control/src/mcp/oauth.ts @@ -26,6 +26,11 @@ const MAX_REDIRECT_URIS = 10 const REGISTRATIONS_PER_MINUTE_PER_IP = 10 const CONSENTS_PER_MINUTE_PER_IP = 20 const TOKENS_PER_MINUTE_PER_CLIENT = 60 +const TOKENS_PER_MINUTE_PER_IP = 120 +/** A grant ends 90 days after its code exchange, however often it is refreshed: re-consent then. */ +export const GRANT_TTL_MS = 90 * 86_400_000 +/** Registrations that never complete a code exchange are purged after this. */ +const UNUSED_CLIENT_TTL_MS = 86_400_000 /** What an MCP client gets when it asks for nothing specific: everything but admin and project edits. */ export const DEFAULT_MCP_SCOPES: readonly ApiScope[] = ['workspaces:read', 'workspaces:write', 'messages:write', 'projects:read', 'routines:read', 'routines:write'] @@ -322,10 +327,10 @@ export async function authorize(request: Request, url: URL, env: OAuthEnv): Prom const familyId = randomId('fam') await env.db .prepare( - `INSERT INTO oauth_codes (code_hash, client_id, account_id, user_id, creator_role, scopes, redirect_uri, code_challenge, resource, expires_at, family_id, created_at) - VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)` + `INSERT INTO oauth_codes (code_hash, client_id, account_id, user_id, creator_role, scopes, redirect_uri, code_challenge, resource, expires_at, family_id, created_at, grantor_key_id) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)` ) - .bind(await sha256Hex(code), req.clientId, grantor.accountId, grantor.userId, grantor.role, scopes.join(' '), req.redirectUri, req.codeChallenge, req.resource, now + CODE_TTL_MS, familyId, now) + .bind(await sha256Hex(code), req.clientId, grantor.accountId, grantor.userId, grantor.role, scopes.join(' '), req.redirectUri, req.codeChallenge, req.resource, now + CODE_TTL_MS, familyId, now, grantor.keyId) .run() await audit( env.db, @@ -360,7 +365,7 @@ function viewFor(req: AuthorizeParams, client: ClientRow, csrf: string, error: s } } -type Grantor = { accountId: string; userId: string; role: OrgRole | null; scopes: Set; via: 'approval_code' | 'api_key' } +type Grantor = { accountId: string; userId: string; role: OrgRole | null; scopes: Set; via: 'approval_code' | 'api_key'; keyId: string | null } function orgOf(accountId: string): string | undefined { return accountId.startsWith('org:') ? accountId.slice(4) : accountId.startsWith('dev:org:') ? accountId.slice(8) : undefined @@ -375,7 +380,7 @@ async function resolveGrantor(db: D1Database, credential: string, now: number): .bind(now, hash, now) .first<{ account_id: string; user_id: string; creator_role: OrgRole | null; scopes: string }>() if (!claimed) return null - return { accountId: claimed.account_id, userId: claimed.user_id, role: claimed.creator_role, scopes: new Set(parseScopes(claimed.scopes)), via: 'approval_code' } + return { accountId: claimed.account_id, userId: claimed.user_id, role: claimed.creator_role, scopes: new Set(parseScopes(claimed.scopes)), via: 'approval_code', keyId: null } } const key = await resolveApiKey(db, credential, now) // Only a person's own key; workspace and OAuth tokens cannot bootstrap new grants. @@ -387,7 +392,8 @@ async function resolveGrantor(db: D1Database, credential: string, now: number): userId: key.created_by, role: key.creator_role, scopes: new Set(parseScopes(key.scopes).filter((scope) => allowed.has(scope))), - via: 'api_key' + via: 'api_key', + keyId: key.id } } @@ -413,20 +419,50 @@ async function pkceS256(verifier: string): Promise { return btoa(String.fromCharCode(...digest)).replaceAll('+', '-').replaceAll('/', '_').replace(/=+$/, '') } -async function revokeFamily(db: D1Database, familyId: string, now: number): Promise { - await db.prepare(`UPDATE api_keys SET revoked_at = ?, revoked_by = 'system' WHERE family_id = ? AND revoked_at IS NULL`).bind(now, familyId).run() +/** Ends a grant for good: its tokens are revoked and the family is remembered (see issueTokens). */ +export async function revokeFamily(db: D1Database, familyId: string, reason: string, now: number): Promise { + const [, revoked] = await db.batch([ + db.prepare('INSERT OR IGNORE INTO oauth_revoked_families (family_id, reason, revoked_at) VALUES (?, ?, ?)').bind(familyId, reason, now), + db.prepare(`UPDATE api_keys SET revoked_at = ?, revoked_by = 'system' WHERE family_id = ? AND revoked_at IS NULL`).bind(now, familyId) + ]) + return revoked!.meta.changes +} + +async function familyRevoked(db: D1Database, familyId: string): Promise { + return (await db.prepare('SELECT 1 AS x FROM oauth_revoked_families WHERE family_id = ?').bind(familyId).first()) !== null } async function issueTokens( env: OAuthEnv, - grant: { accountId: string; userId: string; role: OrgRole | null; scopes: ApiScope[]; clientId: string; clientName: string | null; familyId: string }, - now: number + grant: { + accountId: string + userId: string + role: OrgRole | null + scopes: ApiScope[] + clientId: string + clientName: string | null + familyId: string + grantorKeyId: string | null + familyExpiresAt: number + }, + now: number, + /** Re-checked after minting: true when the code was replayed meanwhile. */ + replayed: () => Promise ): Promise { const name = `MCP · ${grant.clientName ?? grant.clientId}`.slice(0, 100) - const oauth = { clientId: grant.clientId, audience: mcpResource(env.origin), familyId: grant.familyId } + const oauth = { clientId: grant.clientId, audience: mcpResource(env.origin), familyId: grant.familyId, grantorKeyId: grant.grantorKeyId, familyExpiresAt: grant.familyExpiresAt } const base = { accountId: grant.accountId, createdBy: grant.userId, creatorRole: grant.role, name, scopes: grant.scopes, oauth } - const access = await mintApiKey(env.db, { ...base, kind: 'oauth', expiresAt: now + ACCESS_TOKEN_TTL_MS }, now) - const refresh = await mintApiKey(env.db, { ...base, kind: 'oauth_refresh', expiresAt: now + REFRESH_TOKEN_TTL_MS }, now) + const access = await mintApiKey(env.db, { ...base, kind: 'oauth', expiresAt: Math.min(now + ACCESS_TOKEN_TTL_MS, grant.familyExpiresAt) }, now) + const refresh = await mintApiKey(env.db, { ...base, kind: 'oauth_refresh', expiresAt: Math.min(now + REFRESH_TOKEN_TTL_MS, grant.familyExpiresAt) }, now) + // Close the window between claim and mint: a replay, a revoked family or a revoked approving key + // that landed meanwhile takes these new tokens down too. + const grantorGone = + grant.grantorKeyId !== null && + (await env.db.prepare('SELECT 1 AS x FROM api_keys WHERE id = ? AND revoked_at IS NULL AND (expires_at IS NULL OR expires_at > ?)').bind(grant.grantorKeyId, now).first()) === null + if (grantorGone || (await replayed()) || (await familyRevoked(env.db, grant.familyId))) { + await revokeFamily(env.db, grant.familyId, grantorGone ? 'grantor_revoked' : 'replay', now) + return oauthError('invalid_grant', grantorGone ? 'The credential that approved this grant was revoked' : 'Grant revoked') + } return json({ access_token: access.secret, token_type: 'Bearer', @@ -449,6 +485,8 @@ type CodeRow = { expires_at: number consumed_at: number | null family_id: string | null + grantor_key_id: string | null + replayed_at: number | null } export async function token(request: Request, env: OAuthEnv): Promise { @@ -457,6 +495,8 @@ export async function token(request: Request, env: OAuthEnv): Promise } const form = await readForm(request) const now = env.now() + const tooFast = await limited(env.db, `oauth-token-ip:${clientIp(request)}`, TOKENS_PER_MINUTE_PER_IP, now) + if (tooFast) return tooFast const client = await getClient(env.db, form.get('client_id')) if (!client) { return oauthError('invalid_client', 'Unknown client_id', 401) @@ -477,21 +517,35 @@ export async function token(request: Request, env: OAuthEnv): Promise const hash = await sha256Hex(code) const row = await env.db.prepare('SELECT * FROM oauth_codes WHERE code_hash = ?').bind(hash).first() if (!row || row.client_id !== client.client_id) return oauthError('invalid_grant', 'Invalid authorization code') - if (row.consumed_at !== null) { + const replay = async () => { // A replayed code means it leaked: everything minted from it dies (OAuth 2.1 §4.1.3). - if (row.family_id) await revokeFamily(env.db, row.family_id, now) + await env.db.prepare('UPDATE oauth_codes SET replayed_at = ? WHERE code_hash = ?').bind(now, hash).run() + if (row.family_id) await revokeFamily(env.db, row.family_id, 'code_replay', now) await audit(env.db, { accountId: row.account_id, type: 'mcp', id: client.client_id, userId: row.user_id }, { action: 'mcp.code_replay', targetType: 'oauth_client', targetId: client.client_id }, now) return oauthError('invalid_grant', 'Authorization code already used') } + if (row.consumed_at !== null) return replay() if (row.expires_at <= now) return oauthError('invalid_grant', 'Authorization code expired') if (form.get('redirect_uri') !== null && form.get('redirect_uri') !== row.redirect_uri) return oauthError('invalid_grant', 'redirect_uri does not match') if (!timingSafeEqual(await pkceS256(verifier), row.code_challenge)) return oauthError('invalid_grant', 'PKCE verification failed') const claimed = await env.db.prepare('UPDATE oauth_codes SET consumed_at = ? WHERE code_hash = ? AND consumed_at IS NULL').bind(now, hash).run() - if (claimed.meta.changes !== 1) return oauthError('invalid_grant', 'Authorization code already used') + if (claimed.meta.changes !== 1) return replay() + await env.db.prepare('UPDATE oauth_clients SET last_granted_at = ? WHERE client_id = ?').bind(now, client.client_id).run() return issueTokens( env, - { accountId: row.account_id, userId: row.user_id, role: row.creator_role, scopes: parseScopes(row.scopes), clientId: client.client_id, clientName: client.client_name, familyId: row.family_id ?? randomId('fam') }, - now + { + accountId: row.account_id, + userId: row.user_id, + role: row.creator_role, + scopes: parseScopes(row.scopes), + clientId: client.client_id, + clientName: client.client_name, + familyId: row.family_id ?? randomId('fam'), + grantorKeyId: row.grantor_key_id, + familyExpiresAt: now + GRANT_TTL_MS + }, + now, + async () => (await env.db.prepare('SELECT replayed_at FROM oauth_codes WHERE code_hash = ?').bind(hash).first<{ replayed_at: number | null }>())?.replayed_at != null ) } @@ -503,12 +557,17 @@ export async function token(request: Request, env: OAuthEnv): Promise if (row.revoked_at !== null) { // Rotated tokens are single use: a second use means a copy leaked, so end the whole grant. if (row.family_id && row.revoked_by === 'rotated') { - await revokeFamily(env.db, row.family_id, now) + await revokeFamily(env.db, row.family_id, 'refresh_replay', now) await audit(env.db, { accountId: row.account_id, type: 'mcp', id: client.client_id, userId: row.created_by }, { action: 'mcp.refresh_replay', targetType: 'oauth_client', targetId: client.client_id }, now) } return oauthError('invalid_grant', 'Refresh token revoked') } if (row.expires_at !== null && row.expires_at <= now) return oauthError('invalid_grant', 'Refresh token expired') + // Rows from before migration 0028 have no family end: count 90 days from this token. + const familyExpiresAt = row.family_expires_at ?? row.created_at + GRANT_TTL_MS + if (familyExpiresAt <= now) return oauthError('invalid_grant', 'Grant expired; reconnect to approve again') + if (!row.family_id || (await familyRevoked(env.db, row.family_id))) return oauthError('invalid_grant', 'Grant revoked') + const familyId = row.family_id const rotated = await env.db.prepare(`UPDATE api_keys SET revoked_at = ?, revoked_by = 'rotated' WHERE id = ? AND revoked_at IS NULL`).bind(now, row.id).run() if (rotated.meta.changes !== 1) return oauthError('invalid_grant', 'Refresh token revoked') const granted = parseScopes(row.scopes) @@ -517,8 +576,20 @@ export async function token(request: Request, env: OAuthEnv): Promise if (scopes.length === 0) return oauthError('invalid_scope', 'Requested scope exceeds the grant') return issueTokens( env, - { accountId: row.account_id, userId: row.created_by, role: row.creator_role, scopes, clientId: client.client_id, clientName: client.client_name, familyId: row.family_id ?? randomId('fam') }, - now + { + accountId: row.account_id, + userId: row.created_by, + role: row.creator_role, + scopes, + clientId: client.client_id, + clientName: client.client_name, + familyId, + grantorKeyId: row.grantor_key_id ?? null, + familyExpiresAt + }, + now, + // A replay of the token just rotated revokes the family, which issueTokens re-checks. + async () => false ) } @@ -536,7 +607,7 @@ export async function revoke(request: Request, env: OAuthEnv): Promise if (/^orca_(oat|ort)_[A-Za-z0-9]{32}$/.test(presented)) { const row = await env.db.prepare(`SELECT * FROM api_keys WHERE secret_hash = ? AND kind IN ('oauth', 'oauth_refresh')`).bind(await sha256Hex(presented)).first() if (row && row.family_id && (clientId === null || clientId === row.client_id)) { - await revokeFamily(env.db, row.family_id, env.now()) + await revokeFamily(env.db, row.family_id, 'client_revoked', env.now()) } } return json({}) @@ -558,3 +629,72 @@ export async function mintApproval( .run() return { code, expiresAt, scopes } } + +// --- Grants (connected apps) and housekeeping ----------------------------------------------- + +export type GrantRow = { + family_id: string + client_id: string + client_name: string | null + created_by: string + scopes: string + family_expires_at: number | null + granted_at: number + last_used_at: number | null + via_key: string | null +} + +/** Live grants: one row per family that still has an unrevoked, unexpired refresh token. */ +export async function listGrants(db: D1Database, accountId: string, now: number, createdBy?: string): Promise { + const { results } = await db + .prepare( + `SELECT r.family_id, r.client_id, c.client_name, r.created_by, r.scopes, r.family_expires_at, + (SELECT MIN(f.created_at) FROM api_keys f WHERE f.family_id = r.family_id) AS granted_at, + (SELECT MAX(f.last_used_at) FROM api_keys f WHERE f.family_id = r.family_id) AS last_used_at, + (SELECT k.prefix FROM api_keys k WHERE k.id = r.grantor_key_id) AS via_key + FROM api_keys r LEFT JOIN oauth_clients c ON c.client_id = r.client_id + WHERE r.account_id = ? AND r.kind = 'oauth_refresh' AND r.revoked_at IS NULL AND r.expires_at > ? AND r.family_id IS NOT NULL + ${createdBy ? 'AND r.created_by = ?' : ''} + ORDER BY granted_at DESC LIMIT 200` + ) + .bind(accountId, now, ...(createdBy ? [createdBy] : [])) + .all() + return results +} + +export function grantView(row: GrantRow) { + return { + id: row.family_id, + clientId: row.client_id, + clientName: row.client_name, + userId: row.created_by, + scopes: row.scopes.split(' ').filter(Boolean), + approvedWith: row.via_key ? { apiKeyPrefix: row.via_key } : { approvalCode: true }, + grantedAt: row.granted_at, + lastUsedAt: row.last_used_at, + expiresAt: row.family_expires_at + } +} + +/** Who owns a grant family inside an account, or null when it is not this account's. */ +export function grantOwner(db: D1Database, accountId: string, familyId: string): Promise<{ created_by: string; client_id: string | null } | null> { + return db + .prepare(`SELECT created_by, client_id FROM api_keys WHERE family_id = ? AND account_id = ? AND kind IN ('oauth', 'oauth_refresh') LIMIT 1`) + .bind(familyId, accountId) + .first() +} + +/** Cron housekeeping: abandoned registrations and dead codes/approvals. Returns rows deleted. */ +export async function purgeOAuth(db: D1Database, now: number): Promise { + const results = await db.batch([ + db + .prepare( + `DELETE FROM oauth_clients WHERE last_granted_at IS NULL AND created_at < ? + AND NOT EXISTS (SELECT 1 FROM oauth_codes c WHERE c.client_id = oauth_clients.client_id AND c.expires_at > ?)` + ) + .bind(now - UNUSED_CLIENT_TTL_MS, now), + db.prepare('DELETE FROM oauth_codes WHERE expires_at < ?').bind(now - 86_400_000), + db.prepare('DELETE FROM oauth_approvals WHERE expires_at < ?').bind(now - 86_400_000) + ]) + return results.reduce((sum, r) => sum + r.meta.changes, 0) +} diff --git a/cloud/apps/sandbox-control/src/operator.ts b/cloud/apps/sandbox-control/src/operator.ts index 5d41b8d8d2d..22745c0667a 100644 --- a/cloud/apps/sandbox-control/src/operator.ts +++ b/cloud/apps/sandbox-control/src/operator.ts @@ -124,6 +124,11 @@ export async function handleOperator( .prepare(`UPDATE api_keys SET revoked_at = ?, revoked_by = 'system:member_removed' WHERE account_id = ? AND created_by = ? AND revoked_at IS NULL`) .bind(now, body.data.accountId, body.data.userId) .run() + // MCP OAuth tokens are api_keys rows (revoked above); pending codes and approvals die too. + await life.db.batch([ + life.db.prepare('UPDATE oauth_codes SET consumed_at = ? WHERE account_id = ? AND user_id = ? AND consumed_at IS NULL').bind(now, body.data.accountId, body.data.userId), + life.db.prepare('UPDATE oauth_approvals SET consumed_at = ? WHERE account_id = ? AND user_id = ? AND consumed_at IS NULL').bind(now, body.data.accountId, body.data.userId) + ]) await audit(life.db, { accountId: body.data.accountId, type: 'system', id: 'auth', userId: body.data.userId }, { action: 'member.removed', detail: { keysRevoked: result.meta.changes } }, now) return Response.json({ revoked: result.meta.changes }) } diff --git a/cloud/apps/sandbox-control/test/mcp.test.ts b/cloud/apps/sandbox-control/test/mcp.test.ts index 0b5aa8850e7..7abafbeeb85 100644 --- a/cloud/apps/sandbox-control/test/mcp.test.ts +++ b/cloud/apps/sandbox-control/test/mcp.test.ts @@ -1,13 +1,14 @@ import { createHash, randomBytes } from 'node:crypto' import { describe, expect, it } from 'vitest' +import { purgeOAuth } from '../src/mcp/oauth' import { createHarness } from './harness' const ORIGIN = 'https://sandbox.test' const REDIRECT = 'http://127.0.0.1:33418/callback' -async function harness() { +async function harness(overrides: Record = {}) { const pending: Promise[] = [] - const h = await createHarness({ PUBLIC_API_URL: ORIGIN }, { waitUntil: (p) => void pending.push(p) }) + const h = await createHarness({ PUBLIC_API_URL: ORIGIN, ...overrides }, { waitUntil: (p) => void pending.push(p) }) let rpcId = 0 async function rpc(bearer: string | null, method: string, params?: unknown, headers: Record = {}) { @@ -431,3 +432,123 @@ describe('MCP authorization', () => { expect(again.res.status).toBe(401) }) }) + +describe('MCP grants', () => { + it('lists connected apps, lets owners and admins disconnect them, and audits it', async () => { + const h = await harness() + const memberJwt = await h.token('bob', 'acme', 'sandbox', { orgRole: 'member' }) + const adminJwt = await h.token('carol', 'acme', 'sandbox', { orgRole: 'admin' }) + const bobKey = await h.apiKey(['workspaces:read'], 'bob', { org: 'acme', role: 'member' }) + const carolKey = await h.apiKey(['workspaces:read'], 'carol', { org: 'acme', role: 'admin' }) + const bob = await h.connect(bobKey, { scope: 'workspaces:read' }) + const carol = await h.connect(carolKey, { scope: 'workspaces:read' }) + await h.rpc(bob.tokens.access_token, 'ping') + + const mine = await h.call('GET', '/v1/oauth-grants', { bearer: memberJwt }) + expect(mine.status).toBe(200) + expect(mine.body.grants).toHaveLength(1) + const grant = mine.body.grants[0] + expect(grant).toMatchObject({ clientName: 'Test Client', userId: 'bob', scopes: ['workspaces:read'], approvedWith: { apiKeyPrefix: bobKey.slice(0, 12) } }) + expect(grant.id).toMatch(/^fam_/) + expect(grant.expiresAt).toBe(grant.grantedAt + 90 * 86_400_000) + expect((await h.call('GET', '/v1/oauth-grants', { bearer: adminJwt })).body.grants).toHaveLength(2) + // Keys cannot manage grants; other accounts cannot see them. + expect((await h.call('GET', '/v1/oauth-grants', { bearer: bobKey })).status).toBe(403) + expect((await h.call('GET', '/v1/oauth-grants', { as: 'mallory' })).body.grants).toHaveLength(0) + + // A member cannot disconnect the admin's app; the admin can disconnect the member's. + const carolGrant = (await h.call('GET', '/v1/oauth-grants', { bearer: adminJwt })).body.grants.find((g: { userId: string }) => g.userId === 'carol') + expect((await h.call('DELETE', `/v1/oauth-grants/${carolGrant.id}`, { bearer: memberJwt })).status).toBe(404) + expect((await h.call('DELETE', `/v1/oauth-grants/${grant.id}`, { as: 'mallory' })).status).toBe(404) + expect((await h.call('DELETE', `/v1/oauth-grants/${grant.id}`, { bearer: adminJwt })).status).toBe(200) + expect((await h.rpc(bob.tokens.access_token, 'ping')).status).toBe(401) + expect((await h.tokenRequest({ grant_type: 'refresh_token', refresh_token: bob.tokens.refresh_token, client_id: bob.client.client_id })).body.error).toBe('invalid_grant') + expect((await h.rpc(carol.tokens.access_token, 'ping')).status).toBe(200) + expect((await h.call('GET', '/v1/oauth-grants', { bearer: memberJwt })).body.grants).toHaveLength(0) + expect(h.raw.prepare(`SELECT actor_type, target_id FROM audit_log WHERE action = 'mcp.grant_revoke'`).all()).toEqual([{ actor_type: 'user', target_id: grant.id }]) + }) + + it('revokes every grant an API key approved when that key is revoked, pending codes included', async () => { + const h = await harness() + const key = await h.apiKey(['workspaces:read']) + const { client, tokens } = await h.connect(key, { scope: 'workspaces:read' }) + const next = await h.tokenRequest({ grant_type: 'refresh_token', refresh_token: tokens.refresh_token, client_id: client.client_id }) + expect(next.status).toBe(200) + // A code approved with the key but not exchanged yet. + const pending = h.pkce() + const consent = await h.consent(h.authorizeParams(client.client_id, pending.challenge, { scope: 'workspaces:read' }), key) + const id = (h.raw.prepare(`SELECT id FROM api_keys WHERE kind = 'user'`).get() as { id: string }).id + expect((await h.call('DELETE', `/v1/api-keys/${id}`)).status).toBe(200) + expect((await h.rpc(next.body.access_token, 'ping')).status).toBe(401) + expect((await h.tokenRequest({ grant_type: 'refresh_token', refresh_token: next.body.refresh_token, client_id: client.client_id })).body.error).toBe('invalid_grant') + const late = await h.tokenRequest({ grant_type: 'authorization_code', code: consent.location!.searchParams.get('code')!, code_verifier: pending.verifier, client_id: client.client_id }) + expect(late.body).toMatchObject({ error: 'invalid_grant', error_description: 'The credential that approved this grant was revoked' }) + expect(h.raw.prepare(`SELECT COUNT(*) AS n FROM api_keys WHERE kind IN ('oauth', 'oauth_refresh') AND revoked_at IS NULL`).get()).toEqual({ n: 0 }) + }) + + it('ends a grant 90 days after approval however often it is refreshed', async () => { + const h = await harness() + const key = await h.apiKey(['workspaces:read']) + const { client, tokens } = await h.connect(key, { scope: 'workspaces:read' }) + let refresh = tokens.refresh_token + for (let i = 0; i < 3; i++) { + h.advance(29 * 86_400_000) + const next = await h.tokenRequest({ grant_type: 'refresh_token', refresh_token: refresh, client_id: client.client_id }) + expect(next.status).toBe(200) + refresh = next.body.refresh_token + } + // The last pair cannot outlive the grant. + const last = h.raw.prepare(`SELECT expires_at, family_expires_at FROM api_keys WHERE kind = 'oauth_refresh' AND revoked_at IS NULL`).get() as { expires_at: number; family_expires_at: number } + expect(last.expires_at).toBe(last.family_expires_at) + h.advance(3 * 86_400_000 + 1) + expect((await h.tokenRequest({ grant_type: 'refresh_token', refresh_token: refresh, client_id: client.client_id })).body.error).toBe('invalid_grant') + }) + + it('kills tokens minted while the code was being replayed (claim/mint race)', async () => { + const h = await harness() + const key = await h.apiKey(['workspaces:read']) + const client = (await h.register()).body + const { verifier, challenge } = h.pkce() + const { location } = await h.consent(h.authorizeParams(client.client_id, challenge, { scope: 'workspaces:read' }), key) + // Simulate a replay landing between the claim and the mint: the first token insert flags it. + h.raw.exec(`CREATE TRIGGER replay_mid_flight AFTER INSERT ON api_keys WHEN NEW.kind = 'oauth' BEGIN UPDATE oauth_codes SET replayed_at = 1; END`) + const res = await h.tokenRequest({ grant_type: 'authorization_code', code: location!.searchParams.get('code')!, code_verifier: verifier, client_id: client.client_id }) + expect(res.body).toMatchObject({ error: 'invalid_grant', error_description: 'Grant revoked' }) + expect(h.raw.prepare(`SELECT COUNT(*) AS n FROM api_keys WHERE kind IN ('oauth', 'oauth_refresh') AND revoked_at IS NULL`).get()).toEqual({ n: 0 }) + }) + + it("consumes a removed member's pending codes and approval codes", async () => { + const OP = 'o'.repeat(40) + const h = await harness({ OPERATOR_TOKEN: OP }) + const bobJwt = await h.token('bob', 'acme', 'sandbox', { orgRole: 'member' }) + const approval = (await h.call('POST', '/v1/oauth-approvals', { bearer: bobJwt, body: {} })).body.code + const unused = (await h.call('POST', '/v1/oauth-approvals', { bearer: bobJwt, body: {} })).body.code + const client = (await h.register()).body + const { verifier, challenge } = h.pkce() + const { location } = await h.consent(h.authorizeParams(client.client_id, challenge, { scope: 'workspaces:read' }), approval) + const removed = await h.call('POST', '/v1/internal/members/removed', { bearer: OP, body: { accountId: 'org:acme', userId: 'bob' } }) + expect(removed.status).toBe(200) + expect((await h.tokenRequest({ grant_type: 'authorization_code', code: location!.searchParams.get('code')!, code_verifier: verifier, client_id: client.client_id })).body.error).toBe('invalid_grant') + expect((await h.consent(h.authorizeParams(client.client_id, h.pkce().challenge), unused)).res.status).toBe(401) + }) + + it('purges abandoned registrations and dead codes, and limits the token endpoint per IP', async () => { + const h = await harness() + const key = await h.apiKey(['workspaces:read']) + const used = await h.connect(key, { scope: 'workspaces:read' }) + const abandoned = (await h.register()).body + h.advance(25 * 3_600_000) + expect(await purgeOAuth(h.env.SANDBOX_DB, h.now())).toBeGreaterThanOrEqual(1) + const left = (h.raw.prepare('SELECT client_id FROM oauth_clients').all() as { client_id: string }[]).map((r) => r.client_id) + expect(left).toContain(used.client.client_id) + expect(left).not.toContain(abandoned.client_id) + h.advance(24 * 3_600_000) + await purgeOAuth(h.env.SANDBOX_DB, h.now()) + expect(h.raw.prepare('SELECT COUNT(*) AS n FROM oauth_codes').get()).toEqual({ n: 0 }) + + const hit = () => + h.send(new Request(`${ORIGIN}/oauth/token`, { method: 'POST', headers: { 'content-type': 'application/x-www-form-urlencoded', 'cf-connecting-ip': '203.0.113.9' }, body: 'grant_type=refresh_token&client_id=mcpc_00000000000000000000' })) + for (let i = 0; i < 120; i++) expect((await hit()).status).toBe(401) + expect((await hit()).status).toBe(429) + }) +})