MCP sign-in: the popup's Allow survives a second Approve load; Deny is final; refresh hardening

CHANGELOG.md

@@ -8,6 +8,17 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 
 ## [Unreleased]
 
+### Fixed — approving in the sign-in popup can't be broken by the email tab (mcp)
+
+The hosted MCP's sign-in popup and the email-link tab share one cookie, and every load of the Approve page used to mint a new CSRF token into it. If the link tab's fallback link (or a reload) loaded the page again, the Allow button already showing in the popup failed with "csrf validation failed". The only Allow that still worked was the one in the email tab, which returns to ChatGPT without its pending state, so the connection never finished.
+
+- **One token per sign-in.** The Approve page reuses the CSRF token the request's cookie already carries, so every window of the browser agrees on it.
+- **Deny is final.** Denying consumes the request, so an Allow button still open in another window can't connect afterwards.
+- **The fallback says what it costs.** The email tab's link now reads "Approve in this tab instead" and warns that the app may not finish connecting from there.
+- **Token responses are never cached.** `/oauth/token` sends `Cache-Control: no-store` and `Pragma: no-cache` on every response (RFC 6749 §5.1).
+- **Failed refreshes are logged, and a wrong client doesn't burn the token.** A refused refresh logs its status and a client-id prefix, so a connection that drops after its 1-hour token shows in `wrangler tail`. A refresh sent with another client's id is refused before the token is used up, so it no longer turns the owner's next refresh into a false theft alarm that revokes the connection.
+- **`ping` answers.** The server returns an empty result for MCP's `ping`, instead of "method not found".
+
 ### Fixed — the served API spec offers only the networks a buyer can pay on (api)
 
 Circle's marketplace reviewers and buying agents read `/openapi.json`. Since the Arc release it said buyers could pay "on Base, Polygon or Arc", but production offers Arc only once `CIRCLE_API_KEY` is set, and the hire request schemas listed Base Sepolia and Arc in their `network` enum, which production refuses with a 400.

MCP_CONNECTOR_SPEC.md

@@ -72,21 +72,22 @@ The request is persisted for 10 minutes. A signed, httpOnly, `SameSite=Lax` cook
 
 1. **Email form**, `POST /oauth/email`. The CSRF token must match the cookie. The send path is throttled to 10 requests per IP per hour, before any account lookup. The page always says "if an account exists…", so it can't be used to enumerate accounts. A one-time link (15-minute login challenge) is mailed only when an owner account exists and that owner is under 5 sends per hour. The mail goes out after the response (`waitUntil`), so its round-trip is not a timing signal.
 2. **No account yet.** The sign-in only works for existing BasedAgents owner accounts. The authorize and "check your email" pages link to `https://app.basedagents.ai/start` (one email field) to create one first.
-3. **Landing**, `GET /oauth/continue?lt=…&req=…`. The login challenge is consumed atomically and is single-use. It must belong to `req`. **The same-browser binding is mandatory:** the click must carry the `mcp_authreq` cookie for the same `req`. A link opened on another device or browser is rejected with "Finish in the same browser". This blocks cross-account authorization, where an attacker starts a request and seeds a challenge to a victim's email. The owner is bound to the request exactly once. This tab shows no Allow form. It says "go back to the window where you started", with an "Approve here instead" link for flows that ran in this same tab.
+3. **Landing**, `GET /oauth/continue?lt=…&req=…`. The login challenge is consumed atomically and is single-use. It must belong to `req`. **The same-browser binding is mandatory:** the click must carry the `mcp_authreq` cookie for the same `req`. A link opened on another device or browser is rejected with "Finish in the same browser". This blocks cross-account authorization, where an attacker starts a request and seeds a challenge to a victim's email. The owner is bound to the request exactly once. This tab shows no Allow form. It says "go back to the window where you started", with an "Approve in this tab instead" link for flows that ran in this same tab or whose window was closed (the app may not finish connecting from there).
 4. **Back to the window that started it.** OAuth clients (ChatGPT, claude.ai) open `/oauth/authorize` in a popup and expect the redirect back in that window, which holds their pending state. The email link opens a new tab, and a redirect from there fails ("Missing OAuth callback data"). So the "check your email" page polls `GET /oauth/status` every 2.5 seconds for up to 15 minutes.
    - `/oauth/status` answers only for the request named in the browser's signed cookie: `pending`, `ready`, `done` or `expired`. It sends `Cache-Control: no-store`.
-   - On `ready`, the page moves to `GET /oauth/approve`. That page requires the same cookie and an owner already bound, which only step 3 does. It renders consent with a fresh CSRF token.
+   - On `ready`, the page moves to `GET /oauth/approve`. That page requires the same cookie and an owner already bound, which only step 3 does. It renders consent with the request's CSRF token from the cookie, minting one only if the cookie has none. Every window of the browser shares that cookie, so a second load (the link tab's fallback, a reload) must not replace the token: that would fail the Allow form already open in the popup.
    - Both endpoints reveal nothing without the cookie. Neither adds a path around the same-browser binding.
    - A no-JavaScript fallback link, "I've clicked the link, continue", goes to the same page.
-5. **Consent**, `POST /oauth/decision`. The page names the client and its redirect host. Allow consumes the request atomically and mints a 60-second single-use code bound to the full tuple (client, owner, redirect URI, challenge, resource, scope). Deny redirects with `error=access_denied`.
+5. **Consent**, `POST /oauth/decision`. The page names the client and its redirect host. Allow consumes the request atomically and mints a 60-second single-use code bound to the full tuple (client, owner, redirect URI, challenge, resource, scope). Deny also consumes the request, so an Allow form still open in another window can't mint a code afterwards, then redirects with `error=access_denied`.
 
 ## §4 Tokens
 
 `POST /oauth/token` (form-encoded):
 
 - **`authorization_code`.** The code is consumed atomically, so a replay is dead. The server re-verifies `client_id`, the exact `redirect_uri`, PKCE (`base64url(sha256(code_verifier))`) and `resource` if it is resent. It returns an access token (1 hour), a refresh token (30 days), `token_type: Bearer` and the scope.
-- **`refresh_token`.** The token rotates on every use. **Reuse detection:** presenting an already-consumed refresh token revokes the whole chain. If `client_id` is sent, it must be the token's own client.
+- **`refresh_token`.** The token rotates on every use. **Reuse detection:** presenting an already-consumed refresh token revokes the whole chain. If `client_id` is sent, it must be the token's own client. That is checked before the token is consumed, so a mismatch (`invalid_client`) leaves the token usable by its own client.
 - Errors use RFC 6749 §5.2 JSON (`invalid_grant`, `invalid_client`, `invalid_target`, `unsupported_grant_type`).
+- Every response, success or error, carries `Cache-Control: no-store` and `Pragma: no-cache` (RFC 6749 §5.1). A failed refresh logs `[oauth] refresh failed` with its status (`reused` means the chain was just revoked) and the first 8 characters of `client_id`, so a dropped connection shows in `wrangler tail`.
 
 ## §5 The `/mcp` resource server
 

packages/api/src/mcp/handler.test.ts

@@ -220,6 +220,13 @@ describe('/mcp handler', () => {
     }
   });
 
+  it('ping answers an empty result, anonymously too', async () => {
+    const pong = (await (await rpc(null, { jsonrpc: '2.0', id: 'p1', method: 'ping' })).json()) as Rpc;
+    expect(pong.error).toBeUndefined();
+    expect(pong.result).toEqual({});
+    expect(pong.id).toBe('p1');
+  });
+
   it('unknown method → -32601; malformed JSON body → -32700', async () => {
     makeOwner('ow_e', 'e@example.com');
     const token = await mintToken({ ownerId: 'ow_e', scope: 'registry:read' });

packages/api/src/mcp/handler.ts

@@ -1256,6 +1256,9 @@ app.post('/mcp', bearerMiddleware, async (c) => {
         instructions: chatgptMeta.instructions,
       });
     }
+    case 'ping':
+      // MCP basic utilities: a receiver MUST answer ping promptly with an empty result.
+      return rpcResult(c, id, {});
     case 'tools/list':
       return rpcResult(c, id, {
         tools: TOOLS.map((t) => ({

packages/api/src/mcp/oauth-store.test.ts

@@ -200,6 +200,13 @@ describe('oauth_refresh_tokens — rotation + reuse detection', () => {
     expect((await store.rotateRefreshToken(token, iso())).status).toBe('invalid');
   });
 
+  it('another client_id → client_mismatch, and the token stays usable by its own client', async () => {
+    seedOwner('ow_a');
+    const { token } = await store.mintRefreshToken({ clientId: 'oc_1', ownerId: 'ow_a', resource: RESOURCE, scope: 'board:post' });
+    expect((await store.rotateRefreshToken(token, iso(), 'oc_2')).status).toBe('client_mismatch');
+    expect((await store.rotateRefreshToken(token, iso(), 'oc_1')).status).toBe('ok');
+  });
+
   it('REUSE of a consumed token revokes the ENTIRE successor chain', async () => {
     seedOwner('ow_a');
     const { token: r0 } = await store.mintRefreshToken({ clientId: 'oc_1', ownerId: 'ow_a', resource: RESOURCE, scope: 'board:post' });

packages/api/src/mcp/oauth-store.ts

@@ -118,7 +118,8 @@ export interface AccessTokenRow {
 export type RotateResult =
   | { status: 'ok'; refresh_token: string; owner_id: string; client_id: string; resource: string; scope: string }
   | { status: 'reused' } // presented an already-consumed token → chain revoked
-  | { status: 'invalid' }; // unknown / revoked / expired
+  | { status: 'invalid' } // unknown / revoked / expired
+  | { status: 'client_mismatch' }; // live token presented by another client_id; left unconsumed
 
 // Narrow raw-row typing without pulling a helper: every column we read is TEXT.
 type Raw = Record<string, unknown>;
@@ -506,6 +507,9 @@ export class OAuthStore {
    *  - already CONSUMED (replay = theft) → walk `rotated_to` from the presented
    *    row forward and revoke the entire successor chain, forcing full re-auth →
    *    {status:'reused'}
+   *  - live, but `expectedClientId` given and not the token's own client
+   *                                      → {status:'client_mismatch'}, token left
+   *    untouched (a mismatch must not burn the legitimate holder's token)
    *  - live + unconsumed                 → atomically consume it (SET consumed_at,
    *    rotated_to=<new hash>; changes===1 is the gate), then mint the successor →
    *    {status:'ok', refresh_token, …binding}
@@ -514,7 +518,7 @@ export class OAuthStore {
    * consume, changes!==1 and we mint no successor (no orphan valid token). The
    * caller mints the paired access token from the returned binding.
    */
-  async rotateRefreshToken(token: string, nowIso: string): Promise<RotateResult> {
+  async rotateRefreshToken(token: string, nowIso: string, expectedClientId?: string): Promise<RotateResult> {
     const presentedHash = sha256hex(token);
     const row = await this.db.get<Raw>(
       `SELECT client_id, owner_id, resource, scope, expires_at, rotated_to,
@@ -533,6 +537,9 @@ export class OAuthStore {
     if (row.revoked_at != null || s(row.expires_at) <= nowIso) {
       return { status: 'invalid' };
     }
+    if (expectedClientId !== undefined && expectedClientId !== s(row.client_id)) {
+      return { status: 'client_mismatch' };
+    }
 
     const binding = {
       client_id: s(row.client_id),

packages/api/src/mcp/oauth.test.ts

@@ -564,6 +564,41 @@ describe('sign-in completes in the window that started it', () => {
     expect(await status(appr.cookie)).toBe('done');
   });
 
+  it('a second Approve load (the link tab or a reload) keeps the popup\'s Allow form valid', async () => {
+    const { popupCookie, lt, req } = await startAndSend('twice@example.com', 'st-twice');
+    const linkTab = await app.request(`/oauth/continue?lt=${encodeURIComponent(lt)}&req=${encodeURIComponent(req)}`, { headers: { Cookie: popupCookie } }, ENV);
+    const jar = cookieOf(linkTab);
+    // The popup polls to ready and renders its Allow form first...
+    const popup = await approve(jar);
+    // ...then the link tab's "Approve in this tab instead" loads the page again
+    // in the same browser, re-arming the shared cookie.
+    const linkTabApprove = await approve(popup.cookie);
+    expect(linkTabApprove.csrf).toBe(popup.csrf);
+    // The browser now holds the cookie from the latest load; the popup's form
+    // (rendered earlier) must still verify against it.
+    const dec = await app.request('/oauth/decision', { method: 'POST', headers: { ...FORM, Cookie: linkTabApprove.cookie }, body: new URLSearchParams({ req, csrf: popup.csrf, decision: 'allow' }).toString() }, ENV);
+    expect(dec.status).toBe(302);
+    const loc = new URL(dec.headers.get('location')!);
+    expect(loc.searchParams.get('code')).toBeTruthy();
+    expect(loc.searchParams.get('state')).toBe('st-twice');
+  });
+
+  it('Deny is final: an Allow form still open in another window cannot mint a code afterwards', async () => {
+    const { popupCookie, lt, req } = await startAndSend('denyfinal@example.com', 'st-deny');
+    const jar = cookieOf(await app.request(`/oauth/continue?lt=${encodeURIComponent(lt)}&req=${encodeURIComponent(req)}`, { headers: { Cookie: popupCookie } }, ENV));
+    const popup = await approve(jar);
+    const linkTab = await approve(popup.cookie); // same browser, same token
+    const deny = await app.request('/oauth/decision', { method: 'POST', headers: { ...FORM, Cookie: linkTab.cookie }, body: new URLSearchParams({ req, csrf: linkTab.csrf, decision: 'deny' }).toString() }, ENV);
+    expect(deny.status).toBe(302);
+    expect(new URL(deny.headers.get('location')!).searchParams.get('error')).toBe('access_denied');
+    expect(await status(linkTab.cookie)).toBe('done');
+
+    const lateAllow = await app.request('/oauth/decision', { method: 'POST', headers: { ...FORM, Cookie: linkTab.cookie }, body: new URLSearchParams({ req, csrf: popup.csrf, decision: 'allow' }).toString() }, ENV);
+    expect(lateAllow.status).toBe(400);
+    const codes = rawDb.prepare('SELECT COUNT(*) AS n FROM oauth_auth_codes').get() as { n: number };
+    expect(codes.n).toBe(0);
+  });
+
   it('status and approve answer only for the cookie: no cookie or a forged one gets nothing', async () => {
     const { lt, req, popupCookie } = await startAndSend('nocookie@example.com');
     await app.request(`/oauth/continue?lt=${encodeURIComponent(lt)}&req=${encodeURIComponent(req)}`, { headers: { Cookie: popupCookie } }, ENV);
@@ -622,6 +657,19 @@ describe('/oauth/token', () => {
     expect(body.scope).toBe('registry:read board:post');
   });
 
+  it('marks every token response, success or error, as not cacheable', async () => {
+    const verifier = 'nostore-verifier-0000000';
+    const { cid, code } = await freshCode('nostore@example.com', verifier);
+    const ok = await tokenExchange({ grant_type: 'authorization_code', code, client_id: cid, redirect_uri: REDIRECT, code_verifier: verifier });
+    expect(ok.status).toBe(200);
+    const bad = await tokenExchange({ grant_type: 'refresh_token', refresh_token: 'not-a-real-refresh-token', client_id: cid });
+    expect(bad.status).toBe(400);
+    for (const res of [ok, bad]) {
+      expect(res.headers.get('cache-control')).toBe('no-store');
+      expect(res.headers.get('pragma')).toBe('no-cache');
+    }
+  });
+
   it('rejects a wrong PKCE verifier with invalid_grant', async () => {
     const { cid, code } = await freshCode('t2@example.com', 'right-verifier-111111');
     const res = await tokenExchange({ grant_type: 'authorization_code', code, client_id: cid, redirect_uri: REDIRECT, code_verifier: 'WRONG-verifier' });
@@ -680,6 +728,20 @@ describe('/oauth/token', () => {
     expect(live.n).toBe(0);
   });
 
+  it('a refresh from the wrong client_id is refused without burning the token', async () => {
+    const verifier = 'mismatch-verifier-666666';
+    const { cid, code } = await freshCode('t7@example.com', verifier);
+    const first = (await (await tokenExchange({ grant_type: 'authorization_code', code, client_id: cid, redirect_uri: REDIRECT, code_verifier: verifier })).json()) as { refresh_token: string };
+
+    const wrong = await tokenExchange({ grant_type: 'refresh_token', refresh_token: first.refresh_token, client_id: 'oc_someone_else' });
+    expect(wrong.status).toBe(400);
+    expect((await wrong.json()) as { error: string }).toMatchObject({ error: 'invalid_client' });
+
+    // The token's own client can still use it: nothing was consumed or revoked.
+    const right = await tokenExchange({ grant_type: 'refresh_token', refresh_token: first.refresh_token, client_id: cid });
+    expect(right.status).toBe(200);
+  });
+
   it('rejects an unsupported grant_type', async () => {
     const res = await tokenExchange({ grant_type: 'password', username: 'x', password: 'y' });
     expect(res.status).toBe(400);