fix(mcp): reliable OAuth across runtimes and honest pre-restart UI
MCP authorization was broken in several stacked ways. The browser return leg landed on the SPA behind the auth gate, so the system browser saw a login page instead of finishing; the pending-context store silently saved nothing because its route had no JSON body parser; and the callback-URL config write started deferring behind Apply & Restart, so authorization ran against a runtime without the URL and dead-ended on OpenCode's loopback listener. The return leg is now completed entirely server-side by an unauthenticated GET /mcp/oauth/callback that only forwards a code whose state matches a parked context. Desktop with the local server and VS Code switch to OpenCode's native flow over its fixed loopback port — no config writes or restarts at all, with a one-time cleanup of the previously written callback URL — and its completion signal drives the page instead of blind status polling. Remote, hosted-web, and mobile keep the server-callback flow, applying a queued callback-URL write immediately since authorization cannot wait for a manual restart. Also: a server queued behind Apply & Restart now shows an Awaiting restart badge and explanation instead of connect/reauthorize buttons that can only fail, and Reauthorize is offered only while the server is actually connected.
This commit is contained in:
@@ -0,0 +1,111 @@
|
||||
import { afterEach, describe, expect, it, vi } from 'vitest';
|
||||
import express from 'express';
|
||||
import request from 'supertest';
|
||||
import { registerOpenCodeRoutes } from './routes.js';
|
||||
|
||||
// No global body parser on purpose: the real server parses JSON per-route, so
|
||||
// these tests must fail if the pending route loses its own parser again.
|
||||
const createApp = (overrides = {}) => {
|
||||
const app = express();
|
||||
const dependencies = {
|
||||
buildOpenCodeUrl: (path) => `http://opencode.local${path}`,
|
||||
getOpenCodeAuthHeaders: () => ({ 'x-opencode-auth': 'test' }),
|
||||
...overrides,
|
||||
};
|
||||
registerOpenCodeRoutes(app, dependencies);
|
||||
return { app, dependencies };
|
||||
};
|
||||
|
||||
const queuePending = (app, { state, name, directory = null, origin = null }) =>
|
||||
request(app)
|
||||
.post('/api/mcp/auth/pending')
|
||||
.send({ state, name, directory, origin })
|
||||
.expect(200);
|
||||
|
||||
afterEach(() => {
|
||||
vi.unstubAllGlobals();
|
||||
});
|
||||
|
||||
describe('MCP OAuth browser callback route', () => {
|
||||
it('completes authorization server-side for a parked state and clears it', async () => {
|
||||
const upstreamFetch = vi.fn(async () => new Response(JSON.stringify({ success: true }), { status: 200 }));
|
||||
vi.stubGlobal('fetch', upstreamFetch);
|
||||
const { app } = createApp();
|
||||
await queuePending(app, { state: 'state-1', name: 'linear', directory: '/projects/demo', origin: 'desktop' });
|
||||
|
||||
const response = await request(app)
|
||||
.get('/mcp/oauth/callback')
|
||||
.query({ state: 'state-1', code: 'auth-code', server: 'linear' })
|
||||
.expect(200);
|
||||
|
||||
expect(upstreamFetch).toHaveBeenCalledTimes(1);
|
||||
const [url, init] = upstreamFetch.mock.calls[0];
|
||||
expect(String(url)).toBe('http://opencode.local/mcp/linear/auth/callback?directory=%2Fprojects%2Fdemo');
|
||||
expect(init.method).toBe('POST');
|
||||
expect(JSON.parse(init.body)).toEqual({ code: 'auth-code' });
|
||||
expect(init.headers['x-opencode-auth']).toBe('test');
|
||||
|
||||
expect(response.text).toContain('Authorization Complete');
|
||||
// Started from the desktop shell: the page hands control back via deep link.
|
||||
expect(response.text).toContain('openchamber://focus/mcp-auth');
|
||||
|
||||
await request(app).get('/api/mcp/auth/pending').query({ state: 'state-1' }).expect(404);
|
||||
});
|
||||
|
||||
it('never forwards a code whose state is unknown', async () => {
|
||||
const upstreamFetch = vi.fn();
|
||||
vi.stubGlobal('fetch', upstreamFetch);
|
||||
const { app } = createApp();
|
||||
|
||||
const response = await request(app)
|
||||
.get('/mcp/oauth/callback')
|
||||
.query({ state: 'forged', code: 'attacker-code', server: 'linear' })
|
||||
.expect(400);
|
||||
|
||||
expect(upstreamFetch).not.toHaveBeenCalled();
|
||||
expect(response.text).toContain('Authorization Failed');
|
||||
});
|
||||
|
||||
it('omits the desktop deep link for flows started outside the desktop shell', async () => {
|
||||
vi.stubGlobal('fetch', vi.fn(async () => new Response('{}', { status: 200 })));
|
||||
const { app } = createApp();
|
||||
await queuePending(app, { state: 'state-web', name: 'linear' });
|
||||
|
||||
const response = await request(app)
|
||||
.get('/mcp/oauth/callback')
|
||||
.query({ state: 'state-web', code: 'auth-code' })
|
||||
.expect(200);
|
||||
|
||||
expect(response.text).not.toContain('openchamber://');
|
||||
});
|
||||
|
||||
it('reports a provider error without contacting OpenCode', async () => {
|
||||
const upstreamFetch = vi.fn();
|
||||
vi.stubGlobal('fetch', upstreamFetch);
|
||||
const { app } = createApp();
|
||||
await queuePending(app, { state: 'state-2', name: 'linear' });
|
||||
|
||||
const response = await request(app)
|
||||
.get('/mcp/oauth/callback')
|
||||
.query({ state: 'state-2', error: 'access_denied', error_description: 'User <denied> access' })
|
||||
.expect(400);
|
||||
|
||||
expect(upstreamFetch).not.toHaveBeenCalled();
|
||||
// Interpolated provider text is escaped, not rendered as markup.
|
||||
expect(response.text).toContain('User <denied> access');
|
||||
await request(app).get('/api/mcp/auth/pending').query({ state: 'state-2' }).expect(404);
|
||||
});
|
||||
|
||||
it('surfaces an OpenCode rejection as a failed page', async () => {
|
||||
vi.stubGlobal('fetch', vi.fn(async () => new Response(JSON.stringify({ error: 'invalid code' }), { status: 400 })));
|
||||
const { app } = createApp();
|
||||
await queuePending(app, { state: 'state-3', name: 'linear' });
|
||||
|
||||
const response = await request(app)
|
||||
.get('/mcp/oauth/callback')
|
||||
.query({ state: 'state-3', code: 'stale-code' })
|
||||
.expect(502);
|
||||
|
||||
expect(response.text).toContain('invalid code');
|
||||
});
|
||||
});
|
||||
@@ -839,5 +839,9 @@ export const registerOpenCodeProxy = (app, deps) => {
|
||||
|
||||
app.use('/api', applyProxyResponseDeadline);
|
||||
app.post('/api/provider/:providerID/oauth/callback', interactiveOAuthProxy);
|
||||
// OpenCode's native MCP OAuth flow: the request blocks until the user
|
||||
// finishes authorization in the browser (up to OpenCode's 5-minute callback
|
||||
// timeout), so it needs the interactive-OAuth deadline, not the default one.
|
||||
app.post('/api/mcp/:name/auth/authenticate', interactiveOAuthProxy);
|
||||
app.use('/api', apiProxy);
|
||||
};
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import express from 'express';
|
||||
import { createProjectIdFromPath } from '../projects/project-id.js';
|
||||
import fs from 'fs';
|
||||
import os from 'os';
|
||||
@@ -46,6 +47,46 @@ export const registerOpenCodeRoutes = (app, dependencies) => {
|
||||
return trimmed || null;
|
||||
};
|
||||
|
||||
const escapeHtml = (value) => String(value)
|
||||
.replace(/&/g, '&')
|
||||
.replace(/</g, '<')
|
||||
.replace(/>/g, '>')
|
||||
.replace(/"/g, '"')
|
||||
.replace(/'/g, ''');
|
||||
|
||||
// Self-contained page for the OAuth return leg: the system browser has no UI
|
||||
// session, so it cannot load the SPA behind the auth gate — everything it
|
||||
// needs ships inline. `openchamber://focus/mcp-auth` raises the desktop app;
|
||||
// the link stays visible because some browsers only follow custom-protocol
|
||||
// URLs from a user gesture.
|
||||
const renderMcpOAuthCallbackPage = ({ title, message, desktopReturn }) => `<!doctype html>
|
||||
<html lang="en">
|
||||
<head>
|
||||
<meta charset="utf-8">
|
||||
<meta name="viewport" content="width=device-width, initial-scale=1">
|
||||
<title>${escapeHtml(title)} — OpenChamber</title>
|
||||
<style>
|
||||
:root { color-scheme: light dark; }
|
||||
body { margin: 0; min-height: 100vh; display: flex; align-items: center; justify-content: center;
|
||||
font-family: -apple-system, BlinkMacSystemFont, 'Segoe UI', sans-serif;
|
||||
background: Canvas; color: CanvasText; }
|
||||
main { max-width: 34rem; padding: 2.5rem 2rem; text-align: center; }
|
||||
h1 { font-size: 1.25rem; margin: 0 0 0.75rem; }
|
||||
p { margin: 0; line-height: 1.5; opacity: 0.85; }
|
||||
a.return { display: inline-block; margin-top: 1.5rem; padding: 0.5rem 1.25rem; border-radius: 0.5rem;
|
||||
border: 1px solid color-mix(in srgb, CanvasText 25%, transparent); color: inherit; text-decoration: none; }
|
||||
</style>
|
||||
</head>
|
||||
<body>
|
||||
<main>
|
||||
<h1>${escapeHtml(title)}</h1>
|
||||
<p>${escapeHtml(message)}</p>
|
||||
${desktopReturn ? `<a class="return" href="openchamber://focus/mcp-auth">Return to OpenChamber</a>
|
||||
<script>window.location.href = 'openchamber://focus/mcp-auth';</script>` : ''}
|
||||
</main>
|
||||
</body>
|
||||
</html>`;
|
||||
|
||||
const readOpenCodeCurrentVersion = async () => {
|
||||
const healthResponse = await fetch(buildOpenCodeUrl('/global/health', ''), {
|
||||
method: 'GET',
|
||||
@@ -341,7 +382,10 @@ export const registerOpenCodeRoutes = (app, dependencies) => {
|
||||
}
|
||||
});
|
||||
|
||||
app.post('/api/mcp/auth/pending', async (req, res) => {
|
||||
// The body parser is per-route on this server; without it req.body is
|
||||
// undefined here, the state read as absent, and the "parked" context was
|
||||
// silently never stored — the callback then always failed as unknown.
|
||||
app.post('/api/mcp/auth/pending', express.json({ limit: '16kb' }), async (req, res) => {
|
||||
try {
|
||||
pruneExpiredPendingMcpAuthContexts();
|
||||
|
||||
@@ -417,6 +461,89 @@ export const registerOpenCodeRoutes = (app, dependencies) => {
|
||||
}
|
||||
});
|
||||
|
||||
// Browser return leg of the MCP OAuth flow, completed entirely server-side.
|
||||
//
|
||||
// The provider redirects the SYSTEM browser here, and that browser has no
|
||||
// OpenChamber UI session — the SPA route this path used to land on sits
|
||||
// behind the client-side auth gate, so the user saw a login page instead of
|
||||
// a finished authorization. No session can be required on this path.
|
||||
//
|
||||
// Safe without auth because it acts only on a code+state pair whose `state`
|
||||
// matches a context parked by an authenticated start call: `state` is the
|
||||
// OAuth CSRF secret, generated per flow and known only to the initiating
|
||||
// client and the provider. Without a match the code is NOT forwarded, so an
|
||||
// unauthenticated caller cannot bind this server's MCP entry to a foreign
|
||||
// account by fabricating a callback. The endpoint reads nothing and mutates
|
||||
// nothing else.
|
||||
app.get('/mcp/oauth/callback', async (req, res) => {
|
||||
const queryValue = (key) => normalizePendingString(Array.isArray(req.query?.[key]) ? req.query[key][0] : req.query?.[key]);
|
||||
const state = queryValue('state');
|
||||
const code = queryValue('code');
|
||||
const providerError = queryValue('error');
|
||||
const providerErrorDescription = queryValue('error_description');
|
||||
|
||||
pruneExpiredPendingMcpAuthContexts();
|
||||
const context = state ? pendingMcpAuthContextByState.get(state) ?? null : null;
|
||||
const startedFromDesktop = context?.origin === 'desktop';
|
||||
|
||||
const finish = (status, { title, message }) => {
|
||||
if (state) pendingMcpAuthContextByState.delete(state);
|
||||
res.status(status).type('html').send(renderMcpOAuthCallbackPage({
|
||||
title,
|
||||
message,
|
||||
// Browsers only follow custom-protocol links from a user gesture in
|
||||
// some configurations, so the page both tries the jump and keeps a
|
||||
// visible link as the fallback.
|
||||
desktopReturn: startedFromDesktop,
|
||||
}));
|
||||
};
|
||||
|
||||
if (providerError) {
|
||||
return finish(400, {
|
||||
title: 'Authorization Failed',
|
||||
message: providerErrorDescription || providerError,
|
||||
});
|
||||
}
|
||||
if (!code) {
|
||||
return finish(400, {
|
||||
title: 'Authorization Failed',
|
||||
message: 'The provider did not return an authorization code. Start authorization again from MCP Settings.',
|
||||
});
|
||||
}
|
||||
if (!context?.name) {
|
||||
return finish(400, {
|
||||
title: 'Authorization Failed',
|
||||
message: 'This authorization session has expired or is unknown to the running app. Return to OpenChamber and click Authorize again.',
|
||||
});
|
||||
}
|
||||
|
||||
try {
|
||||
const callbackUrl = new URL(buildOpenCodeUrl(`/mcp/${encodeURIComponent(context.name)}/auth/callback`, ''));
|
||||
if (context.directory) callbackUrl.searchParams.set('directory', context.directory);
|
||||
const upstream = await fetch(callbackUrl, {
|
||||
method: 'POST',
|
||||
headers: { 'Content-Type': 'application/json', Accept: 'application/json', ...getOpenCodeAuthHeaders() },
|
||||
body: JSON.stringify({ code }),
|
||||
});
|
||||
if (!upstream.ok) {
|
||||
const payload = await upstream.json().catch(() => null);
|
||||
return finish(502, {
|
||||
title: 'Authorization Failed',
|
||||
message: payload?.error || payload?.message || `OpenCode rejected the authorization code (${upstream.status}). Start authorization again from MCP Settings.`,
|
||||
});
|
||||
}
|
||||
return finish(200, {
|
||||
title: 'Authorization Complete',
|
||||
message: 'You can close this tab and return to OpenChamber.',
|
||||
});
|
||||
} catch (error) {
|
||||
return finish(502, {
|
||||
title: 'Authorization Failed',
|
||||
message: error?.message || 'Failed to complete MCP authorization.',
|
||||
});
|
||||
}
|
||||
});
|
||||
|
||||
app.get('/api/provider/:providerId/source', async (req, res) => {
|
||||
try {
|
||||
const { providerId } = req.params;
|
||||
|
||||
Reference in New Issue
Block a user