From e3446cd3c8a6f31ed2fe80aa5f5d9a14d6487719 Mon Sep 17 00:00:00 2001 From: Santiago Palenque Date: Fri, 16 Oct 2026 15:30:22 -0300 Subject: [PATCH 1/3] chore: remove setSessionClearingState from refreshToken and clear authInfo --- .../security/__tests__/methods.test.js | 160 +++++++++++++++++- src/components/security/methods.js | 27 ++- 2 files changed, 173 insertions(+), 14 deletions(-) diff --git a/src/components/security/__tests__/methods.test.js b/src/components/security/__tests__/methods.test.js index 0a00d16d..f0e32d8a 100644 --- a/src/components/security/__tests__/methods.test.js +++ b/src/components/security/__tests__/methods.test.js @@ -1,9 +1,21 @@ 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, getAccessToken, setAccessTokenResolver, setAuthHandlers, getAuthHandlers, initLogOut } from '../methods'; +import { + refreshAccessToken, + retryWithBackoff, + getAccessToken, + setAccessTokenResolver, + setAuthHandlers, + getAuthHandlers, + initLogOut, + storeAuthInfo, + getAuthInfo, +} from '../methods'; // Mock utils/methods imports used by security/methods jest.mock('../../../utils/methods', () => ({ @@ -53,7 +65,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 +139,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 +242,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 +256,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 () => { @@ -476,3 +514,111 @@ describe('setAuthHandlers / initLogOut', () => { expect(getAuthHandlers()).toEqual({ initLogOut: null, authErrorHandler: null }); }); }); + +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 ffdf0907..aa6bcdd5 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'; @@ -302,15 +301,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 @@ -521,7 +534,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}`); } @@ -535,7 +547,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} @@ -551,8 +562,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) { From 8afc5c2b3b894453a74d8e08cd495816f14e1734 Mon Sep 17 00:00:00 2001 From: Santiago Palenque Date: Fri, 16 Oct 2026 16:51:33 -0300 Subject: [PATCH 2/3] chore: fix tests --- src/components/security/__tests__/methods.test.js | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/components/security/__tests__/methods.test.js b/src/components/security/__tests__/methods.test.js index f0e32d8a..34898ba0 100644 --- a/src/components/security/__tests__/methods.test.js +++ b/src/components/security/__tests__/methods.test.js @@ -577,11 +577,16 @@ describe('getAccessToken with a rejected refresh token', () => { 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' }); + // skip the backoff waits, runAllTimersAsync needs jest >= 29.5 + jest.useRealTimers(); + const setTimeoutSpy = jest.spyOn(global, 'setTimeout').mockImplementation((cb) => { cb(); return 0; }); - const result = getAccessToken().catch((e) => e); - await jest.runAllTimersAsync(); + try { + await expect(getAccessToken()).rejects.toThrow(AUTH_ERROR_REFRESH_TOKEN_NETWORK_ERROR); + } finally { + setTimeoutSpy.mockRestore(); + } - 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'); }); From acedda2d34abcd2c0baf7bcefb51b26102d03204 Mon Sep 17 00:00:00 2001 From: Santiago Palenque Date: Wed, 28 Oct 2026 13:54:33 -0300 Subject: [PATCH 3/3] v4.2.40-beta.0 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index fce4ffdd..40415dd7 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "openstack-uicore-foundation", - "version": "4.2.39", + "version": "4.2.40-beta.0", "description": "ui reactjs components for openstack marketing site", "main": "lib/openstack-uicore-foundation.js", "scripts": {