From 465d30120855895d4704f13d6c8ce550229a14e0 Mon Sep 17 00:00:00 2001 From: Zak Burke Date: Fri, 25 Oct 2024 08:25:39 -0400 Subject: [PATCH 1/4] STCOR-882 provide a "you have logged out" page Consolidate `` and `` into a single component, accessible through different routes, and always redirect there. This changes the behavior of user-directed "logout" by showing a "You have logged out" page with a "login again?" button. Previously, the sequence was a redirect to `/`, which would eventually be followed by a redirect for authn. The problem is that the router, ``, would receive the redirect to `/` before re-rendering with `isAuthenticated: false`. This causes `` to render the "Welcome to FOLIO" page, followed by the re-render and the redirect to authn. Refs STCOR-882 --- src/RootWithIntl.js | 12 ++- .../LogoutTimeout.css => Logout/Logout.css} | 0 src/components/Logout/Logout.js | 69 +++++++++++--- src/components/Logout/Logout.test.js | 80 ++++++++++------- src/components/LogoutTimeout/LogoutTimeout.js | 90 ------------------- .../LogoutTimeout/LogoutTimeout.test.js | 71 --------------- src/components/LogoutTimeout/index.js | 1 - src/components/index.js | 1 - translations/stripes-core/en.json | 1 + translations/stripes-core/en_US.json | 8 +- 10 files changed, 123 insertions(+), 210 deletions(-) rename src/components/{LogoutTimeout/LogoutTimeout.css => Logout/Logout.css} (100%) delete mode 100644 src/components/LogoutTimeout/LogoutTimeout.js delete mode 100644 src/components/LogoutTimeout/LogoutTimeout.test.js delete mode 100644 src/components/LogoutTimeout/index.js diff --git a/src/RootWithIntl.js b/src/RootWithIntl.js index 2b0be727f..1dcf29c63 100644 --- a/src/RootWithIntl.js +++ b/src/RootWithIntl.js @@ -28,7 +28,6 @@ import { HandlerManager, TitleManager, Logout, - LogoutTimeout, OverlayContainer, CreateResetPassword, CheckEmailStatusPage, @@ -99,7 +98,7 @@ const RootWithIntl = ({ stripes, token = '', isAuthenticated = false, disableAut } + component={} /> } + component={} /> @@ -156,10 +155,15 @@ const RootWithIntl = ({ stripes, token = '', isAuthenticated = false, disableAut path="/check-email" component={} /> + } + /> } + component={} /> { const stripes = useStripes(); const [didLogout, setDidLogout] = useState(false); + const location = useLocation(); + + const messageId = location.pathName === '/logout-timeout' ? 'stripes-core.rtr.idleSession.sessionExpiredSoSad' : 'stripes-core.logoutComplete'; useEffect( () => { - getLocale(stripes.okapi.url, stripes.store, stripes.okapi.tenant) - .then(logout(stripes.okapi.url, stripes.store)) - .then(setDidLogout(true)); + if (stripes.okapi.isAuthenticated) { + // returns a promise, which we ignore + logout(stripes.okapi.url, stripes.store) + .then(setDidLogout(true)); + } else { + setDidLogout(true); + } }, // no dependencies because we only want to start the logout process once. - // we don't care about changes to history or stripes; certainly those - // could be updated as part of the logout process + // we don't care about changes to stripes; certainly it'll be updated as + // part of the logout process // eslint-disable-next-line react-hooks/exhaustive-deps [] ); - return didLogout ? : ; + if (!didLogout) { + return ; + } + + return ( +
+
+
+ + + + + + + + + + + + + + + +
+
+
+ ); }; export default Logout; diff --git a/src/components/Logout/Logout.test.js b/src/components/Logout/Logout.test.js index 383c89d0e..aa950b0da 100644 --- a/src/components/Logout/Logout.test.js +++ b/src/components/Logout/Logout.test.js @@ -1,46 +1,64 @@ -import { render, screen, waitFor } from '@folio/jest-config-stripes/testing-library/react'; +import { render, screen } from '@folio/jest-config-stripes/testing-library/react'; +import { useLocation } from 'react-router'; -import Harness from '../../../test/jest/helpers/harness'; import Logout from './Logout'; +import { useStripes } from '../../StripesContext'; import { logout } from '../../loginServices'; +jest.mock('../OrganizationLogo'); +jest.mock('../../StripesContext'); +jest.mock('react-router'); + jest.mock('../../loginServices', () => ({ - ...(jest.requireActual('../../loginServices')), - getLocale: () => Promise.resolve(), - logout: jest.fn() + logout: jest.fn(() => Promise.resolve()), })); -jest.mock('react-router', () => ({ - ...(jest.requireActual('react-router')), - Redirect: () =>
Redirect
-})); +describe('Logout', () => { + describe('Direct logout', () => { + beforeEach(() => { + const mockUseLocation = useLocation; + mockUseLocation.mockReturnValue({ pathName: '/logout' }); + }); -const stripes = { - config: { - rtr: { - idleModalTTL: '3s', - idleSessionTTL: '3s', - } - }, - okapi: { - url: 'https://blah', - }, - logger: { log: jest.fn() }, - store: { - getState: jest.fn(), - }, -}; + it('if not authenticated, renders a logout message', async () => { + const mockUseStripes = useStripes; + mockUseStripes.mockReturnValue({ okapi: { isAuthenticated: false } }); -describe('Logout', () => { - it('calls logout and redirects', async () => { - logout.mockReturnValue(Promise.resolve()); + render(); + screen.getByText('stripes-core.logoutComplete'); + }); + + it('if authenticated, calls logout then renders a logout message', async () => { + const mockUseStripes = useStripes; + mockUseStripes.mockReturnValue({ okapi: { isAuthenticated: true } }); - render(); + render(); + expect(logout).toHaveBeenCalled(); + screen.getByText('stripes-core.logoutComplete'); + }); + }); - await waitFor(() => { - screen.getByText('Redirect'); + describe('Timeout logout', () => { + beforeEach(() => { + const mockUseLocation = useLocation; + mockUseLocation.mockReturnValue({ pathName: '/logout-timeout' }); }); - expect(logout).toHaveBeenCalledTimes(1); + it('if not authenticated, renders a timeout message', async () => { + const mockUseStripes = useStripes; + mockUseStripes.mockReturnValue({ okapi: { isAuthenticated: false } }); + + render(); + screen.getByText('stripes-core.rtr.idleSession.sessionExpiredSoSad'); + }); + + it('if authenticated, calls logout then renders a timeout message', async () => { + const mockUseStripes = useStripes; + mockUseStripes.mockReturnValue({ okapi: { isAuthenticated: true } }); + + render(); + expect(logout).toHaveBeenCalled(); + screen.getByText('stripes-core.rtr.idleSession.sessionExpiredSoSad'); + }); }); }); diff --git a/src/components/LogoutTimeout/LogoutTimeout.js b/src/components/LogoutTimeout/LogoutTimeout.js deleted file mode 100644 index 557a32c05..000000000 --- a/src/components/LogoutTimeout/LogoutTimeout.js +++ /dev/null @@ -1,90 +0,0 @@ -import { useEffect, useState } from 'react'; -import { FormattedMessage } from 'react-intl'; -import { branding } from 'stripes-config'; - -import { - Button, - Col, - Headline, - LoadingView, - Row, -} from '@folio/stripes-components'; - -import OrganizationLogo from '../OrganizationLogo'; -import { useStripes } from '../../StripesContext'; -import { - getUnauthorizedPathFromSession, - logout, - removeUnauthorizedPathFromSession, -} from '../../loginServices'; - -import styles from './LogoutTimeout.css'; - -/** - * LogoutTimeout - * Show a "sorry, your session timed out message"; if the session is still - * active, call logout() to end it. - * - * Having a static route to this page allows the logout handler to choose - * between redirecting straight to the login page (if the user chose to - * logout) or to this page (if the session timed out). - * - * This corresponds to the '/logout-timeout' route. - */ -const LogoutTimeout = () => { - const stripes = useStripes(); - const [didLogout, setDidLogout] = useState(false); - - useEffect( - () => { - if (stripes.okapi.isAuthenticated) { - // returns a promise, which we ignore - logout(stripes.okapi.url, stripes.store) - .then(setDidLogout(true)); - } else { - setDidLogout(true); - } - }, - // no dependencies because we only want to start the logout process once. - // we don't care about changes to stripes; certainly it'll be updated as - // part of the logout process - // eslint-disable-next-line react-hooks/exhaustive-deps - [] - ); - - const handleClick = (_e) => { - removeUnauthorizedPathFromSession(); - }; - - const redirectTo = getUnauthorizedPathFromSession() || '/'; - - if (!didLogout) { - return ; - } - - return ( -
-
-
- - - - - - - - - - - - - - - -
-
-
- ); -}; - -export default LogoutTimeout; diff --git a/src/components/LogoutTimeout/LogoutTimeout.test.js b/src/components/LogoutTimeout/LogoutTimeout.test.js deleted file mode 100644 index cb8606eac..000000000 --- a/src/components/LogoutTimeout/LogoutTimeout.test.js +++ /dev/null @@ -1,71 +0,0 @@ -import { render, screen } from '@folio/jest-config-stripes/testing-library/react'; -import { userEvent } from '@folio/jest-config-stripes/testing-library/user-event'; - -import LogoutTimeout from './LogoutTimeout'; -import { useStripes } from '../../StripesContext'; -import { getUnauthorizedPathFromSession, logout, setUnauthorizedPathToSession } from '../../loginServices'; - -jest.mock('../OrganizationLogo'); -jest.mock('../../StripesContext'); -jest.mock('react-router', () => ({ - Redirect: () =>
Redirect
, -})); - -jest.mock('../../loginServices', () => ({ - logout: jest.fn(() => Promise.resolve()), - getUnauthorizedPathFromSession: jest.fn(), - removeUnauthorizedPathFromSession: jest.fn(), - setUnauthorizedPathToSession: jest.fn(), -})); - -describe('LogoutTimeout', () => { - describe('if not authenticated', () => { - it('renders a timeout message', async () => { - const mockUseStripes = useStripes; - mockUseStripes.mockReturnValue({ okapi: { isAuthenticated: false } }); - - render(); - screen.getByText('stripes-core.rtr.idleSession.sessionExpiredSoSad'); - }); - - it('clears previous path from storage after clicking "log in again"', async () => { - const previousPath = '/monkey?bagel'; - setUnauthorizedPathToSession(previousPath); - const user = userEvent.setup(); - const mockUseStripes = useStripes; - mockUseStripes.mockReturnValue({ okapi: { isAuthenticated: false } }); - - render(); - - await user.click(screen.getByRole('button')); - expect(getUnauthorizedPathFromSession()).toBeFalsy(); - }); - }); - - describe('if not authenticated', () => { - it('calls logout then renders a timeout message', async () => { - const mockUseStripes = useStripes; - mockUseStripes.mockReturnValue({ okapi: { isAuthenticated: true } }); - - render(); - expect(logout).toHaveBeenCalled(); - screen.getByText('stripes-core.rtr.idleSession.sessionExpiredSoSad'); - }); - - it('clears previous path from storage after clicking "log in again"', async () => { - const previousPath = '/monkey?bagel'; - setUnauthorizedPathToSession(previousPath); - const user = userEvent.setup(); - const mockUseStripes = useStripes; - mockUseStripes.mockReturnValue({ okapi: { isAuthenticated: true } }); - - render(); - - expect(logout).toHaveBeenCalled(); - screen.getByText('stripes-core.rtr.idleSession.sessionExpiredSoSad'); - - await user.click(screen.getByRole('button')); - expect(getUnauthorizedPathFromSession()).toBeFalsy(); - }); - }); -}); diff --git a/src/components/LogoutTimeout/index.js b/src/components/LogoutTimeout/index.js deleted file mode 100644 index a355b4149..000000000 --- a/src/components/LogoutTimeout/index.js +++ /dev/null @@ -1 +0,0 @@ -export { default } from './LogoutTimeout'; diff --git a/src/components/index.js b/src/components/index.js index fa72c92b4..015f3867a 100644 --- a/src/components/index.js +++ b/src/components/index.js @@ -6,7 +6,6 @@ export { default as AppCtxMenuProvider } from './MainNav/CurrentApp/AppCtxMenuPr export { LastVisitedContext, withLastVisited } from './LastVisited'; export { default as Login } from './Login'; export { default as Logout } from './Logout'; -export { default as LogoutTimeout } from './LogoutTimeout'; export { default as MainContainer } from './MainContainer'; export { default as MainNav } from './MainNav'; export { default as ModuleContainer } from './ModuleContainer'; diff --git a/translations/stripes-core/en.json b/translations/stripes-core/en.json index 8ccf97c25..c0c98413a 100644 --- a/translations/stripes-core/en.json +++ b/translations/stripes-core/en.json @@ -90,6 +90,7 @@ "logout": "Log out", "logoutPending": "Log out in process...", "logoutKeepSso": "Log out from FOLIO, keep SSO session", + "logoutComplete": "You have logged out.", "login": "Log in", "username": "Username", "password": "Password", diff --git a/translations/stripes-core/en_US.json b/translations/stripes-core/en_US.json index 1bfbcd716..8dfb33d4b 100644 --- a/translations/stripes-core/en_US.json +++ b/translations/stripes-core/en_US.json @@ -152,6 +152,7 @@ "rtr.idleSession.keepWorking": "Keep working", "rtr.idleSession.sessionExpiredSoSad": "Your session expired due to inactivity.", "rtr.idleSession.logInAgain": "Log in again", +<<<<<<< Updated upstream "title.logout": "Log out", "about.applicationCount": "{count} applications", "about.applicationsVersionsTitle": "Applications/modules/interfaces", @@ -159,4 +160,9 @@ "tenantLibrary": "Tenant/Library", "errors.saml.missingToken": "No code query parameter.", "rtr.fixedLengthSession.timeRemaining": "Your session will end soon! Time remaining:" -} \ No newline at end of file +} +======= + "rtr.fixedLengthSession.timeRemaining": "Your session will end soon! Time remaining:", + "logoutComplete": "You have logged out." +} +>>>>>>> Stashed changes From 95d0e9afd25896b348b5e0ed943c55cdeedeadae Mon Sep 17 00:00:00 2001 From: Zak Burke Date: Fri, 25 Oct 2024 08:28:43 -0400 Subject: [PATCH 2/4] whoops; bad translation merge --- translations/stripes-core/en_US.json | 5 ----- 1 file changed, 5 deletions(-) diff --git a/translations/stripes-core/en_US.json b/translations/stripes-core/en_US.json index 8dfb33d4b..e6225264e 100644 --- a/translations/stripes-core/en_US.json +++ b/translations/stripes-core/en_US.json @@ -152,17 +152,12 @@ "rtr.idleSession.keepWorking": "Keep working", "rtr.idleSession.sessionExpiredSoSad": "Your session expired due to inactivity.", "rtr.idleSession.logInAgain": "Log in again", -<<<<<<< Updated upstream "title.logout": "Log out", "about.applicationCount": "{count} applications", "about.applicationsVersionsTitle": "Applications/modules/interfaces", "tenantChoose": "Select your tenant/library", "tenantLibrary": "Tenant/Library", "errors.saml.missingToken": "No code query parameter.", - "rtr.fixedLengthSession.timeRemaining": "Your session will end soon! Time remaining:" -} -======= "rtr.fixedLengthSession.timeRemaining": "Your session will end soon! Time remaining:", "logoutComplete": "You have logged out." } ->>>>>>> Stashed changes From 47294977cc09e6738beda05973d337610e784e28 Mon Sep 17 00:00:00 2001 From: Zak Burke Date: Fri, 25 Oct 2024 08:44:08 -0400 Subject: [PATCH 3/4] update ancient logout BTOG test --- test/bigtest/tests/profile-dropdown-test.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/test/bigtest/tests/profile-dropdown-test.js b/test/bigtest/tests/profile-dropdown-test.js index a0a3f475a..7ae3ef120 100644 --- a/test/bigtest/tests/profile-dropdown-test.js +++ b/test/bigtest/tests/profile-dropdown-test.js @@ -19,7 +19,6 @@ const ProfileMenuInteractor = HTML.extend('profile menu') describe('Profile dropdown', () => { const profileDropdown = DropdownInteractor({ id: 'profileDropdown' }); const profileMenu = ProfileMenuInteractor(); - const loginInteractor = LoginInteractor({ id: 'input-username' }); const modules = [{ type: 'app', @@ -104,7 +103,9 @@ describe('Profile dropdown', () => { await profileMenu.find(HTML('Log out')).click(); }); - it('changes the url', () => loginInteractor.exists()); + it('changes the url', function () { + expect(this.location.pathname).to.equal('/logout'); + }); }); }); }); From 3191253b229ec590a9847423a610ddc4328e92a9 Mon Sep 17 00:00:00 2001 From: Zak Burke Date: Fri, 25 Oct 2024 12:40:08 -0400 Subject: [PATCH 4/4] lint --- test/bigtest/tests/profile-dropdown-test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/bigtest/tests/profile-dropdown-test.js b/test/bigtest/tests/profile-dropdown-test.js index 7ae3ef120..affd2b8ca 100644 --- a/test/bigtest/tests/profile-dropdown-test.js +++ b/test/bigtest/tests/profile-dropdown-test.js @@ -1,7 +1,7 @@ import { describe, beforeEach, it } from 'mocha'; import { expect } from 'chai'; import React, { Component } from 'react'; -import { Dropdown as DropdownInteractor, HTML, TextField as LoginInteractor } from '@folio/stripes-testing'; +import { Dropdown as DropdownInteractor, HTML } from '@folio/stripes-testing'; import setupApplication from '../helpers/setup-application'; class DummyApp extends Component {