-
Notifications
You must be signed in to change notification settings - Fork 26
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
STCOR-827: get gateway url from tenantOptions config field, change value of okapi.url in redux store #1454
Draft
aidynoJ
wants to merge
35
commits into
keycloak-ramsons-20240611
Choose a base branch
from
STCOR-827
base: keycloak-ramsons-20240611
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
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
There's a lot going on here, but fundamentally the changes are split into two main categories: * route authentication requests to/from keycloak * handle discovery dynamically via an API request AFTER authentication instead of reading a static module list from `stripes.config.js`
* Update URL redirect to allow back button support * Clean up unused and duplicate code * lint * Add URL param to indicate Consortium --------- Co-authored-by: Zak Burke <zburke@ebsco.com>
* handle legacy discovery via Okapi APIs * handle legacy logout via internal redirect to `/` * handle legacy version display on `/settings/about` There is not really as much work here as it appears. All the new components were split out of `About` in order to allow sub-sections to be reused with both application-based and module-based discovery information. Likewise, `loginServices.js` and `discoveryServices.js` were modestly refactored to handled both APIs. And there are Jest/RTL tests to replace the BTOG test that could not be easily updated to handle the new APIs since its `stripes-config` stub is part of `@folio/stripes-cli` instead of being declared locally. Refs STCOR-773
…1399) When the `users-keycloak` interface is available, use the endpoints it provides in place of the legacy endpoints. Refs STCOR-795, UIU-3031
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)
* STCOR-803 Add config option for logout mode * Lint fix
…component. (#1411) (#1422) * move async localforage.clear to afterEach * remove BTOG sso login tests, add sso login jest tests * Update CHANGELOG.md * move describe block comments to it blocks.. remove describe blocks (cherry picked from commit 79c76c4) Co-authored-by: John Coburn <jcoburn@ebsco.com>
If the response from `/auth/token?code=...` is OK, parse it to immediately store the AT/RT expiration values (or use a near-future date if values are not provided). Stripes must expect the RT to be valid for any future API call to succeed; otherwise, it will assume the RT has expired resulting in a race condition with the RTR handler dispatching an RTR_ERROR_EVENT but discovery succeeding and re-rendering. This would result in the API call to `.../_self` issued by `requestUserWithPerms` being swallowed and `stripes.user` being populated with an empty object, causing all kinds of problems down the line for any code that leveraged it. Refs STCOR-811
) Include the `X-Okapi-Tenant` header in `/authn/logout` requests, and clear `localStorage` settings as well. `X-Okapi-Tenant` is required for requests to be properly routed; if this request failed, the browser session would be destroyed by the keycloak session would remain active, a security risk. Clearing `localStorage.tenant` is necessary to prevent an incorrect value from being cached and inadvertently reused on subsequent login requests. Refs STCOR-812
The shape of the permissions object differs between responses from calls to `login` and calls to `_self`. This is not awesome. We didn't notice this glitch prior to implementing keycloak because when resuming an existing session (i.e. when calling `_self`), permissions are set as the union of permissions in storage (i.e. stored by a call to `login`) and those from the call to `_self`. We just never noticed that the latter was always empty. With keycloak handling authentication, however, the _only_ permissions we ever receive are in the response from `_self`, so we noticed this immediately. Refs STCOR-813
Remove references to the `stripes.config.js::config` values `tenantManagerUrl` and `applicationManagerUrl`. These were present in early drafts of this work but have since been deprecated and therefore must be removed from code as well. Refs STCOR-810
…page (#1426) * STCOR-803 Add config option for logout mode * Lint fix * Revert "STCOR-803 Add config option for logout mode" This reverts commit b9d2604. * STCOR-803 Simplify logout workflow to bypass keycloak confirmation page. * STCOR-803 PR comments * Revert "STCOR-803 PR comments" This reverts commit 037b6a2. * STCOR-803 Restore console log
Remove references to the `stripes.config.js::config` values `tenantManagerUrl` and `applicationManagerUrl`. These were present in early drafts of the new discovery work but have since been deprecated and therefore must be removed from code as well. Replaces #1418, which did this work incorrectly (referring to `okapi.[...]` instead of `config.[...]`). Refs STCOR-810
* STCOR-816 only fetch /saml/check when login-saml is present When restoring an existing session, after discovery, do not fetch from `/saml/check` unless the `login-saml` interface (indicating SSO/SAML is available). The 404 clutters the log. Refs STCOR-816 * Missing semicolon --------- Co-authored-by: Ryan Berger <rberger@ebsco.com>
* STCOR-776 show "Keep working?" prompt when session ages The main feature here is to track the RT's TTL and use it to show a "Your session is about to expire; keep working?" prompt so the user can fire off RTR in order to keep the session alive. Knock-on effects include tracking such events across multiple windows, so logging out in one window immediately logs you out in others, and so a successful RTR event in one window closes any open "Still working?" prompts in others. It sounds big, and it looks big, but once you wrap your head around it isn't so bad. A couple things to note: The `loginServices::eventManager()` function provides two event-related function, `listen` and `emit` that handle single-window events (i.e. `window.dispatchEvent()`/`window.addEventListener()`) and multi-window events (i.e. BroadcastChannel.post() and BroadcastChannel.addEventListener()`). This simplifies the API for sending and receiving events. [Documentation for BroadcastChannel](https://developer.mozilla.org/en-US/docs/Web/API/Broadcast_Channel_API) is pretty good and worth a look. The thing to keep in mind with single-window events is that they are sent and received in the same window, whereas BroadcastChannel events are sent in one window but received in all others. `<SessionEventContainer>` is instantiated near the top of `<RootWithIntl>`, just like `<OverlayContainer>`. It sets itself up as the listener for all session-related events, including RTR-success (which will close an open "Keep working?" prompt), RTR-failure (which will cause logout), logout (i.e. a logout event from another window), and idle session (i.e. the RT is about to expire, which will show the "Keep working?" modal). Other session-related event handlers have been removed in favor of consolidating them all in this component. The `<KeepWorkingModal>` calls RTR if the user closes it, causing the session to be extended. By default it displays 60 seconds before the session expires but this can be changed by adjusting `stripes.config.js::config.idleSessionWarningSeconds`. If the timer counts down to 0, it emits a session-expired event, causing logout. Refs STCOR-776 * test repair * tests are nice * test infrastructure cleanup * import from @folio/jest-config-stripes/testing-library to get current versions * rename `dismissible` to `_dismissible` on destructure to prevent complaints about it not being used. lint and jest are both happy! * fix prop-types in test props, which is probably a losing battle * tests for SessionEventContainer, KeepWorkingModal Externalize SessionEventContainer event handlers and call them with 1,000 arguments, but at least DI makes them testable. I don't love this, but jest really couldn't grok having an event-handler trigger a state-change. If the render and event were both triggered within a single `act()` the re-render got swallowed. If the original render was outside an `act()` then jest complained state was changing outside the `act()` function. Whaddayawant? * codesmell cleanup recommended by sonar * tyop * Lint fixes * Remove commented-out code --------- Co-authored-by: Ryan Berger <rberger@ebsco.com>
…LFetch` (#1438) * STCOR-821 Add `idName` and `limit` as passable parameters to `useChunkedCQLFetch` * Update CHANGELOG
Refs STCOR-789.
…in the path or query arguments (#1445) * STCOR-820 Add support for optionaly passing token by URL param * Remove console.log * Update CHANGELOG.md
…ssible API (#1452) * STCOR-789: add /authn/token to always-permissible list. Refs STCOR-789 Include `/authn/token` on the list of always-permissible API in order to allow OTP-for-cookie exchange on return from authentication. Without this allowance in place, stripes will get stuck in a loop bouncing between the authn-server (which believes, correctly, that the user has authenticated) and stripes (which believes, wrongly, that the user has not authenticated because its "valid AT?" check fails). The AT won't be valid until after we get to exchange the OTP for an AT by visiting `/authn/token`. --------- Co-authored-by: Ryan Berger <rberger@ebsco.com> Co-authored-by: Zak Burke <zburke@ebsco.com>
ryandberger
approved these changes
Apr 16, 2024
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.
Enable the ability to specify the gateway (Kong) URL for each tenant.