STCOR-776 optionally schedule RTR based on session data

src/components/Root/FFetch.js

@@ -47,6 +47,7 @@ import {
   setRtrTimeout
 } from '../../okapiActions';
 
+import { getTokenExpiry } from '../../loginServices';
 import {
   getPromise,
   isAuthenticationRequest,
@@ -58,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 = {
@@ -107,19 +108,42 @@ export class FFetch {
   rotateCallback = (res) => {
     this.logger.log('rtr', 'rotation callback setup');
 
-    // set a short rotation interval by default, then inspect the response for
-    // token-expiration data to use instead if available. not all responses
-    // (e.g. those from _self) contain token-expiration values, so it is
-    // necessary to provide a default.
-    let rotationInterval = 10 * 1000;
+    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)));
+      });
+    };
+
+    // When starting a new session, the response from /bl-users/login-with-expiry
+    // will contain AT expiration info, but when restarting an existing session,
+    // the response from /bl-users/_self will NOT, although that information will
+    // have been cached in local-storage.
+    //
+    // This means there are many places we have to check to figure out when the
+    // AT is likely to expire, and thus when we want to rotate. First inspect
+    // the response, otherwise the session. Default to 10 seconds.
     if (res?.accessTokenExpiration) {
-      rotationInterval = (new Date(res.accessTokenExpiration).getTime() - Date.now()) * RTR_AT_TTL_FRACTION;
+      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);
+
+      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;
+        }
+
+        // default: 10 seconds
+        this.logger.log('rtr', 'rotation scheduled with default value');
+        return ms(RTR_AT_EXPIRY_IF_UNKNOWN);
+      });
+
+      scheduleRotation(rotationPromise);
     }
-
-    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)));
   }
 
   /**
@@ -158,13 +182,16 @@ export class FFetch {
         this.logger.log('rtr', 'authn request');
         return this.nativeFetch.apply(global, [resource, options && { ...options, ...OKAPI_FETCH_OPTIONS }])
           .then(res => {
-            this.logger.log('rtr', 'authn success!');
             // a response can only be read once, so we clone it to grab the
             // tokenExpiration in order to kick of the rtr cycle, then return
             // the original
-            res.clone().json().then(json => {
-              this.rotateCallback(json.tokenExpiration);
-            });
+            const clone = res.clone();
+            if (clone.ok) {
+              this.logger.log('rtr', 'authn success!');
+              clone.json().then(json => {
+                this.rotateCallback(json.tokenExpiration);
+              });
+            }
 
             return res;
           });

src/components/Root/FFetch.test.js

@@ -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')),
@@ -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')

src/components/Root/constants.js

@@ -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';

src/loginServices.js

@@ -742,7 +742,9 @@ export function validateUser(okapiUrl, store, tenant, session) {
 
 /**
  * checkOkapiSession
- * 1. Pull the session from local storage; if non-empty validate it, dispatching load-resources actions.
+ * 1. Pull the session from local storage; if it contains a user id,
+ *    validate it by fetching /_self to verify that it is still active,
+ *    dispatching load-resources actions.
  * 2. Check if SSO (SAML) is enabled, dispatching check-sso actions
  * 3. dispatch set-okapi-ready.
  *
@@ -753,7 +755,7 @@ export function validateUser(okapiUrl, store, tenant, session) {
 export function checkOkapiSession(okapiUrl, store, tenant) {
   getOkapiSession()
     .then((sess) => {
-      return sess !== null ? validateUser(okapiUrl, store, tenant, sess) : null;
+      return sess?.user?.id ? validateUser(okapiUrl, store, tenant, sess) : null;
     })
     .then(() => {
       return getSSOEnabled(okapiUrl, store, tenant);