From b156881f5b45bd474d0e48e8479a16c4f9ed90d6 Mon Sep 17 00:00:00 2001
From: jbsky <webmaster@jbsky.fr>
Date: Fri, 8 May 2026 18:50:54 +0000
Subject: [PATCH] fix(auth): propagate saveTokens errors during token refresh

Previously, auth() wrapped both refreshAuthorization() and
saveTokens() in the same try/catch. Non-OAuthError exceptions
(including I/O errors from saveTokens) were silently discarded.

This is destructive when the AS uses rotating refresh tokens:
the old RT is invalidated server-side upon issuing a new one,
but since saveTokens failure was swallowed, the new tokens were
lost. The client was left with an invalid refresh token and had
to fully re-authenticate.

Fix: move saveTokens() outside the try/catch so its errors
propagate naturally. Only refreshAuthorization() failures are
caught and allow fallthrough to a new authorization flow.

Add test verifying saveTokens errors are propagated.
---
 packages/client/src/client/auth.ts       | 16 ++++++---
 packages/client/test/client/auth.test.ts | 43 ++++++++++++++++++++++++
 2 files changed, 54 insertions(+), 5 deletions(-)

diff --git a/packages/client/src/client/auth.ts b/packages/client/src/client/auth.ts
index 5f55fb7a08..7f989b5c0c 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,18 +774,23 @@ 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) {
-                // Could not refresh OAuth tokens
+                // Could not refresh OAuth tokens — fall through to new authorization
             } else {
                 // Refresh failed for another reason, re-throw
                 throw error;
             }
         }
+
+        if (newTokens) {
+            // saveTokens errors must propagate — the AS may have already
+            // rotated the refresh token, so silently losing the new tokens
+            // would leave the client with an invalid refresh token.
+            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..417dc27457 100644
--- a/packages/client/test/client/auth.test.ts
+++ b/packages/client/test/client/auth.test.ts
@@ -1422,6 +1422,49 @@ describe('OAuth Authorization', () => {
             expect(prmCalls).toHaveLength(1);
             expect(prmCalls[0]![0].toString()).toBe(cachedPrmUrl);
         });
+
+        it('propagates saveTokens errors instead of silently swallowing them during refresh', async () => {
+            const saveTokensError = new Error('Filesystem write failed');
+            const provider = createMockProvider({
+                discoveryState: vi.fn().mockResolvedValue({
+                    authorizationServerUrl: 'https://auth.example.com',
+                    resourceMetadata: validResourceMetadata,
+                    authorizationServerMetadata: validAuthMetadata
+                }),
+                tokens: vi.fn().mockResolvedValue({
+                    access_token: 'old-access',
+                    refresh_token: 'refresh-token',
+                    token_type: 'bearer'
+                }),
+                saveTokens: vi.fn().mockRejectedValue(saveTokensError)
+            });
+
+            mockFetch.mockImplementation(url => {
+                const urlString = url.toString();
+
+                if (urlString.includes('/token')) {
+                    return Promise.resolve({
+                        ok: true,
+                        status: 200,
+                        json: async () => ({
+                            access_token: 'new-token',
+                            token_type: 'bearer',
+                            expires_in: 3600,
+                            refresh_token: 'new-refresh-token'
+                        })
+                    });
+                }
+
+                return Promise.reject(new Error(`Unexpected fetch: ${urlString}`));
+            });
+
+            // saveTokens failure must propagate — not be silently swallowed
+            await expect(auth(provider, { serverUrl: 'https://resource.example.com' })).rejects.toThrow('Filesystem write failed');
+
+            // Verify the refresh exchange itself succeeded (token endpoint was called)
+            const tokenCall = mockFetch.mock.calls.find(call => call[0].toString().includes('/token'));
+            expect(tokenCall).toBeDefined();
+        });
     });
 
     describe('selectClientAuthMethod', () => {