STCOR-1012 💉 botox for the botched RTR implementation

[zburke]

RTR, boy oh boy, where to start?

• STCOR-671 handle access-control via cookies #1340? Nah, that never even got off the ground.
• STCOR-671 handle access-control via cookies #1346? Oh hell no! 🧄 ✝️ Do you not remember that hellfire? Mothers cried. Children screamed. One of my chickens actually died. 🪦 We killed off this implementation in Revert RTR #1371.
• That brings us to STCOR-671 handle access-control via cookies and RTR 👋 🔄 🔒 😅  #1376 (but also STCOR-811 parse /authn/token response for AT/RT expirations #1417, and STCOR-815 Add SSO Login path to permissible resources in RTR code. #1423, and STCOR-895 wait a loooong time for a "stale" rotation request #1547, and don't forgot STCOR-930 correctly dispatch rtrFlsWarningTimeout #1579, STCOR-978 Use BroadcastChannel to isolate token to last-focused window. #1642, STCOR-983: Remove RTR_IS_ROTATING from local storage before dispatching the RTR_SUCCESS_EVENT to correctly determine the RTR_IS_ROTATING state in the rotationHandler function. #1643, STCOR-978 Use BroadcastChannel to isolate token to last-focused window. #1647, and STCOR-1029: Remove the /users-keycloak/_self (and /bl-users/_self) requests. They were included in isAuthenticationRequest, which caused them to bypass the RTR queue (getPromise). #1688).

The implementations thus far all worked by setting a timer that pinged shortly before the AT expired, executing a callback that paused API requests while executing the rotation request. Of course, it was never that simple; starting rotation after authenticating was subtly different than starting rotation for an existing session because the login-response includes token-expiration data but the /bl-users/_self doesn't. All those extra tweaks made the code even messier, and while they each made the UX a little better, none of them made it actually make it good. And then STCOR-1012 showed up, telling us things were not only not good; they were, in fact, pretty bad. It was a mess. It was just a mess. 💩

And that brings us here, to the fourth full-blown rewrite of RTR. Hopefully, the logic here is simpler, better isolated, and easier to reason about. At the highest level, things are still the same: we're replacing native-fetch with FFetch, which transparently handles rotation. The FFetch algorithm now works like this:

• get a shared lock, preventing API requests from firing while rotation is in progress
  • make the API request
  • release the lock
• if the response isn't ok, call rotateAndReplay() to rotate the tokens and replay the original request
• return the response

rotateAndReplay works like this:

• inspect the original response. if it doesn't look like an authentication failure, reject with the original response, allowing it to bubble back to the code that made the original request
• get an exclusive lock, preventing any API requests from firing while rotation is in progress
  • request token-expiration data from storage; if it appears valid, replay the original request and return its response
  • make the rotation request
  • callback to store the updated token-expiration data
  • replay the original request
  • release the lock

Checking, while inside the lock, whether the tokens have already been rotated is a crucial step that handles the situation of many requests firing simultaneously after the tokens have expired. The Web Locks API elects one request, allowing rotation, storage, and replay to proceed for that request only while all others wait for the lock's promise to resolve. After that first request releases its lock, the next request in the queue proceeds through rotateAndReplay(), but this time it will return early since the token-expiration data in storage will be valid. Note: this means requests that are queued together proceed single-threaded through the lock. This sounds worse than it is since rotateAndReplay() short-circuits after the first request, jumping immediately to "replay" since valid tokens are now available.

But wait! There's more! The original implementation stored the end-of-session timer-IDs in redux. It's important to keep track of those IDs so they can be cleared if the user logs out early or session expiration is configured as a window that slides on rotation, as non-keycloak sessions are. Anybody remember UICHKOUT-869? Using redux was problematic for two reasons:

1. Mind-bogglingly, clearing the old timer was buried inside the redux action, which, just, why??? 🤯 Dude, have you never heard of single-responsibility? Redux actions should be responsible for redux and only redux.
2. Bigly-mind-bogglingly, keeping timer data on the okapi slice of redux meant that Root re-rendered every time tokens rotated, which was a real head-scratcher (STCOR-1008 et al), until @vashjs made things plain to us in UITEN-333 🙇. Like I said, it was a mess. 💩 💩

The self-contained, self-clearing timer ResetTimer (maybe I should've called it SelfClearingTimer? 🤔) resolves both: redux actions are once again pure, and Root no longer re-renders with each rotation.

Refs STCOR-1012, UITEN-333, STCOR-1025

[github-actions]

Jest Unit Test Results

  1 files  ± 0   85 suites  ±0   1m 5s ⏱️ -38s
503 tests  - 22  503 ✅  - 22  0 💤 ±0  0 ❌ ±0 
509 runs   - 22  509 ✅  - 22  0 💤 ±0  0 ❌ ±0 

Results for commit a4f1623. ± Comparison against base commit 5ab64db.

This pull request removes 55 and adds 33 tests. Note that renamed tests count towards both.

FFetch class Calling a FOLIO API fetch calls native fetch once ‑ FFetch class Calling a FOLIO API fetch calls native fetch once
FFetch class Calling a non-FOLIO API calls native fetch once ‑ FFetch class Calling a non-FOLIO API calls native fetch once
FFetch class Calling an okapi fetch when all tokens are expired triggers an RTR error ‑ FFetch class Calling an okapi fetch when all tokens are expired triggers an RTR error
FFetch class Calling an okapi fetch with a malformed resource triggers an Unexpected Resource Error ‑ FFetch class Calling an okapi fetch with a malformed resource triggers an Unexpected Resource Error
FFetch class Calling an okapi fetch with missing token and failing rotation... triggers rtr...calls fetch 2 times, failed call, failed token call, throws error ‑ FFetch class Calling an okapi fetch with missing token and failing rotation... triggers rtr...calls fetch 2 times, failed call, failed token call, throws error
FFetch class Calling an okapi fetch with missing token and reported error from auth service... throws an RTR error ‑ FFetch class Calling an okapi fetch with missing token and reported error from auth service... throws an RTR error
FFetch class Calling an okapi fetch with missing token... returns the error ‑ FFetch class Calling an okapi fetch with missing token... returns the error
FFetch class Calling an okapi fetch with valid token, but failing request... returns response from failed fetch, only calls fetch once. ‑ FFetch class Calling an okapi fetch with valid token, but failing request... returns response from failed fetch, only calls fetch once.
FFetch class calling authentication resources avoids rotation when AT and RT expire together ‑ FFetch class calling authentication resources avoids rotation when AT and RT expire together
FFetch class calling authentication resources handles RTR data in the response ‑ FFetch class calling authentication resources handles RTR data in the response
…

FFetch behavior and rotation helpers passes through non-Okapi requests ‑ FFetch behavior and rotation helpers passes through non-Okapi requests
FFetch behavior and rotation helpers rotationConfig helpers isValidToken returns true when token expiry is in future and false otherwise ‑ FFetch behavior and rotation helpers rotationConfig helpers isValidToken returns true when token expiry is in future and false otherwise
FFetch behavior and rotation helpers rotationConfig helpers replaceXMLHttpRequest sets global.XMLHttpRequest and preserves the original ‑ FFetch behavior and rotation helpers rotationConfig helpers replaceXMLHttpRequest sets global.XMLHttpRequest and preserves the original
FFetch behavior and rotation helpers rotationConfig helpers rotate() performs a refresh and returns parsed expirations on success ‑ FFetch behavior and rotation helpers rotationConfig helpers rotate() performs a refresh and returns parsed expirations on success
FFetch behavior and rotation helpers rotationConfig helpers rotate() throws when refresh response is missing fields ‑ FFetch behavior and rotation helpers rotationConfig helpers rotate() throws when refresh response is missing fields
FFetch behavior and rotation helpers rotationConfig helpers rotate() throws when refresh response is not ok ‑ FFetch behavior and rotation helpers rotationConfig helpers rotate() throws when refresh response is not ok
FFetch behavior and rotation helpers rotationConfig helpers rotationConfig.onFailure dispatches an RTR error event ‑ FFetch behavior and rotation helpers rotationConfig helpers rotationConfig.onFailure dispatches an RTR error event
FFetch behavior and rotation helpers rotationConfig helpers rotationConfig.onSuccess calls the provided onRotate callback with new tokens ‑ FFetch behavior and rotation helpers rotationConfig helpers rotationConfig.onSuccess calls the provided onRotate callback with new tokens
FFetch behavior and rotation helpers rotationConfig helpers rotationConfig.options merges OKAPI_FETCH_OPTIONS into options ‑ FFetch behavior and rotation helpers rotationConfig helpers rotationConfig.options merges OKAPI_FETCH_OPTIONS into options
FFetch behavior and rotation helpers rotationConfig helpers shouldRotate given a 400 with generic text, returns false ‑ FFetch behavior and rotation helpers rotationConfig helpers shouldRotate given a 400 with generic text, returns false
…

♻️ This comment has been updated with latest results.

[github-actions]

Bigtest Unit Test Results

  1 files  ±0    1 suites  ±0   0s ⏱️ ±0s
  2 tests ±0    2 ✅ ±0  0 💤 ±0  0 ❌ ±0 
152 runs  ±0  152 ✅ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit a4f1623. ± Comparison against base commit 5ab64db.

♻️ This comment has been updated with latest results.

[JohnC-80]

Tiny cleanup is all I've got so far... this really cleaned a lot of house. 👏 👏 👏

[maccabeelevine]

// Ruhroh, Raggy, rotation railed!

😆

[sonarqubecloud]

Quality Gate passed

Issues
2 New issues
0 Accepted issues

Measures
0 Security Hotspots
89.1% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud