Skip to content

Commit

Permalink
STCOR-574 rotate tokens after 80% of their TTL (#1361)
Browse files Browse the repository at this point in the history
Rotate tokens well before they expire. This solves a problem in
ui-data-import where every-five-second polling has caused some requests to
land in a gap of about three seconds between when the AT was actually
minted and when we stored it on the client side, which could cause the
UI to send an AT that mod-auth thinks is expired even though we thought
it was still valid.

i.e. it makes it much less likely that a token will expire in flight.

Refs STCOR-574
  • Loading branch information
zburke authored Oct 31, 2023
1 parent 27d2948 commit dd71819
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 12 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
* Provide optional tenant argument to `useOkapiKy` hook. Refs STCOR-747.
* 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.

## [10.0.0](https://github.com/folio-org/stripes-core/tree/v10.0.0) (2023-10-11)
[Full Changelog](https://github.com/folio-org/stripes-core/compare/v9.0.0...v10.0.0)
Expand Down
29 changes: 27 additions & 2 deletions src/service-worker.js
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,31 @@ 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 @@ -70,7 +95,7 @@ const IS_ROTATING_INTERVAL = 100;
*/
export const isValidAT = (te) => {
if (shouldLog) console.log(`-- (rtr-sw) => at expires ${new Date(te?.atExpires || null).toISOString()}`);
return !!(te?.atExpires > Date.now());
return !!(te?.atExpires * TTL_WINDOW > Date.now());
};

/**
Expand All @@ -81,7 +106,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 > Date.now());
return !!(te?.rtExpires * TTL_WINDOW > Date.now());
};

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

// reassign console.log to keep things quiet
Expand All @@ -25,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', () => {
Expand All @@ -39,8 +44,12 @@ describe('isValidAT', () => {
});

describe('isValidRT', () => {
it('returns true for valid ATs', () => {
expect(isValidRT({ rtExpires: Date.now() + 1000 })).toBe(true);
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 false for expired RTs', () => {
Expand Down Expand Up @@ -118,7 +127,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() + 1000, rtExpires: Date.now() + 1000 };
const te = { atExpires: (Date.now() / TTL_WINDOW) + 1000, rtExpires: (Date.now() / TTL_WINDOW) + 1000 };
expect(isPermissibleRequest(req, te, '')).toBe(true);
});
});
Expand Down Expand Up @@ -295,7 +304,7 @@ describe('passThrough', () => {
clone: () => req,
}
};
const tokenExpiration = { atExpires: Date.now() + 10000 };
const tokenExpiration = { atExpires: (Date.now() / TTL_WINDOW) + 10000 };

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

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

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

const response = 'los alamos';
Expand Down

0 comments on commit dd71819

Please sign in to comment.