From dcbe92cd07b30482f3fc39b87b940c68d05a7e11 Mon Sep 17 00:00:00 2001 From: Xu Zhipei Date: Wed, 29 Jul 2026 21:53:01 +0800 Subject: [PATCH 1/6] feat(oauth): add rotating refresh tokens --- src/auth/oauthAuthorizationFlow.ts | 18 +- .../sdkOAuthServerProvider.refresh.test.ts | 215 ++++++++++++++++++ src/auth/sdkOAuthServerProvider.test.ts | 52 ++++- src/auth/sdkOAuthServerProvider.ts | 173 ++++++++++++-- src/auth/sessionTypes.ts | 14 ++ src/auth/storage/fileStorageService.ts | 1 + src/auth/storage/oauthStorageService.ts | 10 +- .../storage/refreshTokenFamilyRepository.ts | 152 +++++++++++++ src/auth/storage/sessionRepository.ts | 10 +- src/constants/auth.ts | 11 +- src/core/runtime/runtimeIdentityService.ts | 6 +- src/transport/http/server.ts | 5 +- test/e2e/oauth-loopback-consent.e2e.test.ts | 174 ++++++++++++++ 13 files changed, 814 insertions(+), 27 deletions(-) create mode 100644 src/auth/sdkOAuthServerProvider.refresh.test.ts create mode 100644 src/auth/storage/refreshTokenFamilyRepository.ts create mode 100644 test/e2e/oauth-loopback-consent.e2e.test.ts diff --git a/src/auth/oauthAuthorizationFlow.ts b/src/auth/oauthAuthorizationFlow.ts index 7c9f5706..e4d91337 100644 --- a/src/auth/oauthAuthorizationFlow.ts +++ b/src/auth/oauthAuthorizationFlow.ts @@ -15,7 +15,14 @@ export interface OAuthAuthorizationFlowStorage { getClient(clientId: string): unknown | null | undefined; processConsentApproval(authRequestId: string, selectedScopes: string[]): Promise<{ redirectUrl: URL }>; processConsentDenial(authRequestId: string): Promise; - createSessionWithId(tokenId: string, clientId: string, resource: string, scopes: string[], ttlMs: number): string; + createSessionWithId( + tokenId: string, + clientId: string, + resource: string, + scopes: string[], + ttlMs: number, + refreshFamilyId?: string, + ): string; } export interface OAuthAuthorizationFlowStorageService { @@ -26,7 +33,14 @@ export interface OAuthAuthorizationFlowStorageService { processConsentApproval(authRequestId: string, selectedScopes: string[]): Promise<{ redirectUrl: URL }>; processConsentDenial(authRequestId: string): Promise; sessionRepository: { - createWithId(tokenId: string, clientId: string, resource: string, scopes: string[], ttlMs: number): string; + createWithId( + tokenId: string, + clientId: string, + resource: string, + scopes: string[], + ttlMs: number, + refreshFamilyId?: string, + ): string; }; } diff --git a/src/auth/sdkOAuthServerProvider.refresh.test.ts b/src/auth/sdkOAuthServerProvider.refresh.test.ts new file mode 100644 index 00000000..8c80a321 --- /dev/null +++ b/src/auth/sdkOAuthServerProvider.refresh.test.ts @@ -0,0 +1,215 @@ +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; + +import { + InvalidGrantError, + InvalidScopeError, + InvalidTargetError, +} from '@modelcontextprotocol/sdk/server/auth/errors.js'; +import type { OAuthClientInformationFull, OAuthTokens } from '@modelcontextprotocol/sdk/shared/auth.js'; + +import { AUTH_CONFIG } from '@src/constants.js'; +import { AgentConfigManager } from '@src/core/server/agentConfig.js'; +import logger from '@src/logger/logger.js'; + +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +import { SDKOAuthServerProvider } from './sdkOAuthServerProvider.js'; + +const CLIENT: OAuthClientInformationFull = { + client_id: 'refresh-client', + redirect_uris: ['http://127.0.0.1:3000/callback'], + grant_types: ['authorization_code', 'refresh_token'], + response_types: ['code'], + token_endpoint_auth_method: 'none', +}; +const OTHER_CLIENT: OAuthClientInformationFull = { ...CLIENT, client_id: 'other-client' }; +const ACCESS_ONLY_CLIENT: OAuthClientInformationFull = { + ...CLIENT, + client_id: 'access-only-client', + grant_types: ['authorization_code'], +}; +const RESOURCE = 'https://resource.example/mcp'; +const SCOPES = ['tag:alpha', 'tag:beta']; + +describe('SDKOAuthServerProvider refresh token families', () => { + let provider: SDKOAuthServerProvider; + let tempDir: string; + let originalAuthEnabled: boolean; + + beforeEach(() => { + tempDir = fs.mkdtempSync(path.join(os.tmpdir(), '1mcp-refresh-family-')); + provider = new SDKOAuthServerProvider(tempDir, 'runtime-scope-a'); + const configManager = AgentConfigManager.getInstance(); + originalAuthEnabled = configManager.get('features').auth; + configManager.get('features').auth = true; + }); + + afterEach(() => { + provider.shutdown(); + AgentConfigManager.getInstance().get('features').auth = originalAuthEnabled; + fs.rmSync(tempDir, { recursive: true, force: true }); + vi.restoreAllMocks(); + }); + + it('issues refresh tokens only to opted-in clients and persists only their SHA-256 digest', async () => { + const renewable = await exchangeAuthorizationCode(provider, CLIENT); + const accessOnly = await exchangeAuthorizationCode(provider, ACCESS_ONLY_CLIENT); + + expect(renewable.refresh_token).toMatch(/^rt-[A-Za-z0-9_-]{43}$/); + expect(accessOnly).not.toHaveProperty('refresh_token'); + + const familyFiles = listFamilyFiles(tempDir); + expect(familyFiles).toHaveLength(1); + const stored = fs.readFileSync(familyFiles[0], 'utf8'); + expect(stored).not.toContain(renewable.refresh_token!); + expect(JSON.parse(stored)).toMatchObject({ + runtimeScopeId: 'runtime-scope-a', + clientId: CLIENT.client_id, + scopeCeiling: SCOPES, + resource: RESOURCE, + status: 'active', + currentTokenDigest: expect.stringMatching(/^[a-f0-9]{64}$/), + }); + }); + + it('persists families across provider restarts and rotation does not extend their fixed expiry', async () => { + const initial = await exchangeAuthorizationCode(provider, CLIENT); + const before = readOnlyFamily(tempDir); + provider.shutdown(); + provider = new SDKOAuthServerProvider(tempDir, 'runtime-scope-a'); + + const rotated = await provider.exchangeRefreshToken(CLIENT, initial.refresh_token!); + const after = readOnlyFamily(tempDir); + + expect(rotated.refresh_token).toMatch(/^rt-/); + expect(rotated.refresh_token).not.toBe(initial.refresh_token); + expect(after.createdAt).toBe(before.createdAt); + expect(after.expires).toBe(before.expires); + expect(after.expires - after.createdAt).toBe(AUTH_CONFIG.SERVER.REFRESH_FAMILY.TTL_MS); + expect(after.consumedTokenDigests).toHaveLength(1); + }); + + it('preserves the original scope ceiling and resource while allowing equal or narrower access', async () => { + const initial = await exchangeAuthorizationCode(provider, CLIENT); + + await expect( + provider.exchangeRefreshToken(CLIENT, initial.refresh_token!, ['tag:alpha', 'tag:outside']), + ).rejects.toBeInstanceOf(InvalidScopeError); + await expect( + provider.exchangeRefreshToken(CLIENT, initial.refresh_token!, undefined, new URL('https://other.example/mcp')), + ).rejects.toBeInstanceOf(InvalidTargetError); + + const narrowed = await provider.exchangeRefreshToken( + CLIENT, + initial.refresh_token!, + ['tag:alpha'], + new URL(RESOURCE), + ); + expect(narrowed.scope).toBe('tag:alpha'); + + const restored = await provider.exchangeRefreshToken(CLIENT, narrowed.refresh_token!); + expect(restored.scope).toBe(SCOPES.join(' ')); + expect(readOnlyFamily(tempDir).scopeCeiling).toEqual(SCOPES); + }); + + it('allows exactly one concurrent rotation and replay revokes the family and every associated access token', async () => { + const initial = await exchangeAuthorizationCode(provider, CLIENT); + const attempts = await Promise.allSettled([ + provider.exchangeRefreshToken(CLIENT, initial.refresh_token!), + provider.exchangeRefreshToken(CLIENT, initial.refresh_token!), + ]); + + expect(attempts.filter((attempt) => attempt.status === 'fulfilled')).toHaveLength(1); + const rejection = attempts.find((attempt) => attempt.status === 'rejected') as PromiseRejectedResult; + expect(rejection.reason).toBeInstanceOf(InvalidGrantError); + expect(readOnlyFamily(tempDir).status).toBe('revoked'); + + const successful = ( + attempts.find((attempt) => attempt.status === 'fulfilled') as PromiseFulfilledResult + ).value; + await expect(provider.verifyAccessToken(initial.access_token)).rejects.toThrow('Invalid or expired access token'); + await expect(provider.verifyAccessToken(successful.access_token)).rejects.toThrow( + 'Invalid or expired access token', + ); + await expect(provider.exchangeRefreshToken(CLIENT, successful.refresh_token!)).rejects.toBeInstanceOf( + InvalidGrantError, + ); + }); + + it('does not mutate a rightful family when another client presents its refresh token', async () => { + const initial = await exchangeAuthorizationCode(provider, CLIENT); + + await expect(provider.exchangeRefreshToken(OTHER_CLIENT, initial.refresh_token!)).rejects.toBeInstanceOf( + InvalidGrantError, + ); + const rightful = await provider.exchangeRefreshToken(CLIENT, initial.refresh_token!); + expect(rightful.refresh_token).toMatch(/^rt-/); + }); + + it('keeps earlier access tokens valid on normal rotation and applies token-specific revocation semantics', async () => { + const initial = await exchangeAuthorizationCode(provider, CLIENT); + const rotated = await provider.exchangeRefreshToken(CLIENT, initial.refresh_token!); + + await expect(provider.verifyAccessToken(initial.access_token)).resolves.toMatchObject({ + clientId: CLIENT.client_id, + }); + await provider.revokeToken(CLIENT, { token: initial.access_token }); + await expect(provider.verifyAccessToken(initial.access_token)).rejects.toThrow('Invalid or expired access token'); + await expect(provider.verifyAccessToken(rotated.access_token)).resolves.toMatchObject({ + clientId: CLIENT.client_id, + }); + + const next = await provider.exchangeRefreshToken(CLIENT, rotated.refresh_token!); + await provider.revokeToken(CLIENT, { token: next.refresh_token! }); + await expect(provider.verifyAccessToken(rotated.access_token)).rejects.toThrow('Invalid or expired access token'); + await expect(provider.verifyAccessToken(next.access_token)).rejects.toThrow('Invalid or expired access token'); + await expect(provider.exchangeRefreshToken(CLIENT, next.refresh_token!)).rejects.toBeInstanceOf(InvalidGrantError); + + await expect(provider.revokeToken(CLIENT, { token: 'unknown-token' })).resolves.toBeUndefined(); + }); + + it('never passes refresh bearer values to the logger', async () => { + const debug = vi.spyOn(logger, 'debug'); + const info = vi.spyOn(logger, 'info'); + const warn = vi.spyOn(logger, 'warn'); + const error = vi.spyOn(logger, 'error'); + const initial = await exchangeAuthorizationCode(provider, CLIENT); + const rotated = await provider.exchangeRefreshToken(CLIENT, initial.refresh_token!); + await provider.revokeToken(CLIENT, { token: rotated.refresh_token! }); + + const logged = JSON.stringify([...debug.mock.calls, ...info.mock.calls, ...warn.mock.calls, ...error.mock.calls]); + expect(logged).not.toContain(initial.refresh_token!); + expect(logged).not.toContain(rotated.refresh_token!); + }); +}); + +async function exchangeAuthorizationCode( + provider: SDKOAuthServerProvider, + client: OAuthClientInformationFull, +): Promise { + const code = provider.oauthStorage.authCodeRepository.create( + client.client_id, + client.redirect_uris[0], + RESOURCE, + SCOPES, + 60_000, + 'challenge', + ); + return provider.exchangeAuthorizationCode(client, code, undefined, client.redirect_uris[0], new URL(RESOURCE)); +} + +function listFamilyFiles(tempDir: string): string[] { + const serverDir = path.join(tempDir, AUTH_CONFIG.SERVER.STORAGE.DIR, AUTH_CONFIG.SERVER.SESSION.SUBDIR); + return fs + .readdirSync(serverDir) + .filter((fileName) => fileName.startsWith(AUTH_CONFIG.SERVER.REFRESH_FAMILY.FILE_PREFIX)) + .map((fileName) => path.join(serverDir, fileName)); +} + +function readOnlyFamily(tempDir: string): Record { + const familyFiles = listFamilyFiles(tempDir); + expect(familyFiles).toHaveLength(1); + return JSON.parse(fs.readFileSync(familyFiles[0], 'utf8')); +} diff --git a/src/auth/sdkOAuthServerProvider.test.ts b/src/auth/sdkOAuthServerProvider.test.ts index 9a7f52e3..6ade92b3 100644 --- a/src/auth/sdkOAuthServerProvider.test.ts +++ b/src/auth/sdkOAuthServerProvider.test.ts @@ -189,11 +189,61 @@ describe('SDKOAuthProvider', () => { expect(response.set).toHaveBeenCalledWith( 'Content-Security-Policy', - "default-src 'none'; form-action 'self'; style-src 'unsafe-inline'; img-src 'self' data:; base-uri 'none'; frame-ancestors 'none';", + "default-src 'none'; form-action 'self' http://localhost:3000; style-src 'unsafe-inline'; img-src 'self' data:; base-uri 'none'; frame-ancestors 'none';", ); expect(response.set).toHaveBeenCalledWith('Content-Type', 'text/html'); expect(response.send).toHaveBeenCalledTimes(1); expect(response.removeHeader).not.toHaveBeenCalledWith('Content-Security-Policy'); }); + + it('discloses renewable access only for clients registered for refresh tokens', () => { + const refreshClient: OAuthClientInformationFull = { + client_id: 'refresh-client', + redirect_uris: ['http://127.0.0.1:3000/callback'], + grant_types: ['authorization_code', 'refresh_token'], + response_types: ['code'], + token_endpoint_auth_method: 'none', + }; + const accessOnlyClient: OAuthClientInformationFull = { + ...refreshClient, + client_id: 'access-only-client', + grant_types: ['authorization_code'], + }; + + expect(provider['generateConsentPageHtml'](refreshClient, 'request-id', [], [])).toContain( + 'renew this access for up to 30 days', + ); + expect(provider['generateConsentPageHtml'](accessOnlyClient, 'request-id', [], [])).not.toContain( + 'renew this access', + ); + }); + + it('does not add non-loopback redirect origins to form-action', async () => { + const client: OAuthClientInformationFull = { + client_id: 'web-client', + redirect_uris: ['https://client.example/callback'], + grant_types: ['authorization_code'], + response_types: ['code'], + token_endpoint_auth_method: 'none', + }; + const response = { set: vi.fn(), send: vi.fn() } as any; + + await provider['renderConsentPage']( + client, + { + redirectUri: 'https://client.example/callback', + codeChallenge: 'challenge', + scopes: [], + }, + [], + [], + response, + ); + + expect(response.set).toHaveBeenCalledWith( + 'Content-Security-Policy', + "default-src 'none'; form-action 'self'; style-src 'unsafe-inline'; img-src 'self' data:; base-uri 'none'; frame-ancestors 'none';", + ); + }); }); }); diff --git a/src/auth/sdkOAuthServerProvider.ts b/src/auth/sdkOAuthServerProvider.ts index eebd63df..c97a92ac 100644 --- a/src/auth/sdkOAuthServerProvider.ts +++ b/src/auth/sdkOAuthServerProvider.ts @@ -1,6 +1,11 @@ import { randomUUID } from 'node:crypto'; import type { OAuthRegisteredClientsStore } from '@modelcontextprotocol/sdk/server/auth/clients.js'; +import { + InvalidGrantError, + InvalidScopeError, + InvalidTargetError, +} from '@modelcontextprotocol/sdk/server/auth/errors.js'; import type { AuthorizationParams, OAuthServerProvider } from '@modelcontextprotocol/sdk/server/auth/provider.js'; import type { AuthInfo } from '@modelcontextprotocol/sdk/server/auth/types.js'; import type { @@ -11,6 +16,7 @@ import type { import { McpConfigManager } from '@src/config/mcpConfigManager.js'; import { AUTH_CONFIG } from '@src/constants.js'; +import { RuntimeIdentityService } from '@src/core/runtime/runtimeIdentityService.js'; import { AgentConfigManager } from '@src/core/server/agentConfig.js'; import logger from '@src/logger/logger.js'; import { escapeHtml } from '@src/utils/validation/sanitization.js'; @@ -25,8 +31,9 @@ import type { Response } from 'express'; import { OAuthStorageService } from './storage/oauthStorageService.js'; -const OAUTH_CONSENT_PAGE_CSP = - "default-src 'none'; form-action 'self'; style-src 'unsafe-inline'; img-src 'self' data:; base-uri 'none'; frame-ancestors 'none';"; +const OAUTH_CONSENT_PAGE_CSP_SUFFIX = + "style-src 'unsafe-inline'; img-src 'self' data:; base-uri 'none'; frame-ancestors 'none';"; +const LOOPBACK_HOSTS = new Set(['localhost', '127.0.0.1', '[::1]']); /** * File-based OAuth clients store implementation using the new repository architecture @@ -84,8 +91,10 @@ export class SDKOAuthServerProvider implements OAuthServerProvider { private configManager: AgentConfigManager; private _clientsStore: OAuthRegisteredClientsStore; - constructor(sessionStoragePath?: string) { - this.oauthStorage = new OAuthStorageService(sessionStoragePath); + constructor(sessionStoragePath?: string, runtimeScopeId?: string) { + const scopeId = + runtimeScopeId ?? new RuntimeIdentityService({ storageDir: sessionStoragePath }).getRuntimeScopeId(); + this.oauthStorage = new OAuthStorageService(sessionStoragePath, scopeId); this.configManager = AgentConfigManager.getInstance(); this._clientsStore = new FileBasedClientsStore(this.oauthStorage); } @@ -180,7 +189,7 @@ export class SDKOAuthServerProvider implements OAuthServerProvider { const scopeTags = scopesToTags(requestedScopes); const consentPageHtml = this.generateConsentPageHtml(client, authRequestId, scopeTags, availableTags); - res.set('Content-Security-Policy', OAUTH_CONSENT_PAGE_CSP); + res.set('Content-Security-Policy', createConsentPageCsp(client, params.redirectUri)); res.set('Content-Type', 'text/html'); res.send(consentPageHtml); } @@ -240,6 +249,9 @@ export class SDKOAuthServerProvider implements OAuthServerProvider { ): string { const clientName = escapeHtml(client.client_name || client.client_id); const escapedAuthRequestId = escapeHtml(authRequestId); + const renewableAccessNotice = client.grant_types?.includes('refresh_token') + ? '

This application can renew this access for up to 30 days.

' + : ''; return ` @@ -276,6 +288,7 @@ export class SDKOAuthServerProvider implements OAuthServerProvider {
Security Notice: Only grant access to server groups that this application needs. + ${renewableAccessNotice}
@@ -327,7 +340,7 @@ export class SDKOAuthServerProvider implements OAuthServerProvider { const codeData = this.oauthStorage.authCodeRepository.get(authorizationCode); if (!codeData || codeData.clientId !== client.client_id) { - throw new Error('Invalid authorization code'); + throw new InvalidGrantError('Invalid authorization code'); } return codeData.codeChallenge || ''; @@ -354,22 +367,22 @@ export class SDKOAuthServerProvider implements OAuthServerProvider { const codeData = this.oauthStorage.authCodeRepository.get(authorizationCode); if (!codeData) { - throw new Error('Invalid or expired authorization code'); + throw new InvalidGrantError('Invalid or expired authorization code'); } // Validate client ID if (codeData.clientId !== client.client_id) { - throw new Error('Client ID mismatch'); + throw new InvalidGrantError('Invalid or expired authorization code'); } // Validate redirect URI if provided if (redirectUri && codeData.redirectUri !== redirectUri) { - throw new Error('Redirect URI mismatch'); + throw new InvalidGrantError('Redirect URI mismatch'); } // Validate resource if provided if (resource && codeData.resource && codeData.resource !== resource.toString()) { - throw new Error('Resource mismatch'); + throw new InvalidTargetError('Resource mismatch'); } // Delete the authorization code (one-time use) @@ -380,6 +393,15 @@ export class SDKOAuthServerProvider implements OAuthServerProvider { const accessToken = AUTH_CONFIG.SERVER.TOKEN.ID_PREFIX + tokenId; const ttlMs = this.configManager.get('auth').oauthTokenTtlMs; + const refreshFamily = client.grant_types?.includes('refresh_token') + ? this.oauthStorage.refreshTokenFamilyRepository.create( + client.client_id, + codeData.scopes, + codeData.resource || '', + tokenId, + ) + : undefined; + // Store session for token validation this.oauthStorage.sessionRepository.createWithId( tokenId, @@ -387,6 +409,7 @@ export class SDKOAuthServerProvider implements OAuthServerProvider { codeData.resource || '', codeData.scopes, ttlMs, + refreshFamily?.family.familyId, ); const tokens: OAuthTokens = { @@ -394,6 +417,7 @@ export class SDKOAuthServerProvider implements OAuthServerProvider { token_type: 'Bearer', expires_in: Math.floor(ttlMs / 1000), scope: codeData.scopes ? codeData.scopes.join(' ') : '', + ...(refreshFamily ? { refresh_token: refreshFamily.refreshToken } : {}), }; logger.info(`Exchanged authorization code for access token`, { @@ -406,22 +430,75 @@ export class SDKOAuthServerProvider implements OAuthServerProvider { } /** - * Exchanges refresh token for new access token (not implemented) + * Atomically rotates a single-use refresh token and issues a new access token. */ async exchangeRefreshToken( - _client: OAuthClientInformationFull, - _refreshToken: string, - _scopes?: string[], - _resource?: URL, + client: OAuthClientInformationFull, + refreshToken: string, + scopes?: string[], + resource?: URL, ): Promise { - throw new Error('Refresh tokens not supported'); + const repository = this.oauthStorage.refreshTokenFamilyRepository; + const family = repository.findByToken(refreshToken); + if (!family || family.clientId !== client.client_id) { + throw new InvalidGrantError('Invalid refresh token'); + } + + const tokenState = repository.getTokenState(family, refreshToken); + if (tokenState === 'consumed') { + const replay = repository.consume(refreshToken, client.client_id, randomUUID()); + if (replay.status === 'replay') { + this.revokeFamilyAccessTokens(replay.family.accessTokenIds); + } + throw new InvalidGrantError('Refresh token replay detected'); + } + if (tokenState !== 'current' || family.status !== 'active') { + throw new InvalidGrantError('Invalid refresh token'); + } + + const requestedScopes = scopes ?? family.scopeCeiling; + if (requestedScopes.some((scope) => !family.scopeCeiling.includes(scope))) { + throw new InvalidScopeError('Requested scope exceeds the originally consented scope'); + } + + if (resource && resource.toString() !== family.resource) { + throw new InvalidTargetError('Requested resource does not match the original resource'); + } + + const tokenId = randomUUID(); + const rotation = repository.consume(refreshToken, client.client_id, tokenId); + if (rotation.status === 'replay') { + this.revokeFamilyAccessTokens(rotation.family.accessTokenIds); + throw new InvalidGrantError('Refresh token replay detected'); + } + if (rotation.status !== 'rotated') { + throw new InvalidGrantError('Invalid refresh token'); + } + + const ttlMs = this.configManager.get('auth').oauthTokenTtlMs; + this.oauthStorage.sessionRepository.createWithId( + tokenId, + client.client_id, + family.resource, + requestedScopes, + ttlMs, + family.familyId, + ); + + return { + access_token: AUTH_CONFIG.SERVER.TOKEN.ID_PREFIX + tokenId, + token_type: 'Bearer', + expires_in: Math.floor(ttlMs / 1000), + scope: requestedScopes.join(' '), + refresh_token: rotation.refreshToken, + }; } /** * Verifies access token and returns auth info with granted scopes */ async verifyAccessToken(token: string): Promise { - logger.debug('Verifying access token', { token }); + logger.debug('Verifying access token'); if (!this.configManager.get('features').auth) { // Auth disabled, return minimal auth info with all available tags as scopes @@ -460,17 +537,30 @@ export class SDKOAuthServerProvider implements OAuthServerProvider { * Revokes a token */ async revokeToken(client: OAuthClientInformationFull, request: OAuthTokenRevocationRequest): Promise { - logger.debug('Revoking token', { clientId: client.client_id, request }); + logger.debug('Revoking OAuth token', { clientId: client.client_id }); const token = request.token; + const refreshFamily = this.oauthStorage.refreshTokenFamilyRepository.findByToken(token); + if (refreshFamily) { + const revokedFamily = this.oauthStorage.refreshTokenFamilyRepository.revokeForClient( + refreshFamily, + client.client_id, + ); + if (revokedFamily) { + this.revokeFamilyAccessTokens(revokedFamily.accessTokenIds); + } + return; + } + // Strip prefix if present const tokenId = token.startsWith(AUTH_CONFIG.SERVER.TOKEN.ID_PREFIX) ? token.slice(AUTH_CONFIG.SERVER.TOKEN.ID_PREFIX.length) : token; const sessionId = AUTH_CONFIG.SERVER.SESSION.ID_PREFIX + tokenId; - const success = this.oauthStorage.sessionRepository.delete(sessionId); + const session = this.oauthStorage.sessionRepository.get(sessionId); + const success = session?.clientId === client.client_id && this.oauthStorage.sessionRepository.delete(sessionId); if (success) { logger.info(`Revoked access token for client ${client.client_id}`, { @@ -479,6 +569,12 @@ export class SDKOAuthServerProvider implements OAuthServerProvider { } } + private revokeFamilyAccessTokens(accessTokenIds: string[]): void { + for (const accessTokenId of accessTokenIds) { + this.oauthStorage.sessionRepository.delete(AUTH_CONFIG.SERVER.SESSION.ID_PREFIX + accessTokenId); + } + } + /** * Graceful shutdown */ @@ -486,3 +582,42 @@ export class SDKOAuthServerProvider implements OAuthServerProvider { this.oauthStorage.shutdown(); } } + +function createConsentPageCsp(client: OAuthClientInformationFull, requestedRedirectUri: string): string { + const callbackOrigin = getValidatedLoopbackCallbackOrigin(client.redirect_uris, requestedRedirectUri); + const formAction = callbackOrigin ? `form-action 'self' ${callbackOrigin};` : "form-action 'self';"; + return `default-src 'none'; ${formAction} ${OAUTH_CONSENT_PAGE_CSP_SUFFIX}`; +} + +function getValidatedLoopbackCallbackOrigin( + registeredRedirectUris: string[], + requestedRedirectUri: string, +): string | null { + let requested: URL; + try { + requested = new URL(requestedRedirectUri); + } catch { + return null; + } + + if (!LOOPBACK_HOSTS.has(requested.hostname)) { + return null; + } + + const matchesRegistered = registeredRedirectUris.some((registeredRedirectUri) => { + try { + const registered = new URL(registeredRedirectUri); + return ( + LOOPBACK_HOSTS.has(registered.hostname) && + requested.protocol === registered.protocol && + requested.hostname === registered.hostname && + requested.pathname === registered.pathname && + requested.search === registered.search + ); + } catch { + return false; + } + }); + + return matchesRegistered ? requested.origin : null; +} diff --git a/src/auth/sessionTypes.ts b/src/auth/sessionTypes.ts index d0865975..7302ce24 100644 --- a/src/auth/sessionTypes.ts +++ b/src/auth/sessionTypes.ts @@ -15,6 +15,20 @@ export interface SessionData extends ExpirableData { clientId: string; resource: string; scopes: string[]; + refreshFamilyId?: string; +} + +export interface RefreshTokenFamilyData extends ExpirableData { + familyId: string; + runtimeScopeId: string; + clientId: string; + scopeCeiling: string[]; + resource: string; + currentTokenDigest: string; + consumedTokenDigests: string[]; + accessTokenIds: string[]; + status: 'active' | 'revoked'; + revokedAt?: number; } export interface ClientData extends ExpirableData, OAuthClientInformationFull {} diff --git a/src/auth/storage/fileStorageService.ts b/src/auth/storage/fileStorageService.ts index a1828469..abb516b1 100644 --- a/src/auth/storage/fileStorageService.ts +++ b/src/auth/storage/fileStorageService.ts @@ -221,6 +221,7 @@ export class FileStorageService { AUTH_CONFIG.SERVER.SESSION.ID_PREFIX, AUTH_CONFIG.SERVER.AUTH_CODE.ID_PREFIX, AUTH_CONFIG.SERVER.AUTH_REQUEST.ID_PREFIX, + AUTH_CONFIG.SERVER.REFRESH_FAMILY.ID_PREFIX, AUTH_CONFIG.SERVER.STREAMABLE_SESSION.ID_PREFIX, ]; diff --git a/src/auth/storage/oauthStorageService.ts b/src/auth/storage/oauthStorageService.ts index 5085eb2f..59e4f750 100644 --- a/src/auth/storage/oauthStorageService.ts +++ b/src/auth/storage/oauthStorageService.ts @@ -1,4 +1,5 @@ import { AUTH_CONFIG } from '@src/constants.js'; +import { RuntimeIdentityService } from '@src/core/runtime/runtimeIdentityService.js'; import logger from '@src/logger/logger.js'; import { auditScopeOperation } from '@src/utils/validation/scopeValidation.js'; @@ -6,6 +7,7 @@ import { AuthCodeRepository } from './authCodeRepository.js'; import { AuthRequestRepository } from './authRequestRepository.js'; import { ClientDataRepository } from './clientDataRepository.js'; import { FileStorageService } from './fileStorageService.js'; +import { RefreshTokenFamilyRepository } from './refreshTokenFamilyRepository.js'; import { SessionRepository } from './sessionRepository.js'; /** @@ -22,13 +24,15 @@ export class OAuthStorageService { private authCodes: AuthCodeRepository; private authRequests: AuthRequestRepository; private clientData: ClientDataRepository; + private refreshTokenFamilies: RefreshTokenFamilyRepository; - constructor(storageDir?: string) { + constructor(storageDir?: string, runtimeScopeId = new RuntimeIdentityService({ storageDir }).getRuntimeScopeId()) { this.storage = new FileStorageService(storageDir, AUTH_CONFIG.SERVER.SESSION.SUBDIR); this.sessions = new SessionRepository(this.storage); this.authCodes = new AuthCodeRepository(this.storage); this.authRequests = new AuthRequestRepository(this.storage); this.clientData = new ClientDataRepository(this.storage); + this.refreshTokenFamilies = new RefreshTokenFamilyRepository(this.storage, runtimeScopeId); } /** @@ -155,6 +159,10 @@ export class OAuthStorageService { return this.clientData; } + get refreshTokenFamilyRepository(): RefreshTokenFamilyRepository { + return this.refreshTokenFamilies; + } + /** * Gets the storage directory path */ diff --git a/src/auth/storage/refreshTokenFamilyRepository.ts b/src/auth/storage/refreshTokenFamilyRepository.ts new file mode 100644 index 00000000..3f153898 --- /dev/null +++ b/src/auth/storage/refreshTokenFamilyRepository.ts @@ -0,0 +1,152 @@ +import { createHash, randomBytes, randomUUID, timingSafeEqual } from 'node:crypto'; + +import { RefreshTokenFamilyData } from '@src/auth/sessionTypes.js'; +import { AUTH_CONFIG } from '@src/constants.js'; + +import { FileStorageService } from './fileStorageService.js'; + +export type RefreshTokenConsumptionResult = + | { status: 'rotated'; family: RefreshTokenFamilyData; refreshToken: string } + | { status: 'replay'; family: RefreshTokenFamilyData } + | { status: 'invalid' } + | { status: 'client_mismatch' }; + +export class RefreshTokenFamilyRepository { + constructor( + private readonly storage: FileStorageService, + private readonly runtimeScopeId: string, + ) {} + + create( + clientId: string, + scopeCeiling: string[], + resource: string, + accessTokenId: string, + ): { + family: RefreshTokenFamilyData; + refreshToken: string; + } { + const familyId = AUTH_CONFIG.SERVER.REFRESH_FAMILY.ID_PREFIX + randomUUID(); + const refreshToken = createRefreshToken(); + const now = Date.now(); + const family: RefreshTokenFamilyData = { + familyId, + runtimeScopeId: this.runtimeScopeId, + clientId, + scopeCeiling: [...scopeCeiling], + resource, + currentTokenDigest: digestRefreshToken(refreshToken), + consumedTokenDigests: [], + accessTokenIds: [accessTokenId], + status: 'active', + createdAt: now, + expires: now + AUTH_CONFIG.SERVER.REFRESH_FAMILY.TTL_MS, + }; + + this.save(family); + return { family, refreshToken }; + } + + findByToken(refreshToken: string): RefreshTokenFamilyData | null { + const digest = digestRefreshToken(refreshToken); + return this.list().find((family) => tokenDigestMatches(family, digest)) ?? null; + } + + getTokenState(family: RefreshTokenFamilyData, refreshToken: string): 'current' | 'consumed' | 'unknown' { + const digest = digestRefreshToken(refreshToken); + if (safeDigestEqual(family.currentTokenDigest, digest)) { + return 'current'; + } + if (family.consumedTokenDigests.some((consumed) => safeDigestEqual(consumed, digest))) { + return 'consumed'; + } + return 'unknown'; + } + + consume(refreshToken: string, clientId: string, accessTokenId: string): RefreshTokenConsumptionResult { + const digest = digestRefreshToken(refreshToken); + const family = this.list().find((candidate) => tokenDigestMatches(candidate, digest)); + if (!family || family.runtimeScopeId !== this.runtimeScopeId) { + return { status: 'invalid' }; + } + + if (family.clientId !== clientId) { + return { status: 'client_mismatch' }; + } + + if (family.consumedTokenDigests.some((consumed) => safeDigestEqual(consumed, digest))) { + const revokedFamily = this.revoke(family); + return { status: 'replay', family: revokedFamily }; + } + + if (family.status !== 'active' || !safeDigestEqual(family.currentTokenDigest, digest)) { + return { status: 'invalid' }; + } + + const nextRefreshToken = createRefreshToken(); + const rotatedFamily: RefreshTokenFamilyData = { + ...family, + currentTokenDigest: digestRefreshToken(nextRefreshToken), + consumedTokenDigests: [...family.consumedTokenDigests, family.currentTokenDigest], + accessTokenIds: [...family.accessTokenIds, accessTokenId], + }; + this.save(rotatedFamily); + return { status: 'rotated', family: rotatedFamily, refreshToken: nextRefreshToken }; + } + + revokeForClient(family: RefreshTokenFamilyData, clientId: string): RefreshTokenFamilyData | null { + if (family.runtimeScopeId !== this.runtimeScopeId || family.clientId !== clientId) { + return null; + } + return this.revoke(family); + } + + private revoke(family: RefreshTokenFamilyData): RefreshTokenFamilyData { + if (family.status === 'revoked') { + return family; + } + const revokedFamily: RefreshTokenFamilyData = { + ...family, + status: 'revoked', + revokedAt: Date.now(), + }; + this.save(revokedFamily); + return revokedFamily; + } + + private list(): RefreshTokenFamilyData[] { + const { FILE_PREFIX, ID_PREFIX } = AUTH_CONFIG.SERVER.REFRESH_FAMILY; + const extension = AUTH_CONFIG.SERVER.STORAGE.FILE_EXTENSION; + return this.storage + .listFiles(FILE_PREFIX) + .map((fileName) => fileName.slice(FILE_PREFIX.length, -extension.length)) + .filter((familyId) => familyId.startsWith(ID_PREFIX)) + .map((familyId) => this.storage.readData(FILE_PREFIX, familyId)) + .filter((family): family is RefreshTokenFamilyData => family !== null); + } + + private save(family: RefreshTokenFamilyData): void { + this.storage.writeData(AUTH_CONFIG.SERVER.REFRESH_FAMILY.FILE_PREFIX, family.familyId, family); + } +} + +export function digestRefreshToken(refreshToken: string): string { + return createHash('sha256').update(refreshToken).digest('hex'); +} + +function createRefreshToken(): string { + return AUTH_CONFIG.SERVER.REFRESH_FAMILY.TOKEN_PREFIX + randomBytes(32).toString('base64url'); +} + +function tokenDigestMatches(family: RefreshTokenFamilyData, digest: string): boolean { + return ( + safeDigestEqual(family.currentTokenDigest, digest) || + family.consumedTokenDigests.some((consumed) => safeDigestEqual(consumed, digest)) + ); +} + +function safeDigestEqual(left: string, right: string): boolean { + const leftBuffer = Buffer.from(left, 'hex'); + const rightBuffer = Buffer.from(right, 'hex'); + return leftBuffer.length === rightBuffer.length && timingSafeEqual(leftBuffer, rightBuffer); +} diff --git a/src/auth/storage/sessionRepository.ts b/src/auth/storage/sessionRepository.ts index 0a659f8e..2acaadf3 100644 --- a/src/auth/storage/sessionRepository.ts +++ b/src/auth/storage/sessionRepository.ts @@ -36,12 +36,20 @@ export class SessionRepository { /** * Creates a session with a specific token ID (for access tokens) */ - createWithId(tokenId: string, clientId: string, resource: string, scopes: string[], ttlMs: number): string { + createWithId( + tokenId: string, + clientId: string, + resource: string, + scopes: string[], + ttlMs: number, + refreshFamilyId?: string, + ): string { const sessionId = AUTH_CONFIG.SERVER.SESSION.ID_PREFIX + tokenId; const sessionData: SessionData = { clientId, resource, scopes, + refreshFamilyId, expires: Date.now() + ttlMs, createdAt: Date.now(), }; diff --git a/src/constants/auth.ts b/src/constants/auth.ts index 1d2b53ea..8e7e9ce7 100644 --- a/src/constants/auth.ts +++ b/src/constants/auth.ts @@ -44,6 +44,15 @@ export const AUTH_CONFIG = { ID_PREFIX: 'tk-', }, + // OAuth refresh token families + REFRESH_FAMILY: { + TTL_MS: 30 * 24 * 60 * 60 * 1000, // 30 days, fixed at family creation + ID_PREFIX: 'rf-', + FILE_PREFIX: 'refresh_family_', + TOKEN_PREFIX: 'rt-', + SUBDIR: 'server', + }, + // Streamable HTTP sessions STREAMABLE_SESSION: { TTL_MS: 24 * 60 * 60 * 1000, // 24 hours @@ -101,7 +110,7 @@ export const STORAGE_SUBDIRS = { // File prefix mapping for migration logic export const FILE_PREFIX_MAPPING = { - SERVER: ['session_', 'auth_code_', 'auth_request_'], + SERVER: ['session_', 'auth_code_', 'auth_request_', 'refresh_family_'], CLIENT: ['oauth_', 'cli_', 'tok_', 'ver_', 'sta_'], TRANSPORT: ['streamable_session_'], } as const; diff --git a/src/core/runtime/runtimeIdentityService.ts b/src/core/runtime/runtimeIdentityService.ts index da978629..4b167fb3 100644 --- a/src/core/runtime/runtimeIdentityService.ts +++ b/src/core/runtime/runtimeIdentityService.ts @@ -42,7 +42,7 @@ export class RuntimeIdentityService { } getRuntimeIdentity(input: RuntimeIdentityInput): RuntimeIdentity { - const runtimeScopeId = this.getOrCreateRuntimeScopeId(); + const runtimeScopeId = this.getRuntimeScopeId(); const identity: RuntimeIdentity = { identityProtocolVersion: '1', runtimeScopeId, @@ -57,6 +57,10 @@ export class RuntimeIdentityService { return identity; } + getRuntimeScopeId(): string { + return this.getOrCreateRuntimeScopeId(); + } + private getOrCreateRuntimeScopeId(): string { const existing = this.readPersistedIdentity(); if (existing?.runtimeScopeId) { diff --git a/src/transport/http/server.ts b/src/transport/http/server.ts index 6998fef1..30905fe6 100644 --- a/src/transport/http/server.ts +++ b/src/transport/http/server.ts @@ -266,7 +266,10 @@ export class ExpressServer { // Initialize OAuth provider with custom session storage path if configured const sessionStoragePath = this.configManager.get('auth').sessionStoragePath; - this.oauthProvider = new SDKOAuthServerProvider(sessionStoragePath); + this.oauthProvider = new SDKOAuthServerProvider( + sessionStoragePath, + this.runtimeIdentityService.getRuntimeScopeId(), + ); // Initialize streamable session repository with 'transport' subdirectory const fileStorageService = new FileStorageService(sessionStoragePath, STORAGE_SUBDIRS.TRANSPORT); diff --git a/test/e2e/oauth-loopback-consent.e2e.test.ts b/test/e2e/oauth-loopback-consent.e2e.test.ts new file mode 100644 index 00000000..79899307 --- /dev/null +++ b/test/e2e/oauth-loopback-consent.e2e.test.ts @@ -0,0 +1,174 @@ +import { createHash, randomBytes } from 'node:crypto'; +import fs from 'node:fs'; +import { createServer, type Server } from 'node:http'; +import os from 'node:os'; +import path from 'node:path'; + +import { mcpAuthRouter } from '@modelcontextprotocol/sdk/server/auth/router.js'; + +import { SDKOAuthServerProvider } from '@src/auth/sdkOAuthServerProvider.js'; +import { McpConfigManager } from '@src/config/mcpConfigManager.js'; +import { createOAuthRoutes } from '@src/transport/http/routes/oauthRoutes.js'; + +import express from 'express'; +import { type Browser, chromium } from 'playwright'; +import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest'; + +describe('OAuth loopback consent browser flow', () => { + let authServer: Server; + let callbackServer: Server; + let authBaseUrl: string; + let callbackUrl: string; + let browser: Browser; + let provider: SDKOAuthServerProvider; + let storageDir: string; + let callbackReceived: Promise; + + beforeAll(async () => { + storageDir = fs.mkdtempSync(path.join(os.tmpdir(), '1mcp-oauth-browser-')); + vi.spyOn(McpConfigManager, 'getInstance').mockReturnValue({ + getAvailableTags: () => ['test'], + } as unknown as McpConfigManager); + + let resolveCallback!: (url: URL) => void; + callbackReceived = new Promise((resolve) => { + resolveCallback = resolve; + }); + callbackServer = createServer((request, response) => { + resolveCallback(new URL(request.url ?? '/', callbackUrl)); + response.writeHead(200, { 'Content-Type': 'text/plain' }); + response.end('Authorization complete'); + }); + callbackUrl = `${await listenOnLoopback(callbackServer)}/callback`; + + const app = express(); + app.use(express.urlencoded({ extended: false })); + authServer = createServer(app); + authBaseUrl = await listenOnLoopback(authServer); + provider = new SDKOAuthServerProvider(storageDir, 'browser-runtime-scope'); + const issuerUrl = new URL(`${authBaseUrl}/`); + app.use( + mcpAuthRouter({ + provider, + issuerUrl, + baseUrl: issuerUrl, + scopesSupported: ['tag:test'], + authorizationOptions: { rateLimit: false }, + tokenOptions: { rateLimit: false }, + revocationOptions: { rateLimit: false }, + clientRegistrationOptions: { rateLimit: false }, + }), + ); + app.use('/oauth', createOAuthRoutes(provider)); + browser = await chromium.launch({ headless: true }); + }, 30_000); + + afterAll(async () => { + await browser?.close(); + provider?.shutdown(); + await closeServer(authServer); + await closeServer(callbackServer); + fs.rmSync(storageDir, { recursive: true, force: true }); + vi.restoreAllMocks(); + }); + + it('completes DCR, cross-port loopback consent, PKCE exchange, and refresh rotation', async () => { + const registrationResponse = await fetch(`${authBaseUrl}/register`, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ + client_name: 'Loopback Browser Client', + redirect_uris: ['http://127.0.0.1:1/callback'], + grant_types: ['authorization_code', 'refresh_token'], + response_types: ['code'], + token_endpoint_auth_method: 'none', + }), + }); + expect(registrationResponse.status, await registrationResponse.clone().text()).toBe(201); + const client = (await registrationResponse.json()) as { client_id: string }; + + const codeVerifier = randomBytes(48).toString('base64url'); + const codeChallenge = createHash('sha256').update(codeVerifier).digest('base64url'); + const resource = `${authBaseUrl}/mcp`; + const authorizationUrl = new URL(`${authBaseUrl}/authorize`); + authorizationUrl.search = new URLSearchParams({ + response_type: 'code', + client_id: client.client_id, + redirect_uri: callbackUrl, + scope: 'tag:test', + state: 'browser-state', + code_challenge: codeChallenge, + code_challenge_method: 'S256', + resource, + }).toString(); + + const context = await browser.newContext(); + const page = await context.newPage(); + try { + const consentResponse = await page.goto(authorizationUrl.toString()); + expect(consentResponse?.status()).toBe(200); + expect(consentResponse?.headers()['content-security-policy']).toContain( + `form-action 'self' ${new URL(callbackUrl).origin}`, + ); + await page.getByText('renew this access for up to 30 days').waitFor({ state: 'visible' }); + + await page.getByRole('button', { name: 'Approve' }).click(); + const callback = await callbackReceived; + expect(callback.searchParams.get('state')).toBe('browser-state'); + const authorizationCode = callback.searchParams.get('code'); + expect(authorizationCode).toMatch(/^code-/); + + const tokenResponse = await postForm(`${authBaseUrl}/token`, { + grant_type: 'authorization_code', + client_id: client.client_id, + code: authorizationCode!, + redirect_uri: callbackUrl, + code_verifier: codeVerifier, + resource, + }); + expect(tokenResponse.status, await tokenResponse.clone().text()).toBe(200); + const tokens = (await tokenResponse.json()) as { access_token: string; refresh_token: string; scope: string }; + expect(tokens.access_token).toMatch(/^tk-/); + expect(tokens.refresh_token).toMatch(/^rt-[A-Za-z0-9_-]{43}$/); + + const refreshResponse = await postForm(`${authBaseUrl}/token`, { + grant_type: 'refresh_token', + client_id: client.client_id, + refresh_token: tokens.refresh_token, + resource, + }); + expect(refreshResponse.status, await refreshResponse.clone().text()).toBe(200); + const rotated = (await refreshResponse.json()) as { access_token: string; refresh_token: string; scope: string }; + expect(rotated.access_token).toMatch(/^tk-/); + expect(rotated.refresh_token).toMatch(/^rt-/); + expect(rotated.refresh_token).not.toBe(tokens.refresh_token); + expect(rotated.scope).toBe('tag:test'); + } finally { + await context.close(); + } + }, 30_000); +}); + +async function listenOnLoopback(server: Server): Promise { + await new Promise((resolve) => server.listen(0, '127.0.0.1', resolve)); + const address = server.address(); + if (!address || typeof address === 'string') { + throw new Error('Test server did not bind to a loopback port'); + } + return `http://127.0.0.1:${address.port}`; +} + +async function closeServer(server: Server | undefined): Promise { + if (!server?.listening) { + return; + } + await new Promise((resolve, reject) => server.close((error) => (error ? reject(error) : resolve()))); +} + +function postForm(url: string, values: Record): Promise { + return fetch(url, { + method: 'POST', + headers: { 'Content-Type': 'application/x-www-form-urlencoded' }, + body: new URLSearchParams(values), + }); +} From cace7178e3c85a10c81f9ef244d455b2ab4a1af2 Mon Sep 17 00:00:00 2001 From: Xu Zhipei Date: Wed, 29 Jul 2026 23:04:24 +0800 Subject: [PATCH 2/6] fix(oauth): make refresh rotation durable --- ...th-refresh-tokens-use-rotating-families.md | 7 + docs/en/guide/advanced/authentication.md | 6 + docs/zh/guide/advanced/authentication.md | 6 + .../sdkOAuthServerProvider.refresh.test.ts | 115 +++++++++ src/auth/sdkOAuthServerProvider.ts | 72 ++++-- src/auth/sessionTypes.ts | 57 +++-- src/auth/storage/fileStorageService.test.ts | 34 +++ src/auth/storage/fileStorageService.ts | 234 +++++++++++++++++- .../storage/refreshTokenFamilyRepository.ts | 221 ++++++++++++----- src/auth/storage/sessionRepository.ts | 64 ++++- src/constants/auth.ts | 4 +- test/e2e/fixtures/oauth-refresh-worker.mjs | 37 +++ test/e2e/oauth-loopback-consent.e2e.test.ts | 107 +++++++- .../e2e/oauth-refresh-concurrency.e2e.test.ts | 126 ++++++++++ 14 files changed, 968 insertions(+), 122 deletions(-) create mode 100644 docs/adr/0011-oauth-refresh-tokens-use-rotating-families.md create mode 100644 test/e2e/fixtures/oauth-refresh-worker.mjs create mode 100644 test/e2e/oauth-refresh-concurrency.e2e.test.ts diff --git a/docs/adr/0011-oauth-refresh-tokens-use-rotating-families.md b/docs/adr/0011-oauth-refresh-tokens-use-rotating-families.md new file mode 100644 index 00000000..af072574 --- /dev/null +++ b/docs/adr/0011-oauth-refresh-tokens-use-rotating-families.md @@ -0,0 +1,7 @@ +# OAuth Refresh Tokens Use Rotating Families + +1MCP issues refresh tokens only to registered clients that request the `refresh_token` grant. Each approved authorization creates an independent, client-, resource-, and scope-bound Refresh Token Family that persists in its Runtime Scope for a fixed 30 days. Every successful refresh rotates a single-use opaque token; only token digests and family lineage are stored. + +Because public PKCE clients are not sender-constrained, reuse of any consumed family member revokes its family and every access token issued from it. There is no retry grace period: concurrent refreshes permit one success, and later use is replay. A normal refresh may narrow the new access token's scopes and leaves earlier access tokens valid until their own expiry; explicit access-token revocation remains local to that token. + +This chooses RFC 9700 rotation and strong replay containment over retry availability. A lost response or concurrent retry can force reauthorization, but the runtime never permits two valid successors or extends the family beyond its original lifetime. diff --git a/docs/en/guide/advanced/authentication.md b/docs/en/guide/advanced/authentication.md index 84d16668..7d6e3003 100644 --- a/docs/en/guide/advanced/authentication.md +++ b/docs/en/guide/advanced/authentication.md @@ -27,6 +27,12 @@ npx -y @1mcp/agent --config mcp.json --enable-auth This will activate the OAuth 2.1 endpoints and require authentication for all incoming requests. +## Refresh Token Rotation + +Registered clients receive a refresh token only when they request the `refresh_token` grant. Refresh tokens are single-use and rotate on every successful exchange. Each family has a fixed 30-day lifetime, remains bound to its original client and resource, and permits only equal or narrower scopes. + +Reusing any consumed refresh token revokes the entire family and every access token issued from it. Concurrent refreshes therefore allow exactly one successful rotation; clients must reauthorize after a replay response or a lost response that causes a retry. + ## OAuth Management Dashboard Once authentication is enabled, you can use the OAuth Management Dashboard to manage the authorization flow with your backend services. The dashboard is available at the `/oauth` endpoint of your agent's URL (e.g., `http://localhost:3050/oauth`). diff --git a/docs/zh/guide/advanced/authentication.md b/docs/zh/guide/advanced/authentication.md index f43041cf..d26cc5ab 100644 --- a/docs/zh/guide/advanced/authentication.md +++ b/docs/zh/guide/advanced/authentication.md @@ -27,6 +27,12 @@ npx -y @1mcp/agent --config mcp.json --enable-auth 这将激活 OAuth 2.1 端点，并要求对所有传入请求进行身份验证。 +## 刷新令牌轮换 + +只有请求 `refresh_token` 授权类型的已注册客户端才会收到刷新令牌。刷新令牌只能使用一次，每次成功交换都会轮换。每个令牌家族的生命周期固定为 30 天，并始终绑定到原始客户端和资源；后续请求只能保持或缩小原始作用域。 + +重复使用任何已消费的刷新令牌会撤销整个家族及其签发的所有访问令牌。因此，并发刷新只允许一次成功轮换；发生重放响应，或响应丢失后重试导致重放时，客户端必须重新授权。 + ## OAuth 管理仪表板 启用身份验证后，您可以使用 OAuth 管理仪表板来管理与后端服务的授权流程。该仪表板可在代理 URL 的 `/oauth` 端点处获得（例如，`http://localhost:3050/oauth`）。 diff --git a/src/auth/sdkOAuthServerProvider.refresh.test.ts b/src/auth/sdkOAuthServerProvider.refresh.test.ts index 8c80a321..62016cfb 100644 --- a/src/auth/sdkOAuthServerProvider.refresh.test.ts +++ b/src/auth/sdkOAuthServerProvider.refresh.test.ts @@ -13,6 +13,8 @@ import { AUTH_CONFIG } from '@src/constants.js'; import { AgentConfigManager } from '@src/core/server/agentConfig.js'; import logger from '@src/logger/logger.js'; +import { FileStorageService } from './storage/fileStorageService.js'; + import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { SDKOAuthServerProvider } from './sdkOAuthServerProvider.js'; @@ -91,6 +93,64 @@ describe('SDKOAuthServerProvider refresh token families', () => { expect(after.consumedTokenDigests).toHaveLength(1); }); + it('fails closed and cleans persisted family state after the fixed expiry', async () => { + const initial = await exchangeAuthorizationCode(provider, CLIENT); + for (const filePath of [...listFamilyFiles(tempDir), ...listLookupFiles(tempDir)]) { + const record = JSON.parse(fs.readFileSync(filePath, 'utf8')) as Record; + fs.writeFileSync(filePath, JSON.stringify({ ...record, expires: Date.now() - 1 })); + } + + await expect(provider.exchangeRefreshToken(CLIENT, initial.refresh_token!)).rejects.toBeInstanceOf( + InvalidGrantError, + ); + await expect(provider.verifyAccessToken(initial.access_token)).rejects.toThrow('Invalid or expired access token'); + expect(listFamilyFiles(tempDir)).toHaveLength(0); + expect(listLookupFiles(tempDir)).toHaveLength(0); + }); + + it('does not consume the current refresh token when access-session persistence fails', async () => { + const initial = await exchangeAuthorizationCode(provider, CLIENT); + const createSession = vi + .spyOn(provider.oauthStorage.sessionRepository, 'createRefreshFamilyAccessSession') + .mockImplementationOnce(() => { + throw new Error('session persistence failed'); + }); + + await expect(provider.exchangeRefreshToken(CLIENT, initial.refresh_token!)).rejects.toThrow( + 'session persistence failed', + ); + + createSession.mockRestore(); + await expect(provider.exchangeRefreshToken(CLIENT, initial.refresh_token!)).resolves.toMatchObject({ + refresh_token: expect.stringMatching(/^rt-/), + }); + }); + + it('does not consume the current refresh token when the family commit fails', async () => { + const initial = await exchangeAuthorizationCode(provider, CLIENT); + const originalWriteData = FileStorageService.prototype.writeDataDurable; + let failedFamilyCommit = false; + const writeData = vi.spyOn(FileStorageService.prototype, 'writeDataDurable').mockImplementation(function ( + this: FileStorageService, + filePrefix: string, + id: string, + data: Parameters[2], + ) { + if (filePrefix === AUTH_CONFIG.SERVER.REFRESH_FAMILY.FILE_PREFIX && !failedFamilyCommit) { + failedFamilyCommit = true; + throw new Error('family commit failed'); + } + return originalWriteData.call(this, filePrefix, id, data); + }); + + await expect(provider.exchangeRefreshToken(CLIENT, initial.refresh_token!)).rejects.toThrow('family commit failed'); + writeData.mockRestore(); + + await expect(provider.exchangeRefreshToken(CLIENT, initial.refresh_token!)).resolves.toMatchObject({ + refresh_token: expect.stringMatching(/^rt-/), + }); + }); + it('preserves the original scope ceiling and resource while allowing equal or narrower access', async () => { const initial = await exchangeAuthorizationCode(provider, CLIENT); @@ -138,6 +198,53 @@ describe('SDKOAuthServerProvider refresh token families', () => { ); }); + it('keeps family state bounded while older consumed-token lookups retain replay containment', async () => { + const initial = await exchangeAuthorizationCode(provider, CLIENT); + const first = await provider.exchangeRefreshToken(CLIENT, initial.refresh_token!); + const second = await provider.exchangeRefreshToken(CLIENT, first.refresh_token!); + await provider.exchangeRefreshToken(CLIENT, second.refresh_token!); + + expect(readOnlyFamily(tempDir).consumedTokenDigests).toHaveLength(1); + await expect(provider.exchangeRefreshToken(CLIENT, initial.refresh_token!)).rejects.toBeInstanceOf( + InvalidGrantError, + ); + expect(readOnlyFamily(tempDir).status).toBe('revoked'); + }); + + it('isolates refresh families by Runtime Scope even when storage is shared', async () => { + const initial = await exchangeAuthorizationCode(provider, CLIENT); + const otherScopeProvider = new SDKOAuthServerProvider(tempDir, 'runtime-scope-b'); + try { + await expect( + otherScopeProvider.exchangeRefreshToken(CLIENT, initial.refresh_token!), + ).rejects.toBeInstanceOf(InvalidGrantError); + await expect(provider.exchangeRefreshToken(CLIENT, initial.refresh_token!)).resolves.toMatchObject({ + refresh_token: expect.stringMatching(/^rt-/), + }); + } finally { + otherScopeProvider.shutdown(); + } + }); + + it('propagates replay-cascade deletion failures while revoked family state blocks residual sessions', async () => { + const initial = await exchangeAuthorizationCode(provider, CLIENT); + const rotated = await provider.exchangeRefreshToken(CLIENT, initial.refresh_token!); + const originalUnlinkSync = fs.unlinkSync; + const unlink = vi.spyOn(fs, 'unlinkSync').mockImplementation((filePath) => { + if (String(filePath).includes(AUTH_CONFIG.SERVER.SESSION.FILE_PREFIX)) { + throw new Error('session deletion failed'); + } + return originalUnlinkSync(filePath); + }); + + await expect(provider.exchangeRefreshToken(CLIENT, initial.refresh_token!)).rejects.toThrow( + 'Failed to revoke access sessions for refresh token family', + ); + unlink.mockRestore(); + await expect(provider.verifyAccessToken(initial.access_token)).rejects.toThrow('Invalid or expired access token'); + await expect(provider.verifyAccessToken(rotated.access_token)).rejects.toThrow('Invalid or expired access token'); + }); + it('does not mutate a rightful family when another client presents its refresh token', async () => { const initial = await exchangeAuthorizationCode(provider, CLIENT); @@ -208,6 +315,14 @@ function listFamilyFiles(tempDir: string): string[] { .map((fileName) => path.join(serverDir, fileName)); } +function listLookupFiles(tempDir: string): string[] { + const serverDir = path.join(tempDir, AUTH_CONFIG.SERVER.STORAGE.DIR, AUTH_CONFIG.SERVER.SESSION.SUBDIR); + return fs + .readdirSync(serverDir) + .filter((fileName) => fileName.startsWith(AUTH_CONFIG.SERVER.REFRESH_FAMILY.LOOKUP_FILE_PREFIX)) + .map((fileName) => path.join(serverDir, fileName)); +} + function readOnlyFamily(tempDir: string): Record { const familyFiles = listFamilyFiles(tempDir); expect(familyFiles).toHaveLength(1); diff --git a/src/auth/sdkOAuthServerProvider.ts b/src/auth/sdkOAuthServerProvider.ts index c97a92ac..9a7656ef 100644 --- a/src/auth/sdkOAuthServerProvider.ts +++ b/src/auth/sdkOAuthServerProvider.ts @@ -394,23 +394,32 @@ export class SDKOAuthServerProvider implements OAuthServerProvider { const ttlMs = this.configManager.get('auth').oauthTokenTtlMs; const refreshFamily = client.grant_types?.includes('refresh_token') - ? this.oauthStorage.refreshTokenFamilyRepository.create( + ? await this.oauthStorage.refreshTokenFamilyRepository.create( client.client_id, codeData.scopes, codeData.resource || '', tokenId, + (familyId) => + this.oauthStorage.sessionRepository.createRefreshFamilyAccessSession({ + tokenId, + clientId: client.client_id, + resource: codeData.resource || '', + scopes: codeData.scopes, + ttlMs, + familyId, + }), ) : undefined; - // Store session for token validation - this.oauthStorage.sessionRepository.createWithId( - tokenId, - client.client_id, - codeData.resource || '', - codeData.scopes, - ttlMs, - refreshFamily?.family.familyId, - ); + if (!refreshFamily) { + this.oauthStorage.sessionRepository.createWithId( + tokenId, + client.client_id, + codeData.resource || '', + codeData.scopes, + ttlMs, + ); + } const tokens: OAuthTokens = { access_token: accessToken, @@ -446,7 +455,7 @@ export class SDKOAuthServerProvider implements OAuthServerProvider { const tokenState = repository.getTokenState(family, refreshToken); if (tokenState === 'consumed') { - const replay = repository.consume(refreshToken, client.client_id, randomUUID()); + const replay = await repository.consume(refreshToken, client.client_id, randomUUID(), () => undefined); if (replay.status === 'replay') { this.revokeFamilyAccessTokens(replay.family.accessTokenIds); } @@ -466,7 +475,17 @@ export class SDKOAuthServerProvider implements OAuthServerProvider { } const tokenId = randomUUID(); - const rotation = repository.consume(refreshToken, client.client_id, tokenId); + const ttlMs = this.configManager.get('auth').oauthTokenTtlMs; + const rotation = await repository.consume(refreshToken, client.client_id, tokenId, (familyId) => + this.oauthStorage.sessionRepository.createRefreshFamilyAccessSession({ + tokenId, + clientId: client.client_id, + resource: family.resource, + scopes: requestedScopes, + ttlMs, + familyId, + }), + ); if (rotation.status === 'replay') { this.revokeFamilyAccessTokens(rotation.family.accessTokenIds); throw new InvalidGrantError('Refresh token replay detected'); @@ -475,16 +494,6 @@ export class SDKOAuthServerProvider implements OAuthServerProvider { throw new InvalidGrantError('Invalid refresh token'); } - const ttlMs = this.configManager.get('auth').oauthTokenTtlMs; - this.oauthStorage.sessionRepository.createWithId( - tokenId, - client.client_id, - family.resource, - requestedScopes, - ttlMs, - family.familyId, - ); - return { access_token: AUTH_CONFIG.SERVER.TOKEN.ID_PREFIX + tokenId, token_type: 'Bearer', @@ -524,6 +533,13 @@ export class SDKOAuthServerProvider implements OAuthServerProvider { throw new Error('Invalid or expired access token'); } + if (sessionData.refreshFamilyId) { + const family = this.oauthStorage.refreshTokenFamilyRepository.findById(sessionData.refreshFamilyId); + if (!family || family.status !== 'active' || !family.accessTokenIds.includes(tokenId)) { + throw new Error('Invalid or expired access token'); + } + } + return { token, clientId: sessionData.clientId, @@ -543,7 +559,7 @@ export class SDKOAuthServerProvider implements OAuthServerProvider { const refreshFamily = this.oauthStorage.refreshTokenFamilyRepository.findByToken(token); if (refreshFamily) { - const revokedFamily = this.oauthStorage.refreshTokenFamilyRepository.revokeForClient( + const revokedFamily = await this.oauthStorage.refreshTokenFamilyRepository.revokeForClient( refreshFamily, client.client_id, ); @@ -570,8 +586,16 @@ export class SDKOAuthServerProvider implements OAuthServerProvider { } private revokeFamilyAccessTokens(accessTokenIds: string[]): void { + const failures: unknown[] = []; for (const accessTokenId of accessTokenIds) { - this.oauthStorage.sessionRepository.delete(AUTH_CONFIG.SERVER.SESSION.ID_PREFIX + accessTokenId); + try { + this.oauthStorage.sessionRepository.delete(AUTH_CONFIG.SERVER.SESSION.ID_PREFIX + accessTokenId); + } catch (error) { + failures.push(error); + } + } + if (failures.length > 0) { + throw new AggregateError(failures, 'Failed to revoke access sessions for refresh token family'); } } diff --git a/src/auth/sessionTypes.ts b/src/auth/sessionTypes.ts index 7302ce24..019f7d1f 100644 --- a/src/auth/sessionTypes.ts +++ b/src/auth/sessionTypes.ts @@ -1,5 +1,6 @@ // Shared session types for server and client session managers import { OAuthClientInformationFull } from '@modelcontextprotocol/sdk/shared/auth.js'; +import { z } from 'zod'; import { ContextNamespace, EnvironmentContext, UserContext } from '@src/types/context.js'; @@ -11,25 +12,45 @@ export interface ExpirableData { createdAt: number; } -export interface SessionData extends ExpirableData { - clientId: string; - resource: string; - scopes: string[]; - refreshFamilyId?: string; -} +const ExpirableDataShape = { + expires: z.number().finite(), + createdAt: z.number().finite(), +}; -export interface RefreshTokenFamilyData extends ExpirableData { - familyId: string; - runtimeScopeId: string; - clientId: string; - scopeCeiling: string[]; - resource: string; - currentTokenDigest: string; - consumedTokenDigests: string[]; - accessTokenIds: string[]; - status: 'active' | 'revoked'; - revokedAt?: number; -} +export const SessionDataSchema = z.object({ + ...ExpirableDataShape, + clientId: z.string().min(1), + resource: z.string(), + scopes: z.array(z.string()), + refreshFamilyId: z.string().optional(), +}); +export type SessionData = z.infer; + +const RefreshTokenDigestSchema = z.string().regex(/^[a-f0-9]{64}$/); + +export const RefreshTokenFamilyDataSchema = z.object({ + ...ExpirableDataShape, + familyId: z.string().min(1), + runtimeScopeId: z.string().min(1), + clientId: z.string().min(1), + scopeCeiling: z.array(z.string()), + resource: z.string(), + currentTokenDigest: RefreshTokenDigestSchema, + consumedTokenDigests: z.array(RefreshTokenDigestSchema), + accessTokenIds: z.array(z.string().uuid()), + status: z.enum(['active', 'revoked']), + revokedAt: z.number().finite().optional(), +}); +export type RefreshTokenFamilyData = z.infer; + +export const RefreshTokenLookupDataSchema = z.object({ + ...ExpirableDataShape, + familyId: z.string().min(1), + runtimeScopeId: z.string().min(1), + tokenDigest: RefreshTokenDigestSchema, + state: z.enum(['current', 'consumed']), +}); +export type RefreshTokenLookupData = z.infer; export interface ClientData extends ExpirableData, OAuthClientInformationFull {} diff --git a/src/auth/storage/fileStorageService.test.ts b/src/auth/storage/fileStorageService.test.ts index f7618515..1a6165cc 100644 --- a/src/auth/storage/fileStorageService.test.ts +++ b/src/auth/storage/fileStorageService.test.ts @@ -38,6 +38,7 @@ describe('FileStorageService', () => { afterEach(() => { service.shutdown(); + vi.restoreAllMocks(); // Clean up temp directory if (fs.existsSync(tempDir)) { fs.rmSync(tempDir, { recursive: true, force: true }); @@ -84,6 +85,25 @@ describe('FileStorageService', () => { expect(retrieved).toEqual(testData); }); + it('preserves the previous record when a replacement write fails after truncation', () => { + service.writeData(testPrefix, testId, testData); + const targetPath = service.getFilePath(testPrefix, testId); + const originalWriteFileSync = fs.writeFileSync; + vi.spyOn(fs, 'writeFileSync').mockImplementation((file, data, options) => { + originalWriteFileSync(file, '', options); + throw new Error(`simulated crash while writing ${String(file)}`); + }); + + expect(() => + service.writeDataDurable(testPrefix, testId, { + ...testData, + value: 'replacement value', + }), + ).toThrow('simulated crash'); + + expect(JSON.parse(fs.readFileSync(targetPath, 'utf8'))).toEqual(testData); + }); + it('should return null for non-existent data', () => { const result = service.readData(testPrefix, 'nonexistent'); expect(result).toBeNull(); @@ -110,6 +130,20 @@ describe('FileStorageService', () => { }); }); + describe('Exclusive storage locks', () => { + it('reclaims a lock left by a crashed process', async () => { + const lockPath = path.join(service.getStorageDir(), '.refresh-test.lock'); + fs.mkdirSync(lockPath); + fs.writeFileSync( + path.join(lockPath, 'owner.json'), + JSON.stringify({ operationId: 'abandoned-operation', pid: 2_147_483_647, createdAt: Date.now() }), + ); + + await expect(service.withExclusiveLock('refresh-test', () => 'acquired')).resolves.toBe('acquired'); + expect(fs.existsSync(lockPath)).toBe(false); + }); + }); + describe('Path Security', () => { it('should prevent path traversal attacks', () => { const maliciousId = '../../../etc/passwd'; diff --git a/src/auth/storage/fileStorageService.ts b/src/auth/storage/fileStorageService.ts index abb516b1..6ec3333a 100644 --- a/src/auth/storage/fileStorageService.ts +++ b/src/auth/storage/fileStorageService.ts @@ -1,9 +1,17 @@ import fs from 'fs'; import path from 'path'; +import { randomUUID } from 'node:crypto'; import { ExpirableData } from '@src/auth/sessionTypes.js'; import { AUTH_CONFIG, FILE_PREFIX_MAPPING, getGlobalConfigDir, STORAGE_SUBDIRS } from '@src/constants.js'; import logger from '@src/logger/logger.js'; +import { z, type ZodType } from 'zod'; + +const StorageLockOwnerSchema = z.object({ + operationId: z.string().min(1), + pid: z.number().int().positive(), + createdAt: z.number().finite(), +}); /** * Generic file storage service with unified cleanup for all expirable data types. @@ -216,6 +224,11 @@ export class FileStorageService { return false; } + if (filePrefix === AUTH_CONFIG.SERVER.REFRESH_FAMILY.LOOKUP_FILE_PREFIX) { + const { LOOKUP_ID_PREFIX } = AUTH_CONFIG.SERVER.REFRESH_FAMILY; + return id.startsWith(LOOKUP_ID_PREFIX) && /^[a-f0-9]{64}$/.test(id.slice(LOOKUP_ID_PREFIX.length)); + } + // Check for valid server-side prefix const serverPrefixes = [ AUTH_CONFIG.SERVER.SESSION.ID_PREFIX, @@ -281,11 +294,43 @@ export class FileStorageService { } } + /** + * Atomically replaces a record and flushes it before returning. + */ + writeDataDurable(filePrefix: string, id: string, data: T): void { + let temporaryPath: string | undefined; + try { + const filePath = this.getFilePath(filePrefix, id); + temporaryPath = `${filePath}.${process.pid}.${randomUUID()}.tmp`; + const fileDescriptor = fs.openSync(temporaryPath, 'wx', 0o600); + try { + fs.writeFileSync(fileDescriptor, JSON.stringify(data, null, 2)); + fs.fsyncSync(fileDescriptor); + } finally { + fs.closeSync(fileDescriptor); + } + fs.renameSync(temporaryPath, filePath); + temporaryPath = undefined; + this.flushStorageDirectory(); + logger.debug(`Wrote data to ${filePath}`); + } catch (error) { + if (temporaryPath) { + try { + fs.unlinkSync(temporaryPath); + } catch { + // A later startup cleanup removes abandoned temporary files. + } + } + logger.error(`Failed to write data for ${id}: ${error}`); + throw error; + } + } + /** * Reads data from a file with the specified prefix and ID * Returns null if file doesn't exist or data is expired */ - readData(filePrefix: string, id: string): T | null { + readData(filePrefix: string, id: string, schema?: ZodType): T | null { if (!this.isValidId(id, filePrefix)) { logger.warn(`Rejected readData with invalid ID: ${id}`); return null; @@ -298,7 +343,8 @@ export class FileStorageService { } const data = fs.readFileSync(filePath, 'utf8'); - const parsedData: T = JSON.parse(data) as T; + const parsed: unknown = JSON.parse(data); + const parsedData = schema ? schema.parse(parsed) : (parsed as T); // Check if data is expired if (parsedData.expires < Date.now()) { @@ -332,7 +378,36 @@ export class FileStorageService { return false; } catch (error) { logger.error(`Failed to delete data for ${id}: ${error}`); - return false; + throw error; + } + } + + /** + * Runs a storage transition under an inter-process lock. + * + * Lock ownership is recorded so a process that dies while holding the lock + * cannot block the Runtime Scope permanently. + */ + async withExclusiveLock(lockName: string, operation: () => Promise | T): Promise { + if (!/^[a-z0-9-]+$/.test(lockName)) { + throw new Error(`Invalid storage lock name: ${lockName}`); + } + + const lockPath = path.join(this.storageDir, `.${lockName}.lock`); + const operationId = randomUUID(); + const deadline = Date.now() + 10_000; + + while (!this.tryAcquireLock(lockPath, operationId)) { + if (Date.now() >= deadline) { + throw new Error(`Timed out acquiring storage lock: ${lockName}`); + } + await new Promise((resolve) => setTimeout(resolve, 5 + Math.floor(Math.random() * 10))); + } + + try { + return await operation(); + } finally { + this.releaseLock(lockPath, operationId); } } @@ -358,6 +433,20 @@ export class FileStorageService { let cleanedCount = 0; for (const file of files) { + if (file.includes('.json.') && file.endsWith('.tmp')) { + const temporaryPath = path.join(this.storageDir, file); + try { + const ageMs = Date.now() - fs.statSync(temporaryPath).mtimeMs; + if (ageMs >= 60_000) { + fs.unlinkSync(temporaryPath); + cleanedCount++; + } + } catch (error) { + logger.warn(`Failed to clean temporary file ${file}: ${error}`); + } + continue; + } + if (file.endsWith(AUTH_CONFIG.SERVER.STORAGE.FILE_EXTENSION)) { const filePath = path.join(this.storageDir, file); try { @@ -440,4 +529,143 @@ export class FileStorageService { logger.info('FileStorageService cleanup interval stopped'); } } + + private flushStorageDirectory(): void { + try { + const directoryDescriptor = fs.openSync(this.storageDir, 'r'); + try { + fs.fsyncSync(directoryDescriptor); + } finally { + fs.closeSync(directoryDescriptor); + } + } catch (error) { + if ( + process.platform === 'win32' && + error instanceof Error && + 'code' in error && + ['EINVAL', 'ENOTSUP', 'EPERM'].includes(String(error.code)) + ) { + return; + } + throw error; + } + } + + private tryAcquireLock(lockPath: string, operationId: string): boolean { + try { + fs.mkdirSync(lockPath, { mode: 0o700 }); + } catch (error) { + if (!isFileExistsError(error)) { + throw error; + } + this.reclaimAbandonedLock(lockPath); + return false; + } + + try { + fs.writeFileSync( + path.join(lockPath, 'owner.json'), + JSON.stringify({ operationId, pid: process.pid, createdAt: Date.now() }), + { mode: 0o600 }, + ); + this.flushStorageDirectory(); + return true; + } catch (error) { + try { + removeLockDirectory(lockPath); + } catch (cleanupError) { + throw new AggregateError([error, cleanupError], `Failed to initialize storage lock: ${lockPath}`); + } + throw error; + } + } + + private reclaimAbandonedLock(lockPath: string): void { + const owner = this.readLockOwner(lockPath); + if (owner && isProcessAlive(owner.pid)) { + return; + } + + if (!owner) { + try { + if (Date.now() - fs.statSync(lockPath).mtimeMs < 1_000) { + return; + } + } catch { + return; + } + } + + const observedOperationId = owner?.operationId; + const tombstonePath = `${lockPath}.${randomUUID()}.stale`; + try { + fs.renameSync(lockPath, tombstonePath); + } catch { + return; + } + + const movedOwner = this.readLockOwner(tombstonePath); + if (observedOperationId && movedOwner?.operationId !== observedOperationId) { + try { + fs.renameSync(tombstonePath, lockPath); + } catch { + // Another contender will reconcile the surviving generation. + } + return; + } + + removeLockDirectory(tombstonePath); + } + + private releaseLock(lockPath: string, operationId: string): void { + const owner = this.readLockOwner(lockPath); + if (owner?.operationId !== operationId) { + logger.error(`Storage lock ownership changed before release: ${lockPath}`); + return; + } + + const tombstonePath = `${lockPath}.${operationId}.releasing`; + try { + fs.renameSync(lockPath, tombstonePath); + removeLockDirectory(tombstonePath); + this.flushStorageDirectory(); + } catch (error) { + logger.error(`Failed to release storage lock ${lockPath}: ${error}`); + throw error; + } + } + + private readLockOwner(lockPath: string): { operationId: string; pid: number } | null { + try { + const value: unknown = JSON.parse(fs.readFileSync(path.join(lockPath, 'owner.json'), 'utf8')); + const result = StorageLockOwnerSchema.safeParse(value); + return result.success ? { operationId: result.data.operationId, pid: result.data.pid } : null; + } catch { + return null; + } + } +} + +function isFileExistsError(error: unknown): error is Error & { code: string } { + return error instanceof Error && 'code' in error && error.code === 'EEXIST'; +} + +function isProcessAlive(pid: number): boolean { + try { + process.kill(pid, 0); + return true; + } catch (error) { + return error instanceof Error && 'code' in error && error.code === 'EPERM'; + } +} + +function removeLockDirectory(lockPath: string): void { + try { + fs.unlinkSync(path.join(lockPath, 'owner.json')); + } catch (error) { + if (!(error instanceof Error && 'code' in error && error.code === 'ENOENT')) { + throw error; + } + } + fs.rmdirSync(lockPath); } diff --git a/src/auth/storage/refreshTokenFamilyRepository.ts b/src/auth/storage/refreshTokenFamilyRepository.ts index 3f153898..773ad172 100644 --- a/src/auth/storage/refreshTokenFamilyRepository.ts +++ b/src/auth/storage/refreshTokenFamilyRepository.ts @@ -1,6 +1,13 @@ import { createHash, randomBytes, randomUUID, timingSafeEqual } from 'node:crypto'; -import { RefreshTokenFamilyData } from '@src/auth/sessionTypes.js'; +import { + RefreshTokenFamilyData, + RefreshTokenFamilyDataSchema, + RefreshTokenLookupData, + RefreshTokenLookupDataSchema, + SessionData, + SessionDataSchema, +} from '@src/auth/sessionTypes.js'; import { AUTH_CONFIG } from '@src/constants.js'; import { FileStorageService } from './fileStorageService.js'; @@ -11,45 +18,60 @@ export type RefreshTokenConsumptionResult = | { status: 'invalid' } | { status: 'client_mismatch' }; +const REFRESH_FAMILY_LOCK = 'refresh-token-families'; + export class RefreshTokenFamilyRepository { constructor( private readonly storage: FileStorageService, private readonly runtimeScopeId: string, ) {} - create( + async create( clientId: string, scopeCeiling: string[], resource: string, accessTokenId: string, - ): { + persistAccessSession: (familyId: string) => void, + ): Promise<{ family: RefreshTokenFamilyData; refreshToken: string; - } { - const familyId = AUTH_CONFIG.SERVER.REFRESH_FAMILY.ID_PREFIX + randomUUID(); - const refreshToken = createRefreshToken(); - const now = Date.now(); - const family: RefreshTokenFamilyData = { - familyId, - runtimeScopeId: this.runtimeScopeId, - clientId, - scopeCeiling: [...scopeCeiling], - resource, - currentTokenDigest: digestRefreshToken(refreshToken), - consumedTokenDigests: [], - accessTokenIds: [accessTokenId], - status: 'active', - createdAt: now, - expires: now + AUTH_CONFIG.SERVER.REFRESH_FAMILY.TTL_MS, - }; + }> { + return this.storage.withExclusiveLock(REFRESH_FAMILY_LOCK, () => { + const familyId = AUTH_CONFIG.SERVER.REFRESH_FAMILY.ID_PREFIX + randomUUID(); + const refreshToken = createRefreshToken(); + const now = Date.now(); + const family: RefreshTokenFamilyData = { + familyId, + runtimeScopeId: this.runtimeScopeId, + clientId, + scopeCeiling: [...scopeCeiling], + resource, + currentTokenDigest: digestRefreshToken(refreshToken), + consumedTokenDigests: [], + accessTokenIds: [accessTokenId], + status: 'active', + createdAt: now, + expires: now + AUTH_CONFIG.SERVER.REFRESH_FAMILY.TTL_MS, + }; - this.save(family); - return { family, refreshToken }; + persistAccessSession(familyId); + this.saveLookup(family, family.currentTokenDigest, 'current'); + this.save(family); + return { family, refreshToken }; + }); } findByToken(refreshToken: string): RefreshTokenFamilyData | null { - const digest = digestRefreshToken(refreshToken); - return this.list().find((family) => tokenDigestMatches(family, digest)) ?? null; + return this.locateByDigest(digestRefreshToken(refreshToken))?.family ?? null; + } + + findById(familyId: string): RefreshTokenFamilyData | null { + const family = this.storage.readData( + AUTH_CONFIG.SERVER.REFRESH_FAMILY.FILE_PREFIX, + familyId, + RefreshTokenFamilyDataSchema, + ); + return family?.runtimeScopeId === this.runtimeScopeId ? family : null; } getTokenState(family: RefreshTokenFamilyData, refreshToken: string): 'current' | 'consumed' | 'unknown' { @@ -60,45 +82,75 @@ export class RefreshTokenFamilyRepository { if (family.consumedTokenDigests.some((consumed) => safeDigestEqual(consumed, digest))) { return 'consumed'; } - return 'unknown'; + + const lookup = this.readLookup(digest); + return lookup?.familyId === family.familyId ? lookup.state : 'unknown'; } - consume(refreshToken: string, clientId: string, accessTokenId: string): RefreshTokenConsumptionResult { + async consume( + refreshToken: string, + clientId: string, + accessTokenId: string, + persistAccessSession: (familyId: string) => void, + ): Promise { const digest = digestRefreshToken(refreshToken); - const family = this.list().find((candidate) => tokenDigestMatches(candidate, digest)); - if (!family || family.runtimeScopeId !== this.runtimeScopeId) { - return { status: 'invalid' }; - } + return this.storage.withExclusiveLock(REFRESH_FAMILY_LOCK, () => { + const located = this.locateByDigest(digest); + if (!located || located.family.runtimeScopeId !== this.runtimeScopeId) { + return { status: 'invalid' }; + } - if (family.clientId !== clientId) { - return { status: 'client_mismatch' }; - } + const { family } = located; + if (family.clientId !== clientId) { + return { status: 'client_mismatch' }; + } - if (family.consumedTokenDigests.some((consumed) => safeDigestEqual(consumed, digest))) { - const revokedFamily = this.revoke(family); - return { status: 'replay', family: revokedFamily }; - } + const tokenState = safeDigestEqual(family.currentTokenDigest, digest) + ? 'current' + : family.consumedTokenDigests.some((consumed) => safeDigestEqual(consumed, digest)) || + located.lookup?.state === 'consumed' + ? 'consumed' + : 'unknown'; - if (family.status !== 'active' || !safeDigestEqual(family.currentTokenDigest, digest)) { - return { status: 'invalid' }; - } + if (tokenState === 'consumed') { + const revokedFamily = this.revoke(family); + return { status: 'replay', family: revokedFamily }; + } - const nextRefreshToken = createRefreshToken(); - const rotatedFamily: RefreshTokenFamilyData = { - ...family, - currentTokenDigest: digestRefreshToken(nextRefreshToken), - consumedTokenDigests: [...family.consumedTokenDigests, family.currentTokenDigest], - accessTokenIds: [...family.accessTokenIds, accessTokenId], - }; - this.save(rotatedFamily); - return { status: 'rotated', family: rotatedFamily, refreshToken: nextRefreshToken }; + if (tokenState !== 'current' || family.status !== 'active') { + return { status: 'invalid' }; + } + + const nextRefreshToken = createRefreshToken(); + const nextDigest = digestRefreshToken(nextRefreshToken); + const activeAccessTokenIds = family.accessTokenIds.filter((tokenId) => this.isAccessSessionActive(tokenId)); + const rotatedFamily: RefreshTokenFamilyData = { + ...family, + currentTokenDigest: nextDigest, + // Older consumed digests remain replay-detectable through lookup records. + consumedTokenDigests: [family.currentTokenDigest], + accessTokenIds: [...activeAccessTokenIds, accessTokenId], + }; + + persistAccessSession(family.familyId); + for (const historicDigest of family.consumedTokenDigests) { + this.saveLookup(family, historicDigest, 'consumed'); + } + this.saveLookup(family, family.currentTokenDigest, 'consumed'); + this.saveLookup(family, nextDigest, 'current'); + this.save(rotatedFamily); + return { status: 'rotated', family: rotatedFamily, refreshToken: nextRefreshToken }; + }); } - revokeForClient(family: RefreshTokenFamilyData, clientId: string): RefreshTokenFamilyData | null { - if (family.runtimeScopeId !== this.runtimeScopeId || family.clientId !== clientId) { - return null; - } - return this.revoke(family); + async revokeForClient(family: RefreshTokenFamilyData, clientId: string): Promise { + return this.storage.withExclusiveLock(REFRESH_FAMILY_LOCK, () => { + const currentFamily = this.findById(family.familyId); + if (!currentFamily || currentFamily.clientId !== clientId) { + return null; + } + return this.revoke(currentFamily); + }); } private revoke(family: RefreshTokenFamilyData): RefreshTokenFamilyData { @@ -114,6 +166,54 @@ export class RefreshTokenFamilyRepository { return revokedFamily; } + private locateByDigest( + digest: string, + ): { family: RefreshTokenFamilyData; lookup?: RefreshTokenLookupData } | null { + const lookup = this.readLookup(digest); + if (lookup?.runtimeScopeId === this.runtimeScopeId) { + const family = this.findById(lookup.familyId); + if (family) { + return { family, lookup }; + } + } + + const family = this.list().find((candidate) => tokenDigestMatches(candidate, digest)); + return family ? { family } : null; + } + + private readLookup(digest: string): RefreshTokenLookupData | null { + return this.storage.readData( + AUTH_CONFIG.SERVER.REFRESH_FAMILY.LOOKUP_FILE_PREFIX, + lookupId(digest), + RefreshTokenLookupDataSchema, + ); + } + + private saveLookup( + family: RefreshTokenFamilyData, + tokenDigest: string, + state: RefreshTokenLookupData['state'], + ): void { + this.storage.writeDataDurable(AUTH_CONFIG.SERVER.REFRESH_FAMILY.LOOKUP_FILE_PREFIX, lookupId(tokenDigest), { + familyId: family.familyId, + runtimeScopeId: family.runtimeScopeId, + tokenDigest, + state, + createdAt: family.createdAt, + expires: family.expires, + }); + } + + private isAccessSessionActive(accessTokenId: string): boolean { + return ( + this.storage.readData( + AUTH_CONFIG.SERVER.SESSION.FILE_PREFIX, + AUTH_CONFIG.SERVER.SESSION.ID_PREFIX + accessTokenId, + SessionDataSchema, + ) !== null + ); + } + private list(): RefreshTokenFamilyData[] { const { FILE_PREFIX, ID_PREFIX } = AUTH_CONFIG.SERVER.REFRESH_FAMILY; const extension = AUTH_CONFIG.SERVER.STORAGE.FILE_EXTENSION; @@ -121,12 +221,15 @@ export class RefreshTokenFamilyRepository { .listFiles(FILE_PREFIX) .map((fileName) => fileName.slice(FILE_PREFIX.length, -extension.length)) .filter((familyId) => familyId.startsWith(ID_PREFIX)) - .map((familyId) => this.storage.readData(FILE_PREFIX, familyId)) - .filter((family): family is RefreshTokenFamilyData => family !== null); + .map((familyId) => this.storage.readData(FILE_PREFIX, familyId, RefreshTokenFamilyDataSchema)) + .filter( + (family): family is RefreshTokenFamilyData => + family !== null && family.runtimeScopeId === this.runtimeScopeId, + ); } private save(family: RefreshTokenFamilyData): void { - this.storage.writeData(AUTH_CONFIG.SERVER.REFRESH_FAMILY.FILE_PREFIX, family.familyId, family); + this.storage.writeDataDurable(AUTH_CONFIG.SERVER.REFRESH_FAMILY.FILE_PREFIX, family.familyId, family); } } @@ -138,6 +241,10 @@ function createRefreshToken(): string { return AUTH_CONFIG.SERVER.REFRESH_FAMILY.TOKEN_PREFIX + randomBytes(32).toString('base64url'); } +function lookupId(digest: string): string { + return AUTH_CONFIG.SERVER.REFRESH_FAMILY.LOOKUP_ID_PREFIX + digest; +} + function tokenDigestMatches(family: RefreshTokenFamilyData, digest: string): boolean { return ( safeDigestEqual(family.currentTokenDigest, digest) || diff --git a/src/auth/storage/sessionRepository.ts b/src/auth/storage/sessionRepository.ts index 2acaadf3..21d8a3cd 100644 --- a/src/auth/storage/sessionRepository.ts +++ b/src/auth/storage/sessionRepository.ts @@ -1,11 +1,20 @@ import { randomUUID } from 'node:crypto'; -import { SessionData } from '@src/auth/sessionTypes.js'; +import { SessionData, SessionDataSchema } from '@src/auth/sessionTypes.js'; import { AUTH_CONFIG } from '@src/constants.js'; import logger from '@src/logger/logger.js'; import { FileStorageService } from './fileStorageService.js'; +export interface RefreshFamilyAccessSessionInput { + tokenId: string; + clientId: string; + resource: string; + scopes: string[]; + ttlMs: number; + familyId: string; +} + /** * Repository for session operations * @@ -44,18 +53,53 @@ export class SessionRepository { ttlMs: number, refreshFamilyId?: string, ): string { - const sessionId = AUTH_CONFIG.SERVER.SESSION.ID_PREFIX + tokenId; + return this.persistWithId({ tokenId, clientId, resource, scopes, ttlMs, refreshFamilyId }, false); + } + + /** + * Persists a refresh-family access session before the family commit point. + */ + createRefreshFamilyAccessSession(input: RefreshFamilyAccessSessionInput): string { + return this.persistWithId( + { + tokenId: input.tokenId, + clientId: input.clientId, + resource: input.resource, + scopes: input.scopes, + ttlMs: input.ttlMs, + refreshFamilyId: input.familyId, + }, + true, + ); + } + + private persistWithId( + input: { + tokenId: string; + clientId: string; + resource: string; + scopes: string[]; + ttlMs: number; + refreshFamilyId?: string; + }, + durable: boolean, + ): string { + const sessionId = AUTH_CONFIG.SERVER.SESSION.ID_PREFIX + input.tokenId; const sessionData: SessionData = { - clientId, - resource, - scopes, - refreshFamilyId, - expires: Date.now() + ttlMs, + clientId: input.clientId, + resource: input.resource, + scopes: input.scopes, + refreshFamilyId: input.refreshFamilyId, + expires: Date.now() + input.ttlMs, createdAt: Date.now(), }; - this.storage.writeData(AUTH_CONFIG.SERVER.SESSION.FILE_PREFIX, sessionId, sessionData); - logger.info(`Created session with ID: ${sessionId} for client: ${clientId}`); + if (durable) { + this.storage.writeDataDurable(AUTH_CONFIG.SERVER.SESSION.FILE_PREFIX, sessionId, sessionData); + } else { + this.storage.writeData(AUTH_CONFIG.SERVER.SESSION.FILE_PREFIX, sessionId, sessionData); + } + logger.info(`Created session with ID: ${sessionId} for client: ${input.clientId}`); return sessionId; } @@ -63,7 +107,7 @@ export class SessionRepository { * Retrieves a session by ID */ get(sessionId: string): SessionData | null { - return this.storage.readData(AUTH_CONFIG.SERVER.SESSION.FILE_PREFIX, sessionId); + return this.storage.readData(AUTH_CONFIG.SERVER.SESSION.FILE_PREFIX, sessionId, SessionDataSchema); } /** diff --git a/src/constants/auth.ts b/src/constants/auth.ts index 8e7e9ce7..d6b21bba 100644 --- a/src/constants/auth.ts +++ b/src/constants/auth.ts @@ -49,6 +49,8 @@ export const AUTH_CONFIG = { TTL_MS: 30 * 24 * 60 * 60 * 1000, // 30 days, fixed at family creation ID_PREFIX: 'rf-', FILE_PREFIX: 'refresh_family_', + LOOKUP_ID_PREFIX: 'rtl-', + LOOKUP_FILE_PREFIX: 'refresh_lookup_', TOKEN_PREFIX: 'rt-', SUBDIR: 'server', }, @@ -110,7 +112,7 @@ export const STORAGE_SUBDIRS = { // File prefix mapping for migration logic export const FILE_PREFIX_MAPPING = { - SERVER: ['session_', 'auth_code_', 'auth_request_', 'refresh_family_'], + SERVER: ['session_', 'auth_code_', 'auth_request_', 'refresh_family_', 'refresh_lookup_'], CLIENT: ['oauth_', 'cli_', 'tok_', 'ver_', 'sta_'], TRANSPORT: ['streamable_session_'], } as const; diff --git a/test/e2e/fixtures/oauth-refresh-worker.mjs b/test/e2e/fixtures/oauth-refresh-worker.mjs new file mode 100644 index 00000000..e8a9912c --- /dev/null +++ b/test/e2e/fixtures/oauth-refresh-worker.mjs @@ -0,0 +1,37 @@ +import fs from 'node:fs'; +import { randomUUID } from 'node:crypto'; + +import { AUTH_CONFIG } from '../../../build/constants.js'; +import { FileStorageService } from '../../../build/auth/storage/fileStorageService.js'; +import { RefreshTokenFamilyRepository } from '../../../build/auth/storage/refreshTokenFamilyRepository.js'; +import { SessionRepository } from '../../../build/auth/storage/sessionRepository.js'; + +const [storageDir, runtimeScopeId, refreshToken, clientId, markerPath, releasePath] = process.argv.slice(2); +const storage = new FileStorageService(storageDir, AUTH_CONFIG.SERVER.SESSION.SUBDIR); +const sessions = new SessionRepository(storage); +const repository = new RefreshTokenFamilyRepository(storage, runtimeScopeId); +const accessTokenId = randomUUID(); + +try { + const result = await repository.consume(refreshToken, clientId, accessTokenId, (familyId) => { + fs.writeFileSync(markerPath, String(process.pid)); + if (releasePath) { + const deadline = Date.now() + 10_000; + while (!fs.existsSync(releasePath) && Date.now() < deadline) { + Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, 10); + } + if (!fs.existsSync(releasePath)) { + throw new Error('Timed out waiting for transition release'); + } + } + sessions.createWithId(accessTokenId, clientId, 'https://resource.example/mcp', ['tag:alpha'], 60_000, familyId); + }); + process.stdout.write(`RESULT ${JSON.stringify({ status: result.status })}\n`); +} catch (error) { + process.stdout.write( + `RESULT ${JSON.stringify({ status: 'error', message: error instanceof Error ? error.message : String(error) })}\n`, + ); + process.exitCode = 1; +} finally { + storage.shutdown(); +} diff --git a/test/e2e/oauth-loopback-consent.e2e.test.ts b/test/e2e/oauth-loopback-consent.e2e.test.ts index 79899307..13bf44de 100644 --- a/test/e2e/oauth-loopback-consent.e2e.test.ts +++ b/test/e2e/oauth-loopback-consent.e2e.test.ts @@ -8,12 +8,15 @@ import { mcpAuthRouter } from '@modelcontextprotocol/sdk/server/auth/router.js'; import { SDKOAuthServerProvider } from '@src/auth/sdkOAuthServerProvider.js'; import { McpConfigManager } from '@src/config/mcpConfigManager.js'; +import { AgentConfigManager } from '@src/core/server/agentConfig.js'; import { createOAuthRoutes } from '@src/transport/http/routes/oauthRoutes.js'; import express from 'express'; import { type Browser, chromium } from 'playwright'; import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest'; +let resolveCallback: ((url: URL) => void) | undefined; + describe('OAuth loopback consent browser flow', () => { let authServer: Server; let callbackServer: Server; @@ -22,20 +25,19 @@ describe('OAuth loopback consent browser flow', () => { let browser: Browser; let provider: SDKOAuthServerProvider; let storageDir: string; - let callbackReceived: Promise; + let originalAuthEnabled: boolean; beforeAll(async () => { storageDir = fs.mkdtempSync(path.join(os.tmpdir(), '1mcp-oauth-browser-')); + originalAuthEnabled = AgentConfigManager.getInstance().get('features').auth; + AgentConfigManager.getInstance().get('features').auth = true; vi.spyOn(McpConfigManager, 'getInstance').mockReturnValue({ - getAvailableTags: () => ['test'], + getAvailableTags: () => ['test', 'other'], } as unknown as McpConfigManager); - let resolveCallback!: (url: URL) => void; - callbackReceived = new Promise((resolve) => { - resolveCallback = resolve; - }); callbackServer = createServer((request, response) => { - resolveCallback(new URL(request.url ?? '/', callbackUrl)); + resolveCallback?.(new URL(request.url ?? '/', callbackUrl)); + resolveCallback = undefined; response.writeHead(200, { 'Content-Type': 'text/plain' }); response.end('Authorization complete'); }); @@ -52,7 +54,7 @@ describe('OAuth loopback consent browser flow', () => { provider, issuerUrl, baseUrl: issuerUrl, - scopesSupported: ['tag:test'], + scopesSupported: ['tag:test', 'tag:other'], authorizationOptions: { rateLimit: false }, tokenOptions: { rateLimit: false }, revocationOptions: { rateLimit: false }, @@ -69,6 +71,7 @@ describe('OAuth loopback consent browser flow', () => { await closeServer(authServer); await closeServer(callbackServer); fs.rmSync(storageDir, { recursive: true, force: true }); + AgentConfigManager.getInstance().get('features').auth = originalAuthEnabled; vi.restoreAllMocks(); }); @@ -95,7 +98,7 @@ describe('OAuth loopback consent browser flow', () => { response_type: 'code', client_id: client.client_id, redirect_uri: callbackUrl, - scope: 'tag:test', + scope: 'tag:test tag:other', state: 'browser-state', code_challenge: codeChallenge, code_challenge_method: 'S256', @@ -112,6 +115,7 @@ describe('OAuth loopback consent browser flow', () => { ); await page.getByText('renew this access for up to 30 days').waitFor({ state: 'visible' }); + const callbackReceived = waitForCallback(); await page.getByRole('button', { name: 'Approve' }).click(); const callback = await callbackReceived; expect(callback.searchParams.get('state')).toBe('browser-state'); @@ -131,10 +135,30 @@ describe('OAuth loopback consent browser flow', () => { expect(tokens.access_token).toMatch(/^tk-/); expect(tokens.refresh_token).toMatch(/^rt-[A-Za-z0-9_-]{43}$/); + const expandedScopeResponse = await postForm(`${authBaseUrl}/token`, { + grant_type: 'refresh_token', + client_id: client.client_id, + refresh_token: tokens.refresh_token, + scope: 'tag:test tag:outside', + resource, + }); + expect(expandedScopeResponse.status).toBe(400); + await expect(expandedScopeResponse.json()).resolves.toMatchObject({ error: 'invalid_scope' }); + + const wrongResourceResponse = await postForm(`${authBaseUrl}/token`, { + grant_type: 'refresh_token', + client_id: client.client_id, + refresh_token: tokens.refresh_token, + resource: `${authBaseUrl}/other`, + }); + expect(wrongResourceResponse.status).toBe(400); + await expect(wrongResourceResponse.json()).resolves.toMatchObject({ error: 'invalid_target' }); + const refreshResponse = await postForm(`${authBaseUrl}/token`, { grant_type: 'refresh_token', client_id: client.client_id, refresh_token: tokens.refresh_token, + scope: 'tag:test', resource, }); expect(refreshResponse.status, await refreshResponse.clone().text()).toBe(200); @@ -143,12 +167,77 @@ describe('OAuth loopback consent browser flow', () => { expect(rotated.refresh_token).toMatch(/^rt-/); expect(rotated.refresh_token).not.toBe(tokens.refresh_token); expect(rotated.scope).toBe('tag:test'); + + const replayResponse = await postForm(`${authBaseUrl}/token`, { + grant_type: 'refresh_token', + client_id: client.client_id, + refresh_token: tokens.refresh_token, + resource, + }); + expect(replayResponse.status).toBe(400); + await expect(replayResponse.json()).resolves.toMatchObject({ error: 'invalid_grant' }); + await expect(provider.verifyAccessToken(tokens.access_token)).rejects.toThrow('Invalid or expired access token'); + await expect(provider.verifyAccessToken(rotated.access_token)).rejects.toThrow( + 'Invalid or expired access token', + ); + + const secondVerifier = randomBytes(48).toString('base64url'); + const secondChallenge = createHash('sha256').update(secondVerifier).digest('base64url'); + const secondAuthorizationUrl = new URL(`${authBaseUrl}/authorize`); + secondAuthorizationUrl.search = new URLSearchParams({ + response_type: 'code', + client_id: client.client_id, + redirect_uri: callbackUrl, + scope: 'tag:test', + state: 'revocation-state', + code_challenge: secondChallenge, + code_challenge_method: 'S256', + resource, + }).toString(); + + await page.goto(secondAuthorizationUrl.toString()); + const secondCallbackReceived = waitForCallback(); + await page.getByRole('button', { name: 'Approve' }).click(); + const secondCallback = await secondCallbackReceived; + const secondTokenResponse = await postForm(`${authBaseUrl}/token`, { + grant_type: 'authorization_code', + client_id: client.client_id, + code: secondCallback.searchParams.get('code')!, + redirect_uri: callbackUrl, + code_verifier: secondVerifier, + resource, + }); + expect(secondTokenResponse.status).toBe(200); + const secondTokens = (await secondTokenResponse.json()) as { access_token: string; refresh_token: string }; + + const revocationResponse = await postForm(`${authBaseUrl}/revoke`, { + token: secondTokens.refresh_token, + client_id: client.client_id, + }); + expect(revocationResponse.status, await revocationResponse.clone().text()).toBe(200); + await expect(provider.verifyAccessToken(secondTokens.access_token)).rejects.toThrow( + 'Invalid or expired access token', + ); + const revokedRefreshResponse = await postForm(`${authBaseUrl}/token`, { + grant_type: 'refresh_token', + client_id: client.client_id, + refresh_token: secondTokens.refresh_token, + resource, + }); + expect(revokedRefreshResponse.status).toBe(400); + await expect(revokedRefreshResponse.json()).resolves.toMatchObject({ error: 'invalid_grant' }); } finally { await context.close(); } }, 30_000); }); +function waitForCallback(): Promise { + return new Promise((resolve) => { + resolveCallback = resolve; + }); +} + async function listenOnLoopback(server: Server): Promise { await new Promise((resolve) => server.listen(0, '127.0.0.1', resolve)); const address = server.address(); diff --git a/test/e2e/oauth-refresh-concurrency.e2e.test.ts b/test/e2e/oauth-refresh-concurrency.e2e.test.ts new file mode 100644 index 00000000..2cc449c9 --- /dev/null +++ b/test/e2e/oauth-refresh-concurrency.e2e.test.ts @@ -0,0 +1,126 @@ +import { spawn } from 'node:child_process'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; + +import type { OAuthClientInformationFull } from '@modelcontextprotocol/sdk/shared/auth.js'; + +import { SDKOAuthServerProvider } from '@src/auth/sdkOAuthServerProvider.js'; + +import { afterEach, describe, expect, it } from 'vitest'; + +const CLIENT: OAuthClientInformationFull = { + client_id: 'cross-process-client', + redirect_uris: ['http://127.0.0.1:3000/callback'], + grant_types: ['authorization_code', 'refresh_token'], + response_types: ['code'], + token_endpoint_auth_method: 'none', +}; +const RESOURCE = 'https://resource.example/mcp'; +const RUNTIME_SCOPE_ID = 'cross-process-runtime-scope'; +const WORKER_PATH = fileURLToPath(new URL('./fixtures/oauth-refresh-worker.mjs', import.meta.url)); + +describe('refresh token family cross-process persistence', () => { + let tempDir: string | undefined; + + afterEach(() => { + if (tempDir) { + fs.rmSync(tempDir, { recursive: true, force: true }); + } + }); + + it('allows exactly one process to commit a refresh-token rotation', async () => { + tempDir = fs.mkdtempSync(path.join(os.tmpdir(), '1mcp-refresh-process-')); + const provider = new SDKOAuthServerProvider(tempDir, RUNTIME_SCOPE_ID); + const code = provider.oauthStorage.authCodeRepository.create( + CLIENT.client_id, + CLIENT.redirect_uris[0], + RESOURCE, + ['tag:alpha'], + 60_000, + 'challenge', + ); + const initial = await provider.exchangeAuthorizationCode( + CLIENT, + code, + undefined, + CLIENT.redirect_uris[0], + new URL(RESOURCE), + ); + provider.shutdown(); + + const firstMarker = path.join(tempDir, 'first.entered'); + const secondMarker = path.join(tempDir, 'second.entered'); + const releasePath = path.join(tempDir, 'release-first'); + const first = runWorker(tempDir, initial.refresh_token!, firstMarker, releasePath); + await waitForFile(firstMarker); + + const second = runWorker(tempDir, initial.refresh_token!, secondMarker); + await delay(150); + expect(fs.existsSync(secondMarker)).toBe(false); + + fs.writeFileSync(releasePath, 'release'); + const results = await Promise.all([first.result, second.result]); + expect(results.map((result) => result.status).sort()).toEqual(['replay', 'rotated']); + }, 20_000); +}); + +function runWorker( + storageDir: string, + refreshToken: string, + markerPath: string, + releasePath?: string, +): { result: Promise<{ status: string }> } { + const child = spawn( + process.execPath, + [ + WORKER_PATH, + storageDir, + RUNTIME_SCOPE_ID, + refreshToken, + CLIENT.client_id, + markerPath, + releasePath ?? '', + ], + { cwd: process.cwd(), stdio: ['ignore', 'pipe', 'pipe'] }, + ); + + const result = new Promise<{ status: string }>((resolve, reject) => { + let stdout = ''; + let stderr = ''; + child.stdout?.on('data', (chunk) => { + stdout += String(chunk); + }); + child.stderr?.on('data', (chunk) => { + stderr += String(chunk); + }); + child.once('error', reject); + child.once('exit', (code) => { + const line = stdout + .split('\n') + .find((candidate) => candidate.startsWith('RESULT ')); + if (!line) { + reject(new Error(`Refresh worker exited ${code}: ${stderr || stdout}`)); + return; + } + resolve(JSON.parse(line.slice('RESULT '.length)) as { status: string }); + }); + }); + + return { result }; +} + +async function waitForFile(filePath: string): Promise { + const deadline = Date.now() + 5_000; + while (!fs.existsSync(filePath)) { + if (Date.now() >= deadline) { + throw new Error(`Timed out waiting for ${filePath}`); + } + await delay(20); + } +} + +function delay(milliseconds: number): Promise { + return new Promise((resolve) => setTimeout(resolve, milliseconds)); +} From 48014e9772e814a3340f3a0a047653091f35fd3f Mon Sep 17 00:00:00 2001 From: Xu Zhipei Date: Thu, 30 Jul 2026 22:56:13 +0800 Subject: [PATCH 3/6] fix(oauth): address refresh rotation review --- ...th-refresh-tokens-use-rotating-families.md | 4 ++ src/auth/oauthAuthorizationFlow.ts | 4 +- .../sdkOAuthServerProvider.refresh.test.ts | 28 ++++---- src/auth/sdkOAuthServerProvider.ts | 12 ---- src/auth/sessionTypes.ts | 5 +- src/auth/storage/fileStorageService.test.ts | 12 ++++ src/auth/storage/fileStorageService.ts | 6 +- .../storage/refreshTokenFamilyRepository.ts | 24 ++----- test/e2e/fixtures/oauth-refresh-worker.mjs | 11 +-- test/e2e/oauth-loopback-consent.e2e.test.ts | 17 +++-- .../e2e/oauth-refresh-concurrency.e2e.test.ts | 72 ++++++++++++------- 11 files changed, 108 insertions(+), 87 deletions(-) diff --git a/docs/adr/0011-oauth-refresh-tokens-use-rotating-families.md b/docs/adr/0011-oauth-refresh-tokens-use-rotating-families.md index af072574..4c544c24 100644 --- a/docs/adr/0011-oauth-refresh-tokens-use-rotating-families.md +++ b/docs/adr/0011-oauth-refresh-tokens-use-rotating-families.md @@ -1,3 +1,7 @@ +--- +status: accepted +--- + # OAuth Refresh Tokens Use Rotating Families 1MCP issues refresh tokens only to registered clients that request the `refresh_token` grant. Each approved authorization creates an independent, client-, resource-, and scope-bound Refresh Token Family that persists in its Runtime Scope for a fixed 30 days. Every successful refresh rotates a single-use opaque token; only token digests and family lineage are stored. diff --git a/src/auth/oauthAuthorizationFlow.ts b/src/auth/oauthAuthorizationFlow.ts index e4d91337..9f46336b 100644 --- a/src/auth/oauthAuthorizationFlow.ts +++ b/src/auth/oauthAuthorizationFlow.ts @@ -396,8 +396,8 @@ export function createOAuthAuthorizationFlowFromStorage( processConsentApproval: (authRequestId, selectedScopes) => storage.processConsentApproval(authRequestId, selectedScopes), processConsentDenial: (authRequestId) => storage.processConsentDenial(authRequestId), - createSessionWithId: (tokenId, clientId, resource, scopes, ttlMs) => - storage.sessionRepository.createWithId(tokenId, clientId, resource, scopes, ttlMs), + createSessionWithId: (tokenId, clientId, resource, scopes, ttlMs, refreshFamilyId) => + storage.sessionRepository.createWithId(tokenId, clientId, resource, scopes, ttlMs, refreshFamilyId), }, }); } diff --git a/src/auth/sdkOAuthServerProvider.refresh.test.ts b/src/auth/sdkOAuthServerProvider.refresh.test.ts index 62016cfb..69eaaf8e 100644 --- a/src/auth/sdkOAuthServerProvider.refresh.test.ts +++ b/src/auth/sdkOAuthServerProvider.refresh.test.ts @@ -9,15 +9,15 @@ import { } from '@modelcontextprotocol/sdk/server/auth/errors.js'; import type { OAuthClientInformationFull, OAuthTokens } from '@modelcontextprotocol/sdk/shared/auth.js'; +import { RefreshTokenFamilyDataSchema } from '@src/auth/sessionTypes.js'; import { AUTH_CONFIG } from '@src/constants.js'; import { AgentConfigManager } from '@src/core/server/agentConfig.js'; import logger from '@src/logger/logger.js'; -import { FileStorageService } from './storage/fileStorageService.js'; - import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { SDKOAuthServerProvider } from './sdkOAuthServerProvider.js'; +import { FileStorageService } from './storage/fileStorageService.js'; const CLIENT: OAuthClientInformationFull = { client_id: 'refresh-client', @@ -78,12 +78,12 @@ describe('SDKOAuthServerProvider refresh token families', () => { it('persists families across provider restarts and rotation does not extend their fixed expiry', async () => { const initial = await exchangeAuthorizationCode(provider, CLIENT); - const before = readOnlyFamily(tempDir); + const before = readSoleFamily(tempDir); provider.shutdown(); provider = new SDKOAuthServerProvider(tempDir, 'runtime-scope-a'); const rotated = await provider.exchangeRefreshToken(CLIENT, initial.refresh_token!); - const after = readOnlyFamily(tempDir); + const after = readSoleFamily(tempDir); expect(rotated.refresh_token).toMatch(/^rt-/); expect(rotated.refresh_token).not.toBe(initial.refresh_token); @@ -112,7 +112,7 @@ describe('SDKOAuthServerProvider refresh token families', () => { const initial = await exchangeAuthorizationCode(provider, CLIENT); const createSession = vi .spyOn(provider.oauthStorage.sessionRepository, 'createRefreshFamilyAccessSession') - .mockImplementationOnce(() => { + .mockImplementation(() => { throw new Error('session persistence failed'); }); @@ -171,7 +171,7 @@ describe('SDKOAuthServerProvider refresh token families', () => { const restored = await provider.exchangeRefreshToken(CLIENT, narrowed.refresh_token!); expect(restored.scope).toBe(SCOPES.join(' ')); - expect(readOnlyFamily(tempDir).scopeCeiling).toEqual(SCOPES); + expect(readSoleFamily(tempDir).scopeCeiling).toEqual(SCOPES); }); it('allows exactly one concurrent rotation and replay revokes the family and every associated access token', async () => { @@ -184,7 +184,7 @@ describe('SDKOAuthServerProvider refresh token families', () => { expect(attempts.filter((attempt) => attempt.status === 'fulfilled')).toHaveLength(1); const rejection = attempts.find((attempt) => attempt.status === 'rejected') as PromiseRejectedResult; expect(rejection.reason).toBeInstanceOf(InvalidGrantError); - expect(readOnlyFamily(tempDir).status).toBe('revoked'); + expect(readSoleFamily(tempDir).status).toBe('revoked'); const successful = ( attempts.find((attempt) => attempt.status === 'fulfilled') as PromiseFulfilledResult @@ -204,20 +204,20 @@ describe('SDKOAuthServerProvider refresh token families', () => { const second = await provider.exchangeRefreshToken(CLIENT, first.refresh_token!); await provider.exchangeRefreshToken(CLIENT, second.refresh_token!); - expect(readOnlyFamily(tempDir).consumedTokenDigests).toHaveLength(1); + expect(readSoleFamily(tempDir).consumedTokenDigests).toHaveLength(1); await expect(provider.exchangeRefreshToken(CLIENT, initial.refresh_token!)).rejects.toBeInstanceOf( InvalidGrantError, ); - expect(readOnlyFamily(tempDir).status).toBe('revoked'); + expect(readSoleFamily(tempDir).status).toBe('revoked'); }); it('isolates refresh families by Runtime Scope even when storage is shared', async () => { const initial = await exchangeAuthorizationCode(provider, CLIENT); const otherScopeProvider = new SDKOAuthServerProvider(tempDir, 'runtime-scope-b'); try { - await expect( - otherScopeProvider.exchangeRefreshToken(CLIENT, initial.refresh_token!), - ).rejects.toBeInstanceOf(InvalidGrantError); + await expect(otherScopeProvider.exchangeRefreshToken(CLIENT, initial.refresh_token!)).rejects.toBeInstanceOf( + InvalidGrantError, + ); await expect(provider.exchangeRefreshToken(CLIENT, initial.refresh_token!)).resolves.toMatchObject({ refresh_token: expect.stringMatching(/^rt-/), }); @@ -323,8 +323,8 @@ function listLookupFiles(tempDir: string): string[] { .map((fileName) => path.join(serverDir, fileName)); } -function readOnlyFamily(tempDir: string): Record { +function readSoleFamily(tempDir: string) { const familyFiles = listFamilyFiles(tempDir); expect(familyFiles).toHaveLength(1); - return JSON.parse(fs.readFileSync(familyFiles[0], 'utf8')); + return RefreshTokenFamilyDataSchema.parse(JSON.parse(fs.readFileSync(familyFiles[0], 'utf8'))); } diff --git a/src/auth/sdkOAuthServerProvider.ts b/src/auth/sdkOAuthServerProvider.ts index 9a7656ef..9e345ae0 100644 --- a/src/auth/sdkOAuthServerProvider.ts +++ b/src/auth/sdkOAuthServerProvider.ts @@ -453,18 +453,6 @@ export class SDKOAuthServerProvider implements OAuthServerProvider { throw new InvalidGrantError('Invalid refresh token'); } - const tokenState = repository.getTokenState(family, refreshToken); - if (tokenState === 'consumed') { - const replay = await repository.consume(refreshToken, client.client_id, randomUUID(), () => undefined); - if (replay.status === 'replay') { - this.revokeFamilyAccessTokens(replay.family.accessTokenIds); - } - throw new InvalidGrantError('Refresh token replay detected'); - } - if (tokenState !== 'current' || family.status !== 'active') { - throw new InvalidGrantError('Invalid refresh token'); - } - const requestedScopes = scopes ?? family.scopeCeiling; if (requestedScopes.some((scope) => !family.scopeCeiling.includes(scope))) { throw new InvalidScopeError('Requested scope exceeds the originally consented scope'); diff --git a/src/auth/sessionTypes.ts b/src/auth/sessionTypes.ts index 019f7d1f..3d25c1b8 100644 --- a/src/auth/sessionTypes.ts +++ b/src/auth/sessionTypes.ts @@ -1,9 +1,10 @@ // Shared session types for server and client session managers import { OAuthClientInformationFull } from '@modelcontextprotocol/sdk/shared/auth.js'; -import { z } from 'zod'; import { ContextNamespace, EnvironmentContext, UserContext } from '@src/types/context.js'; +import { z } from 'zod'; + /** * Base interface for all data that can expire */ @@ -37,7 +38,7 @@ export const RefreshTokenFamilyDataSchema = z.object({ resource: z.string(), currentTokenDigest: RefreshTokenDigestSchema, consumedTokenDigests: z.array(RefreshTokenDigestSchema), - accessTokenIds: z.array(z.string().uuid()), + accessTokenIds: z.array(z.uuid()), status: z.enum(['active', 'revoked']), revokedAt: z.number().finite().optional(), }); diff --git a/src/auth/storage/fileStorageService.test.ts b/src/auth/storage/fileStorageService.test.ts index 1a6165cc..7418adb9 100644 --- a/src/auth/storage/fileStorageService.test.ts +++ b/src/auth/storage/fileStorageService.test.ts @@ -142,6 +142,18 @@ describe('FileStorageService', () => { await expect(service.withExclusiveLock('refresh-test', () => 'acquired')).resolves.toBe('acquired'); expect(fs.existsSync(lockPath)).toBe(false); }); + + it('preserves the operation result when lock cleanup fails', async () => { + const originalRenameSync = fs.renameSync; + vi.spyOn(fs, 'renameSync').mockImplementation((oldPath, newPath) => { + if (String(newPath).endsWith('.releasing')) { + throw new Error('simulated lock cleanup failure'); + } + return originalRenameSync(oldPath, newPath); + }); + + await expect(service.withExclusiveLock('refresh-test', () => 'committed')).resolves.toBe('committed'); + }); }); describe('Path Security', () => { diff --git a/src/auth/storage/fileStorageService.ts b/src/auth/storage/fileStorageService.ts index 6ec3333a..1f9921fd 100644 --- a/src/auth/storage/fileStorageService.ts +++ b/src/auth/storage/fileStorageService.ts @@ -1,10 +1,11 @@ import fs from 'fs'; -import path from 'path'; import { randomUUID } from 'node:crypto'; +import path from 'path'; import { ExpirableData } from '@src/auth/sessionTypes.js'; import { AUTH_CONFIG, FILE_PREFIX_MAPPING, getGlobalConfigDir, STORAGE_SUBDIRS } from '@src/constants.js'; import logger from '@src/logger/logger.js'; + import { z, type ZodType } from 'zod'; const StorageLockOwnerSchema = z.object({ @@ -543,7 +544,7 @@ export class FileStorageService { process.platform === 'win32' && error instanceof Error && 'code' in error && - ['EINVAL', 'ENOTSUP', 'EPERM'].includes(String(error.code)) + ['EACCES', 'EISDIR', 'EINVAL', 'ENOTSUP', 'EPERM'].includes(String(error.code)) ) { return; } @@ -631,7 +632,6 @@ export class FileStorageService { this.flushStorageDirectory(); } catch (error) { logger.error(`Failed to release storage lock ${lockPath}: ${error}`); - throw error; } } diff --git a/src/auth/storage/refreshTokenFamilyRepository.ts b/src/auth/storage/refreshTokenFamilyRepository.ts index 773ad172..adbf9b8c 100644 --- a/src/auth/storage/refreshTokenFamilyRepository.ts +++ b/src/auth/storage/refreshTokenFamilyRepository.ts @@ -74,19 +74,6 @@ export class RefreshTokenFamilyRepository { return family?.runtimeScopeId === this.runtimeScopeId ? family : null; } - getTokenState(family: RefreshTokenFamilyData, refreshToken: string): 'current' | 'consumed' | 'unknown' { - const digest = digestRefreshToken(refreshToken); - if (safeDigestEqual(family.currentTokenDigest, digest)) { - return 'current'; - } - if (family.consumedTokenDigests.some((consumed) => safeDigestEqual(consumed, digest))) { - return 'consumed'; - } - - const lookup = this.readLookup(digest); - return lookup?.familyId === family.familyId ? lookup.state : 'unknown'; - } - async consume( refreshToken: string, clientId: string, @@ -166,9 +153,7 @@ export class RefreshTokenFamilyRepository { return revokedFamily; } - private locateByDigest( - digest: string, - ): { family: RefreshTokenFamilyData; lookup?: RefreshTokenLookupData } | null { + private locateByDigest(digest: string): { family: RefreshTokenFamilyData; lookup?: RefreshTokenLookupData } | null { const lookup = this.readLookup(digest); if (lookup?.runtimeScopeId === this.runtimeScopeId) { const family = this.findById(lookup.familyId); @@ -221,10 +206,11 @@ export class RefreshTokenFamilyRepository { .listFiles(FILE_PREFIX) .map((fileName) => fileName.slice(FILE_PREFIX.length, -extension.length)) .filter((familyId) => familyId.startsWith(ID_PREFIX)) - .map((familyId) => this.storage.readData(FILE_PREFIX, familyId, RefreshTokenFamilyDataSchema)) + .map((familyId) => + this.storage.readData(FILE_PREFIX, familyId, RefreshTokenFamilyDataSchema), + ) .filter( - (family): family is RefreshTokenFamilyData => - family !== null && family.runtimeScopeId === this.runtimeScopeId, + (family): family is RefreshTokenFamilyData => family !== null && family.runtimeScopeId === this.runtimeScopeId, ); } diff --git a/test/e2e/fixtures/oauth-refresh-worker.mjs b/test/e2e/fixtures/oauth-refresh-worker.mjs index e8a9912c..bdf6d059 100644 --- a/test/e2e/fixtures/oauth-refresh-worker.mjs +++ b/test/e2e/fixtures/oauth-refresh-worker.mjs @@ -1,24 +1,27 @@ -import fs from 'node:fs'; import { randomUUID } from 'node:crypto'; +import fs from 'node:fs'; -import { AUTH_CONFIG } from '../../../build/constants.js'; import { FileStorageService } from '../../../build/auth/storage/fileStorageService.js'; import { RefreshTokenFamilyRepository } from '../../../build/auth/storage/refreshTokenFamilyRepository.js'; import { SessionRepository } from '../../../build/auth/storage/sessionRepository.js'; +import { AUTH_CONFIG } from '../../../build/constants.js'; -const [storageDir, runtimeScopeId, refreshToken, clientId, markerPath, releasePath] = process.argv.slice(2); +const [storageDir, runtimeScopeId, refreshToken, clientId, startedPath, markerPath, releasePath] = + process.argv.slice(2); const storage = new FileStorageService(storageDir, AUTH_CONFIG.SERVER.SESSION.SUBDIR); const sessions = new SessionRepository(storage); const repository = new RefreshTokenFamilyRepository(storage, runtimeScopeId); const accessTokenId = randomUUID(); try { + fs.writeFileSync(startedPath, String(process.pid)); const result = await repository.consume(refreshToken, clientId, accessTokenId, (familyId) => { fs.writeFileSync(markerPath, String(process.pid)); if (releasePath) { const deadline = Date.now() + 10_000; + const sleepHandle = new Int32Array(new SharedArrayBuffer(4)); while (!fs.existsSync(releasePath) && Date.now() < deadline) { - Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, 10); + Atomics.wait(sleepHandle, 0, 0, 10); } if (!fs.existsSync(releasePath)) { throw new Error('Timed out waiting for transition release'); diff --git a/test/e2e/oauth-loopback-consent.e2e.test.ts b/test/e2e/oauth-loopback-consent.e2e.test.ts index 13bf44de..b3a23bfd 100644 --- a/test/e2e/oauth-loopback-consent.e2e.test.ts +++ b/test/e2e/oauth-loopback-consent.e2e.test.ts @@ -177,9 +177,7 @@ describe('OAuth loopback consent browser flow', () => { expect(replayResponse.status).toBe(400); await expect(replayResponse.json()).resolves.toMatchObject({ error: 'invalid_grant' }); await expect(provider.verifyAccessToken(tokens.access_token)).rejects.toThrow('Invalid or expired access token'); - await expect(provider.verifyAccessToken(rotated.access_token)).rejects.toThrow( - 'Invalid or expired access token', - ); + await expect(provider.verifyAccessToken(rotated.access_token)).rejects.toThrow('Invalid or expired access token'); const secondVerifier = randomBytes(48).toString('base64url'); const secondChallenge = createHash('sha256').update(secondVerifier).digest('base64url'); @@ -232,9 +230,16 @@ describe('OAuth loopback consent browser flow', () => { }, 30_000); }); -function waitForCallback(): Promise { - return new Promise((resolve) => { - resolveCallback = resolve; +function waitForCallback(timeoutMs = 10_000): Promise { + return new Promise((resolve, reject) => { + const timer = setTimeout(() => { + resolveCallback = undefined; + reject(new Error('Timed out waiting for the loopback callback')); + }, timeoutMs); + resolveCallback = (url) => { + clearTimeout(timer); + resolve(url); + }; }); } diff --git a/test/e2e/oauth-refresh-concurrency.e2e.test.ts b/test/e2e/oauth-refresh-concurrency.e2e.test.ts index 2cc449c9..e65bac4c 100644 --- a/test/e2e/oauth-refresh-concurrency.e2e.test.ts +++ b/test/e2e/oauth-refresh-concurrency.e2e.test.ts @@ -23,8 +23,16 @@ const WORKER_PATH = fileURLToPath(new URL('./fixtures/oauth-refresh-worker.mjs', describe('refresh token family cross-process persistence', () => { let tempDir: string | undefined; + const children = new Set>(); + + afterEach(async () => { + const activeChildren = [...children].filter((child) => child.exitCode === null && child.signalCode === null); + for (const child of activeChildren) { + child.kill('SIGKILL'); + } + await Promise.all(activeChildren.map(waitForExit)); + children.clear(); - afterEach(() => { if (tempDir) { fs.rmSync(tempDir, { recursive: true, force: true }); } @@ -33,31 +41,36 @@ describe('refresh token family cross-process persistence', () => { it('allows exactly one process to commit a refresh-token rotation', async () => { tempDir = fs.mkdtempSync(path.join(os.tmpdir(), '1mcp-refresh-process-')); const provider = new SDKOAuthServerProvider(tempDir, RUNTIME_SCOPE_ID); - const code = provider.oauthStorage.authCodeRepository.create( - CLIENT.client_id, - CLIENT.redirect_uris[0], - RESOURCE, - ['tag:alpha'], - 60_000, - 'challenge', - ); - const initial = await provider.exchangeAuthorizationCode( - CLIENT, - code, - undefined, - CLIENT.redirect_uris[0], - new URL(RESOURCE), - ); - provider.shutdown(); + const initial = await (async () => { + try { + const code = provider.oauthStorage.authCodeRepository.create( + CLIENT.client_id, + CLIENT.redirect_uris[0], + RESOURCE, + ['tag:alpha'], + 60_000, + 'challenge', + ); + return await provider.exchangeAuthorizationCode( + CLIENT, + code, + undefined, + CLIENT.redirect_uris[0], + new URL(RESOURCE), + ); + } finally { + provider.shutdown(); + } + })(); const firstMarker = path.join(tempDir, 'first.entered'); const secondMarker = path.join(tempDir, 'second.entered'); const releasePath = path.join(tempDir, 'release-first'); - const first = runWorker(tempDir, initial.refresh_token!, firstMarker, releasePath); + const first = runWorker(tempDir, initial.refresh_token!, firstMarker, children, releasePath); await waitForFile(firstMarker); - const second = runWorker(tempDir, initial.refresh_token!, secondMarker); - await delay(150); + const second = runWorker(tempDir, initial.refresh_token!, secondMarker, children); + await second.started; expect(fs.existsSync(secondMarker)).toBe(false); fs.writeFileSync(releasePath, 'release'); @@ -70,8 +83,10 @@ function runWorker( storageDir: string, refreshToken: string, markerPath: string, + children: Set>, releasePath?: string, -): { result: Promise<{ status: string }> } { +): { started: Promise; result: Promise<{ status: string }> } { + const startedPath = `${markerPath}.started`; const child = spawn( process.execPath, [ @@ -80,11 +95,13 @@ function runWorker( RUNTIME_SCOPE_ID, refreshToken, CLIENT.client_id, + startedPath, markerPath, releasePath ?? '', ], { cwd: process.cwd(), stdio: ['ignore', 'pipe', 'pipe'] }, ); + children.add(child); const result = new Promise<{ status: string }>((resolve, reject) => { let stdout = ''; @@ -97,9 +114,7 @@ function runWorker( }); child.once('error', reject); child.once('exit', (code) => { - const line = stdout - .split('\n') - .find((candidate) => candidate.startsWith('RESULT ')); + const line = stdout.split('\n').find((candidate) => candidate.startsWith('RESULT ')); if (!line) { reject(new Error(`Refresh worker exited ${code}: ${stderr || stdout}`)); return; @@ -108,7 +123,14 @@ function runWorker( }); }); - return { result }; + return { started: waitForFile(startedPath), result }; +} + +async function waitForExit(child: ReturnType): Promise { + if (child.exitCode !== null || child.signalCode !== null) { + return; + } + await new Promise((resolve) => child.once('exit', () => resolve())); } async function waitForFile(filePath: string): Promise { From 1c55765d6f377090d4beb098357646ea12558513 Mon Sep 17 00:00:00 2001 From: Xu Zhipei Date: Thu, 30 Jul 2026 23:10:15 +0800 Subject: [PATCH 4/6] fix(storage): recover failed lock releases --- src/auth/storage/fileStorageService.test.ts | 7 +++-- src/auth/storage/fileStorageService.ts | 30 +++++++++++++++++++++ 2 files changed, 35 insertions(+), 2 deletions(-) diff --git a/src/auth/storage/fileStorageService.test.ts b/src/auth/storage/fileStorageService.test.ts index 7418adb9..60a3ac62 100644 --- a/src/auth/storage/fileStorageService.test.ts +++ b/src/auth/storage/fileStorageService.test.ts @@ -143,16 +143,19 @@ describe('FileStorageService', () => { expect(fs.existsSync(lockPath)).toBe(false); }); - it('preserves the operation result when lock cleanup fails', async () => { + it('recovers from a release rename failure without blocking the next acquisition', async () => { const originalRenameSync = fs.renameSync; + let failedReleaseRename = false; vi.spyOn(fs, 'renameSync').mockImplementation((oldPath, newPath) => { - if (String(newPath).endsWith('.releasing')) { + if (!failedReleaseRename && String(newPath).endsWith('.releasing')) { + failedReleaseRename = true; throw new Error('simulated lock cleanup failure'); } return originalRenameSync(oldPath, newPath); }); await expect(service.withExclusiveLock('refresh-test', () => 'committed')).resolves.toBe('committed'); + await expect(service.withExclusiveLock('refresh-test', () => 'acquired-again')).resolves.toBe('acquired-again'); }); }); diff --git a/src/auth/storage/fileStorageService.ts b/src/auth/storage/fileStorageService.ts index 1f9921fd..7371db46 100644 --- a/src/auth/storage/fileStorageService.ts +++ b/src/auth/storage/fileStorageService.ts @@ -628,6 +628,36 @@ export class FileStorageService { const tombstonePath = `${lockPath}.${operationId}.releasing`; try { fs.renameSync(lockPath, tombstonePath); + } catch (renameError) { + const currentOwner = this.readLockOwner(lockPath); + if (!currentOwner && !fs.existsSync(lockPath)) { + logger.warn(`Storage lock disappeared during release: ${lockPath}`); + return; + } + if (currentOwner?.operationId !== operationId) { + logger.error(`Storage lock ownership changed during release: ${lockPath}`); + throw renameError; + } + + try { + removeLockDirectory(lockPath); + } catch (cleanupError) { + throw new AggregateError( + [renameError, cleanupError], + `Failed to release storage lock after rename failure: ${lockPath}`, + ); + } + + try { + this.flushStorageDirectory(); + } catch (flushError) { + logger.error(`Failed to flush storage directory after lock release ${lockPath}: ${flushError}`); + } + logger.warn(`Released storage lock without rename after rename failure: ${lockPath}`); + return; + } + + try { removeLockDirectory(tombstonePath); this.flushStorageDirectory(); } catch (error) { From d9cea16de140d3d5e5b38d1c78088a15c2826e80 Mon Sep 17 00:00:00 2001 From: Xu Zhipei Date: Thu, 30 Jul 2026 23:23:22 +0800 Subject: [PATCH 5/6] test(storage): assert release failure injection --- src/auth/storage/fileStorageService.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/auth/storage/fileStorageService.test.ts b/src/auth/storage/fileStorageService.test.ts index 60a3ac62..907b56f4 100644 --- a/src/auth/storage/fileStorageService.test.ts +++ b/src/auth/storage/fileStorageService.test.ts @@ -155,6 +155,7 @@ describe('FileStorageService', () => { }); await expect(service.withExclusiveLock('refresh-test', () => 'committed')).resolves.toBe('committed'); + expect(failedReleaseRename).toBe(true); await expect(service.withExclusiveLock('refresh-test', () => 'acquired-again')).resolves.toBe('acquired-again'); }); }); From 6bd2af488cb5dc2e5f024627eb97ecdd3559002d Mon Sep 17 00:00:00 2001 From: Xu Zhipei Date: Thu, 30 Jul 2026 23:31:11 +0800 Subject: [PATCH 6/6] fix(storage): preserve exclusive lock ownership --- src/auth/storage/fileStorageService.test.ts | 52 +++++++++++++++++++++ src/auth/storage/fileStorageService.ts | 16 +++++-- 2 files changed, 63 insertions(+), 5 deletions(-) diff --git a/src/auth/storage/fileStorageService.test.ts b/src/auth/storage/fileStorageService.test.ts index 907b56f4..6c76e096 100644 --- a/src/auth/storage/fileStorageService.test.ts +++ b/src/auth/storage/fileStorageService.test.ts @@ -158,6 +158,58 @@ describe('FileStorageService', () => { expect(failedReleaseRename).toBe(true); await expect(service.withExclusiveLock('refresh-test', () => 'acquired-again')).resolves.toBe('acquired-again'); }); + + it('does not overwrite a newer owner created during owner-less lock reclamation', async () => { + const lockPath = path.join(service.getStorageDir(), '.owner-race-test.lock'); + const ownerPath = path.join(lockPath, 'owner.json'); + const replacementOperationId = 'replacement-operation'; + const originalMkdirSync = fs.mkdirSync; + const originalWriteFileSync = fs.writeFileSync; + let injectedReplacementOwner = false; + let exclusiveWriteRejected = false; + let replacementOwnerPreserved = false; + let releaseReplacementOwner = Promise.resolve(); + + vi.spyOn(fs, 'writeFileSync').mockImplementation((file, data, options) => { + if (String(file) === ownerPath && !injectedReplacementOwner) { + injectedReplacementOwner = true; + fs.rmSync(lockPath, { recursive: true, force: true }); + originalMkdirSync(lockPath, { mode: 0o700 }); + originalWriteFileSync( + ownerPath, + JSON.stringify({ operationId: replacementOperationId, pid: process.pid, createdAt: Date.now() }), + { mode: 0o600, flag: 'wx' }, + ); + releaseReplacementOwner = new Promise((resolve) => { + queueMicrotask(() => { + try { + const owner = JSON.parse(fs.readFileSync(ownerPath, 'utf8')) as { operationId?: string }; + replacementOwnerPreserved = owner.operationId === replacementOperationId; + } catch { + replacementOwnerPreserved = false; + } + fs.rmSync(lockPath, { recursive: true, force: true }); + resolve(); + }); + }); + } + + try { + return originalWriteFileSync(file, data, options); + } catch (error) { + if (String(file) === ownerPath) { + exclusiveWriteRejected = true; + } + throw error; + } + }); + + await expect(service.withExclusiveLock('owner-race-test', () => 'acquired')).resolves.toBe('acquired'); + await releaseReplacementOwner; + expect(injectedReplacementOwner).toBe(true); + expect(exclusiveWriteRejected).toBe(true); + expect(replacementOwnerPreserved).toBe(true); + }); }); describe('Path Security', () => { diff --git a/src/auth/storage/fileStorageService.ts b/src/auth/storage/fileStorageService.ts index 7371db46..80e70d6c 100644 --- a/src/auth/storage/fileStorageService.ts +++ b/src/auth/storage/fileStorageService.ts @@ -567,15 +567,21 @@ export class FileStorageService { fs.writeFileSync( path.join(lockPath, 'owner.json'), JSON.stringify({ operationId, pid: process.pid, createdAt: Date.now() }), - { mode: 0o600 }, + { mode: 0o600, flag: 'wx' }, ); this.flushStorageDirectory(); return true; } catch (error) { - try { - removeLockDirectory(lockPath); - } catch (cleanupError) { - throw new AggregateError([error, cleanupError], `Failed to initialize storage lock: ${lockPath}`); + const owner = this.readLockOwner(lockPath); + if (owner?.operationId === operationId) { + try { + removeLockDirectory(lockPath); + } catch (cleanupError) { + throw new AggregateError([error, cleanupError], `Failed to initialize storage lock: ${lockPath}`); + } + } + if (isFileExistsError(error)) { + return false; } throw error; }