Skip to content

Commit

Permalink
fix: Remove session expiry tracker (#2988)
Browse files Browse the repository at this point in the history
  • Loading branch information
spalmurray-codecov authored Jul 8, 2024
1 parent 2664937 commit bfe41cb
Show file tree
Hide file tree
Showing 15 changed files with 57 additions and 313 deletions.
5 changes: 2 additions & 3 deletions src/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,10 @@ const defaultConfig = {
GH_APP: 'codecov',
}

export const LOCAL_STORAGE_SESSION_EXPIRED_KEY = 'expired-session'
// To be removed after we're satisfied session_expiry cookie cleanup is complete
export const LOCAL_STORAGE_SESION_EXPIRED_KEY = 'expired_session'
export const LOCAL_STORAGE_SESSION_TRACKING_KEY = 'tracking-session-expiry'

export const COOKIE_SESSION_EXPIRY = 'session_expiry'
export const COOKIE_SESSION_ID = 'sessionid'

export function removeReactAppPrefix(obj) {
// in .env file, the variable must start with REACT_APP_
Expand Down
18 changes: 18 additions & 0 deletions src/index.tsx
Original file line number Diff line number Diff line change
@@ -1,14 +1,22 @@
import * as Sentry from '@sentry/react'
import { QueryClient, QueryClientProvider } from '@tanstack/react-query'
import { createBrowserHistory } from 'history'
import Cookies from 'js-cookie'
import React from 'react'
import { createRoot } from 'react-dom/client'
import ReactModal from 'react-modal'
import { Router } from 'react-router-dom'
import { CompatRouter } from 'react-router-dom-v5-compat'

import config, {
COOKIE_SESSION_EXPIRY,
LOCAL_STORAGE_SESION_EXPIRED_KEY,
LOCAL_STORAGE_SESSION_TRACKING_KEY,
} from 'config'

import ErrorBoundary from 'layouts/shared/ErrorBoundary'
import { withFeatureFlagProvider } from 'shared/featureFlags'
import { metrics } from 'shared/utils/metrics'

import App from './App'
import './globals.css'
Expand Down Expand Up @@ -43,6 +51,16 @@ const queryClient = new QueryClient({
},
})

if (Cookies.get(COOKIE_SESSION_EXPIRY)) {
localStorage.removeItem(LOCAL_STORAGE_SESION_EXPIRED_KEY)
localStorage.removeItem(LOCAL_STORAGE_SESSION_TRACKING_KEY)
Cookies.remove(COOKIE_SESSION_EXPIRY, {
path: '/',
domain: `.${(config.BASE_URL as string).split('/')[2]}`,
}) // Remove http(s)://
metrics.increment('old_session_expiry.cleanup')
}

const domNode = document.getElementById('root')

if (!domNode) {
Expand Down
2 changes: 0 additions & 2 deletions src/layouts/BaseLayout/BaseLayout.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import { useFlags } from 'shared/featureFlags'
import GlobalBanners from 'shared/GlobalBanners'
import GlobalTopBanners from 'shared/GlobalTopBanners'
import LoadingLogo from 'ui/LoadingLogo'
import SessionExpiryTracker from 'ui/SessionExpiryTracker'

import { useUserAccessGate } from './hooks/useUserAccessGate'

Expand Down Expand Up @@ -74,7 +73,6 @@ function BaseLayout({ children }) {

return (
<>
<SessionExpiryTracker />
{isFullExperience ? (
<>
{newHeader ? <Header /> : <OldHeader />}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,12 @@ import { QueryClient, QueryClientProvider } from '@tanstack/react-query'
import { render, screen } from '@testing-library/react'
import { MemoryRouter, Route, useLocation } from 'react-router-dom'

import { LOCAL_STORAGE_SESSION_EXPIRED_KEY } from 'config'

import EnterpriseLoginLayout from './EnterpriseLoginLayout'

jest.mock('./Header', () => () => 'Header')
jest.mock('layouts/Footer', () => () => 'Footer')
jest.mock('shared/GlobalBanners', () => () => 'GlobalBanners')
jest.mock('layouts/ToastNotifications', () => () => 'ToastNotifications')
jest.mock('ui/SessionExpiryTracker', () => () => 'SessionExpiryTracker')

jest.mock('react-router-dom', () => ({
...jest.requireActual('react-router-dom'),
Expand Down Expand Up @@ -42,7 +39,7 @@ describe('EnterpriseLoginLayout', () => {
console.error = () => {}
})
beforeEach(() => {
mockedUseLocation.mockReturnValue({ search: [] })
mockedUseLocation.mockReturnValue({ search: [''] })
})

afterAll(() => {
Expand Down Expand Up @@ -92,8 +89,7 @@ describe('EnterpriseLoginLayout', () => {
})

it('renders the session expired banner if session is expired', () => {
localStorage.setItem(LOCAL_STORAGE_SESSION_EXPIRED_KEY, 'true')

mockedUseLocation.mockReturnValue({ search: ['expired'] })
render(<>children</>, { wrapper })

const session = screen.getByText(/Your session has expired/)
Expand Down
9 changes: 1 addition & 8 deletions src/layouts/EnterpriseLoginLayout/EnterpriseLoginLayout.tsx
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
import { Suspense } from 'react'
import { useLocation } from 'react-router-dom'

import { LOCAL_STORAGE_SESSION_EXPIRED_KEY } from 'config'

import Footer from 'layouts/Footer'
import ErrorBoundary from 'layouts/shared/ErrorBoundary'
import NetworkErrorBoundary from 'layouts/shared/NetworkErrorBoundary'
Expand All @@ -22,15 +20,10 @@ const FullPageLoader = () => (
function EnterpriseLoginLayout({ children }: { children: React.ReactNode }) {
const location = useLocation()

const showExpiryBanner = localStorage.getItem(
LOCAL_STORAGE_SESSION_EXPIRED_KEY
)
return (
<>
<Header />
{(location.search.includes('expired') || showExpiryBanner) && (
<SessionExpiredBanner />
)}
{location.search.includes('expired') && <SessionExpiredBanner />}
<Suspense fallback={<FullPageLoader />}>
<ErrorBoundary sentryScopes={[['layout', 'base']]}>
<NetworkErrorBoundary>
Expand Down
45 changes: 14 additions & 31 deletions src/layouts/Header/components/UserDropdown/UserDropdown.spec.tsx
Original file line number Diff line number Diff line change
@@ -1,15 +1,11 @@
import { QueryClient, QueryClientProvider } from '@tanstack/react-query'
import { render, screen, waitFor } from '@testing-library/react'
import userEvent from '@testing-library/user-event'
import Cookies from 'js-cookie'
import { graphql, rest } from 'msw'
import { setupServer } from 'msw/node'
import { MemoryRouter, Route, Switch } from 'react-router-dom'
import { MemoryRouter, Route, Switch, useLocation } from 'react-router-dom'

import config, {
COOKIE_SESSION_EXPIRY,
LOCAL_STORAGE_SESSION_TRACKING_KEY,
} from 'config'
import config from 'config'

import { useImage } from 'services/image'

Expand Down Expand Up @@ -67,6 +63,7 @@ const queryClient = new QueryClient({
defaultOptions: { queries: { retry: false } },
})
const server = setupServer()
let testLocation: ReturnType<typeof useLocation>

const wrapper: (initialEntries?: string) => React.FC<React.PropsWithChildren> =
(initialEntries = '/gh') =>
Expand All @@ -77,6 +74,13 @@ const wrapper: (initialEntries?: string) => React.FC<React.PropsWithChildren> =
<Switch>
<Route path="/:provider" exact>
{children}
<Route
path="*"
render={({ location }) => {
testLocation = location
return null
}}
/>
</Route>
</Switch>
</MemoryRouter>
Expand Down Expand Up @@ -104,10 +108,6 @@ describe('UserDropdown', () => {
})
config.IS_SELF_HOSTED = selfHosted
config.API_URL = ''
const mockRemoveItem = jest.spyOn(
window.localStorage.__proto__,
'removeItem'
)

server.use(
rest.post('/logout', (req, res, ctx) => res(ctx.status(205))),
Expand All @@ -118,7 +118,6 @@ describe('UserDropdown', () => {

return {
user: userEvent.setup(),
mockRemoveItem,
}
}

Expand Down Expand Up @@ -172,10 +171,9 @@ describe('UserDropdown', () => {
})

it('handles sign out', async () => {
const { user, mockRemoveItem } = setup()
const { user } = setup()

jest.spyOn(console, 'error').mockImplementation()
const removeSpy = jest.spyOn(Cookies, 'remove').mockReturnValue()
render(<UserDropdown />, {
wrapper: wrapper(),
})
Expand All @@ -187,14 +185,7 @@ describe('UserDropdown', () => {
expect(button).toBeVisible()
await user.click(button)

await waitFor(() =>
expect(mockRemoveItem).toHaveBeenCalledWith(
LOCAL_STORAGE_SESSION_TRACKING_KEY
)
)
await waitFor(() =>
expect(removeSpy).toHaveBeenCalledWith(COOKIE_SESSION_EXPIRY)
)
await waitFor(() => expect(testLocation.pathname).toBe('/login'))
})

it('shows manage app access link', async () => {
Expand Down Expand Up @@ -253,10 +244,9 @@ describe('UserDropdown', () => {
})

it('handles sign out', async () => {
const { user, mockRemoveItem } = setup()
const { user } = setup()

jest.spyOn(console, 'error').mockImplementation()
const removeSpy = jest.spyOn(Cookies, 'remove').mockReturnValue()
render(<UserDropdown />, {
wrapper: wrapper(),
})
Expand All @@ -268,14 +258,7 @@ describe('UserDropdown', () => {
expect(button).toBeVisible()
await user.click(button)

await waitFor(() =>
expect(mockRemoveItem).toHaveBeenCalledWith(
LOCAL_STORAGE_SESSION_TRACKING_KEY
)
)
await waitFor(() =>
expect(removeSpy).toHaveBeenCalledWith(COOKIE_SESSION_EXPIRY)
)
await waitFor(() => expect(testLocation.pathname).toBe('/login'))
})

it('does not show manage app access link', async () => {
Expand Down
8 changes: 1 addition & 7 deletions src/layouts/Header/components/UserDropdown/UserDropdown.tsx
Original file line number Diff line number Diff line change
@@ -1,11 +1,7 @@
import { useSelect } from 'downshift'
import Cookies from 'js-cookie'
import { useHistory, useParams } from 'react-router-dom'

import config, {
COOKIE_SESSION_EXPIRY,
LOCAL_STORAGE_SESSION_TRACKING_KEY,
} from 'config'
import config from 'config'

import { useUser } from 'services/user'
import { cn } from 'shared/utils/cn'
Expand Down Expand Up @@ -55,8 +51,6 @@ function UserDropdown() {
method: 'POST',
credentials: 'include',
})
localStorage.removeItem(LOCAL_STORAGE_SESSION_TRACKING_KEY)
Cookies.remove(COOKIE_SESSION_EXPIRY)
history.replace('/login')
}

Expand Down
12 changes: 0 additions & 12 deletions src/layouts/LoginLayout/LoginLayout.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -99,18 +99,6 @@ describe('LoginLayout', () => {
})

describe('when session is expired', () => {
it('renders the expiry banner when local storage prop set', async () => {
setup()

jest
.spyOn(window.localStorage.__proto__, 'getItem')
.mockReturnValue('true')

render(<LoginLayout>child content</LoginLayout>, { wrapper: wrapper() })
await waitFor(() => {
expect(screen.getByText(/Your session has expired/)).toBeInTheDocument()
})
})
it('renders the expiry banner when query param set', async () => {
setup()

Expand Down
9 changes: 1 addition & 8 deletions src/layouts/LoginLayout/LoginLayout.tsx
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
import { Suspense } from 'react'
import { useLocation } from 'react-router-dom'

import { LOCAL_STORAGE_SESSION_EXPIRED_KEY } from 'config'

import Footer from 'layouts/Footer'
import Header from 'layouts/Header'
import OldHeader from 'layouts/OldHeader'
Expand All @@ -23,14 +21,9 @@ const LoginLayout: React.FC<React.PropsWithChildren> = ({ children }) => {
newHeader: false,
})

const showExpiryBanner = localStorage.getItem(
LOCAL_STORAGE_SESSION_EXPIRED_KEY
)
return (
<>
{(location.search.includes('expired') || showExpiryBanner) && (
<SessionExpiredBanner />
)}
{location.search.includes('expired') && <SessionExpiredBanner />}
{newHeader ? <Header /> : <OldHeader />}
<Suspense fallback={<FullPageLoader />}>
<main className="container mb-8 mt-2 flex grow flex-col gap-2 md:p-0">
Expand Down
Loading

0 comments on commit bfe41cb

Please sign in to comment.