From 2f75d430cc24a758073dd8dbe3f71d4d518de16b Mon Sep 17 00:00:00 2001 From: Zak Burke Date: Thu, 9 Jan 2025 16:59:06 -0500 Subject: [PATCH 1/2] STCOR-933 fetch /saml/check when starting a new session (#1582) This is a follow-up to the botched implementation of [STCOR-816](https://folio-org.atlassian.net/browse/STCOR-816) in PR #1432. The description there is "When restoring an existing session..." but in fact the implementation is "When starting a new session or restoring an existing one...". When starting a new session, however, discovery has not happened yet, so there is no information about what interfaces are present, and the conditional would always return false. The implementation here corrects the conditional: 1. if we are starting a new session 2. if we are resuming an existing session and the `login-saml` interface is present. Refs STCOR-933 --- CHANGELOG.md | 1 + src/loginServices.js | 7 +++++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6032c8b1..984f777b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ * Don't override initial discovery and okapi data in test mocks. Refs STCOR-913. * `` must consume `QueryClient` in order to supply it to `loginServices::logout()`. Refs STCOR-907. * On resuming session, spread session and `_self` together to preserve session values. Refs STCOR-912. +* Fetch `/saml/check` when starting a new session, i.e. before discovery. Refs STCOR-933, STCOR-816. ## [10.2.0](https://github.com/folio-org/stripes-core/tree/v10.2.0) (2024-10-11) [Full Changelog](https://github.com/folio-org/stripes-core/compare/v10.1.1...v10.2.0) diff --git a/src/loginServices.js b/src/loginServices.js index 5a38dc46..71670c70 100644 --- a/src/loginServices.js +++ b/src/loginServices.js @@ -851,8 +851,11 @@ export function checkOkapiSession(okapiUrl, store, tenant) { .then((sess) => { return sess?.user?.id ? validateUser(okapiUrl, store, tenant, sess) : null; }) - .then(() => { - if (store.getState().discovery?.interfaces?.['login-saml']) { + .then((res) => { + // check whether SSO is enabled if either + // 1. res is null (when we are starting a new session) + // 2. login-saml interface is present (when we are resuming an existing session) + if (!res || store.getState().discovery?.interfaces?.['login-saml']) { return getSSOEnabled(okapiUrl, store, tenant); } return Promise.resolve(); From 983ce869e2c6853ca683493b54763d8006df38e8 Mon Sep 17 00:00:00 2001 From: Zak Burke Date: Thu, 9 Jan 2025 17:08:47 -0500 Subject: [PATCH 2/2] STCOR-926 validate that cookies and storage are available (#1581) Detect the presence of localStorage, sessionStorage, and cookies early early early in the stripes-init process and show an error message if any are unavailable. This prevents a white screen of death if, say, session storage is unavailable but we call it anyway, resulting in an untrapped exception (Here's looking at you, OIDCRedirect). Refs STCOR-926 --- src/App.js | 63 ++++++++++++++++--- src/App.test.js | 36 +++++++++++ .../AppConfigError/AppConfigError.css | 32 ++++++++++ .../AppConfigError/AppConfigError.js | 47 ++++++++++++++ .../AppConfigError/AppConfigError.test.js | 12 ++++ src/components/AppConfigError/index.js | 1 + src/components/OIDCRedirect.js | 10 ++- 7 files changed, 192 insertions(+), 9 deletions(-) create mode 100644 src/App.test.js create mode 100644 src/components/AppConfigError/AppConfigError.css create mode 100644 src/components/AppConfigError/AppConfigError.js create mode 100644 src/components/AppConfigError/AppConfigError.test.js create mode 100644 src/components/AppConfigError/index.js diff --git a/src/App.js b/src/App.js index 7f72ce32..4cd8ca07 100644 --- a/src/App.js +++ b/src/App.js @@ -3,6 +3,7 @@ import PropTypes from 'prop-types'; import { okapi as okapiConfig, config } from 'stripes-config'; import merge from 'lodash/merge'; +import AppConfigError from './components/AppConfigError'; import connectErrorEpic from './connectErrorEpic'; import configureEpics from './configureEpics'; import configureLogger from './configureLogger'; @@ -23,6 +24,39 @@ const StrictWrapper = ({ children }) => { return {children}; }; +/** + * isStorageEnabled + * Return true if local-storage, session-storage, and cookies are all enabled. + * Return false otherwise. + * @returns boolean true if storages are enabled; false otherwise. + */ +export const isStorageEnabled = () => { + let isEnabled = true; + // local storage + try { + localStorage.getItem('test-key'); + } catch (e) { + console.warn('local storage is disabled'); // eslint-disable-line no-console + isEnabled = false; + } + + // session storage + try { + sessionStorage.getItem('test-key'); + } catch (e) { + console.warn('session storage is disabled'); // eslint-disable-line no-console + isEnabled = false; + } + + // cookies + if (!navigator.cookieEnabled) { + console.warn('cookies are disabled'); // eslint-disable-line no-console + isEnabled = false; + } + + return isEnabled; +}; + StrictWrapper.propTypes = { children: PropTypes.node.isRequired, }; @@ -36,17 +70,23 @@ export default class StripesCore extends Component { constructor(props) { super(props); - const parsedTenant = getStoredTenant(); + if (isStorageEnabled()) { + const parsedTenant = getStoredTenant(); - const okapi = (typeof okapiConfig === 'object' && Object.keys(okapiConfig).length > 0) - ? { ...okapiConfig, tenant: parsedTenant?.tenantName || okapiConfig.tenant, clientId: parsedTenant?.clientId || okapiConfig.clientId } : { withoutOkapi: true }; + const okapi = (typeof okapiConfig === 'object' && Object.keys(okapiConfig).length > 0) + ? { ...okapiConfig, tenant: parsedTenant?.tenantName || okapiConfig.tenant, clientId: parsedTenant?.clientId || okapiConfig.clientId } : { withoutOkapi: true }; - const initialState = merge({}, { okapi }, props.initialState); + const initialState = merge({}, { okapi }, props.initialState); - this.logger = configureLogger(config); - this.epics = configureEpics(connectErrorEpic); - this.store = configureStore(initialState, this.logger, this.epics); - this.actionNames = gatherActions(); + this.logger = configureLogger(config); + this.epics = configureEpics(connectErrorEpic); + this.store = configureStore(initialState, this.logger, this.epics); + this.actionNames = gatherActions(); + + this.state = { isStorageEnabled: true }; + } else { + this.state = { isStorageEnabled: false }; + } } componentWillUnmount() { @@ -54,6 +94,13 @@ export default class StripesCore extends Component { } render() { + // Stripes requires cookies (for login) and session and local storage + // (for session state and all manner of things). If these are not enabled, + // stop and show an error message. + if (!this.state.isStorageEnabled) { + return ; + } + // no need to pass along `initialState` // eslint-disable-next-line no-unused-vars const { initialState, ...props } = this.props; diff --git a/src/App.test.js b/src/App.test.js new file mode 100644 index 00000000..eb805f6b --- /dev/null +++ b/src/App.test.js @@ -0,0 +1,36 @@ +import { isStorageEnabled } from './App'; + +const storageMock = () => ({ + getItem: () => { + throw new Error(); + }, +}); + +describe('isStorageEnabled', () => { + afterEach(() => { + jest.restoreAllMocks(); + }); + + it('returns true when all storage options are enabled', () => { + expect(isStorageEnabled()).toBeTrue; + }); + + describe('returns false when any storage option is disabled', () => { + it('handles local storage', () => { + Object.defineProperty(window, 'localStorage', { value: storageMock }); + const isEnabled = isStorageEnabled(); + expect(isEnabled).toBeFalse; + }); + it('handles session storage', () => { + Object.defineProperty(window, 'sessionStorage', { value: storageMock }); + const isEnabled = isStorageEnabled(); + expect(isEnabled).toBeFalse; + }); + + it('handles cookies', () => { + jest.spyOn(navigator, 'cookieEnabled', 'get').mockReturnValue(false); + const isEnabled = isStorageEnabled(); + expect(isEnabled).toBeFalse; + }); + }); +}); diff --git a/src/components/AppConfigError/AppConfigError.css b/src/components/AppConfigError/AppConfigError.css new file mode 100644 index 00000000..e8cc7eff --- /dev/null +++ b/src/components/AppConfigError/AppConfigError.css @@ -0,0 +1,32 @@ +@import "@folio/stripes-components/lib/variables.css"; + +.wrapper { + display: flex; + justify-content: center; + min-height: 100vh; +} + +.container { + width: 100%; + max-width: 940px; + min-height: 330px; + margin: 12vh 2rem 0; +} + +@media (--medium-up) { + .container { + min-height: initial; + } +} + +@media (--large-up) { + .header { + font-size: var(--font-size-xx-large); + } +} + +@media (height <= 440px) { + .container { + min-height: 330px; + } +} diff --git a/src/components/AppConfigError/AppConfigError.js b/src/components/AppConfigError/AppConfigError.js new file mode 100644 index 00000000..bf3623bc --- /dev/null +++ b/src/components/AppConfigError/AppConfigError.js @@ -0,0 +1,47 @@ +import { branding } from 'stripes-config'; + +import { + Row, + Col, + Headline, +} from '@folio/stripes-components'; + +import OrganizationLogo from '../OrganizationLogo'; +import styles from './AppConfigError.css'; + +/** + * AppConfigError + * Show an error message. This component is rendered by App, before anything + * else, when it detects that local storage, session storage, or cookies are + * unavailable. This happens _before_ Root has been initialized, i.e. before + * an IntlProvider is available, hence the hard-coded, English-only message. + * + * @returns English-only error message + */ +const AppConfigError = () => { + return ( +
+
+
+ + + + + + + + + FOLIO requires cookies, sessionStorage, and localStorage. Please enable these features and try again. + + + +
+
+
+ ); +}; + +export default AppConfigError; diff --git a/src/components/AppConfigError/AppConfigError.test.js b/src/components/AppConfigError/AppConfigError.test.js new file mode 100644 index 00000000..95f88aa3 --- /dev/null +++ b/src/components/AppConfigError/AppConfigError.test.js @@ -0,0 +1,12 @@ +import { render, screen } from '@folio/jest-config-stripes/testing-library/react'; +import AppConfigError from './AppConfigError'; + +jest.mock('../OrganizationLogo', () => () => 'OrganizationLogo'); +describe('AppConfigError', () => { + it('displays a warning message', async () => { + render(); + + expect(screen.getByText(/cookies/i)).toBeInTheDocument(); + expect(screen.getByText(/storage/i)).toBeInTheDocument(); + }); +}); diff --git a/src/components/AppConfigError/index.js b/src/components/AppConfigError/index.js new file mode 100644 index 00000000..f6a6ec98 --- /dev/null +++ b/src/components/AppConfigError/index.js @@ -0,0 +1 @@ +export { default } from './AppConfigError'; diff --git a/src/components/OIDCRedirect.js b/src/components/OIDCRedirect.js index 9d463fe9..8898ca6e 100644 --- a/src/components/OIDCRedirect.js +++ b/src/components/OIDCRedirect.js @@ -5,7 +5,15 @@ import { getUnauthorizedPathFromSession, removeUnauthorizedPathFromSession } fro // Setting at top of component since value should be retained during re-renders // but will be correctly re-fetched when redirected from Keycloak login page. -const unauthorizedPath = getUnauthorizedPathFromSession(); +// The empty try/catch is necessary because, by setting this at the top of +// the component, it is automatically executed even before renders. +// IOW, even though we check for session-storage in App, we still have to +// protect the call here. +let unauthorizedPath = null; +try { + unauthorizedPath = getUnauthorizedPathFromSession(); +} catch (e) { // eslint-disable-line no-empty +} /** * OIDCRedirect authenticated route handler for /oidc-landing.