Skip to content

Commit

Permalink
test coverage is nice. magic numbers are not.
Browse files Browse the repository at this point in the history
  • Loading branch information
zburke committed Jun 10, 2024
1 parent 85334c4 commit d4fff99
Show file tree
Hide file tree
Showing 3 changed files with 179 additions and 2 deletions.
4 changes: 2 additions & 2 deletions src/components/Root/FFetch.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,10 +59,10 @@ import {
RTRError,
} from './Errors';
import {
RTR_AT_EXPIRY_IF_UNKNOWN,
RTR_AT_TTL_FRACTION,
RTR_ERROR_EVENT,
} from './constants';

import FXHR from './FXHR';

const OKAPI_FETCH_OPTIONS = {
Expand Down Expand Up @@ -139,7 +139,7 @@ export class FFetch {

// default: 10 seconds
this.logger.log('rtr', 'rotation scheduled with default value');
return 10 * 1000;
return ms(RTR_AT_EXPIRY_IF_UNKNOWN);
});

scheduleRotation(rotationPromise);
Expand Down
163 changes: 163 additions & 0 deletions src/components/Root/FFetch.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,15 @@
// FFetch for the reassign globals side-effect in its constructor.
/* eslint-disable no-unused-vars */

import ms from 'ms';

import { getTokenExpiry } from '../../loginServices';
import { FFetch } from './FFetch';
import { RTRError, UnexpectedResourceError } from './Errors';
import {
RTR_AT_EXPIRY_IF_UNKNOWN,
RTR_AT_TTL_FRACTION,
} from './constants';

jest.mock('../../loginServices', () => ({
...(jest.requireActual('../../loginServices')),
Expand Down Expand Up @@ -145,6 +151,163 @@ describe('FFetch class', () => {
});
});

describe('calling authentication resources', () => {
it('handles RTR data in the response', async () => {
// a static timestamp representing "now"
const whatTimeIsItMrFox = 1718042609734;

// a static timestamp of when the AT will expire, in the future
// this value will be pushed into the response returned from the fetch
const accessTokenExpiration = whatTimeIsItMrFox + 5000;

const st = jest.spyOn(window, 'setTimeout');

// dummy date data: assume session
Date.now = () => whatTimeIsItMrFox;

const cloneJson = jest.fn();
const clone = () => ({
ok: true,
json: () => Promise.resolve({ tokenExpiration: { accessTokenExpiration } })
});

mockFetch.mockResolvedValueOnce({
ok: false,
clone,
});

mockFetch.mockResolvedValueOnce('okapi success');
const testFfetch = new FFetch({
logger: { log },
store: {
dispatch: jest.fn(),
}
});
testFfetch.replaceFetch();
testFfetch.replaceXMLHttpRequest();

const response = await global.fetch('okapiUrl/bl-users/_self', { testOption: 'test' });
// why this extra await/setTimeout? Because RTR happens in an un-awaited
// promise in a separate thread fired off by setTimout, and we need to
// give it the chance to complete. on the one hand, this feels super
// 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);
});

it('handles RTR data in the session', async () => {
// a static timestamp representing "now"
const whatTimeIsItMrFox = 1718042609734;

// a static timestamp of when the AT will expire, in the future
// this value will be retrieved from local storage via getTokenExpiry
const atExpires = whatTimeIsItMrFox + 5000;

const st = jest.spyOn(window, 'setTimeout');

getTokenExpiry.mockResolvedValue({ atExpires });
Date.now = () => whatTimeIsItMrFox;

const cloneJson = jest.fn();
const clone = () => ({
ok: true,
json: () => Promise.resolve({}),
});

mockFetch.mockResolvedValueOnce({
ok: false,
clone,
});

mockFetch.mockResolvedValueOnce('okapi success');
const testFfetch = new FFetch({
logger: { log },
store: {
dispatch: jest.fn(),
}
});
testFfetch.replaceFetch();
testFfetch.replaceXMLHttpRequest();

const response = await global.fetch('okapiUrl/bl-users/_self', { testOption: 'test' });
// why this extra await/setTimeout? Because RTR happens in an un-awaited
// promise in a separate thread fired off by setTimout, and we need to
// give it the chance to complete. on the one hand, this feels super
// 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);
});

it('handles missing RTR data', async () => {
const st = jest.spyOn(window, 'setTimeout');
getTokenExpiry.mockResolvedValue({});

const cloneJson = jest.fn();
const clone = () => ({
ok: true,
json: () => Promise.resolve({}),
});

mockFetch.mockResolvedValueOnce({
ok: false,
clone,
});

mockFetch.mockResolvedValueOnce('okapi success');
const testFfetch = new FFetch({
logger: { log },
store: {
dispatch: jest.fn(),
}
});
testFfetch.replaceFetch();
testFfetch.replaceXMLHttpRequest();

const response = await global.fetch('okapiUrl/bl-users/_self', { testOption: 'test' });
// why this extra await/setTimeout? Because RTR happens in an un-awaited
// promise in a separate thread fired off by setTimout, and we need to
// give it the chance to complete. on the one hand, this feels super
// 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), ms(RTR_AT_EXPIRY_IF_UNKNOWN));
});

it('handles unsuccessful responses', async () => {
jest.spyOn(window, 'dispatchEvent');
jest.spyOn(console, 'error');

const cloneJson = jest.fn();
const clone = () => ({
ok: false,
json: cloneJson,
});

mockFetch.mockResolvedValueOnce({
ok: false,
clone,
});

mockFetch.mockResolvedValueOnce('okapi success');
const testFfetch = new FFetch({
logger: { log },
store: {
dispatch: jest.fn(),
}
});
testFfetch.replaceFetch();
testFfetch.replaceXMLHttpRequest();

const response = await global.fetch('okapiUrl/bl-users/_self', { testOption: 'test' });
expect(mockFetch.mock.calls).toHaveLength(1);
expect(cloneJson).not.toHaveBeenCalled();
});
});


describe('Calling an okapi fetch with missing token...', () => {
it('returns the error', async () => {
mockFetch.mockResolvedValue('success')
Expand Down
14 changes: 14 additions & 0 deletions src/components/Root/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,3 +47,17 @@ export const RTR_IDLE_SESSION_TTL = '60m';
* value must be a string parsable by ms()
*/
export const RTR_IDLE_MODAL_TTL = '1m';

/**
* When resuming an existing session but there is no token-expiration
* data in the session, we can't properly schedule RTR.
* 1. the real expiration data is in the cookie, but it's HTTPOnly
* 2. the resume-session API endpoint, _self, doesn't include
* token-expiration data in its response
* 3. the session _should_ contain a value, but maybe the session
* was corrupt.
* Given the resume-session API call succeeded, we know the AT must have been
* valid at the time, so we punt and schedule rotation in the future by this
* (relatively short) interval.
*/
export const RTR_AT_EXPIRY_IF_UNKNOWN = '10s';

0 comments on commit d4fff99

Please sign in to comment.