Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[STCOR-888] RTR dynamic configuration, debugging event #1535

Open
wants to merge 16 commits into
base: master
Choose a base branch
from
27 changes: 22 additions & 5 deletions src/components/Root/FFetch.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@
*/

import ms from 'ms';
import { okapi } from 'stripes-config';
import { okapi, config } from 'stripes-config';
import {
setRtrTimeout
} from '../../okapiActions';
Expand All @@ -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';

Expand All @@ -76,6 +76,24 @@ export class FFetch {
this.store = store;
}

/**
* 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, 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);
}

/**
* save a reference to fetch, and then reassign the global :scream:
*/
Expand Down Expand Up @@ -110,7 +128,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)));
Expand All @@ -127,14 +144,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
Expand Down
34 changes: 31 additions & 3 deletions src/components/Root/FFetch.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => ({
Expand All @@ -24,6 +25,11 @@ jest.mock('stripes-config', () => ({
okapi: {
url: 'okapiUrl',
tenant: 'okapiTenant'
},
config: {
rtr: {
rotationIntervalFraction: 0.5,
}
}
}),
{ virtual: true });
Expand All @@ -32,13 +38,18 @@ const log = jest.fn();

const mockFetch = jest.fn();

// to ensure we cleanup after each test
const instancesWithEventListeners = [];

describe('FFetch class', () => {
beforeEach(() => {
global.fetch = mockFetch;
getTokenExpiry.mockResolvedValue({
atExpires: Date.now() + (10 * 60 * 1000),
rtExpires: Date.now() + (10 * 60 * 1000),
});
instancesWithEventListeners.forEach(instance => instance.unregisterEventListener());
instancesWithEventListeners.length = 0;
});

afterEach(() => {
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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);
ncovercash marked this conversation as resolved.
Show resolved Hide resolved
});

it('handles RTR data in the session', async () => {
Expand Down Expand Up @@ -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 () => {
Expand Down
7 changes: 5 additions & 2 deletions src/components/Root/Root.js
Original file line number Diff line number Diff line change
Expand Up @@ -67,10 +67,15 @@ 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,
});
this.ffetch.registerEventListener();
this.ffetch.replaceFetch();
this.ffetch.replaceXMLHttpRequest();
}
Expand Down Expand Up @@ -125,8 +130,6 @@ class Root extends Component {
}

// make sure RTR is configured
config.rtr = configureRtr(this.props.config.rtr);

const stripes = new Stripes({
logger,
store,
Expand Down
8 changes: 7 additions & 1 deletion src/components/Root/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Expand All @@ -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;

/**
Expand Down
6 changes: 6 additions & 0 deletions src/components/Root/token-util.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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;
Expand Down
30 changes: 16 additions & 14 deletions src/components/Root/token-util.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zburke can you comment on this? As-is, this configuration cannot be modified at runtime by ui-developer.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably I started with a non-empty array and it didn't work as expected, so I added the empty array as a crutch and never looked back. Your observation/criticism is fair, but I don't have time to debug this further since the only thing "broken" about it is that it can't be modified at runtime via dev-tools.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested it, and it works beautifully. Only the RTR config in ui-developer causes the effect to re-run, nothing else (not even the other config forms in developer)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does rely on not calling configureRtr on render in Root, though.


// show the idle-session warning modal if necessary;
Expand Down
Loading