Skip to content

Commit 80ffefc

Browse files
Copilotrickyrombo
andauthored
Add hasRefreshToken guard to token refresh middleware (#13858)
`addTokenRefreshMiddleware` was calling `oauth.refreshAccessToken()` on every 401, including unauthenticated requests, causing `_surfaceError('No refresh token available.')` to fire as noise on normal 401 responses. ## Changes - **`OAuth.hasRefreshToken` getter** — cheap boolean check on `tokenStore?.refreshToken` - **Middleware guard** — short-circuits on 401 before attempting refresh when `hasRefreshToken` is false, avoiding the noisy error callback entirely - **Test coverage** — asserts `refreshAccessToken` is never called when `hasRefreshToken` is false ```ts // Before: refreshAccessToken() called on every 401 → _surfaceError fired for unauthenticated clients // After: guard skips refresh entirely when no token is stored if (!oauth.hasRefreshToken) { return context.response } ``` <!-- START COPILOT CODING AGENT TIPS --> --- 🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. [Learn more about Advanced Security.](https://gh.io/cca-advanced-security) --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: rickyrombo <3690498+rickyrombo@users.noreply.github.com>
1 parent e800370 commit 80ffefc

3 files changed

Lines changed: 36 additions & 1 deletion

File tree

packages/sdk/src/sdk/middleware/addTokenRefreshMiddleware.test.ts

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,11 @@ function mockResponse(status: number, body?: object): Response {
1313
}
1414

1515
function createMockOAuth(
16-
refreshBehaviour: () => Promise<string | null>
16+
refreshBehaviour: () => Promise<string | null>,
17+
hasRefreshToken = true
1718
): OAuth {
1819
return {
20+
hasRefreshToken,
1921
refreshAccessToken: refreshBehaviour
2022
} as unknown as OAuth
2123
}
@@ -40,6 +42,23 @@ describe('addTokenRefreshMiddleware', () => {
4042
expect(result).toBe(response)
4143
})
4244

45+
it('passes through 401 without calling refreshAccessToken when unauthenticated', async () => {
46+
const refreshFn = vi.fn()
47+
const oauth = createMockOAuth(refreshFn, false)
48+
const mw = addTokenRefreshMiddleware({ oauth })
49+
const response = mockResponse(401)
50+
51+
const result = await mw.post!({
52+
fetch,
53+
url: 'https://api.example.com/v1/users/me',
54+
init: {},
55+
response
56+
})
57+
58+
expect(result).toBe(response)
59+
expect(refreshFn).not.toHaveBeenCalled()
60+
})
61+
4362
it('passes through 401 when refresh returns null (no refresh token)', async () => {
4463
const oauth = createMockOAuth(async () => null)
4564
const mw = addTokenRefreshMiddleware({ oauth })

packages/sdk/src/sdk/middleware/addTokenRefreshMiddleware.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,9 @@ import type { OAuth } from '../oauth/OAuth'
88
* `OAuth.refreshAccessToken()` which checks for a refresh token, performs the
99
* HTTP exchange, and updates the token store. On success the original request
1010
* is retried with the fresh access token. On failure the 401 propagates.
11+
*
12+
* When the client is unauthenticated (no refresh token stored) the middleware
13+
* short-circuits immediately, avoiding noisy error callbacks.
1114
*/
1215
export const addTokenRefreshMiddleware = ({
1316
oauth
@@ -22,6 +25,11 @@ export const addTokenRefreshMiddleware = ({
2225
return context.response
2326
}
2427

28+
// Skip refresh when unauthenticated to avoid noisy error callbacks.
29+
if (!oauth.hasRefreshToken) {
30+
return context.response
31+
}
32+
2533
// Coalesce concurrent 401s into a single refresh call.
2634
if (!refreshInFlight) {
2735
refreshInFlight = oauth

packages/sdk/src/sdk/oauth/OAuth.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -483,6 +483,14 @@ export class OAuth {
483483
}
484484
}
485485

486+
/**
487+
* Returns true if a refresh token is currently stored and a refresh
488+
* exchange could be attempted.
489+
*/
490+
get hasRefreshToken(): boolean {
491+
return !!(this.config.tokenStore?.refreshToken)
492+
}
493+
486494
/**
487495
* Refresh the access token using the stored refresh token.
488496
* Updates the token store on success.

0 commit comments

Comments
 (0)