STCOR-776 optionally schedule RTR based on session data (#1488)

Not all authentication-related responses are created equal. If the
response contains AT expiration data, i.e. it is a response from
`/bl-users/login-with-expiry`, use that data to configure RTR. If the
response does not contain such data, i.e. it is a response from
`/bl-users/_self`, pull AT expiration data from the session and use
that. If we still don't have any AT expiration data, cross your fingers
and try rotating in 10 seconds.

Previously, we did not ever check the session, so RTR always fired 10
seconds into a resumed, which is exactly the situation faced in e2e
tests.

This doesn't precisely handle the issue faced by e2e tests that stall
when the rtr requests fires, but leveraging the session data should
push the rtr request far enough into the future that most e2e tests
will now avoid it. We should still try to understand that problem and
solve it, but in the mean time this may enable us to work around it.

Refs STCOR-776

(cherry picked from commit e93a5af)

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

@@ -762,7 +762,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.
  *
@@ -773,7 +775,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(() => {
       if (store.getState().discovery?.interfaces?.['login-saml']) {