Skip to content

Commit

Permalink
STCOR-869 do not store /logout as a "return-to" URL
Browse files Browse the repository at this point in the history
When a session ends due to timeout, the current location is stored in
order to allow the subsequent session to begin where the previous one
left off. If the "session timeout" event fires more than once**,
however, this could lead to the `/logout` location being stored as
the "return to" location with obvious dire consequences.

There are two changes here:
1. Don't allow locations beginning with `/logout` to be stored. This
   fixes the symptom, not the root cause, but is still worthwhile.
2. Store the session-timeout interval ID in redux, and manage that timer
   via a redux action. Even though this _still_ shouldn't fire more than
   once, if it does, this allows us to cancel the previous timer before
   adding the next one. This is an attempt to fix the root cause.

Refs STCOR-869
  • Loading branch information
zburke committed Jul 25, 2024
1 parent 48ca517 commit 3a7a5f1
Show file tree
Hide file tree
Showing 6 changed files with 111 additions and 6 deletions.
11 changes: 7 additions & 4 deletions src/components/Root/FFetch.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ import { okapi as okapiConfig } from 'stripes-config';
import {
setRtrTimeout,
setRtrFlsTimeout,
setRtrFlsWarningTimeout,
} from '../../okapiActions';

import { getTokenExpiry } from '../../loginServices';
Expand Down Expand Up @@ -100,7 +101,7 @@ export class FFetch {
/**
* scheduleRotation
* Given a promise that resolves with timestamps for the AT's and RT's
* expiration, configure relevant corresponding timers:
* expiration, configure relevant corresponding timers:
* * before the AT expires, conduct RTR
* * when the RT is about to expire, send a "session will end" event
* * when the RT expires, send a "session ended" event"
Expand Down Expand Up @@ -131,15 +132,17 @@ 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.store.dispatch(setRtrFlsTimeout(setTimeout(() => {
this.store.dispatch(setRtrFlsWarningTimeout(setTimeout(() => {
this.logger.log('rtr-fls', 'emitting RTR_FLS_WARNING_EVENT');
window.dispatchEvent(new Event(RTR_FLS_WARNING_EVENT));
}, rtWarningInterval)));

// schedule FLS end-of-session logout
this.logger.log('rtr-fls', `session will end at ${new Date(rotationInterval.refreshTokenExpiration)}`);
setTimeout(() => {
this.store.dispatch(setRtrFlsTimeout(setTimeout(() => {
this.logger.log('rtr-fls', 'emitting RTR_FLS_TIMEOUT_EVENT');
window.dispatchEvent(new Event(RTR_FLS_TIMEOUT_EVENT));
}, rtTimeoutInterval);
}, rtTimeoutInterval)));
});
};

Expand Down
5 changes: 4 additions & 1 deletion src/loginServices.js
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,10 @@ export const setTokenExpiry = async (te) => {
const UNAUTHORIZED_PATH = 'unauthorized_path';
export const removeUnauthorizedPathFromSession = () => sessionStorage.removeItem(UNAUTHORIZED_PATH);
export const setUnauthorizedPathToSession = (pathname) => {
sessionStorage.setItem(UNAUTHORIZED_PATH, pathname ?? `${window.location.pathname}${window.location.search}`);
const path = pathname ?? `${window.location.pathname}${window.location.search}`;
if (!path.startsWith('/logout')) {
sessionStorage.setItem(UNAUTHORIZED_PATH, pathname ?? `${window.location.pathname}${window.location.search}`);
}
};
export const getUnauthorizedPathFromSession = () => sessionStorage.getItem(UNAUTHORIZED_PATH);

Expand Down
38 changes: 38 additions & 0 deletions src/loginServices.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,13 @@ import {
createOkapiSession,
getOkapiSession,
getTokenExpiry,
getUnauthorizedPathFromSession,
handleLoginError,
loadTranslations,
logout,
processOkapiSession,
setTokenExpiry,
setUnauthorizedPathToSession,
spreadUserWithPerms,
supportedLocales,
supportedNumberingSystems,
Expand Down Expand Up @@ -557,3 +559,39 @@ describe('logout', () => {
});
});
});

describe('setUnauthorizedPathToSession', () => {
beforeEach(() => {
window.sessionStorage.clear();
});

afterEach(() => {
window.sessionStorage.clear();
});

it('with an argument, uses it', () => {
const monkey = 'bagel';
setUnauthorizedPathToSession(monkey);
expect(getUnauthorizedPathFromSession()).toEqual(monkey);
});

it('without an argument, pulls value from window.location', () => {
window.location.pathname = '/monkey-bagel';
setUnauthorizedPathToSession();
expect(getUnauthorizedPathFromSession()).toEqual(window.location.pathname);
});

describe('refuses to set locations beginning with "/logout"', () => {
it('with an argument', () => {
const monkey = '/logout-timeout';
setUnauthorizedPathToSession(monkey);
expect(getUnauthorizedPathFromSession()).toBeFalsy();
});

it('without an argument', () => {
window.location.pathname = '/logout-timeout';
setUnauthorizedPathToSession();
expect(getUnauthorizedPathFromSession()).toBeFalsy();
});
});
});
16 changes: 15 additions & 1 deletion src/okapiActions.js
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,19 @@ function clearRtrTimeout() {
};
}

function setRtrFlsWarningTimeout(rtrFlsWarningTimeout) {
return {
type: OKAPI_REDUCER_ACTIONS.SET_RTR_FLS_WARNING_TIMEOUT,
rtrFlsWarningTimeout,
};
}

function clearRtrFlsWarningTimeout() {
return {
type: OKAPI_REDUCER_ACTIONS.CLEAR_RTR_FLS_WARNING_TIMEOUT,
};
}

function setRtrFlsTimeout(rtrFlsTimeout) {
return {
type: OKAPI_REDUCER_ACTIONS.SET_RTR_FLS_TIMEOUT,
Expand All @@ -177,7 +190,6 @@ function clearRtrFlsTimeout() {
};
}


function toggleRtrModal(isVisible) {
return {
type: OKAPI_REDUCER_ACTIONS.TOGGLE_RTR_MODAL,
Expand All @@ -190,6 +202,7 @@ export {
clearCurrentUser,
clearOkapiToken,
clearRtrFlsTimeout,
clearRtrFlsWarningTimeout,
clearRtrTimeout,
setAuthError,
setBindings,
Expand All @@ -203,6 +216,7 @@ export {
setOkapiToken,
setPlugins,
setRtrFlsTimeout,
setRtrFlsWarningTimeout,
setRtrTimeout,
setServerDown,
setSessionData,
Expand Down
17 changes: 17 additions & 0 deletions src/okapiReducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ export const OKAPI_REDUCER_ACTIONS = {
CLEAR_CURRENT_USER: 'CLEAR_CURRENT_USER',
CLEAR_OKAPI_TOKEN: 'CLEAR_OKAPI_TOKEN',
CLEAR_RTR_FLS_TIMEOUT: 'CLEAR_RTR_FLS_TIMEOUT',
CLEAR_RTR_FLS_WARNING_TIMEOUT: 'CLEAR_RTR_FLS_WARNING_TIMEOUT',
CLEAR_RTR_TIMEOUT: 'CLEAR_RTR_TIMEOUT',
OKAPI_READY: 'OKAPI_READY',
SERVER_DOWN: 'SERVER_DOWN',
Expand All @@ -18,6 +19,7 @@ export const OKAPI_REDUCER_ACTIONS = {
SET_OKAPI_TOKEN: 'SET_OKAPI_TOKEN',
SET_PLUGINS: 'SET_PLUGINS',
SET_RTR_FLS_TIMEOUT: 'SET_RTR_FLS_TIMEOUT',
SET_RTR_FLS_WARNING_TIMEOUT: 'SET_RTR_FLS_WARNING_TIMEOUT',
SET_RTR_TIMEOUT: 'SET_RTR_TIMEOUT',
SET_SESSION_DATA: 'SET_SESSION_DATA',
SET_SINGLE_PLUGIN: 'SET_SINGLE_PLUGIN',
Expand Down Expand Up @@ -47,9 +49,11 @@ export default function okapiReducer(state = {}, action) {
// if we're logging out, clear the RTR timeouts and related values
if (!action.isAuthenticated) {
clearTimeout(state.rtrTimeout);
clearTimeout(state.rtrFlsWarningTimeout);
clearTimeout(state.rtrFlsTimeout);
newState.rtrModalIsVisible = false;
newState.rtrTimeout = undefined;
newState.rtrFlsWarningTimeout = undefined;
newState.rtrFlsTimeout = undefined;
}

Expand Down Expand Up @@ -99,10 +103,23 @@ export default function okapiReducer(state = {}, action) {
clearTimeout(state.rtrTimeout);
return { ...state, rtrTimeout: action.rtrTimeout };
}

case OKAPI_REDUCER_ACTIONS.CLEAR_RTR_TIMEOUT: {
clearTimeout(state.rtrTimeout);
return { ...state, rtrTimeout: undefined };
}

// clear existing FLS warning timeout and set a new one
case OKAPI_REDUCER_ACTIONS.SET_RTR_FLS_WARNING_TIMEOUT: {
clearTimeout(state.rtrFlsWarningTimeout);
return { ...state, rtrFlsWarningTimeout: action.rtrFlsWarningTimeout };
}

case OKAPI_REDUCER_ACTIONS.CLEAR_RTR_FLS_WARNING_TIMEOUT: {
clearTimeout(state.rtrFlsWarningTimeout);
return { ...state, rtrFlsWarningTimeout: undefined };
}

// clear existing FLS timeout and set a new one
case OKAPI_REDUCER_ACTIONS.SET_RTR_FLS_TIMEOUT: {
clearTimeout(state.rtrFlsTimeout);
Expand Down
30 changes: 30 additions & 0 deletions src/okapiReducer.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,16 @@ describe('okapiReducer', () => {
const state = {
rtrModalIsVisible: true,
rtrTimeout: 123,
rtrFlsTimeout: 123,
rtrFlsWarningTimeout: 123,
};
const ct = jest.spyOn(window, 'clearTimeout');
const o = okapiReducer(state, { type: OKAPI_REDUCER_ACTIONS.SET_IS_AUTHENTICATED, isAuthenticated: false });
expect(o.isAuthenticated).toBe(false);
expect(o.rtrModalIsVisible).toBe(false);
expect(o.rtrTimeout).toBe(undefined);
expect(o.rtrFlsTimeout).toBe(undefined);
expect(o.rtrFlsWarningTimeout).toBe(undefined);
expect(ct).toHaveBeenCalled();
});
});
Expand Down Expand Up @@ -122,4 +125,31 @@ describe('okapiReducer', () => {
expect(o).toMatchObject({});
expect(ct).toHaveBeenCalledWith(state.rtrFlsTimeout);
});

it('SET_RTR_FLS_WARNING_TIMEOUT', () => {
const ct = jest.spyOn(window, 'clearTimeout');

const state = {
rtrFlsWarningTimeout: 991,
};

const newState = { rtrFlsWarningTimeout: 997 };

const o = okapiReducer(state, { type: OKAPI_REDUCER_ACTIONS.SET_RTR_FLS_WARNING_TIMEOUT, rtrFlsWarningTimeout: newState.rtrFlsWarningTimeout });
expect(o).toMatchObject(newState);

expect(ct).toHaveBeenCalledWith(state.rtrFlsWarningTimeout);
});

it('CLEAR_RTR_FLS_WARNING_TIMEOUT', () => {
const ct = jest.spyOn(window, 'clearTimeout');

const state = {
rtrFlsWarningTimeout: 991,
};

const o = okapiReducer(state, { type: OKAPI_REDUCER_ACTIONS.CLEAR_RTR_FLS_WARNING_TIMEOUT });
expect(o).toMatchObject({});
expect(ct).toHaveBeenCalledWith(state.rtrFlsWarningTimeout);
});
});

0 comments on commit 3a7a5f1

Please sign in to comment.