fix(oauth): refresh expired cached access tokens
This commit is contained in:
parent
0ea394356f
commit
f9f60d7cc4
@ -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
|
||||
|
||||
|
||||
33
src/oauth-client-info.ts
Normal file
33
src/oauth-client-info.ts
Normal file
@ -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;
|
||||
}
|
||||
@ -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<void>;
|
||||
}
|
||||
|
||||
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<void> {
|
||||
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<void> {
|
||||
await saveVaultEntry(this.definition, { tokens });
|
||||
await saveVaultEntry(this.definition, { tokens: withStoredExpiry(tokens) });
|
||||
}
|
||||
|
||||
async readClientInfo(): Promise<OAuthClientInformationMixed | undefined> {
|
||||
@ -310,8 +372,43 @@ export async function readCachedAccessToken(
|
||||
): Promise<string | undefined> {
|
||||
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;
|
||||
}
|
||||
|
||||
35
src/oauth.ts
35
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<OAuthClientInformationMixed | undefined> {
|
||||
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,
|
||||
};
|
||||
|
||||
@ -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<typeof import('@modelcontextprotocol/sdk/client/auth.js')>()),
|
||||
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();
|
||||
});
|
||||
});
|
||||
|
||||
Loading…
Reference in New Issue
Block a user