STCOR-671 handle access-control via cookies and RTR 👋 🔄 🔒 😅 #1376
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
STCOR-748, FOLREL-584
Handle access-control via HTTP-only cookies instead of storing the JWT in local storage and providing it in the `X-Okapi-Token` header of fetch requests, and proxy all requests through a service worker that performs Refresh Token Rotation as needed to make sure the access-token remains fresh. Notable changes: * Fetch requests are proxied by a Service Worker that intercepts them, validates that the Access Token is still valid, and performs token rotation (if it is not) before completing the original fetch. * Sessions automatically end (i.e. the user is automatically logged out) when the Refresh Token expires. * Access control is managed by including an HTTP-only cookie with all requests. This means the Access Token formerly available in the response-header as `X-Okapi-Token` is never accessible to JS code. * Requires folio-org/stripes-connect/pull/223 * Requires folio-org/stripes-smart-components/pull/1397 * Requires folio-org/stripes-webpack/pull/125 *Note that this work DOES NOT address [STCOR-758](https://issues.folio.org/browse/STCOR-758) (pages loaded with shift-reload have their ATs timeout after 10 minutes, causing one or more error alerts followed by auto-logout) or [STCOR-759](https://issues.folio.org/browse/STCOR-759) (AT may timeout >= 10 minutes causing one or more error alerts followed by auto-logout).* * (cherry picked from commit 27d2948) * (cherry picked from commit dd71819) * (cherry picked from commit 607dc5c) Refs [STCOR-671](https://issues.folio.org/browse/STCOR-671), [STCOR-574](https://issues.folio.org/browse/STCOR-754), [STCOR-756](https://issues.folio.org/browse/STCOR-756), [FOLIO-3627](https://issues.folio.org/browse/FOLIO-3627)
) * [STCOR-752]: Ensure <AppIcon> is not cut off * Only apply min-width to appIcon classes * Add changelog (cherry picked from commit 2a0a328)
`@folio/stripes-webpack` still looks for `service-worker.js`, so we provide a dummy function here to keep the API intact. This API is never invoked; this service worker is never registered (though it wouldn't matter since it doesn't listen for any events). Refs STCOR-671
Refs STCOR-671
…th utility, rtr-specific error.
* comments are in sync with function arguments * consistently handle non-string arguments passed to fetch
…s-core into STCOR-671-nkotb
This is mostly tests and comments. The lone exception is in `Fetch::ffetch()` which was significantly refactored to handle RTR exceptions at the innermost level where they occur, instead of wrapping the whole block in a try/catch and handling them at the outer level, which was not reliable: exceptions thrown from `rtr()` were escaping. It seems like some aspect of the stack was synchronous, allowing that exception to escape instead of being translated into a rejection.
…s-core into STCOR-671-nkotb
zburke
changed the title
BigTest Unit Test Statistics 1 files ±0 1 suites ±0 10s ⏱️ ±0s Results for commit c8f9d8c. ± Comparison against base commit 6872a92. This pull request removes 5 and adds 3 tests. Note that renamed tests count towards both.This pull request skips 1 test.♻️ This comment has been updated with latest results. |
|
SonarCloud Quality Gate failed.
|
The `storage` event is specifically designed for cross-window communication, in fact, _exclusively_ designed for cross-window communication. Thus, in order to handle both scenarios (rotation is pending in the current window or in a separate window), we need to set up listeners for both types of events.
Instead of a boolean, store a timestamp for the value of the pending-rtr-request flag. This allows us to detect stale flags (i.e. a flag indicating that a rotation request is active when in fact none is) and clear them. Without this check, the `isRotating` value could get stuck in the cache, causing all future rotation requests to wait for a non-existent request to complete, which of course wouldn't end well.
Manually revert the merge commit inadvertently applied to this branch while playing with #1387. I (stupidly) didn't realize that resolving the conflicts there would add commits on the source branch; I was only thinking about the destination branch. Bad developer! No biscuit!
Provide the token, if available, in `_self` requests when switching the active tenant in an ECS environment.
In Poppy, a missing token is reported in a 403. In Quesnelia, it is reported in a 400 (MODAT-160).
|
zburke
added a commit
that referenced
this pull request
Jan 26, 2024
Move auth tokens into HTTP-only cookies and implement refresh token rotation (STCOR-671) by overriding `global.fetch` and `global.XMLHttpRequest`, disabling login when cookies are disabled (STCOR-762). This functionality is implemented behind an opt-in feature-flag (STCOR-763). The core RTR logic here is largely the same as it was in PR #1346 😬 , though with several important differences: 1. No buggy service-worker 2. Handle `fetch` and `XMLHttpRequest` 3. Disable login if cookies are disabled 4. Everything is opt-in 😌 Not _everything_ in PR #1346 was awful, despite it being reverted in \#1371 😬 . The fundamental difference here is that the global `fetch` and `XMLHttpRequest` functions have been replaced 🤢 by new implementations that handle RTR instead of intercepting such requests via the service-worker proxy. This is not lovely. It is not elegant. It isn't pretty in any way, but it is extremely simple and effective. Certainly, we want to migrate away from it, but given the options we thought it was best choice in the short-term. The options: 1. Centralized fix within stripes-core by fixing the service worker. Let's be honest, I didn't get it right in #1346 and then couldn't get it right in #1361 or #1363 or #1366 or #1369. Why would anybody possibly believe that I could get it right now? 2. Decentralized fix: handle this in each UI-* repository by exporting a new function from stripes and refactoring each UI repo to leverage the new code. Probably not a big refactor, but not a small effort. 3. Centralized fix within stripes-core by overwriting `global.fetch`. Gross, but effective, and long term we can make this a decentralized approach by exporting our new `fetch` function, doing the refactor described in 2 (above), and removing the global-overwrite once all the refactoring is done. In summary: * Replaces #1340. It was gross and I really don't want to talk about it. Let us never mention it again. * Replaces #1346. It was a terrible, horrible, no good, very bad PR. Alexander hated that PR more than lima beans. Additional requirements: * Requires folio-org/stripes-connect#223 * Requires folio-org/stripes-smart-components#1397 * Requires folio-org/stripes-webpack#125 Refs STCOR-671, FOLIO-3627 (cherry picked from commit 0361353)
zburke
added a commit
that referenced
this pull request
Jan 31, 2024
Move auth tokens into HTTP-only cookies and implement refresh token rotation (STCOR-671) by overriding global.fetch and global.XMLHttpRequest, disabling login when cookies are disabled (STCOR-762). This functionality is implemented behind an opt-in feature-flag (STCOR-763). Okapi and Keycloak do not handle the same situations in the same ways. Changes from the original implementation in PR #1376: * When a token is missing: * Okapi sends a 400 `text/plain` response * Keycloak sends a 401 `application/json` response * Keycloak authentication includes the extra step of exchanging the OTP for the AT/RT and that request needs the `credentials` and `mode` options * Some `loginServices` functions now retrieve the host the access from the `stripes-config` import instead of a function argument * always permit `/authn/token` requests to go through Refs STCOR-796, STCOR-671 (cherry picked from commit 0361353)
zburke
added a commit
that referenced
this pull request
Mar 28, 2024
Move auth tokens into HTTP-only cookies and implement refresh token rotation (STCOR-671) by overriding global.fetch and global.XMLHttpRequest, disabling login when cookies are disabled (STCOR-762). This functionality is implemented behind an opt-in feature-flag (STCOR-763). Okapi and Keycloak do not handle the same situations in the same ways. Changes from the original implementation in PR #1376: * When a token is missing: * Okapi sends a 400 `text/plain` response * Keycloak sends a 401 `application/json` response * Keycloak authentication includes the extra step of exchanging the OTP for the AT/RT and that request needs the `credentials` and `mode` options * Some `loginServices` functions now retrieve the host the access from the `stripes-config` import instead of a function argument * always permit `/authn/token` requests to go through Refs STCOR-796, STCOR-671 (cherry picked from commit 0361353)
zburke
added a commit
that referenced
this pull request
Mar 28, 2024
Move auth tokens into HTTP-only cookies and implement refresh token rotation (STCOR-671) by overriding global.fetch and global.XMLHttpRequest, disabling login when cookies are disabled (STCOR-762). This functionality is implemented behind an opt-in feature-flag (STCOR-763). Okapi and Keycloak do not handle the same situations in the same ways. Changes from the original implementation in PR #1376: * When a token is missing: * Okapi sends a 400 `text/plain` response * Keycloak sends a 401 `application/json` response * Keycloak authentication includes the extra step of exchanging the OTP for the AT/RT and that request needs the `credentials` and `mode` options * Some `loginServices` functions now retrieve the host the access from the `stripes-config` import instead of a function argument * always permit `/authn/token` requests to go through Refs STCOR-796, STCOR-671 (cherry picked from commit 0361353)
aidynoJ
pushed a commit
that referenced
this pull request
May 6, 2024
Move auth tokens into HTTP-only cookies and implement refresh token rotation (STCOR-671) by overriding global.fetch and global.XMLHttpRequest, disabling login when cookies are disabled (STCOR-762). This functionality is implemented behind an opt-in feature-flag (STCOR-763). Okapi and Keycloak do not handle the same situations in the same ways. Changes from the original implementation in PR #1376: * When a token is missing: * Okapi sends a 400 `text/plain` response * Keycloak sends a 401 `application/json` response * Keycloak authentication includes the extra step of exchanging the OTP for the AT/RT and that request needs the `credentials` and `mode` options * Some `loginServices` functions now retrieve the host the access from the `stripes-config` import instead of a function argument * always permit `/authn/token` requests to go through Refs STCOR-796, STCOR-671 (cherry picked from commit 0361353)
ryandberger
pushed a commit
that referenced
this pull request
May 15, 2024
Move auth tokens into HTTP-only cookies and implement refresh token rotation (STCOR-671) by overriding global.fetch and global.XMLHttpRequest, disabling login when cookies are disabled (STCOR-762). This functionality is implemented behind an opt-in feature-flag (STCOR-763). Okapi and Keycloak do not handle the same situations in the same ways. Changes from the original implementation in PR #1376: * When a token is missing: * Okapi sends a 400 `text/plain` response * Keycloak sends a 401 `application/json` response * Keycloak authentication includes the extra step of exchanging the OTP for the AT/RT and that request needs the `credentials` and `mode` options * Some `loginServices` functions now retrieve the host the access from the `stripes-config` import instead of a function argument * always permit `/authn/token` requests to go through Refs STCOR-796, STCOR-671 (cherry picked from commit 0361353)
zburke
added a commit
that referenced
this pull request
Jun 11, 2024
Move auth tokens into HTTP-only cookies and implement refresh token rotation (STCOR-671) by overriding global.fetch and global.XMLHttpRequest, disabling login when cookies are disabled (STCOR-762). This functionality is implemented behind an opt-in feature-flag (STCOR-763). Okapi and Keycloak do not handle the same situations in the same ways. Changes from the original implementation in PR #1376: * When a token is missing: * Okapi sends a 400 `text/plain` response * Keycloak sends a 401 `application/json` response * Keycloak authentication includes the extra step of exchanging the OTP for the AT/RT and that request needs the `credentials` and `mode` options * Some `loginServices` functions now retrieve the host the access from the `stripes-config` import instead of a function argument * always permit `/authn/token` requests to go through Refs STCOR-796, STCOR-671 (cherry picked from commit 0361353)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Move auth tokens into HTTP-only cookies and implement refresh token rotation (STCOR-671) by overriding
global.fetchandglobal.XMLHttpRequest, disabling login when cookies are disabled (STCOR-762). This functionality is implemented behind an opt-in feature-flag (STCOR-763). The core RTR logic here is largely the same as it was in PR #1346 😬 , though with several important differences:fetchandXMLHttpRequestNot everything in PR #1346 was awful, despite it being reverted in #1371 😬 . The fundamental difference here is that the global
fetchandXMLHttpRequestfunctions have been replaced 🤢 by new implementations that handle RTR instead of intercepting such requests via the service-worker proxy. This is not lovely. It is not elegant. It isn't pretty in any way, but it is extremely simple and effective. Certainly, we want to migrate away from it, but given the options we thought it was best choice in the short-term. The options:global.fetch. Gross, but effective, and long term we can make this a decentralized approach by exporting our newfetchfunction, doing the refactor described in 2 (above), and removing the global-overwrite once all the refactoring is done.In summary:
Additional requirements:
Refs STCOR-671, FOLIO-3627