diff --git a/scripts/config/vale/styles/config/vocabularies/words/accept.txt b/scripts/config/vale/styles/config/vocabularies/words/accept.txt index 78bfe5403..9af6eacc8 100644 --- a/scripts/config/vale/styles/config/vocabularies/words/accept.txt +++ b/scripts/config/vale/styles/config/vocabularies/words/accept.txt @@ -3,6 +3,7 @@ bot (?i)CLI config Cyber +cybersecurity [Dd]ependabot [Dd]ev Dockerfile diff --git a/src/app/_components/inactivity/InactivityDialog.test.tsx b/src/app/_components/inactivity/InactivityDialog.test.tsx index 003ccb97a..42b458286 100644 --- a/src/app/_components/inactivity/InactivityDialog.test.tsx +++ b/src/app/_components/inactivity/InactivityDialog.test.tsx @@ -25,8 +25,11 @@ jest.mock("next/navigation", () => ({ usePathname: jest.fn(() => mockUrlPath), })); +jest.mock("@src/utils/auth/user-logout", () => ({ + userLogout: jest.fn(), +})); + jest.mock("@src/utils/auth/inactivity-timer"); -jest.mock("@src/utils/auth/user-logout"); let idleSession = false; let timedOutSession = false; diff --git a/src/app/our-policies/cookies-policy/CookiesTable.test.tsx b/src/app/our-policies/cookies-policy/CookiesTable.test.tsx index 547edcc57..92e0773fe 100644 --- a/src/app/our-policies/cookies-policy/CookiesTable.test.tsx +++ b/src/app/our-policies/cookies-policy/CookiesTable.test.tsx @@ -18,34 +18,47 @@ describe("CookiesTable component", () => { const rowHeader1: HTMLElement = screen.getByRole("rowheader", { name: "__Host-authjs.csrf-token" }); const rowHeader2: HTMLElement = screen.getByRole("rowheader", { name: "__Secure-authjs.callback-url" }); const rowHeader3: HTMLElement = screen.getByRole("rowheader", { name: "__Secure-authjs.session-token" }); + const rowHeader4: HTMLElement = screen.getByRole("rowheader", { name: "__Host-Http-session-id" }); + const rowHeader5: HTMLElement = screen.getByRole("rowheader", { name: "__Secure-signout" }); expect(rowHeader1).toBeVisible(); expect(rowHeader2).toBeVisible(); expect(rowHeader3).toBeVisible(); + expect(rowHeader4).toBeVisible(); + expect(rowHeader5).toBeVisible(); }); it("displays table with correct cell values", () => { render(); - const cell1: HTMLElement = screen.getByRole("cell", { + + const csrfCookieText: HTMLElement = screen.getByRole("cell", { name: "Helps keep the site secure by preventing cross-site request forgery (CSRF) attacks", }); - const cell2: HTMLElement = screen.getByRole("cell", { + const redirectUrlCookieText: HTMLElement = screen.getByRole("cell", { name: "After a successful login, this stores the URL that you are redirected to", }); - const cell3: HTMLElement = screen.getByRole("cell", { + const encryptedSessionTokenCookieText: HTMLElement = screen.getByRole("cell", { name: "Stores information in an encrypted format that allows us to communicate with other services", }); - const cell4: HTMLElement = screen.getByRole("cell", { + const sessionIdCookieText: HTMLElement = screen.getByRole("cell", { name: "Stores a unique, randomly generated session ID used in operational logs to help our IT support team investigate issues", }); - const cells5and6: HTMLElement[] = screen.getAllByRole("cell", { name: "When you close the browser" }); - const cells7and8: HTMLElement[] = screen.getAllByRole("cell", { name: "After 1 hour" }); - - expect(cell1).toBeVisible(); - expect(cell2).toBeVisible(); - expect(cell3).toBeVisible(); - expect(cell4).toBeVisible(); - expect(cells5and6.length).toBe(2); - expect(cells7and8.length).toBe(2); + const signoutCookieText: HTMLElement = screen.getByRole("cell", { + name: "Used when you sign out or after a period of inactivity. It temporarily stores the session ID to help securely end your session and keep your information secure.", + }); + + const onBrowserCloseCookieTime: HTMLElement[] = screen.getAllByRole("cell", { name: "When you close the browser" }); + const after1hourCookieTime: HTMLElement[] = screen.getAllByRole("cell", { name: "After 1 hour" }); + const after30sCookieTime: HTMLElement[] = screen.getAllByRole("cell", { name: "After 30 seconds" }); + + expect(csrfCookieText).toBeVisible(); + expect(redirectUrlCookieText).toBeVisible(); + expect(encryptedSessionTokenCookieText).toBeVisible(); + expect(sessionIdCookieText).toBeVisible(); + expect(signoutCookieText).toBeVisible(); + + expect(onBrowserCloseCookieTime.length).toBe(2); + expect(after1hourCookieTime.length).toBe(2); + expect(after30sCookieTime.length).toBe(1); }); }); diff --git a/src/app/our-policies/cookies-policy/CookiesTable.tsx b/src/app/our-policies/cookies-policy/CookiesTable.tsx index 39e345f24..eb313744c 100644 --- a/src/app/our-policies/cookies-policy/CookiesTable.tsx +++ b/src/app/our-policies/cookies-policy/CookiesTable.tsx @@ -54,6 +54,16 @@ const CookiesTable = (): JSX.Element => { After 1 hour + + + __Secure-signout + + + Used when you sign out or after a period of inactivity. It temporarily stores the session ID to help + securely end your session and keep your information secure. + + After 30 seconds + ); diff --git a/src/utils/auth/callbacks/get-token.test.ts b/src/utils/auth/callbacks/get-token.test.ts index d25ef6193..3857a40dd 100644 --- a/src/utils/auth/callbacks/get-token.test.ts +++ b/src/utils/auth/callbacks/get-token.test.ts @@ -4,10 +4,14 @@ import { getOrRefreshApimCredentials } from "@src/utils/auth/apim/get-or-refresh import { getToken } from "@src/utils/auth/callbacks/get-token"; import { MaxAgeInSeconds } from "@src/utils/auth/types"; import config from "@src/utils/config"; +import { SESSION_ID_COOKIE_NAME, SIGNOUT_FLAG_COOKIE_NAME } from "@src/utils/constants"; import { ConfigMock, configBuilder } from "@test-data/config/builders"; import { jwtDecode } from "jwt-decode"; import { Account, Profile } from "next-auth"; import { JWT } from "next-auth/jwt"; +import { RequestCookie } from "next/dist/compiled/@edge-runtime/cookies"; +import { ReadonlyRequestCookies } from "next/dist/server/web/spec-extension/adapters/request-cookies"; +import { cookies } from "next/headers"; jest.mock("@project/auth", () => ({ auth: jest.fn(), @@ -20,6 +24,10 @@ jest.mock("@src/utils/auth/apim/get-or-refresh-apim-credentials", () => ({ jest.mock("jwt-decode"); jest.mock("sanitize-data", () => ({ sanitize: jest.fn() })); jest.mock("@src/utils/config"); +jest.mock("next/headers", () => ({ + cookies: jest.fn(), + headers: jest.fn(), +})); describe("getToken", () => { const mockedConfig = config as ConfigMock; @@ -55,6 +63,16 @@ describe("getToken", () => { jest.useFakeTimers().setSystemTime(nowInSeconds * 1000); process.env.NEXT_RUNTIME = "nodejs"; + const fakeRequestCookies: ReadonlyRequestCookies = { + get(name: string): RequestCookie { + return { + name: `fake-${name}-name`, + value: `fake-${name}-value`, + }; + }, + } as ReadonlyRequestCookies; + (cookies as jest.Mock).mockResolvedValue(fakeRequestCookies); + (jwtDecode as jest.Mock).mockReturnValue({ jti: "jti_test", }); @@ -171,6 +189,42 @@ describe("getToken", () => { maxAgeInSeconds, ); }); + + it.each<{ + signoutCookieValue: string; + sessionIdCookieValue: string; + shouldBeNull: boolean; + description: string; + }>([ + { + signoutCookieValue: "test-session-id", + sessionIdCookieValue: "test-session-id", + shouldBeNull: true, + description: "should return null when signout cookie matches current session id", + }, + { + signoutCookieValue: "old-session-id", + sessionIdCookieValue: "current-session-id", + shouldBeNull: false, + description: "should return token when signout cookie does not match current session id", + }, + ])("$description", async ({ signoutCookieValue, sessionIdCookieValue, shouldBeNull }) => { + const fakeRequestCookies: ReadonlyRequestCookies = { + get(name: string): RequestCookie | undefined { + if (name === SIGNOUT_FLAG_COOKIE_NAME) return { name: SIGNOUT_FLAG_COOKIE_NAME, value: signoutCookieValue }; + if (name === SESSION_ID_COOKIE_NAME) return { name: SESSION_ID_COOKIE_NAME, value: sessionIdCookieValue }; + return { name: `fake-${name}-name`, value: `fake-${name}-value` }; + }, + } as ReadonlyRequestCookies; + (cookies as jest.Mock).mockResolvedValue(fakeRequestCookies); + + const token = { apim: {}, nhs_login: { id_token: "id-token" } } as JWT; + const maxAgeInSeconds = 600 as MaxAgeInSeconds; + + const result = await getToken(token, account, profile, maxAgeInSeconds); + + expect(result === null).toBe(shouldBeNull); + }); }); describe("when AUTH APIM is not available", () => { diff --git a/src/utils/auth/callbacks/get-token.ts b/src/utils/auth/callbacks/get-token.ts index a3adca09f..93e4b255f 100644 --- a/src/utils/auth/callbacks/get-token.ts +++ b/src/utils/auth/callbacks/get-token.ts @@ -4,9 +4,11 @@ import { NhsNumber } from "@src/models/vaccine"; import { getOrRefreshApimCredentials } from "@src/utils/auth/apim/get-or-refresh-apim-credentials"; import { ApimAccessCredentials } from "@src/utils/auth/apim/types"; import { BirthDate, IdToken, MaxAgeInSeconds, NowInSeconds } from "@src/utils/auth/types"; +import { SESSION_ID_COOKIE_NAME, SIGNOUT_FLAG_COOKIE_NAME } from "@src/utils/constants"; import { logger } from "@src/utils/logger"; import { Account, Profile } from "next-auth"; import { JWT } from "next-auth/jwt"; +import { cookies } from "next/headers"; import { Logger } from "pino"; const log: Logger = logger.child({ module: "utils-auth-callbacks-get-token" }); @@ -29,8 +31,16 @@ const getToken = async ( return null; } + const requestCookies = await cookies(); const nowInSeconds = Math.floor(Date.now() / 1000); + const signOutFlagValue = requestCookies?.get(SIGNOUT_FLAG_COOKIE_NAME)?.value; + const currentSessionId = requestCookies?.get(SESSION_ID_COOKIE_NAME)?.value; + if (signOutFlagValue && currentSessionId && signOutFlagValue === currentSessionId) { + log.info("getToken: User has recently been signed out. Returning null"); + return null; + } + // Maximum age reached scenario: invalidate session after fixedExpiry if (token.fixedExpiry && nowInSeconds >= token.fixedExpiry) { log.info("getToken: Token has reached fixedExpiry time, or session has reached the max age. Returning null"); diff --git a/src/utils/auth/setSignOutFlagCookie.test.ts b/src/utils/auth/setSignOutFlagCookie.test.ts new file mode 100644 index 000000000..a988e0d42 --- /dev/null +++ b/src/utils/auth/setSignOutFlagCookie.test.ts @@ -0,0 +1,66 @@ +import setSignOutFlagCookie from "@src/utils/auth/setSignOutFlagCookie"; +import { SESSION_ID_COOKIE_NAME, SIGNOUT_FLAG_COOKIE_NAME } from "@src/utils/constants"; + +const mockSetCookie = jest.fn(); +const mockGetCookie = jest.fn(); + +jest.mock("@src/utils/requestScopedStorageWrapper", () => ({ + requestScopedStorageWrapper: jest.fn((fn) => fn()), +})); + +jest.mock("@src/utils/logger", () => ({ + mockWarn: jest.fn(), + logger: { + child: jest.fn(() => { + const mockedLoggerModule = jest.requireMock("@src/utils/logger") as { mockWarn: jest.Mock }; + return { warn: mockedLoggerModule.mockWarn }; + }), + }, +})); + +jest.mock("next/headers", () => ({ + cookies: jest.fn(() => ({ + get: mockGetCookie, + set: mockSetCookie, + })), + headers: jest.fn(), +})); + +describe("setSignOutFlagCookie", () => { + const mockSessionId = "session-id-123"; + const getLoggerWarnMock = (): jest.Mock => { + const mockedLoggerModule = jest.requireMock("@src/utils/logger") as { mockWarn: jest.Mock }; + return mockedLoggerModule.mockWarn; + }; + + beforeEach(() => { + jest.clearAllMocks(); + mockGetCookie.mockImplementation((name: string) => { + if (name === SESSION_ID_COOKIE_NAME) return { value: mockSessionId }; + return undefined; + }); + }); + + it("should set signout cookie with the current session id", async () => { + await setSignOutFlagCookie(); + + const expectedCookieTimeoutSeconds = 30; + expect(mockSetCookie).toHaveBeenCalledWith(SIGNOUT_FLAG_COOKIE_NAME, mockSessionId, { + maxAge: expectedCookieTimeoutSeconds, + secure: true, + httpOnly: true, + sameSite: "lax", + path: "/", + }); + }); + + it("should not set signout cookie if session id is missing", async () => { + mockGetCookie.mockReturnValue(undefined); + + await setSignOutFlagCookie(); + + expect(mockSetCookie).not.toHaveBeenCalled(); + const warnMock = getLoggerWarnMock(); + expect(warnMock).toHaveBeenCalledWith("Session ID missing, skipping signout cookie"); + }); +}); diff --git a/src/utils/auth/setSignOutFlagCookie.ts b/src/utils/auth/setSignOutFlagCookie.ts new file mode 100644 index 000000000..cf13b9203 --- /dev/null +++ b/src/utils/auth/setSignOutFlagCookie.ts @@ -0,0 +1,31 @@ +"use server"; + +import { SESSION_ID_COOKIE_NAME, SIGNOUT_FLAG_COOKIE_NAME } from "@src/utils/constants"; +import { logger } from "@src/utils/logger"; +import { requestScopedStorageWrapper } from "@src/utils/requestScopedStorageWrapper"; +import { cookies } from "next/headers"; + +const log = logger.child({ module: "utils-auth-setSignOutFlagCookie" }); + +const setSignOutFlagCookie = async () => { + return requestScopedStorageWrapper(setSignOutFlagCookieAction); +}; + +const setSignOutFlagCookieAction = async () => { + const SIGN_OUT_FLAG_COOKIE_MAX_AGE_SECONDS = 30; + const cookieStore = await cookies(); + const currentSessionId = cookieStore.get(SESSION_ID_COOKIE_NAME)?.value ?? ""; + if (!currentSessionId) { + log.warn("Session ID missing, skipping signout cookie"); + return; + } + cookieStore.set(SIGNOUT_FLAG_COOKIE_NAME, currentSessionId, { + secure: true, + httpOnly: true, + sameSite: "lax", + path: "/", + maxAge: SIGN_OUT_FLAG_COOKIE_MAX_AGE_SECONDS, + }); +}; + +export default setSignOutFlagCookie; diff --git a/src/utils/auth/user-logout.test.ts b/src/utils/auth/user-logout.test.ts index cbaa85e5b..245d225c4 100644 --- a/src/utils/auth/user-logout.test.ts +++ b/src/utils/auth/user-logout.test.ts @@ -1,11 +1,14 @@ import { SESSION_LOGOUT_ROUTE } from "@src/app/session-logout/constants"; import { SESSION_TIMEOUT_ROUTE } from "@src/app/session-timeout/constants"; +import setSignOutFlagCookie from "@src/utils/auth/setSignOutFlagCookie"; import { userLogout } from "@src/utils/auth/user-logout"; import { signOut } from "next-auth/react"; jest.mock("next-auth/react", () => ({ signOut: jest.fn(), })); +jest.mock("@src/utils/auth/setSignOutFlagCookie"); +jest.mock("sanitize-data", () => ({ sanitize: jest.fn() })); describe("user-logout", () => { it("should call signOut to be redirected to logout page by default", async () => { @@ -25,4 +28,10 @@ describe("user-logout", () => { redirectTo: SESSION_TIMEOUT_ROUTE, }); }); + + it("should setSignOutFlagCookie to prevent race condition with concurrent getSession calls", async () => { + await userLogout(true); + + expect(setSignOutFlagCookie).toHaveBeenCalled(); + }); }); diff --git a/src/utils/auth/user-logout.ts b/src/utils/auth/user-logout.ts index d3d972852..6b42a6092 100644 --- a/src/utils/auth/user-logout.ts +++ b/src/utils/auth/user-logout.ts @@ -2,9 +2,11 @@ import { SESSION_LOGOUT_ROUTE } from "@src/app/session-logout/constants"; import { SESSION_TIMEOUT_ROUTE } from "@src/app/session-timeout/constants"; +import setSignOutFlagCookie from "@src/utils/auth/setSignOutFlagCookie"; import { signOut } from "next-auth/react"; const userLogout = async (reasonTimeout: boolean = false) => { + await setSignOutFlagCookie(); await signOut({ redirect: true, redirectTo: reasonTimeout ? SESSION_TIMEOUT_ROUTE : SESSION_LOGOUT_ROUTE, diff --git a/src/utils/constants.ts b/src/utils/constants.ts index 2367a1ef4..84936217d 100644 --- a/src/utils/constants.ts +++ b/src/utils/constants.ts @@ -27,3 +27,4 @@ export const PageviewTypeUrls: Record = { }; export const SESSION_ID_COOKIE_NAME = "__Host-Http-session-id"; +export const SIGNOUT_FLAG_COOKIE_NAME = "__Secure-signout";