diff --git a/.changeset/mcp-refresh-token-replay-revocation.md b/.changeset/mcp-refresh-token-replay-revocation.md new file mode 100644 index 000000000..54bb2abd2 --- /dev/null +++ b/.changeset/mcp-refresh-token-replay-revocation.md @@ -0,0 +1,5 @@ +--- +'@roomote/web': patch +--- + +Revoke the entire remote MCP OAuth refresh-token family when an already-rotated refresh token is replayed, per the OAuth 2.0 Security BCP. Previously the replay was rejected but the rest of the token family stayed valid, so a stolen-token signal never disabled the remaining tokens. diff --git a/apps/web/src/app/api/mcp-remote-oauth/token/__tests__/route.test.ts b/apps/web/src/app/api/mcp-remote-oauth/token/__tests__/route.test.ts index 500f59b6a..e4fe7f89e 100644 --- a/apps/web/src/app/api/mcp-remote-oauth/token/__tests__/route.test.ts +++ b/apps/web/src/app/api/mcp-remote-oauth/token/__tests__/route.test.ts @@ -7,6 +7,7 @@ const { mockPromoteClient, mockCreateRefreshSession, mockGetRefreshSession, + mockRevokeOnReplay, mockRotateRefreshToken, mockGetClient, mockCreateToken, @@ -17,6 +18,7 @@ const { mockPromoteClient: vi.fn(), mockCreateRefreshSession: vi.fn(), mockGetRefreshSession: vi.fn(), + mockRevokeOnReplay: vi.fn(), mockRotateRefreshToken: vi.fn(), mockGetClient: vi.fn(), mockCreateToken: vi.fn(), @@ -35,6 +37,7 @@ vi.mock('@/lib/server/mcp-remote-oauth', async (importOriginal) => ({ promoteRemoteMcpOAuthClient: mockPromoteClient, createRemoteMcpRefreshSession: mockCreateRefreshSession, getRemoteMcpRefreshSession: mockGetRefreshSession, + revokeRemoteMcpRefreshSessionOnReplay: mockRevokeOnReplay, rotateRemoteMcpRefreshToken: mockRotateRefreshToken, getRemoteMcpOAuthClient: mockGetClient, })); @@ -115,6 +118,7 @@ describe('POST /api/mcp-remote-oauth/token', () => { currentTokenHash: 'token-hash', expiresAt: Math.floor(Date.now() / 1000) + 3_600, }); + mockRevokeOnReplay.mockResolvedValue(false); mockRotateRefreshToken.mockResolvedValue({ status: 'ok', refreshToken: 'rotated-refresh-token', @@ -237,6 +241,33 @@ describe('POST /api/mcp-remote-oauth/token', () => { await expect(response.json()).resolves.toEqual({ error: 'invalid_grant' }); }); + it('revokes the session family when a rotated refresh token is replayed', async () => { + // A rotated token no longer resolves to an active session; the route must + // still report the replay so the family (including its current token) is + // revoked instead of surviving the theft signal. + mockGetRefreshSession.mockResolvedValue(null); + mockRevokeOnReplay.mockResolvedValue(true); + + const response = await POST(refreshRequest()); + + expect(response.status).toBe(400); + await expect(response.json()).resolves.toEqual({ error: 'invalid_grant' }); + expect(mockRevokeOnReplay).toHaveBeenCalledWith('session.refresh-token'); + expect(mockRotateRefreshToken).not.toHaveBeenCalled(); + expect(mockCreateToken).not.toHaveBeenCalled(); + }); + + it('ignores replay revocation for unknown refresh tokens', async () => { + mockGetRefreshSession.mockResolvedValue(null); + + const response = await POST(refreshRequest()); + + expect(response.status).toBe(400); + await expect(response.json()).resolves.toEqual({ error: 'invalid_grant' }); + expect(mockRevokeOnReplay).toHaveBeenCalledWith('session.refresh-token'); + expect(mockRotateRefreshToken).not.toHaveBeenCalled(); + }); + it('rejects a verifier that does not match the authorization code', async () => { const response = await POST( tokenRequest({ diff --git a/apps/web/src/app/api/mcp-remote-oauth/token/route.ts b/apps/web/src/app/api/mcp-remote-oauth/token/route.ts index 16e1aface..010cac8e6 100644 --- a/apps/web/src/app/api/mcp-remote-oauth/token/route.ts +++ b/apps/web/src/app/api/mcp-remote-oauth/token/route.ts @@ -14,6 +14,7 @@ import { getRemoteMcpOAuthClient, getRemoteMcpRefreshSession, promoteRemoteMcpOAuthClient, + revokeRemoteMcpRefreshSessionOnReplay, rotateRemoteMcpRefreshToken, verifyPkceChallenge, } from '@/lib/server/mcp-remote-oauth'; @@ -80,8 +81,15 @@ export async function POST(request: NextRequest) { getRemoteMcpRefreshSession(input.refresh_token), getRemoteMcpOAuthClient(input.client_id), ]); + if (!session) { + // The presented token is not the family's current token. If it is an + // already-rotated token, its replay revokes the entire session family + // per the OAuth 2.0 Security BCP (refresh token rotation with reuse + // detection); unknown or expired tokens are ignored. + await revokeRemoteMcpRefreshSessionOnReplay(input.refresh_token); + return oauthError('invalid_grant'); + } if ( - !session || !client || !client.grantTypes.includes('refresh_token') || session.clientId !== input.client_id || diff --git a/apps/web/src/lib/server/mcp-remote-oauth.test.ts b/apps/web/src/lib/server/mcp-remote-oauth.test.ts index f234debc7..7fd951daa 100644 --- a/apps/web/src/lib/server/mcp-remote-oauth.test.ts +++ b/apps/web/src/lib/server/mcp-remote-oauth.test.ts @@ -142,6 +142,20 @@ vi.mock('@roomote/redis', () => ({ return 1; } + if (script.includes('if marker ~= ARGV[1]')) { + const [rotatedMarker, refreshPrefix] = values; + if (redisState.get(key) !== rotatedMarker) return 0; + const session = redisState.get(keys[1]!); + if (session) { + const decoded = JSON.parse(session) as { + currentTokenHash: string; + }; + redisState.delete(`${refreshPrefix}${decoded.currentTokenHash}`); + } + redisState.delete(keys[1]!); + return 1; + } + if (script.includes("redis.call('GET'")) { const value = redisState.get(key) ?? null; if (value === values[0]) redisState.delete(key); @@ -168,6 +182,7 @@ import { promoteRemoteMcpOAuthClient, registerRemoteMcpOAuthClient, revokeRemoteMcpRefreshSession, + revokeRemoteMcpRefreshSessionOnReplay, rotateRemoteMcpRefreshToken, verifyPkceChallenge, } from './mcp-remote-oauth'; @@ -341,6 +356,59 @@ describe('remote MCP OAuth state', () => { ); }); + it('revokes the whole family when a rotated token is replayed', async () => { + const refreshToken = await createRemoteMcpRefreshSession({ + userId: 'user-1', + clientId: 'client-1', + resource: 'https://roomote.example/mcp', + scopes: ['mcp:roomote'], + }); + const session = await getRemoteMcpRefreshSession(refreshToken); + const rotation = await rotateRemoteMcpRefreshToken(refreshToken, session!); + if (rotation.status !== 'ok') throw new Error('expected refresh rotation'); + + // The rotated token is no longer a valid session, but its replay must + // still kill the family: the current token dies with it. + await expect(getRemoteMcpRefreshSession(refreshToken)).resolves.toBeNull(); + await expect( + revokeRemoteMcpRefreshSessionOnReplay(refreshToken), + ).resolves.toBe(true); + await expect( + getRemoteMcpRefreshSession(rotation.refreshToken), + ).resolves.toBeNull(); + + // The rotated marker persists, so repeat replays keep reporting the + // replay signal (the family is already dead; there is nothing left to + // revoke). + await expect( + revokeRemoteMcpRefreshSessionOnReplay(refreshToken), + ).resolves.toBe(true); + }); + + it('does not revoke a family for its current token or unknown tokens', async () => { + const refreshToken = await createRemoteMcpRefreshSession({ + userId: 'user-1', + clientId: 'client-1', + resource: 'https://roomote.example/mcp', + scopes: ['mcp:roomote'], + }); + + await expect( + revokeRemoteMcpRefreshSessionOnReplay(refreshToken), + ).resolves.toBe(false); + await expect( + revokeRemoteMcpRefreshSessionOnReplay( + `${'b'.repeat(64)}.${'c'.repeat(43)}`, + ), + ).resolves.toBe(false); + await expect( + revokeRemoteMcpRefreshSessionOnReplay('not-a-token'), + ).resolves.toBe(false); + await expect( + getRemoteMcpRefreshSession(refreshToken), + ).resolves.toMatchObject({ userId: 'user-1', clientId: 'client-1' }); + }); + it('revokes a refresh session by client ID', async () => { const refreshToken = await createRemoteMcpRefreshSession({ userId: 'user-1', diff --git a/apps/web/src/lib/server/mcp-remote-oauth.ts b/apps/web/src/lib/server/mcp-remote-oauth.ts index c8a039f94..92ed65fe8 100644 --- a/apps/web/src/lib/server/mcp-remote-oauth.ts +++ b/apps/web/src/lib/server/mcp-remote-oauth.ts @@ -122,6 +122,20 @@ redis.call('SET', KEYS[3], ARGV[4], 'EX', ARGV[6]) return {'ok'} `; +const REVOKE_SESSION_ON_REPLAY_LUA = ` +local marker = redis.call('GET', KEYS[1]) +if marker ~= ARGV[1] then + return 0 +end +local session = redis.call('GET', KEYS[2]) +if session then + local decoded = cjson.decode(session) + redis.call('DEL', ARGV[2] .. decoded.currentTokenHash) +end +redis.call('DEL', KEYS[2]) +return 1 +`; + const REVOKE_REFRESH_SESSION_LUA = ` local marker = redis.call('GET', KEYS[1]) if marker ~= ARGV[3] then @@ -475,6 +489,31 @@ export async function revokeRemoteMcpRefreshSession( ); } +/** + * Replay of an already-rotated refresh token revokes the whole session family + * (OAuth 2.0 Security BCP): the session and its current token are deleted, so + * every descendant token dies with them. Only tokens carrying the `rotated:` + * marker left behind by a successful rotation trigger this; unknown or expired + * tokens are ignored so random garbage cannot kill a live family. Returns true + * when a replay was detected and the family was revoked. + */ +export async function revokeRemoteMcpRefreshSessionOnReplay( + refreshToken: string, +): Promise { + const sessionId = parseRefreshToken(refreshToken); + if (!sessionId) return false; + const tokenHash = refreshTokenHash(refreshToken); + const revoked = await getRedis().eval( + REVOKE_SESSION_ON_REPLAY_LUA, + 2, + refreshTokenKey(tokenHash), + refreshSessionKey(sessionId), + `rotated:${sessionId}`, + REFRESH_TOKEN_KEY_PREFIX, + ); + return revoked === 1; +} + export async function isRemoteMcpRegistrationAllowed( registrationFingerprint: string, ): Promise {