diff --git a/CHANGELOG.md b/CHANGELOG.md index 0a63477..fa3bae1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,7 @@ This project follows the spirit of Keep a Changelog and uses semantic versioning - Internal refactor: `src/cli/start.ts` split into focused modules (`connector-loader`, `interactive`, `tunnel-controller`). Per-connector setup extracted into `src/connectors/{filesystem,obsidian,mempalace}-setup.ts`. Shared `saveConfig()` consolidated on `src/config/loader.ts`. - `PluginSchema` is now a single-variant `z.discriminatedUnion('name', ...)` so adding a second plugin is a schema addition rather than a structural refactor. +- OAuth `/authorize` now defaults a missing `resource` parameter to the instance's canonical `/mcp` resource for client compatibility, while preserving token audience binding and still rejecting explicit wrong resources. ### Deprecated diff --git a/src/server/index.ts b/src/server/index.ts index 7324fc6..c47b68a 100644 --- a/src/server/index.ts +++ b/src/server/index.ts @@ -367,12 +367,14 @@ export async function startHttpServer(router: ToolRouter, options: HttpServerOpt }); app.get('/authorize', authLimiter, oauthOriginMiddleware, (req, res) => { - const params = parseAuthorizeParams(req.query); - if ('error' in params) { - logHttpRequest(requestLog, req, 400, 'oauth.authorize', params.error); - res.status(400).type('text/plain').send(params.error); + const parsed = parseAuthorizeParams(req.query); + if ('error' in parsed) { + logHttpRequest(requestLog, req, 400, 'oauth.authorize', parsed.error); + res.status(400).type('text/plain').send(parsed.error); return; } + const canonicalResource = `${baseUrlFor(req)}/mcp`; + const { params, resourceDefaulted } = defaultAuthorizeResource(parsed, canonicalResource); if (!oauth.isRedirectUriAllowed(params.clientId, params.redirectUri)) { const rejectedHost = safeHost(params.redirectUri); logHttpRequest( @@ -382,7 +384,7 @@ export async function startHttpServer(router: ToolRouter, options: HttpServerOpt res.status(400).type('text/plain').send('redirect_uri is not registered for this client'); return; } - const resourceError = validateAuthorizeResource(params.resource, `${baseUrlFor(req)}/mcp`); + const resourceError = validateAuthorizeResource(params.resource, canonicalResource); if (resourceError) { const redirect = authorizeErrorRedirect(params, 'invalid_target', resourceError.description); logHttpRequest(requestLog, req, 302, 'oauth.authorize', resourceError.code, params.clientId); @@ -395,6 +397,7 @@ export async function startHttpServer(router: ToolRouter, options: HttpServerOpt requestId, redirectUri: params.redirectUri, resource: params.resource, + resourceDefaulted, state: params.state, }); logHttpRequest(requestLog, req, 200, 'oauth.authorize', promptDetail, params.clientId); @@ -402,12 +405,14 @@ export async function startHttpServer(router: ToolRouter, options: HttpServerOpt }); app.post('/authorize', authLimiter, oauthOriginMiddleware, (req, res) => { - const params = parseAuthorizeParams(req.body ?? {}); - if ('error' in params) { - logHttpRequest(requestLog, req, 400, 'oauth.authorize', params.error); - res.status(400).type('text/plain').send(params.error); + const parsed = parseAuthorizeParams(req.body ?? {}); + if ('error' in parsed) { + logHttpRequest(requestLog, req, 400, 'oauth.authorize', parsed.error); + res.status(400).type('text/plain').send(parsed.error); return; } + const canonicalResource = `${baseUrlFor(req)}/mcp`; + const { params, resourceDefaulted } = defaultAuthorizeResource(parsed, canonicalResource); if (!oauth.isRedirectUriAllowed(params.clientId, params.redirectUri)) { const rejectedHost = safeHost(params.redirectUri); logHttpRequest( @@ -417,7 +422,7 @@ export async function startHttpServer(router: ToolRouter, options: HttpServerOpt res.status(400).type('text/plain').send('redirect_uri is not registered for this client'); return; } - const resourceError = validateAuthorizeResource(params.resource, `${baseUrlFor(req)}/mcp`); + const resourceError = validateAuthorizeResource(params.resource, canonicalResource); if (resourceError) { const redirect = authorizeErrorRedirect(params, 'invalid_target', resourceError.description); logHttpRequest(requestLog, req, 302, 'oauth.authorize', resourceError.code, params.clientId); @@ -433,6 +438,7 @@ export async function startHttpServer(router: ToolRouter, options: HttpServerOpt requestId, redirectUri: params.redirectUri, resource: params.resource, + resourceDefaulted, state: params.state, }); logHttpRequest(requestLog, req, 401, 'oauth.authorize', denyDetail, params.clientId); @@ -460,6 +466,7 @@ export async function startHttpServer(router: ToolRouter, options: HttpServerOpt requestId, redirectUri: params.redirectUri, resource: params.resource, + resourceDefaulted, state: params.state, }); logHttpRequest(requestLog, req, 302, 'oauth.authorize', approveDetail, params.clientId); @@ -784,6 +791,17 @@ function parseAuthorizeParams(source: Record): AuthorizeParams }; } +function defaultAuthorizeResource( + params: AuthorizeParams, + canonicalResource: string, +): { params: AuthorizeParams; resourceDefaulted: boolean } { + if (params.resource) return { params, resourceDefaulted: false }; + return { + params: { ...params, resource: canonicalResource }, + resourceDefaulted: true, + }; +} + function stringField(value: unknown): string | undefined { if (typeof value !== 'string') return undefined; const trimmed = value.trim(); @@ -850,9 +868,10 @@ function formatAuthorizeLogDetail(input: { requestId: string; redirectUri: string; resource?: string; + resourceDefaulted?: boolean; state?: string; }): string { - return `${input.phase} rid=${input.requestId} redirect_host=${safeHost(input.redirectUri)} resource_host=${input.resource ? safeHost(input.resource) : '(none)'} state_hash=${hashForLog(input.state)}`; + return `${input.phase} rid=${input.requestId} redirect_host=${safeHost(input.redirectUri)} resource_host=${input.resource ? safeHost(input.resource) : '(none)'} resource_defaulted=${input.resourceDefaulted ? 'true' : 'false'} state_hash=${hashForLog(input.state)}`; } function hashForLog(value: string | undefined): string { diff --git a/tests/server.test.ts b/tests/server.test.ts index 1ea9e2e..500eb5c 100644 --- a/tests/server.test.ts +++ b/tests/server.test.ts @@ -562,48 +562,164 @@ describe('startHttpServer lifecycle', () => { } }); - it('redirects /authorize resource errors to the registered redirect_uri', async () => { + it('defaults a missing GET /authorize resource to the canonical MCP resource', async () => { const router = new ToolRouter([new EmptyConnector()]); await router.initialize(); const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'mvmt-server-test-')); const tokenPath = path.join(tmp, '.mvmt', '.session-token'); + const signingKeyPath = path.join(tmp, '.mvmt', '.signing-key'); const requestLogs: Array<{ kind: string; status: number; detail?: string; clientId?: string }> = []; const server = await startHttpServer(router, { port: 0, tokenPath, + signingKeyPath, requestLog: (entry) => requestLogs.push(entry), }); try { const sessionToken = fs.readFileSync(tokenPath, 'utf-8').trim(); const redirectUri = 'https://claude.ai/api/mcp/auth_callback'; - const { challenge } = s256Pair(); + const { verifier, challenge } = s256Pair(); + const resource = `http://127.0.0.1:${server.port}/mcp`; await registerClient(server.port, 'claude', [redirectUri], sessionToken); - const authorize = (resource?: string) => - fetch(`http://127.0.0.1:${server.port}/authorize`, { - method: 'POST', - redirect: 'manual', - headers: { 'Content-Type': 'application/x-www-form-urlencoded' }, - body: new URLSearchParams({ - response_type: 'code', - client_id: 'claude', - redirect_uri: redirectUri, - ...(resource ? { resource } : {}), - state: 'resource-state', - code_challenge: challenge, - code_challenge_method: 'S256', - session_token: sessionToken, + const promptUrl = new URL(`http://127.0.0.1:${server.port}/authorize`); + promptUrl.search = new URLSearchParams({ + response_type: 'code', + client_id: 'claude', + redirect_uri: redirectUri, + state: 'resource-state', + code_challenge: challenge, + code_challenge_method: 'S256', + }).toString(); + const prompt = await fetch(promptUrl); + expect(prompt.status).toBe(200); + const promptBody = await prompt.text(); + expect(promptBody).toContain(`name="resource" value="${resource}"`); + + const approve = await fetch(`http://127.0.0.1:${server.port}/authorize`, { + method: 'POST', + redirect: 'manual', + headers: { 'Content-Type': 'application/x-www-form-urlencoded' }, + body: new URLSearchParams({ + response_type: 'code', + client_id: 'claude', + redirect_uri: redirectUri, + resource, + state: 'resource-state', + code_challenge: challenge, + code_challenge_method: 'S256', + session_token: sessionToken, + }), + }); + expect(approve.status).toBe(302); + const approveRedirect = new URL(approve.headers.get('location')!); + const code = approveRedirect.searchParams.get('code'); + expect(code).toBeTruthy(); + + const accessToken = await exchangeAuthorizationCodeForToken({ + port: server.port, + clientId: 'claude', + redirectUri, + code: code!, + verifier, + }); + expectAccessTokenAudience(accessToken, signingKeyPath, resource); + expect(requestLogs).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + kind: 'oauth.authorize', + status: 200, + detail: expect.stringContaining('resource_defaulted=true'), }), - }); + ]), + ); + } finally { + await server.close(); + fs.rmSync(tmp, { recursive: true, force: true }); + } + }); + + it('defaults a missing POST /authorize resource to the canonical MCP resource', async () => { + const router = new ToolRouter([new EmptyConnector()]); + await router.initialize(); + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'mvmt-server-test-')); + const tokenPath = path.join(tmp, '.mvmt', '.session-token'); + const signingKeyPath = path.join(tmp, '.mvmt', '.signing-key'); + const requestLogs: Array<{ kind: string; status: number; detail?: string; clientId?: string }> = []; + const server = await startHttpServer(router, { + port: 0, + tokenPath, + signingKeyPath, + requestLog: (entry) => requestLogs.push(entry), + }); + + try { + const sessionToken = fs.readFileSync(tokenPath, 'utf-8').trim(); + const redirectUri = 'https://claude.ai/api/mcp/auth_callback'; + const { verifier, challenge } = s256Pair(); + const resource = `http://127.0.0.1:${server.port}/mcp`; + await registerClient(server.port, 'claude', [redirectUri], sessionToken); + + const authorize = await fetch(`http://127.0.0.1:${server.port}/authorize`, { + method: 'POST', + redirect: 'manual', + headers: { 'Content-Type': 'application/x-www-form-urlencoded' }, + body: new URLSearchParams({ + response_type: 'code', + client_id: 'claude', + redirect_uri: redirectUri, + state: 'resource-state', + code_challenge: challenge, + code_challenge_method: 'S256', + session_token: sessionToken, + }), + }); + expect(authorize.status).toBe(302); + const authorizeRedirect = new URL(authorize.headers.get('location')!); + const code = authorizeRedirect.searchParams.get('code'); + expect(code).toBeTruthy(); + + const accessToken = await exchangeAuthorizationCodeForToken({ + port: server.port, + clientId: 'claude', + redirectUri, + code: code!, + verifier, + }); + expectAccessTokenAudience(accessToken, signingKeyPath, resource); + expect(requestLogs).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + kind: 'oauth.authorize', + status: 302, + detail: expect.stringContaining('resource_defaulted=true'), + }), + ]), + ); + } finally { + await server.close(); + fs.rmSync(tmp, { recursive: true, force: true }); + } + }); + + it('redirects explicit /authorize resource mismatches to the registered redirect_uri', async () => { + const router = new ToolRouter([new EmptyConnector()]); + await router.initialize(); + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'mvmt-server-test-')); + const tokenPath = path.join(tmp, '.mvmt', '.session-token'); + const requestLogs: Array<{ kind: string; status: number; detail?: string; clientId?: string }> = []; + const server = await startHttpServer(router, { + port: 0, + tokenPath, + requestLog: (entry) => requestLogs.push(entry), + }); - const missing = await authorize(); - expect(missing.status).toBe(302); - const missingRedirect = new URL(missing.headers.get('location')!); - expect(`${missingRedirect.origin}${missingRedirect.pathname}`).toBe(redirectUri); - expect(missingRedirect.searchParams.get('error')).toBe('invalid_target'); - expect(missingRedirect.searchParams.get('error_description')).toBe('Missing resource'); - expect(missingRedirect.searchParams.get('state')).toBe('resource-state'); + try { + const sessionToken = fs.readFileSync(tokenPath, 'utf-8').trim(); + const redirectUri = 'https://claude.ai/api/mcp/auth_callback'; + const { challenge } = s256Pair(); + await registerClient(server.port, 'claude', [redirectUri], sessionToken); const wrongUrl = new URL(`http://127.0.0.1:${server.port}/authorize`); wrongUrl.search = new URLSearchParams({ @@ -624,7 +740,6 @@ describe('startHttpServer lifecycle', () => { expect(wrongRedirect.searchParams.get('state')).toBe('get-state'); expect(requestLogs).toEqual( expect.arrayContaining([ - expect.objectContaining({ kind: 'oauth.authorize', status: 302, detail: 'missing_resource' }), expect.objectContaining({ kind: 'oauth.authorize', status: 302, detail: 'invalid_resource' }), ]), ); @@ -1062,6 +1177,110 @@ describe('startHttpServer lifecycle', () => { } }); + it('rejects unknown OAuth clients with a quarantine error once clients[] is configured', async () => { + const router = new ToolRouter([new EmptyConnector()]); + await router.initialize(); + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'mvmt-server-test-')); + const tokenPath = path.join(tmp, '.mvmt', '.session-token'); + const signingKeyPath = path.join(tmp, '.mvmt', '.signing-key'); + const requestLogs: Array<{ kind: string; status: number; detail?: string; clientId?: string }> = []; + const server = await startHttpServer(router, { + port: 0, + tokenPath, + signingKeyPath, + clients: [ + { + id: 'codex', + name: 'Codex CLI', + auth: { type: 'token', tokenHash: sha256Hex('codex-local-token') }, + rawToolsEnabled: true, + permissions: [], + }, + ], + requestLog: (entry) => requestLogs.push(entry), + }); + + try { + const oauthStore = new OAuthStore({ signingKey: ensureSigningKey(signingKeyPath) }); + const accessToken = oauthStore.issueAccessToken({ + clientId: 'unknown-dcr-client', + audience: `http://127.0.0.1:${server.port}/mcp`, + }).token; + const response = await fetch(`http://127.0.0.1:${server.port}/health`, { + headers: { Authorization: `Bearer ${accessToken}` }, + }); + expect(response.status).toBe(403); + expect(await response.json()).toEqual({ + error: 'oauth_client_quarantined', + error_description: 'OAuth client_id is not mapped to a configured mvmt client; admin must approve', + }); + expect(requestLogs).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + kind: 'health.auth', + status: 403, + clientId: 'quarantine:unknown-dcr-client', + detail: 'quarantined oauth_client_id=unknown-dcr-client', + }), + ]), + ); + } finally { + await server.close(); + fs.rmSync(tmp, { recursive: true, force: true }); + } + }); + + it('rejects the session token on /mcp once clients[] is configured', async () => { + const router = new ToolRouter([new EmptyConnector()]); + await router.initialize(); + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'mvmt-server-test-')); + const tokenPath = path.join(tmp, '.mvmt', '.session-token'); + const server = await startHttpServer(router, { + port: 0, + tokenPath, + clients: [ + { + id: 'codex', + name: 'Codex CLI', + auth: { type: 'token', tokenHash: sha256Hex('codex-local-token') }, + rawToolsEnabled: true, + permissions: [], + }, + ], + }); + + try { + const sessionToken = fs.readFileSync(tokenPath, 'utf-8').trim(); + const response = await fetch(`http://127.0.0.1:${server.port}/mcp`, { + method: 'GET', + headers: { Authorization: `Bearer ${sessionToken}` }, + }); + expect(response.status).toBe(401); + } finally { + await server.close(); + fs.rmSync(tmp, { recursive: true, force: true }); + } + }); + + it('preserves legacy session-token access when clients[] is absent', async () => { + const router = new ToolRouter([new EmptyConnector()]); + await router.initialize(); + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'mvmt-server-test-')); + const tokenPath = path.join(tmp, '.mvmt', '.session-token'); + const server = await startHttpServer(router, { port: 0, tokenPath }); + + try { + const sessionToken = fs.readFileSync(tokenPath, 'utf-8').trim(); + const response = await fetch(`http://127.0.0.1:${server.port}/health`, { + headers: { Authorization: `Bearer ${sessionToken}` }, + }); + expect(response.status).toBe(200); + } finally { + await server.close(); + fs.rmSync(tmp, { recursive: true, force: true }); + } + }); + it('revokes outstanding OAuth access tokens the moment the signing key file is rewritten', async () => { const router = new ToolRouter([new EmptyConnector()]); await router.initialize(); @@ -1260,3 +1479,42 @@ async function exchangeAccessToken(port: number, sessionToken: string, resourceB expect(body.access_token).toBeTypeOf('string'); return body.access_token as string; } + +async function exchangeAuthorizationCodeForToken(input: { + port: number; + clientId: string; + redirectUri: string; + code: string; + verifier: string; + resource?: string; +}): Promise { + const token = await fetch(`http://127.0.0.1:${input.port}/token`, { + method: 'POST', + headers: { 'Content-Type': 'application/x-www-form-urlencoded' }, + body: new URLSearchParams({ + grant_type: 'authorization_code', + code: input.code, + client_id: input.clientId, + redirect_uri: input.redirectUri, + ...(input.resource ? { resource: input.resource } : {}), + code_verifier: input.verifier, + }), + }); + const body = await token.json(); + expect(token.status).toBe(200); + expect(body.access_token).toBeTypeOf('string'); + return body.access_token as string; +} + +function expectAccessTokenAudience(accessToken: string, signingKeyPath: string, expectedAudience: string): void { + const validator = new OAuthStore({ signingKey: ensureSigningKey(signingKeyPath) }); + const validated = validator.validateAccessToken(`Bearer ${accessToken}`, { + expectedAudience, + allowLegacyNoAudience: false, + }); + expect(validated?.audience).toBe(expectedAudience); +} + +function sha256Hex(value: string): string { + return createHash('sha256').update(value, 'utf8').digest('hex'); +}