Revert "STCOR-756 correctly evaluate token lifespan (#1363)"

This reverts commit 607dc5c.
folio-org · Nov 10, 2023 · 027bd74 · 027bd74
1 parent a991ba8
commit 027bd74
Show file tree

Hide file tree

Showing 6 changed files with 64 additions and 120 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -7,7 +7,6 @@
 * Ensure `<AppIcon>` is not cut off when app name is long. Refs STCOR-752.
 * Use cookies and RTR instead of directly handling the JWT. Refs STCOR-671, FOLIO-3627.
 * Shrink the token lifespan so we are less likely to use an expired one. Refs STCOR-754.
-* Correctly evaluate token lifespan; use consistent protocol for service worker messages. Refs STCOR-756.
 * Allow console to be preserved on logout. STCOR-761.
 
 ## [10.0.0](https://github.com/folio-org/stripes-core/tree/v10.0.0) (2023-10-11)

diff --git a/src/loginServices.js b/src/loginServices.js
@@ -387,25 +387,20 @@ export async function logout(okapiUrl, store) {
 /**
  * postTokenExpiration
  * send SW a TOKEN_EXPIRATION message
- * @returns {Promise}
  */
 const postTokenExpiration = (tokenExpiration) => {
-  if ('serviceWorker' in navigator) {
-    return navigator.serviceWorker.ready
-      .then((reg) => {
-        const sw = reg.active;
-        if (sw) {
-          const message = { source: '@folio/stripes-core', type: 'TOKEN_EXPIRATION', value: { tokenExpiration } };
-          logger.log('rtr', '<= sending', message);
-          sw.postMessage(message);
-        } else {
-          logger.log('rtr', 'error, could not send TOKEN_EXPIRATION message; no ServiceWorker is active');
-        }
-      });
-  }
-
-  logger.log('rtr', 'error, could not send TOKEN_EXPIRATION message; navigator.serviceWorker is empty');
-  return Promise.resolve();
+  navigator.serviceWorker.ready
+    .then((reg) => {
+      const sw = reg.active;
+      if (sw) {
+        const message = { source: '@folio/stripes-core', type: 'TOKEN_EXPIRATION', tokenExpiration };
+        logger.log('rtr', '<= sending', message);
+        sw.postMessage(message);
+      } else {
+        // eslint-disable-next-line no-console
+        console.warn('could not dispatch message; no active registration');
+      }
+    });
 };
 
 /**
@@ -459,8 +454,9 @@ export function createOkapiSession(okapiUrl, store, tenant, data) {
   };
 
   // provide token-expiration info to the service worker
-  return postTokenExpiration(tokenExpiration)
-    .then(localforage.setItem('loginResponse', data))
+  postTokenExpiration(tokenExpiration);
+
+  return localforage.setItem('loginResponse', data)
     .then(() => localforage.setItem(SESSION_NAME, okapiSess))
     .then(() => {
       store.dispatch(setIsAuthenticated(true));
@@ -490,14 +486,14 @@ export const handleServiceWorkerMessage = (event, store) => {
     // RTR happened: update token expiration timestamps in our store
     if (event.data.type === 'TOKEN_EXPIRATION') {
       store.dispatch(setTokenExpiration({
-        atExpires: new Date(event.data.value.tokenExpiration.atExpires).toISOString(),
-        rtExpires: new Date(event.data.value.tokenExpiration.rtExpires).toISOString(),
+        atExpires: new Date(event.data.tokenExpiration.atExpires).toISOString(),
+        rtExpires: new Date(event.data.tokenExpiration.rtExpires).toISOString(),
       }));
     }
 
     // RTR failed: we have no cookies; logout
     if (event.data.type === 'RTR_ERROR') {
-      logger.log('rtr', 'rtr error; logging out', event.data.error);
+      console.error('-- (rtr) rtr error; logging out', event.data.error); // eslint-disable-line no-console
       store.dispatch(setIsAuthenticated(false));
       store.dispatch(clearCurrentUser());
       store.dispatch(resetStore());
@@ -512,8 +508,6 @@ export function addServiceWorkerListeners(okapiConfig, store) {
     navigator.serviceWorker.addEventListener('message', (e) => {
       handleServiceWorkerMessage(e, store);
     });
-  } else {
-    logger.log('rtr', 'error; navigator.serviceWorker is empty');
   }
 }
 
@@ -664,17 +658,18 @@ export function validateUser(okapiUrl, store, tenant, session) {
           rtExpires: Date.now() + (10 * 60 * 1000),
         };
         // provide token-expiration info to the service-worker
-        return postTokenExpiration(tokenExpiration)
-          .then(() => {
-            store.dispatch(setSessionData({
-              isAuthenticated: true,
-              user,
-              perms,
-              tenant: sessionTenant,
-              tokenExpiration,
-            }));
-            return loadResources(okapiUrl, store, sessionTenant, user.id);
-          });
+        // it returns a promise, but we don't await; the service-worker
+        // can operate asynchronously and that's just fine.
+        postTokenExpiration(tokenExpiration);
+
+        store.dispatch(setSessionData({
+          isAuthenticated: true,
+          user,
+          perms,
+          tenant: sessionTenant,
+          tokenExpiration,
+        }));
+        return loadResources(okapiUrl, store, sessionTenant, user.id);
       });
     } else {
       return logout(okapiUrl, store);

diff --git a/src/loginServices.test.js b/src/loginServices.test.js
@@ -128,11 +128,9 @@ describe('createOkapiSession', () => {
     const message = {
       source: '@folio/stripes-core',
       type: 'TOKEN_EXPIRATION',
-      value: {
-        tokenExpiration: {
-          atExpires: new Date('2023-11-06T18:05:33Z').getTime(),
-          rtExpires: new Date('2023-10-30T18:15:33Z').getTime(),
-        },
+      tokenExpiration: {
+        atExpires: new Date('2023-11-06T18:05:33Z').getTime(),
+        rtExpires: new Date('2023-10-30T18:15:33Z').getTime(),
       }
     };
     expect(postMessage).toHaveBeenCalledWith(message);
@@ -328,12 +326,10 @@ describe('validateUser', () => {
     const message = {
       source: '@folio/stripes-core',
       type: 'TOKEN_EXPIRATION',
-      value: {
-        tokenExpiration: {
-          atExpires: -1,
-          rtExpires: new Date(now).getTime() + (10 * 60 * 1000),
-        },
-      },
+      tokenExpiration: {
+        atExpires: -1,
+        rtExpires: new Date(now).getTime() + (10 * 60 * 1000),
+      }
     };
     expect(postMessage).toHaveBeenCalledWith(message);
 
@@ -484,7 +480,7 @@ describe('handleServiceWorkerMessage', () => {
         data: {
           source: '@folio/stripes-core',
           type: 'TOKEN_EXPIRATION',
-          value: { tokenExpiration },
+          tokenExpiration,
         }
       };
 

diff --git a/src/service-worker.js b/src/service-worker.js
@@ -94,9 +94,8 @@ export const TTL_WINDOW = 0.8;
  * @returns boolean
  */
 export const isValidAT = (te) => {
-  const isValid = !!(te?.atExpires > Date.now());
-  if (shouldLog) console.log(`-- (rtr-sw) => at isValid? ${isValid}; expires ${new Date(te?.atExpires || null).toISOString()}`);
-  return isValid;
+  if (shouldLog) console.log(`-- (rtr-sw) => at expires ${new Date(te?.atExpires || null).toISOString()}`);
+  return !!(te?.atExpires * TTL_WINDOW > Date.now());
 };
 
 /**
@@ -106,9 +105,8 @@ export const isValidAT = (te) => {
  * @returns boolean
  */
 export const isValidRT = (te) => {
-  const isValid = !!(te?.rtExpires > Date.now());
-  if (shouldLog) console.log(`-- (rtr-sw) => rt isValid? ${isValid}; expires ${new Date(te?.rtExpires || null).toISOString()}`);
-  return isValid;
+  if (shouldLog) console.log(`-- (rtr-sw) => rt expires ${new Date(te?.rtExpires || null).toISOString()}`);
+  return !!(te?.rtExpires * TTL_WINDOW > Date.now());
 };
 
 /**
@@ -140,20 +138,6 @@ export const messageToClient = async (event, message) => {
   client.postMessage({ ...message, source: '@folio/stripes-core' });
 };
 
-/**
- * handleTokenExpiration
- * Set the AT and RT token expirations to the fraction of their TTL given by
- * TTL_WINDOW. e.g. if a token should be valid for 100 more seconds and TTL_WINDOW
- * is 0.8, set to the expiration time to 80 seconds from now.
- *
- * @param {object} value { tokenExpiration: { atExpires, rtExpires }} both are millisecond timestamps
- * @returns { tokenExpiration: { atExpires, rtExpires }} both are millisecond timestamps
- */
-export const handleTokenExpiration = (value) => ({
-  atExpires: Date.now() + ((value.tokenExpiration.atExpires - Date.now()) * TTL_WINDOW),
-  rtExpires: Date.now() + ((value.tokenExpiration.rtExpires - Date.now()) * TTL_WINDOW),
-});
-
 /**
  * rtr
  * exchange an RT for a new one.
@@ -216,14 +200,11 @@ export const rtr = async (event) => {
     .then(json => {
       if (shouldLog) console.log('-- (rtr-sw) **     success!');
       isRotating = false;
-      tokenExpiration = handleTokenExpiration({
-        tokenExpiration: {
-          atExpires: new Date(json.accessTokenExpiration).getTime(),
-          rtExpires: new Date(json.refreshTokenExpiration).getTime(),
-        }
-      });
-
-      messageToClient(event, { type: 'TOKEN_EXPIRATION', value: { tokenExpiration } });
+      tokenExpiration = {
+        atExpires: new Date(json.accessTokenExpiration).getTime(),
+        rtExpires: new Date(json.refreshTokenExpiration).getTime(),
+      };
+      messageToClient(event, { type: 'TOKEN_EXPIRATION', tokenExpiration });
     });
 };
 
@@ -306,7 +287,7 @@ const passThroughWithRT = (event) => {
       // Promise.reject() here would result in every single fetch in every
       // single application needing to thoughtfully handle RTR_ERROR responses.
       messageToClient(event, { type: 'RTR_ERROR', error: rtre });
-      return Promise.resolve(new Response(JSON.stringify({})));
+      return Promise.resolve(new Response({}));
     });
 };
 
@@ -367,7 +348,7 @@ export const passThroughLogout = (event) => {
     .catch(e => {
       // kill me softly: return an empty response to allow graceful failure
       console.error('-- (rtr-sw) logout failure', e); // eslint-disable-line no-console
-      return Promise.resolve(new Response(JSON.stringify({})));
+      return Promise.resolve(new Response({}));
     });
 };
 
@@ -408,8 +389,8 @@ export const passThrough = (event, te, oUrl) => {
     // and handle it, hopefully by logging out.
     // Promise.reject() here would result in every single fetch in every
     // single application needing to thoughtfully handle RTR_ERROR responses.
-    messageToClient(event, { type: 'RTR_ERROR', error: `AT/RT failure accessing ${req.url}` });
-    return Promise.resolve(new Response(JSON.stringify({})));
+    messageToClient(event, { type: 'RTR_ERROR', error: 'AT/RT failure' });
+    return Promise.resolve(new Response({}));
   }
 
   // default: pass requests through to the network
@@ -454,21 +435,17 @@ self.addEventListener('message', (event) => {
 
   if (event.data.source === '@folio/stripes-core') {
     if (shouldLog) console.info('-- (rtr-sw) reading', event.data);
-
-    // OKAPI_CONFIG
     if (event.data.type === 'OKAPI_CONFIG') {
       okapiUrl = event.data.value.url;
       okapiTenant = event.data.value.tenant;
     }
 
-    // LOGGER_CONFIG
     if (event.data.type === 'LOGGER_CONFIG') {
       shouldLog = !!event.data.value.categories?.split(',').some(cat => cat === 'rtr-sw');
     }
 
-    // TOKEN_EXPIRATION
     if (event.data.type === 'TOKEN_EXPIRATION') {
-      tokenExpiration = handleTokenExpiration(event.data.value);
+      tokenExpiration = event.data.tokenExpiration;
     }
   }
 });

diff --git a/src/service-worker.test.js b/src/service-worker.test.js
@@ -1,5 +1,4 @@
 import {
-  handleTokenExpiration,
   isLogoutRequest,
   isOkapiRequest,
   isPermissibleRequest,
@@ -27,8 +26,12 @@ afterAll(() => {
 });
 
 describe('isValidAT', () => {
-  it('returns true for valid ATs', () => {
-    expect(isValidAT({ atExpires: Date.now() + 1000 })).toBe(true);
+  it('returns true for ATs with 95% or more of their TTL remaining', () => {
+    expect(isValidAT({ atExpires: (Date.now() / TTL_WINDOW) + 10000 })).toBe(true);
+  });
+
+  it('returns false for ATs 5% or less of their TTL remaining', () => {
+    expect(isValidAT({ atExpires: Date.now() + 1000 })).toBe(false);
   });
 
   it('returns false for expired ATs', () => {
@@ -42,7 +45,11 @@ describe('isValidAT', () => {
 
 describe('isValidRT', () => {
   it('returns true for valid RTs', () => {
-    expect(isValidRT({ rtExpires: Date.now() + 1000 })).toBe(true);
+    expect(isValidRT({ rtExpires: (Date.now() / TTL_WINDOW) + 1000 })).toBe(true);
+  });
+
+  it('returns false for RTs 5% or less of their TTL remaining', () => {
+    expect(isValidRT({ rtExpires: Date.now() + 1000 })).toBe(false);
   });
 
   it('returns false for expired RTs', () => {
@@ -508,25 +515,3 @@ describe('rtr', () => {
   });
 });
 
-describe('handleTokenExpiration', () => {
-  const testWindow = (token) => {
-    const now = Date.now();
-    const window = 1000;
-    const data = {
-      tokenExpiration: {
-        [token]: now + window,
-      },
-    };
-
-    const result = handleTokenExpiration(data);
-    expect(parseFloat(result[token] - now).toPrecision(2)).toEqual(parseFloat(TTL_WINDOW * window).toPrecision(2));
-  };
-
-  it(`shrinks AT's validity window to ${parseFloat(TTL_WINDOW * 100).toPrecision(2)}% of original size`, () => {
-    testWindow('atExpires');
-  });
-
-  it(`shrinks RT's validity window to ${parseFloat(TTL_WINDOW * 100).toPrecision(2)}% of original size`, () => {
-    testWindow('rtExpires');
-  });
-});
diff --git a/test/bigtest/tests/login-test.js b/test/bigtest/tests/login-test.js
@@ -340,15 +340,7 @@ describe('Login', () => {
     });
   });
 
-  // the login workflow invokes navigator.serviceWorker.ready,
-  // a browser property that returns a Promise that waits until
-  // the service worker resolves, but in Karma-land we don't
-  // configure the service-worker. hence, this will time out,
-  // every time.
-  //
-  // we'll need to cover these components with jest/RTL tests
-  // eventually.
-  describe.skip('with valid credentials', () => {
+  describe('with valid credentials', () => {
     beforeEach(async () => {
       const { username, password, submit } = login;