Skip to content
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-747: Provide optional tenant argument to useOkapiKy hook #1348

Merged
merged 5 commits into from
Oct 13, 2023

Conversation

mariia-aloshyna
Copy link
Contributor

@mariia-aloshyna mariia-aloshyna commented Oct 11, 2023

Introduce an optional tenant argument to useOkapiKy hook to be able to override headers with a provided tenant.

Refs: https://issues.folio.org/browse/STCOR-747

@github-actions
Copy link

github-actions bot commented Oct 11, 2023

Jest Unit Test Statistics

77 tests  +3   77 ✔️ +3   18s ⏱️ +6s
13 suites +1     0 💤 ±0 
  1 files   ±0     0 ±0 

Results for commit 0da3f5e. ± Comparison against base commit 0641240.

♻️ This comment has been updated with latest results.

@github-actions
Copy link

github-actions bot commented Oct 11, 2023

BigTest Unit Test Statistics

    1 files  ±0      1 suites  ±0   11s ⏱️ -1s
271 tests ±0  266 ✔️ ±0  5 💤 ±0  0 ±0 
274 runs  ±0  269 ✔️ ±0  5 💤 ±0  0 ±0 

Results for commit 0da3f5e. ± Comparison against base commit 0641240.

This pull request removes 5 and adds 3 tests. Note that renamed tests count towards both.
      equal to check email label in english translation
      equal to check email precautions label in english translation
      equal to sent email precautions label in english translation
Chrome_117_0_0_0_(Linux_x86_64).Forgot username form test ‑ Forgot username form test check email status page tests should have the header with an appropriate text content
Chrome_117_0_0_0_(Linux_x86_64).Forgot username form test ‑ Forgot username form test check email status page tests should have the paragraph with an appropriate text content
Chrome_117_0_0_0_(Linux_x86_64).Forgot username form test ‑ Forgot username form test check email status page tests should have the header with an appropriate text content
      equal to check email label in english translation
Chrome_117_0_0_0_(Linux_x86_64).Forgot username form test ‑ Forgot username form test check email status page tests should have the paragraph with an appropriate text content
      equal to check email precautions label in english translation
Chrome_117_0_0_0_(Linux_x86_64).Forgot username form test ‑ Forgot username form test check email status page tests should have the paragraph with an appropriate text content
      equal to sent email precautions label in english translation

♻️ This comment has been updated with latest results.

@mariia-aloshyna mariia-aloshyna requested a review from a team October 11, 2023 17:22
Copy link
Member

@zburke zburke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. I added tests since there were none at all for this hook.

@mariia-aloshyna
Copy link
Contributor Author

LGTM. I added tests since there were none at all for this hook.

Thank you @zburke ! Adjusted CHANGELOG

@sonarcloud
Copy link

sonarcloud bot commented Oct 13, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

100.0% 100.0% Coverage
0.0% 0.0% Duplication

@mariia-aloshyna mariia-aloshyna merged commit e3d4712 into master Oct 13, 2023
5 checks passed
@mariia-aloshyna mariia-aloshyna deleted the STCOR-747 branch October 13, 2023 10:55
zburke added a commit that referenced this pull request Jan 10, 2024
* bump minor to 10.1 for new development (#1350)

* STCOR-747: Provide optional tenant argument to `useOkapiKy` hook (#1348)

* STCOR-749: Allow to import validateUser function from @folio/stripes/core (#1351)

* STCOR-747: Convert optional arg to object for useOkapiKy hook (#1352)

* CVE-2023-45133 require @babel/traverse >= 7.23.2 (#1353)

`@babel/traverse` < 7.23.2 is vulnerable to CVE-2023-45133.

* [STCOR-752]: Ensure <AppIcon> is not cut off (#1355)

* Revert "[STCOR-752]: Ensure <AppIcon> is not cut off (#1355)" (#1357)

This reverts commit a17be4c.

`<AppIcon>` may be used like `<AppIcon icon="foo">Title of an instance</AppIcon>` in SearchAndSort results lists, and this sets the width incorrectly.

* [STCOR-752]: Ensure <AppIcon> is not cut off (for real this time) (#1358)

* [STCOR-752]: Ensure <AppIcon> is not cut off

* Only apply min-width to appIcon classes

* Add changelog

* CVE-2023-46234 require browserify-sign >= 4.2.2 (#1359)

`browserify-sign` < 4.2.2 is vulnerable to CVE-2023-46234.

* Lokalise: updates

* STCOR-671 handle access-control via cookies (#1346)

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

Replaces #1340. It was gross and I really don't want to talk about it.
Let us never mention it again. 

Refs STCOR-671, FOLIO-3627

* STCOR-574 rotate tokens after 80% of their TTL (#1361)

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

* STCOR-756 correctly evaluate token lifespan (#1363)

The most important work here is fixing the bug from #1361 that
incorrectly evaluated whether a token was still valid. The original
implementation shrank the total lifespan of the token rather than
shrinking only the period of its lifespan in the future.

Additional improvements here include:

* evaluate `navigator.serviceWorker` more cautiously; some browsers (e.g.
  Firefox in Incognito) may deny access to service workers. See STCOR-757
  for additional details.
* use `{ source, type, value }` shaped messages consistently when exchanging
  messages with the service worker.
* shrink the tokens' validity windows when receiving the `TOKEN_EXPIRATION`
  message instead of calculating the shorter window each time the token is
  evaluated. This is more efficient.
* await the promise returned by `postTokenExpiration` from
  `navigator.serviceWorker.ready` in order to prevent requests from being
  sent when the service worker isn't ready for them.
* use the `new Response()` constructor correctly, i.e. stringify the JSON
  value; otherwise clients will receive the string `[object Object]` instead
  of the empty JSON object, `{}`.

Note: the login test is turned off here due to the fact that the login
flow invokes `navigator.serviceWorker.ready`, which returns a Promise that
only returns when the service worker enters a ready state, but the karma
build does not configure the service worker, hence this test times out
every time. This is not great, but resolving it is a non-trivial task.

Refs STCOR-756

* STCOR-759 read okapi config from micro-stripes-config (#1366)

A service worker's global state is reset after each sleep/wake cycle,
meaning the `okapiUrl` and `okapiTenant` values so lovingly sent to the
service worker during registration are likely to be promptly forgotten
as soon as the browser is idle for a few minutes and decides it would be
good to clean up inactive processes.

Here, those values are directly imported from a virtual module created
at build-time by stripes-webpack, which forwards the values from the
stripes-config object (most likely, the `stripes.config.js` file) and
allows them to be compiled directly into the generated
`service-worker.js` asset.

An alternative approach would be to pass in those values as URL
parameters when the service worker is registered.

h/t @mkuklis and @JohnC-80 who did the heavy lifting here, both in
thinking through the potential solutions and actually figuring out how
to implement this in our highly customized build process.

* Requires folio-org/stripes-webpack/pull/132

Refs STCOR-759

---------

Co-authored-by: Michal Kuklis <[email protected]>

* STCOR-761 allow console to be preserved on logout (#1367)

Normally the console is cleared on logout. This is good for security but
bad for debugging in the case where a user may be automatically logged
out due to RTR.

Refs STCOR-761

* STCOR-759 return non-okapi request fetches as-is (#1369)

Instead of catching errors from non-Okapi requests and converting them
to rejected promises, just `return fetch()` straight up, whatever that
response contains.

h/t @MikeTaylor for pointing me in this direction, and reading minified
code to try to suss the problem.

Refs STCOR-759, UILDP-129

* Revert RTR (#1371)

Revert all PRs related to STCOR-571, which implemented RTR
in a service worker. This includes the following:

* Revert "STCOR-759 return non-okapi request fetches as-is (#1369)"
  This reverts commit a323456.
* Revert "STCOR-759 read okapi config from micro-stripes-config (#1366)"
  This reverts commit 6c6a85e.
* Revert "STCOR-756 correctly evaluate token lifespan (#1363)"
  This reverts commit 607dc5c.
* Revert "STCOR-574 rotate tokens after 80% of their TTL (#1361)"
  This reverts commit dd71819.
* Revert "STCOR-671 handle access-control via cookies (#1346)"
  This reverts commit 27d2948.

Also, add a dummy service-worker.js file to appease stripes-webpack.
This removes the need to do extra clean-up work in that repository.

* Revert RTR: auto-unregister zombie service workers (#1372)

* auto-unregister zombie service workers
* lint

* Lokalise: updates

* STCOR-671 handle access control via cookies

Refs STCOR-671

* add request handling to isOkapiRequest, replace localstorage usage with utility, rtr-specific error.

* clean up comments and resource handling

* comments are in sync with function arguments
* consistently handle non-string arguments passed to fetch

* implement fake XHR class

* add implementation of XHR override, move functions to token-util

* tests, comments

* rtr() tests

* tests and comments; rtr() refactor handles inner exceptions

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.

* put RTR behind stripes-config.useSecureTokens

* add FFetch tests

* add FFetch tests

* FFetch tests at 100% coverage

* STCOR-762 disable login when cookies are disabled

* linty mclintface

* when config.useSecureTokens is absent, restore token access

* restore token-based endpoints since BTOG isn't configured with useSecureTokens=true

* replace polling; replace 403+inspection with 400 detection

* add test for FXHR

* restore tokenExpiration checking

Those 403s hitting the console were just too much to handle for me, so
we're restoring the `isValidAT` and `isValidRT` code to intercept
requests that we know will fail before they fail. Since a browser may
fire several fetches at once, we weren't just seeing one request failure
when the AT expired, we saw several, making a mess of the console.

This logic is a little more intricate, but the console is a lot more
clean.

* mock localforage to always return valid expiration data

* FFetch coverage

* lint

* backward compatibility: include x-okapi-token header when token is present

* apply same settings to withOkapiKy as useOkapiKy

* include mode and credentials flags in all okapi calls

* trivial commit

* handle cross-tab communication during rotation with localStorage

* log correctly

* don't worry about checking to see if context.rtrPromise exists

* STCOR-767 Fix duplicated FOLIO in document title in some cases (#1377)

* STCOR-767 Fix duplicated FOLIO in document title in some cases

* STCOR-767 Fix code smell

* CVE-2023-48631 require @adobe/css-tools >= 4.3.2 (#1379)

@adobe/css-tools < 4.3.2 is vulnerable to CVE-2023-48631.

* correctly handle single-window and multi-window rotation

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.

* ignore stale rotation requests

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.

* lint. it's always lint.

* STCOR-768 Refactor away from color() function. (#1380)

* refactor away from color() function in Toast.css

* log changes

* STCOR-770: Export getEventHandler to be able to create events in other modules. (#1383)

* backward compat with http-header token authz

---------

Co-authored-by: Mariia Aloshyna <[email protected]>
Co-authored-by: Oleksandr Hladchenko <[email protected]>
Co-authored-by: Noah Overcash <[email protected]>
Co-authored-by: FOLIO Translations Bot <[email protected]>
Co-authored-by: Michal Kuklis <[email protected]>
Co-authored-by: Peter Murray <[email protected]>
Co-authored-by: John Coburn <[email protected]>
Co-authored-by: John Malconian <[email protected]>
Co-authored-by: Denys Bohdan <[email protected]>
Co-authored-by: Dmytro-Melnyshyn <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants