diff --git a/packages/client/src/client/auth.ts b/packages/client/src/client/auth.ts index 5f55fb7a08..1a3ab130f2 100644 --- a/packages/client/src/client/auth.ts +++ b/packages/client/src/client/auth.ts @@ -763,9 +763,10 @@ async function authInternal( // Handle token refresh or new authorization if (tokens?.refresh_token) { + let newTokens: OAuthTokens | undefined; try { // Attempt to refresh the token - const newTokens = await refreshAuthorization(authorizationServerUrl, { + newTokens = await refreshAuthorization(authorizationServerUrl, { metadata, clientInformation, refreshToken: tokens.refresh_token, @@ -773,9 +774,6 @@ async function authInternal( addClientAuthentication: provider.addClientAuthentication, fetchFn }); - - await provider.saveTokens(newTokens); - return 'AUTHORIZED'; } catch (error) { // 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. if (!(error instanceof OAuthError) || error.code === OAuthErrorCode.ServerError) { @@ -785,6 +783,11 @@ async function authInternal( throw error; } } + + if (newTokens !== undefined) { + await provider.saveTokens(newTokens); + return 'AUTHORIZED'; + } } const state = provider.state ? await provider.state() : undefined; diff --git a/packages/client/test/client/auth.test.ts b/packages/client/test/client/auth.test.ts index 04d7f4a3fb..f27367d664 100644 --- a/packages/client/test/client/auth.test.ts +++ b/packages/client/test/client/auth.test.ts @@ -2591,6 +2591,73 @@ describe('OAuth Authorization', () => { expect(body.get('refresh_token')).toBe('refresh123'); }); + it('does not hide token persistence failures after refresh succeeds', async () => { + mockFetch.mockImplementation(url => { + const urlString = url.toString(); + + if (urlString.includes('/.well-known/oauth-protected-resource')) { + return Promise.resolve({ + ok: true, + status: 200, + json: async () => ({ + resource: 'https://api.example.com/mcp-server', + authorization_servers: ['https://auth.example.com'] + }) + }); + } else if (urlString.includes('/.well-known/oauth-authorization-server')) { + return Promise.resolve({ + ok: true, + status: 200, + json: async () => ({ + issuer: 'https://auth.example.com', + authorization_endpoint: 'https://auth.example.com/authorize', + token_endpoint: 'https://auth.example.com/token', + response_types_supported: ['code'], + code_challenge_methods_supported: ['S256'] + }) + }); + } else if (urlString.includes('/token')) { + return Promise.resolve({ + ok: true, + status: 200, + json: async () => ({ + access_token: 'new-access123', + token_type: 'Bearer', + expires_in: 3600, + refresh_token: 'new-refresh123' + }) + }); + } + + return Promise.resolve({ ok: false, status: 404 }); + }); + + const saveError = new Error('token store unavailable'); + (mockProvider.clientInformation as Mock).mockResolvedValue({ + client_id: 'test-client', + client_secret: 'test-secret' + }); + (mockProvider.tokens as Mock).mockResolvedValue({ + access_token: 'old-access', + refresh_token: 'refresh123' + }); + (mockProvider.saveTokens as Mock).mockRejectedValue(saveError); + + await expect( + auth(mockProvider, { + serverUrl: 'https://api.example.com/mcp-server' + }) + ).rejects.toThrow('token store unavailable'); + + expect(mockProvider.saveTokens).toHaveBeenCalledWith( + expect.objectContaining({ + access_token: 'new-access123', + refresh_token: 'new-refresh123' + }) + ); + expect(mockProvider.redirectToAuthorization).not.toHaveBeenCalled(); + }); + it('skips default PRM resource validation when custom validateResourceURL is provided', async () => { const mockValidateResourceURL = vi.fn().mockResolvedValue(undefined); const providerWithCustomValidation = {