From c25e068e2c1df869b171e663be9eac4f80a5240d Mon Sep 17 00:00:00 2001 From: Zak Burke Date: Fri, 26 Jul 2024 16:12:40 -0400 Subject: [PATCH] STCOR-869 do not store /logout as a "return-to" URL (#1510) 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 --- src/components/Root/FFetch.js | 11 ++++++---- src/loginServices.js | 5 ++++- src/loginServices.test.js | 38 +++++++++++++++++++++++++++++++++++ src/okapiActions.js | 16 ++++++++++++++- src/okapiReducer.js | 17 ++++++++++++++++ src/okapiReducer.test.js | 30 +++++++++++++++++++++++++++ 6 files changed, 111 insertions(+), 6 deletions(-) diff --git a/src/components/Root/FFetch.js b/src/components/Root/FFetch.js index a689523f8..0b10068d1 100644 --- a/src/components/Root/FFetch.js +++ b/src/components/Root/FFetch.js @@ -46,6 +46,7 @@ import { okapi as okapiConfig } from 'stripes-config'; import { setRtrTimeout, setRtrFlsTimeout, + setRtrFlsWarningTimeout, } from '../../okapiActions'; import { getTokenExpiry } from '../../loginServices'; @@ -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" @@ -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))); }); }; diff --git a/src/loginServices.js b/src/loginServices.js index 08407e37a..d30576c46 100644 --- a/src/loginServices.js +++ b/src/loginServices.js @@ -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); diff --git a/src/loginServices.test.js b/src/loginServices.test.js index 6827bf9ae..60579cd8f 100644 --- a/src/loginServices.test.js +++ b/src/loginServices.test.js @@ -4,11 +4,13 @@ import { createOkapiSession, getOkapiSession, getTokenExpiry, + getUnauthorizedPathFromSession, handleLoginError, loadTranslations, logout, processOkapiSession, setTokenExpiry, + setUnauthorizedPathToSession, spreadUserWithPerms, supportedLocales, supportedNumberingSystems, @@ -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(); + }); + }); +}); diff --git a/src/okapiActions.js b/src/okapiActions.js index c9d3795ab..6debc86ca 100644 --- a/src/okapiActions.js +++ b/src/okapiActions.js @@ -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, @@ -177,7 +190,6 @@ function clearRtrFlsTimeout() { }; } - function toggleRtrModal(isVisible) { return { type: OKAPI_REDUCER_ACTIONS.TOGGLE_RTR_MODAL, @@ -190,6 +202,7 @@ export { clearCurrentUser, clearOkapiToken, clearRtrFlsTimeout, + clearRtrFlsWarningTimeout, clearRtrTimeout, setAuthError, setBindings, @@ -203,6 +216,7 @@ export { setOkapiToken, setPlugins, setRtrFlsTimeout, + setRtrFlsWarningTimeout, setRtrTimeout, setServerDown, setSessionData, diff --git a/src/okapiReducer.js b/src/okapiReducer.js index 2fc52174b..023e215d0 100644 --- a/src/okapiReducer.js +++ b/src/okapiReducer.js @@ -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', @@ -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', @@ -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; } @@ -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); diff --git a/src/okapiReducer.test.js b/src/okapiReducer.test.js index ce366491e..5a825bbce 100644 --- a/src/okapiReducer.test.js +++ b/src/okapiReducer.test.js @@ -13,6 +13,8 @@ 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 }); @@ -20,6 +22,7 @@ describe('okapiReducer', () => { expect(o.rtrModalIsVisible).toBe(false); expect(o.rtrTimeout).toBe(undefined); expect(o.rtrFlsTimeout).toBe(undefined); + expect(o.rtrFlsWarningTimeout).toBe(undefined); expect(ct).toHaveBeenCalled(); }); }); @@ -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); + }); });