Skip to content

fix(core): validate oauth expires_in values #5356

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
wenshao merged 1 commit into from
Jun 18, 2026
Merged

fix(core): validate oauth expires_in values #5356

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
159 changes: 159 additions & 0 deletions packages/core/src/mcp/oauth-provider.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,54 @@ describe('MCPOAuthProvider', () => {
);
});

it('should preserve expires_in=0 as an immediate expiry', async () => {
vi.spyOn(Date, 'now').mockReturnValue(1_700_000_000_000);

let callbackHandler: unknown;
vi.mocked(http.createServer).mockImplementation((handler) => {
callbackHandler = handler;
return mockHttpServer as unknown as http.Server;
});

mockHttpServer.listen.mockImplementation((port, callback) => {
callback?.();
setTimeout(() => {
const mockReq = {
url: '/oauth/callback?code=auth_code_123&state=bW9ja19zdGF0ZV8xNl9ieXRlcw',
};
const mockRes = {
writeHead: vi.fn(),
end: vi.fn(),
};
(callbackHandler as (req: unknown, res: unknown) => void)(
mockReq,
mockRes,
);
}, 10);
});

mockFetch.mockResolvedValueOnce(
createMockResponse({
ok: true,
contentType: 'application/x-www-form-urlencoded',
text: 'access_token=access_token_123&token_type=Bearer&expires_in=0&refresh_token=refresh_token_456',
}),
);

const authProvider = new MCPOAuthProvider();
const result = await authProvider.authenticate('test-server', mockConfig);

expect(result.expiresAt).toBe(1_700_000_000_000);
const tokenStorage = new MCPOAuthTokenStorage();
expect(tokenStorage.saveToken).toHaveBeenCalledWith(
'test-server',
expect.objectContaining({ expiresAt: 1_700_000_000_000 }),
'test-client-id',
'https://auth.example.com/token',
undefined,
);
});

it('should handle OAuth discovery when no authorization URL provided', async () => {
// Use a mutable config object
const configWithoutAuth: MCPOAuthConfig = {
Expand Down Expand Up @@ -785,6 +833,52 @@ describe('MCPOAuthProvider', () => {
);
});

it('should normalize JSON string expires_in values', async () => {
mockFetch.mockResolvedValueOnce(
createMockResponse({
ok: true,
contentType: 'application/json',
text: JSON.stringify({
access_token: 'new_access_token',
token_type: 'Bearer',
expires_in: '3600',
}),
}),
);

const authProvider = new MCPOAuthProvider();
const result = await authProvider.refreshAccessToken(
mockConfig,
'refresh_token',
'https://auth.example.com/token',
);

expect(result.expires_in).toBe(3600);
});

it('should reject malformed JSON expires_in values', async () => {
mockFetch.mockResolvedValueOnce(
createMockResponse({
ok: true,
contentType: 'application/json',
text: JSON.stringify({
access_token: 'new_access_token',
token_type: 'Bearer',
expires_in: '3600abc',
}),
}),
);

const authProvider = new MCPOAuthProvider();
await expect(
authProvider.refreshAccessToken(
mockConfig,
'refresh_token',
'https://auth.example.com/token',
),
).rejects.toThrow('Invalid expires_in value');
});

it('should include client secret in refresh request when available', async () => {
mockFetch.mockResolvedValueOnce(
createMockResponse({
Expand Down Expand Up @@ -827,6 +921,25 @@ describe('MCPOAuthProvider', () => {
'Token refresh failed: invalid_request - Invalid refresh token',
);
});

it('should reject malformed form-urlencoded expires_in values', async () => {
mockFetch.mockResolvedValueOnce(
createMockResponse({
ok: true,
contentType: 'application/x-www-form-urlencoded',
text: 'access_token=new_access_token&token_type=Bearer&expires_in=3600abc',
}),
);

const authProvider = new MCPOAuthProvider();
await expect(
authProvider.refreshAccessToken(
mockConfig,
'refresh_token',
'https://auth.example.com/token',
),
).rejects.toThrow('Invalid expires_in value');
});
});

describe('getValidToken', () => {
Expand Down Expand Up @@ -901,6 +1014,52 @@ describe('MCPOAuthProvider', () => {
);
});

it('should preserve expires_in=0 when refreshing an expired token', async () => {
vi.spyOn(Date, 'now').mockReturnValue(1_700_000_000_000);

const expiredCredentials = {
serverName: 'test-server',
token: { ...mockToken, expiresAt: Date.now() - 3600000 },
clientId: 'test-client-id',
tokenUrl: 'https://auth.example.com/token',
updatedAt: Date.now(),
};

const tokenStorage = new MCPOAuthTokenStorage();
vi.mocked(tokenStorage.getCredentials).mockResolvedValue(
expiredCredentials,
);
vi.mocked(tokenStorage.isTokenExpired).mockReturnValue(true);

mockFetch.mockResolvedValueOnce(
createMockResponse({
ok: true,
contentType: 'application/json',
text: JSON.stringify({
access_token: 'new_access_token',
token_type: 'Bearer',
expires_in: 0,
refresh_token: 'new_refresh_token',
}),
}),
);

const authProvider = new MCPOAuthProvider();
const result = await authProvider.getValidToken(
'test-server',
mockConfig,
);

expect(result).toBe('new_access_token');
expect(tokenStorage.saveToken).toHaveBeenCalledWith(
'test-server',
expect.objectContaining({ expiresAt: 1_700_000_000_000 }),
'test-client-id',
'https://auth.example.com/token',
undefined,
);
});

it('should return null when no credentials exist', async () => {
const tokenStorage = new MCPOAuthTokenStorage();
vi.mocked(tokenStorage.getCredentials).mockResolvedValue(null);
Expand Down
66 changes: 56 additions & 10 deletions packages/core/src/mcp/oauth-provider.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,34 @@ export interface OAuthTokenResponse {
scope?: string;
}

function parseExpiresIn(value: unknown): number | undefined {
if (value === undefined || value === null) {
return undefined;
}

const seconds =
typeof value === 'number'
? value
: typeof value === 'string' && /^\d+$/.test(value.trim())
? Number(value.trim())
: NaN;

if (!Number.isSafeInteger(seconds) || seconds < 0) {
throw new Error(`Invalid expires_in value: ${String(value)}`);
}

return seconds;
}

function normalizeTokenResponse(
response: OAuthTokenResponse,
): OAuthTokenResponse {
return {
...response,
expires_in: parseExpiresIn(response.expires_in),
};
}

/**
* Dynamic client registration request.
*/
Expand Down Expand Up @@ -496,8 +524,16 @@ export class MCPOAuthProvider {

// Try to parse as JSON first, fall back to form-urlencoded
try {
return JSON.parse(responseText) as OAuthTokenResponse;
} catch {
return normalizeTokenResponse(
JSON.parse(responseText) as OAuthTokenResponse,
);
} catch (error) {
if (
error instanceof Error &&
error.message.startsWith('Invalid expires_in value')
) {
throw error;
}
// Parse form-urlencoded response
const tokenParams = new URLSearchParams(responseText);
const accessToken = tokenParams.get('access_token');
Expand All @@ -518,7 +554,7 @@ export class MCPOAuthProvider {
return {
access_token: accessToken,
token_type: tokenType,
expires_in: expiresIn ? parseInt(expiresIn, 10) : undefined,
expires_in: parseExpiresIn(expiresIn),
refresh_token: refreshToken || undefined,
scope: scope || undefined,
} as OAuthTokenResponse;
Expand Down Expand Up @@ -618,8 +654,16 @@ export class MCPOAuthProvider {

// Try to parse as JSON first, fall back to form-urlencoded
try {
return JSON.parse(responseText) as OAuthTokenResponse;
} catch {
return normalizeTokenResponse(
JSON.parse(responseText) as OAuthTokenResponse,
);
} catch (error) {
if (
error instanceof Error &&
error.message.startsWith('Invalid expires_in value')
) {
throw error;
}
// Parse form-urlencoded response
const tokenParams = new URLSearchParams(responseText);
const accessToken = tokenParams.get('access_token');
Expand All @@ -640,7 +684,7 @@ export class MCPOAuthProvider {
return {
access_token: accessToken,
token_type: tokenType,
expires_in: expiresIn ? parseInt(expiresIn, 10) : undefined,
expires_in: parseExpiresIn(expiresIn),
refresh_token: refreshToken || undefined,
scope: scope || undefined,
} as OAuthTokenResponse;
Expand Down Expand Up @@ -866,8 +910,9 @@ export class MCPOAuthProvider {
scope: tokenResponse.scope,
};

if (tokenResponse.expires_in) {
token.expiresAt = Date.now() + tokenResponse.expires_in * 1000;
const expiresIn = parseExpiresIn(tokenResponse.expires_in);
if (expiresIn !== undefined) {
token.expiresAt = Date.now() + expiresIn * 1000;
}

// Save token
Expand Down Expand Up @@ -958,8 +1003,9 @@ export class MCPOAuthProvider {
scope: newTokenResponse.scope || token.scope,
};

if (newTokenResponse.expires_in) {
newToken.expiresAt = Date.now() + newTokenResponse.expires_in * 1000;
const expiresIn = parseExpiresIn(newTokenResponse.expires_in);
if (expiresIn !== undefined) {
newToken.expiresAt = Date.now() + expiresIn * 1000;
}

await this.tokenStorage.saveToken(
Expand Down
Loading