diff --git a/docs/configuration.md b/docs/configuration.md index 3502a98b..ac09c5d3 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -81,6 +81,18 @@ DevSpace uses a single-user OAuth approval flow. | `DEVSPACE_OAUTH_REFRESH_TOKEN_TTL_SECONDS` | `2592000` | | `DEVSPACE_OAUTH_SCOPES` | `devspace` | | `DEVSPACE_OAUTH_ALLOWED_REDIRECT_HOSTS` | `chatgpt.com,localhost,127.0.0.1` | +| `DEVSPACE_OAUTH_CLIENT_REGISTRATION_KEY` | Generated by `devspace init`; older configs derive a compatibility key from the Owner password; at least 32 characters when supplied explicitly | + +New public OAuth registrations use a signed client identifier. If the SQLite +client row is later lost, a reconnect can reconstruct the original registration, +revalidate its redirect URI against the current allowlist, and show the Owner +password approval page again. The client row is restored only after successful +approval. + +Access and refresh tokens remain opaque, hashed, stateful, and revocable. They +are never reconstructed from the client identifier. Registrations created by an +older DevSpace version still use random client identifiers and must register +once with the newer version before this recovery path applies. MCP clients discover metadata from: diff --git a/docs/security.md b/docs/security.md index d7ec0e1d..3c2d5215 100644 --- a/docs/security.md +++ b/docs/security.md @@ -43,10 +43,23 @@ reach. When an MCP client connects, DevSpace shows an approval page. Enter the Owner password only when you intentionally want that client to access this server. +Fresh setups store a separate random key in the same private file to authenticate +recoverable OAuth client registrations. Older auth files without that field use +a memory-hard compatibility key derived from the Owner password. The resulting +client identifier is public and is not a credential: recovery still requires an +exact registered redirect URI, PKCE, the current redirect-host allowlist, and a +fresh Owner password approval. Access and refresh tokens are not recoverable and +remain revocable server-side state. + +Repository coverage exercises this recovery through `SingleUserOAuthProvider` +in process. A packaged reconnect through a real MCP host has not yet been +verified. + For env-driven deployments, set a long random value: ```bash DEVSPACE_OAUTH_OWNER_TOKEN="$(openssl rand -base64 32)" +DEVSPACE_OAUTH_CLIENT_REGISTRATION_KEY="$(openssl rand -base64 32)" ``` ## Public URL And Host Allowlist diff --git a/package.json b/package.json index 5d4a7faf..a2904851 100644 --- a/package.json +++ b/package.json @@ -28,7 +28,7 @@ "dev": "node scripts/dev-server.mjs", "postinstall": "node scripts/fix-node-pty-permissions.mjs", "start": "node dist/cli.js serve", - "test": "tsx src/config.test.ts && tsx src/request-meta.test.ts && tsx src/incoming-artifacts.test.ts && tsx src/artifact-download.test.ts && tsx src/ui/card-types.test.ts && tsx src/ui/patch-display.test.ts && tsx src/ui/tool-display.test.ts && tsx src/apply-patch.test.ts && tsx src/process-platform.test.ts && tsx src/process-sessions.test.ts && tsx src/mcp-sessions.test.ts && tsx src/server-shutdown.test.ts && tsx src/local-agent-runtime.test.ts && tsx src/local-agent-adapters.test.ts && tsx src/local-agent-availability.test.ts && tsx src/local-agent-profiles.test.ts && tsx src/local-agent-targets.test.ts && tsx src/local-agent-store.test.ts && tsx src/roots.test.ts && tsx src/skills.test.ts && tsx src/workspaces.test.ts && tsx src/workspace-conversation.test.ts && tsx src/review-checkpoints.test.ts && tsx src/server.test.ts && tsx src/oauth-store.test.ts && tsx src/cli.test.ts", + "test": "tsx src/config.test.ts && tsx src/request-meta.test.ts && tsx src/incoming-artifacts.test.ts && tsx src/artifact-download.test.ts && tsx src/ui/card-types.test.ts && tsx src/ui/patch-display.test.ts && tsx src/ui/tool-display.test.ts && tsx src/apply-patch.test.ts && tsx src/process-platform.test.ts && tsx src/process-sessions.test.ts && tsx src/mcp-sessions.test.ts && tsx src/server-shutdown.test.ts && tsx src/local-agent-runtime.test.ts && tsx src/local-agent-adapters.test.ts && tsx src/local-agent-availability.test.ts && tsx src/local-agent-profiles.test.ts && tsx src/local-agent-targets.test.ts && tsx src/local-agent-store.test.ts && tsx src/roots.test.ts && tsx src/skills.test.ts && tsx src/workspaces.test.ts && tsx src/workspace-conversation.test.ts && tsx src/review-checkpoints.test.ts && tsx src/server.test.ts && tsx src/oauth-client-registration.test.ts && tsx src/oauth-store.test.ts && tsx src/cli.test.ts", "typecheck": "tsc -p tsconfig.json --noEmit" }, "keywords": [], diff --git a/src/cli.ts b/src/cli.ts index 7a1ac63f..2ba0ccaa 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -30,6 +30,8 @@ import { createLocalAgentStore, type LocalAgentRecord } from "./local-agent-stor import type { LocalAgentRunResult } from "./local-agent-runtime.js"; import { ensureDevspaceDefaultSkills, + deriveClientRegistrationKey, + generateClientRegistrationKey, generateOwnerToken, loadDevspaceFiles, resolveSubagentsFlag, @@ -163,8 +165,14 @@ async function runInit({ force }: { force: boolean }): Promise { publicBaseUrl, subagents: resolveSubagentsFlag(files.config), }; + const ownerToken = files.auth.ownerToken ?? generateOwnerToken(); const auth = { - ownerToken: files.auth.ownerToken ?? generateOwnerToken(), + ownerToken, + clientRegistrationKey: + files.auth.clientRegistrationKey ?? + (files.auth.ownerToken + ? deriveClientRegistrationKey(files.auth.ownerToken) + : generateClientRegistrationKey()), }; const configPath = writeDevspaceConfig(config); diff --git a/src/config.test.ts b/src/config.test.ts index 9bc8b4c9..822d9c92 100644 --- a/src/config.test.ts +++ b/src/config.test.ts @@ -104,6 +104,24 @@ assert.throws( ); assert.equal(loadConfig(baseEnv).oauth.ownerToken, "test-owner-token-that-is-long-enough"); +const derivedClientRegistrationKey = loadConfig(baseEnv).oauth.clientRegistrationKey; +assert.equal(derivedClientRegistrationKey.length, 43); +assert.equal(loadConfig(baseEnv).oauth.clientRegistrationKey, derivedClientRegistrationKey); +assert.equal( + loadConfig({ + ...baseEnv, + DEVSPACE_OAUTH_CLIENT_REGISTRATION_KEY: "explicit-client-registration-key-long-enough", + }).oauth.clientRegistrationKey, + "explicit-client-registration-key-long-enough", +); +assert.throws( + () => + loadConfig({ + ...baseEnv, + DEVSPACE_OAUTH_CLIENT_REGISTRATION_KEY: "too-short", + }), + /DEVSPACE_OAUTH_CLIENT_REGISTRATION_KEY must be at least 32 characters long/, +); assert.deepEqual(loadConfig(baseEnv).oauth.scopes, ["devspace"]); assert.deepEqual(loadConfig(baseEnv).oauth.allowedRedirectHosts, [ "chatgpt.com", @@ -182,12 +200,17 @@ writeFileSync( join(configDir, "auth.json"), JSON.stringify({ ownerToken: "persisted-owner-token-long-enough", + clientRegistrationKey: "persisted-client-registration-key-long-enough", }), ); const fileConfig = loadConfig({ DEVSPACE_CONFIG_DIR: configDir }); assert.equal(fileConfig.port, 8787); assert.equal(fileConfig.oauth.ownerToken, "persisted-owner-token-long-enough"); +assert.equal( + fileConfig.oauth.clientRegistrationKey, + "persisted-client-registration-key-long-enough", +); assert.equal(fileConfig.publicBaseUrl, "https://devspace.example.com"); assert.equal(fileConfig.subagents, true); assert.equal(fileConfig.artifactsEnabled, true); diff --git a/src/config.ts b/src/config.ts index f8c8b995..4ca94786 100644 --- a/src/config.ts +++ b/src/config.ts @@ -3,7 +3,12 @@ import { join, resolve } from "node:path"; import { expandHomePath } from "./roots.js"; import type { LoggingConfig, LogFormat, LogLevel } from "./logger.js"; import type { OAuthConfig } from "./oauth-provider.js"; -import { devspaceAgentsDir, devspaceSkillsDir, loadDevspaceFiles } from "./user-config.js"; +import { + deriveClientRegistrationKey, + devspaceAgentsDir, + devspaceSkillsDir, + loadDevspaceFiles, +} from "./user-config.js"; export type ToolMode = "minimal" | "full" | "codex"; export type WidgetMode = "off" | "changes" | "full"; @@ -162,20 +167,39 @@ function parseWidgetMode(value: string | undefined): WidgetMode { throw new Error(`Invalid DEVSPACE_WIDGETS: ${value}`); } -function parseRequiredSecret(value: string | undefined, name: string): string { +function parseRequiredSecret( + value: string | undefined, + name: string, + minimumLength = 16, +): string { const secret = value?.trim(); if (!secret) { throw new Error(`${name} is required for DevSpace OAuth. Run: devspace init`); } - if (secret.length < 16) { - throw new Error(`${name} must be at least 16 characters long.`); + if (secret.length < minimumLength) { + throw new Error(`${name} must be at least ${minimumLength} characters long.`); } return secret; } -function parseOAuthConfig(env: NodeJS.ProcessEnv, ownerToken: string | undefined): OAuthConfig { +function parseOAuthConfig( + env: NodeJS.ProcessEnv, + ownerToken: string | undefined, + clientRegistrationKey: string | undefined, +): OAuthConfig { + const resolvedOwnerToken = parseRequiredSecret( + env.DEVSPACE_OAUTH_OWNER_TOKEN ?? ownerToken, + "DEVSPACE_OAUTH_OWNER_TOKEN", + ); return { - ownerToken: parseRequiredSecret(env.DEVSPACE_OAUTH_OWNER_TOKEN ?? ownerToken, "DEVSPACE_OAUTH_OWNER_TOKEN"), + ownerToken: resolvedOwnerToken, + clientRegistrationKey: parseRequiredSecret( + env.DEVSPACE_OAUTH_CLIENT_REGISTRATION_KEY ?? + clientRegistrationKey ?? + deriveClientRegistrationKey(resolvedOwnerToken), + "DEVSPACE_OAUTH_CLIENT_REGISTRATION_KEY", + 32, + ), accessTokenTtlSeconds: parsePositiveInteger( env.DEVSPACE_OAUTH_ACCESS_TOKEN_TTL_SECONDS, DEFAULT_OAUTH_ACCESS_TOKEN_TTL_SECONDS, @@ -226,7 +250,7 @@ export function loadConfig(env: NodeJS.ProcessEnv = process.env): ServerConfig { return { host, port, - oauth: parseOAuthConfig(env, files.auth.ownerToken), + oauth: parseOAuthConfig(env, files.auth.ownerToken, files.auth.clientRegistrationKey), allowedRoots: parseAllowedRoots(env.DEVSPACE_ALLOWED_ROOTS ?? files.config.allowedRoots), allowedHosts: parseAllowedHosts(env.DEVSPACE_ALLOWED_HOSTS, derivedAllowedHosts), publicBaseUrl, diff --git a/src/oauth-client-registration.test.ts b/src/oauth-client-registration.test.ts new file mode 100644 index 00000000..c1e50f38 --- /dev/null +++ b/src/oauth-client-registration.test.ts @@ -0,0 +1,64 @@ +import assert from "node:assert/strict"; +import { + createRecoverableClientId, + recoverClientRegistration, +} from "./oauth-client-registration.js"; + +const signingKey = "test-client-registration-key-that-is-long-enough"; +const client = { + redirect_uris: ["https://chatgpt.com/connector/oauth/test"], + client_name: "ChatGPT", + client_id_issued_at: 1_786_032_000, + token_endpoint_auth_method: "none" as const, + grant_types: ["authorization_code", "refresh_token"], + response_types: ["code"], +}; + +const created = createRecoverableClientId(client, signingKey); +assert.equal(created.kind, "recoverable"); +if (created.kind !== "recoverable") throw new Error("Expected recoverable client ID"); +const clientId = created.clientId; +assert.match(clientId, /^devspace-v1\./); + +const recovered = recoverClientRegistration(clientId, signingKey); +assert.ok(recovered); +assert.equal(recovered.client_id, clientId); +assert.equal(recovered.client_name, "ChatGPT"); +assert.deepEqual(recovered.redirect_uris, client.redirect_uris); +assert.deepEqual(recovered.grant_types, client.grant_types); + +const parts = clientId.split("."); +assert.equal(parts.length, 3); +assert.equal( + recoverClientRegistration(`${parts[0]}.${parts[1]}x.${parts[2]}`, signingKey), + undefined, +); +assert.equal( + recoverClientRegistration(`${parts[0]}.${parts[1]}.${parts[2]}x`, signingKey), + undefined, +); +assert.equal(recoverClientRegistration(clientId, `${signingKey}-wrong`), undefined); +assert.equal(recoverClientRegistration(`devspace-v1.${"x".repeat(5000)}.signature`, signingKey), undefined); +assert.equal( + recoverClientRegistration("devspace-0b3f9c1e-2d4a-4f77-9c0e-1a2b3c4d5e6f", signingKey), + undefined, +); +assert.equal(recoverClientRegistration(`${clientId}.extra`, signingKey), undefined); + +assert.deepEqual( + createRecoverableClientId( + { + ...client, + token_endpoint_auth_method: "client_secret_post", + client_secret: "must-not-be-embedded", + }, + signingKey, + ), + { kind: "unsupported" }, +); + +const oversized = createRecoverableClientId( + { ...client, client_name: "x".repeat(5000) }, + signingKey, +); +assert.equal(oversized.kind, "too_large"); diff --git a/src/oauth-client-registration.ts b/src/oauth-client-registration.ts new file mode 100644 index 00000000..6a3d6523 --- /dev/null +++ b/src/oauth-client-registration.ts @@ -0,0 +1,103 @@ +import { createHmac, timingSafeEqual } from "node:crypto"; +import { + OAuthClientInformationFullSchema, + type OAuthClientInformationFull, +} from "@modelcontextprotocol/sdk/shared/auth.js"; + +const CLIENT_ID_PREFIX = "devspace-v1"; +const MAX_CLIENT_ID_LENGTH = 4096; + +type ClientRegistrationPayload = Omit; + +export type RecoverableClientIdResult = + | { + kind: "recoverable"; + clientId: string; + registration: ClientRegistrationPayload; + } + | { kind: "unsupported" } + | { kind: "too_large"; length: number; maxLength: number }; + +export function createRecoverableClientId( + client: ClientRegistrationPayload, + signingKey: string, +): RecoverableClientIdResult { + const parsed = OAuthClientInformationFullSchema.safeParse({ + ...client, + client_id: "pending", + }); + if (!parsed.success || !isPublicClient(parsed.data)) { + return { kind: "unsupported" }; + } + + const { client_id: _clientId, ...validatedRegistration } = parsed.data; + const payload = Buffer.from(JSON.stringify(validatedRegistration)).toString("base64url"); + const signedValue = `${CLIENT_ID_PREFIX}.${payload}`; + const signature = sign(signedValue, signingKey); + const clientId = `${signedValue}.${signature}`; + if (clientId.length > MAX_CLIENT_ID_LENGTH) { + return { + kind: "too_large", + length: clientId.length, + maxLength: MAX_CLIENT_ID_LENGTH, + }; + } + + return { + kind: "recoverable", + clientId, + registration: validatedRegistration, + }; +} + +export function recoverClientRegistration( + clientId: string, + signingKey: string, +): OAuthClientInformationFull | undefined { + if (clientId.length > MAX_CLIENT_ID_LENGTH) return undefined; + + const [prefix, payload, signature, extra] = clientId.split("."); + if (prefix !== CLIENT_ID_PREFIX || !payload || !signature || extra !== undefined) { + return undefined; + } + + const signedValue = `${prefix}.${payload}`; + if (!safeEquals(signature, sign(signedValue, signingKey))) return undefined; + + let decoded: unknown; + try { + decoded = JSON.parse(Buffer.from(payload, "base64url").toString("utf8")) as unknown; + } catch { + return undefined; + } + + const parsed = OAuthClientInformationFullSchema.safeParse({ + ...(isRecord(decoded) ? decoded : {}), + client_id: clientId, + }); + if (!parsed.success || !isPublicClient(parsed.data)) return undefined; + return parsed.data; +} + +function isPublicClient(client: OAuthClientInformationFull): boolean { + return ( + client.token_endpoint_auth_method === "none" && + client.client_secret === undefined && + client.client_secret_expires_at === undefined + ); +} + +function sign(value: string, signingKey: string): string { + return createHmac("sha256", signingKey).update(value).digest("base64url"); +} + +function safeEquals(left: string, right: string): boolean { + const leftBuffer = Buffer.from(left); + const rightBuffer = Buffer.from(right); + if (leftBuffer.byteLength !== rightBuffer.byteLength) return false; + return timingSafeEqual(leftBuffer, rightBuffer); +} + +function isRecord(value: unknown): value is Record { + return typeof value === "object" && value !== null && !Array.isArray(value); +} diff --git a/src/oauth-provider.ts b/src/oauth-provider.ts index e6503788..77919ef4 100644 --- a/src/oauth-provider.ts +++ b/src/oauth-provider.ts @@ -14,6 +14,7 @@ import { SqliteOAuthClientsStore, SqliteOAuthStore } from "./oauth-store.js"; export interface OAuthConfig { ownerToken: string; + clientRegistrationKey: string; accessTokenTtlSeconds: number; refreshTokenTtlSeconds: number; scopes: string[]; @@ -124,7 +125,11 @@ export class SingleUserOAuthProvider implements OAuthServerProvider { ) { this.resourceServerUrl = resourceUrlFromServerUrl(resourceServerUrl); this.oauthStore = new SqliteOAuthStore(stateDir); - this.clientsStore = new SqliteOAuthClientsStore(this.oauthStore, config.allowedRedirectHosts); + this.clientsStore = new SqliteOAuthClientsStore( + this.oauthStore, + config.allowedRedirectHosts, + config.clientRegistrationKey, + ); } async authorize( @@ -167,6 +172,10 @@ export class SingleUserOAuthProvider implements OAuthServerProvider { return; } + if (!this.oauthStore.getClient(client.client_id)) { + this.oauthStore.restoreClient(client, this.config.allowedRedirectHosts); + } + const code = `code-${randomUUID()}`; this.codes.set(code, { clientId: client.client_id, diff --git a/src/oauth-store.test.ts b/src/oauth-store.test.ts index e47f8121..44514ec6 100644 --- a/src/oauth-store.test.ts +++ b/src/oauth-store.test.ts @@ -3,7 +3,12 @@ import { createHash } from "node:crypto"; import { mkdtemp, rm, stat } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; -import { InvalidGrantError, InvalidTokenError } from "@modelcontextprotocol/sdk/server/auth/errors.js"; +import type { Response } from "express"; +import { + InvalidGrantError, + InvalidRequestError, + InvalidTokenError, +} from "@modelcontextprotocol/sdk/server/auth/errors.js"; import { databasePath, openDatabase } from "./db/client.js"; import { SingleUserOAuthProvider } from "./oauth-provider.js"; import { SqliteOAuthClientsStore, SqliteOAuthStore } from "./oauth-store.js"; @@ -11,6 +16,7 @@ import { SqliteOAuthClientsStore, SqliteOAuthStore } from "./oauth-store.js"; const root = await mkdtemp(join(tmpdir(), "devspace-oauth-test-")); const oauthConfig = { ownerToken: "test-owner-token-that-is-long-enough", + clientRegistrationKey: "test-client-registration-key-that-is-long-enough", accessTokenTtlSeconds: 3600, refreshTokenTtlSeconds: 2592000, scopes: ["devspace"], @@ -24,6 +30,7 @@ try { testPersistenceAndTokenHashing(join(root, "persistence")); testExpiredTokenCleanup(join(root, "expiration")); testTransactionalTokenRotation(join(root, "rotation")); + await testClientRegistrationRecovery(join(root, "registration-recovery")); await testProviderRestartRotationAndRevocation(join(root, "provider")); } finally { await rm(root, { recursive: true, force: true }); @@ -60,7 +67,11 @@ function testPersistenceAndTokenHashing(stateDir: string): void { const accessToken = "access-token-example"; const refreshToken = "refresh-token-example"; const firstStore = new SqliteOAuthStore(stateDir); - const firstClients = new SqliteOAuthClientsStore(firstStore, oauthConfig.allowedRedirectHosts); + const firstClients = new SqliteOAuthClientsStore( + firstStore, + oauthConfig.allowedRedirectHosts, + oauthConfig.clientRegistrationKey, + ); const client = firstClients.registerClient({ redirect_uris: [redirectUri], client_name: "ChatGPT", @@ -115,9 +126,11 @@ function testPersistenceAndTokenHashing(stateDir: string): void { function testExpiredTokenCleanup(stateDir: string): void { const store = new SqliteOAuthStore(stateDir); - const client = new SqliteOAuthClientsStore(store, oauthConfig.allowedRedirectHosts).registerClient({ - redirect_uris: [redirectUri], - }); + const client = new SqliteOAuthClientsStore( + store, + oauthConfig.allowedRedirectHosts, + oauthConfig.clientRegistrationKey, + ).registerClient({ redirect_uris: [redirectUri] }); const expiredAt = Math.floor(Date.now() / 1000) - 1; store.saveTokenPair({ accessTokenHash: "expired-access-hash", @@ -139,9 +152,11 @@ function testExpiredTokenCleanup(stateDir: string): void { function testTransactionalTokenRotation(stateDir: string): void { const store = new SqliteOAuthStore(stateDir); try { - const client = new SqliteOAuthClientsStore(store, oauthConfig.allowedRedirectHosts).registerClient({ - redirect_uris: [redirectUri], - }); + const client = new SqliteOAuthClientsStore( + store, + oauthConfig.allowedRedirectHosts, + oauthConfig.clientRegistrationKey, + ).registerClient({ redirect_uris: [redirectUri] }); const expiresAt = Math.floor(Date.now() / 1000) + 3600; store.saveRefreshToken("old-refresh-hash", { clientId: client.client_id, @@ -184,6 +199,125 @@ function testTransactionalTokenRotation(stateDir: string): void { } } +async function testClientRegistrationRecovery(stateDir: string): Promise { + const registrationStateDir = join(stateDir, "registered"); + const emptyStateDir = join(stateDir, "empty"); + const firstProvider = new SingleUserOAuthProvider(oauthConfig, mcpUrl, registrationStateDir); + const client = await firstProvider.clientsStore.registerClient?.({ + redirect_uris: [redirectUri], + client_name: "ChatGPT", + }); + assert.ok(client); + assert.match(client.client_id, /^devspace-v1\./); + firstProvider.close(); + + const recoveredProvider = new SingleUserOAuthProvider(oauthConfig, mcpUrl, emptyStateDir); + try { + const recovered = await recoveredProvider.clientsStore.getClient(client.client_id); + assert.ok(recovered); + assert.equal(recovered.client_id, client.client_id); + + const beforeApproval = new SqliteOAuthStore(emptyStateDir); + assert.equal(beforeApproval.getClient(client.client_id), undefined); + beforeApproval.close(); + + const params = { + redirectUri, + codeChallenge: "challenge", + scopes: ["devspace"], + resource: mcpUrl, + }; + const rejectedResponse = authorizationResponse("wrong-owner-token"); + await recoveredProvider.authorize( + recovered, + params, + rejectedResponse, + ); + assert.equal(rejectedResponse.statusCode, 401); + + const afterRejectedApproval = new SqliteOAuthStore(emptyStateDir); + assert.equal(afterRejectedApproval.getClient(client.client_id), undefined); + afterRejectedApproval.close(); + + let redirectLocation: string | undefined; + await recoveredProvider.authorize( + recovered, + params, + authorizationResponse(oauthConfig.ownerToken, (location) => { + redirectLocation = location; + }), + ); + assert.ok(redirectLocation); + + const afterApproval = new SqliteOAuthStore(emptyStateDir); + assert.equal(afterApproval.getClient(client.client_id)?.client_name, "ChatGPT"); + afterApproval.close(); + + const policyStore = new SqliteOAuthStore(join(stateDir, "restore-policy")); + assert.throws( + () => policyStore.restoreClient(recovered, ["example.com"]), + InvalidRequestError, + ); + assert.equal(policyStore.getClient(client.client_id), undefined); + policyStore.close(); + } finally { + recoveredProvider.close(); + } + + const changedPolicyProvider = new SingleUserOAuthProvider( + { ...oauthConfig, allowedRedirectHosts: ["example.com"] }, + mcpUrl, + join(stateDir, "changed-policy"), + ); + try { + assert.equal(await changedPolicyProvider.clientsStore.getClient(client.client_id), undefined); + } finally { + changedPolicyProvider.close(); + } + + const oversizedProvider = new SingleUserOAuthProvider( + oauthConfig, + mcpUrl, + join(stateDir, "oversized"), + ); + try { + assert.throws( + () => oversizedProvider.clientsStore.registerClient?.({ + redirect_uris: [redirectUri], + client_name: "x".repeat(5000), + }), + InvalidRequestError, + ); + } finally { + oversizedProvider.close(); + } +} + +function authorizationResponse( + ownerToken: string, + onRedirect?: (location: string) => void, +): Response { + const response = { + req: { method: "POST", body: { owner_token: ownerToken } }, + statusCode: 200, + status(code: number) { + response.statusCode = code; + return response; + }, + setHeader() { + return response; + }, + send() { + return response; + }, + redirect(status: number, location: string) { + response.statusCode = status; + onRedirect?.(location); + }, + }; + return response as unknown as Response; +} + async function testProviderRestartRotationAndRevocation(stateDir: string): Promise { const firstProvider = new SingleUserOAuthProvider(oauthConfig, mcpUrl, stateDir); const client = await firstProvider.clientsStore.registerClient?.({ diff --git a/src/oauth-store.ts b/src/oauth-store.ts index 2567a40e..9e752a6f 100644 --- a/src/oauth-store.ts +++ b/src/oauth-store.ts @@ -3,6 +3,10 @@ import type { OAuthRegisteredClientsStore } from "@modelcontextprotocol/sdk/serv import { InvalidRequestError } from "@modelcontextprotocol/sdk/server/auth/errors.js"; import type { OAuthClientInformationFull } from "@modelcontextprotocol/sdk/shared/auth.js"; import { openDatabase, type DatabaseHandle } from "./db/client.js"; +import { + createRecoverableClientId, + recoverClientRegistration, +} from "./oauth-client-registration.js"; export interface PersistedAccessTokenRecord { clientId: string; @@ -56,28 +60,59 @@ export class SqliteOAuthStore { registerClient( client: Omit, allowedRedirectHosts: string[], + clientRegistrationKey: string, ): OAuthClientInformationFull { if (!client.redirect_uris.every((uri) => redirectHostAllowed(String(uri), allowedRedirectHosts))) { throw new InvalidRequestError("Client redirect_uri is not allowed for this DevSpace server"); } const now = Math.floor(Date.now() / 1000); - const registered: OAuthClientInformationFull = { + const registration = { ...client, - client_id: `devspace-${randomUUID()}`, client_id_issued_at: now, token_endpoint_auth_method: client.token_endpoint_auth_method ?? "none", grant_types: client.grant_types ?? ["authorization_code", "refresh_token"], response_types: client.response_types ?? ["code"], }; + const recoverable = createRecoverableClientId(registration, clientRegistrationKey); + if (recoverable.kind === "too_large") { + throw new InvalidRequestError( + `Client registration is too large for a recoverable client identifier (${recoverable.length} > ${recoverable.maxLength})`, + ); + } - this.database.sqlite - .prepare("insert into oauth_clients (client_id, client_json, issued_at) values (?, ?, ?)") - .run(registered.client_id, JSON.stringify(registered), now); + const registered: OAuthClientInformationFull = recoverable.kind === "recoverable" + ? { + ...recoverable.registration, + client_id: recoverable.clientId, + } + : { + ...registration, + client_id: `devspace-${randomUUID()}`, + }; + + this.saveClient(registered); return registered; } + restoreClient(client: OAuthClientInformationFull, allowedRedirectHosts: string[]): void { + if (!client.redirect_uris.every((uri) => redirectHostAllowed(String(uri), allowedRedirectHosts))) { + throw new InvalidRequestError("Client redirect_uri is not allowed for this DevSpace server"); + } + this.saveClient(client); + } + + private saveClient(client: OAuthClientInformationFull): void { + this.database.sqlite + .prepare( + `insert into oauth_clients (client_id, client_json, issued_at) + values (?, ?, ?) + on conflict(client_id) do nothing`, + ) + .run(client.client_id, JSON.stringify(client), client.client_id_issued_at ?? 0); + } + saveAccessToken(tokenHash: string, record: PersistedAccessTokenRecord): void { this.database.sqlite .prepare( @@ -191,16 +226,29 @@ export class SqliteOAuthClientsStore implements OAuthRegisteredClientsStore { constructor( private readonly store: SqliteOAuthStore, private readonly allowedRedirectHosts: string[], + private readonly clientRegistrationKey: string, ) {} getClient(clientId: string): OAuthClientInformationFull | undefined { - return this.store.getClient(clientId); + const stored = this.store.getClient(clientId); + if (stored) return stored; + + const recovered = recoverClientRegistration(clientId, this.clientRegistrationKey); + if (!recovered) return undefined; + if (!recovered.redirect_uris.every((uri) => redirectHostAllowed(String(uri), this.allowedRedirectHosts))) { + return undefined; + } + return recovered; } registerClient( client: Omit, ): OAuthClientInformationFull { - return this.store.registerClient(client, this.allowedRedirectHosts); + return this.store.registerClient( + client, + this.allowedRedirectHosts, + this.clientRegistrationKey, + ); } } diff --git a/src/user-config.ts b/src/user-config.ts index 5dd793ef..1837521e 100644 --- a/src/user-config.ts +++ b/src/user-config.ts @@ -1,4 +1,4 @@ -import { randomBytes } from "node:crypto"; +import { randomBytes, scryptSync } from "node:crypto"; import { existsSync, mkdirSync, @@ -25,6 +25,7 @@ export interface DevspaceUserConfig { export interface DevspaceAuthConfig { ownerToken?: string; + clientRegistrationKey?: string; } export interface DevspaceFiles { @@ -99,6 +100,21 @@ export function generateOwnerToken(): string { return randomBytes(32).toString("base64url"); } +export function generateClientRegistrationKey(): string { + return randomBytes(32).toString("base64url"); +} + +export function deriveClientRegistrationKey(ownerToken: string): string { + // Compatibility keys are bound to the Owner password. Rotating that password + // before persisting a separate key invalidates previously issued signed IDs. + return scryptSync( + ownerToken, + "devspace-oauth-client-registration-v1", + 32, + { N: 16_384, r: 8, p: 1 }, + ).toString("base64url"); +} + export function ensureDevspaceDefaultSkills(env: NodeJS.ProcessEnv = process.env): string[] { const targetPath = join(devspaceSkillsDir(env), "subagent-delegation", "SKILL.md"); if (existsSync(targetPath)) return [];