From 38501f6574f7c15f9f0b29f2b180f22adb6d3a06 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Wed, 29 Jul 2026 13:40:03 +0200 Subject: [PATCH] test(01-01): specify transactional refresh rotation - require one transaction for rotation and successor insertion - pin CAS-loss revocation and insertion rollback semantics --- src/app/api/mcp/oauth/__tests__/token.test.ts | 104 +++++++++++++++++- .../mcp/oauth/__tests__/connections.test.ts | 72 ++++++++++++ 2 files changed, 172 insertions(+), 4 deletions(-) diff --git a/src/app/api/mcp/oauth/__tests__/token.test.ts b/src/app/api/mcp/oauth/__tests__/token.test.ts index 76e3fbb07..54bf4e451 100644 --- a/src/app/api/mcp/oauth/__tests__/token.test.ts +++ b/src/app/api/mcp/oauth/__tests__/token.test.ts @@ -7,6 +7,20 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; process.env.API_TOKEN_HMAC_KEY = "x".repeat(48); process.env.APP_URL = "https://health.example"; +const { transactionClient, transaction } = vi.hoisted(() => { + const transactionClient = { marker: "mcp-refresh-transaction" }; + return { + transactionClient, + transaction: vi.fn( + async (callback: (tx: typeof transactionClient) => unknown) => + callback(transactionClient), + ), + }; +}); + +vi.mock("@/lib/db", () => ({ + prisma: { $transaction: transaction }, +})); vi.mock("@/lib/logging/background", () => ({ withBackgroundEvent: (_n: string, fn: () => unknown) => fn(), })); @@ -51,7 +65,7 @@ import { createConnection, rotateConnection, } from "@/lib/mcp/oauth/connections"; -import { signArtifact } from "@/lib/mcp/oauth/artifacts"; +import { signArtifact, verifyArtifact } from "@/lib/mcp/oauth/artifacts"; import { s256Challenge } from "@/lib/mcp/oauth/pkce"; const CLIENT_ID = "https://app.example/client.json"; @@ -95,6 +109,10 @@ beforeEach(() => { vi.mocked(isApiGloballyEnabled).mockResolvedValue(true); vi.mocked(createConnection).mockResolvedValue("conn-1"); vi.mocked(rotateConnection).mockResolvedValue({ ok: true }); + transaction.mockImplementation( + async (callback: (tx: typeof transactionClient) => unknown) => + callback(transactionClient), + ); }); function refreshToken(overrides: Record = {}): string { @@ -217,10 +235,11 @@ describe("authorization_code grant", () => { describe("refresh_token grant", () => { it("rotates: issues a new access + refresh pair against a live connection", async () => { + const presented = refreshToken({ jti: "presented-jti" }); const res = await POST( tokenReq({ grant_type: "refresh_token", - refresh_token: refreshToken(), + refresh_token: presented, client_id: CLIENT_ID, }) as never, ); @@ -228,9 +247,86 @@ describe("refresh_token grant", () => { const body = await res.json(); expect(body.access_token).toMatch(/^hlk_/); expect(body.refresh_token).toMatch(/^hlrt_/); - expect(rotateConnection).toHaveBeenCalledWith( - expect.objectContaining({ connectionId: "conn-1", clientId: CLIENT_ID }), + expect(vi.mocked(rotateConnection).mock.calls[0]?.[0]).toEqual( + expect.objectContaining({ + connectionId: "conn-1", + clientId: CLIENT_ID, + presentedJti: "presented-jti", + }), ); + + const successor = verifyArtifact<{ + cid: string; + jti: string; + sub: string; + }>("refreshToken", body.refresh_token); + expect(successor.ok).toBe(true); + if (successor.ok) { + expect(successor.claims.cid).toBe("conn-1"); + expect(successor.claims.sub).toBe("user-1"); + expect(successor.claims.jti).not.toBe("presented-jti"); + } + }); + + it("rotates and inserts the successor access token inside one transaction", async () => { + const res = await POST( + tokenReq({ + grant_type: "refresh_token", + refresh_token: refreshToken({ jti: "transactional-jti" }), + client_id: CLIENT_ID, + }) as never, + ); + + expect(res.status).toBe(200); + expect(transaction).toHaveBeenCalledTimes(1); + expect(rotateConnection).toHaveBeenCalledWith( + expect.objectContaining({ presentedJti: "transactional-jti" }), + transactionClient, + ); + expect(issueApiToken).toHaveBeenCalledWith( + expect.objectContaining({ + userId: "user-1", + mcpConnectionId: "conn-1", + }), + transactionClient, + ); + }); + + it("rolls back JTI advancement and access revocation when successor insertion fails", async () => { + const state = { currentJti: "rollback-jti", accessRevoked: false }; + vi.mocked(rotateConnection).mockImplementationOnce(async (args) => { + state.currentJti = args.newJti; + state.accessRevoked = true; + return { ok: true }; + }); + vi.mocked(issueApiToken).mockRejectedValueOnce( + new Error("injected successor insertion failure"), + ); + transaction.mockImplementationOnce(async (callback) => { + const snapshot = { ...state }; + try { + return await callback(transactionClient); + } catch (error) { + Object.assign(state, snapshot); + throw error; + } + }); + + await expect( + POST( + tokenReq({ + grant_type: "refresh_token", + refresh_token: refreshToken({ jti: "rollback-jti" }), + client_id: CLIENT_ID, + }) as never, + ), + ).rejects.toThrow("injected successor insertion failure"); + + expect(transaction).toHaveBeenCalledTimes(1); + expect(state).toEqual({ + currentJti: "rollback-jti", + accessRevoked: false, + }); }); it("rejects a refresh whose connection was revoked (H2 — settings revoke)", async () => { diff --git a/src/lib/mcp/oauth/__tests__/connections.test.ts b/src/lib/mcp/oauth/__tests__/connections.test.ts index c9223fa9d..007d3b99f 100644 --- a/src/lib/mcp/oauth/__tests__/connections.test.ts +++ b/src/lib/mcp/oauth/__tests__/connections.test.ts @@ -37,6 +37,18 @@ const CONN = { revokedAt: null as Date | null, }; +const TX = { + mcpOAuthConnection: { + create: vi.fn(), + findUnique: vi.fn(), + updateMany: vi.fn(), + findMany: vi.fn(), + }, + apiToken: { + updateMany: vi.fn(), + }, +}; + beforeEach(() => { vi.clearAllMocks(); vi.mocked(prisma.apiToken.updateMany).mockResolvedValue({ @@ -45,6 +57,8 @@ beforeEach(() => { vi.mocked(prisma.mcpOAuthConnection.updateMany).mockResolvedValue({ count: 1, } as never); + TX.mcpOAuthConnection.updateMany.mockResolvedValue({ count: 1 }); + TX.apiToken.updateMany.mockResolvedValue({ count: 0 }); }); describe("createConnection", () => { @@ -96,6 +110,33 @@ describe("rotateConnection — happy path", () => { }), ); }); + + it("uses the supplied transaction client for the entire rotation", async () => { + TX.mcpOAuthConnection.findUnique.mockResolvedValue({ ...CONN }); + + const out = await rotateConnection( + { + connectionId: "conn-1", + presentedJti: "jti-current", + newJti: "jti-next", + clientId: CONN.clientId, + userId: "user-1", + }, + TX as never, + ); + + expect(out.ok).toBe(true); + expect(TX.mcpOAuthConnection.findUnique).toHaveBeenCalledTimes(1); + expect(TX.mcpOAuthConnection.updateMany).toHaveBeenCalledTimes(1); + expect(TX.apiToken.updateMany).toHaveBeenCalledWith( + expect.objectContaining({ + where: { mcpConnectionId: "conn-1", revoked: false }, + }), + ); + expect(prisma.mcpOAuthConnection.findUnique).not.toHaveBeenCalled(); + expect(prisma.mcpOAuthConnection.updateMany).not.toHaveBeenCalled(); + expect(prisma.apiToken.updateMany).not.toHaveBeenCalled(); + }); }); describe("rotateConnection — revoked connection stops refresh", () => { @@ -158,6 +199,37 @@ describe("rotateConnection — reuse detection revokes the family", () => { }); expect(out).toEqual({ ok: false, reason: "not_found" }); }); + + it("revokes the family and linked access tokens when the CAS contender loses", async () => { + vi.mocked(prisma.mcpOAuthConnection.findUnique).mockResolvedValue({ + ...CONN, + } as never); + vi.mocked(prisma.mcpOAuthConnection.updateMany) + .mockResolvedValueOnce({ count: 0 } as never) + .mockResolvedValueOnce({ count: 1 } as never); + + const out = await rotateConnection({ + connectionId: "conn-1", + presentedJti: "jti-current", + newJti: "jti-loser", + clientId: CONN.clientId, + userId: "user-1", + }); + + expect(out).toEqual({ ok: false, reason: "reuse_detected" }); + expect(prisma.mcpOAuthConnection.updateMany).toHaveBeenLastCalledWith( + expect.objectContaining({ + where: { id: "conn-1", revokedAt: null }, + data: expect.objectContaining({ revokedAt: expect.any(Date) }), + }), + ); + expect(prisma.apiToken.updateMany).toHaveBeenCalledWith( + expect.objectContaining({ + where: { mcpConnectionId: "conn-1", revoked: false }, + data: { revoked: true }, + }), + ); + }); }); describe("revokeConnectionForUser — settings revoke kills the chain", () => {