Skip to content

Commit c35d16f

Browse files
committed
fix: surface OAuth token persistence failures
1 parent 5fc42e9 commit c35d16f

3 files changed

Lines changed: 79 additions & 4 deletions

File tree

.changeset/forty-mayflies-tease.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@modelcontextprotocol/client": patch
3+
---
4+
5+
fix(client): surface OAuth token persistence failures

packages/client/src/client/auth.ts

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -763,19 +763,17 @@ async function authInternal(
763763

764764
// Handle token refresh or new authorization
765765
if (tokens?.refresh_token) {
766+
let newTokens: OAuthTokens | undefined;
766767
try {
767768
// Attempt to refresh the token
768-
const newTokens = await refreshAuthorization(authorizationServerUrl, {
769+
newTokens = await refreshAuthorization(authorizationServerUrl, {
769770
metadata,
770771
clientInformation,
771772
refreshToken: tokens.refresh_token,
772773
resource,
773774
addClientAuthentication: provider.addClientAuthentication,
774775
fetchFn
775776
});
776-
777-
await provider.saveTokens(newTokens);
778-
return 'AUTHORIZED';
779777
} catch (error) {
780778
// If this is a ServerError, or an unknown type, log it out and try to continue. Otherwise, escalate so we can fix things and retry.
781779
if (!(error instanceof OAuthError) || error.code === OAuthErrorCode.ServerError) {
@@ -785,6 +783,11 @@ async function authInternal(
785783
throw error;
786784
}
787785
}
786+
787+
if (newTokens !== undefined) {
788+
await provider.saveTokens(newTokens);
789+
return 'AUTHORIZED';
790+
}
788791
}
789792

790793
const state = provider.state ? await provider.state() : undefined;

packages/client/test/client/auth.test.ts

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2591,6 +2591,73 @@ describe('OAuth Authorization', () => {
25912591
expect(body.get('refresh_token')).toBe('refresh123');
25922592
});
25932593

2594+
it('does not hide token persistence failures after refresh succeeds', async () => {
2595+
mockFetch.mockImplementation(url => {
2596+
const urlString = url.toString();
2597+
2598+
if (urlString.includes('/.well-known/oauth-protected-resource')) {
2599+
return Promise.resolve({
2600+
ok: true,
2601+
status: 200,
2602+
json: async () => ({
2603+
resource: 'https://api.example.com/mcp-server',
2604+
authorization_servers: ['https://auth.example.com']
2605+
})
2606+
});
2607+
} else if (urlString.includes('/.well-known/oauth-authorization-server')) {
2608+
return Promise.resolve({
2609+
ok: true,
2610+
status: 200,
2611+
json: async () => ({
2612+
issuer: 'https://auth.example.com',
2613+
authorization_endpoint: 'https://auth.example.com/authorize',
2614+
token_endpoint: 'https://auth.example.com/token',
2615+
response_types_supported: ['code'],
2616+
code_challenge_methods_supported: ['S256']
2617+
})
2618+
});
2619+
} else if (urlString.includes('/token')) {
2620+
return Promise.resolve({
2621+
ok: true,
2622+
status: 200,
2623+
json: async () => ({
2624+
access_token: 'new-access123',
2625+
token_type: 'Bearer',
2626+
expires_in: 3600,
2627+
refresh_token: 'new-refresh123'
2628+
})
2629+
});
2630+
}
2631+
2632+
return Promise.resolve({ ok: false, status: 404 });
2633+
});
2634+
2635+
const saveError = new Error('token store unavailable');
2636+
(mockProvider.clientInformation as Mock).mockResolvedValue({
2637+
client_id: 'test-client',
2638+
client_secret: 'test-secret'
2639+
});
2640+
(mockProvider.tokens as Mock).mockResolvedValue({
2641+
access_token: 'old-access',
2642+
refresh_token: 'refresh123'
2643+
});
2644+
(mockProvider.saveTokens as Mock).mockRejectedValue(saveError);
2645+
2646+
await expect(
2647+
auth(mockProvider, {
2648+
serverUrl: 'https://api.example.com/mcp-server'
2649+
})
2650+
).rejects.toThrow('token store unavailable');
2651+
2652+
expect(mockProvider.saveTokens).toHaveBeenCalledWith(
2653+
expect.objectContaining({
2654+
access_token: 'new-access123',
2655+
refresh_token: 'new-refresh123'
2656+
})
2657+
);
2658+
expect(mockProvider.redirectToAuthorization).not.toHaveBeenCalled();
2659+
});
2660+
25942661
it('skips default PRM resource validation when custom validateResourceURL is provided', async () => {
25952662
const mockValidateResourceURL = vi.fn().mockResolvedValue(undefined);
25962663
const providerWithCustomValidation = {

0 commit comments

Comments
 (0)