Skip to content

Commit fa125f2

Browse files
committed
fix: propagate refresh token persistence errors
1 parent 5fc42e9 commit fa125f2

2 files changed

Lines changed: 76 additions & 4 deletions

File tree

packages/client/src/client/auth.ts

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

764764
// Handle token refresh or new authorization
765765
if (tokens?.refresh_token) {
766+
let newTokens: OAuthTokens | undefined;
767+
766768
try {
767769
// Attempt to refresh the token
768-
const newTokens = await refreshAuthorization(authorizationServerUrl, {
770+
newTokens = await refreshAuthorization(authorizationServerUrl, {
769771
metadata,
770772
clientInformation,
771773
refreshToken: tokens.refresh_token,
772774
resource,
773775
addClientAuthentication: provider.addClientAuthentication,
774776
fetchFn
775777
});
776-
777-
await provider.saveTokens(newTokens);
778-
return 'AUTHORIZED';
779778
} catch (error) {
780779
// 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.
781780
if (!(error instanceof OAuthError) || error.code === OAuthErrorCode.ServerError) {
@@ -785,6 +784,11 @@ async function authInternal(
785784
throw error;
786785
}
787786
}
787+
788+
if (newTokens) {
789+
await provider.saveTokens(newTokens);
790+
return 'AUTHORIZED';
791+
}
788792
}
789793

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

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

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

2594+
it('propagates token persistence errors after a successful refresh', 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('could not persist refreshed tokens');
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).mockRejectedValueOnce(saveError);
2645+
(mockProvider.redirectToAuthorization as Mock).mockResolvedValue(undefined);
2646+
2647+
await expect(
2648+
auth(mockProvider, {
2649+
serverUrl: 'https://api.example.com/mcp-server'
2650+
})
2651+
).rejects.toThrow('could not persist refreshed tokens');
2652+
2653+
expect(mockProvider.saveTokens).toHaveBeenCalledWith(
2654+
expect.objectContaining({
2655+
access_token: 'new-access123',
2656+
refresh_token: 'new-refresh123'
2657+
})
2658+
);
2659+
expect(mockProvider.redirectToAuthorization).not.toHaveBeenCalled();
2660+
});
2661+
25942662
it('skips default PRM resource validation when custom validateResourceURL is provided', async () => {
25952663
const mockValidateResourceURL = vi.fn().mockResolvedValue(undefined);
25962664
const providerWithCustomValidation = {

0 commit comments

Comments
 (0)