From b76afa986ae00e10c4215c70ab4b2825a3e2f254 Mon Sep 17 00:00:00 2001 From: Hana Worku Date: Mon, 8 Jan 2024 13:23:04 -0600 Subject: [PATCH] Fix tests - logout should be required function - handle auth error alert banner on Landing, not Header --- .../src/components/Header/Header.test.tsx | 31 ----------- .../app-web/src/components/Header/Header.tsx | 16 +----- services/app-web/src/contexts/AuthContext.tsx | 53 ++++++++++++++++--- services/app-web/src/pages/Auth/Auth.test.tsx | 6 +-- services/app-web/src/pages/Auth/Login.tsx | 14 ++--- .../src/pages/Landing/Landing.test.tsx | 22 ++++++++ .../app-web/src/pages/Landing/Landing.tsx | 18 +++++-- .../Wrapper/AuthenticatedRouteWrapper.tsx | 13 ++--- 8 files changed, 99 insertions(+), 74 deletions(-) diff --git a/services/app-web/src/components/Header/Header.test.tsx b/services/app-web/src/components/Header/Header.test.tsx index 74c2dc9cd3..99eebfe425 100644 --- a/services/app-web/src/components/Header/Header.test.tsx +++ b/services/app-web/src/components/Header/Header.test.tsx @@ -141,36 +141,5 @@ describe('Header', () => { await waitFor(() => expect(spy).toHaveBeenCalledTimes(1)) }) - - it('calls setAlert when logout is unsuccessful', async () => { - const spy = jest - .spyOn(CognitoAuthApi, 'signOut') - .mockRejectedValue('This logout failed!') - const mockAlert = jest.fn() - - renderWithProviders( -
, - { - apolloProvider: { - mocks: [ - fetchCurrentUserMock({ statusCode: 200 }), - fetchCurrentUserMock({ statusCode: 403 }), - ], - }, - } - ) - - await waitFor(() => { - const signOutButton = screen.getByRole('button', { - name: /Sign out/i, - }) - - expect(signOutButton).toBeInTheDocument() - void userEvent.click(signOutButton) - }) - - await waitFor(() => expect(spy).toHaveBeenCalledTimes(1)) - await waitFor(() => expect(mockAlert).toHaveBeenCalled()) - }) }) }) diff --git a/services/app-web/src/components/Header/Header.tsx b/services/app-web/src/components/Header/Header.tsx index d3dc65cae7..88a8ffd21c 100644 --- a/services/app-web/src/components/Header/Header.tsx +++ b/services/app-web/src/components/Header/Header.tsx @@ -10,12 +10,9 @@ import { Logo } from '../Logo' import styles from './Header.module.scss' import { PageHeadingRow } from './PageHeadingRow/PageHeadingRow' import { UserLoginInfo } from './UserLoginInfo/UserLoginInfo' -import { recordJSException } from '../../otelHelpers' -import { ErrorAlertSignIn } from '../ErrorAlert' export type HeaderProps = { authMode: AuthModeType - setAlert?: React.Dispatch disableLogin?: boolean } @@ -24,25 +21,16 @@ export type HeaderProps = { */ export const Header = ({ authMode, - setAlert, disableLogin = false, }: HeaderProps): React.ReactElement => { const { logout, loggedInUser, loginStatus } = useAuth() const { heading } = usePage() const { currentRoute: route } = useCurrentRoute() - const handleLogout = ( + const handleLogout = async ( e: React.MouseEvent ) => { - if (!logout) { - console.info('Something went wrong ', e) - return - } - - logout({ sessionTimeout: false }).catch((e) => { - recordJSException(`Error with logout: ${e}`) - setAlert && setAlert() - }) + await logout({ sessionTimeout: false }) } return ( diff --git a/services/app-web/src/contexts/AuthContext.tsx b/services/app-web/src/contexts/AuthContext.tsx index 026fd4c5d2..388b061891 100644 --- a/services/app-web/src/contexts/AuthContext.tsx +++ b/services/app-web/src/contexts/AuthContext.tsx @@ -21,11 +21,19 @@ export const MODAL_COUNTDOWN_DURATION = 2 * 60 // session expiration modal count type AuthContextType = { /* See docs/AuthContext.md for an explanation of some of these variables */ - checkAuth: () => Promise | Error> // manually refetch to confirm auth status, logout if not authenticated + checkAuth: ( + failureRedirect?: string + ) => Promise | Error> checkIfSessionsIsAboutToExpire: () => void loggedInUser: UserType | undefined loginStatus: LoginStatusType - logout: ({ sessionTimeout }: { sessionTimeout: boolean }) => Promise // sessionTimeout true when logout is due to session ending + logout: ({ + sessionTimeout, + redirectPath, + }: { + sessionTimeout: boolean + redirectPath?: string + }) => Promise logoutCountdownDuration: number sessionExpirationTime: MutableRefObject sessionIsExpiring: boolean @@ -163,11 +171,25 @@ function AuthProvider({ } } - const checkAuth = async () => { + /* + Refetches current user and confirms authentication + @param {failureRedirectPath} passed through to logout which is called on certain checkAuth failures + + Use this function to reconfirm the user is logged in. Also used in CognitoLogin + */ + const checkAuth: AuthContextType['checkAuth'] = async ( + failureRedirectPath = '/' + ) => { try { return await refetch() } catch (e) { - await logout({ sessionTimeout: true }) + // if we fail auth at a time we expected logged in user, the session may have timed out. Logout fully to reflect that and force Reat state to update + if (loggedInUser) { + await logout({ + sessionTimeout: true, + redirectPath: failureRedirectPath, + }) + } return new Error(e) } } @@ -211,20 +233,35 @@ function AuthProvider({ } } - const logout = async ({ sessionTimeout }: { sessionTimeout: boolean }) => { + /* + Close user session and handle redirect afterward + + @param {sessionTimeout} will pass along a URL query param to display session expired alert + @param {redirectPath} optionally changes redirect path on logout - useful for cognito and local login\ + + Logout is called when user clicks to logout from header or session expiration modal + Also called in the background with session times out + */ + const logout: AuthContextType['logout'] = async ({ + sessionTimeout, + redirectPath = '/', + }) => { const realLogout = authMode === 'LOCAL' ? logoutLocalUser : cognitoSignOut + + updateSessionExpirationState(false) + try { await realLogout() if (sessionTimeout) { - window.location.href = '/?session-timeout=true' + window.location.href = `${redirectPath}?session-timeout=true'` } else { - window.location.href = '/' + window.location.href = redirectPath } return } catch (e) { recordJSException(new Error(`Logout Failed. ${JSON.stringify(e)}`)) - window.location.href = '/' + window.location.href = redirectPath return } } diff --git a/services/app-web/src/pages/Auth/Auth.test.tsx b/services/app-web/src/pages/Auth/Auth.test.tsx index 1ddcb53beb..51cfc9a26f 100644 --- a/services/app-web/src/pages/Auth/Auth.test.tsx +++ b/services/app-web/src/pages/Auth/Auth.test.tsx @@ -13,7 +13,7 @@ import { import { CognitoLogin } from './CognitoLogin' import { LocalLogin } from '../../localAuth' import { fetchCurrentUserMock } from '../../testHelpers/apolloMocks' -/* +/* This file should only have basic user flows for auth. Form and implementation details are tested at the component level. */ @@ -243,7 +243,7 @@ describe('Auth', () => { }) }) - it('when login fails, stay on page and display error alert', async () => { + it('when login fails, kick back to landing page', async () => { let testLocation: Location renderWithProviders( @@ -266,7 +266,7 @@ describe('Auth', () => { await userClickByTestId(screen, 'TophButton') await waitFor(() => { - expect(testLocation.pathname).toBe('/auth') + expect(testLocation.pathname).toBe('/') }) }) }) diff --git a/services/app-web/src/pages/Auth/Login.tsx b/services/app-web/src/pages/Auth/Login.tsx index b1a812e305..8c374983b6 100644 --- a/services/app-web/src/pages/Auth/Login.tsx +++ b/services/app-web/src/pages/Auth/Login.tsx @@ -58,17 +58,19 @@ export function Login({ defaultEmail }: Props): React.ReactElement { setShowFormAlert(true) console.info('Error', result.message) } else { - try { - await checkAuth() - } catch (e) { - console.info('UNEXPECTED NOT LOGGED IN AFTER LOGGIN', e) + const result = await checkAuth('/auth') + if (result instanceof Error) { + console.error( + 'UNEXPECTED - NOT AUTHENTICATED BUT COGNITO PASSWORD LOGIN HAS RESOLVED WITHOUT ERROR', + result + ) setShowFormAlert(true) } - navigate('/') } } catch (err) { - console.info('Unexpected error signing in:', err) + setShowFormAlert(true) + console.info('Sign in error:', err) } } diff --git a/services/app-web/src/pages/Landing/Landing.test.tsx b/services/app-web/src/pages/Landing/Landing.test.tsx index 46aa0d635f..1e62f66a13 100644 --- a/services/app-web/src/pages/Landing/Landing.test.tsx +++ b/services/app-web/src/pages/Landing/Landing.test.tsx @@ -34,4 +34,26 @@ describe('Landing', () => { screen.queryByText(/You have been logged out due to inactivity/) ).toBeNull() }) + + it('displays sign in error when query parameter included', async () => { + ldUseClientSpy({ 'site-under-maintenance-banner': false }) + renderWithProviders(, { + routerProvider: { route: '/?auth-error' }, + }) + expect( + screen.queryByRole('heading', { name: 'Sign in error' }) + ).toBeNull() + }) + + it('does not display sign in error by default', async () => { + ldUseClientSpy({ 'site-under-maintenance-banner': false }) + renderWithProviders(, { + routerProvider: { route: '/' }, + }) + expect( + screen.queryByRole('heading', { + name: 'Sign in error', + }) + ).toBeNull() + }) }) diff --git a/services/app-web/src/pages/Landing/Landing.tsx b/services/app-web/src/pages/Landing/Landing.tsx index 6999387dff..ad6bd42972 100644 --- a/services/app-web/src/pages/Landing/Landing.tsx +++ b/services/app-web/src/pages/Landing/Landing.tsx @@ -8,9 +8,12 @@ import { ErrorAlertSiteUnavailable, ErrorAlertSessionExpired, ErrorAlertScheduledMaintenance, + ErrorAlertSignIn, } from '../../components' -function maintenanceBannerForVariation(flag: string): React.ReactNode { +function maintenanceBannerForVariation( + flag: string +): React.ReactElement | undefined { if (flag === 'UNSCHEDULED') { return } else if (flag === 'SCHEDULED') { @@ -27,7 +30,7 @@ export const Landing = (): React.ReactElement => { featureFlags.SITE_UNDER_MAINTENANCE_BANNER.defaultValue ) - const maybeMaintenaceBanner = maintenanceBannerForVariation( + const maintainenceBanner = maintenanceBannerForVariation( siteUnderMaintenanceBannerFlag ) @@ -35,14 +38,21 @@ export const Landing = (): React.ReactElement => { 'session-timeout' ) + const redirectFromAuthError = new URLSearchParams(location.search).get( + 'auth-error' + ) + return ( <>
- {maybeMaintenaceBanner} - {redirectFromSessionTimeout && !maybeMaintenaceBanner && ( + {maintainenceBanner} + {redirectFromSessionTimeout && !maintainenceBanner && ( )} + {redirectFromAuthError && !maintainenceBanner && ( + + )}
diff --git a/services/app-web/src/pages/Wrapper/AuthenticatedRouteWrapper.tsx b/services/app-web/src/pages/Wrapper/AuthenticatedRouteWrapper.tsx index 9a1e991145..ae34d9abfd 100644 --- a/services/app-web/src/pages/Wrapper/AuthenticatedRouteWrapper.tsx +++ b/services/app-web/src/pages/Wrapper/AuthenticatedRouteWrapper.tsx @@ -36,15 +36,12 @@ export const AuthenticatedRouteWrapper = ({ const logoutSession = useCallback( (forcedSessionSignout: boolean) => { - updateSessionExpirationState(false) - if (logout) { - logout({ sessionTimeout: forcedSessionSignout }).catch((e) => { - recordJSException(`Error with logout: ${e}`) - setAlert && setAlert() - }) - } + logout({ sessionTimeout: forcedSessionSignout }).catch((e) => { + recordJSException(`Error with logout: ${e}`) + setAlert && setAlert() + }) }, - [logout, setAlert, updateSessionExpirationState] + [logout, setAlert] ) const resetSessionTimeout = () => {