STCOR-907 cautiously evaluate localforage data

Be thorough when evaluating data from localforage to determine whether a
session exists by checking for `user.id`, `tenant`, and
`isAuthenticated` values rather than accepting the existence of a
(possibly empty, or mostly empty) object as proof.

Without this check, a sparsely-populated value may be passed to
`validateUser()`, causing it to throw when evaluating `user.id`, and
misleadingly dispatching a server-down message.

It is likely that a rogue RTR process is responsible for writing this
garbled/sparse session data. We need to research that and resolve that
problem too, but at least we have a handle on this from the other end.

Refs STCOR-907

CHANGELOG.md

@@ -1,5 +1,9 @@
 # Change history for stripes-core
 
+## 10.1.3 IN PROGRESS
+
+* Cautiously evaluate localforage data when restoring a session. Refs STCOR-907.
+
 ## [10.1.2](https://github.com/folio-org/stripes-core/tree/v10.1.2) (2024-10-21)
 [Full Changelog](https://github.com/folio-org/stripes-core/compare/v10.1.1...v10.1.2)
 

src/components/Root/Root.js

@@ -81,7 +81,11 @@ class Root extends Component {
 
   componentDidMount() {
     const { okapi, store, locale, defaultTranslations } = this.props;
-    if (this.withOkapi) checkOkapiSession(okapi.url, store, okapi.tenant);
+    if (this.withOkapi) {
+      // checkOkapiSession is async and will cause a re-render after a delay
+      // when an API call's .then() handler updates the store.
+      checkOkapiSession(okapi.url, store, okapi.tenant);
+    }
     // TODO: remove this after we load locale and translations at start from a public endpoint
     loadTranslations(store, locale, defaultTranslations);
   }

src/components/Root/token-util.test.js

@@ -1,3 +1,5 @@
+import localforage from 'localforage';
+
 import { RTRError, UnexpectedResourceError } from './Errors';
 import {
   isFolioApiRequest,
@@ -12,6 +14,16 @@ import {
 } from './token-util';
 import { RTR_SUCCESS_EVENT } from './Events';
 
+jest.mock('localforage', () => ({
+  getItem: jest.fn(() => Promise.resolve({
+    user: { id: 'robert' },
+    tenant: 'manhattan',
+    isAuthenticated: 'i am',
+  })),
+  setItem: jest.fn((k, v) => Promise.resolve(v)),
+  removeItem: jest.fn(() => Promise.resolve()),
+}));
+
 describe('isFolioApiRequest', () => {
   it('accepts requests whose origin matches okapi\'s', () => {
     const oUrl = 'https://millicent-sounds-kinda-like-malificent.edu';

src/loginServices.js

@@ -76,18 +76,18 @@ const SESSION_NAME = 'okapiSess';
  * simple wrapper around access to values stored in localforage
  * to insulate RTR functions from that API.
  *
- * @returns {object}
+ * @returns Promise.resolve({object})
  */
 export const getOkapiSession = async () => {
   return localforage.getItem(SESSION_NAME);
 };
 
 /**
- * getTokenSess
+ * getTokenExpiry
  * simple wrapper around access to values stored in localforage
  * to insulate RTR functions from that API.
  *
- * @returns {object} shaped like { atExpires, rtExpires }; each is a millisecond timestamp
+ * @returns Promise.resolve({object}) shaped like { atExpires, rtExpires }; each is a millisecond timestamp
  */
 export const getTokenExpiry = async () => {
   const sess = await getOkapiSession();
@@ -101,11 +101,15 @@ export const getTokenExpiry = async () => {
  * session with updated token expiration data.
  *
  * @param {object} shaped like { atExpires, rtExpires }; each is a millisecond timestamp
- * @returns {object} updated session object
+ * @returns Promise.resolve({object}) updated session object
  */
 export const setTokenExpiry = async (te) => {
   const sess = await getOkapiSession();
-  return localforage.setItem(SESSION_NAME, { ...sess, tokenExpiration: te });
+  if (sess) {
+    return localforage.setItem(SESSION_NAME, { ...sess, tokenExpiration: te });
+  }
+
+  return null;
 };
 
 
@@ -708,18 +712,23 @@ export function validateUser(okapiUrl, store, tenant, session) {
 
 /**
  * checkOkapiSession
- * 1. Pull the session from local storage; if non-empty validate it, dispatching load-resources actions.
- * 2. Check if SSO (SAML) is enabled, dispatching check-sso actions
- * 3. dispatch set-okapi-ready.
+ * 1 Pull the session from local storage; if non-empty validate it,
+ *   dispatching load-resources actions to populate the store.
+ * 2 Check if SSO (SAML) is enabled, dispatching check-sso actions
+ * 3 dispatch set-okapi-ready.
  *
  * @param {string} okapiUrl
  * @param {redux store} store
  * @param {string} tenant
  */
 export function checkOkapiSession(okapiUrl, store, tenant) {
-  getOkapiSession()
+  return getOkapiSession()
     .then((sess) => {
-      return sess !== null ? validateUser(okapiUrl, store, tenant, sess) : null;
+      if (sess?.user?.id && sess?.tenant && sess?.isAuthenticated) {
+        return validateUser(okapiUrl, store, tenant, sess);
+      }
+
+      return null;
     })
     .then(() => {
       return getSSOEnabled(okapiUrl, store, tenant);
@@ -821,27 +830,38 @@ export function requestSSOLogin(okapiUrl, tenant) {
 export function updateUser(store, data) {
   return getOkapiSession()
     .then((sess) => {
-      sess.user = { ...sess.user, ...data };
-      return localforage.setItem(SESSION_NAME, sess);
+      if (sess?.user?.id && sess?.tenant && sess?.isAuthenticated) {
+        sess.user = { ...sess.user, ...data };
+        return localforage.setItem(SESSION_NAME, sess);
+      }
+
+      throw Error('The user could not be updated because an existing session could not be found');
     })
     .then(() => {
       store.dispatch(updateCurrentUser(data));
+    })
+    .catch((e) => {
+      console.error(e); // eslint-disable-line no-console
     });
 }
 
 /**
  * updateTenant
  * 1. prepare user info for requested tenant
- * 2. update okapi session
+ * 2. update okapi session with user and tenant data
  * @param {object} okapi
  * @param {string} tenant
  *
  * @returns {Promise}
  */
 export async function updateTenant(okapi, tenant) {
-  const okapiSess = await getOkapiSession();
-  const userWithPermsResponse = await fetchUserWithPerms(okapi.url, tenant, okapi.token);
-  const userWithPerms = await userWithPermsResponse.json();
+  const sess = await getOkapiSession();
+  if (sess?.user?.id && sess?.tenant && sess?.isAuthenticated) {
+    const userWithPermsResponse = await fetchUserWithPerms(okapi.url, tenant, okapi.token);
+    const userWithPerms = await userWithPermsResponse.json();
 
-  await localforage.setItem(SESSION_NAME, { ...okapiSess, tenant, ...spreadUserWithPerms(userWithPerms) });
+    await localforage.setItem(SESSION_NAME, { ...sess, tenant, ...spreadUserWithPerms(userWithPerms) });
+  } else {
+    console.error('The tenant could not be updated because an existing session could not be found'); // eslint-disable-line no-console
+  }
 }

src/loginServices.test.js

@@ -342,17 +342,41 @@ describe('validateUser', () => {
 });
 
 describe('updateUser', () => {
-  it('dispatches updateCurrentUser', async () => {
+  it('dispatches updateCurrentUser when a session exists', async () => {
+    const s = {
+      user: { id: 'robert' },
+      tenant: 'manhattan',
+      isAuthenticated: 'i am',
+    };
+    localforage.getItem = () => Promise.resolve(s);
+
     const store = {
       dispatch: jest.fn(),
     };
     const data = { thunder: 'chicken' };
     await updateUser(store, data);
     expect(store.dispatch).toHaveBeenCalledWith(updateCurrentUser(data));
   });
+
+  it('does nothing when session is not found', async () => {
+    localforage.getItem = () => Promise.resolve(null);
+
+    const store = {
+      dispatch: jest.fn(),
+    };
+    const data = { thunder: 'chicken' };
+    await updateUser(store, data);
+    expect(store.dispatch).not.toHaveBeenCalled();
+  });
 });
 
 describe('updateTenant', () => {
+  const s = {
+    user: { id: 'robert' },
+    tenant: 'manhattan',
+    isAuthenticated: 'i am',
+  };
+
   const okapi = {
     currentPerms: {},
   };
@@ -371,16 +395,30 @@ describe('updateTenant', () => {
     localforage.setItem.mockClear();
   });
 
-  it('should set tenant and updated user in session', async () => {
+  it('updates tenant and user when session exists', async () => {
+    localforage.getItem = () => Promise.resolve(s);
+    localforage.setItem = jest.fn(() => Promise.resolve(s));
+
     mockFetchSuccess(data);
     await updateTenant(okapi, tenant);
     mockFetchCleanUp();
 
     expect(localforage.setItem).toHaveBeenCalledWith('okapiSess', {
+      ...s,
       ...spreadUserWithPerms(data),
       tenant,
     });
   });
+
+  it('does nothing when session is not found', async () => {
+    localforage.getItem = () => Promise.resolve(null);
+
+    mockFetchSuccess(data);
+    await updateTenant(okapi, tenant);
+    mockFetchCleanUp();
+
+    expect(localforage.setItem).not.toHaveBeenCalled();
+  });
 });
 
 describe('localforage session wrapper', () => {
@@ -410,22 +448,37 @@ describe('localforage session wrapper', () => {
     });
   });
 
-  it('setTokenExpiry set', async () => {
-    const o = {
-      margo: 'timmins',
-      margot: 'margot with a t looks better',
-      also: 'i thought we were talking about margot robbie?',
-      tokenExpiration: 'time out of mind',
-    };
-    localforage.getItem = () => Promise.resolve(o);
-    localforage.setItem = (k, v) => Promise.resolve(v);
+  describe('setTokenExpiry', () => {
+    it('saves data when a session exists', async () => {
+      const o = {
+        margo: 'timmins',
+        margot: 'margot with a t looks better',
+        also: 'i thought we were talking about margot robbie?',
+        tokenExpiration: 'time out of mind',
+      };
+      localforage.getItem = () => Promise.resolve(o);
+      localforage.setItem = (k, v) => Promise.resolve(v);
 
-    const te = {
-      trinity: 'cowboy junkies',
-      sweet: 'james',
-    };
+      const te = {
+        trinity: 'cowboy junkies',
+        sweet: 'james',
+      };
+
+      const s = await setTokenExpiry(te);
+      expect(s).toMatchObject({ ...o, tokenExpiration: te });
+    });
+
+    it('does nothing when session does not exist', async () => {
+      const o = null;
+      localforage.getItem = () => Promise.resolve(o);
+      localforage.setItem = (k, v) => Promise.resolve(v);
 
-    const s = await setTokenExpiry(te);
-    expect(s).toMatchObject({ ...o, tokenExpiration: te });
+      const te = {
+        now: 'i am become death, destroyer of sessions',
+      };
+
+      const s = await setTokenExpiry(te);
+      expect(s).toBeNull();
+    });
   });
 });