diff --git a/CHANGELOG.md b/CHANGELOG.md index cf7a3bd..1bf1a43 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,21 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 _No changes yet._ +## [1.2.1] — 2026-07-22 + +### Fixed + +- **`BearerTokenAuth` no longer discards a valid token when a refresh yields + nothing.** On a 401/403, `onAuthError` invoked the refresh callback and then + called `store.setToken(newToken ?? null)` — so a refresh that returned `null` + (no refresh configured, or an error a refresh cannot fix, such as a permission + `403`) overwrote the still-valid token with `null`. Every subsequent request + then went out with no `Authorization` header and failed with `401`, turning a + single permission error into a cascade of unauthenticated failures across + unrelated requests. The stored token is now left untouched unless the refresh + actually produces a new one; the failing request still surfaces its original + error. Regression test added. + ## [1.2.0] — 2026-07-22 Adds an optional client-side request governor for staying within an upstream @@ -421,7 +436,8 @@ PKCE refresh recovery, full type-system rigor across primitive bodies and JSON encoding, and `duplex: 'half'` propagation for `ReadableStream` bodies — is captured here as the canonical first set of release notes. -[Unreleased]: https://github.com/erelsop/fetch-enh/compare/v1.2.0...HEAD +[Unreleased]: https://github.com/erelsop/fetch-enh/compare/v1.2.1...HEAD +[1.2.1]: https://github.com/erelsop/fetch-enh/compare/v1.2.0...v1.2.1 [1.2.0]: https://github.com/erelsop/fetch-enh/compare/v1.1.0...v1.2.0 [1.1.0]: https://github.com/erelsop/fetch-enh/compare/v1.0.0...v1.1.0 [1.0.0]: https://github.com/erelsop/fetch-enh/releases/tag/v1.0.0 diff --git a/package-lock.json b/package-lock.json index 0194932..de16e59 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@erelsop/fetch-enh", - "version": "1.2.0", + "version": "1.2.1", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@erelsop/fetch-enh", - "version": "1.2.0", + "version": "1.2.1", "license": "MIT", "devDependencies": { "@types/jest": "^29.5.6", diff --git a/package.json b/package.json index bb24539..bd223b2 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@erelsop/fetch-enh", - "version": "1.2.0", + "version": "1.2.1", "description": "An enhanced fetch utility with built-in retry logic, authentication strategies, request/response interceptors, and intelligent error handling.", "main": "dist/index.js", "types": "dist/index.d.ts", diff --git a/src/auth/strategies.ts b/src/auth/strategies.ts index 0a65001..fd93ea9 100644 --- a/src/auth/strategies.ts +++ b/src/auth/strategies.ts @@ -112,7 +112,13 @@ export class BearerTokenAuth implements AuthStrategy { if (!this.refreshingPromise) { this.refreshingPromise = this.refresh() .then(async (newToken) => { - await this.store.setToken(newToken ?? null); + // Only persist a *successful* refresh. A null/empty result means the + // refresh could not produce a new token (e.g. no refresh configured, + // or a 403 that a refresh cannot fix) — in that case leave the stored + // token untouched. Overwriting it with null would strip the still-valid + // token from every subsequent request, turning one 401/403 into a + // cascade of unauthenticated failures. + if (newToken) await this.store.setToken(newToken); return newToken ?? null; }) .finally(() => { @@ -120,7 +126,7 @@ export class BearerTokenAuth implements AuthStrategy { }); } const token = await this.refreshingPromise; - if (!token) return; // give up; let caller handle error + if (!token) return; // give up; let caller handle the original error const headers = new Headers(request.headers); headers.set('Authorization', `Bearer ${token}`); diff --git a/tests/fetchEnh.critical.test.ts b/tests/fetchEnh.critical.test.ts index 3a3b937..a584f69 100644 --- a/tests/fetchEnh.critical.test.ts +++ b/tests/fetchEnh.critical.test.ts @@ -464,4 +464,29 @@ describe('concurrent auth-refresh deduplication', () => { // Both callers should have received a valid retry response expect(retryFn).toHaveBeenCalledTimes(2); }); + + test('BearerTokenAuth: a null refresh result preserves the existing token (no clobber)', async () => { + // A no-op refresh (returns null) must NOT overwrite the still-valid stored + // token. Otherwise a single 401/403 strips auth from every later request, + // turning one permission error into a cascade of unauthenticated failures. + const store = new MemoryTokenStore('valid-token'); + const refresh = jest.fn(async () => null); + const auth = new BearerTokenAuth(store, refresh); + + const req = new Request('https://api.test/forbidden'); + const res = new Response('Forbidden', { status: 403 }); + const retryFn = jest.fn(async (r: Request) => new Response('', { status: 200 })); + + // onAuthError gives up (no new token) and does not retry. + const result = await auth.onAuthError(req, res, retryFn); + expect(result).toBeUndefined(); + expect(retryFn).not.toHaveBeenCalled(); + + // Crucially, the original token survives for the next request. + expect(await store.getToken()).toBe('valid-token'); + + // And onRequest still attaches it. + const decorated = await auth.onRequest(new Request('https://api.test/next')); + expect((decorated as Request).headers.get('Authorization')).toBe('Bearer valid-token'); + }); });