diff --git a/CHANGELOG.md b/CHANGELOG.md index 7dd4443..f2558a6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ ### OAuth - Proactively complete OAuth for configured HTTP servers that allow unauthenticated `initialize`/`listTools` but require credentials for tool calls, and close the local callback server promptly after browser authorization. (PR #159, thanks @Spacefish) +- Refresh expired cached OAuth access tokens during non-interactive `mcporter list` without opening a browser or clearing cached credentials when refresh fails. (Issue #166, thanks @chrisabad) ## [0.10.2] - 2026-05-09 diff --git a/src/oauth-client-info.ts b/src/oauth-client-info.ts new file mode 100644 index 0000000..cf1952a --- /dev/null +++ b/src/oauth-client-info.ts @@ -0,0 +1,33 @@ +import type { OAuthClientInformationMixed } from '@modelcontextprotocol/sdk/shared/auth.js'; +import type { ServerDefinition } from './config.js'; + +export function buildStaticClientInformation( + definition: ServerDefinition, + options: { redirectUrl?: URL | string } = {} +): OAuthClientInformationMixed | undefined { + if (!definition.oauthClientId) { + return undefined; + } + const clientSecret = resolveOAuthClientSecret(definition); + return { + client_id: definition.oauthClientId, + ...(clientSecret ? { client_secret: clientSecret } : {}), + ...(options.redirectUrl ? { redirect_uris: [options.redirectUrl.toString()] } : {}), + grant_types: ['authorization_code', 'refresh_token'], + response_types: ['code'], + ...(definition.oauthTokenEndpointAuthMethod + ? { token_endpoint_auth_method: definition.oauthTokenEndpointAuthMethod } + : {}), + } as OAuthClientInformationMixed; +} + +export function resolveOAuthClientSecret(definition: ServerDefinition): string | undefined { + if (definition.oauthClientSecretEnv) { + const value = process.env[definition.oauthClientSecretEnv]; + if (!value) { + throw new Error(`Environment variable '${definition.oauthClientSecretEnv}' is required for OAuth client secret.`); + } + return value; + } + return definition.oauthClientSecret; +} diff --git a/src/oauth-persistence.ts b/src/oauth-persistence.ts index 5b075d2..b372684 100644 --- a/src/oauth-persistence.ts +++ b/src/oauth-persistence.ts @@ -1,10 +1,17 @@ import fs from 'node:fs/promises'; import os from 'node:os'; import path from 'node:path'; -import type { OAuthClientInformationMixed, OAuthTokens } from '@modelcontextprotocol/sdk/shared/auth.js'; +import { discoverOAuthServerInfo, refreshAuthorization } from '@modelcontextprotocol/sdk/client/auth.js'; +import type { + OAuthClientInformationMixed, + OAuthProtectedResourceMetadata, + OAuthTokens, +} from '@modelcontextprotocol/sdk/shared/auth.js'; +import { checkResourceAllowed, resourceUrlFromServerUrl } from '@modelcontextprotocol/sdk/shared/auth-utils.js'; import type { ServerDefinition } from './config.js'; import { readJsonFile, writeJsonFile } from './fs-json.js'; import type { Logger } from './logging.js'; +import { buildStaticClientInformation } from './oauth-client-info.js'; import { clearVaultEntry, getOAuthVaultPath, loadVaultEntry, saveVaultEntry } from './oauth-vault.js'; import { legacyMcporterDir } from './paths.js'; @@ -23,6 +30,61 @@ export interface OAuthPersistence { clear(scope: OAuthClearScope): Promise; } +type StoredOAuthTokens = OAuthTokens & { + expires_at?: number; + expiresAt?: number; +}; + +const TOKEN_EXPIRY_SKEW_SECONDS = 60; + +function withStoredExpiry(tokens: OAuthTokens): OAuthTokens { + const stored = tokens as StoredOAuthTokens; + if (typeof stored.expires_at === 'number' || typeof stored.expiresAt === 'number') { + return tokens; + } + if (typeof tokens.expires_in === 'number' && Number.isFinite(tokens.expires_in)) { + return { + ...tokens, + expires_at: Math.floor(Date.now() / 1000) + tokens.expires_in, + } as OAuthTokens; + } + return tokens; +} + +function tokenExpirySeconds(tokens: OAuthTokens): number | undefined { + const stored = tokens as StoredOAuthTokens; + for (const candidate of [stored.expires_at, stored.expiresAt]) { + if (typeof candidate === 'number' && Number.isFinite(candidate)) { + return candidate; + } + } + return undefined; +} + +function shouldRefreshCachedToken(tokens: OAuthTokens): boolean { + const expiresAt = tokenExpirySeconds(tokens); + if (expiresAt !== undefined) { + return expiresAt <= Math.floor(Date.now() / 1000) + TOKEN_EXPIRY_SKEW_SECONDS; + } + return typeof tokens.expires_in === 'number' && typeof tokens.refresh_token === 'string'; +} + +function resourceForRefresh( + serverUrl: URL, + resourceMetadata: OAuthProtectedResourceMetadata | undefined +): URL | undefined { + if (!resourceMetadata) { + return undefined; + } + const defaultResource = resourceUrlFromServerUrl(serverUrl); + if (!checkResourceAllowed({ requestedResource: defaultResource, configuredResource: resourceMetadata.resource })) { + throw new Error( + `Protected resource ${resourceMetadata.resource} does not match expected ${defaultResource} (or origin)` + ); + } + return new URL(resourceMetadata.resource); +} + class DirectoryPersistence implements OAuthPersistence { private readonly tokenPath: string; private readonly clientInfoPath: string; @@ -53,7 +115,7 @@ class DirectoryPersistence implements OAuthPersistence { async saveTokens(tokens: OAuthTokens): Promise { await this.ensureDir(); - await writeJsonFile(this.tokenPath, tokens); + await writeJsonFile(this.tokenPath, withStoredExpiry(tokens)); this.logger?.debug?.(`Saved tokens to ${this.tokenPath}`); } @@ -131,7 +193,7 @@ class VaultPersistence implements OAuthPersistence { } async saveTokens(tokens: OAuthTokens): Promise { - await saveVaultEntry(this.definition, { tokens }); + await saveVaultEntry(this.definition, { tokens: withStoredExpiry(tokens) }); } async readClientInfo(): Promise { @@ -310,8 +372,43 @@ export async function readCachedAccessToken( ): Promise { const persistence = await buildOAuthPersistence(definition, logger); const tokens = await persistence.readTokens(); - if (tokens && typeof tokens.access_token === 'string' && tokens.access_token.trim().length > 0) { + if (!tokens || typeof tokens.access_token !== 'string' || tokens.access_token.trim().length === 0) { + return undefined; + } + if (!shouldRefreshCachedToken(tokens)) { + return tokens.access_token; + } + if (typeof tokens.refresh_token !== 'string' || tokens.refresh_token.trim().length === 0) { + return tokens.access_token; + } + try { + const clientInformation = buildStaticClientInformation(definition) ?? (await persistence.readClientInfo()); + if (!clientInformation) { + logger?.debug?.( + `Cached OAuth token for '${definition.name}' is expired, but no client information is available.` + ); + return tokens.access_token; + } + if (definition.command.kind !== 'http') { + return tokens.access_token; + } + const serverInfo = await discoverOAuthServerInfo(definition.command.url); + const resource = resourceForRefresh(definition.command.url, serverInfo.resourceMetadata); + const refreshed = await refreshAuthorization(serverInfo.authorizationServerUrl, { + metadata: serverInfo.authorizationServerMetadata, + clientInformation, + refreshToken: tokens.refresh_token, + ...(resource ? { resource } : {}), + }); + await persistence.saveTokens(refreshed); + logger?.debug?.(`Refreshed cached OAuth access token for '${definition.name}' (non-interactive).`); + return refreshed.access_token; + } catch (error) { + logger?.debug?.( + `Failed to refresh cached OAuth token for '${definition.name}' non-interactively: ${ + error instanceof Error ? error.message : String(error) + }` + ); return tokens.access_token; } - return undefined; } diff --git a/src/oauth.ts b/src/oauth.ts index 161c9c0..feb10a4 100644 --- a/src/oauth.ts +++ b/src/oauth.ts @@ -9,6 +9,7 @@ import type { OAuthTokens, } from '@modelcontextprotocol/sdk/shared/auth.js'; import type { ServerDefinition } from './config.js'; +import { buildStaticClientInformation } from './oauth-client-info.js'; import type { OAuthPersistence } from './oauth-persistence.js'; import { buildOAuthPersistence } from './oauth-persistence.js'; @@ -237,7 +238,7 @@ class PersistentOAuthClientProvider implements OAuthClientProvider { } async clientInformation(): Promise { - const staticClient = buildStaticClientInformation(this.definition, this.redirectUrlValue); + const staticClient = buildStaticClientInformation(this.definition, { redirectUrl: this.redirectUrlValue }); if (staticClient) { return staticClient; } @@ -358,38 +359,6 @@ function firstRedirectUri(client: OAuthClientInformationMixed | undefined): stri return typeof first === 'string' ? first : undefined; } -function buildStaticClientInformation( - definition: ServerDefinition, - redirectUrl: URL -): OAuthClientInformationMixed | undefined { - if (!definition.oauthClientId) { - return undefined; - } - const clientSecret = resolveOAuthClientSecret(definition); - const metadata = { - client_id: definition.oauthClientId, - ...(clientSecret ? { client_secret: clientSecret } : {}), - redirect_uris: [redirectUrl.toString()], - grant_types: ['authorization_code', 'refresh_token'], - response_types: ['code'], - ...(definition.oauthTokenEndpointAuthMethod - ? { token_endpoint_auth_method: definition.oauthTokenEndpointAuthMethod } - : {}), - }; - return metadata as OAuthClientInformationMixed; -} - -function resolveOAuthClientSecret(definition: ServerDefinition): string | undefined { - if (definition.oauthClientSecretEnv) { - const value = process.env[definition.oauthClientSecretEnv]; - if (!value) { - throw new Error(`Environment variable '${definition.oauthClientSecretEnv}' is required for OAuth client secret.`); - } - return value; - } - return definition.oauthClientSecret; -} - export const __oauthInternals = { openExternal, }; diff --git a/tests/oauth-persistence.test.ts b/tests/oauth-persistence.test.ts index c317ad4..1920a6f 100644 --- a/tests/oauth-persistence.test.ts +++ b/tests/oauth-persistence.test.ts @@ -4,9 +4,20 @@ import path from 'node:path'; import { afterEach, describe, expect, it, vi } from 'vitest'; import type { ServerDefinition } from '../src/config.js'; import { readJsonFile } from '../src/fs-json.js'; -import { buildOAuthPersistence, clearOAuthCaches } from '../src/oauth-persistence.js'; +import { buildOAuthPersistence, clearOAuthCaches, readCachedAccessToken } from '../src/oauth-persistence.js'; import { loadVaultEntry, vaultKeyForDefinition } from '../src/oauth-vault.js'; +const authMocks = vi.hoisted(() => ({ + discoverOAuthServerInfo: vi.fn(), + refreshAuthorization: vi.fn(), +})); + +vi.mock('@modelcontextprotocol/sdk/client/auth.js', async (importOriginal) => ({ + ...(await importOriginal()), + discoverOAuthServerInfo: authMocks.discoverOAuthServerInfo, + refreshAuthorization: authMocks.refreshAuthorization, +})); + const mkDef = (name: string, tokenCacheDir?: string): ServerDefinition => ({ name, description: `${name} server`, @@ -22,6 +33,7 @@ describe('oauth persistence', () => { let hasSpy = false; afterEach(async () => { + vi.clearAllMocks(); process.env = { ...originalEnv }; if (hasSpy) { homedirSpy.mockRestore(); @@ -164,4 +176,150 @@ describe('oauth persistence', () => { const entry = await loadVaultEntry(definition); expect(entry).toBeUndefined(); }); + + it('refreshes expired cached OAuth access tokens without starting a browser flow', async () => { + const tmp = await fs.mkdtemp(path.join(os.tmpdir(), 'mcporter-oauth-refresh-')); + tempRoots.push(tmp); + homedirSpy = vi.spyOn(os, 'homedir').mockReturnValue(tmp); + hasSpy = true; + + const cacheDir = path.join(tmp, 'cache'); + await fs.mkdir(cacheDir, { recursive: true }); + await fs.writeFile( + path.join(cacheDir, 'tokens.json'), + JSON.stringify({ + access_token: 'expired-token', + token_type: 'Bearer', + refresh_token: 'refresh-123', + expires_at: Math.floor(Date.now() / 1000) - 30, + }) + ); + await fs.writeFile(path.join(cacheDir, 'client.json'), JSON.stringify({ client_id: 'client-123' })); + + authMocks.discoverOAuthServerInfo.mockResolvedValue({ + authorizationServerUrl: 'https://auth.example.com', + authorizationServerMetadata: { token_endpoint: 'https://auth.example.com/token' }, + resourceMetadata: { resource: 'https://example.com/mcp' }, + }); + authMocks.refreshAuthorization.mockResolvedValue({ + access_token: 'fresh-token', + token_type: 'Bearer', + refresh_token: 'refresh-456', + expires_in: 3600, + }); + + const definition = mkDef('refresh-service', cacheDir); + await expect(readCachedAccessToken(definition)).resolves.toBe('fresh-token'); + + expect(authMocks.refreshAuthorization).toHaveBeenCalledWith( + 'https://auth.example.com', + expect.objectContaining({ + clientInformation: { client_id: 'client-123' }, + refreshToken: 'refresh-123', + resource: new URL('https://example.com/mcp'), + }) + ); + const persisted = (await readJsonFile(path.join(cacheDir, 'tokens.json'))) as + | { access_token?: string; refresh_token?: string; expires_at?: number } + | undefined; + expect(persisted?.access_token).toBe('fresh-token'); + expect(persisted?.refresh_token).toBe('refresh-456'); + expect(persisted?.expires_at).toBeGreaterThan(Math.floor(Date.now() / 1000)); + }); + + it('omits OAuth resource during silent refresh when protected-resource metadata is absent', async () => { + const tmp = await fs.mkdtemp(path.join(os.tmpdir(), 'mcporter-oauth-refresh-no-resource-')); + tempRoots.push(tmp); + homedirSpy = vi.spyOn(os, 'homedir').mockReturnValue(tmp); + hasSpy = true; + + const cacheDir = path.join(tmp, 'cache'); + await fs.mkdir(cacheDir, { recursive: true }); + await fs.writeFile( + path.join(cacheDir, 'tokens.json'), + JSON.stringify({ + access_token: 'expired-token', + token_type: 'Bearer', + refresh_token: 'refresh-123', + expires_at: Math.floor(Date.now() / 1000) - 30, + }) + ); + await fs.writeFile(path.join(cacheDir, 'client.json'), JSON.stringify({ client_id: 'client-123' })); + + authMocks.discoverOAuthServerInfo.mockResolvedValue({ + authorizationServerUrl: 'https://auth.example.com', + authorizationServerMetadata: { token_endpoint: 'https://auth.example.com/token' }, + }); + authMocks.refreshAuthorization.mockResolvedValue({ + access_token: 'fresh-token', + token_type: 'Bearer', + refresh_token: 'refresh-456', + expires_in: 3600, + }); + + const definition = mkDef('refresh-without-resource-service', cacheDir); + await expect(readCachedAccessToken(definition)).resolves.toBe('fresh-token'); + + const [, options] = authMocks.refreshAuthorization.mock.calls[0] ?? []; + expect(options).not.toHaveProperty('resource'); + }); + + it('keeps the original cached OAuth token when silent refresh fails', async () => { + const tmp = await fs.mkdtemp(path.join(os.tmpdir(), 'mcporter-oauth-refresh-fail-')); + tempRoots.push(tmp); + homedirSpy = vi.spyOn(os, 'homedir').mockReturnValue(tmp); + hasSpy = true; + + const cacheDir = path.join(tmp, 'cache'); + await fs.mkdir(cacheDir, { recursive: true }); + await fs.writeFile( + path.join(cacheDir, 'tokens.json'), + JSON.stringify({ + access_token: 'expired-token', + token_type: 'Bearer', + refresh_token: 'refresh-123', + expires_at: Math.floor(Date.now() / 1000) - 30, + }) + ); + await fs.writeFile(path.join(cacheDir, 'client.json'), JSON.stringify({ client_id: 'client-123' })); + + authMocks.discoverOAuthServerInfo.mockResolvedValue({ authorizationServerUrl: 'https://auth.example.com' }); + authMocks.refreshAuthorization.mockRejectedValue(new Error('invalid_grant')); + + const definition = mkDef('refresh-fail-service', cacheDir); + await expect(readCachedAccessToken(definition)).resolves.toBe('expired-token'); + + const persisted = (await readJsonFile(path.join(cacheDir, 'tokens.json'))) as + | { access_token?: string; refresh_token?: string } + | undefined; + expect(persisted).toEqual( + expect.objectContaining({ + access_token: 'expired-token', + refresh_token: 'refresh-123', + }) + ); + }); + + it('uses unexpired cached OAuth tokens without refresh', async () => { + const tmp = await fs.mkdtemp(path.join(os.tmpdir(), 'mcporter-oauth-current-')); + tempRoots.push(tmp); + homedirSpy = vi.spyOn(os, 'homedir').mockReturnValue(tmp); + hasSpy = true; + + const cacheDir = path.join(tmp, 'cache'); + await fs.mkdir(cacheDir, { recursive: true }); + await fs.writeFile( + path.join(cacheDir, 'tokens.json'), + JSON.stringify({ + access_token: 'current-token', + token_type: 'Bearer', + refresh_token: 'refresh-123', + expires_at: Math.floor(Date.now() / 1000) + 3600, + }) + ); + + const definition = mkDef('current-service', cacheDir); + await expect(readCachedAccessToken(definition)).resolves.toBe('current-token'); + expect(authMocks.refreshAuthorization).not.toHaveBeenCalled(); + }); });