From 9fd1e57602e71c521d9448bc404cfa96588e38b9 Mon Sep 17 00:00:00 2001 From: Santiago Palenque Date: Fri, 16 Oct 2026 16:46:30 -0300 Subject: [PATCH] chore: port v4 changes on failed refresh token fix --- .../security/__tests__/methods.test.js | 156 +++++++++++++++++- src/components/security/methods.js | 27 ++- 2 files changed, 169 insertions(+), 14 deletions(-) diff --git a/src/components/security/__tests__/methods.test.js b/src/components/security/__tests__/methods.test.js index f1e6089a..218ee7d9 100644 --- a/src/components/security/__tests__/methods.test.js +++ b/src/components/security/__tests__/methods.test.js @@ -1,9 +1,17 @@ import { AUTH_ERROR_REFRESH_TOKEN_REQUEST_ERROR, AUTH_ERROR_REFRESH_TOKEN_NETWORK_ERROR, + AUTH_ERROR_MISSING_AUTH_INFO, + AUTH_ERROR_MISSING_REFRESH_TOKEN, } from '../constants'; -import { refreshAccessToken, retryWithBackoff } from '../methods'; +import { + refreshAccessToken, + retryWithBackoff, + getAccessToken, + storeAuthInfo, + getAuthInfo, +} from '../methods'; // Mock utils/methods imports used by security/methods jest.mock('../../../utils/methods', () => ({ @@ -53,7 +61,22 @@ jest.mock('../actions', () => ({ SET_LOGGED_USER: 'SET_LOGGED_USER', })); -const { setSessionClearingState } = require('../../../utils/methods'); +const { + setSessionClearingState, + getFromLocalStorage, + putOnLocalStorage, + removeFromLocalStorage, + retryPromise, +} = require('../../../utils/methods'); + +// backs the mocked localStorage helpers with an in-memory store +const useMemoryStorage = () => { + const storage = {}; + getFromLocalStorage.mockImplementation((key) => storage[key] ?? null); + putOnLocalStorage.mockImplementation((key, value) => { storage[key] = value; }); + removeFromLocalStorage.mockImplementation((key) => { delete storage[key]; }); + return storage; +}; // Helper to set window globals needed by methods.js const setupWindowGlobals = () => { @@ -112,8 +135,6 @@ describe('refreshAccessToken', () => { await expect(refreshAccessToken('revoked-token')) .rejects .toThrow(AUTH_ERROR_REFRESH_TOKEN_REQUEST_ERROR); - - expect(setSessionClearingState).toHaveBeenCalledWith(true); }); it('should throw AUTH_ERROR_REFRESH_TOKEN_NETWORK_ERROR on HTTP 500', async () => { @@ -217,7 +238,7 @@ describe('refreshAccessToken', () => { expect(setSessionClearingState).not.toHaveBeenCalled(); }); - it('should call setSessionClearingState(true) only on HTTP 400', async () => { + it('should not call setSessionClearingState on HTTP 400', async () => { const mockResponse = { ok: false, status: 400, @@ -231,8 +252,21 @@ describe('refreshAccessToken', () => { // expected } - expect(setSessionClearingState).toHaveBeenCalledWith(true); - expect(setSessionClearingState).toHaveBeenCalledTimes(1); + expect(setSessionClearingState).not.toHaveBeenCalled(); + }); + + it('should not call setSessionClearingState when the response has no access_token', async () => { + global.fetch = jest.fn().mockResolvedValue({ + ok: true, + status: 200, + json: jest.fn().mockResolvedValue({ refresh_token: 'rt' }), + }); + + await expect(refreshAccessToken('some-token')) + .rejects + .toThrow(AUTH_ERROR_REFRESH_TOKEN_REQUEST_ERROR); + + expect(setSessionClearingState).not.toHaveBeenCalled(); }); it('should include status in error message for 5xx', async () => { @@ -370,3 +404,111 @@ describe('retryWithBackoff', () => { setTimeoutSpy.mockRestore(); }); }); + +describe('getAccessToken with a rejected refresh token', () => { + // moment().unix() is mocked to 1000, so a token updated at 0 with a 100s lifetime is expired + const EXPIRED_AUTH_INFO = { accessToken: 'old-at', expiresIn: 100, accessTokenUpdatedAt: 0 }; + + let storage; + + const storeExpired = (refreshToken) => { + storage.authInfo = JSON.stringify({ ...EXPIRED_AUTH_INFO, refreshToken }); + }; + + const rejectRefresh = () => { + global.fetch = jest.fn().mockResolvedValue({ ok: false, status: 400, statusText: 'Bad Request' }); + }; + + beforeEach(() => { + storage = useMemoryStorage(); + retryPromise.mockResolvedValue(true); + }); + + afterEach(() => { + [getFromLocalStorage, putOnLocalStorage, removeFromLocalStorage, retryPromise] + .forEach((mock) => mock.mockReset()); + }); + + it('sends the rejected token once, clears auth info and fails the next call without a request', async () => { + storeExpired('dead-rt'); + rejectRefresh(); + + await expect(getAccessToken()).rejects.toThrow(AUTH_ERROR_REFRESH_TOKEN_REQUEST_ERROR); + expect(global.fetch).toHaveBeenCalledTimes(1); + expect(getAuthInfo()).toBeNull(); + + await expect(getAccessToken()).rejects.toThrow(AUTH_ERROR_MISSING_AUTH_INFO); + expect(global.fetch).toHaveBeenCalledTimes(1); + }); + + it('clears auth info when the refresh response has no access_token', async () => { + storeExpired('dead-rt'); + global.fetch = jest.fn().mockResolvedValue({ + ok: true, + status: 200, + json: jest.fn().mockResolvedValue({}), + }); + + await expect(getAccessToken()).rejects.toThrow(AUTH_ERROR_REFRESH_TOKEN_REQUEST_ERROR); + expect(getAuthInfo()).toBeNull(); + }); + + it('keeps auth info when a newer refresh token was stored during the request', async () => { + storeExpired('dead-rt'); + global.fetch = jest.fn().mockImplementation(async () => { + storeExpired('new-rt'); + return { ok: false, status: 400, statusText: 'Bad Request' }; + }); + + await expect(getAccessToken()).rejects.toThrow(AUTH_ERROR_REFRESH_TOKEN_REQUEST_ERROR); + expect(getAuthInfo().refreshToken).toBe('new-rt'); + }); + + it('keeps auth info when the refresh keeps failing with a retryable error', async () => { + storeExpired('valid-rt'); + global.fetch = jest.fn().mockResolvedValue({ ok: false, status: 500, statusText: 'Server Error' }); + + const result = getAccessToken().catch((e) => e); + await jest.runAllTimersAsync(); + + expect((await result).message).toContain(AUTH_ERROR_REFRESH_TOKEN_NETWORK_ERROR); + expect(global.fetch.mock.calls.length).toBeGreaterThan(1); + expect(getAuthInfo().refreshToken).toBe('valid-rt'); + }); + + it.each(['null', 'undefined'])('never sends a "%s" refresh token to the IDP', async (refreshToken) => { + storeExpired(refreshToken); + global.fetch = jest.fn(); + + await expect(getAccessToken()).rejects.toThrow(AUTH_ERROR_MISSING_REFRESH_TOKEN); + expect(global.fetch).not.toHaveBeenCalled(); + expect(getAuthInfo()).toBeNull(); + }); +}); + +describe('storeAuthInfo', () => { + let storage; + + beforeEach(() => { + storage = useMemoryStorage(); + }); + + afterEach(() => { + [getFromLocalStorage, putOnLocalStorage, removeFromLocalStorage].forEach((mock) => mock.mockReset()); + }); + + it.each(['null', 'undefined'])('keeps the previous refresh token when given "%s"', (refreshToken) => { + storeAuthInfo('at-1', 3600, 'good-rt'); + storeAuthInfo('at-2', 3600, refreshToken); + + expect(getAuthInfo().refreshToken).toBe('good-rt'); + }); + + it('does not store a "null" refresh token left by an older version', () => { + storage.authInfo = JSON.stringify({ accessToken: 'at-1', refreshToken: 'null' }); + + storeAuthInfo('at-2', 3600, null); + + expect(getAuthInfo()).not.toHaveProperty('refreshToken'); + }); +}); diff --git a/src/components/security/methods.js b/src/components/security/methods.js index 95339055..44556aaa 100644 --- a/src/components/security/methods.js +++ b/src/components/security/methods.js @@ -7,7 +7,6 @@ import { getOrigin, putOnLocalStorage, retryPromise, - setSessionClearingState, } from "../../utils/methods"; import moment from "moment-timezone"; import request from 'superagent/lib/client'; @@ -294,15 +293,29 @@ export const retryWithBackoff = async (fn, maxRetries = MAX_RETRIES, baseDelayMs } }; +// a "null"/"undefined" string is a serialized missing token, the IDP would reject it anyway +const isMissingRefreshToken = (refreshToken) => + !refreshToken || refreshToken === 'null' || refreshToken === 'undefined'; + const processRefreshToken = async (flow, refreshToken) => { if (flow === RESPONSE_TYPE_CODE && useOAuth2RefreshToken()) { - if (!refreshToken) { + if (isMissingRefreshToken(refreshToken)) { clearAuthInfo(); throw Error(AUTH_ERROR_MISSING_REFRESH_TOKEN); } - let response = await retryWithBackoff(() => refreshAccessToken(refreshToken)); + let response; + try { + response = await retryWithBackoff(() => refreshAccessToken(refreshToken)); + } catch (err) { + // drop a rejected refresh token so it is never sent again, unless a newer one was stored meanwhile + if (err.message && err.message.startsWith(AUTH_ERROR_REFRESH_TOKEN_REQUEST_ERROR) + && getAuthInfo()?.refreshToken === refreshToken) { + clearAuthInfo(); + } + throw err; + } let {access_token, expires_in, refresh_token, id_token} = response; if (typeof refresh_token === 'undefined') { refresh_token = null; // not using rotate policy @@ -455,7 +468,6 @@ export const refreshAccessToken = async (refresh_token) => { throw Error(`${AUTH_ERROR_REFRESH_TOKEN_NETWORK_ERROR}: ${response.status} - ${response.statusText}`); } // token is genuinely revoked — this is a real auth error - setSessionClearingState(true); throw Error(`${AUTH_ERROR_REFRESH_TOKEN_REQUEST_ERROR}: ${response.status} - ${response.statusText}`); } @@ -469,7 +481,6 @@ export const refreshAccessToken = async (refresh_token) => { let {access_token, refresh_token: new_refresh_token, expires_in, id_token} = json; // Defensively ensure we never propagate an undefined access token. if (!access_token) { - setSessionClearingState(true); throw Error(`${AUTH_ERROR_REFRESH_TOKEN_REQUEST_ERROR}: missing access_token in refresh response`); } return {access_token, refresh_token: new_refresh_token, expires_in, id_token} @@ -485,8 +496,10 @@ export const storeAuthInfo = (accessToken, expiresIn, refreshToken = null, idTok accessTokenUpdatedAt: Math.floor(Date.now() / 1000), }; - if (refreshToken == null && formerAuthInfo) { - refreshToken = formerAuthInfo.refreshToken; + if (isMissingRefreshToken(refreshToken)) { + refreshToken = formerAuthInfo && !isMissingRefreshToken(formerAuthInfo.refreshToken) + ? formerAuthInfo.refreshToken + : null; } if (idToken == null && formerAuthInfo) {