From 9eb247e57285fa498418382a56b01cdba1991dd1 Mon Sep 17 00:00:00 2001 From: Noah Overcash Date: Wed, 18 Sep 2024 16:55:54 -0400 Subject: [PATCH 01/12] [STCOR-888] RTR dynamic configuration, debugging event --- src/components/Root/FFetch.js | 14 +++++++++----- src/components/Root/Root.js | 6 ++++-- src/components/Root/constants.js | 8 +++++++- src/components/Root/token-util.js | 6 ++++++ .../SessionEventContainer/SessionEventContainer.js | 1 + 5 files changed, 27 insertions(+), 8 deletions(-) diff --git a/src/components/Root/FFetch.js b/src/components/Root/FFetch.js index ef84814b8..b3de885e4 100644 --- a/src/components/Root/FFetch.js +++ b/src/components/Root/FFetch.js @@ -42,7 +42,7 @@ */ import ms from 'ms'; -import { okapi } from 'stripes-config'; +import { okapi, config } from 'stripes-config'; import { setRtrTimeout } from '../../okapiActions'; @@ -60,8 +60,8 @@ import { } from './Errors'; import { RTR_AT_EXPIRY_IF_UNKNOWN, - RTR_AT_TTL_FRACTION, RTR_ERROR_EVENT, + RTR_FORCE_REFRESH_EVENT, } from './constants'; import FXHR from './FXHR'; @@ -74,6 +74,11 @@ export class FFetch { constructor({ logger, store }) { this.logger = logger; this.store = store; + + window.addEventListener(RTR_FORCE_REFRESH_EVENT, () => { + this.logger.log('rtr', 'forcing rotation due to RTR_FORCE_REFRESH_EVENT'); + rtr(this.nativeFetch, console, this.rotateCallback); + }); } /** @@ -110,7 +115,6 @@ export class FFetch { const scheduleRotation = (rotationP) => { rotationP.then((rotationInterval) => { - this.logger.log('rtr', `rotation fired from rotateCallback; next callback in ${ms(rotationInterval)}`); this.store.dispatch(setRtrTimeout(setTimeout(() => { rtr(this.nativeFetch, this.logger, this.rotateCallback); }, rotationInterval))); @@ -127,14 +131,14 @@ export class FFetch { // the response, otherwise the session. Default to 10 seconds. if (res?.accessTokenExpiration) { this.logger.log('rtr', 'rotation scheduled with login response data'); - const rotationPromise = Promise.resolve((new Date(res.accessTokenExpiration).getTime() - Date.now()) * RTR_AT_TTL_FRACTION); + const rotationPromise = Promise.resolve((new Date(res.accessTokenExpiration).getTime() - Date.now()) * config.rtr.rotationIntervalFraction); scheduleRotation(rotationPromise); } else { const rotationPromise = getTokenExpiry().then((expiry) => { if (expiry.atExpires) { this.logger.log('rtr', 'rotation scheduled with cached session data'); - return (new Date(expiry.atExpires).getTime() - Date.now()) * RTR_AT_TTL_FRACTION; + return (new Date(expiry.atExpires).getTime() - Date.now()) * config.rtr.rotationIntervalFraction; } // default: 10 seconds diff --git a/src/components/Root/Root.js b/src/components/Root/Root.js index 12bc67360..1dec9895b 100644 --- a/src/components/Root/Root.js +++ b/src/components/Root/Root.js @@ -67,6 +67,10 @@ class Root extends Component { // * configure fetch and xhr interceptors to conduct RTR // * see SessionEventContainer for RTR handling if (this.props.config.useSecureTokens) { + // FFetch relies on some of these properties, so we must ensure + // they are filled before initialization + this.props.config.rtr = configureRtr(this.props.config.rtr); + this.ffetch = new FFetch({ logger: this.props.logger, store, @@ -125,8 +129,6 @@ class Root extends Component { } // make sure RTR is configured - config.rtr = configureRtr(this.props.config.rtr); - const stripes = new Stripes({ logger, store, diff --git a/src/components/Root/constants.js b/src/components/Root/constants.js index 771467234..a0f7fa92b 100644 --- a/src/components/Root/constants.js +++ b/src/components/Root/constants.js @@ -4,6 +4,9 @@ export const RTR_SUCCESS_EVENT = '@folio/stripes/core::RTRSuccess'; /** dispatched during RTR if RTR itself fails */ export const RTR_ERROR_EVENT = '@folio/stripes/core::RTRError'; +/** dispatched by ui-developer to force a token rotation */ +export const RTR_FORCE_REFRESH_EVENT = '@folio/stripes/core::RTRForceRefresh'; + /** * dispatched if the session is idle (without activity) for too long */ @@ -12,7 +15,10 @@ export const RTR_TIMEOUT_EVENT = '@folio/stripes/core::RTRIdleSessionTimeout'; /** BroadcastChannel for cross-window activity pings */ export const RTR_ACTIVITY_CHANNEL = '@folio/stripes/core::RTRActivityChannel'; -/** how much of an AT's lifespan can elapse before it is considered expired */ +/** + * how much of an AT's lifespan can elapse before it is considered expired. + * overridden in stripes.config.js::config.rtr.rotationIntervalFraction. + */ export const RTR_AT_TTL_FRACTION = 0.8; /** diff --git a/src/components/Root/token-util.js b/src/components/Root/token-util.js index b622101a6..77f1d6639 100644 --- a/src/components/Root/token-util.js +++ b/src/components/Root/token-util.js @@ -5,6 +5,7 @@ import { getTokenExpiry, setTokenExpiry } from '../../loginServices'; import { RTRError, UnexpectedResourceError } from './Errors'; import { RTR_ACTIVITY_EVENTS, + RTR_AT_TTL_FRACTION, RTR_ERROR_EVENT, RTR_IDLE_MODAL_TTL, RTR_IDLE_SESSION_TTL, @@ -319,6 +320,11 @@ export const configureRtr = (config = {}) => { conf.idleModalTTL = RTR_IDLE_MODAL_TTL; } + // what fraction of the way through the session should we rotate? + if (!conf.rotationIntervalFraction) { + conf.rotationIntervalFraction = RTR_AT_TTL_FRACTION; + } + // what events constitute activity? if (isEmpty(conf.activityEvents)) { conf.activityEvents = RTR_ACTIVITY_EVENTS; diff --git a/src/components/SessionEventContainer/SessionEventContainer.js b/src/components/SessionEventContainer/SessionEventContainer.js index b82f34004..9071b217e 100644 --- a/src/components/SessionEventContainer/SessionEventContainer.js +++ b/src/components/SessionEventContainer/SessionEventContainer.js @@ -250,6 +250,7 @@ const SessionEventContainer = ({ history, queryClient }) => { // We only want to configure the event listeners once, not every time // there is a change to stripes or history. Hence, an empty dependency // array. + // should `stripes.rtr` go here? }, []); // eslint-disable-line react-hooks/exhaustive-deps // show the idle-session warning modal if necessary; From 8313e4808c138cce45956c40c65f96afcf15a739 Mon Sep 17 00:00:00 2001 From: Noah Overcash Date: Wed, 18 Sep 2024 17:26:17 -0400 Subject: [PATCH 02/12] tests are nice --- src/components/Root/FFetch.js | 19 ++++++++++++++--- src/components/Root/FFetch.test.js | 34 +++++++++++++++++++++++++++--- src/components/Root/Root.js | 1 + 3 files changed, 48 insertions(+), 6 deletions(-) diff --git a/src/components/Root/FFetch.js b/src/components/Root/FFetch.js index b3de885e4..365703996 100644 --- a/src/components/Root/FFetch.js +++ b/src/components/Root/FFetch.js @@ -74,11 +74,24 @@ export class FFetch { constructor({ logger, store }) { this.logger = logger; this.store = store; + } - window.addEventListener(RTR_FORCE_REFRESH_EVENT, () => { + /** + * registers a listener for the RTR_FORCE_REFRESH_EVENT + */ + registerEventListener = () => { + this.globalEventCallback = () => { this.logger.log('rtr', 'forcing rotation due to RTR_FORCE_REFRESH_EVENT'); - rtr(this.nativeFetch, console, this.rotateCallback); - }); + rtr(this.nativeFetch, this.logger, this.rotateCallback); + }; + window.addEventListener(RTR_FORCE_REFRESH_EVENT, this.globalEventCallback); + } + + /** + * unregister the listener for the RTR_FORCE_REFRESH_EVENT + */ + unregisterEventListener = () => { + window.removeEventListener(RTR_FORCE_REFRESH_EVENT, this.globalEventCallback); } /** diff --git a/src/components/Root/FFetch.test.js b/src/components/Root/FFetch.test.js index 8ada9a7c4..e93ba23f2 100644 --- a/src/components/Root/FFetch.test.js +++ b/src/components/Root/FFetch.test.js @@ -3,13 +3,14 @@ /* eslint-disable no-unused-vars */ import ms from 'ms'; +import { waitFor } from '@testing-library/react'; import { getTokenExpiry } from '../../loginServices'; import { FFetch } from './FFetch'; import { RTRError, UnexpectedResourceError } from './Errors'; import { RTR_AT_EXPIRY_IF_UNKNOWN, - RTR_AT_TTL_FRACTION, + RTR_FORCE_REFRESH_EVENT, } from './constants'; jest.mock('../../loginServices', () => ({ @@ -24,6 +25,11 @@ jest.mock('stripes-config', () => ({ okapi: { url: 'okapiUrl', tenant: 'okapiTenant' + }, + config: { + rtr: { + rotationIntervalFraction: 0.5, + } } }), { virtual: true }); @@ -32,6 +38,9 @@ const log = jest.fn(); const mockFetch = jest.fn(); +// to ensure we cleanup after each test +const instancesWithEventListeners = []; + describe('FFetch class', () => { beforeEach(() => { global.fetch = mockFetch; @@ -39,6 +48,8 @@ describe('FFetch class', () => { atExpires: Date.now() + (10 * 60 * 1000), rtExpires: Date.now() + (10 * 60 * 1000), }); + instancesWithEventListeners.forEach(instance => instance.unregisterEventListener()); + instancesWithEventListeners.length = 0; }); afterEach(() => { @@ -151,6 +162,23 @@ describe('FFetch class', () => { }); }); + describe('force refresh event', () => { + it('Invokes a refresh on RTR_FORCE_REFRESH_EVENT...', async () => { + mockFetch.mockResolvedValueOnce('okapi success'); + + const instance = new FFetch({ logger: { log } }); + instance.replaceFetch(); + instance.replaceXMLHttpRequest(); + + instance.registerEventListener(); + instancesWithEventListeners.push(instance); + + window.dispatchEvent(new Event(RTR_FORCE_REFRESH_EVENT)); + + await waitFor(() => expect(mockFetch.mock.calls).toHaveLength(1)); + }); + }); + describe('calling authentication resources', () => { it('handles RTR data in the response', async () => { // a static timestamp representing "now" @@ -193,7 +221,7 @@ describe('FFetch class', () => { // gross, but on the other, since we're deliberately pushing rotation // into a separate thread, I'm note sure of a better way to handle this. await setTimeout(Promise.resolve(), 2000); - expect(st).toHaveBeenCalledWith(expect.any(Function), (accessTokenExpiration - whatTimeIsItMrFox) * RTR_AT_TTL_FRACTION); + expect(st).toHaveBeenCalledWith(expect.any(Function), (accessTokenExpiration - whatTimeIsItMrFox) * 0.5); }); it('handles RTR data in the session', async () => { @@ -237,7 +265,7 @@ describe('FFetch class', () => { // gross, but on the other, since we're deliberately pushing rotation // into a separate thread, I'm note sure of a better way to handle this. await setTimeout(Promise.resolve(), 2000); - expect(st).toHaveBeenCalledWith(expect.any(Function), (atExpires - whatTimeIsItMrFox) * RTR_AT_TTL_FRACTION); + expect(st).toHaveBeenCalledWith(expect.any(Function), (atExpires - whatTimeIsItMrFox) * 0.5); }); it('handles missing RTR data', async () => { diff --git a/src/components/Root/Root.js b/src/components/Root/Root.js index 1dec9895b..4bb418947 100644 --- a/src/components/Root/Root.js +++ b/src/components/Root/Root.js @@ -75,6 +75,7 @@ class Root extends Component { logger: this.props.logger, store, }); + this.ffetch.registerEventListener(); this.ffetch.replaceFetch(); this.ffetch.replaceXMLHttpRequest(); } From 7825038465acdf4a65f8422dd891f0a1ba76fb14 Mon Sep 17 00:00:00 2001 From: Noah Overcash Date: Wed, 18 Sep 2024 17:41:30 -0400 Subject: [PATCH 03/12] token-util tests --- src/components/Root/token-util.test.js | 30 ++++++++++++++------------ 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/src/components/Root/token-util.test.js b/src/components/Root/token-util.test.js index 4463ec97a..169b2a194 100644 --- a/src/components/Root/token-util.test.js +++ b/src/components/Root/token-util.test.js @@ -337,19 +337,21 @@ describe('getPromise', () => { }); describe('configureRtr', () => { - it('sets idleSessionTTL and idleModalTTL', () => { - const res = configureRtr({}); - expect(res.idleSessionTTL).toBe('60m'); - expect(res.idleModalTTL).toBe('1m'); - }); - - it('leaves existing settings in place', () => { - const res = configureRtr({ - idleSessionTTL: '5m', - idleModalTTL: '5m', - }); - - expect(res.idleSessionTTL).toBe('5m'); - expect(res.idleModalTTL).toBe('5m'); + it.each([ + [ + {}, + { idleSessionTTL: '60m', idleModalTTL: '1m', rotationIntervalFraction: 0.8, activityEvents: ['keydown', 'mousedown'] } + ], + [ + { idleSessionTTL: '1s', idleModalTTL: '2m' }, + { idleSessionTTL: '1s', idleModalTTL: '2m', rotationIntervalFraction: 0.8, activityEvents: ['keydown', 'mousedown'] } + ], + [ + { idleSessionTTL: '1s', idleModalTTL: '2m', rotationIntervalFraction: -1, activityEvents: ['cha-cha-slide'] }, + { idleSessionTTL: '1s', idleModalTTL: '2m', rotationIntervalFraction: -1, activityEvents: ['cha-cha-slide'] } + ], + ])('sets default values as applicable', (config, expected) => { + const res = configureRtr(config); + expect(res).toMatchObject(expected); }); }); From b1dacce46313491efeea488c7399913ed2ab80fa Mon Sep 17 00:00:00 2001 From: Noah Overcash Date: Fri, 20 Sep 2024 09:23:10 -0400 Subject: [PATCH 04/12] Re-export constants --- index.js | 1 + src/loginServices.js | 4 +--- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/index.js b/index.js index 67fd4d1d0..bb3a4f68f 100644 --- a/index.js +++ b/index.js @@ -49,6 +49,7 @@ export { userLocaleConfig } from './src/loginServices'; export * from './src/consortiaServices'; export { default as queryLimit } from './src/queryLimit'; export { default as init } from './src/init'; +export * as RTR_CONSTANTS from './src/components/Root/constants'; /* localforage wrappers hide the session key */ export { getOkapiSession, getTokenExpiry, setTokenExpiry } from './src/loginServices'; diff --git a/src/loginServices.js b/src/loginServices.js index a40bf5ac7..6d246c09b 100644 --- a/src/loginServices.js +++ b/src/loginServices.js @@ -28,9 +28,7 @@ import { } from './okapiActions'; import processBadResponse from './processBadResponse'; -import { - RTR_TIMEOUT_EVENT -} from './components/Root/constants'; +import { RTR_TIMEOUT_EVENT } from './components/Root/constants'; // export supported locales, i.e. the languages we provide translations for export const supportedLocales = [ From cbd03ca6151f56abb2e6e64303f58363694269c6 Mon Sep 17 00:00:00 2001 From: Noah Overcash Date: Tue, 24 Sep 2024 18:29:40 -0400 Subject: [PATCH 05/12] cleanup diffs --- src/components/Root/FFetch.js | 4 ++-- src/components/Root/constants.js | 2 +- src/loginServices.js | 4 +++- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/components/Root/FFetch.js b/src/components/Root/FFetch.js index db68adefd..d4eb92b9e 100644 --- a/src/components/Root/FFetch.js +++ b/src/components/Root/FFetch.js @@ -133,7 +133,7 @@ export class FFetch { // RT timeout interval (session will end) and warning interval (warning that session will end) const rtTimeoutInterval = (rotationInterval.refreshTokenExpiration - Date.now()); - const rtWarningInterval = (rotationInterval.refreshTokenExpiration - Date.now()) - ms(this.rtrConfig.fixedLengthSessionWarningTTL); + const rtWarningInterval = (rotationInterval.refreshTokenExpiration - Date.now()) - ms(config.rtr.fixedLengthSessionWarningTTL); // schedule AT rotation IFF the AT will expire before the RT. this avoids // refresh-thrashing near the end of the FLS with progressively shorter @@ -149,7 +149,7 @@ export class FFetch { } // schedule FLS end-of-session warning - this.logger.log('rtr-fls', `end-of-session warning at ${new Date(rotationInterval.refreshTokenExpiration - ms(this.rtrConfig.fixedLengthSessionWarningTTL))}`); + this.logger.log('rtr-fls', `end-of-session warning at ${new Date(rotationInterval.refreshTokenExpiration - ms(config.rtr.fixedLengthSessionWarningTTL))}`); this.store.dispatch(setRtrFlsWarningTimeout(setTimeout(() => { this.logger.log('rtr-fls', 'emitting RTR_FLS_WARNING_EVENT'); window.dispatchEvent(new Event(RTR_FLS_WARNING_EVENT)); diff --git a/src/components/Root/constants.js b/src/components/Root/constants.js index 58b530c3d..ff4a2b4a1 100644 --- a/src/components/Root/constants.js +++ b/src/components/Root/constants.js @@ -33,7 +33,7 @@ export const RTR_FLS_WARNING_TTL = '1m'; export const RTR_ACTIVITY_CHANNEL = '@folio/stripes/core::RTRActivityChannel'; /** - * how much of a token's lifespan can elapse before it is considered expired. + * how much of a token's lifespan can elapse before it is considered expired? * For the AT, we want a very safe margin because we don't ever want to fall * off the end of the AT since it would be a very misleading failure given * the RT is still good at that point. Since rotation happens in the background diff --git a/src/loginServices.js b/src/loginServices.js index ca66a607d..060f52dd4 100644 --- a/src/loginServices.js +++ b/src/loginServices.js @@ -28,7 +28,9 @@ import { } from './okapiActions'; import processBadResponse from './processBadResponse'; -import { RTR_TIMEOUT_EVENT } from './components/Root/constants'; +import { + RTR_TIMEOUT_EVENT +} from './components/Root/constants'; // export supported locales, i.e. the languages we provide translations for export const supportedLocales = [ From 6e85a9188db8c8b40003c751fd6006619d950faa Mon Sep 17 00:00:00 2001 From: Noah Overcash Date: Tue, 24 Sep 2024 20:10:22 -0400 Subject: [PATCH 06/12] Pass config in new way in test --- src/components/Root/FFetch.test.js | 24 ++++++++---------------- 1 file changed, 8 insertions(+), 16 deletions(-) diff --git a/src/components/Root/FFetch.test.js b/src/components/Root/FFetch.test.js index 494416586..b158bf1f6 100644 --- a/src/components/Root/FFetch.test.js +++ b/src/components/Root/FFetch.test.js @@ -11,7 +11,6 @@ import { RTRError, UnexpectedResourceError } from './Errors'; import { RTR_AT_EXPIRY_IF_UNKNOWN, RTR_FORCE_REFRESH_EVENT, - RTR_AT_TTL_FRACTION, RTR_FLS_WARNING_TTL, RTR_TIME_MARGIN_IN_MS, } from './constants'; @@ -32,6 +31,7 @@ jest.mock('stripes-config', () => ({ config: { rtr: { rotationIntervalFraction: 0.5, + fixedLengthSessionWarningTTL: '1m', } } }), @@ -165,7 +165,7 @@ describe('FFetch class', () => { }); }); - describe('force refresh event', () => { + describe.skip('force refresh event', () => { it('Invokes a refresh on RTR_FORCE_REFRESH_EVENT...', async () => { mockFetch.mockResolvedValueOnce('okapi success'); @@ -214,9 +214,7 @@ describe('FFetch class', () => { store: { dispatch: jest.fn(), }, - rtrConfig: { - fixedLengthSessionWarningTTL: '1m', - }, + }); testFfetch.replaceFetch(); testFfetch.replaceXMLHttpRequest(); @@ -270,9 +268,7 @@ describe('FFetch class', () => { store: { dispatch: jest.fn(), }, - rtrConfig: { - fixedLengthSessionWarningTTL: '1m', - }, + }); testFfetch.replaceFetch(); testFfetch.replaceXMLHttpRequest(); @@ -308,9 +304,7 @@ describe('FFetch class', () => { store: { dispatch: jest.fn(), }, - rtrConfig: { - fixedLengthSessionWarningTTL: '1m', - }, + }); testFfetch.replaceFetch(); testFfetch.replaceXMLHttpRequest(); @@ -323,7 +317,7 @@ describe('FFetch class', () => { // into a separate thread, I'm not sure of a better way to handle this. await setTimeout(Promise.resolve(), 2000); - expect(st).toHaveBeenCalledWith(expect.any(Function), ms(RTR_AT_EXPIRY_IF_UNKNOWN) * RTR_AT_TTL_FRACTION); + expect(st).toHaveBeenCalledWith(expect.any(Function), ms(RTR_AT_EXPIRY_IF_UNKNOWN) * 0.5); }); it('handles unsuccessful responses', async () => { @@ -387,9 +381,7 @@ describe('FFetch class', () => { store: { dispatch: jest.fn(), }, - rtrConfig: { - fixedLengthSessionWarningTTL: '1m', - }, + }); testFfetch.replaceFetch(); testFfetch.replaceXMLHttpRequest(); @@ -403,7 +395,7 @@ describe('FFetch class', () => { await setTimeout(Promise.resolve(), 2000); // AT rotation - expect(st).not.toHaveBeenCalledWith(expect.any(Function), (accessTokenExpiration - whatTimeIsItMrFox) * RTR_AT_TTL_FRACTION); + expect(st).not.toHaveBeenCalledWith(expect.any(Function), (accessTokenExpiration - whatTimeIsItMrFox) * 0.5); // FLS warning expect(st).toHaveBeenCalledWith(expect.any(Function), (refreshTokenExpiration - whatTimeIsItMrFox) - ms(RTR_FLS_WARNING_TTL)); From 8fb564226145c774642755f2e34fa7574cc6ae96 Mon Sep 17 00:00:00 2001 From: Noah Overcash Date: Tue, 24 Sep 2024 20:23:40 -0400 Subject: [PATCH 07/12] Fix event test --- src/components/Root/FFetch.js | 2 +- src/components/Root/FFetch.test.js | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/components/Root/FFetch.js b/src/components/Root/FFetch.js index d4eb92b9e..6a621b087 100644 --- a/src/components/Root/FFetch.js +++ b/src/components/Root/FFetch.js @@ -88,7 +88,7 @@ export class FFetch { registerEventListener = () => { this.globalEventCallback = () => { this.logger.log('rtr', 'forcing rotation due to RTR_FORCE_REFRESH_EVENT'); - rtr(this.nativeFetch, this.logger, this.rotateCallback); + rtr(this.nativeFetch, this.logger, this.rotateCallback, this.store.getState().okapi); }; window.addEventListener(RTR_FORCE_REFRESH_EVENT, this.globalEventCallback); } diff --git a/src/components/Root/FFetch.test.js b/src/components/Root/FFetch.test.js index b158bf1f6..1837e113c 100644 --- a/src/components/Root/FFetch.test.js +++ b/src/components/Root/FFetch.test.js @@ -4,6 +4,7 @@ import ms from 'ms'; import { waitFor } from '@testing-library/react'; +import { okapi } from 'stripes-config'; import { getTokenExpiry } from '../../loginServices'; import { FFetch } from './FFetch'; @@ -165,11 +166,11 @@ describe('FFetch class', () => { }); }); - describe.skip('force refresh event', () => { + describe('force refresh event', () => { it('Invokes a refresh on RTR_FORCE_REFRESH_EVENT...', async () => { mockFetch.mockResolvedValueOnce('okapi success'); - const instance = new FFetch({ logger: { log } }); + const instance = new FFetch({ logger: { log }, store: { getState: () => ({ okapi }) } }); instance.replaceFetch(); instance.replaceXMLHttpRequest(); From 7c63eef408e43917f86c0e7280e5c77352b22e90 Mon Sep 17 00:00:00 2001 From: Noah Overcash Date: Thu, 10 Oct 2024 13:44:13 -0400 Subject: [PATCH 08/12] Use cleaner constant sharing for FFetch test --- src/components/Root/FFetch.test.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/components/Root/FFetch.test.js b/src/components/Root/FFetch.test.js index 1837e113c..2ea85dc65 100644 --- a/src/components/Root/FFetch.test.js +++ b/src/components/Root/FFetch.test.js @@ -4,7 +4,7 @@ import ms from 'ms'; import { waitFor } from '@testing-library/react'; -import { okapi } from 'stripes-config'; +import { okapi, config } from 'stripes-config'; import { getTokenExpiry } from '../../loginServices'; import { FFetch } from './FFetch'; @@ -229,7 +229,7 @@ describe('FFetch class', () => { await setTimeout(Promise.resolve(), 2000); // AT rotation - expect(st).toHaveBeenCalledWith(expect.any(Function), (accessTokenExpiration - whatTimeIsItMrFox) * 0.5); + expect(st).toHaveBeenCalledWith(expect.any(Function), (accessTokenExpiration - whatTimeIsItMrFox) * config.rtr.rotationIntervalFraction); // FLS warning expect(st).toHaveBeenCalledWith(expect.any(Function), (refreshTokenExpiration - whatTimeIsItMrFox) - ms(RTR_FLS_WARNING_TTL)); @@ -281,7 +281,7 @@ describe('FFetch class', () => { // gross, but on the other, since we're deliberately pushing rotation // into a separate thread, I'm note sure of a better way to handle this. await setTimeout(Promise.resolve(), 2000); - expect(st).toHaveBeenCalledWith(expect.any(Function), (atExpires - whatTimeIsItMrFox) * 0.5); + expect(st).toHaveBeenCalledWith(expect.any(Function), (atExpires - whatTimeIsItMrFox) * config.rtr.rotationIntervalFraction); }); it('handles missing RTR data', async () => { @@ -318,7 +318,7 @@ describe('FFetch class', () => { // into a separate thread, I'm not sure of a better way to handle this. await setTimeout(Promise.resolve(), 2000); - expect(st).toHaveBeenCalledWith(expect.any(Function), ms(RTR_AT_EXPIRY_IF_UNKNOWN) * 0.5); + expect(st).toHaveBeenCalledWith(expect.any(Function), ms(RTR_AT_EXPIRY_IF_UNKNOWN) * config.rtr.rotationIntervalFraction); }); it('handles unsuccessful responses', async () => { @@ -396,7 +396,7 @@ describe('FFetch class', () => { await setTimeout(Promise.resolve(), 2000); // AT rotation - expect(st).not.toHaveBeenCalledWith(expect.any(Function), (accessTokenExpiration - whatTimeIsItMrFox) * 0.5); + expect(st).not.toHaveBeenCalledWith(expect.any(Function), (accessTokenExpiration - whatTimeIsItMrFox) * config.rtr.rotationIntervalFraction); // FLS warning expect(st).toHaveBeenCalledWith(expect.any(Function), (refreshTokenExpiration - whatTimeIsItMrFox) - ms(RTR_FLS_WARNING_TTL)); From f013931109a529c212d57caa666002ea250dc24e Mon Sep 17 00:00:00 2001 From: Noah Overcash Date: Thu, 10 Oct 2024 15:06:56 -0400 Subject: [PATCH 09/12] Listen to changes in SessionEventContainer --- src/components/Root/Root.js | 21 ++++++++----------- .../SessionEventContainer.js | 9 ++++---- 2 files changed, 14 insertions(+), 16 deletions(-) diff --git a/src/components/Root/Root.js b/src/components/Root/Root.js index b8f317c42..7fe8b9cbc 100644 --- a/src/components/Root/Root.js +++ b/src/components/Root/Root.js @@ -63,14 +63,21 @@ class Root extends Component { this.apolloClient = createApolloClient(okapi); this.reactQueryClient = createReactQueryClient(); + // configure RTR with default props if needed + // gross: this overwrites whatever is currently stored at config.rtr + // gross: technically, the props could change and not get re-run through + // here. Realistically, that'll never happen since config values are read + // only once from a static file at build time, but still, props are props + // so technically it's possible. + // Also, ui-developer provides facilities to change some of this + this.props.config.rtr = configureRtr(this.props.config.rtr); + // enhanced security mode: // * configure fetch and xhr interceptors to conduct RTR // * see SessionEventContainer for RTR handling if (this.props.config.useSecureTokens) { // FFetch relies on some of these properties, so we must ensure // they are filled before initialization - this.props.config.rtr = configureRtr(this.props.config.rtr); - this.ffetch = new FFetch({ logger: this.props.logger, store, @@ -130,16 +137,6 @@ class Root extends Component { return (); } - // make sure RTR is configured - // gross: this overwrites whatever is currently stored at config.rtr - // gross: technically, this may be different than what is configured - // in the constructor since the constructor only runs once but - // render runs when props change. realistically, that'll never happen - // since config values are read only once from a static file at build - // time, but still, props are props so technically it's possible. - // Also, ui-developer provides facilities to change some of this - config.rtr = configureRtr(this.props.config.rtr); - const stripes = new Stripes({ logger, store, diff --git a/src/components/SessionEventContainer/SessionEventContainer.js b/src/components/SessionEventContainer/SessionEventContainer.js index 3e05f28fe..92f9786ba 100644 --- a/src/components/SessionEventContainer/SessionEventContainer.js +++ b/src/components/SessionEventContainer/SessionEventContainer.js @@ -264,10 +264,11 @@ const SessionEventContainer = ({ history }) => { // no deps? It should be history and stripes!!! >:) // We only want to configure the event listeners once, not every time - // there is a change to stripes or history. Hence, an empty dependency - // array. - // should `stripes.rtr` go here? - }, []); // eslint-disable-line react-hooks/exhaustive-deps + // there is a change to stripes or history. Hence, those are left out. + // we do include stripes.config.rtr, though, because these are configurable + // at runtime via ui-developer. + + }, [stripes.config.rtr]); // eslint-disable-line react-hooks/exhaustive-deps const renderList = []; From 50c1291646b2f84e7980d74361142feed1e5e529 Mon Sep 17 00:00:00 2001 From: Noah Overcash Date: Thu, 10 Oct 2024 15:07:37 -0400 Subject: [PATCH 10/12] fix typo --- src/components/SessionEventContainer/SessionEventContainer.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/components/SessionEventContainer/SessionEventContainer.js b/src/components/SessionEventContainer/SessionEventContainer.js index 92f9786ba..85790c5bc 100644 --- a/src/components/SessionEventContainer/SessionEventContainer.js +++ b/src/components/SessionEventContainer/SessionEventContainer.js @@ -267,7 +267,6 @@ const SessionEventContainer = ({ history }) => { // there is a change to stripes or history. Hence, those are left out. // we do include stripes.config.rtr, though, because these are configurable // at runtime via ui-developer. - }, [stripes.config.rtr]); // eslint-disable-line react-hooks/exhaustive-deps const renderList = []; From 46b503f66ce6a8e08df4491d14b1783efc66f215 Mon Sep 17 00:00:00 2001 From: Noah Overcash Date: Thu, 10 Oct 2024 16:37:26 -0400 Subject: [PATCH 11/12] bye bye --- src/components/Root/FFetch.js | 11 ++++--- src/components/Root/FFetch.test.js | 33 ++++++++++--------- src/components/Root/Root.js | 24 +++++++------- .../SessionEventContainer.js | 7 ++-- 4 files changed, 39 insertions(+), 36 deletions(-) diff --git a/src/components/Root/FFetch.js b/src/components/Root/FFetch.js index 6a621b087..fd54672c2 100644 --- a/src/components/Root/FFetch.js +++ b/src/components/Root/FFetch.js @@ -42,7 +42,7 @@ */ import ms from 'ms'; -import { okapi as okapiConfig, config } from 'stripes-config'; +import { okapi as okapiConfig } from 'stripes-config'; import { setRtrTimeout, setRtrFlsTimeout, @@ -77,9 +77,10 @@ const OKAPI_FETCH_OPTIONS = { }; export class FFetch { - constructor({ logger, store }) { + constructor({ logger, store, rtrConfig }) { this.logger = logger; this.store = store; + this.rtrConfig = rtrConfig; } /** @@ -129,11 +130,11 @@ export class FFetch { scheduleRotation = (rotationP) => { rotationP.then((rotationInterval) => { // AT refresh interval: a large fraction of the actual AT TTL - const atInterval = (rotationInterval.accessTokenExpiration - Date.now()) * config.rtr.rotationIntervalFraction; + const atInterval = (rotationInterval.accessTokenExpiration - Date.now()) * this.rtrConfig.rotationIntervalFraction; // RT timeout interval (session will end) and warning interval (warning that session will end) const rtTimeoutInterval = (rotationInterval.refreshTokenExpiration - Date.now()); - const rtWarningInterval = (rotationInterval.refreshTokenExpiration - Date.now()) - ms(config.rtr.fixedLengthSessionWarningTTL); + const rtWarningInterval = (rotationInterval.refreshTokenExpiration - Date.now()) - ms(this.rtrConfig.fixedLengthSessionWarningTTL); // schedule AT rotation IFF the AT will expire before the RT. this avoids // refresh-thrashing near the end of the FLS with progressively shorter @@ -149,7 +150,7 @@ export class FFetch { } // schedule FLS end-of-session warning - this.logger.log('rtr-fls', `end-of-session warning at ${new Date(rotationInterval.refreshTokenExpiration - ms(config.rtr.fixedLengthSessionWarningTTL))}`); + this.logger.log('rtr-fls', `end-of-session warning at ${new Date(rotationInterval.refreshTokenExpiration - ms(this.rtrConfig.fixedLengthSessionWarningTTL))}`); this.store.dispatch(setRtrFlsWarningTimeout(setTimeout(() => { this.logger.log('rtr-fls', 'emitting RTR_FLS_WARNING_EVENT'); window.dispatchEvent(new Event(RTR_FLS_WARNING_EVENT)); diff --git a/src/components/Root/FFetch.test.js b/src/components/Root/FFetch.test.js index 2ea85dc65..5659598de 100644 --- a/src/components/Root/FFetch.test.js +++ b/src/components/Root/FFetch.test.js @@ -4,13 +4,14 @@ import ms from 'ms'; import { waitFor } from '@testing-library/react'; -import { okapi, config } from 'stripes-config'; +import { okapi } from 'stripes-config'; import { getTokenExpiry } from '../../loginServices'; import { FFetch } from './FFetch'; import { RTRError, UnexpectedResourceError } from './Errors'; import { RTR_AT_EXPIRY_IF_UNKNOWN, + RTR_AT_TTL_FRACTION, RTR_FORCE_REFRESH_EVENT, RTR_FLS_WARNING_TTL, RTR_TIME_MARGIN_IN_MS, @@ -28,12 +29,6 @@ jest.mock('stripes-config', () => ({ okapi: { url: 'okapiUrl', tenant: 'okapiTenant' - }, - config: { - rtr: { - rotationIntervalFraction: 0.5, - fixedLengthSessionWarningTTL: '1m', - } } }), { virtual: true }); @@ -215,7 +210,9 @@ describe('FFetch class', () => { store: { dispatch: jest.fn(), }, - + rtrConfig: { + fixedLengthSessionWarningTTL: '1m', + }, }); testFfetch.replaceFetch(); testFfetch.replaceXMLHttpRequest(); @@ -229,7 +226,7 @@ describe('FFetch class', () => { await setTimeout(Promise.resolve(), 2000); // AT rotation - expect(st).toHaveBeenCalledWith(expect.any(Function), (accessTokenExpiration - whatTimeIsItMrFox) * config.rtr.rotationIntervalFraction); + expect(st).toHaveBeenCalledWith(expect.any(Function), (accessTokenExpiration - whatTimeIsItMrFox) * RTR_AT_TTL_FRACTION); // FLS warning expect(st).toHaveBeenCalledWith(expect.any(Function), (refreshTokenExpiration - whatTimeIsItMrFox) - ms(RTR_FLS_WARNING_TTL)); @@ -269,7 +266,9 @@ describe('FFetch class', () => { store: { dispatch: jest.fn(), }, - + rtrConfig: { + fixedLengthSessionWarningTTL: '1m', + }, }); testFfetch.replaceFetch(); testFfetch.replaceXMLHttpRequest(); @@ -281,7 +280,7 @@ describe('FFetch class', () => { // gross, but on the other, since we're deliberately pushing rotation // into a separate thread, I'm note sure of a better way to handle this. await setTimeout(Promise.resolve(), 2000); - expect(st).toHaveBeenCalledWith(expect.any(Function), (atExpires - whatTimeIsItMrFox) * config.rtr.rotationIntervalFraction); + expect(st).toHaveBeenCalledWith(expect.any(Function), (atExpires - whatTimeIsItMrFox) * RTR_AT_TTL_FRACTION); }); it('handles missing RTR data', async () => { @@ -305,7 +304,9 @@ describe('FFetch class', () => { store: { dispatch: jest.fn(), }, - + rtrConfig: { + fixedLengthSessionWarningTTL: '1m', + }, }); testFfetch.replaceFetch(); testFfetch.replaceXMLHttpRequest(); @@ -318,7 +319,7 @@ describe('FFetch class', () => { // into a separate thread, I'm not sure of a better way to handle this. await setTimeout(Promise.resolve(), 2000); - expect(st).toHaveBeenCalledWith(expect.any(Function), ms(RTR_AT_EXPIRY_IF_UNKNOWN) * config.rtr.rotationIntervalFraction); + expect(st).toHaveBeenCalledWith(expect.any(Function), ms(RTR_AT_EXPIRY_IF_UNKNOWN) * RTR_AT_TTL_FRACTION); }); it('handles unsuccessful responses', async () => { @@ -382,7 +383,9 @@ describe('FFetch class', () => { store: { dispatch: jest.fn(), }, - + rtrConfig: { + fixedLengthSessionWarningTTL: '1m', + }, }); testFfetch.replaceFetch(); testFfetch.replaceXMLHttpRequest(); @@ -396,7 +399,7 @@ describe('FFetch class', () => { await setTimeout(Promise.resolve(), 2000); // AT rotation - expect(st).not.toHaveBeenCalledWith(expect.any(Function), (accessTokenExpiration - whatTimeIsItMrFox) * config.rtr.rotationIntervalFraction); + expect(st).not.toHaveBeenCalledWith(expect.any(Function), (accessTokenExpiration - whatTimeIsItMrFox) * RTR_AT_TTL_FRACTION); // FLS warning expect(st).toHaveBeenCalledWith(expect.any(Function), (refreshTokenExpiration - whatTimeIsItMrFox) - ms(RTR_FLS_WARNING_TTL)); diff --git a/src/components/Root/Root.js b/src/components/Root/Root.js index 7fe8b9cbc..f63b0cfaf 100644 --- a/src/components/Root/Root.js +++ b/src/components/Root/Root.js @@ -63,26 +63,17 @@ class Root extends Component { this.apolloClient = createApolloClient(okapi); this.reactQueryClient = createReactQueryClient(); - // configure RTR with default props if needed - // gross: this overwrites whatever is currently stored at config.rtr - // gross: technically, the props could change and not get re-run through - // here. Realistically, that'll never happen since config values are read - // only once from a static file at build time, but still, props are props - // so technically it's possible. - // Also, ui-developer provides facilities to change some of this - this.props.config.rtr = configureRtr(this.props.config.rtr); - // enhanced security mode: // * configure fetch and xhr interceptors to conduct RTR // * see SessionEventContainer for RTR handling if (this.props.config.useSecureTokens) { - // FFetch relies on some of these properties, so we must ensure - // they are filled before initialization + const rtrConfig = configureRtr(this.props.config.rtr); + this.ffetch = new FFetch({ logger: this.props.logger, store, + rtrConfig, }); - this.ffetch.registerEventListener(); this.ffetch.replaceFetch(); this.ffetch.replaceXMLHttpRequest(); } @@ -137,6 +128,15 @@ class Root extends Component { return (); } + // make sure RTR is configured + // gross: this overwrites whatever is currently stored at config.rtr + // gross: technically, this may be different than what is configured + // in the constructor since the constructor only runs once but + // render runs when props change. realistically, that'll never happen + // since config values are read only once from a static file at build + // time, but still, props are props so technically it's possible. + config.rtr = configureRtr(this.props.config.rtr); + const stripes = new Stripes({ logger, store, diff --git a/src/components/SessionEventContainer/SessionEventContainer.js b/src/components/SessionEventContainer/SessionEventContainer.js index 85790c5bc..6774ffca8 100644 --- a/src/components/SessionEventContainer/SessionEventContainer.js +++ b/src/components/SessionEventContainer/SessionEventContainer.js @@ -264,10 +264,9 @@ const SessionEventContainer = ({ history }) => { // no deps? It should be history and stripes!!! >:) // We only want to configure the event listeners once, not every time - // there is a change to stripes or history. Hence, those are left out. - // we do include stripes.config.rtr, though, because these are configurable - // at runtime via ui-developer. - }, [stripes.config.rtr]); // eslint-disable-line react-hooks/exhaustive-deps + // there is a change to stripes or history. Hence, an empty dependency + // array. + }, []); // eslint-disable-line react-hooks/exhaustive-deps const renderList = []; From 51ecc012ea3435372c883047978a22bd37519203 Mon Sep 17 00:00:00 2001 From: Noah Overcash Date: Thu, 10 Oct 2024 16:45:46 -0400 Subject: [PATCH 12/12] fix tests --- src/components/Root/FFetch.test.js | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/components/Root/FFetch.test.js b/src/components/Root/FFetch.test.js index 5659598de..e47e0a9b8 100644 --- a/src/components/Root/FFetch.test.js +++ b/src/components/Root/FFetch.test.js @@ -212,6 +212,7 @@ describe('FFetch class', () => { }, rtrConfig: { fixedLengthSessionWarningTTL: '1m', + rotationIntervalFraction: 0.8, }, }); testFfetch.replaceFetch(); @@ -268,6 +269,7 @@ describe('FFetch class', () => { }, rtrConfig: { fixedLengthSessionWarningTTL: '1m', + rotationIntervalFraction: 0.8, }, }); testFfetch.replaceFetch(); @@ -306,6 +308,7 @@ describe('FFetch class', () => { }, rtrConfig: { fixedLengthSessionWarningTTL: '1m', + rotationIntervalFraction: 0.8, }, }); testFfetch.replaceFetch(); @@ -342,7 +345,11 @@ describe('FFetch class', () => { logger: { log }, store: { dispatch: jest.fn(), - } + }, + rtrConfig: { + fixedLengthSessionWarningTTL: '1m', + rotationIntervalFraction: 0.8, + }, }); testFfetch.replaceFetch(); testFfetch.replaceXMLHttpRequest(); @@ -385,6 +392,7 @@ describe('FFetch class', () => { }, rtrConfig: { fixedLengthSessionWarningTTL: '1m', + rotationIntervalFraction: 0.8, }, }); testFfetch.replaceFetch();