Skip to content

Commit

Permalink
Revert "STCOR-574 rotate tokens after 80% of their TTL (#1361)"
Browse files Browse the repository at this point in the history
This reverts commit dd71819.
  • Loading branch information
zburke committed Nov 10, 2023
1 parent 027bd74 commit 8cae49d
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 47 deletions.
1 change: 0 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
* Avoid private path when import `validateUser` function. Refs STCOR-749.
* 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.
* 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)
Expand Down
29 changes: 2 additions & 27 deletions src/service-worker.js
Original file line number Diff line number Diff line change
Expand Up @@ -62,31 +62,6 @@ const IS_ROTATING_RETRIES = 100;
/** how long to wait before rechecking the lock, in milliseconds (100 * 100) === 10 seconds */
const IS_ROTATING_INTERVAL = 100;

/**
* TTL_WINDOW
* How much of a token's TTL can elapse before it is considered expired?
* This helps us avoid a race-like condition where a token expires in the
* gap between when we check whether we think it's expired and when we use
* it to authorize a new request. Say the last RTR response took a long time
* to arrive, so it was generated at 12:34:56 but we didn't process it until
* 12:34:59. That could cause problems if (just totally hypothetically) we
* had an application (again, TOTALLY hypothetically) that was polling every
* five seconds and one of its requests landed in that three-second gap. Oh,
* hey STCOR-754, what are you doing here?
*
* So this is a buffer. Instead of letting a token be used up until the very
* last second of its life, we'll consider it expired a little early. This will
* cause RTR to happen a little early (i.e. a little more frequently) but that
* should be OK since it increases our confidence that when an AT accompanies
* the RTR request it is still valid.
*
* Value is a float, 0 to 1, inclusive. Closer to 0 means more frequent
* rotation; 1 means a token is valid up the very last moment of its TTL.
* 0.8 is just a SWAG at a "likely to be useful" value. Given a 600 second
* TTL (the current default for ATs) it corresponds to 480 seconds.
*/
export const TTL_WINDOW = 0.8;

/**
* isValidAT
* return true if tokenExpiration.atExpires is in the future
Expand All @@ -95,7 +70,7 @@ export const TTL_WINDOW = 0.8;
*/
export const isValidAT = (te) => {
if (shouldLog) console.log(`-- (rtr-sw) => at expires ${new Date(te?.atExpires || null).toISOString()}`);
return !!(te?.atExpires * TTL_WINDOW > Date.now());
return !!(te?.atExpires > Date.now());
};

/**
Expand All @@ -106,7 +81,7 @@ export const isValidAT = (te) => {
*/
export const isValidRT = (te) => {
if (shouldLog) console.log(`-- (rtr-sw) => rt expires ${new Date(te?.rtExpires || null).toISOString()}`);
return !!(te?.rtExpires * TTL_WINDOW > Date.now());
return !!(te?.rtExpires > Date.now());
};

/**
Expand Down
29 changes: 10 additions & 19 deletions src/service-worker.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import {
passThrough,
passThroughLogout,
rtr,
TTL_WINDOW,
} from './service-worker';

// reassign console.log to keep things quiet
Expand All @@ -26,12 +25,8 @@ afterAll(() => {
});

describe('isValidAT', () => {
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 true for valid ATs', () => {
expect(isValidAT({ atExpires: Date.now() + 1000 })).toBe(true);
});

it('returns false for expired ATs', () => {
Expand All @@ -44,12 +39,8 @@ describe('isValidAT', () => {
});

describe('isValidRT', () => {
it('returns true for valid RTs', () => {
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 true for valid ATs', () => {
expect(isValidRT({ rtExpires: Date.now() + 1000 })).toBe(true);
});

it('returns false for expired RTs', () => {
Expand Down Expand Up @@ -127,7 +118,7 @@ describe('isPermissibleRequest', () => {
describe('when AT is valid', () => {
it('when AT is valid, accepts any endpoint', () => {
const req = { url: 'monkey' };
const te = { atExpires: (Date.now() / TTL_WINDOW) + 1000, rtExpires: (Date.now() / TTL_WINDOW) + 1000 };
const te = { atExpires: Date.now() + 1000, rtExpires: Date.now() + 1000 };
expect(isPermissibleRequest(req, te, '')).toBe(true);
});
});
Expand Down Expand Up @@ -304,7 +295,7 @@ describe('passThrough', () => {
clone: () => req,
}
};
const tokenExpiration = { atExpires: (Date.now() / TTL_WINDOW) + 10000 };
const tokenExpiration = { atExpires: Date.now() + 10000 };

const response = { ok: true };
global.fetch = jest.fn(() => Promise.resolve(response));
Expand All @@ -322,7 +313,7 @@ describe('passThrough', () => {
clone: () => req,
}
};
const tokenExpiration = { atExpires: (Date.now() / TTL_WINDOW) + 10000 };
const tokenExpiration = { atExpires: Date.now() + 10000 };

const response = {
ok: false,
Expand All @@ -347,8 +338,8 @@ describe('passThrough', () => {
}
};
const tokenExpiration = {
atExpires: (Date.now() / TTL_WINDOW) + 1000, // at says it's valid, but ok == false
rtExpires: (Date.now() / TTL_WINDOW) + 1000
atExpires: Date.now() + 1000, // at says it's valid, but ok == false
rtExpires: Date.now() + 1000
};

const response = 'los alamos';
Expand Down Expand Up @@ -382,7 +373,7 @@ describe('passThrough', () => {
};
const tokenExpiration = {
atExpires: Date.now() - 1000,
rtExpires: (Date.now() / TTL_WINDOW) + 1000
rtExpires: Date.now() + 1000
};

const response = 'los alamos';
Expand Down

0 comments on commit 8cae49d

Please sign in to comment.