Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/app/_components/inactivity/InactivityDialog.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ jest.mock("next/navigation", () => ({
usePathname: jest.fn(() => mockUrlPath),
}));

jest.mock("sanitize-data", () => ({ sanitize: jest.fn() }));
jest.mock("@src/utils/auth/inactivity-timer");
jest.mock("@src/utils/auth/user-logout");

Expand Down
39 changes: 26 additions & 13 deletions src/app/our-policies/cookies-policy/CookiesTable.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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(<CookiesTable />);
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: "Stores temporary information used to identify when you sign out or are signed out after a period of inactivity.",
});

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);
});
});
10 changes: 10 additions & 0 deletions src/app/our-policies/cookies-policy/CookiesTable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,16 @@ const CookiesTable = (): JSX.Element => {
</td>
<td className="nhsuk-table__cell nhsuk-table__cell">After 1 hour</td>
</tr>
<tr className="nhsuk-table__row">
<th scope="row" className="nhsuk-table__header">
__Secure-signout
</th>
<td className="nhsuk-table__cell nhsuk-table__cell">
Stores temporary information used to identify when you sign out or are signed out after a period of
inactivity.
</td>
<td className="nhsuk-table__cell nhsuk-table__cell">After 30 seconds</td>
</tr>
</tbody>
</table>
);
Expand Down
71 changes: 71 additions & 0 deletions src/utils/auth/callbacks/get-token.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand All @@ -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;
Expand Down Expand Up @@ -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",
});
Expand Down Expand Up @@ -171,6 +189,41 @@ describe("getToken", () => {
maxAgeInSeconds,
);
});

it("should not return session if signout cookie value matches current session id", async () => {
const mockSessionId = "test-session-id";
const fakeRequestCookies: ReadonlyRequestCookies = {
get(name: string): RequestCookie | undefined {
if (name === SIGNOUT_FLAG_COOKIE_NAME) return { name: SIGNOUT_FLAG_COOKIE_NAME, value: mockSessionId };
if (name === SESSION_ID_COOKIE_NAME) return { name: SESSION_ID_COOKIE_NAME, value: mockSessionId };
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).toBeNull();
});

it("should ignore signout cookie if its value does not match current session id", async () => {
const fakeRequestCookies: ReadonlyRequestCookies = {
get(name: string): RequestCookie | undefined {
if (name === SIGNOUT_FLAG_COOKIE_NAME) return { name: SIGNOUT_FLAG_COOKIE_NAME, value: "old-session-id" };
if (name === SESSION_ID_COOKIE_NAME) return { name: SESSION_ID_COOKIE_NAME, value: "current-session-id" };
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).not.toBeNull();
});
});

describe("when AUTH APIM is not available", () => {
Expand All @@ -196,6 +249,24 @@ describe("getToken", () => {
maxAgeInSeconds,
);
});

it("should not return session if signout cookie value matches current session id", async () => {
const mockSessionId = "test-session-id";
const fakeRequestCookies: ReadonlyRequestCookies = {
get(name: string): RequestCookie | undefined {
if (name === SIGNOUT_FLAG_COOKIE_NAME) return { name: SIGNOUT_FLAG_COOKIE_NAME, value: mockSessionId };
if (name === SESSION_ID_COOKIE_NAME) return { name: SESSION_ID_COOKIE_NAME, value: mockSessionId };
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).toBeNull();
});
});

const expectResultToMatchTokenWith = (
Expand Down
14 changes: 12 additions & 2 deletions src/utils/auth/callbacks/get-token.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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" });
Expand All @@ -24,13 +26,21 @@ const getToken = async (
profile: Profile | undefined,
maxAgeInSeconds: MaxAgeInSeconds,
) => {
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;
}

if (!token) {
log.error("getToken: No token available in jwt callback. Returning null");
return null;
}

const nowInSeconds = Math.floor(Date.now() / 1000);

// 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");
Expand Down
46 changes: 46 additions & 0 deletions src/utils/auth/setSignOutFlagCookie.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
import setSignOutFlagCookie from "@src/utils/auth/setSignOutFlagCookie";
import { SESSION_ID_COOKIE_NAME, SIGNOUT_FLAG_COOKIE_NAME } from "@src/utils/constants";

const mockSessionId = "session-id-123";
const setCookie = jest.fn();
jest.mock("sanitize-data", () => ({ sanitize: jest.fn() }));
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This mock is required because our current Jest setup does not transpile the sanitize-data ESM package in tests, so replacing it with a stub avoids the module parse error and lets the tests run.

jest.mock("@src/utils/config", () => ({
__esModule: true,
default: {
MAX_SESSION_AGE_MINUTES: Promise.resolve(2),
},
}));

jest.mock("next/headers", () => ({
cookies: jest.fn(() => ({
get: jest.fn((name) => {
if (name === SESSION_ID_COOKIE_NAME) {
return { value: mockSessionId };
}
return undefined;
}),
set: setCookie,
})),
headers: jest.fn(),
}));

describe("setSignOutFlagCookie", () => {
beforeEach(() => {
jest.useFakeTimers().setSystemTime(new Date("2026-01-01T00:00:00.000Z"));
});

afterEach(() => {
jest.useRealTimers();
});

it("should set signout cookie with the current session id", async () => {
await setSignOutFlagCookie();
const expectedCookieTimeoutSeconds = 30;
expect(setCookie).toHaveBeenCalledWith(SIGNOUT_FLAG_COOKIE_NAME, mockSessionId, {
maxAge: expectedCookieTimeoutSeconds,
secure: true,
httpOnly: true,
sameSite: "lax",
});
});
});
23 changes: 23 additions & 0 deletions src/utils/auth/setSignOutFlagCookie.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
"use server";

import { SESSION_ID_COOKIE_NAME, SIGNOUT_FLAG_COOKIE_NAME } from "@src/utils/constants";
import { requestScopedStorageWrapper } from "@src/utils/requestScopedStorageWrapper";
import { cookies } from "next/headers";

const setSignOutFlagCookie = async () => {
return requestScopedStorageWrapper(setSignOutFlagCookieAction);
};

const setSignOutFlagCookieAction = async () => {
const SIGN_OUT_FLAG_COOKIE_MAX_AGE_SECONDS = 30;
Copy link
Copy Markdown
Contributor Author

@liming-cheung-nhs liming-cheung-nhs Apr 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've set it as 30s based on comments in the commit.
I wonder if we should have it higher as the session refresh is every 60s (see ClientProviders.tsx).

If we get a refresh, then a logout within next 30s, then the next refresh in 60s would not see the cookie as it has expired? 🤔

const cookieStore = await cookies();
const currentSessionId = cookieStore.get(SESSION_ID_COOKIE_NAME)?.value ?? "";
cookieStore.set(SIGNOUT_FLAG_COOKIE_NAME, currentSessionId, {
secure: true,
httpOnly: true,
sameSite: "lax",
maxAge: SIGN_OUT_FLAG_COOKIE_MAX_AGE_SECONDS,
});
};

export default setSignOutFlagCookie;
9 changes: 9 additions & 0 deletions src/utils/auth/user-logout.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,13 @@ import { SESSION_LOGOUT_ROUTE } from "@src/app/session-logout/constants";
import { SESSION_TIMEOUT_ROUTE } from "@src/app/session-timeout/constants";
import { userLogout } from "@src/utils/auth/user-logout";
import { signOut } from "next-auth/react";
import setSignOutFlagCookie from "@src/utils/auth/setSignOutFlagCookie";

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 () => {
Expand All @@ -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();
});
});
2 changes: 2 additions & 0 deletions src/utils/auth/user-logout.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,10 @@
import { SESSION_LOGOUT_ROUTE } from "@src/app/session-logout/constants";
import { SESSION_TIMEOUT_ROUTE } from "@src/app/session-timeout/constants";
import { signOut } from "next-auth/react";
import setSignOutFlagCookie from "@src/utils/auth/setSignOutFlagCookie";

const userLogout = async (reasonTimeout: boolean = false) => {
await setSignOutFlagCookie();
await signOut({
redirect: true,
redirectTo: reasonTimeout ? SESSION_TIMEOUT_ROUTE : SESSION_LOGOUT_ROUTE,
Expand Down
1 change: 1 addition & 0 deletions src/utils/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,3 +27,4 @@ export const PageviewTypeUrls: Record<ClientSidePageviewTypes, string> = {
};

export const SESSION_ID_COOKIE_NAME = "__Host-Http-session-id";
export const SIGNOUT_FLAG_COOKIE_NAME = "__Secure-signout";