From 9baed64f55dad7ba4fa686dc94d30a9115791160 Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Mon, 30 Mar 2026 17:22:33 +0100 Subject: [PATCH 01/21] Fix: [AEA-6434] - update prescriptionStatusHistory extension url (#1978) ## Summary - Routine Change ### Details Fix prescriptionStatusHistory extension url --- packages/prescriptionDetailsLambda/src/utils/types.ts | 2 +- .../tests/test_responseMapper.test.ts | 4 ++-- packages/prescriptionListLambda/src/utils/responseMapper.ts | 2 +- packages/prescriptionListLambda/tests/mockObjects.ts | 2 +- .../prescriptionListLambda/tests/test_responseMapper.test.ts | 4 ++-- 5 files changed, 7 insertions(+), 7 deletions(-) diff --git a/packages/prescriptionDetailsLambda/src/utils/types.ts b/packages/prescriptionDetailsLambda/src/utils/types.ts index 7674110ee3..d7bf5c5037 100644 --- a/packages/prescriptionDetailsLambda/src/utils/types.ts +++ b/packages/prescriptionDetailsLambda/src/utils/types.ts @@ -84,7 +84,7 @@ export const extensionUrlMappings = { "https://fhir.nhs.uk/StructureDefinition/Extension-EPS-TaskBusinessStatus" ], PRESCRIPTION_STATUS_HISTORY: [ - "https://fhir.nhs.uk/StructureDefinition/Extension-EPS-PrescriptionStatusHistory" + "https://fhir.nhs.uk/StructureDefinition/Extension-DM-PrescriptionStatusHistory" ], DM_PRESCRIPTION_STATUS_UPDATE_HISTORY: [ "https://fhir.nhs.uk/StructureDefinition/Extension-DM-PrescriptionStatusHistory" diff --git a/packages/prescriptionDetailsLambda/tests/test_responseMapper.test.ts b/packages/prescriptionDetailsLambda/tests/test_responseMapper.test.ts index 238c8d1d10..313323acf9 100644 --- a/packages/prescriptionDetailsLambda/tests/test_responseMapper.test.ts +++ b/packages/prescriptionDetailsLambda/tests/test_responseMapper.test.ts @@ -95,7 +95,7 @@ describe("mergePrescriptionDetails", () => { authoredOn: "2020-01-01T00:00:00Z", extension: [ { - url: "https://fhir.nhs.uk/StructureDefinition/Extension-EPS-PrescriptionStatusHistory", + url: "https://fhir.nhs.uk/StructureDefinition/Extension-DM-PrescriptionStatusHistory", extension: [ {url: "status", valueCoding: { system: "https://fhir.nhs.uk/CodeSystem/EPS-task-business-status", @@ -121,7 +121,7 @@ describe("mergePrescriptionDetails", () => { ] }, { - url: "https://fhir.nhs.uk/StructureDefinition/Extension-EPS-PrescriptionStatusHistory", + url: "https://fhir.nhs.uk/StructureDefinition/Extension-DM-PrescriptionStatusHistory", extension: [ {url: "cancellationReason", valueCoding: { system: "https://fhir.nhs.uk/CodeSystem/medicationrequest-status-reason", diff --git a/packages/prescriptionListLambda/src/utils/responseMapper.ts b/packages/prescriptionListLambda/src/utils/responseMapper.ts index d36c359c22..f876170a90 100644 --- a/packages/prescriptionListLambda/src/utils/responseMapper.ts +++ b/packages/prescriptionListLambda/src/utils/responseMapper.ts @@ -98,7 +98,7 @@ export const mapResponseToPrescriptionSummary = ( // Extract status code - fixed to match the structure const statusExtension = resource.extension?.find(ext => - ext.url === "https://fhir.nhs.uk/StructureDefinition/Extension-EPS-PrescriptionStatusHistory" + ext.url === "https://fhir.nhs.uk/StructureDefinition/Extension-DM-PrescriptionStatusHistory" ) const statusCode = statusExtension?.extension?.find(ext => ext.url === "status" diff --git a/packages/prescriptionListLambda/tests/mockObjects.ts b/packages/prescriptionListLambda/tests/mockObjects.ts index 9e8d275bb7..8e9a2d10fb 100644 --- a/packages/prescriptionListLambda/tests/mockObjects.ts +++ b/packages/prescriptionListLambda/tests/mockObjects.ts @@ -420,7 +420,7 @@ export const mockPrescriptionBundle: Bundle = { }, extension: [ { - url: "https://fhir.nhs.uk/StructureDefinition/Extension-EPS-PrescriptionStatusHistory", + url: "https://fhir.nhs.uk/StructureDefinition/Extension-DM-PrescriptionStatusHistory", extension: [{ url: "status", valueCoding: { diff --git a/packages/prescriptionListLambda/tests/test_responseMapper.test.ts b/packages/prescriptionListLambda/tests/test_responseMapper.test.ts index 4dcca0447d..f562e9bf6e 100644 --- a/packages/prescriptionListLambda/tests/test_responseMapper.test.ts +++ b/packages/prescriptionListLambda/tests/test_responseMapper.test.ts @@ -271,7 +271,7 @@ describe("Response Mapper Tests", () => { authoredOn: "20250204000000", extension: [ { - url: "https://fhir.nhs.uk/StructureDefinition/Extension-EPS-PrescriptionStatusHistory", + url: "https://fhir.nhs.uk/StructureDefinition/Extension-DM-PrescriptionStatusHistory", extension: [{ url: "status", valueCoding : { @@ -329,7 +329,7 @@ describe("Response Mapper Tests", () => { authoredOn: "20250204000000", extension: [ { - url: "https://fhir.nhs.uk/StructureDefinition/Extension-EPS-PrescriptionStatusHistory", + url: "https://fhir.nhs.uk/StructureDefinition/Extension-DM-PrescriptionStatusHistory", extension: [{ url: "status", valueCoding : { From a6c61836a5d337770b62b2a95b5d81f7c5abb961 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 30 Mar 2026 20:26:22 +0000 Subject: [PATCH 02/21] Upgrade: [dependabot] - bump pygments from 2.19.2 to 2.20.0 (#1980) Bumps [pygments](https://github.com/pygments/pygments) from 2.19.2 to 2.20.0.
Release notes

Sourced from pygments's releases.

2.20.0

  • New lexers:

  • Updated lexers:

    • archetype: Fix catastrophic backtracking in GUID and ID patterns (#3064)
    • ASN.1: Recognize minus sign and fix range operator (#3014, #3060)
    • C++: Add C++26 keywords (#2955), add integer literal suffixes (#2966)
    • ComponentPascal: Fix analyse_text (#3028, #3032)
    • Coq renamed to Rocq (#2883, #2908)
    • Cython: Various improvements (#2932, #2933)
    • Debian control: Improve architecture parsing (#3052)
    • Devicetree: Add support for overlay/fragments (#3021), add bytestring support (#3022), fix catastrophic backtracking (#3057)
    • Fennel: Various improvements (#2911)
    • Haskell: Handle escape sequences in character literals (#3069, #1795)
    • Java: Add module keywords (#2955)
    • Lean4: Add operators ]', ]?, ]! (#2946)
    • LESS: Support single-line comments (#3005)
    • LilyPond: Update to 2.25.29 (#2974)
    • LLVM: Support C-style comments (#3023, #2978)
    • Lua(u): Fix catastrophic backtracking (#3047)
    • Macaulay2: Update to 1.25.05 (#2893), 1.25.11 (#2988)
    • Mathematica: Various improvements (#2957)
    • meson: Add additional operators (#2919)
    • MySQL: Update keywords (#2970)
    • org-Mode: Support both schedule and deadline (#2899)
    • PHP: Add __PROPERTY__ magic constant (#2924), add reserved keywords (#3002)
    • PostgreSQL: Add more keywords (#2985)
    • protobuf: Fix namespace tokenization (#2929)
    • Python: Add t-string support (#2973, #3009, #3010)
    • Tablegen: Fix infinite loop (#2972, #2940)
    • Tera Term macro: Add commands introduced in v5.3 through v5.6 (#2951)
    • TOML: Support TOML 1.1.0 (#3026, #3027)
    • Turtle: Allow empty comment lines (#2980)
    • XML: Added .xbrl as file ending (#2890, #2891)

  • Drop Python 3.8, and add Python 3.14 as a supported version (#2987, #3012)

  • Various improvements to autopygmentize (#2894)

  • Update onedark style to support more token types (#2977)

  • Update rtt style to support more token types (#2895)

  • Cache entry points to improve performance (#2979)

  • Fix xterm-256 color table (#3043)

  • Fix kwargs dictionary getting mutated on each call (#3044)

Changelog

Sourced from pygments's changelog.

Version 2.20.0

(released March 29th, 2026)

  • New lexers:

  • Updated lexers:

    • archetype: Fix catastrophic backtracking in GUID and ID patterns (#3064)
    • ASN.1: Recognize minus sign and fix range operator (#3014, #3060)
    • C++: Add C++26 keywords (#2955), add integer literal suffixes (#2966)
    • ComponentPascal: Fix analyse_text (#3028, #3032)
    • Coq renamed to Rocq (#2883, #2908)
    • Cython: Various improvements (#2932, #2933)
    • Debian control: Improve architecture parsing (#3052)
    • Devicetree: Add support for overlay/fragments (#3021), add bytestring support (#3022), fix catastrophic backtracking (#3057)
    • Fennel: Various improvements (#2911)
    • Haskell: Handle escape sequences in character literals (#3069, #1795)
    • Java: Add module keywords (#2955)
    • Lean4: Add operators ]', ]?, ]! (#2946)
    • LESS: Support single-line comments (#3005)
    • LilyPond: Update to 2.25.29 (#2974)
    • LLVM: Support C-style comments (#3023, #2978)
    • Lua(u): Fix catastrophic backtracking (#3047)
    • Macaulay2: Update to 1.25.05 (#2893), 1.25.11 (#2988)
    • Mathematica: Various improvements (#2957)
    • meson: Add additional operators (#2919)
    • MySQL: Update keywords (#2970)
    • org-Mode: Support both schedule and deadline (#2899)
    • PHP: Add __PROPERTY__ magic constant (#2924), add reserved keywords (#3002)
    • PostgreSQL: Add more keywords (#2985)
    • protobuf: Fix namespace tokenization (#2929)
    • Python: Add t-string support (#2973, #3009, #3010)
    • Tablegen: Fix infinite loop (#2972, #2940)
    • Tera Term macro: Add commands introduced in v5.3 through v5.6 (#2951)
    • TOML: Support TOML 1.1.0 (#3026, #3027)
    • Turtle: Allow empty comment lines (#2980)
    • XML: Added .xbrl as file ending (#2890, #2891)

  • Drop Python 3.8, and add Python 3.14 as a supported version (#2987, #3012)

  • Various improvements to autopygmentize (#2894)

  • Update onedark style to support more token types (#2977)

  • Update rtt style to support more token types (#2895)

  • Cache entry points to improve performance (#2979)

  • Fix xterm-256 color table (#3043)

  • Fix kwargs dictionary getting mutated on each call (#3044)

Commits

[![Dependabot compatibility score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=pygments&package-manager=pip&previous-version=2.19.2&new-version=2.20.0)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) ---
Dependabot commands and options
You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot show ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) You can disable automated security fix PRs for this repo from the [Security Alerts page](https://github.com/NHSDigital/eps-prescription-tracker-ui/network/alerts).
Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- poetry.lock | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/poetry.lock b/poetry.lock index 8b59e16376..4451cc8635 100644 --- a/poetry.lock +++ b/poetry.lock @@ -304,14 +304,14 @@ virtualenv = ">=20.10.0" [[package]] name = "pygments" -version = "2.19.2" +version = "2.20.0" description = "Pygments is a syntax highlighting package written in Python." optional = false -python-versions = ">=3.8" +python-versions = ">=3.9" groups = ["main"] files = [ - {file = "pygments-2.19.2-py3-none-any.whl", hash = "sha256:86540386c03d588bb81d44bc3928634ff26449851e99741617ecb9037ee5ec0b"}, - {file = "pygments-2.19.2.tar.gz", hash = "sha256:636cb2477cec7f8952536970bc533bc43743542f70392ae026374600add5b887"}, + {file = "pygments-2.20.0-py3-none-any.whl", hash = "sha256:81a9e26dd42fd28a23a2d169d86d7ac03b46e2f8b59ed4698fb4785f946d0176"}, + {file = "pygments-2.20.0.tar.gz", hash = "sha256:6757cd03768053ff99f3039c1a36d6c0aa0b263438fcab17520b30a303a82b5f"}, ] [package.extras] From 539c690aa6144e93f3a7754badb464bef51f226a Mon Sep 17 00:00:00 2001 From: Connor Avery Date: Tue, 31 Mar 2026 11:15:25 +0100 Subject: [PATCH 03/21] Fix: [AEA-6446] - Bug fix single patient return not loading prescription findings (#1975) ## Summary - Routine Change ### Details Fix the bug where a single patient returned on basic details search was being incorrectly redirected to search by prescription ID page, because it didn't have the searchType handled correctly. --------- Signed-off-by: Connor Avery <214469360+connoravo-nhs@users.noreply.github.com> --- .../BasicDetailsSearchResultsPage.test.tsx | 15 ++++++++++----- .../cpt-ui/__tests__/EpsPrescriptionList.test.tsx | 2 +- .../__tests__/PrescriptionListPage.test.tsx | 2 +- .../src/pages/BasicDetailsSearchResultsPage.tsx | 8 ++++++-- .../cpt-ui/src/pages/PrescriptionListPage.tsx | 8 +++----- 5 files changed, 21 insertions(+), 14 deletions(-) diff --git a/packages/cpt-ui/__tests__/BasicDetailsSearchResultsPage.test.tsx b/packages/cpt-ui/__tests__/BasicDetailsSearchResultsPage.test.tsx index f64147c405..eebf0a566e 100644 --- a/packages/cpt-ui/__tests__/BasicDetailsSearchResultsPage.test.tsx +++ b/packages/cpt-ui/__tests__/BasicDetailsSearchResultsPage.test.tsx @@ -403,7 +403,8 @@ describe("BasicDetailsSearchResultsPage", () => { dobMonth: "01", dobYear: "1990", postcode: "SW1A 1AA", - nhsNumber: "9726919207" + nhsNumber: "9726919207", + searchType: "basicDetails" }) }) }) @@ -430,7 +431,8 @@ describe("BasicDetailsSearchResultsPage", () => { dobMonth: "01", dobYear: "1990", postcode: "SW1A 1AA", - nhsNumber: "9726919207" + nhsNumber: "9726919207", + searchType: "basicDetails" }) }) }) @@ -456,7 +458,8 @@ describe("BasicDetailsSearchResultsPage", () => { dobMonth: "01", dobYear: "1990", postcode: "SW1A 1AA", - nhsNumber: "9726919207" + nhsNumber: "9726919207", + searchType: "basicDetails" }) }) }) @@ -498,7 +501,8 @@ describe("BasicDetailsSearchResultsPage", () => { dobMonth: "01", dobYear: "1990", postcode: "SW1A 1AA", - nhsNumber: "9726919207" + nhsNumber: "9726919207", + searchType: "basicDetails" }) }) }) @@ -524,7 +528,8 @@ describe("BasicDetailsSearchResultsPage", () => { dobMonth: "01", dobYear: "1990", postcode: "SW1A 1AA", - nhsNumber: "9726919207" + nhsNumber: "9726919207", + searchType: "basicDetails" }) }) }) diff --git a/packages/cpt-ui/__tests__/EpsPrescriptionList.test.tsx b/packages/cpt-ui/__tests__/EpsPrescriptionList.test.tsx index a9119a08ef..b65a48f49c 100644 --- a/packages/cpt-ui/__tests__/EpsPrescriptionList.test.tsx +++ b/packages/cpt-ui/__tests__/EpsPrescriptionList.test.tsx @@ -522,7 +522,7 @@ describe("PrescriptionListPage", () => { await waitFor(() => { expect(loggerInfoSpy).toHaveBeenCalledWith( - "No search parameter provided - redirecting to prescription ID search" + "No search type available - redirecting to prescription ID search" ) }) diff --git a/packages/cpt-ui/__tests__/PrescriptionListPage.test.tsx b/packages/cpt-ui/__tests__/PrescriptionListPage.test.tsx index b5ae6adb66..e00213ac37 100644 --- a/packages/cpt-ui/__tests__/PrescriptionListPage.test.tsx +++ b/packages/cpt-ui/__tests__/PrescriptionListPage.test.tsx @@ -368,7 +368,7 @@ describe("PrescriptionListPage", () => { }) expect(logger.info).toHaveBeenCalledWith( - "No search parameter provided - redirecting to prescription ID search" + "No search type available - redirecting to prescription ID search" ) }) diff --git a/packages/cpt-ui/src/pages/BasicDetailsSearchResultsPage.tsx b/packages/cpt-ui/src/pages/BasicDetailsSearchResultsPage.tsx index 1938cbf51a..11c72f8a8b 100644 --- a/packages/cpt-ui/src/pages/BasicDetailsSearchResultsPage.tsx +++ b/packages/cpt-ui/src/pages/BasicDetailsSearchResultsPage.tsx @@ -73,7 +73,8 @@ const TableResultsRow = ({patient}: TableResultsRowProps) => { navigationContext.getRelevantSearchParameters("basicDetails") searchContext.setAllSearchParameters({ ...relevantParams, - nhsNumber: nhsNumber + nhsNumber: nhsNumber, + searchType: "basicDetails" }) navigate(`${FRONTEND_PATHS.PRESCRIPTION_LIST_CURRENT}`) } @@ -152,9 +153,12 @@ export default function SearchResultsPage() { if (payload.length === 1) { const relevantParams = navigationContext.getRelevantSearchParameters("basicDetails") + // Set the search type before passing to prescription list current page + // So that the useEffect will detect and treat the search type correctly searchContext.setAllSearchParameters({ ...relevantParams, - nhsNumber: payload[0].nhsNumber + nhsNumber: payload[0].nhsNumber, + searchType: "basicDetails" }) navigate(FRONTEND_PATHS.PRESCRIPTION_LIST_CURRENT) return diff --git a/packages/cpt-ui/src/pages/PrescriptionListPage.tsx b/packages/cpt-ui/src/pages/PrescriptionListPage.tsx index 588ee04aeb..dfd5ea7b55 100644 --- a/packages/cpt-ui/src/pages/PrescriptionListPage.tsx +++ b/packages/cpt-ui/src/pages/PrescriptionListPage.tsx @@ -56,12 +56,10 @@ export default function PrescriptionListPage() { // Handle basic details case - redirect only if we still have no NHS number or prescription ID const hasAnyNhsNumber = Boolean(originalSearchParams?.nhsNumber || searchContext.nhsNumber) - const hasAnyPrescriptionId = Boolean(originalSearchParams?.prescriptionId || searchContext.prescriptionId) if (originalSearchParams && (originalSearchParams.firstName || originalSearchParams.lastName) && - !hasAnyNhsNumber && - !hasAnyPrescriptionId) { - logger.info("Basic details present but no NHS number/ prescription ID - redirecting to prescription ID search") + (!hasAnyNhsNumber)) { + logger.info("Basic details present but no NHS number - redirecting to prescription ID search") navigate(FRONTEND_PATHS.SEARCH_BY_PRESCRIPTION_ID) return } @@ -89,7 +87,7 @@ export default function PrescriptionListPage() { break default: // Unrecognized search type - redirect to search page - logger.info("No search parameter provided - redirecting to prescription ID search") + logger.info("No search type available - redirecting to prescription ID search") navigate(FRONTEND_PATHS.SEARCH_BY_PRESCRIPTION_ID) return } From 352787de9511fc647b7b8a37f7d1e171ae1334eb Mon Sep 17 00:00:00 2001 From: Connor Avery Date: Tue, 31 Mar 2026 12:44:24 +0100 Subject: [PATCH 04/21] New: [AEA-6292] - Authentication redirect & blocking render improvements (#1865) ## Summary - :sparkles: New Feature - :warning: Potential issues that might be caused by this change ### Details Aims to resolve the bugs identified in: - AEA-6292 - AEA-6310 - AEA-5831 - AEA-6326 Changes made: The ensureRoleSelected function has been somewhat re-written to ensure hierarchical order is correct & that if statements are "looser" fitting to negative scenarios. Public paths shouldn't be affected by redirections apart from root and login. The shouldBlockChildren function has also had additional conditions added, that will ensure a render block occurs, so that on-page useEffects don't run. The shouldBlockChildren now dictates within AccessContext.Provider return, whether the children or loading page should be shown. This is believed to resolve an issue where re-rendering wasn't occurring in the event of a state change if a user was on the loading page in an auth transition state. Token lambdas now check for a 'complete' token information in the DynamoDb table, to prevent the cause of eventual consistency of a session logout, where a dynamoDb row remains with skeleton information and no actual credentials. --------- Signed-off-by: Connor Avery <214469360+connoravo-nhs@users.noreply.github.com> Co-authored-by: Jonno Co-authored-by: jonathanwelch1-nhs <148754575+jonathanwelch1-nhs@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: anthony-nhs <121869075+anthony-nhs@users.noreply.github.com> --- .github/workflows/run_regression_tests.yml | 4 +- packages/cognito/src/token.ts | 23 +- packages/cognito/src/tokenMock.ts | 21 +- .../cognito/tests/test_token.cis2.test.ts | 14 +- .../cognito/tests/test_token.mock.test.ts | 14 +- ...AccessProvider.ensureRoleSelected.test.tsx | 408 ++++++++++++++++++ ...ccessProvider.shouldBlockChildren.test.tsx | 370 ++++++++++++++++ .../cpt-ui/__tests__/AccessProvider.test.tsx | 61 ++- .../cpt-ui/__tests__/AuthProvider.test.tsx | 43 +- .../__tests__/CookiePolicyPage.test.tsx | 7 - .../__tests__/EpsRoleSelectionPage.test.tsx | 110 +---- .../cpt-ui/__tests__/LoginFunctions.test.tsx | 126 +++++- packages/cpt-ui/__tests__/LogoutPage.test.tsx | 35 +- .../__tests__/PrescriptionListPage.test.tsx | 18 +- packages/cpt-ui/__tests__/RBACBanner.test.tsx | 36 +- packages/cpt-ui/__tests__/axios.test.tsx | 13 +- packages/cpt-ui/__tests__/logout.test.tsx | 300 ++++++++----- .../cpt-ui/__tests__/mocks/AuthStateMock.tsx | 1 + packages/cpt-ui/__tests__/tabHelpers.test.ts | 308 +++++++++++++ .../__tests__/useSessionTimeout.test.tsx | 36 +- packages/cpt-ui/jest.setup.ts | 11 +- packages/cpt-ui/src/components/EpsHeader.tsx | 11 +- .../src/components/EpsRoleSelectionPage.tsx | 36 +- .../components/EpsRoleSelectionPage.utils.ts | 11 +- packages/cpt-ui/src/components/RBACBanner.tsx | 8 +- packages/cpt-ui/src/constants/environment.ts | 14 +- .../cpt-ui/src/context/AccessProvider.tsx | 233 +++++++--- packages/cpt-ui/src/context/AuthProvider.tsx | 90 ++-- packages/cpt-ui/src/helpers/axios.tsx | 9 +- .../cpt-ui/src/helpers/getSearchParams.tsx | 3 +- .../cpt-ui/src/helpers/loginFunctions.tsx | 33 +- packages/cpt-ui/src/helpers/logout.tsx | 180 ++++++-- packages/cpt-ui/src/helpers/tabHelpers.ts | 145 +++++++ .../cpt-ui/src/hooks/useSessionTimeout.ts | 11 +- .../pages/BasicDetailsSearchResultsPage.tsx | 4 +- packages/cpt-ui/src/pages/LoadingPage.tsx | 2 +- packages/cpt-ui/src/pages/LoginPage.tsx | 52 +-- packages/cpt-ui/src/pages/LogoutPage.tsx | 28 +- .../src/pages/PrescriptionDetailsPage.tsx | 4 +- .../cpt-ui/src/pages/PrescriptionListPage.tsx | 7 +- .../cpt-ui/src/pages/SessionSelection.tsx | 8 +- 41 files changed, 2262 insertions(+), 586 deletions(-) create mode 100644 packages/cpt-ui/__tests__/AccessProvider.ensureRoleSelected.test.tsx create mode 100644 packages/cpt-ui/__tests__/AccessProvider.shouldBlockChildren.test.tsx create mode 100644 packages/cpt-ui/__tests__/tabHelpers.test.ts create mode 100644 packages/cpt-ui/src/helpers/tabHelpers.ts diff --git a/.github/workflows/run_regression_tests.yml b/.github/workflows/run_regression_tests.yml index 46f82d2680..17dfba6ca2 100644 --- a/.github/workflows/run_regression_tests.yml +++ b/.github/workflows/run_regression_tests.yml @@ -57,8 +57,8 @@ jobs: GITHUB-TOKEN: ${{ steps.generate-token.outputs.token }} run: | if [[ "$TARGET_ENVIRONMENT" != "prod" && "$TARGET_ENVIRONMENT" != "ref" ]]; then - REGRESSION_TEST_REPO_TAG="v3.9.10" # This is the tag or branch of the regression test code to run, usually a version tag like v3.1.0 or a branch name - REGRESSION_TEST_WORKFLOW_TAG="v3.9.10" # This is the tag of the github workflow to run, usually the same as REGRESSION_TEST_REPO_TAG + REGRESSION_TEST_REPO_TAG="v3.10.9" # This is the tag or branch of the regression test code to run, usually a version tag like v3.1.0 or a branch name + REGRESSION_TEST_WORKFLOW_TAG="v3.10.9" # This is the tag of the github workflow to run, usually the same as REGRESSION_TEST_REPO_TAG if [[ -z "$REGRESSION_TEST_REPO_TAG" || -z "$REGRESSION_TEST_WORKFLOW_TAG" ]]; then diff --git a/packages/cognito/src/token.ts b/packages/cognito/src/token.ts index 7377c6a112..2a46bc1bfc 100644 --- a/packages/cognito/src/token.ts +++ b/packages/cognito/src/token.ts @@ -17,7 +17,7 @@ import {MiddyErrorHandler} from "@cpt-ui-common/middyErrorHandler" import {headers} from "@cpt-ui-common/lambdaUtils" import {rewriteRequestBody} from "./helpers" -import {insertTokenMapping, tryGetTokenMapping} from "@cpt-ui-common/dynamoFunctions" +import {insertTokenMapping, tryGetTokenMapping, TokenMappingItem} from "@cpt-ui-common/dynamoFunctions" /* This is the lambda code that is used to intercept calls to token endpoint as part of the cognito login flow @@ -71,6 +71,18 @@ const errorResponseBody = { const middyErrorHandler = new MiddyErrorHandler(errorResponseBody) +function checkIfValidTokenMapping(tokenMapping: TokenMappingItem | undefined): boolean { + const fifteenMinutes = 15 * 60 * 1000 + + return tokenMapping !== undefined && + tokenMapping.sessionId !== undefined && + tokenMapping.cis2AccessToken !== undefined && + tokenMapping.cis2IdToken !== undefined && + tokenMapping.cis2ExpiresIn !== undefined && + tokenMapping.lastActivityTime !== undefined && + tokenMapping.lastActivityTime > Date.now() - fifteenMinutes +} + const lambdaHandler = async (event: APIGatewayProxyEvent): Promise => { logger.appendKeys({ "apigw-request-id": event.requestContext?.requestId @@ -131,13 +143,13 @@ const lambdaHandler = async (event: APIGatewayProxyEvent): Promise Date.now() - fifteenMinutes) { + const validToken = checkIfValidTokenMapping(existingTokenMapping) + if (validToken) { logger.info("User already exists in token mapping table, creating draft session", {username}, {sessionManagementTableName}) await insertTokenMapping(documentClient, sessionManagementTableName, tokenMappingItem, logger) diff --git a/packages/cognito/src/tokenMock.ts b/packages/cognito/src/tokenMock.ts index ab1284b1b5..bebb4f2c3d 100644 --- a/packages/cognito/src/tokenMock.ts +++ b/packages/cognito/src/tokenMock.ts @@ -9,7 +9,7 @@ import inputOutputLogger from "@middy/input-output-logger" import {parse} from "querystring" import {PrivateKey} from "jsonwebtoken" import {exchangeTokenForApigeeAccessToken, fetchUserInfo, initializeOidcConfig} from "@cpt-ui-common/authFunctions" -import {insertTokenMapping, tryGetTokenMapping} from "@cpt-ui-common/dynamoFunctions" +import {insertTokenMapping, tryGetTokenMapping, TokenMappingItem} from "@cpt-ui-common/dynamoFunctions" import {MiddyErrorHandler} from "@cpt-ui-common/middyErrorHandler" import jwt from "jsonwebtoken" import axios from "axios" @@ -72,6 +72,17 @@ async function createSignedJwt(claims: Record) { }) } +function checkIfValidTokenMapping(tokenMapping: TokenMappingItem | undefined): boolean { + const fifteenMinutes = 15 * 60 * 1000 + + return tokenMapping !== undefined && + tokenMapping.sessionId !== undefined && + tokenMapping.apigeeAccessToken !== undefined && + tokenMapping.apigeeExpiresIn !== undefined && + tokenMapping.lastActivityTime !== undefined && + tokenMapping.lastActivityTime > Date.now() - fifteenMinutes +} + const lambdaHandler = async (event: APIGatewayProxyEvent): Promise => { const apigeeApiKey = await getSecret(apigeeApiKeyArn) const apigeeApiSecret = await getSecret(apigeeApiSecretArn) @@ -154,7 +165,7 @@ const lambdaHandler = async (event: APIGatewayProxyEvent): Promise Date.now() - fifteenMinutes) { + const validToken = checkIfValidTokenMapping(existingTokenMapping) + if (validToken) { const username = tokenMappingItem.username logger.info("User already exists in token mapping table, creating draft session", {username}, {sessionManagementTableName}) diff --git a/packages/cognito/tests/test_token.cis2.test.ts b/packages/cognito/tests/test_token.cis2.test.ts index 4da2c38b15..2eb0357f72 100644 --- a/packages/cognito/tests/test_token.cis2.test.ts +++ b/packages/cognito/tests/test_token.cis2.test.ts @@ -206,7 +206,11 @@ describe("cis2 token handler", () => { mockTryGetTokenMapping.mockImplementationOnce(() => { return Promise.resolve({ username: `${CIS2_USER_POOL_IDP}_foo`, - lastActivityTime: Date.now() - (10 * 60 * 1000) // 10 minutes ago (less than 15) + lastActivityTime: Date.now() - (10 * 60 * 1000), // 10 minutes ago (less than 15) + sessionId: "session-id", + cis2AccessToken: "foo", + cis2IdToken: "bar", + cis2ExpiresIn: Date.now() - - (10 * 60 * 1000) }) }) @@ -353,14 +357,18 @@ describe("cis2 token handler", () => { it("creates concurrent session when user exists and last activity exactly at 15 minute boundary", async () => { // Mock Date.now() to ensure consistent timing for boundary test - const fixedTime = 1000000000000 // Fixed timestamp + const fixedTime = 1000000 // Fixed timestamp const dateNowSpy = vi.spyOn(Date, "now").mockReturnValue(fixedTime) const fifteenMinutes = 15 * 60 * 1000 mockTryGetTokenMapping.mockImplementationOnce(() => { return Promise.resolve({ username: `${CIS2_USER_POOL_IDP}_foo`, - lastActivityTime: fixedTime - fifteenMinutes + 1 // Just barely within 15 minute window + lastActivityTime: fixedTime - fifteenMinutes + 1, // Just barely within 15 minute window + sessionId: "session-id", + cis2AccessToken: "foo", + cis2IdToken: "bar", + cis2ExpiresIn: fixedTime - fifteenMinutes + 1 }) }) diff --git a/packages/cognito/tests/test_token.mock.test.ts b/packages/cognito/tests/test_token.mock.test.ts index feb7f0d38f..4540977722 100644 --- a/packages/cognito/tests/test_token.mock.test.ts +++ b/packages/cognito/tests/test_token.mock.test.ts @@ -224,7 +224,10 @@ describe("token mock handler", () => { mockTryGetTokenMapping.mockImplementation(() => { return Promise.resolve({ username: "Mock_user_details_sub", - lastActivityTime: Date.now() + lastActivityTime: Date.now(), + sessionId: "session-id", + apigeeAccessToken: "foo", + apigeeExpiresIn: "3600" }) }) @@ -259,10 +262,11 @@ describe("token mock handler", () => { }, dummyContext) // check call + expect(mockInsertTokenMapping).toHaveBeenCalledOnce() expect(mockInsertTokenMapping).toHaveBeenCalledWith( expect.anything(), // documentClient "test-session-management-table", // tableName - { + ({ username: "Mock_user_details_sub", apigeeAccessToken: "new-access-token", apigeeRefreshToken: "new-refresh-token", @@ -279,7 +283,7 @@ describe("token mock handler", () => { }, lastActivityTime: expect.any(Number), sessionId: expect.any(String) - }, // item + }), // item expect.anything() // logger ) // Check response structure @@ -342,7 +346,7 @@ describe("token mock handler", () => { expect(mockInsertTokenMapping).toHaveBeenCalledWith( expect.anything(), // documentClient "dummyTable", // tableName - { + ({ username: "Mock_user_details_sub", apigeeAccessToken: "new-access-token", apigeeRefreshToken: "new-refresh-token", @@ -359,7 +363,7 @@ describe("token mock handler", () => { }, lastActivityTime: expect.any(Number), sessionId: expect.any(String) - }, // item + }), // item expect.anything() // logger ) // Check response structure diff --git a/packages/cpt-ui/__tests__/AccessProvider.ensureRoleSelected.test.tsx b/packages/cpt-ui/__tests__/AccessProvider.ensureRoleSelected.test.tsx new file mode 100644 index 0000000000..4b2ecbe153 --- /dev/null +++ b/packages/cpt-ui/__tests__/AccessProvider.ensureRoleSelected.test.tsx @@ -0,0 +1,408 @@ +import React from "react" +import {render, waitFor} from "@testing-library/react" +import {AccessProvider} from "@/context/AccessProvider" +import {FRONTEND_PATHS, LOGOUT_MARKER_STORAGE_GROUP, LOGOUT_MARKER_STORAGE_KEY} from "@/constants/environment" +import {AuthContext, AuthContextType} from "@/context/AuthProvider" +import {LogoutMarker} from "@/helpers/logout" +import {getOrCreateTabId, getOpenTabCount} from "@/helpers/tabHelpers" +import {MemoryRouter} from "react-router-dom" +import {mockAuthState} from "./mocks/AuthStateMock" + +jest.mock("@/components/EpsHeader", () => ({ + __esModule: true, + default: jest.fn(() => null) +})) + +jest.mock("@/helpers/logger", () => ({ + logger: { + debug: jest.fn(), + info: jest.fn(), + warn: jest.fn(), + error: jest.fn() + } +})) + +const mockHandleSignoutEvent = jest.fn() +jest.mock("@/helpers/logout", () => { + const actual = jest.requireActual("@/helpers/logout") + return { + ...actual, + handleSignoutEvent: (...args: Array) => mockHandleSignoutEvent(...args), + signOut: jest.fn() + } +}) + +// Mock useNavigate and assign to a variable for assertions +const mockNavigate = jest.fn() +jest.mock("react-router-dom", () => { + const actual = jest.requireActual("react-router-dom") + return { + ...actual, + useNavigate: () => mockNavigate + } +}) + +jest.mock("@/helpers/tabHelpers", () => { + const actual = jest.requireActual("@/helpers/tabHelpers") + return { + ...actual, + getOrCreateTabId: jest.fn().mockReturnValue("default-tab"), + getOpenTabCount: jest.fn().mockReturnValue(1), + updateOpenTabs: jest.fn(), + heartbeatTab: jest.fn(), + pruneStaleTabIds: jest.fn() + } +}) + +describe("ensureRoleSelected", () => { + beforeEach(() => { + jest.clearAllMocks() + localStorage.clear() + sessionStorage.clear() + }) + + const renderWithProvider = ( + authStateOverrides: Partial = {}, + initialPath: string + ) => { + const authContextValue: AuthContextType = { + ...mockAuthState, + updateTrackerUserInfo: jest.fn().mockResolvedValue({error: null, remainingSessionTime: 30000}), + clearAuthState: jest.fn(), + ...authStateOverrides + } + + render( + + + <> + + + + ) + } + + type Scenario = { + name: string; + initialPath: string; + authStateOverrides: Partial; + expectedPath?: string; + logoutMarker?: Partial + } + + const redirectScenarios: Array = [ + { + name: "signed in, has selected role, on root path, redirects to search", // has a render block test + initialPath: "/", + authStateOverrides: { + isSignedIn: true, + isSigningIn: false, + selectedRole: {role_name: "Test Role"} + }, + expectedPath: FRONTEND_PATHS.SEARCH_BY_PRESCRIPTION_ID + }, + { + name: "signed in, has selected role, on login, redirects to search", // has a render block test + initialPath: FRONTEND_PATHS.LOGIN, + authStateOverrides: { + isSignedIn: true, + isSigningIn: false, + selectedRole: {role_name: "Test Role"} + }, + expectedPath: FRONTEND_PATHS.SEARCH_BY_PRESCRIPTION_ID + }, + { + name: "not signed in, on root path, redirects to login", // has a render block test + initialPath: "/", + authStateOverrides: { + isSignedIn: false, + isSigningIn: false, + selectedRole: undefined + }, + expectedPath: FRONTEND_PATHS.LOGIN + }, + { + name: "not signed in, not signing in, on protected path, redirects to login", // has a render block test + initialPath: FRONTEND_PATHS.SEARCH_BY_PRESCRIPTION_ID, + authStateOverrides: { + isSignedIn: false, + isSigningIn: false, + selectedRole: undefined + }, + expectedPath: FRONTEND_PATHS.LOGIN + }, + { // has a render block test + name: "not signed in, signing out, has invalid session cause, on protected path, redirects to login", + initialPath: FRONTEND_PATHS.SEARCH_BY_PRESCRIPTION_ID, + authStateOverrides: { + isSignedIn: false, + isSigningIn: false, + isSigningOut: true, + invalidSessionCause: "Session expired", + selectedRole: undefined + }, + expectedPath: FRONTEND_PATHS.LOGIN + }, + { // has render block test + name: "not signed in, on protected path, marker in state only, redirects to login", + initialPath: FRONTEND_PATHS.SEARCH_BY_PRESCRIPTION_ID, + authStateOverrides: { + isSignedIn: false, + isSigningIn: false, + isSigningOut: false, + selectedRole: undefined, + logoutMarker: { + timestamp: Date.now(), + reason: "signOut", + initiatedByTabId: "tab-a" + } + }, + expectedPath: FRONTEND_PATHS.LOGIN + }, + { // has render block test + name: "not signed in, on protected path, stale logout marker, redirects to login", + initialPath: FRONTEND_PATHS.SEARCH_BY_PRESCRIPTION_ID, + authStateOverrides: { + isSignedIn: false, + isSigningIn: false, + isSigningOut: false, + selectedRole: undefined, + logoutMarker: { + timestamp: Date.now() - 60000, + reason: "signOut", + initiatedByTabId: "tab-a" + } + }, + expectedPath: FRONTEND_PATHS.LOGIN + }, + { // has a render block test + name: "signed in, no selected role selected, on protected path, is redirected to select role", + initialPath: FRONTEND_PATHS.SEARCH_BY_PRESCRIPTION_ID, + authStateOverrides: { + isSignedIn: true, + isSigningIn: false, + isSigningOut: false, + selectedRole: undefined + }, + expectedPath: FRONTEND_PATHS.SELECT_YOUR_ROLE + }, + { // has a render block test + name: "signed in, concurrent session, on protected path, is redirected to session selection", + initialPath: FRONTEND_PATHS.SEARCH_BY_PRESCRIPTION_ID, + authStateOverrides: { + isSignedIn: true, + isSigningIn: false, + isSigningOut: false, + isConcurrentSession: true, + selectedRole: {role_name: "Test Role"} + }, + expectedPath: FRONTEND_PATHS.SESSION_SELECTION + }, + { // has a render block test + name: "not signed in, signing in, on protected path, redirected to login", + initialPath: FRONTEND_PATHS.SEARCH_BY_PRESCRIPTION_ID, + authStateOverrides: { + isSignedIn: false, + isSigningIn: true, + isSigningOut: false, + selectedRole: undefined + }, + expectedPath: FRONTEND_PATHS.LOGIN + } + ] + + const noRedirectScenarios: Array = [ + { + name: "not signed in, on public path, doesnt redirect", // has a render block test + initialPath: FRONTEND_PATHS.COOKIES, + authStateOverrides: { + isSignedIn: false, + isSigningIn: false, + selectedRole: undefined + } + }, + { + name: "signed in, role selected, on public path, doesnt redirect", // has a render block test + initialPath: FRONTEND_PATHS.COOKIES, + authStateOverrides: { + isSignedIn: true, + isSigningIn: false, + selectedRole: {role_name: "Test Role"} + } + }, + { + name: "signed in, role selected, on allowed no roles path, doesnt redirect", // has a render block test + initialPath: FRONTEND_PATHS.SELECT_YOUR_ROLE, + authStateOverrides: { + isSignedIn: true, + isSigningIn: false, + selectedRole: {role_name: "Test Role"} + } + }, + { // has a render block test + name: "signed in, signing out, on public path, doesnt redirect", + initialPath: FRONTEND_PATHS.COOKIES, + authStateOverrides: { + isSignedIn: true, + isSigningOut: true + } + }, + { + name: "not signed in, signing in, on select your role, with search params, doesnt redirect", + initialPath: FRONTEND_PATHS.SELECT_YOUR_ROLE + "?code=test", + authStateOverrides: { + isSignedIn: false, + isSigningIn: true + } + } + ] + + const logoutScenarios: Array = [ + { + name: "not signed in, signing in, on select your role, with no search params, redirects to session logged out", + initialPath: FRONTEND_PATHS.SELECT_YOUR_ROLE, + authStateOverrides: { + isSignedIn: false, + isSigningIn: true + } + }, + { // has a render block test + name: "signed in, signing out, has invalid session cause, on protected path, redirects to session logged out", + initialPath: FRONTEND_PATHS.SEARCH_BY_PRESCRIPTION_ID, + authStateOverrides: { + isSignedIn: true, + isSigningOut: true, + invalidSessionCause: "Session expired" + }, + expectedPath: FRONTEND_PATHS.SESSION_LOGGED_OUT + }, + { // has a render block test + name: "signed in, signing out, on protected path, redirects to logout", + initialPath: FRONTEND_PATHS.SEARCH_BY_PRESCRIPTION_ID, + authStateOverrides: { + isSignedIn: true, + isSigningOut: true + }, + expectedPath: FRONTEND_PATHS.LOGOUT + }, + { // has a render block test + name: "signed in, signing out, on root path, redirects to logout", + initialPath: "/", + authStateOverrides: { + isSignedIn: true, + isSigningOut: true + }, + expectedPath: FRONTEND_PATHS.LOGOUT + }, + { + name: "signed in, not signing out, on logout path, redirects to logout", + initialPath: FRONTEND_PATHS.LOGOUT, + authStateOverrides: { + isSignedIn: true, + isSigningOut: false + }, + expectedPath: FRONTEND_PATHS.LOGOUT + } + ] + + it.each(redirectScenarios)( + "Redirection expected - $name", + async ({ + initialPath, + authStateOverrides, + expectedPath + }) => { + renderWithProvider(authStateOverrides, initialPath) + + await waitFor(() => { + expect(mockNavigate).toHaveBeenCalledWith(expectedPath) + }) + } + ) + + it.each(noRedirectScenarios)( + "No redirection expected - $name", + async ({ + initialPath, + authStateOverrides + }) => { + renderWithProvider(authStateOverrides, initialPath) + + expect(mockNavigate).not.toHaveBeenCalled() + }) + + it.each(logoutScenarios)( + "Logout expected - $name", + async ({ + initialPath, + authStateOverrides + }) => { + renderWithProvider(authStateOverrides, initialPath) + + await waitFor(() => { + expect(mockNavigate).not.toHaveBeenCalled() + expect(mockHandleSignoutEvent).toHaveBeenCalledWith( + expect.objectContaining(authStateOverrides), mockNavigate, + expect.anything(), authStateOverrides.invalidSessionCause + ) + }) + }) + + it("redirects to logout for non-initiating tab when recent logout marker exists and multiple tabs are open", + async () => { + jest.mocked(getOrCreateTabId).mockReturnValue("tab-b") + jest.mocked(getOpenTabCount).mockReturnValue(2) + localStorage.setItem( + LOGOUT_MARKER_STORAGE_GROUP, + JSON.stringify({ + [LOGOUT_MARKER_STORAGE_KEY]: { + timestamp: Date.now(), + reason: "signOut", + initiatedByTabId: "tab-a" + } + }) + ) + + renderWithProvider( + { + isSignedIn: false, + isSigningIn: false, + isSigningOut: false, + selectedRole: undefined + }, + FRONTEND_PATHS.SEARCH_BY_PRESCRIPTION_ID + ) + + await waitFor(() => { + expect(mockNavigate).toHaveBeenCalledWith(FRONTEND_PATHS.LOGOUT) + }) + }) + + it("does not force logout redirect for initiating tab even when marker is recent", async () => { + jest.mocked(getOrCreateTabId).mockReturnValue("tab-a") + jest.mocked(getOpenTabCount).mockReturnValue(2) + localStorage.setItem( + LOGOUT_MARKER_STORAGE_GROUP, + JSON.stringify({ + [LOGOUT_MARKER_STORAGE_KEY]: { + timestamp: Date.now(), + reason: "signOut", + initiatedByTabId: "tab-a" + } + }) + ) + + renderWithProvider( + { + isSignedIn: false, + isSigningIn: false, + isSigningOut: false, + selectedRole: undefined + }, + FRONTEND_PATHS.SEARCH_BY_PRESCRIPTION_ID + ) + + // Initiating tab should not be force-redirected; recent logout marker causes early return + expect(mockNavigate).not.toHaveBeenCalled() + }) +}) diff --git a/packages/cpt-ui/__tests__/AccessProvider.shouldBlockChildren.test.tsx b/packages/cpt-ui/__tests__/AccessProvider.shouldBlockChildren.test.tsx new file mode 100644 index 0000000000..73cf5941da --- /dev/null +++ b/packages/cpt-ui/__tests__/AccessProvider.shouldBlockChildren.test.tsx @@ -0,0 +1,370 @@ +import React from "react" +import {render, waitFor, screen} from "@testing-library/react" +import {AccessProvider} from "@/context/AccessProvider" +import {AuthContext, AuthContextType} from "@/context/AuthProvider" +import {MemoryRouter} from "react-router-dom" +import {mockAuthState} from "./mocks/AuthStateMock" +import {FRONTEND_PATHS} from "@/constants/environment" + +jest.mock("@/components/EpsHeader", () => ({ + __esModule: true, + default: jest.fn(() => null) +})) + +jest.mock("@/helpers/logger", () => ({ + logger: { + debug: jest.fn(), + info: jest.fn(), + warn: jest.fn(), + error: jest.fn() + } +})) + +jest.mock("@/helpers/logout", () => ({ + handleSignoutEvent: jest.fn(), + signOut: jest.fn(), + checkForRecentLogoutMarker: jest.fn().mockReturnValue(false) +})) + +jest.mock("@/helpers/tabHelpers", () => { + const actual = jest.requireActual("@/helpers/tabHelpers") + return { + ...actual, + getOrCreateTabId: jest.fn().mockReturnValue("default-tab"), + getOpenTabCount: jest.fn().mockReturnValue(1), + updateOpenTabs: jest.fn(), + heartbeatTab: jest.fn(), + pruneStaleTabIds: jest.fn() + } +}) + +// Mock useNavigate and assign to a variable for assertions +const mockNavigate = jest.fn() +jest.mock("react-router-dom", () => { + const actual = jest.requireActual("react-router-dom") + return { + ...actual, + useNavigate: () => mockNavigate + } +}) + +describe("shouldBlockChildren", () => { + beforeEach(() => { + jest.clearAllMocks() + localStorage.clear() + }) + + const renderWithProvider = ( + authStateOverrides: Partial = {}, + initialPath: string + ) => { + const authContextValue: AuthContextType = { + ...mockAuthState, + updateTrackerUserInfo: jest.fn().mockResolvedValue({error: null}), + clearAuthState: jest.fn(), + ...authStateOverrides + } + + render( + + + <>

Not blocked

+ + + + ) + } + + type Scenario = { + name: string; + initialPath: string; + authStateOverrides: Partial; + } + + const blockingScenarios: Array = [ + { + name: "not signed in, not signing in, on protected path", // has a redirection test + initialPath: FRONTEND_PATHS.SEARCH_BY_PRESCRIPTION_ID, + authStateOverrides: { + isSignedIn: false, + isSigningIn: false, + selectedRole: undefined + } + }, + { + name: "not signed in, not signing in, on protected path, with logout marker", // has a redirection test + initialPath: FRONTEND_PATHS.SEARCH_BY_PRESCRIPTION_ID, + authStateOverrides: { + isSignedIn: false, + isSigningIn: false, + selectedRole: undefined, + logoutMarker: { + timestamp: Date.now(), + reason: "signOut", + initiatedByTabId: "tab-a" + } + } + }, + { + name: "not signed in, not signing in, on protected path, with stale logout marker", // has a redirection test + initialPath: FRONTEND_PATHS.SEARCH_BY_PRESCRIPTION_ID, + authStateOverrides: { + isSignedIn: false, + isSigningIn: false, + selectedRole: undefined, + logoutMarker: { + timestamp: Date.now() - 60000, + reason: "signOut", + initiatedByTabId: "tab-a" + } + } + }, + { // has a redirection test + name: "not signed in, signing in, on select your role, with no search params", + initialPath: FRONTEND_PATHS.SELECT_YOUR_ROLE, + authStateOverrides: { + isSignedIn: false, + isSigningIn: true + } + }, + { // has a redirection test + name: "not signed in, signing in, on select your role, with search params", + initialPath: FRONTEND_PATHS.SELECT_YOUR_ROLE + "?code=test", + authStateOverrides: { + isSignedIn: false, + isSigningIn: true + } + }, + { + name: "not signed in, signing in, on protected path", // has a redirection test + initialPath: FRONTEND_PATHS.SEARCH_BY_PRESCRIPTION_ID, + authStateOverrides: { + isSignedIn: false, + isSigningIn: true, + selectedRole: undefined + } + }, + { + name: "not signed in, signing out, has invalid session cause, on protected path", // has a redirection test + initialPath: FRONTEND_PATHS.SEARCH_BY_PRESCRIPTION_ID, + authStateOverrides: { + isSignedIn: false, + isSigningIn: false, + isSigningOut: true, + invalidSessionCause: "Session expired", + selectedRole: undefined + } + }, + { + name: "signed in, concurrent session, on protected page", // has a redirection test + initialPath: FRONTEND_PATHS.SEARCH_BY_PRESCRIPTION_ID, + authStateOverrides: { + isSignedIn: true, + isSigningIn: false, + isSigningOut: false, + isConcurrentSession: true, + selectedRole: undefined + } + }, + { + name: "signed in, no selected role, on protected path", // has a redirection test + initialPath: FRONTEND_PATHS.SEARCH_BY_PRESCRIPTION_ID, + authStateOverrides: { + isSignedIn: true, + isSigningIn: false, + isSigningOut: false, + isConcurrentSession: false, + selectedRole: undefined + } + }, + { + name: "signed in, has selected role, on root path", // has redirection test + initialPath: "/", + authStateOverrides: { + isSignedIn: true, + isSigningIn: false, + isSigningOut: false, + isConcurrentSession: false, + selectedRole: {"role_name": "Test Role"} + } + }, + { + name: "signed in, has selected role, on login", // has redirection test + initialPath: FRONTEND_PATHS.LOGIN, + authStateOverrides: { + isSignedIn: true, + isSigningIn: false, + isSigningOut: false, + isConcurrentSession: false, + selectedRole: {"role_name": "Test Role"} + } + }, + { + name: "signed in, signing in, on protected path", // has a redirection test + initialPath: FRONTEND_PATHS.SEARCH_BY_PRESCRIPTION_ID, + authStateOverrides: { + isSignedIn: true, + isSigningIn: true, + isSigningOut: false, + isConcurrentSession: false, + selectedRole: undefined + } + }, + { + name: "signed in, signing out, on protected path", // has a redirection test + initialPath: FRONTEND_PATHS.SEARCH_BY_PRESCRIPTION_ID, + authStateOverrides: { + isSignedIn: true, + isSigningIn: false, + isSigningOut: true, + isConcurrentSession: false, + selectedRole: undefined + } + }, + { + name: "signed in, not signing out, has recent logout marker", // has a redirection test + initialPath: FRONTEND_PATHS.SEARCH_BY_PRESCRIPTION_ID, + authStateOverrides: { + isSignedIn: true, + isSigningOut: false, + logoutMarker: { + timestamp: Date.now(), + reason: "signOut", + initiatedByTabId: "tab-a" + } + } + } + ] + + const nonBlockingScenarios: Array = [ + { + name: "not signed in, on public path", // has a redirection test + initialPath: FRONTEND_PATHS.COOKIES, + authStateOverrides: { + isSignedIn: false, + isSigningIn: false, + selectedRole: undefined + } + }, + { + name: "not signed in, on root path", // has a redirection test + initialPath: "/", + authStateOverrides: { + isSignedIn: false, + isSigningIn: false, + selectedRole: undefined + } + }, + { + name: "signed in, on public path", + initialPath: FRONTEND_PATHS.COOKIES, + authStateOverrides: { + isSignedIn: true, + isSigningIn: false, + selectedRole: undefined + } + }, + { + name: "signed in, concurrent session, no selected role, on public path", + initialPath: FRONTEND_PATHS.COOKIES, + authStateOverrides: { + isSignedIn: true, + isSigningIn: false, + isConcurrentSession: true, + selectedRole: undefined + } + }, + { + name: "signed in, concurrent session, with selected role, on public path", + initialPath: FRONTEND_PATHS.COOKIES, + authStateOverrides: { + isSignedIn: true, + isSigningIn: false, + isConcurrentSession: true, + selectedRole: {"role_name": "Test Role"} + } + }, + { + name: "signed in, role selected, on public path", // has a redirection test + initialPath: FRONTEND_PATHS.COOKIES, + authStateOverrides: { + isSignedIn: true, + isSigningIn: false, + selectedRole: {"role_name": "Test Role"} + } + }, + { + name: "signed in, on allowed no-role path", // has a redirection test + initialPath: FRONTEND_PATHS.SELECT_YOUR_ROLE, + authStateOverrides: { + isSignedIn: true, + isSigningIn: false, + selectedRole: undefined + } + }, + { + name: "signed in, role selected, on protected path", // has a redirection test + initialPath: FRONTEND_PATHS.SEARCH_BY_NHS_NUMBER, + authStateOverrides: { + isSignedIn: true, + isSigningIn: false, + selectedRole: {"role_name": "Test Role"} + } + }, + { + name: "signed in, role selected, on allowed no roles path, doesnt redirect", // has a redirection test + initialPath: FRONTEND_PATHS.SELECT_YOUR_ROLE, + authStateOverrides: { + isSignedIn: true, + isSigningIn: false, + selectedRole: {role_name: "Test Role"} + } + }, + { + name: "signed in, signing out, on public path, redirects to logout", // has a redirection test + initialPath: FRONTEND_PATHS.COOKIES, + authStateOverrides: { + isSignedIn: true, + isSigningOut: true + } + }, + { + name: "signed in, signing out, on root path, redirects to logout", // has a redirection test + initialPath: "/", + authStateOverrides: { + isSignedIn: true, + isSigningOut: true + } + } + ] + + it.each(blockingScenarios)( + "Blocking expected - $name", + async ({ + initialPath, + authStateOverrides + }) => { + renderWithProvider(authStateOverrides, initialPath) + + await waitFor(() => { + expect(screen.getByTestId("loading-page-header")).toBeInTheDocument() + }) + } + ) + + it.each(nonBlockingScenarios)( + "Non-blocking expected - $name", + async ({ + initialPath, + authStateOverrides + }) => { + renderWithProvider(authStateOverrides, initialPath) + + await waitFor(() => { + // expect(logger.info).toHaveBeenCalledWith("value") + expect(screen.getByTestId("not-blocked-header")).toBeInTheDocument() + expect(screen.queryByTestId("loading-page-header")).not.toBeInTheDocument() + }) + } + ) +}) diff --git a/packages/cpt-ui/__tests__/AccessProvider.test.tsx b/packages/cpt-ui/__tests__/AccessProvider.test.tsx index 72efa33304..96e3973abe 100644 --- a/packages/cpt-ui/__tests__/AccessProvider.test.tsx +++ b/packages/cpt-ui/__tests__/AccessProvider.test.tsx @@ -6,7 +6,7 @@ import {useAuth as mockUseAuth} from "@/context/AuthProvider" import {useNavigate, useLocation, MemoryRouter} from "react-router-dom" import {normalizePath as mockNormalizePath} from "@/helpers/utils" import {logger} from "@/helpers/logger" -import {handleRestartLogin} from "@/helpers/logout" +import {handleSignoutEvent} from "@/helpers/logout" import Layout from "@/Layout" import LoadingPage from "@/pages/LoadingPage" import {mockAuthState} from "./mocks/AuthStateMock" @@ -50,7 +50,8 @@ jest.mock("@/constants/environment", () => ({ "/session-logged-out", "/cookies-selected", "/", - "/select-active-session" + "/select-active-session", + "/select-your-role" ], PUBLIC_PATHS: [ "/login", @@ -82,14 +83,27 @@ jest.mock("@/helpers/logger", () => ({ })) jest.mock("@/helpers/logout", () => ({ - handleRestartLogin: jest.fn(), - signOut: jest.fn() + handleSignoutEvent: jest.fn(), + signOut: jest.fn(), + checkForRecentLogoutMarker: jest.fn().mockReturnValue(false) })) jest.mock("@/helpers/userInfo", () => ({ updateRemoteSelectedRole: jest.fn().mockResolvedValue({currentlySelectedRole: {}}) })) +jest.mock("@/helpers/tabHelpers", () => { + const actual = jest.requireActual("@/helpers/tabHelpers") + return { + ...actual, + getOrCreateTabId: jest.fn().mockReturnValue("default-tab"), + getOpenTabCount: jest.fn().mockReturnValue(1), + updateOpenTabs: jest.fn(), + heartbeatTab: jest.fn(), + pruneStaleTabIds: jest.fn() + } +}) + const TestComponent = () => { useAccess() // Just to test the context return
Test Component
@@ -107,6 +121,7 @@ describe("AccessProvider", () => { jest.useFakeTimers() mockNavigateHook.mockReturnValue(navigate) + mockNormalizePathFn.mockImplementation((path: string) => path) }) afterEach(() => { @@ -199,7 +214,7 @@ describe("AccessProvider", () => { it("skips redirection logic when signing in and on select-your-role path", () => { mockAuthHook.mockReturnValue({ ...mockAuthState, - isSignedIn: false, + isSignedIn: true, isSigningIn: true, updateTrackerUserInfo: jest.fn().mockResolvedValue({error: null}), clearAuthState: jest.fn(), @@ -250,13 +265,14 @@ describe("AccessProvider", () => { clearAuthState: jest.fn(), updateInvalidSessionCause: jest.fn() }) - mockLocationHook.mockReturnValue({pathname: "/"}) - mockNormalizePathFn.mockReturnValue("/") + const path = "/" + mockLocationHook.mockReturnValue({pathname: path}) + mockNormalizePathFn.mockReturnValue(path) renderWithProvider() expect(navigate).toHaveBeenCalledWith(FRONTEND_PATHS.SEARCH_BY_PRESCRIPTION_ID) - expect(logger.info).toHaveBeenCalledWith("Authenticated user on root path - redirecting to search") + expect(logger.info).toHaveBeenCalledWith(`Signed-in user on ${path} - blocking render until redirect`) }) describe("shouldBlockChildren", () => { @@ -415,7 +431,7 @@ describe("AccessProvider", () => { clearIntervalSpy.mockRestore() }) - it("should skip user info check when isSigningIn is true", async () => { + it("shouldn't skip user info check when isSigningIn is true on protected page", async () => { mockAuthHook.mockReturnValue({ ...mockAuthState, isSignedIn: true, @@ -423,7 +439,7 @@ describe("AccessProvider", () => { selectedRole: {name: "TestRole"}, updateTrackerUserInfo: mockUpdateTrackerUserInfo }) - mockLocationHook.mockReturnValue({pathname: "/search-by-prescription-id"}) + mockLocationHook.mockReturnValue({pathname: "/search-by-prescription-id"}) // protected page renderWithProvider() @@ -432,9 +448,9 @@ describe("AccessProvider", () => { }) expect(logger.debug).toHaveBeenCalledWith( - "Not checking user info" + "Refreshing user info" ) - expect(mockUpdateTrackerUserInfo).not.toHaveBeenCalled() + expect(mockUpdateTrackerUserInfo).toHaveBeenCalled() }) it("should skip user info check when on allowed no-role paths", async () => { @@ -502,7 +518,8 @@ describe("AccessProvider", () => { }) expect(mockUpdateTrackerUserInfo).toHaveBeenCalled() - expect(handleRestartLogin).toHaveBeenCalledWith(authContext, "InvalidSession") + expect(handleSignoutEvent).toHaveBeenCalledWith(authContext, + navigate, expect.anything(), "InvalidSession") }) it("should not call updateTrackerUserInfo when user is not signed in", async () => { @@ -522,7 +539,7 @@ describe("AccessProvider", () => { jest.advanceTimersByTime(60001) }) - expect(logger.debug).toHaveBeenCalledWith("Not checking user info") + expect(logger.debug).toHaveBeenCalledWith("No conditions met - not checking user info") expect(mockUpdateTrackerUserInfo).not.toHaveBeenCalled() }) @@ -569,8 +586,13 @@ describe("AccessProvider", () => { jest.advanceTimersByTime(60001) }) - expect(handleRestartLogin).toHaveBeenCalledWith(authContext, "InvalidSession") + expect(handleSignoutEvent).toHaveBeenCalledWith(authContext, + navigate, expect.anything(), "InvalidSession") expect(mockUpdateTrackerUserInfo).toHaveBeenCalledTimes(2) + + // Verify the interval continues to exist and can be cleared + // No need to test second execution since handleSignoutEvent changes auth state + // which causes useEffect to re-run and reset the interval }) }) @@ -598,8 +620,8 @@ describe("AccessProvider", () => { }) it("should show timeout modal when remaining time is within threshold", async () => { - const oneMinuteInMs = 60 * 1000 - const remainingTime = oneMinuteInMs // 1 minute remaining, within 2-minute threshold + const twoMinutesMs = 2 * 60 * 1000 + const remainingTime = twoMinutesMs - 1000 // 1 second less than threshold mockUpdateTrackerUserInfo.mockResolvedValue({ error: null, remainingSessionTime: remainingTime @@ -646,7 +668,8 @@ describe("AccessProvider", () => { expect(logger.warn).toHaveBeenCalledWith("Session expired - automatically logging out user") expect(authContext.updateInvalidSessionCause).toHaveBeenCalledWith("Timeout") - expect(handleRestartLogin).toHaveBeenCalledWith(authContext, "Timeout") + expect(handleSignoutEvent).toHaveBeenCalledWith(authContext, + navigate, "Timeout") }) it("should hide modal when session is still valid", async () => { @@ -699,7 +722,7 @@ describe("AccessProvider", () => { "No remainingSessionTime in response - session may be corrupted, logging out user" ) expect(authContext.updateInvalidSessionCause).toHaveBeenCalledWith("InvalidSession") - expect(handleRestartLogin).toHaveBeenCalledWith(authContext, "InvalidSession") + expect(handleSignoutEvent).toHaveBeenCalledWith(authContext, navigate, "InvalidSession") }) }) }) diff --git a/packages/cpt-ui/__tests__/AuthProvider.test.tsx b/packages/cpt-ui/__tests__/AuthProvider.test.tsx index 9bfb88f76a..88db4b9651 100644 --- a/packages/cpt-ui/__tests__/AuthProvider.test.tsx +++ b/packages/cpt-ui/__tests__/AuthProvider.test.tsx @@ -200,7 +200,7 @@ describe("AuthProvider", () => { // Error Handling - it("should log an error if signOut fails", async () => { + it("should throw an error if signOut fails", async () => { const loggerErrorSpy = jest.spyOn(logger, "error") const signOutError = new Error("Sign out failed"); (signOut as jest.Mock).mockRejectedValue(signOutError as never) @@ -223,13 +223,9 @@ describe("AuthProvider", () => { }) await act(async () => { - await contextValue.cognitoSignOut() + await expect(contextValue.cognitoSignOut()).rejects.toThrow("Failed to sign out") }) - expect(loggerErrorSpy).toHaveBeenCalledWith( - "Failed to sign out:", - signOutError - ) loggerErrorSpy.mockRestore() }) @@ -348,10 +344,11 @@ describe("AuthProvider", () => { await contextValue.cognitoSignIn() }) expect(signInWithRedirect).toHaveBeenCalled() - expect(contextValue.isSigningIn).toBe(true) }) it("should provide cognitoSignOut functions", async () => { + (signOut as jest.Mock).mockResolvedValue(undefined) + // eslint-disable-next-line @typescript-eslint/no-explicit-any let contextValue: any const TestComponent = () => { @@ -375,6 +372,38 @@ describe("AuthProvider", () => { expect(signOut).toHaveBeenCalled() }) + // TODO: This is a signOut helper test + // it("should set logout marker when signOut is called", async () => { + // let contextValue: AuthContextType | null = null + + // const TestComponent = () => { + // contextValue = useContext(AuthContext) + // return null + // } + + // localStorage.removeItem(LOGOUT_MARKER_STORAGE_GROUP) + + // await act(async () => { + // render( + // + // + // + // + // + // ) + // }) + + // await act(async () => { + // await contextValue?.setStateForSignOut() + // }) + + // const markerGroup = JSON.parse(localStorage.getItem(LOGOUT_MARKER_STORAGE_GROUP) ?? "{}") + // expect(markerGroup[LOGOUT_MARKER_STORAGE_KEY]).toEqual( + // expect.objectContaining({reason: "signOut"}) + // ) + // expect(typeof markerGroup[LOGOUT_MARKER_STORAGE_KEY].timestamp).toBe("number") + // }) + it( "should call updateRemoteSelectedRole and update selectedRole state when updateSelectedRole is called", async () => { diff --git a/packages/cpt-ui/__tests__/CookiePolicyPage.test.tsx b/packages/cpt-ui/__tests__/CookiePolicyPage.test.tsx index 141c1352d5..d270dc87a1 100644 --- a/packages/cpt-ui/__tests__/CookiePolicyPage.test.tsx +++ b/packages/cpt-ui/__tests__/CookiePolicyPage.test.tsx @@ -17,13 +17,6 @@ jest.mock("@/helpers/awsRum", () => ({ } })) -jest.mock("@/constants/environment", () => ({ - FRONTEND_PATHS: { - SEARCH_BY_PRESCRIPTION_ID: "/search", - LOGIN: "/login" - } -})) - jest.mock("@/constants/ui-strings/CookieStrings", () => ({ CookieStrings: { banner : { diff --git a/packages/cpt-ui/__tests__/EpsRoleSelectionPage.test.tsx b/packages/cpt-ui/__tests__/EpsRoleSelectionPage.test.tsx index 20d4ed491f..71be8a4b82 100644 --- a/packages/cpt-ui/__tests__/EpsRoleSelectionPage.test.tsx +++ b/packages/cpt-ui/__tests__/EpsRoleSelectionPage.test.tsx @@ -11,20 +11,20 @@ import {useAuth, AuthContextType} from "@/context/AuthProvider" import {MemoryRouter, useNavigate} from "react-router-dom" import {FRONTEND_PATHS} from "@/constants/environment" import {getSearchParams} from "@/helpers/getSearchParams" -import {handleRestartLogin, signOut} from "@/helpers/logout" +import {handleSignoutEvent} from "@/helpers/logout" import axios from "axios" import {RoleDetails} from "@cpt-ui-common/common-types" import {logger} from "@/helpers/logger" import {mockAuthState} from "./mocks/AuthStateMock" -import {LOADING_STRINGS} from "@/constants/ui-strings/LoadingPage" jest.mock("@/context/AuthProvider") jest.mock("@/helpers/getSearchParams") jest.mock("@/helpers/logout", () => ({ - handleRestartLogin: jest.fn(), + handleSignoutEvent: jest.fn(), signOut: jest.fn().mockImplementation((auth: AuthContextType) => { auth.isSigningOut = true - }) + }), + checkForRecentLogoutMarker: jest.fn().mockImplementation(() => undefined) })) jest.mock("@/helpers/axios", () => ({ __esModule: true, @@ -104,26 +104,6 @@ describe("RoleSelectionPage", () => { //windowSpy.mockRestore() }) - it("renders loading spinner if redirecting during sign in", () => { - mockUseAuth.mockReturnValue({ - isSigningIn: true, - rolesWithAccess: [], - rolesWithoutAccess: [], - error: null, - clearAuthState: jest.fn(), - hasSingleRoleAccess: jest.fn().mockReturnValue(false) - }) - mockGetSearchParams.mockReturnValue({ - codeParams: "foo", - stateParams: "bar" - }) - - render( - - ) - expect(screen.getByRole("heading", {name: `${LOADING_STRINGS.HEADER}`})).toBeInTheDocument() - }) - it("renders error message if auth.error exists", () => { mockUseAuth.mockReturnValue({ isSigningIn: false, @@ -156,45 +136,6 @@ describe("RoleSelectionPage", () => { expect(screen.getByText("Please contact support.")).toBeInTheDocument() }) - it("redirects to login if logging in but not in callback", async () => { - const navigateMock = jest.fn() - mockNavigate.mockReturnValue(navigateMock) - const authState = { - ...mockAuthState, - isSigningIn: true - } - mockUseAuth.mockReturnValue(authState) - mockGetSearchParams.mockReturnValue({ - codeParams: undefined, - stateParams: undefined - }) - - const {rerender} = render( - - - - ) - - // Mock signOut to update the authState AND refresh the mock - ;(signOut as jest.Mock).mockImplementation((authState: AuthContextType) => { - authState.isSigningOut = true - mockUseAuth.mockReturnValue(authState) // Update what useAuth returns - }) - - await act(async () => { - signOut(authState) - }) - - // Rerender to pick up the new auth state - rerender( - - - - ) - - expect(screen.getByRole("heading", {name: `${LOADING_STRINGS.HEADER}`})).toBeInTheDocument() - }) - it("redirects if user has single roleWithAccess", () => { const navigateMock = jest.fn() mockNavigate.mockReturnValue(navigateMock) @@ -354,6 +295,7 @@ describe("RoleSelectionPage", () => { // eslint-disable-next-line no-undef jest.spyOn(global.crypto, "randomUUID").mockReturnValueOnce("some-log-id-uuid-value") mockUseAuth.mockReturnValue({ + ...mockAuthState, sessionId: "session-1234", user: "cognito-user", userDetails: { @@ -413,6 +355,7 @@ describe("RoleSelectionPage", () => { // eslint-disable-next-line no-undef jest.spyOn(global.crypto, "randomUUID").mockReturnValueOnce("some-log-id-uuid-value") mockUseAuth.mockReturnValue({ + ...mockAuthState, sessionId: "session-1234", user: "cognito-user", userDetails: { @@ -480,6 +423,7 @@ describe("RoleSelectionPage", () => { // eslint-disable-next-line no-undef jest.spyOn(global.crypto, "randomUUID").mockReturnValueOnce("some-log-id-uuid-value") mockUseAuth.mockReturnValue({ + ...mockAuthState, sessionId: "session-1234", user: "cognito-user", userDetails: { @@ -888,7 +832,6 @@ describe("RoleSelectionPage", () => { ) - expect(screen.getByRole("heading", {name: `${LOADING_STRINGS.HEADER}`})).toBeInTheDocument() // Step 2: Simulate login complete and role assignment act(() => { @@ -1068,9 +1011,9 @@ describe("RoleSelectionPage", () => { }) await waitFor(() => { - expect(handleRestartLogin).toHaveBeenCalledWith( - expect.objectContaining({updateSelectedRole: mockUpdateSelectedRole}), - "session_expired" + expect(handleSignoutEvent).toHaveBeenCalledWith( + expect.objectContaining({updateSelectedRole: mockUpdateSelectedRole}), mockNavigate, + expect.anything(), "session_expired" ) }) }) @@ -1178,37 +1121,4 @@ describe("RoleSelectionPage", () => { expect(cards).toHaveLength(1) }) }) - describe("Loading Page Interactions", () => { - it("Render loading when user clicks signout", async () => { - const authState = { - ...mockAuthState, - isSignedIn: true - } - mockUseAuth.mockReturnValue(authState) - const {rerender} = render( - - - - ) - - // Mock signOut to update the authState AND refresh the mock - ;(signOut as jest.Mock).mockImplementation((authState: AuthContextType) => { - authState.isSigningOut = true - mockUseAuth.mockReturnValue(authState) // Update what useAuth returns - }) - - await act(async () => { - signOut(authState) - }) - - // Rerender to pick up the new auth state - rerender( - - - - ) - - expect(screen.getByRole("heading", {name: `${LOADING_STRINGS.HEADER}`})).toBeInTheDocument() - }) - }) }) diff --git a/packages/cpt-ui/__tests__/LoginFunctions.test.tsx b/packages/cpt-ui/__tests__/LoginFunctions.test.tsx index a943c38a77..867e33df08 100644 --- a/packages/cpt-ui/__tests__/LoginFunctions.test.tsx +++ b/packages/cpt-ui/__tests__/LoginFunctions.test.tsx @@ -1,20 +1,128 @@ -import {getHomeLink} from "@/helpers/loginFunctions" +import {AUTH_CONFIG} from "@/constants/environment" +import {handleSignIn, getHomeLink} from "@/helpers/loginFunctions" +import {logger} from "@/helpers/logger" +import {checkForRecentLogoutMarker, signOut} from "@/helpers/logout" +import {mockAuthState} from "./mocks/AuthStateMock" + +jest.mock("@/helpers/logger", () => ({ + logger: { + info: jest.fn(), + error: jest.fn() + } +})) + +jest.mock("@/helpers/logout", () => ({ + signOut: jest.fn(), + checkForRecentLogoutMarker: jest.fn() +})) describe("getHomeLink", () => { beforeEach(() => { - localStorage.clear() + jest.clearAllMocks() }) + it("returns /login when user is not logged in", () => { expect(getHomeLink(false)).toBe("/login") }) + it("returns /search-by-prescription-id when user is logged in", () => { - const validAuth = { - isSignedIn: true, - user: {username: "mockUser"}, - idToken: {}, - accessToken: {} - } - localStorage.setItem("auth", JSON.stringify(validAuth)) expect(getHomeLink(true)).toBe("/search-by-prescription-id") }) }) + +describe("handleSignIn", () => { + const mockNavigate = jest.fn() + + beforeEach(() => { + jest.clearAllMocks() + ;(checkForRecentLogoutMarker as jest.Mock).mockReturnValue(undefined) + }) + + it("sets sign in state and calls cognitoSignIn for Primary", async () => { + const mockAuth = { + ...mockAuthState, + cognitoSignIn: jest.fn().mockResolvedValue(undefined) + } + + await handleSignIn(mockAuth, "Primary", mockNavigate) + + expect(mockAuth.setStateForSignIn).toHaveBeenCalledTimes(1) + expect(mockAuth.cognitoSignIn).toHaveBeenCalledWith({ + provider: { + custom: "Primary" + } + }) + expect(signOut).not.toHaveBeenCalled() + }) + + it("calls cognitoSignIn for Mock", async () => { + const mockAuth = { + ...mockAuthState, + cognitoSignIn: jest.fn().mockResolvedValue(undefined) + } + + await handleSignIn(mockAuth, "Mock", mockNavigate) + + expect(mockAuth.cognitoSignIn).toHaveBeenCalledWith({ + provider: { + custom: "Mock" + } + }) + }) + + it("signs out when already authenticated error is thrown", async () => { + const alreadyAuthenticatedError = {name: "UserAlreadyAuthenticatedException"} + const mockAuth = { + ...mockAuthState, + cognitoSignIn: jest.fn().mockRejectedValue(alreadyAuthenticatedError) + } + + await handleSignIn(mockAuth, "Primary", mockNavigate) + + expect(signOut).toHaveBeenCalledWith( + mockAuth, + mockNavigate, + AUTH_CONFIG.REDIRECT_SIGN_OUT, + true + ) + expect(logger.error).toHaveBeenCalledWith( + "Error during Primary sign in:", + alreadyAuthenticatedError + ) + }) + + it("does not sign out for non-authenticated errors", async () => { + const unknownError = new Error("Unknown sign in failure") + const mockAuth = { + ...mockAuthState, + cognitoSignIn: jest.fn().mockRejectedValue(unknownError) + } + + await handleSignIn(mockAuth, "Mock", mockNavigate) + + expect(signOut).not.toHaveBeenCalled() + expect(logger.error).toHaveBeenCalledWith("Error during Mock sign in:", unknownError) + }) + + it("logs when a recent logout marker exists and still signs in", async () => { + ;(checkForRecentLogoutMarker as jest.Mock).mockReturnValue({ + timestamp: Date.now(), + reason: "signOut", + initiatedByTabId: "tab-1" + }) + const mockAuth = { + ...mockAuthState, + cognitoSignIn: jest.fn().mockResolvedValue(undefined) + } + + await handleSignIn(mockAuth, "Primary", mockNavigate) + + expect(logger.info).toHaveBeenCalledWith("Attempting to sign-out, prohibit sign as well") + expect(mockAuth.setStateForSignIn).toHaveBeenCalledTimes(1) + expect(mockAuth.cognitoSignIn).toHaveBeenCalledWith({ + provider: { + custom: "Primary" + } + }) + }) +}) diff --git a/packages/cpt-ui/__tests__/LogoutPage.test.tsx b/packages/cpt-ui/__tests__/LogoutPage.test.tsx index 46385347dd..11f08b8fac 100644 --- a/packages/cpt-ui/__tests__/LogoutPage.test.tsx +++ b/packages/cpt-ui/__tests__/LogoutPage.test.tsx @@ -18,6 +18,16 @@ jest.mock("@/constants/environment", () => ({ PRIVACY_NOTICE: "/privacy-notice", COOKIES_SELECTED: "/cookies-selected" }, + PUBLIC_PATHS: [ + "/login", + "/logout", + "/session-logged-out", + "/cookies", + "/privacy-notice", + "/cookies-selected", + "/notfound", + "/" + ], ALLOWED_NO_ROLE_PATHS: [ "/login", "/logout", @@ -48,6 +58,7 @@ jest.mock("@/helpers/utils", () => ({ jest.mock("@/helpers/logger", () => ({ logger: { info: jest.fn(), + warn: jest.fn(), debug: jest.fn() } })) @@ -81,7 +92,7 @@ const MockAuthProvider = ({ clearAuthState: jest.fn(), hasSingleRoleAccess: jest.fn().mockReturnValue(false), updateSelectedRole: jest.fn(), - updateTrackerUserInfo: jest.fn(), + updateTrackerUserInfo: jest.fn().mockResolvedValue({error: null}), updateInvalidSessionCause: jest.fn(), isSigningOut: false, setIsSigningOut: jest.fn(), @@ -113,26 +124,4 @@ describe("LogoutPage", () => { ).toBeInTheDocument() expect(screen.getByRole("link", {name: /log in/i})).toBeInTheDocument() }) - - it("shows a spinner and calls signOut when the user is signed in", async () => { - render( - - - - ) - - expect(screen.getByText(/Logging out/i)).toBeInTheDocument() - expect(screen.getByRole("status")).toBeInTheDocument() - }) - - it("does not call signOut if user is signed in, but we haven't advanced timers yet", () => { - render( - - - - ) - - expect(screen.getByText(/Logging out/i)).toBeInTheDocument() - expect(screen.getByRole("status")).toBeInTheDocument() - }) }) diff --git a/packages/cpt-ui/__tests__/PrescriptionListPage.test.tsx b/packages/cpt-ui/__tests__/PrescriptionListPage.test.tsx index e00213ac37..6a2fc5dad5 100644 --- a/packages/cpt-ui/__tests__/PrescriptionListPage.test.tsx +++ b/packages/cpt-ui/__tests__/PrescriptionListPage.test.tsx @@ -53,7 +53,7 @@ import http from "@/helpers/axios" // Mock logout helpers jest.mock("@/helpers/logout", () => ({ - handleRestartLogin: jest.fn(), + handleSignoutEvent: jest.fn(), signOut: jest.fn() })) @@ -494,8 +494,10 @@ describe("PrescriptionListPage", () => { ) await waitFor(() => { - expect(logoutHelpers.handleRestartLogin).toHaveBeenCalledWith( + expect(logoutHelpers.handleSignoutEvent).toHaveBeenCalledWith( mockAuth, + mockNavigate, + "PrescriptionListPage", "InvalidSession" ) }) @@ -531,8 +533,10 @@ describe("PrescriptionListPage", () => { ) await waitFor(() => { - expect(logoutHelpers.handleRestartLogin).toHaveBeenCalledWith( + expect(logoutHelpers.handleSignoutEvent).toHaveBeenCalledWith( mockAuth, + mockNavigate, + "PrescriptionListPage", "ConcurrentSession" ) }) @@ -568,8 +572,10 @@ describe("PrescriptionListPage", () => { ) await waitFor(() => { - expect(logoutHelpers.handleRestartLogin).toHaveBeenCalledWith( + expect(logoutHelpers.handleSignoutEvent).toHaveBeenCalledWith( mockAuth, + mockNavigate, + "PrescriptionListPage", "Timeout" ) }) @@ -605,8 +611,10 @@ describe("PrescriptionListPage", () => { ) await waitFor(() => { - expect(logoutHelpers.handleRestartLogin).toHaveBeenCalledWith( + expect(logoutHelpers.handleSignoutEvent).toHaveBeenCalledWith( mockAuth, + mockNavigate, + "PrescriptionListPage", undefined ) }) diff --git a/packages/cpt-ui/__tests__/RBACBanner.test.tsx b/packages/cpt-ui/__tests__/RBACBanner.test.tsx index a9d0d38c18..e535fe2e93 100644 --- a/packages/cpt-ui/__tests__/RBACBanner.test.tsx +++ b/packages/cpt-ui/__tests__/RBACBanner.test.tsx @@ -37,7 +37,8 @@ describe("RBACBanner", () => { userDetails: { family_name: "Doe", given_name: "John" - } + }, + isSignedIn: true }) render() @@ -57,7 +58,8 @@ describe("RBACBanner", () => { userDetails: { family_name: "Smith", given_name: "Anna" - } + }, + isSignedIn: true }) render() @@ -77,7 +79,8 @@ describe("RBACBanner", () => { }, userDetails: { // missing family_name and given_name - } + }, + isSignedIn: true }) render() @@ -96,7 +99,8 @@ describe("RBACBanner", () => { userDetails: { family_name: "Brown", given_name: "Charlie" - } + }, + isSignedIn: true }) render() @@ -117,7 +121,8 @@ describe("RBACBanner", () => { userDetails: { family_name: "Green", given_name: "Emma" - } + }, + isSignedIn: true }) render() @@ -134,7 +139,26 @@ describe("RBACBanner", () => { userDetails: { family_name: "Green", given_name: "Emma" - } + }, + isSignedIn: true + }) + + const {container} = render() + expect(container.firstChild).toBeNull() + }) + + it("should render null (nothing) when isSignedIn is false", () => { + mockUseAuth.mockReturnValue({ + selectedRole: { + org_code: "X55555", + // missing org_name + role_name: "Technician" + }, + userDetails: { + family_name: "Green", + given_name: "Emma" + }, + isSignedIn: false }) const {container} = render() diff --git a/packages/cpt-ui/__tests__/axios.test.tsx b/packages/cpt-ui/__tests__/axios.test.tsx index fea316e170..f8ec56eba4 100644 --- a/packages/cpt-ui/__tests__/axios.test.tsx +++ b/packages/cpt-ui/__tests__/axios.test.tsx @@ -135,7 +135,18 @@ describe("HTTP Axios Instance", () => { .mockReturnValueOnce("not a token") mock.onGet("/test").reply(200) - await expect(http.get("/test")).rejects.toThrow("canceled") + await expect(http.get("/test")).rejects.toThrow("Could not get a cognito token") + }) + + it("does not abort when request targets CIS2 signout endpoint without a token", async () => { + (fetchAuthSession as jest.Mock) + .mockReturnValueOnce("not a token") + mock.onGet("/api/cis2-signout").reply(200, {success: true}) + + const response = await http.get("/api/cis2-signout") + + expect(response.status).toBe(200) + expect(response.data).toEqual({success: true}) }) it("Does not retry if response says to restart login", async () => { diff --git a/packages/cpt-ui/__tests__/logout.test.tsx b/packages/cpt-ui/__tests__/logout.test.tsx index ab0687ee9f..a10a094bde 100644 --- a/packages/cpt-ui/__tests__/logout.test.tsx +++ b/packages/cpt-ui/__tests__/logout.test.tsx @@ -1,7 +1,18 @@ import "@testing-library/jest-dom" -import {signOut, handleRestartLogin} from "@/helpers/logout" +import { + handleSignoutEvent, + signOut, + clearLogoutMarkerFromStorage, + LogoutMarker +} from "@/helpers/logout" import {logger} from "@/helpers/logger" -import {AUTH_CONFIG} from "@/constants/environment" +import { + AUTH_CONFIG, + FRONTEND_PATHS, + LOGOUT_MARKER_STORAGE_KEY, + LOGOUT_MARKER_STORAGE_GROUP, + OPEN_TABS_STORAGE_KEY +} from "@/constants/environment" import {AuthContextType} from "@/context/AuthProvider" import {mockAuthState} from "./mocks/AuthStateMock" @@ -9,68 +20,51 @@ import {mockAuthState} from "./mocks/AuthStateMock" jest.mock("@/helpers/logger", () => ({ logger: { info: jest.fn(), + debug: jest.fn(), error: jest.fn() } })) -// Mock AUTH_CONFIG -jest.mock("@/constants/environment", () => ({ - AUTH_CONFIG: { - REDIRECT_SIGN_OUT: "/default-signout", - REDIRECT_SESSION_SIGN_OUT: "/session-signout" +// Mock useNavigate and assign to a variable for assertions +const mockNavigate = jest.fn() +jest.mock("react-router-dom", () => { + const actual = jest.requireActual("react-router-dom") + return { + ...actual, + useNavigate: () => mockNavigate } -})) - -const createMockAuth = (overrides = {}): AuthContextType => ({ - ...mockAuthState, - error: null, - user: null, - isSignedIn: false, - isSigningIn: false, - isSigningOut: false, - isConcurrentSession: false, - invalidSessionCause: undefined, - sessionId: undefined, - deviceId: undefined, - rolesWithAccess: [], - rolesWithoutAccess: [], - selectedRole: undefined, - userDetails: undefined, - remainingSessionTime: undefined, - cognitoSignIn: jest.fn(), - cognitoSignOut: jest.fn().mockResolvedValue(true), - clearAuthState: jest.fn(), - hasSingleRoleAccess: jest.fn().mockReturnValue(false), - updateSelectedRole: jest.fn(), - updateTrackerUserInfo: jest.fn(), - updateInvalidSessionCause: jest.fn(), - setIsSigningOut: jest.fn(), - setStateForSignOut: jest.fn().mockResolvedValue(undefined), - ...overrides }) describe("logout helpers", () => { beforeEach(() => { jest.clearAllMocks() + localStorage.clear() + sessionStorage.clear() }) - describe("signOut", () => { + describe("signOut function", () => { it("calls setStateForSignOut and cognitoSignOut", async () => { - const mockAuth = createMockAuth() + const mockAuth: AuthContextType = { + ...mockAuthState, + cognitoSignOut: jest.fn().mockResolvedValue(true) + } await signOut(mockAuth) expect(mockAuth.setStateForSignOut).toHaveBeenCalledTimes(1) expect(logger.info).toHaveBeenCalledWith( - expect.stringContaining("Called signOut helper from") + expect.stringContaining("Called signOut helper from"), mockAuth ) }) it("uses provided redirectUri when specified", async () => { - const mockAuth = createMockAuth() + const mockAuth: AuthContextType = { + ...mockAuthState, + cognitoSignOut: jest.fn().mockResolvedValue(true) + } const customRedirectUri = "/custom-redirect" - await signOut(mockAuth, customRedirectUri) + await signOut(mockAuth, mockNavigate, customRedirectUri) expect(logger.info).toHaveBeenCalledWith( "Signing out with specified redirect path", @@ -79,145 +73,217 @@ describe("logout helpers", () => { expect(mockAuth.cognitoSignOut).toHaveBeenCalledWith(customRedirectUri) }) - it("uses default redirectUri when not specified", async () => { - const mockAuth = createMockAuth() - - await signOut(mockAuth) - - expect(logger.info).toHaveBeenCalledWith( - "Signing out with default redirect path", - AUTH_CONFIG.REDIRECT_SIGN_OUT - ) - expect(mockAuth.cognitoSignOut).toHaveBeenCalledWith(AUTH_CONFIG.REDIRECT_SIGN_OUT) - }) - - it("handles cognitoSignOut errors and sets isSigningOut to false", async () => { + it("handles cognitoSignOut errors and navigates to logout", async () => { const signOutError = new Error("Cognito signout failed") - const mockAuth = createMockAuth({ + const mockAuth: AuthContextType = { + ...mockAuthState, cognitoSignOut: jest.fn().mockRejectedValue(signOutError) - }) + } - await expect(signOut(mockAuth, "/test-redirect")).rejects.toThrow("Cognito signout failed") + await signOut(mockAuth, mockNavigate, "/test-redirect") - expect(logger.error).toHaveBeenCalledWith("Error during logout:", signOutError) - expect(mockAuth.setIsSigningOut).toHaveBeenCalledWith(false) + expect(logger.error).toHaveBeenCalledWith( + "Error during sign out with specified redirect path: /test-redirect", signOutError + ) + expect(mockNavigate).toHaveBeenCalledWith(FRONTEND_PATHS.LOGOUT) }) - it("re-throws the error after handling it", async () => { + it("handles cognitoSignOut errors without redirect uri", async () => { const signOutError = new Error("Network error") - const mockAuth = createMockAuth({ + const mockAuth: AuthContextType = { + ...mockAuthState, cognitoSignOut: jest.fn().mockRejectedValue(signOutError) - }) - - await expect(signOut(mockAuth)).rejects.toThrow("Network error") - }) - - it("handles null cognitoSignOut gracefully", async () => { - const mockAuth = createMockAuth({ - cognitoSignOut: null - }) - - // Should log but not throw error due to optional chaining - await expect(signOut(mockAuth)).rejects.toThrow("is not a function") - }) - - it("handles undefined cognitoSignOut gracefully", async () => { - const mockAuth = createMockAuth({ - cognitoSignOut: undefined - }) - - await expect(signOut(mockAuth)).rejects.toThrow("is not a function") - }) - }) - - describe("handleRestartLogin", () => { - it("logs the restart login initiation and AUTH_CONFIG values", async () => { - const mockAuth = createMockAuth() - const invalidSessionCause = "Timeout" + } - await handleRestartLogin(mockAuth, invalidSessionCause) + await signOut(mockAuth, mockNavigate) - expect(logger.info).toHaveBeenCalledWith( - "Handling restart login instruction from backend", - invalidSessionCause + expect(logger.error).toHaveBeenCalledWith( + "Error during sign out", signOutError ) - expect(logger.info).toHaveBeenCalledWith("AUTH_CONFIG values:", { - REDIRECT_SIGN_OUT: AUTH_CONFIG.REDIRECT_SIGN_OUT, - REDIRECT_SESSION_SIGN_OUT: AUTH_CONFIG.REDIRECT_SESSION_SIGN_OUT - }) + expect(mockNavigate).toHaveBeenCalledWith(FRONTEND_PATHS.LOGOUT) }) + }) + describe("handleSignoutEvent", () => { it("handles invalid session cause by updating cause and using session sign out", async () => { - const mockAuth = createMockAuth() + const mockAuth: AuthContextType = { + ...mockAuthState, + cognitoSignOut: jest.fn().mockResolvedValue(true) + } const invalidSessionCause = "ConcurrentSession" - await handleRestartLogin(mockAuth, invalidSessionCause) + await handleSignoutEvent(mockAuth, mockNavigate, "Reason", invalidSessionCause) expect(logger.info).toHaveBeenCalledWith( `Invalid session cause supplied, ${invalidSessionCause}` ) expect(mockAuth.updateInvalidSessionCause).toHaveBeenCalledWith(invalidSessionCause) expect(logger.info).toHaveBeenCalledWith( - "About to sign out with REDIRECT_SESSION_SIGN_OUT:", - AUTH_CONFIG.REDIRECT_SESSION_SIGN_OUT + "Handling sign out event with caller: Reason and invalid session reason: ConcurrentSession" ) expect(mockAuth.cognitoSignOut).toHaveBeenCalledWith(AUTH_CONFIG.REDIRECT_SESSION_SIGN_OUT) }) it("uses default sign out when no invalid session cause provided", async () => { - const mockAuth = createMockAuth() + const mockAuth: AuthContextType = { + ...mockAuthState, + cognitoSignOut: jest.fn().mockResolvedValue(true) + } - await handleRestartLogin(mockAuth, undefined) + await handleSignoutEvent(mockAuth, mockNavigate, "Reason", undefined) expect(logger.info).toHaveBeenCalledWith( - "No invalid session cause, using REDIRECT_SIGN_OUT:", - AUTH_CONFIG.REDIRECT_SIGN_OUT + "No invalid session cause, using standard logout" ) expect(mockAuth.updateInvalidSessionCause).not.toHaveBeenCalled() expect(mockAuth.cognitoSignOut).toHaveBeenCalledWith(AUTH_CONFIG.REDIRECT_SIGN_OUT) }) it("uses default sign out when empty string invalid session cause provided", async () => { - const mockAuth = createMockAuth() + const mockAuth: AuthContextType = { + ...mockAuthState, + cognitoSignOut: jest.fn().mockResolvedValue(true) + } - await handleRestartLogin(mockAuth, "") + await handleSignoutEvent(mockAuth, mockNavigate, "Reason") expect(logger.info).toHaveBeenCalledWith( - "No invalid session cause, using REDIRECT_SIGN_OUT:", - AUTH_CONFIG.REDIRECT_SIGN_OUT + "No invalid session cause, using standard logout" ) expect(mockAuth.updateInvalidSessionCause).not.toHaveBeenCalled() expect(mockAuth.cognitoSignOut).toHaveBeenCalledWith(AUTH_CONFIG.REDIRECT_SIGN_OUT) }) it("handles various invalid session cause values", async () => { - const mockAuth = createMockAuth() const testCases = ["Timeout", "Expired", "InvalidToken", "ConcurrentSession"] for (const cause of testCases) { jest.clearAllMocks() + localStorage.clear() + const mockAuth: AuthContextType = { + ...mockAuthState, + cognitoSignOut: jest.fn().mockResolvedValue(true) + } - await handleRestartLogin(mockAuth, cause) + await handleSignoutEvent(mockAuth, mockNavigate, "Reason", cause) expect(mockAuth.updateInvalidSessionCause).toHaveBeenCalledWith(cause) expect(mockAuth.cognitoSignOut).toHaveBeenCalledWith(AUTH_CONFIG.REDIRECT_SESSION_SIGN_OUT) } }) - it("propagates errors from signOut function", async () => { - const mockAuth = createMockAuth({ + it("navigates to logout when signOut encounters an error", async () => { + const mockAuth: AuthContextType = { + ...mockAuthState, cognitoSignOut: jest.fn().mockRejectedValue(new Error("Sign out failed")) - }) + } - await expect(handleRestartLogin(mockAuth, "Timeout")) - .rejects.toThrow("Sign out failed") + await handleSignoutEvent(mockAuth, mockNavigate, "Reason", "Timeout") + + expect(mockNavigate).toHaveBeenCalledWith(FRONTEND_PATHS.LOGOUT) }) it("propagates errors from updateInvalidSessionCause", async () => { - const mockAuth = createMockAuth({ + const mockAuth: AuthContextType = { + ...mockAuthState, updateInvalidSessionCause: jest.fn().mockRejectedValue(new Error("Update failed")) - }) - await expect(handleRestartLogin(mockAuth, "Timeout")) + } + + await expect(handleSignoutEvent(mockAuth, mockNavigate, "Reason", "Timeout")) .rejects.toThrow("Update failed") }) }) + + describe("signOut duplicate marker handling", () => { + const writeMarkerToStorage = (marker: LogoutMarker) => { + const markerGroup: Record = {[LOGOUT_MARKER_STORAGE_KEY]: marker} + localStorage.setItem(LOGOUT_MARKER_STORAGE_GROUP, JSON.stringify(markerGroup)) + } + + it("skips signOut when recent marker exists from an active tab", async () => { + const activeTabId = "active-tab-123" + writeMarkerToStorage({ + timestamp: Date.now(), + reason: "signOut", + initiatedByTabId: activeTabId + }) + // Register the tab as active + localStorage.setItem(OPEN_TABS_STORAGE_KEY, JSON.stringify([activeTabId])) + + const mockAuth: AuthContextType = { + ...mockAuthState, + cognitoSignOut: jest.fn().mockResolvedValue(true) + } + + await signOut(mockAuth) + + expect(mockAuth.cognitoSignOut).not.toHaveBeenCalled() + expect(logger.info).toHaveBeenCalledWith( + "Skipping duplicate signOut call due to in-progress marker from active tab" + ) + }) + + it("proceeds with signOut when recent marker exists from an inactive tab", async () => { + const deadTabId = "dead-tab-456" + writeMarkerToStorage({ + timestamp: Date.now(), + reason: "signOut", + initiatedByTabId: deadTabId + }) + // No tabs registered as active (dead tab closed) + localStorage.setItem(OPEN_TABS_STORAGE_KEY, JSON.stringify([])) + + const mockAuth: AuthContextType = { + ...mockAuthState, + cognitoSignOut: jest.fn().mockResolvedValue(true) + } + + await signOut(mockAuth) + + expect(mockAuth.setStateForSignOut).toHaveBeenCalled() + expect(mockAuth.cognitoSignOut).toHaveBeenCalled() + expect(logger.info).toHaveBeenCalledWith( + "Logout marker exists from inactive tab - allowing signOut to proceed" + ) + }) + + it("bypasses duplicate check when duplicateSignout is true", async () => { + const activeTabId = "active-tab-123" + writeMarkerToStorage({ + timestamp: Date.now(), + reason: "signOut", + initiatedByTabId: activeTabId + }) + localStorage.setItem(OPEN_TABS_STORAGE_KEY, JSON.stringify([activeTabId])) + + const mockAuth: AuthContextType = { + ...mockAuthState, + cognitoSignOut: jest.fn().mockResolvedValue(true) + } + + await signOut(mockAuth, mockNavigate, "/redirect", true) + + expect(mockAuth.cognitoSignOut).toHaveBeenCalled() + }) + }) + + describe("clearLogoutMarkerFromStorage", () => { + it("removes the logout marker from localStorage", () => { + const marker: LogoutMarker = { + timestamp: Date.now(), + reason: "signOut", + initiatedByTabId: "tab-123" + } + const markerGroup: Record = {[LOGOUT_MARKER_STORAGE_KEY]: marker} + localStorage.setItem(LOGOUT_MARKER_STORAGE_GROUP, JSON.stringify(markerGroup)) + + clearLogoutMarkerFromStorage() + + const stored = JSON.parse(localStorage.getItem(LOGOUT_MARKER_STORAGE_GROUP)!) + expect(stored[LOGOUT_MARKER_STORAGE_KEY]).toBeUndefined() + }) + + it("does not throw when localStorage is empty", () => { + expect(() => clearLogoutMarkerFromStorage()).not.toThrow() + }) + }) }) diff --git a/packages/cpt-ui/__tests__/mocks/AuthStateMock.tsx b/packages/cpt-ui/__tests__/mocks/AuthStateMock.tsx index fa90659ba5..d027788e38 100644 --- a/packages/cpt-ui/__tests__/mocks/AuthStateMock.tsx +++ b/packages/cpt-ui/__tests__/mocks/AuthStateMock.tsx @@ -42,6 +42,7 @@ export const mockAuthState = { updateInvalidSessionCause: jest.fn(), setIsSigningOut: jest.fn(), setStateForSignOut: jest.fn().mockImplementation(() => Promise.resolve()), + setStateForSignIn: jest.fn().mockImplementation(() => Promise.resolve()), setSessionTimeoutModalInfo: jest.fn(), setLogoutModalType: jest.fn(), remainingSessionTime: undefined diff --git a/packages/cpt-ui/__tests__/tabHelpers.test.ts b/packages/cpt-ui/__tests__/tabHelpers.test.ts new file mode 100644 index 0000000000..1a26b76cad --- /dev/null +++ b/packages/cpt-ui/__tests__/tabHelpers.test.ts @@ -0,0 +1,308 @@ +import type * as TabHelpersModule from "@/helpers/tabHelpers" + +jest.mock("@/constants/environment", () => ({ + TAB_ID_SESSION_KEY: "tabId", + OPEN_TABS_STORAGE_KEY: "openTabIds", + TAB_HEARTBEATS_STORAGE_KEY: "tabHeartbeats", + TAB_STALE_THRESHOLD_MS: 5 * 60 * 1000 +})) + +const mockUUID = "mock-uuid-1234" +const mockUUID2 = "mock-uuid-5678" + +let uuidCallCount = 0 + +/** + * Import tabHelpers inside jest.isolateModules so each call gets a fresh module + * with its own `resolvedTabId` cache, avoiding test cross-contamination. + */ +const importFreshTabHelpers = (): typeof TabHelpersModule => { + let mod!: typeof TabHelpersModule + jest.isolateModules(() => { + // Use eval to allow dynamic import in CommonJS context for test isolation + mod = eval('require("@/helpers/tabHelpers")') as typeof TabHelpersModule + }) + return mod +} + +beforeEach(() => { + jest.resetModules() + + uuidCallCount = 0 + Object.defineProperty(global, "crypto", { + value: { + randomUUID: jest.fn(() => { + uuidCallCount++ + return uuidCallCount === 1 ? mockUUID : mockUUID2 + }) + }, + writable: true, + configurable: true + }) + + window.sessionStorage.clear() + window.localStorage.clear() +}) + +describe("getOrCreateTabId", () => { + it("creates a new tab ID when sessionStorage is empty", () => { + const {getOrCreateTabId} = importFreshTabHelpers() + + const tabId = getOrCreateTabId() + + expect(tabId).toBe(mockUUID) + expect(window.sessionStorage.getItem("tabId")).toBe(mockUUID) + }) + + it("reuses existing tab ID from sessionStorage when not in open tabs list (reload scenario)", () => { + // Simulate: tab reloaded — beforeunload already removed it from open tabs + window.sessionStorage.setItem("tabId", "existing-tab-id") + // open tabs list does NOT contain "existing-tab-id" + const {getOrCreateTabId} = importFreshTabHelpers() + + const tabId = getOrCreateTabId() + + expect(tabId).toBe("existing-tab-id") + expect(crypto.randomUUID).not.toHaveBeenCalled() + }) + + it("generates a new tab ID when sessionStorage ID is already in open tabs list (duplicate tab scenario)", () => { + // Simulate: tab duplicated — original tab still has its ID in open tabs + window.sessionStorage.setItem("tabId", "original-tab-id") + window.localStorage.setItem("openTabIds", JSON.stringify(["original-tab-id"])) + const {getOrCreateTabId} = importFreshTabHelpers() + + const tabId = getOrCreateTabId() + + expect(tabId).toBe(mockUUID) + expect(tabId).not.toBe("original-tab-id") + expect(window.sessionStorage.getItem("tabId")).toBe(mockUUID) + }) + + it("caches the resolved tab ID for subsequent calls", () => { + const {getOrCreateTabId} = importFreshTabHelpers() + + const firstCall = getOrCreateTabId() + const secondCall = getOrCreateTabId() + + expect(firstCall).toBe(secondCall) + expect(crypto.randomUUID).toHaveBeenCalledTimes(1) + }) + + it("uses module cache even if sessionStorage changes between calls", () => { + const {getOrCreateTabId} = importFreshTabHelpers() + + const firstCall = getOrCreateTabId() + window.sessionStorage.setItem("tabId", "something-else") + + const secondCall = getOrCreateTabId() + + expect(secondCall).toBe(firstCall) + }) +}) + +describe("getOpenTabIds", () => { + it("returns empty array when localStorage is empty", () => { + const {getOpenTabIds} = importFreshTabHelpers() + expect(getOpenTabIds()).toEqual([]) + }) + + it("returns empty array when localStorage value is not valid JSON", () => { + window.localStorage.setItem("openTabIds", "not-json") + const {getOpenTabIds} = importFreshTabHelpers() + + expect(getOpenTabIds()).toEqual([]) + }) + + it("returns empty array when localStorage value is not an array", () => { + window.localStorage.setItem("openTabIds", JSON.stringify({foo: "bar"})) + const {getOpenTabIds} = importFreshTabHelpers() + + expect(getOpenTabIds()).toEqual([]) + }) + + it("filters out non-string values", () => { + window.localStorage.setItem("openTabIds", JSON.stringify(["valid", 123, null, "also-valid"])) + const {getOpenTabIds} = importFreshTabHelpers() + + expect(getOpenTabIds()).toEqual(["valid", "also-valid"]) + }) + + it("returns all string tab IDs", () => { + window.localStorage.setItem("openTabIds", JSON.stringify(["tab-1", "tab-2"])) + const {getOpenTabIds} = importFreshTabHelpers() + + expect(getOpenTabIds()).toEqual(["tab-1", "tab-2"]) + }) +}) + +describe("setOpenTabIds", () => { + it("stores tab IDs as JSON in localStorage", () => { + const {setOpenTabIds} = importFreshTabHelpers() + + setOpenTabIds(["tab-1", "tab-2"]) + + expect(window.localStorage.getItem("openTabIds")).toBe(JSON.stringify(["tab-1", "tab-2"])) + }) +}) + +describe("updateOpenTabs", () => { + it("adds a tab ID that is not already present", () => { + const {setOpenTabIds, updateOpenTabs, getOpenTabIds} = importFreshTabHelpers() + setOpenTabIds(["tab-1"]) + + updateOpenTabs("tab-2", "add") + + expect(getOpenTabIds()).toEqual(["tab-1", "tab-2"]) + }) + + it("does not duplicate a tab ID that is already present", () => { + const {setOpenTabIds, updateOpenTabs, getOpenTabIds} = importFreshTabHelpers() + setOpenTabIds(["tab-1"]) + + updateOpenTabs("tab-1", "add") + + expect(getOpenTabIds()).toEqual(["tab-1"]) + }) + + it("removes a tab ID", () => { + const {setOpenTabIds, updateOpenTabs, getOpenTabIds} = importFreshTabHelpers() + setOpenTabIds(["tab-1", "tab-2"]) + + updateOpenTabs("tab-1", "remove") + + expect(getOpenTabIds()).toEqual(["tab-2"]) + }) + + it("does nothing when removing a tab ID that is not present", () => { + const {setOpenTabIds, updateOpenTabs, getOpenTabIds} = importFreshTabHelpers() + setOpenTabIds(["tab-1"]) + + updateOpenTabs("tab-2", "remove") + + expect(getOpenTabIds()).toEqual(["tab-1"]) + }) +}) + +describe("getOpenTabCount", () => { + it("returns 1 when no tabs are open", () => { + const {getOpenTabCount} = importFreshTabHelpers() + expect(getOpenTabCount()).toBe(1) + }) + + it("returns the actual count when tabs are open", () => { + const {setOpenTabIds, getOpenTabCount} = importFreshTabHelpers() + setOpenTabIds(["tab-1", "tab-2", "tab-3"]) + + expect(getOpenTabCount()).toBe(3) + }) +}) + +describe("heartbeatTab", () => { + it("records a timestamp for the given tab ID", () => { + const {heartbeatTab} = importFreshTabHelpers() + + heartbeatTab("tab-1") + + const heartbeats = JSON.parse(window.localStorage.getItem("tabHeartbeats")!) + expect(heartbeats["tab-1"]).toBeDefined() + expect(typeof heartbeats["tab-1"]).toBe("number") + }) + + it("updates the timestamp on subsequent calls", () => { + const {heartbeatTab} = importFreshTabHelpers() + + const before = Date.now() + heartbeatTab("tab-1") + const heartbeats1 = JSON.parse(window.localStorage.getItem("tabHeartbeats")!) + + expect(heartbeats1["tab-1"]).toBeGreaterThanOrEqual(before) + }) + + it("tracks multiple tabs independently", () => { + const {heartbeatTab} = importFreshTabHelpers() + + heartbeatTab("tab-1") + heartbeatTab("tab-2") + + const heartbeats = JSON.parse(window.localStorage.getItem("tabHeartbeats")!) + expect(heartbeats["tab-1"]).toBeDefined() + expect(heartbeats["tab-2"]).toBeDefined() + }) +}) + +describe("pruneStaleTabIds", () => { + it("removes tabs with no heartbeat from the open list", () => { + const {setOpenTabIds, pruneStaleTabIds, getOpenTabIds} = importFreshTabHelpers() + setOpenTabIds(["tab-1", "tab-2"]) + // No heartbeats recorded — both should be pruned + + pruneStaleTabIds() + + expect(getOpenTabIds()).toEqual([]) + }) + + it("removes tabs whose heartbeat is older than the threshold", () => { + const {setOpenTabIds, heartbeatTab, pruneStaleTabIds, getOpenTabIds} = importFreshTabHelpers() + setOpenTabIds(["fresh-tab", "stale-tab"]) + + heartbeatTab("fresh-tab") + // Manually write a stale heartbeat + const heartbeats = JSON.parse(window.localStorage.getItem("tabHeartbeats")!) + heartbeats["stale-tab"] = Date.now() - (6 * 60 * 1000) // 6 minutes ago + window.localStorage.setItem("tabHeartbeats", JSON.stringify(heartbeats)) + + pruneStaleTabIds() + + expect(getOpenTabIds()).toEqual(["fresh-tab"]) + const updatedHeartbeats = JSON.parse(window.localStorage.getItem("tabHeartbeats")!) + expect(updatedHeartbeats["stale-tab"]).toBeUndefined() + expect(updatedHeartbeats["fresh-tab"]).toBeDefined() + }) + + it("does nothing when all tabs are fresh", () => { + const {setOpenTabIds, heartbeatTab, pruneStaleTabIds, getOpenTabIds} = importFreshTabHelpers() + setOpenTabIds(["tab-1", "tab-2"]) + heartbeatTab("tab-1") + heartbeatTab("tab-2") + + pruneStaleTabIds() + + expect(getOpenTabIds()).toEqual(["tab-1", "tab-2"]) + }) + + it("accepts a custom threshold", () => { + const {setOpenTabIds, pruneStaleTabIds, getOpenTabIds} = importFreshTabHelpers() + setOpenTabIds(["tab-1"]) + + // Write heartbeat 2 seconds ago + const heartbeats = {"tab-1": Date.now() - 2000} + window.localStorage.setItem("tabHeartbeats", JSON.stringify(heartbeats)) + + // Prune with 1-second threshold + pruneStaleTabIds(1000) + + expect(getOpenTabIds()).toEqual([]) + }) +}) + +describe("updateOpenTabs heartbeat integration", () => { + it("records a heartbeat when adding a tab", () => { + const {updateOpenTabs} = importFreshTabHelpers() + + updateOpenTabs("tab-1", "add") + + const heartbeats = JSON.parse(window.localStorage.getItem("tabHeartbeats")!) + expect(heartbeats["tab-1"]).toBeDefined() + }) + + it("removes the heartbeat when removing a tab", () => { + const {updateOpenTabs} = importFreshTabHelpers() + + updateOpenTabs("tab-1", "add") + updateOpenTabs("tab-1", "remove") + + const heartbeats = JSON.parse(window.localStorage.getItem("tabHeartbeats")!) + expect(heartbeats["tab-1"]).toBeUndefined() + }) +}) diff --git a/packages/cpt-ui/__tests__/useSessionTimeout.test.tsx b/packages/cpt-ui/__tests__/useSessionTimeout.test.tsx index a4cfb03693..51a51f9a22 100644 --- a/packages/cpt-ui/__tests__/useSessionTimeout.test.tsx +++ b/packages/cpt-ui/__tests__/useSessionTimeout.test.tsx @@ -1,9 +1,10 @@ import {renderHook, act} from "@testing-library/react" import {useSessionTimeout} from "@/hooks/useSessionTimeout" import {updateRemoteSelectedRole} from "@/helpers/userInfo" -import {handleRestartLogin, signOut} from "@/helpers/logout" +import {handleSignoutEvent} from "@/helpers/logout" import {logger} from "@/helpers/logger" import {useAuth, AuthContextType} from "@/context/AuthProvider" +import {mockAuthState} from "./mocks/AuthStateMock" // Create mock implementations const mockSetSessionTimeoutModalInfo = jest.fn() @@ -11,6 +12,12 @@ const mockSetLogoutModalType = jest.fn() const mockUpdateTrackerUserInfo = jest.fn() const mockUpdateInvalidSessionCause = jest.fn() +const mockNavigate = jest.fn() +jest.mock("react-router-dom", () => ({ + ...jest.requireActual("react-router-dom"), + useNavigate: () => mockNavigate +})) + // Mock modules using factory functions to avoid hoisting issues jest.mock("@/helpers/userInfo") jest.mock("@/helpers/logout") @@ -54,6 +61,7 @@ jest.mock("@/constants/environment", () => ({ })) const createAuthMock = (overrides: Partial = {}): AuthContextType => ({ + ...mockAuthState, error: null, user: null, isSignedIn: true, @@ -94,8 +102,7 @@ describe("useSessionTimeout", () => { jest.clearAllMocks() jest.mocked(updateRemoteSelectedRole).mockResolvedValue({currentlySelectedRole: {}}) - jest.mocked(signOut).mockResolvedValue(undefined) - jest.mocked(handleRestartLogin).mockResolvedValue(undefined) + jest.mocked(handleSignoutEvent).mockResolvedValue(undefined) mockUpdateTrackerUserInfo.mockResolvedValue(undefined) jest.mocked(useAuth).mockReturnValue(createAuthMock()) @@ -162,8 +169,9 @@ describe("useSessionTimeout", () => { expect(updatedState.action).toBeUndefined() }) - it("should log error when selectedRole is missing", async () => { - jest.mocked(useAuth).mockReturnValue(createAuthMock({selectedRole: undefined})) + it("should log error when selectedRole is missing and trigger logout", async () => { + const authMock = createAuthMock({selectedRole: undefined}) + jest.mocked(useAuth).mockReturnValue(authMock) const {result} = renderHook(() => useSessionTimeout()) @@ -173,9 +181,12 @@ describe("useSessionTimeout", () => { expect(logger.error).toHaveBeenCalledWith("No selected role available to extend session") expect(updateRemoteSelectedRole).not.toHaveBeenCalled() + expect(handleSignoutEvent).toHaveBeenCalledWith(authMock, mockNavigate, "Timeout", "Timeout") }) it("should handle error from updateRemoteSelectedRole", async () => { + const authMock = createAuthMock() + jest.mocked(useAuth).mockReturnValue(authMock) jest.mocked(updateRemoteSelectedRole).mockRejectedValue(new Error("API Error")) const {result} = renderHook(() => useSessionTimeout()) @@ -192,6 +203,8 @@ describe("useSessionTimeout", () => { }) expect(updatedState.action).toBe("loggingOut") expect(updatedState.buttonDisabled).toBe(true) + // Should call handleSignoutEvent after error + expect(handleSignoutEvent).toHaveBeenCalledWith(authMock, mockNavigate, "Timeout", "Timeout") }) it("should prevent duplicate calls via actionLockRef", async () => { @@ -221,7 +234,7 @@ describe("useSessionTimeout", () => { }) describe("onLogOut", () => { - it("should call signOut with redirect URL", async () => { + it("should call handleSignoutEvent and clear modal type", async () => { const authMock = createAuthMock() jest.mocked(useAuth).mockReturnValue(authMock) @@ -231,7 +244,7 @@ describe("useSessionTimeout", () => { await result.current.onLogOut() }) - expect(signOut).toHaveBeenCalledWith(authMock, "mock-redirect-url") + expect(handleSignoutEvent).toHaveBeenCalledWith(authMock, mockNavigate, "Timeout", "Timeout") expect(mockSetLogoutModalType).toHaveBeenCalledWith(undefined) expect(logger.info).toHaveBeenCalledWith( "User chose to log out from session timeout modal" @@ -254,7 +267,7 @@ describe("useSessionTimeout", () => { }) it("should prevent duplicate logout calls", async () => { - jest.mocked(signOut).mockImplementation(() => new Promise(() => {})) + jest.mocked(handleSignoutEvent).mockImplementation(() => new Promise(() => {})) const {result} = renderHook(() => useSessionTimeout()) @@ -269,7 +282,7 @@ describe("useSessionTimeout", () => { expect(logger.info).toHaveBeenCalledWith( "Session action already in progress, ignoring duplicate request" ) - expect(signOut).toHaveBeenCalledTimes(1) + expect(handleSignoutEvent).toHaveBeenCalledTimes(1) }) it("should prevent cross-calls (stay logged in then log out)", async () => { @@ -289,7 +302,7 @@ describe("useSessionTimeout", () => { await result.current.onLogOut() }) - expect(signOut).not.toHaveBeenCalled() + expect(handleSignoutEvent).not.toHaveBeenCalled() }) }) @@ -305,8 +318,7 @@ describe("useSessionTimeout", () => { }) expect(logger.warn).toHaveBeenCalledWith("Session automatically timed out") - expect(mockUpdateInvalidSessionCause).toHaveBeenCalledWith("Timeout") - expect(handleRestartLogin).toHaveBeenCalledWith(authMock, "Timeout") + expect(handleSignoutEvent).toHaveBeenCalledWith(authMock, mockNavigate, "Timeout", "Timeout") // clearCountdownTimer should have reset timeLeft to 0 const updaterFn = mockSetSessionTimeoutModalInfo.mock.calls[0][0] const updatedState = updaterFn({ diff --git a/packages/cpt-ui/jest.setup.ts b/packages/cpt-ui/jest.setup.ts index 99f51e834e..aea9323c61 100644 --- a/packages/cpt-ui/jest.setup.ts +++ b/packages/cpt-ui/jest.setup.ts @@ -92,6 +92,7 @@ jest.mock("@/constants/environment", () => ({ SELECT_YOUR_ROLE: "/select-your-role", YOUR_SELECTED_ROLE: "/your-selected-role", CHANGE_YOUR_ROLE: "/change-your-role", + SESSION_SELECTION: "/select-active-session", SEARCH_BY_PRESCRIPTION_ID: "/search-by-prescription-id", SEARCH_BY_NHS_NUMBER: "/search-by-nhs-number", SEARCH_BY_BASIC_DETAILS: "/search-by-basic-details", @@ -103,7 +104,6 @@ jest.mock("@/constants/environment", () => ({ NO_PRESCRIPTIONS_FOUND: "/no-prescriptions-found", PRIVACY_NOTICE: "/privacy-notice", COOKIES_SELECTED: "/cookies-selected", - SESSION_SELECTION: "/select-active-session", NOT_FOUND: "/notfound" }, ALLOWED_NO_ROLE_PATHS: [ @@ -148,7 +148,14 @@ jest.mock("@/constants/environment", () => ({ "/cookies-selected", "/" ], - MOCK_AUTH_ALLOWED_ENVIRONMENTS: ["dev", "dev-pr", "int", "qa"] + MOCK_AUTH_ALLOWED_ENVIRONMENTS: ["dev", "dev-pr", "int", "qa"], + LOGOUT_MARKER_STORAGE_KEY: "logoutMarker", + LOGOUT_MARKER_STORAGE_GROUP: "logoutMarker", + LOGOUT_MARKER_MAX_AGE_MS: 10000, + TAB_ID_SESSION_KEY: "tabId", + OPEN_TABS_STORAGE_KEY: "openTabIds", + TAB_HEARTBEATS_STORAGE_KEY: "tabHeartbeats", + TAB_STALE_THRESHOLD_MS: 5 * 60 * 1000 })) // Allows for tab selection diff --git a/packages/cpt-ui/src/components/EpsHeader.tsx b/packages/cpt-ui/src/components/EpsHeader.tsx index e0ed81f242..9ceb2a6ece 100644 --- a/packages/cpt-ui/src/components/EpsHeader.tsx +++ b/packages/cpt-ui/src/components/EpsHeader.tsx @@ -11,8 +11,8 @@ import {getHomeLink} from "@/helpers/loginFunctions" import {HEADER_STRINGS} from "@/constants/ui-strings/HeaderStrings" import {EpsLogoutModal} from "@/components/EpsLogoutModal" import {normalizePath} from "@/helpers/utils" -import {FRONTEND_PATHS, AUTH_CONFIG} from "@/constants/environment" -import {signOut} from "@/helpers/logout" +import {FRONTEND_PATHS} from "@/constants/environment" +import {handleSignoutEvent} from "@/helpers/logout" import NhsLogo from "@/components/icons/NhsLogo" export default function EpsHeader() { @@ -24,6 +24,7 @@ export default function EpsHeader() { const [shouldShowSelectRole, setShouldShowSelectRole] = useState(false) const [shouldShowChangeRole, setShouldShowChangeRole] = useState(false) const [shouldShowLogoutLink, setShouldShowLogoutLink] = useState(false) + const [showLogoutModal, setShowLogoutModal] = useState(false) const [actionButtonsDisabled, setActionButtonsDisabled] = useState(false) const [isDropdownOpen, setIsDropdownOpen] = useState(false) const [isMobileView, setIsMobileView] = useState(false) @@ -75,6 +76,7 @@ export default function EpsHeader() { const handleLogoutClick = (e: React.MouseEvent) => { e.preventDefault() + setShowLogoutModal(true) authContext.setLogoutModalType("userInitiated") } @@ -87,9 +89,10 @@ export default function EpsHeader() { setActionButtonsDisabled(true) try { - await signOut(authContext, AUTH_CONFIG.REDIRECT_SIGN_OUT) + await handleSignoutEvent(authContext, navigate, "UserInitiatedHeaderLogout") } finally { buttonDisabledRef.current = false + setShowLogoutModal(false) } } @@ -273,7 +276,7 @@ export default function EpsHeader() { { authContext?.setLogoutModalType(undefined) buttonDisabledRef.current = false diff --git a/packages/cpt-ui/src/components/EpsRoleSelectionPage.tsx b/packages/cpt-ui/src/components/EpsRoleSelectionPage.tsx index 11a41f974a..c7338c99c5 100644 --- a/packages/cpt-ui/src/components/EpsRoleSelectionPage.tsx +++ b/packages/cpt-ui/src/components/EpsRoleSelectionPage.tsx @@ -1,4 +1,4 @@ -import React, {useState, useEffect, useRef} from "react" +import React, {useState, useEffect} from "react" import {useNavigate} from "react-router-dom" import { Container, @@ -15,13 +15,11 @@ import "../styles/roleselectionpage.scss" import {useAuth} from "@/context/AuthProvider" import {Button} from "./ReactRouterButton" import {ENV_CONFIG, FRONTEND_PATHS} from "@/constants/environment" -import {getSearchParams} from "@/helpers/getSearchParams" import {logger} from "@/helpers/logger" import {usePageTitle} from "@/hooks/usePageTitle" import axios from "axios" -import {handleRestartLogin} from "@/helpers/logout" +import {handleSignoutEvent} from "@/helpers/logout" import {CHANGE_YOUR_ROLE_PAGE_TEXT} from "@/constants/ui-strings/ChangeRolePageStrings" -import LoadingPage from "@/pages/LoadingPage" import { RolesWithAccessProps, RolesWithoutAccessProps, @@ -137,13 +135,13 @@ export default function RoleSelectionPage({ const auth = useAuth() const navigate = useNavigate() const location = {pathname: globalThis.location.pathname} - const redirecting = useRef(false) const [isSelectingRole, setIsSelectingRole] = useState(false) const [selectedCardId, setSelectedCardId] = useState(null) const [roleComponentProps, setRoleComponentProps] = useState({ rolesWithAccess: [], rolesWithoutAccess: [] }) + const [sentRumRoleLogs, setSentRumRoleLogs] = useState(false) usePageTitle(auth.rolesWithAccess.length === 0 ? CHANGE_YOUR_ROLE_PAGE_TEXT.NO_ACCESS_pageTitle @@ -166,7 +164,7 @@ export default function RoleSelectionPage({ } catch (err) { if (axios.isAxiosError(err) && (err.response?.status === 401)) { const invalidSessionCause = err.response?.data?.invalidSessionCause - handleRestartLogin(auth, invalidSessionCause) + handleSignoutEvent(auth, navigate, "RoleSelection Call Failure", invalidSessionCause) return } logger.error("Error selecting role:", err) @@ -213,25 +211,12 @@ export default function RoleSelectionPage({ noODSCode ) - logRoleChunks(auth, transformedData.rolesWithAccess, transformedData.rolesWithoutAccess, location) + logRoleChunks(auth, transformedData.rolesWithAccess, + transformedData.rolesWithoutAccess, location, sentRumRoleLogs) + setSentRumRoleLogs(true) setRoleComponentProps(transformedData) }, [auth.rolesWithAccess, auth.rolesWithoutAccess]) - // Handle auto-redirect for single role - useEffect(() => { - if (auth.isSigningIn) { - const {codeParams, stateParams} = getSearchParams(window) - if (codeParams && stateParams) { - redirecting.current = true - return - } else { - handleRestartLogin(auth, "NoSearchParams") - } - } else { - redirecting.current = false - } - }, [auth.isSigningIn]) - useEffect(() => { if (auth.hasSingleRoleAccess() && auth.isSignedIn) { logger.debug("Role confirmed", { @@ -252,13 +237,6 @@ export default function RoleSelectionPage({ return } - // TODO: Remove this loading logic in favor of AccessProvider.shouldBlockChildren() - // when auth changes are complete - avoid duplicate auth state handling - const shouldShowLoading = redirecting.current || auth.isSigningOut - if (shouldShowLoading) { - return - } - return ( diff --git a/packages/cpt-ui/src/components/EpsRoleSelectionPage.utils.ts b/packages/cpt-ui/src/components/EpsRoleSelectionPage.utils.ts index ba2291db24..1c4f21febd 100644 --- a/packages/cpt-ui/src/components/EpsRoleSelectionPage.utils.ts +++ b/packages/cpt-ui/src/components/EpsRoleSelectionPage.utils.ts @@ -89,16 +89,17 @@ export function logRoleChunks( auth: AuthContextType, rolesWithAccessComponentProps: Array, rolesWithoutAccessComponentProps: Array, - location: LocationType + location: LocationType, + sentRumRoleLogs: boolean ) { - if (!auth.userDetails?.sub) return + if (!auth.userDetails?.sub || sentRumRoleLogs) return const logId = crypto.randomUUID() logger.debug("Counts of roles returned vs rendered", { logId, sessionId: auth.sessionId, - userId: auth.userDetails.sub, + userId: auth.userDetails?.sub, pageName: location.pathname, currentlySelectedRole: !!auth.selectedRole, returnedRolesWithAccessCount: auth.rolesWithAccess.length, @@ -110,11 +111,11 @@ export function logRoleChunks( logger.debug("Auth context for rendered roles", { logId, sessionId: auth.sessionId, - userId: auth.userDetails.sub, + userId: auth.userDetails?.sub, pageName: location.pathname, authContext: { cognitoUsername: auth.user, - name: auth.userDetails.name, + name: auth.userDetails?.name, currentlySelectedRole: auth.selectedRole, isSignedIn: auth.isSignedIn, isSigningIn: auth.isSigningIn, diff --git a/packages/cpt-ui/src/components/RBACBanner.tsx b/packages/cpt-ui/src/components/RBACBanner.tsx index 938bede7e8..fd3226af3f 100644 --- a/packages/cpt-ui/src/components/RBACBanner.tsx +++ b/packages/cpt-ui/src/components/RBACBanner.tsx @@ -8,10 +8,10 @@ import {logger} from "@/helpers/logger" export default function RBACBanner() { const [bannerText, setBannerText] = useState("") - const {selectedRole, userDetails} = useAuth() + const {isSignedIn, selectedRole, userDetails} = useAuth() useEffect(() => { - if (!selectedRole || !userDetails) { + if (!isSignedIn || !selectedRole || !userDetails) { logger.info("No selected role or user details - hiding RBAC banner.") setBannerText("") return @@ -46,14 +46,14 @@ export default function RBACBanner() { .replace("{orgName}", orgName) .replace("{odsCode}", selectedRole.org_code || RBAC_BANNER_STRINGS.NO_ODS_CODE) ) - }, [selectedRole, userDetails]) + }, [isSignedIn, selectedRole, userDetails]) /** * Hide the banner if the user session is missing or incomplete. * The component should render only after a role is selected. * This check must come after the effect logic to prevent hydration errors in SSR. */ - if (!selectedRole) { + if (!isSignedIn || !selectedRole || !userDetails) { return null } diff --git a/packages/cpt-ui/src/constants/environment.ts b/packages/cpt-ui/src/constants/environment.ts index 81109af8cd..d0771afab0 100644 --- a/packages/cpt-ui/src/constants/environment.ts +++ b/packages/cpt-ui/src/constants/environment.ts @@ -101,11 +101,6 @@ export const ALLOWED_NO_ROLE_PATHS = [ FRONTEND_PATHS.CHANGE_YOUR_ROLE ] as const -export const ALLOWED_NO_REDIRECT_PATHS = [ - ...PUBLIC_PATHS, - FRONTEND_PATHS.SESSION_LOGGED_OUT -] as const - // pages where patient and prescription banners should be shown export const BANNER_ALLOWED_PATHS = [ FRONTEND_PATHS.PRESCRIPTION_LIST_CURRENT, @@ -135,3 +130,12 @@ const validateEnvironment = (env: string): env is Environment => { if (!validateEnvironment(ENV_CONFIG.TARGET_ENVIRONMENT)) { throw new Error(`Invalid environment: ${ENV_CONFIG.TARGET_ENVIRONMENT}`) } + +export const LOGOUT_MARKER_STORAGE_KEY = "logoutMarker" +export const LOGOUT_MARKER_STORAGE_GROUP = "logoutMarker" +export const LOGOUT_MARKER_MAX_AGE_MS = 2000 + +export const TAB_ID_SESSION_KEY = "tabId" +export const OPEN_TABS_STORAGE_KEY = "openTabIds" +export const TAB_HEARTBEATS_STORAGE_KEY = "tabHeartbeats" +export const TAB_STALE_THRESHOLD_MS = 5 * 60 * 1000 // 5 minutes diff --git a/packages/cpt-ui/src/context/AccessProvider.tsx b/packages/cpt-ui/src/context/AccessProvider.tsx index b7612630e4..c4c90af803 100644 --- a/packages/cpt-ui/src/context/AccessProvider.tsx +++ b/packages/cpt-ui/src/context/AccessProvider.tsx @@ -8,9 +8,19 @@ import {useLocation, useNavigate} from "react-router-dom" import {normalizePath} from "@/helpers/utils" import {useAuth} from "./AuthProvider" +import { + getOpenTabCount, + getOrCreateTabId, + updateOpenTabs, + heartbeatTab, + pruneStaleTabIds +} from "@/helpers/tabHelpers" +import {checkForRecentLogoutMarker} from "@/helpers/logout" + import {ALLOWED_NO_ROLE_PATHS, FRONTEND_PATHS, PUBLIC_PATHS} from "@/constants/environment" import {logger} from "@/helpers/logger" -import {handleRestartLogin} from "@/helpers/logout" +import {handleSignoutEvent} from "@/helpers/logout" +import {getSearchParams} from "@/helpers/getSearchParams" import LoadingPage from "@/pages/LoadingPage" import Layout from "@/Layout" @@ -21,31 +31,71 @@ export const AccessProvider = ({children}: {children: ReactNode}) => { const navigate = useNavigate() const location = useLocation() - const shouldBlockChildren = () => { - // TODO: Investigate moving 'ensureRoleSelected' functionality into this blockChildren - // This could potentially stop amplify re-trying login on a pre-existing session + useEffect(() => { + const tabId = getOrCreateTabId() - const path = normalizePath(location.pathname) + updateOpenTabs(tabId, "add") - // not signed in → block on protected paths, and also block root path to prevent flash - if (!auth.isSignedIn && !auth.isSigningIn) { - if (path === "/") { - logger.info("At root path and not signed in - blocking render") - return true - } - return !ALLOWED_NO_ROLE_PATHS.includes(path) + const onBeforeUnload = () => { + updateOpenTabs(tabId, "remove") + } + + window.addEventListener("beforeunload", onBeforeUnload) + return () => { + window.removeEventListener("beforeunload", onBeforeUnload) + updateOpenTabs(tabId, "remove") } + }, []) - // block if concurrent session needs resolution - if (auth.isConcurrentSession && auth.isSignedIn) { - return !ALLOWED_NO_ROLE_PATHS.includes(normalizePath(path)) + const shouldRedirectDueToCrossTabLogout = () => { + const marker = checkForRecentLogoutMarker("CrossTabCheck") + if (!marker) { + return false } - // block if user needs to select a role (but allow specific paths) - if (!auth.selectedRole && !auth.isSigningIn && auth.isSignedIn) { - return ( - ![...ALLOWED_NO_ROLE_PATHS, FRONTEND_PATHS.SELECT_YOUR_ROLE].includes(normalizePath(path)) - ) + const currentTabId = getOrCreateTabId() + if (marker.initiatedByTabId === currentTabId) { + return false + } + + return getOpenTabCount() > 1 + } + + const shouldBlockChildren = () => { + const path = normalizePath(location.pathname) + + if ((!auth.isSignedIn || auth.isSigningIn) && !PUBLIC_PATHS.includes(path)) { + logger.info(`Not signed in fully and trying to access ${path} - blocking render`, auth) + return true + } + + if (auth.isSignedIn) { + if ((path === "/" || path === FRONTEND_PATHS.LOGIN) || !PUBLIC_PATHS.includes(path)) { + if (auth.isConcurrentSession && (path !== FRONTEND_PATHS.SESSION_SELECTION)) { + logger.info(`Concurrent session detected on ${path} - blocking render until redirect to session selection`) + return true + } + + if (!auth.selectedRole && !ALLOWED_NO_ROLE_PATHS.includes(path)) { + logger.info(`No role selected on ${path} - blocking render until redirect to select your role`) + return true + } + + if (auth.selectedRole && (path === "/" || path === FRONTEND_PATHS.LOGIN)) { + logger.info(`Signed-in user on ${path} - blocking render until redirect`) + return true + } + + if (auth.isSigningOut || auth.isSigningIn) { + // Block render if a user is temporarily in a transition state + return !ALLOWED_NO_ROLE_PATHS.includes(normalizePath(path)) + } + + if (checkForRecentLogoutMarker("shouldBlockChildren")) { + logger.info(`Recent logout marker found while accessing ${path} - blocking render until resolved`) + return true + } + } } return false @@ -53,61 +103,109 @@ export const AccessProvider = ({children}: {children: ReactNode}) => { const ensureRoleSelected = () => { const path = normalizePath(location.pathname) - const inNoRoleAllowed = ALLOWED_NO_ROLE_PATHS.includes(path) - const atRoot = path === "/" const redirect = (to: string, msg: string) => { logger.info(msg) navigate(to) } - const loggedOut = !auth.isSignedIn && !auth.isSigningOut - const loggingOut = auth.isSignedIn && auth.isSigningOut - const concurrent = auth.isSignedIn && auth.isConcurrentSession - const noRole = auth.isSignedIn && !auth.isSigningIn && !auth.selectedRole - const authedAtRoot = auth.isSignedIn && !!auth.selectedRole && atRoot - - logger.info(`Requested path: ${path}`) - if (loggedOut && (!inNoRoleAllowed || atRoot)) { - return redirect(FRONTEND_PATHS.LOGIN, "Not signed in - redirecting to login page") + // Public paths (except root) don't need protection + if (PUBLIC_PATHS.includes(path) && path !== "/" && + (path !== FRONTEND_PATHS.LOGIN && path !== FRONTEND_PATHS.LOGOUT)) { + return } - if (auth.isSignedIn && path === FRONTEND_PATHS.LOGIN) { - if (!auth.selectedRole) { - return redirect(FRONTEND_PATHS.SELECT_YOUR_ROLE, "User already logged in. No role selected.") - } else { - return redirect(FRONTEND_PATHS.SEARCH_BY_PRESCRIPTION_ID, "User already logged in. Role already selected.") + // Not signed in + if (!auth.isSignedIn) { + if (path === "/") { + return redirect(FRONTEND_PATHS.LOGIN, "User at root path - redirecting to login page") } - } - if (concurrent && !(PUBLIC_PATHS.includes(path) || path === FRONTEND_PATHS.SESSION_SELECTION)) { - return redirect(FRONTEND_PATHS.SESSION_SELECTION, "Concurrent session found - redirecting to session selection") - } + // If another tab has signed out, other tabs will run the redirection logic independently + // Check if a logout has occurred elsewhere and forward all tabs to logout equally + // Else subsequent tabs with manipulated state will attempt to logout again or redirect to login + if ( + path !== FRONTEND_PATHS.LOGOUT && + path !== FRONTEND_PATHS.SESSION_LOGGED_OUT && + shouldRedirectDueToCrossTabLogout() + ) { + return redirect(FRONTEND_PATHS.LOGOUT, "Recent cross-tab logout detected - redirecting to logout page") + } + + // Transitional states - don't redirect. Login / Logout sequence will take care of it. + if (auth.isSigningOut && !checkForRecentLogoutMarker("NotSignedIn-SigningOut") + && !PUBLIC_PATHS.includes(path) && !auth.invalidSessionCause) { + return handleSignoutEvent(auth, navigate, "NotSignedIn-SigningOut", auth.invalidSessionCause) + } + + // Capture this case to prevent new login session being redirected + // If the path is select your role + if (auth.isSigningIn && path === FRONTEND_PATHS.SELECT_YOUR_ROLE) { + const {codeParams, stateParams, errorParams} = getSearchParams(window) + if ((codeParams || stateParams) && !errorParams) { + // Only allow through if a successful login. + logger.info("User signing in with OAuth - allowing access to path with search params") + return + } + return handleSignoutEvent(auth, navigate, "NotSignedIn-SigningIn", auth.invalidSessionCause) + } + + if (checkForRecentLogoutMarker("NotSignedIn-LogoutMarkerPresent")) { + logger.info("Recent logout marker found, not redirecting, awaiting completion") + return + } + + // Allow the logout page to exist without redirections + if (path === FRONTEND_PATHS.LOGOUT || path === FRONTEND_PATHS.SESSION_LOGGED_OUT) { + return + } - if (!loggingOut && noRole && (!inNoRoleAllowed || atRoot)) { - return redirect(FRONTEND_PATHS.SELECT_YOUR_ROLE, `No selected role - Redirecting from ${path}`) + return redirect(FRONTEND_PATHS.LOGIN, `Not signed in - redirecting to login page`) } - if (authedAtRoot) { - return redirect(FRONTEND_PATHS.SEARCH_BY_PRESCRIPTION_ID, - "Authenticated user on root path - redirecting to search") + // Signed in - check states in priority order + if (auth.isSignedIn) { + if (auth.isSigningOut && + (path !== FRONTEND_PATHS.LOGOUT && path !== FRONTEND_PATHS.SESSION_LOGGED_OUT + && !checkForRecentLogoutMarker("SignedIn-LogoutPath") + )) { + return handleSignoutEvent(auth, navigate, "SignedIn-SigningOut", auth.invalidSessionCause) + } + + if (!auth.isSigningOut && path === FRONTEND_PATHS.LOGOUT) { + // If a user navigates directly to the logout page, forcefully log them out. + return handleSignoutEvent(auth, navigate, "SignedIn-AtLogoutPath", auth.invalidSessionCause) + } + + if (auth.isConcurrentSession && path !== FRONTEND_PATHS.SESSION_SELECTION) { + return redirect(FRONTEND_PATHS.SESSION_SELECTION, "Concurrent session found - redirecting to session selection") + } + + if (!auth.selectedRole && + (path === "/" || path === FRONTEND_PATHS.LOGIN || !ALLOWED_NO_ROLE_PATHS.includes(path))) { + return redirect(FRONTEND_PATHS.SELECT_YOUR_ROLE, `No selected role - Redirecting from ${path}`) + } + + if (auth.selectedRole && (path === "/" || path === FRONTEND_PATHS.LOGIN)) { + return redirect(FRONTEND_PATHS.SEARCH_BY_PRESCRIPTION_ID, "User already logged in. Role already selected.") + } } } const checkUserInfo = () => { // Check if a user is signed in, if it fails sign the user out - if (auth.isSigningIn === true || ALLOWED_NO_ROLE_PATHS.includes(location.pathname)) { + if (auth.isSigningIn && ALLOWED_NO_ROLE_PATHS.includes(location.pathname)) { logger.debug("Not checking user info") return } - if (auth.isSignedIn) { + if (auth.isSignedIn && !auth.isSigningOut && !checkForRecentLogoutMarker("checkUserInfo")) { logger.debug("Refreshing user info") auth.updateTrackerUserInfo().then((response) => { if (response.error) { - logger.debug("Restarting login") - handleRestartLogin(auth, response.invalidSessionCause) + logger.debug("updateTrackerUserInfo returned error, signing out user", response.error) + handleSignoutEvent(auth, navigate, "UserInfoCheck", response.invalidSessionCause) } else { const remainingTime = response.remainingSessionTime const remainingSeconds = remainingTime !== undefined ? Math.floor(remainingTime / 1000) : undefined @@ -131,7 +229,7 @@ export const AccessProvider = ({children}: {children: ReactNode}) => { } else if (remainingSeconds <= 0) { logger.warn("Session expired - automatically logging out user") auth.updateInvalidSessionCause("Timeout") - handleRestartLogin(auth, "Timeout") + handleSignoutEvent(auth, navigate, "Timeout") } else { // Session still valid, ensure modal is hidden and update time info logger.debug("Session still valid - hiding modal if shown", {remainingTime}) @@ -143,59 +241,58 @@ export const AccessProvider = ({children}: {children: ReactNode}) => { }) } } else { - // No remaining session time info available - this indicates a session integrity issue + // No remaining session time info available - this indicates a session integrity issue logger.warn("No remainingSessionTime in response - session may be corrupted, logging out user") auth.updateInvalidSessionCause("InvalidSession") - handleRestartLogin(auth, "InvalidSession") + handleSignoutEvent(auth, navigate, "InvalidSession") } } }) } + logger.debug("No conditions met - not checking user info") return } useEffect(() => { - const currentPath = location.pathname - const onSelectYourRole = currentPath === FRONTEND_PATHS.SELECT_YOUR_ROLE - if (auth.isSigningIn && onSelectYourRole) { - return - } + // Note: Any logical assertions should be placed within the function. + // Any placed here cause developer confusion. + + // Implementation notes: useNavigate as a dependency causes an infinite loop as redirects use it ensureRoleSelected() }, [ auth.isSignedIn, auth.isSigningIn, + auth.isSigningOut, auth.selectedRole, auth.isConcurrentSession, - location.pathname, - navigate + auth.logoutMarker, + location.pathname ]) useEffect(() => { + const tabId = getOrCreateTabId() + // Check if user is logged in on page load. logger.debug("On load user info check") + heartbeatTab(tabId) + pruneStaleTabIds() checkUserInfo() // Then check every minute const interval = setInterval(() => { logger.debug("Periodic user info check") + heartbeatTab(tabId) + pruneStaleTabIds() checkUserInfo() }, 60000) // 60000 ms = 1 minute return () => clearInterval(interval) }, [auth.isSignedIn, auth.isSigningIn, location.pathname]) - if (shouldBlockChildren()) { - return ( - - - - ) - } - return ( - {children} + {shouldBlockChildren() ? : children} ) } diff --git a/packages/cpt-ui/src/context/AuthProvider.tsx b/packages/cpt-ui/src/context/AuthProvider.tsx index 99b0d557f2..3c1cef4e98 100644 --- a/packages/cpt-ui/src/context/AuthProvider.tsx +++ b/packages/cpt-ui/src/context/AuthProvider.tsx @@ -12,6 +12,7 @@ import {authConfig} from "./configureAmplify" import {useLocalStorageState} from "@/helpers/useLocalStorageState" import {API_ENDPOINTS} from "@/constants/environment" +import {getOrCreateTabId} from "@/helpers/tabHelpers" import http from "@/helpers/axios" import { @@ -22,9 +23,13 @@ import { } from "@cpt-ui-common/common-types" import {getTrackerUserInfo, updateRemoteSelectedRole} from "@/helpers/userInfo" import {logger} from "@/helpers/logger" +import {LOGOUT_MARKER_STORAGE_KEY, LOGOUT_MARKER_STORAGE_GROUP} from "@/constants/environment" +import {LogoutMarker, clearLogoutMarkerFromStorage} from "@/helpers/logout" const CIS2SignOutEndpoint = API_ENDPOINTS.CIS2_SIGNOUT_ENDPOINT +export const TAB_ID_SESSION_KEY = getOrCreateTabId() + export interface AuthContextType { error: string | null user: string | null @@ -40,9 +45,11 @@ export interface AuthContextType { selectedRole: RoleDetails | undefined userDetails: UserDetails | undefined remainingSessionTime: number | undefined + logoutMarker: LogoutMarker | undefined sessionTimeoutModalInfo: SessionTimeoutModal logoutModalType: "userInitiated" | "timeout" | undefined setSessionTimeoutModalInfo: (value: SetStateAction) => void + setLogoutMarker: (LogoutMarker: LogoutMarker | undefined) => void setLogoutModalType: (value: "userInitiated" | "timeout" | undefined) => Promise cognitoSignIn: (input?: SignInWithRedirectInput) => Promise cognitoSignOut: (redirectUri?: string) => Promise @@ -52,7 +59,8 @@ export interface AuthContextType { updateTrackerUserInfo: () => Promise updateInvalidSessionCause: (cause: string) => void setIsSigningOut: (value: boolean) => void - setStateForSignOut: () => Promise + setStateForSignOut: () => void + setStateForSignIn: () => void } export const AuthContext = createContext(null) @@ -93,6 +101,12 @@ export const AuthProvider = ({children}: { children: React.ReactNode }) => { "remainingSessionTime", undefined ) + const [logoutMarker, setLogoutMarker] = useLocalStorageState( + LOGOUT_MARKER_STORAGE_KEY, + LOGOUT_MARKER_STORAGE_GROUP, + undefined + ) + const [sessionTimeoutModalInfo, setSessionTimeoutModalInfo] = useLocalStorageState( "sessionTimeoutModalInfo", "sessionTimeoutModalInfo", @@ -120,27 +134,28 @@ export const AuthProvider = ({children}: { children: React.ReactNode }) => { setIsConcurrentSession(false) setRemainingSessionTime(undefined) setSessionTimeoutModalInfo({showModal: false, timeLeft: 0, buttonDisabled: false, action: undefined}) - // updateTrackerUserInfo will set InvalidSessionCause to undefined - } - - /** Sign out state helper */ - const setStateForSignOut = async () => { - setIsSigningOut(true) + setSessionId(undefined) } const updateTrackerUserInfo = async () => { const trackerUserInfo = await getTrackerUserInfo() if (!trackerUserInfo.error) { - setRolesWithAccess(trackerUserInfo.rolesWithAccess) - setRolesWithoutAccess(trackerUserInfo.rolesWithoutAccess) - setSelectedRole(trackerUserInfo.selectedRole) - setUserDetails(trackerUserInfo.userDetails) + if (!(rolesWithAccess === trackerUserInfo.rolesWithAccess)) + setRolesWithAccess(trackerUserInfo.rolesWithAccess) + if (!(rolesWithoutAccess === trackerUserInfo.rolesWithoutAccess)) + setRolesWithoutAccess(trackerUserInfo.rolesWithoutAccess) + if (!(selectedRole === trackerUserInfo.selectedRole)) + setSelectedRole(trackerUserInfo.selectedRole) + if (!(userDetails === trackerUserInfo.userDetails)) + setUserDetails(trackerUserInfo.userDetails) } + setInvalidSessionCause(trackerUserInfo.invalidSessionCause) // Set first setIsConcurrentSession(trackerUserInfo.isConcurrentSession) setSessionId(trackerUserInfo.sessionId) setRemainingSessionTime(trackerUserInfo.remainingSessionTime) setError(trackerUserInfo.error) - setInvalidSessionCause(trackerUserInfo.invalidSessionCause) + + getOrCreateTabId() // Ensure tab ID is set in session/local storage for logout marker logic return trackerUserInfo } @@ -154,13 +169,13 @@ export const AuthProvider = ({children}: { children: React.ReactNode }) => { // On successful signIn or token refresh, get the latest user state case "signedIn": { logger.info("Processing signedIn event") - logger.info("User %s logged in", payload.data.username) + logger.info(`User ${payload.data.username} logged in at ${new Date().toISOString()}`) await updateTrackerUserInfo() setIsSignedIn(true) setIsSigningIn(false) setUser(payload.data.username) - logger.info("Finished the signedIn event ") + logger.info("Finished the signedIn event") break } case "tokenRefresh": @@ -169,6 +184,7 @@ export const AuthProvider = ({children}: { children: React.ReactNode }) => { break case "signInWithRedirect": logger.info("Processing signInWithRedirect event") + setIsSigningIn(true) setError(null) break @@ -186,14 +202,12 @@ export const AuthProvider = ({children}: { children: React.ReactNode }) => { case "signedOut": logger.info("Processing signedOut event") - setIsSigningOut(true) clearAuthState() setError(null) break default: logger.info("Received unknown event", payload) - // Other auth events? The type-defined cases are already handled above. break } }) @@ -203,39 +217,60 @@ export const AuthProvider = ({children}: { children: React.ReactNode }) => { } }, []) + /** signOut state helper + * Don't use outside of logout helper + */ + const setStateForSignOut = () => { + setIsSignedIn(false) + setIsSigningIn(false) + setIsSigningOut(true) + } + + /** signOut state helper + * Don't use outside of logout helper + */ + const setStateForSignIn = () => { + setIsSigningIn(true) + setIsSigningOut(false) + setInvalidSessionCause(undefined) + logger.info("Setting log out marker to undefined") + clearLogoutMarkerFromStorage("SetStateForSignIn") + } + /** - * Sign out process. + * Cognito signout process with redirect URL. + * Don't use directly, use the helper functions signOut or handleLogoutEvent */ - const cognitoSignOut = async (signoutRedirectUrl?: string): Promise => { - logger.info("Signing out in authProvider...") + const cognitoSignOut = async ( + signoutRedirectUrl?: string | undefined + ): Promise => { try { + logger.info("Signing out in authProvider...") // Call CIS2 signout first, this ensures a session remains on Amplify side. logger.info(`Calling CIS2 Signout ${CIS2SignOutEndpoint}`) try { await http.get(CIS2SignOutEndpoint) logger.info("Successfully signed out of CIS2") } catch (err) { - logger.error("Failed to sign out of CIS2:", err) + throw new Error("Failed to sign out of CIS2:", {cause: err}) } + // handleSignoutEvent helper always sets a redirection URL if (signoutRedirectUrl) { logger.info("Calling Amplify Signout, with redirect URL", signoutRedirectUrl) await signOut({ global: true, oauth: {redirectUrl: signoutRedirectUrl} }) - } else { - logger.info("Calling Amplify Signout, no redirect URL") await signOut({global: true}) } logger.info("Frontend amplify signout OK!") return true } catch (err) { - logger.error("Failed to sign out:", err) setError(String(err)) - return false + throw new Error("Failed to sign out", {cause: err}) } } @@ -244,8 +279,8 @@ export const AuthProvider = ({children}: { children: React.ReactNode }) => { */ const cognitoSignIn = async (input?: SignInWithRedirectInput) => { logger.info("Initiating sign-in process...") + await signInWithRedirect(input) - setIsSigningIn(true) } const updateSelectedRole = async (newRole: RoleDetails) => { @@ -286,9 +321,11 @@ export const AuthProvider = ({children}: { children: React.ReactNode }) => { sessionId, remainingSessionTime, deviceId, + logoutMarker, sessionTimeoutModalInfo, logoutModalType, setSessionTimeoutModalInfo, + setLogoutMarker, setLogoutModalType: setLogoutModalTypeAsync, cognitoSignIn, cognitoSignOut, @@ -298,7 +335,8 @@ export const AuthProvider = ({children}: { children: React.ReactNode }) => { updateTrackerUserInfo, updateInvalidSessionCause, setIsSigningOut, - setStateForSignOut + setStateForSignOut, + setStateForSignIn }}> {children} diff --git a/packages/cpt-ui/src/helpers/axios.tsx b/packages/cpt-ui/src/helpers/axios.tsx index e3fbb39eaa..3649df0648 100644 --- a/packages/cpt-ui/src/helpers/axios.tsx +++ b/packages/cpt-ui/src/helpers/axios.tsx @@ -54,11 +54,14 @@ http.interceptors.request.use( const authSession = await fetchAuthSession() const idToken = authSession.tokens?.idToken - if (idToken === undefined) { - logger.error("Could not get a cognito token") + const isAmplifyHostRequest = config.url?.includes("/api/cis2-signout") ?? false + if (idToken === undefined && !isAmplifyHostRequest) { controller.abort() + throw new Error("Could not get a cognito token") + } + if (idToken) { + config.headers.Authorization = `Bearer ${idToken.toString()}` } - config.headers.Authorization = `Bearer ${idToken?.toString()}` // Make sure we have a retry counter in headers so we can track how many times we've retried if (!config.headers[x_retry_header]) { diff --git a/packages/cpt-ui/src/helpers/getSearchParams.tsx b/packages/cpt-ui/src/helpers/getSearchParams.tsx index b8c9ad2f64..b9dbfcc80b 100644 --- a/packages/cpt-ui/src/helpers/getSearchParams.tsx +++ b/packages/cpt-ui/src/helpers/getSearchParams.tsx @@ -2,5 +2,6 @@ export function getSearchParams(window: Window & typeof globalThis) { const params = new URLSearchParams(window.location.search) const codeParams = params.get("code") const stateParams = params.get("state") - return {codeParams, stateParams} + const errorParams = params.get("error") + return {codeParams, stateParams, errorParams} } diff --git a/packages/cpt-ui/src/helpers/loginFunctions.tsx b/packages/cpt-ui/src/helpers/loginFunctions.tsx index 40992337b8..3950216b76 100644 --- a/packages/cpt-ui/src/helpers/loginFunctions.tsx +++ b/packages/cpt-ui/src/helpers/loginFunctions.tsx @@ -1,5 +1,36 @@ -import {FRONTEND_PATHS} from "@/constants/environment" +import {FRONTEND_PATHS, AUTH_CONFIG} from "@/constants/environment" +import {AuthContextType} from "@/context/AuthProvider" +import {logger} from "@/helpers/logger" +import {signOut, checkForRecentLogoutMarker} from "@/helpers/logout" +import {AuthError} from "@aws-amplify/auth" export const getHomeLink = (isSignedIn: boolean) => { return isSignedIn ? FRONTEND_PATHS.SEARCH_BY_PRESCRIPTION_ID : FRONTEND_PATHS.LOGIN } + +export const handleSignIn = async ( + auth: AuthContextType, + type: "Primary" | "Mock", + navigate: (path: string) => void) => { + if (checkForRecentLogoutMarker("SignIn")) { + logger.info("Attempting to sign-out, prohibit sign as well") + } + + auth.setStateForSignIn() + logger.info(`Redirecting user to ${type} login`) + + try { + await auth?.cognitoSignIn({ + provider: { + custom: type + } + }) + } catch (err) { + if ((err as AuthError).name === "UserAlreadyAuthenticatedException") { + logger.info("User already authenticated, operating signout") + signOut(auth, navigate, AUTH_CONFIG.REDIRECT_SIGN_OUT, true) + } + logger.error(`Error during ${type} sign in:`, err) + } + logger.info("Signed in: ", auth) +} diff --git a/packages/cpt-ui/src/helpers/logout.tsx b/packages/cpt-ui/src/helpers/logout.tsx index 546d3889f1..e4a88c2f0c 100644 --- a/packages/cpt-ui/src/helpers/logout.tsx +++ b/packages/cpt-ui/src/helpers/logout.tsx @@ -1,49 +1,167 @@ import {AuthContextType} from "@/context/AuthProvider" import {logger} from "./logger" -import {AUTH_CONFIG} from "@/constants/environment" +import {AUTH_CONFIG, FRONTEND_PATHS} from "@/constants/environment" +import {readItemGroupFromLocalStorage} from "@/helpers/useLocalStorageState" +import {getOrCreateTabId, getOpenTabIds} from "@/helpers/tabHelpers" +import { + LOGOUT_MARKER_STORAGE_KEY, + LOGOUT_MARKER_STORAGE_GROUP, + LOGOUT_MARKER_MAX_AGE_MS +} from "@/constants/environment" -/* -** Universal sign out helper functions -** Used on the LogoutPage & SessionLoggedOutPage for a consistent sign out process +export type LogoutMarker = { + timestamp: number + reason: "signOut" + initiatedByTabId: string +} + +export const handleSignoutEvent = async ( + auth: AuthContextType, + navigate: (path: string) => void, + caller?: string, + invalidSessionCause?: string +) => { + const invalidSessionReason = auth.invalidSessionCause ? auth.invalidSessionCause : invalidSessionCause + + logger.info(`Handling sign out event with caller: ${caller}` + + (invalidSessionReason ? ` and invalid session reason: ${invalidSessionReason}` : "")) + + if (invalidSessionReason) { + logger.info(`Invalid session cause supplied, ${invalidSessionReason}`) + await auth.updateInvalidSessionCause(invalidSessionReason) + await signOut(auth, navigate, AUTH_CONFIG.REDIRECT_SESSION_SIGN_OUT, false) + } else { + logger.info("No invalid session cause, using standard logout") + await signOut(auth, navigate, AUTH_CONFIG.REDIRECT_SIGN_OUT, false) + } +} + +/* signOut offers explicit control over the sign out process. +* For most use cases handleSignoutEvent should be used to ensure the invalid session cause is properly handled +* and passed through to the logout redirect page if needed. */ +export const signOut = async ( + authParam: AuthContextType, + navigate?: (path: string) => void, + redirectUri?: string | undefined, + duplicateSignout?: boolean +) => { + if (!duplicateSignout) { + const existingMarker = checkForRecentLogoutMarker("SignOut") + if (existingMarker) { + const openTabs = getOpenTabIds() + if (openTabs.includes(existingMarker.initiatedByTabId)) { + logger.info("Skipping duplicate signOut call due to in-progress marker from active tab") + return + } + logger.info("Logout marker exists from inactive tab - allowing signOut to proceed") + } + } -export const signOut = async (authParam: AuthContextType, redirectUri?: string) => { - await authParam.setStateForSignOut() + // Prepare state before actioning logout + createOrUpdateLogoutMarker() + authParam.setStateForSignOut() const location = window.location.pathname - logger.info(`Called signOut helper from ${location} with redirect of ${redirectUri}`) + logger.info(`Called signOut helper from ${location}` + + (redirectUri ? ` with redirect of ${redirectUri}` : "") + + (duplicateSignout ? " allowing duplicate sign out" : ""), + authParam) try { - if (redirectUri) { - logger.info("Signing out with specified redirect path", redirectUri) - await authParam?.cognitoSignOut(redirectUri) - } else { - const defaultUri = AUTH_CONFIG.REDIRECT_SIGN_OUT - logger.info("Signing out with default redirect path", defaultUri) - await authParam?.cognitoSignOut(defaultUri) + logger.info("Signing out with specified redirect path", redirectUri) + await authParam?.cognitoSignOut(redirectUri) + authParam.clearAuthState() + // Don't clear logout marker here - let login sequence reset it + // That way mid-logout redirections don't occur + } catch (err) { + logger.error(`Error during sign out` + (redirectUri ? ` with specified redirect path: ${redirectUri}` : ""), err) + + // Unwrap the full error cause chain so the root cause is visible in the console + let current: unknown = err + while (current instanceof Error) { + // eslint-disable-next-line no-console + console.error("[signOut] Error:", current.message, current) + current = current.cause } - // Status hub will clear auth state + authParam.clearAuthState() + + navigate?.(FRONTEND_PATHS.LOGOUT) + } + // Status hub will clear auth state +} + +const createOrUpdateLogoutMarker = (): LogoutMarker => { + const existingMarker = checkForRecentLogoutMarker("CheckOrUpdate") + if (existingMarker) { + existingMarker.timestamp = Date.now() + existingMarker.initiatedByTabId = getOrCreateTabId() + writeLogoutMarker(existingMarker) + return existingMarker + } + + const now = Date.now() + const newMarker: LogoutMarker = { + timestamp: now, + reason: "signOut", + initiatedByTabId: getOrCreateTabId() + } + writeLogoutMarker(newMarker) + return newMarker +} + +export const readLogoutMarker = () => { + const markerGroup = + readItemGroupFromLocalStorage(LOGOUT_MARKER_STORAGE_GROUP) as Record + return markerGroup[LOGOUT_MARKER_STORAGE_KEY] +} + +const writeLogoutMarker = (marker: LogoutMarker) => { + logger.info("Writing log out marker to storage", marker) + try { + const markerGroup = + readItemGroupFromLocalStorage(LOGOUT_MARKER_STORAGE_GROUP) as Record + markerGroup[LOGOUT_MARKER_STORAGE_KEY] = marker + window.localStorage.setItem(LOGOUT_MARKER_STORAGE_GROUP, JSON.stringify(markerGroup)) } catch (error) { - logger.error("Error during logout:", error) - authParam.setIsSigningOut(false) - throw error + logger.error("Unable to write logout marker to storage", error) } } -export const handleRestartLogin = async (auth: AuthContextType, invalidSessionCause: string | undefined) => { - logger.info("Handling restart login instruction from backend", invalidSessionCause) - logger.info("AUTH_CONFIG values:", { - REDIRECT_SIGN_OUT: AUTH_CONFIG.REDIRECT_SIGN_OUT, - REDIRECT_SESSION_SIGN_OUT: AUTH_CONFIG.REDIRECT_SESSION_SIGN_OUT - }) +/* Exported functions used within this helper, AuthProvider or AccessProvider */ +export const checkForRecentLogoutMarker = (caller?: string) => { + const existingMarker = readLogoutMarker() + if (existingMarker) { + logger.info(`Found existing logout marker in storage. ${caller ? `Called by ${caller}` : ""}`, existingMarker) + if (isRecentMarker(existingMarker)) { + logger.info(`Existing marker is recent. ${caller ? `Called by ${caller}` : ""}`, existingMarker) + return existingMarker + } + clearLogoutMarkerFromStorage("CheckForRecentLogoutMarker") + return undefined + } + logger.debug(`No recent logout marker found. ${caller ? `Called by ${caller}` : ""}`) + return undefined +} - if (invalidSessionCause) { - logger.info(`Invalid session cause supplied, ${invalidSessionCause}`) - await auth.updateInvalidSessionCause(invalidSessionCause) - logger.info("About to sign out with REDIRECT_SESSION_SIGN_OUT:", AUTH_CONFIG.REDIRECT_SESSION_SIGN_OUT) - await signOut(auth, AUTH_CONFIG.REDIRECT_SESSION_SIGN_OUT) +export const clearLogoutMarkerFromStorage = (caller?: string) => { + if (typeof window === "undefined") { + logger.info("No window defined, unable to clear logout marker from storage") return } - logger.info("No invalid session cause, using REDIRECT_SIGN_OUT:", AUTH_CONFIG.REDIRECT_SIGN_OUT) - await signOut(auth, AUTH_CONFIG.REDIRECT_SIGN_OUT) + + try { + logger.debug(`Clearing logout marker from storage. ${caller ? `Called by ${caller}` : ""}`) + let markerGroup = + readItemGroupFromLocalStorage(LOGOUT_MARKER_STORAGE_GROUP) as Record + markerGroup[LOGOUT_MARKER_STORAGE_KEY] = undefined + window.localStorage.setItem(LOGOUT_MARKER_STORAGE_GROUP, JSON.stringify(markerGroup)) + } catch (error) { + logger.error("Unable to clear logout marker from storage", error) + } +} + +export const isRecentMarker = (marker: LogoutMarker | undefined) => { + const isRecent = !!marker && Date.now() - marker.timestamp <= LOGOUT_MARKER_MAX_AGE_MS + return isRecent } diff --git a/packages/cpt-ui/src/helpers/tabHelpers.ts b/packages/cpt-ui/src/helpers/tabHelpers.ts new file mode 100644 index 0000000000..010d125191 --- /dev/null +++ b/packages/cpt-ui/src/helpers/tabHelpers.ts @@ -0,0 +1,145 @@ +import { + TAB_ID_SESSION_KEY, + OPEN_TABS_STORAGE_KEY, + TAB_HEARTBEATS_STORAGE_KEY, + TAB_STALE_THRESHOLD_MS +} from "@/constants/environment" + +// Module-level cache so we only resolve once per page lifecycle +let resolvedTabId: string | null = null + +export const getOrCreateTabId = () => { + if (typeof window === "undefined") { + return "server" + } + + if (resolvedTabId) { + return resolvedTabId + } + + const existingTabId = window.sessionStorage.getItem(TAB_ID_SESSION_KEY) + if (existingTabId) { + // Check if another tab is already using this ID (i.e. this tab was duplicated). + // On a normal reload, beforeunload removes the ID from the open tabs list first, + // so it won't appear here. On a duplicate, the original tab is still open and + // its ID remains in the list, so we detect the collision and create a new one. + const openTabIds = getOpenTabIds() + if (!openTabIds.includes(existingTabId)) { + resolvedTabId = existingTabId + return existingTabId + } + } + + const createdTabId = crypto.randomUUID() + window.sessionStorage.setItem(TAB_ID_SESSION_KEY, createdTabId) + resolvedTabId = createdTabId + return createdTabId +} + +export const getOpenTabIds = () => { + if (typeof window === "undefined") { + return [] as Array + } + + try { + const raw = window.localStorage.getItem(OPEN_TABS_STORAGE_KEY) + if (!raw) { + return [] as Array + } + + const parsed = JSON.parse(raw) + return Array.isArray(parsed) ? parsed.filter((tabId): tabId is string => typeof tabId === "string") : [] + } catch { + return [] as Array + } +} + +export const setOpenTabIds = (tabIds: Array) => { + if (typeof window === "undefined") { + return + } + + window.localStorage.setItem(OPEN_TABS_STORAGE_KEY, JSON.stringify(tabIds)) +} + +export const updateOpenTabs = (tabId: string, action: "add" | "remove") => { + const existingTabIds = getOpenTabIds() + + if (action === "add") { + const updatedTabIds = existingTabIds.includes(tabId) ? existingTabIds : [...existingTabIds, tabId] + setOpenTabIds(updatedTabIds) + heartbeatTab(tabId) + return + } + + setOpenTabIds(existingTabIds.filter((existingTabId) => existingTabId !== tabId)) + removeTabHeartbeat(tabId) +} + +export const getOpenTabCount = () => { + const count = getOpenTabIds().length + return count === 0 ? 1 : count +} + +/* Heartbeat tracking — each tab periodically records a timestamp so stale entries can be detected */ + +const getHeartbeats = (): Record => { + if (typeof window === "undefined") { + return {} + } + try { + const raw = window.localStorage.getItem(TAB_HEARTBEATS_STORAGE_KEY) + if (!raw) return {} + const parsed = JSON.parse(raw) + return typeof parsed === "object" && parsed !== null && !Array.isArray(parsed) + ? parsed as Record + : {} + } catch { + return {} + } +} + +const setHeartbeats = (heartbeats: Record) => { + if (typeof window === "undefined") return + window.localStorage.setItem(TAB_HEARTBEATS_STORAGE_KEY, JSON.stringify(heartbeats)) +} + +export const heartbeatTab = (tabId: string) => { + const heartbeats = getHeartbeats() + heartbeats[tabId] = Date.now() + setHeartbeats(heartbeats) +} + +const removeTabHeartbeat = (tabId: string) => { + const heartbeats = getHeartbeats() + delete heartbeats[tabId] + setHeartbeats(heartbeats) +} + +/** + * Remove tab IDs that have not sent a heartbeat within the threshold. + * Should be called periodically (e.g. every minute in AccessProvider). + */ +export const pruneStaleTabIds = (thresholdMs: number = TAB_STALE_THRESHOLD_MS) => { + const now = Date.now() + const heartbeats = getHeartbeats() + const openTabIds = getOpenTabIds() + + const staleTabIds = openTabIds.filter((tabId) => { + const lastSeen = heartbeats[tabId] + // If no heartbeat recorded, consider it stale + return lastSeen === undefined || (now - lastSeen) > thresholdMs + }) + + if (staleTabIds.length === 0) return + + // Remove stale tabs from the open list and heartbeat map + const freshTabIds = openTabIds.filter((tabId) => !staleTabIds.includes(tabId)) + setOpenTabIds(freshTabIds) + + const freshHeartbeats = {...heartbeats} + for (const tabId of staleTabIds) { + delete freshHeartbeats[tabId] + } + setHeartbeats(freshHeartbeats) +} diff --git a/packages/cpt-ui/src/hooks/useSessionTimeout.ts b/packages/cpt-ui/src/hooks/useSessionTimeout.ts index 20ae97d5b4..9abe96c2f1 100644 --- a/packages/cpt-ui/src/hooks/useSessionTimeout.ts +++ b/packages/cpt-ui/src/hooks/useSessionTimeout.ts @@ -2,9 +2,8 @@ import {useCallback, useRef} from "react" import {logger} from "@/helpers/logger" import {useAuth} from "@/context/AuthProvider" import {updateRemoteSelectedRole} from "@/helpers/userInfo" -import {handleRestartLogin, signOut} from "@/helpers/logout" -import {AUTH_CONFIG} from "@/constants/environment" - +import {handleSignoutEvent} from "@/helpers/logout" +import {useNavigate} from "react-router-dom" export interface SessionTimeoutProps { onStayLoggedIn: () => Promise onLogOut: () => Promise @@ -13,6 +12,7 @@ export interface SessionTimeoutProps { export const useSessionTimeout = () => { const auth = useAuth() + const navigate = useNavigate() const actionLockRef = useRef<"extending" | "loggingOut" | undefined>(undefined) const clearCountdownTimer = () => { @@ -67,15 +67,14 @@ export const useSessionTimeout = () => { actionLockRef.current = "loggingOut" logger.info("User chose to log out from session timeout modal") auth.setSessionTimeoutModalInfo(prev => ({...prev, action: "loggingOut", buttonDisabled: true})) - await signOut(auth, AUTH_CONFIG.REDIRECT_SIGN_OUT) + await handleSignoutEvent(auth, navigate, "Timeout", "Timeout") auth.setLogoutModalType(undefined) }, [auth]) const handleTimeout = useCallback(async () => { logger.warn("Session automatically timed out") clearCountdownTimer() - auth.updateInvalidSessionCause("Timeout") - await handleRestartLogin(auth, "Timeout") + await handleSignoutEvent(auth, navigate, "Timeout", "Timeout") }, [auth]) return { diff --git a/packages/cpt-ui/src/pages/BasicDetailsSearchResultsPage.tsx b/packages/cpt-ui/src/pages/BasicDetailsSearchResultsPage.tsx index 11c72f8a8b..eac5f6101e 100644 --- a/packages/cpt-ui/src/pages/BasicDetailsSearchResultsPage.tsx +++ b/packages/cpt-ui/src/pages/BasicDetailsSearchResultsPage.tsx @@ -25,7 +25,7 @@ import UnknownErrorMessage from "@/components/UnknownErrorMessage" import {usePageTitle} from "@/hooks/usePageTitle" import axios from "axios" import {useAuth} from "@/context/AuthProvider" -import {handleRestartLogin} from "@/helpers/logout" +import {handleSignoutEvent} from "@/helpers/logout" import {STRINGS} from "@/constants/ui-strings/PatientDetailsBannerStrings" import {format} from "date-fns" import {DOB_FORMAT, NHS_NUMBER_FORMAT_REGEX} from "@/constants/misc" @@ -169,7 +169,7 @@ export default function SearchResultsPage() { } catch (err) { if (axios.isAxiosError(err) && err.response?.status === 401) { const invalidSessionCause = err.response?.data?.invalidSessionCause - handleRestartLogin(auth, invalidSessionCause) + handleSignoutEvent(auth, navigate, "BasicDetailsError", invalidSessionCause) return } logger.error("Error loading search results:", err) diff --git a/packages/cpt-ui/src/pages/LoadingPage.tsx b/packages/cpt-ui/src/pages/LoadingPage.tsx index d273f7bb1f..88c3e1cb07 100644 --- a/packages/cpt-ui/src/pages/LoadingPage.tsx +++ b/packages/cpt-ui/src/pages/LoadingPage.tsx @@ -36,7 +36,7 @@ export default function LoadingPage() { -

{LOADING_STRINGS.HEADER}

+

{LOADING_STRINGS.HEADER}

If you have not been redirected after 1 minute,{" "} log out diff --git a/packages/cpt-ui/src/pages/LoginPage.tsx b/packages/cpt-ui/src/pages/LoginPage.tsx index 7cde402244..af0c0a09cb 100644 --- a/packages/cpt-ui/src/pages/LoginPage.tsx +++ b/packages/cpt-ui/src/pages/LoginPage.tsx @@ -9,61 +9,32 @@ import {EpsLoginPageStrings} from "@/constants/ui-strings/EpsLoginPageStrings" import {AUTO_LOGIN_ENVIRONMENTS, ENV_CONFIG, type Environment} from "@/constants/environment" import {Button} from "@/components/ReactRouterButton" import {logger} from "@/helpers/logger" -import {AUTH_CONFIG} from "@/constants/environment" -import {signOut} from "@/helpers/logout" +import {handleSignoutEvent} from "@/helpers/logout" +import {handleSignIn} from "@/helpers/loginFunctions" +import {useNavigate} from "react-router-dom" export default function LoginPage() { const auth = useAuth() + const navigate = useNavigate() const target_environment: string = ENV_CONFIG.TARGET_ENVIRONMENT as Environment const isAutoLoginEnvironment = AUTO_LOGIN_ENVIRONMENTS.map(x => x.environment).includes(target_environment) - const mockSignIn = async () => { - logger.info("Signing in (Mock)", auth) - await auth?.cognitoSignIn({ - provider: { - custom: "Mock" - } - }) - } - - const cis2SignIn = async () => { - logger.info("Signing in (Primary)", auth) - await auth?.cognitoSignIn({ - provider: { - custom: "Primary" - } - }) - logger.info("Signed in: ", auth) - } - - const logout = async () => { - logger.info("Signing out", auth) - await signOut(auth, AUTH_CONFIG.REDIRECT_SIGN_OUT) - logger.info("Signed out: ", auth) - } - useEffect(() => { logger.info( "Login page loaded. What environment are we in?", target_environment ) - if (isAutoLoginEnvironment) { + if (isAutoLoginEnvironment && !auth.isSignedIn) { logger.info("performing auto login") const autoLoginDetails = AUTO_LOGIN_ENVIRONMENTS.find(x => x.environment === target_environment) - if (autoLoginDetails?.loginMethod === "cis2") { - logger.info("Redirecting user to cis2 login") - cis2SignIn() - } else { - logger.info("Redirecting user to mock login") - mockSignIn() - } + handleSignIn(auth, autoLoginDetails?.loginMethod === "cis2" ? "Primary" : "Mock", navigate) } }, []) - if (isAutoLoginEnvironment) { + if (isAutoLoginEnvironment && !auth.isSignedIn) { return (

@@ -104,9 +75,12 @@ export default function LoginPage() { - - - + + + {auth && ( diff --git a/packages/cpt-ui/src/pages/LogoutPage.tsx b/packages/cpt-ui/src/pages/LogoutPage.tsx index d656fbc653..4fdbcb3774 100644 --- a/packages/cpt-ui/src/pages/LogoutPage.tsx +++ b/packages/cpt-ui/src/pages/LogoutPage.tsx @@ -2,10 +2,7 @@ import React, {Fragment, useEffect} from "react" import {Container} from "nhsuk-react-components" import {Link} from "react-router-dom" import {useAuth} from "@/context/AuthProvider" -import EpsSpinner from "@/components/EpsSpinner" import {EpsLogoutStrings} from "@/constants/ui-strings/EpsLogoutPageStrings" -import {signOut} from "@/helpers/logout" -import {AUTH_CONFIG} from "@/constants/environment" import {usePageTitle} from "@/hooks/usePageTitle" export default function LogoutPage() { @@ -14,30 +11,19 @@ export default function LogoutPage() { usePageTitle(EpsLogoutStrings.PAGE_TITLE) useEffect(() => { - // Only call signOut if we arrived here without already starting the logout process - if (!auth.isSigningOut && (auth.isSignedIn || auth.isSigningIn)) { - signOut(auth, AUTH_CONFIG.REDIRECT_SIGN_OUT) - } else if (auth.isSigningOut && !auth.isSignedIn && !auth.isSigningIn) { - // Reset signing out state if logout completed + if (auth.isSigningOut) { auth.setIsSigningOut(false) } - }, [auth.isSignedIn, auth.isSigningIn, auth.isSigningOut]) + }, [auth.isSigningOut]) return (
- {(!auth?.isSignedIn && !auth.isSigningIn && !auth.isSigningOut) ? ( - -

{EpsLogoutStrings.TITLE}

-

{EpsLogoutStrings.BODY}

- {EpsLogoutStrings.LOGIN_LINK} - - ) : ( - -

{EpsLogoutStrings.LOADING}

- - - )} + +

{EpsLogoutStrings.TITLE}

+

{EpsLogoutStrings.BODY}

+ {EpsLogoutStrings.LOGIN_LINK} +
) diff --git a/packages/cpt-ui/src/pages/PrescriptionDetailsPage.tsx b/packages/cpt-ui/src/pages/PrescriptionDetailsPage.tsx index b5ee0a9aa5..093a19e709 100644 --- a/packages/cpt-ui/src/pages/PrescriptionDetailsPage.tsx +++ b/packages/cpt-ui/src/pages/PrescriptionDetailsPage.tsx @@ -26,7 +26,7 @@ import http from "@/helpers/axios" import {logger} from "@/helpers/logger" import {useSearchContext} from "@/context/SearchProvider" import axios from "axios" -import {handleRestartLogin} from "@/helpers/logout" +import {handleSignoutEvent} from "@/helpers/logout" import {useAuth} from "@/context/AuthProvider" import {usePageTitle} from "@/hooks/usePageTitle" @@ -78,7 +78,7 @@ export default function PrescriptionDetailsPage() { if (axios.isAxiosError(err) && (err.response?.status === 401)) { const invalidSessionCause = err.response?.data?.invalidSessionCause logger.warn("prescriptionDetails triggered restart login due to:", invalidSessionCause) - handleRestartLogin(auth, invalidSessionCause) + handleSignoutEvent(auth, navigate, "PrescriptionDetailsPage", invalidSessionCause) return } logger.error("Failed to fetch prescription details", err) diff --git a/packages/cpt-ui/src/pages/PrescriptionListPage.tsx b/packages/cpt-ui/src/pages/PrescriptionListPage.tsx index dfd5ea7b55..effeb73dad 100644 --- a/packages/cpt-ui/src/pages/PrescriptionListPage.tsx +++ b/packages/cpt-ui/src/pages/PrescriptionListPage.tsx @@ -23,9 +23,8 @@ import http from "@/helpers/axios" import {logger} from "@/helpers/logger" import {useSearchContext} from "@/context/SearchProvider" import {useNavigationContext} from "@/context/NavigationProvider" -import {handleRestartLogin, signOut} from "@/helpers/logout" +import {handleSignoutEvent} from "@/helpers/logout" import {useAuth} from "@/context/AuthProvider" -import {AUTH_CONFIG} from "@/constants/environment" import {usePageTitle} from "@/hooks/usePageTitle" export default function PrescriptionListPage() { @@ -152,7 +151,7 @@ export default function PrescriptionListPage() { if ((err.response?.status === 401) && err.response.data?.restartLogin) { const invalidSessionCause = err.response?.data?.invalidSessionCause logger.warn("prescriptionList triggered restart login due to:", invalidSessionCause) - handleRestartLogin(auth, invalidSessionCause) + handleSignoutEvent(auth, navigate, "PrescriptionListPage", invalidSessionCause) return } else if (err.response?.status === 404) { logger.warn("No search results were returned", err) @@ -164,7 +163,7 @@ export default function PrescriptionListPage() { } } else if (err instanceof Error && err.message === "canceled") { logger.warn("Signing out due to request cancellation") - signOut(auth, AUTH_CONFIG.REDIRECT_SIGN_OUT) + handleSignoutEvent(auth, navigate, "PrescriptionListPage", "RequestCanceled") return } else { setError(true) diff --git a/packages/cpt-ui/src/pages/SessionSelection.tsx b/packages/cpt-ui/src/pages/SessionSelection.tsx index 6bcda698ce..09dec22fc4 100644 --- a/packages/cpt-ui/src/pages/SessionSelection.tsx +++ b/packages/cpt-ui/src/pages/SessionSelection.tsx @@ -1,11 +1,11 @@ import {Container, Col, Row} from "nhsuk-react-components" import {Button} from "@/components/ReactRouterButton" import {useNavigate} from "react-router-dom" -import {AUTH_CONFIG, FRONTEND_PATHS} from "@/constants/environment" +import {FRONTEND_PATHS} from "@/constants/environment" import {postSessionManagementUpdate} from "@/helpers/sessionManagement" import {useAuth} from "@/context/AuthProvider" import {logger} from "@/helpers/logger" -import {signOut} from "@/helpers/logout" +import {handleSignoutEvent} from "@/helpers/logout" import {useState} from "react" import {usePageTitle} from "@/hooks/usePageTitle" import {SESSION_SELECTION_PAGE_STRINGS} from "@/constants/ui-strings/SessionSelectionPage" @@ -18,7 +18,7 @@ export default function SessionSelectionPage() { usePageTitle(SESSION_SELECTION_PAGE_STRINGS.pageTitle) const logout = async () => { - signOut(auth, AUTH_CONFIG.REDIRECT_SIGN_OUT) + handleSignoutEvent(auth, navigate, "SessionSelectionPage") } const redirectUser = (destination: string) => { @@ -37,7 +37,7 @@ export default function SessionSelectionPage() { redirectUser(FRONTEND_PATHS.SEARCH_BY_PRESCRIPTION_ID) } else { logger.info("Redirecting user to login") - signOut(auth, AUTH_CONFIG.REDIRECT_SIGN_OUT) + handleSignoutEvent(auth, navigate, "SessionSelectionPage") } } From 0e7e58d7f0f2c622464af7392cab8171036ac36b Mon Sep 17 00:00:00 2001 From: jonathanwelch1-nhs <148754575+jonathanwelch1-nhs@users.noreply.github.com> Date: Wed, 1 Apr 2026 10:51:22 +0100 Subject: [PATCH 05/21] Fix: [AEA-6021] - conditional render for displaying roles without access table (#1983) ## Summary **Remove items from this list if they are not relevant. Remove this line once this has been done** - Routine Change dont render the roles without access table if there are no roles --- .../src/components/EpsRoleSelectionPage.tsx | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/packages/cpt-ui/src/components/EpsRoleSelectionPage.tsx b/packages/cpt-ui/src/components/EpsRoleSelectionPage.tsx index c7338c99c5..701735515a 100644 --- a/packages/cpt-ui/src/components/EpsRoleSelectionPage.tsx +++ b/packages/cpt-ui/src/components/EpsRoleSelectionPage.tsx @@ -85,7 +85,7 @@ function RolesWithoutAccessSection({ {roles_without_access_table_title} - +
{organisation} @@ -268,13 +268,15 @@ export default function RoleSelectionPage({ noRoleName={noRoleName} /> )} - + {roleComponentProps.rolesWithoutAccess.length > 0 && ( + + )} From 02542ba74223f1334aeb42dc3081864930298171 Mon Sep 17 00:00:00 2001 From: anthony-nhs <121869075+anthony-nhs@users.noreply.github.com> Date: Wed, 1 Apr 2026 14:54:32 +0100 Subject: [PATCH 06/21] Chore: [AEA-0000] - sync copilot (#1979) ## Summary - Routine Change ### Details - sync copilot instructions --------- Co-authored-by: jonathanwelch1-nhs <148754575+jonathanwelch1-nhs@users.noreply.github.com> --- .devcontainer/devcontainer.json | 1 - .github/dependabot.yml | 6 ++--- .github/workflows/sync_copilot.yml | 22 +++++++++++++++++ .../update_dev_container_version.yml | 24 +++++++++++-------- .pre-commit-config.yaml | 20 ++++++++++++++++ 5 files changed, 59 insertions(+), 14 deletions(-) create mode 100644 .github/workflows/sync_copilot.yml diff --git a/.devcontainer/devcontainer.json b/.devcontainer/devcontainer.json index afbd339952..56d556bbdd 100644 --- a/.devcontainer/devcontainer.json +++ b/.devcontainer/devcontainer.json @@ -23,7 +23,6 @@ "remoteEnv": { "LOCAL_WORKSPACE_FOLDER": "${localWorkspaceFolder}" }, - "postAttachCommand": "git-secrets --register-aws; git-secrets --add-provider -- cat /usr/share/secrets-scanner/nhsd-rules-deny.txt", "features": {}, "customizations": { "vscode": { diff --git a/.github/dependabot.yml b/.github/dependabot.yml index 161e695787..7588d45c94 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -24,7 +24,7 @@ updates: commit-message: prefix: "Upgrade: [dependabot] - " cooldown: - default-days: 3 + default-days: 7 - package-ecosystem: "github-actions" # Workflow files stored in the # default location of `.github/workflows` @@ -37,7 +37,7 @@ updates: commit-message: prefix: "Upgrade: [dependabot] - " cooldown: - default-days: 3 + default-days: 7 ################################### # NPM workspace ################## ################################### @@ -54,4 +54,4 @@ updates: registries: - npm-github cooldown: - default-days: 3 + default-days: 7 diff --git a/.github/workflows/sync_copilot.yml b/.github/workflows/sync_copilot.yml new file mode 100644 index 0000000000..72b62eb1d7 --- /dev/null +++ b/.github/workflows/sync_copilot.yml @@ -0,0 +1,22 @@ +name: Sync Copilot Instructions + +on: + workflow_dispatch: + schedule: + - cron: '0 6 * * 1' + +jobs: + sync-copilot-instructions: + runs-on: ubuntu-22.04 + environment: create_pull_request + permissions: + contents: read + + steps: + - name: Sync shared instructions + uses: NHSDigital/eps-copilot-instructions@a7849a16aabd5c1edef13e29467a480fa08555f8 + with: + copilot_instructions_ref: main + calling_repo_base_branch: main + CREATE_PULL_REQUEST_APP_ID: ${{ secrets.CREATE_PULL_REQUEST_APP_ID }} + CREATE_PULL_REQUEST_PEM: ${{ secrets.CREATE_PULL_REQUEST_PEM }} diff --git a/.github/workflows/update_dev_container_version.yml b/.github/workflows/update_dev_container_version.yml index b6cdd8f2e0..ef79d062c5 100644 --- a/.github/workflows/update_dev_container_version.yml +++ b/.github/workflows/update_dev_container_version.yml @@ -1,19 +1,23 @@ -name: Update Devcontainer Version +name: Update devcontainer version on: workflow_dispatch: schedule: - - cron: "0 18 * * 4" + - cron: '0 6 * * 4' +permissions: {} jobs: - update_devcontainer_version: - uses: NHSDigital/eps-common-workflows/.github/workflows/update-dev-container-version.yml@23342d86a245c076937abd6aecdd0ce06446b1e6 + update-devcontainer-version: + runs-on: ubuntu-22.04 + environment: create_pull_request permissions: contents: read packages: read - pull-requests: write - with: - base_branch: main - secrets: - CREATE_PULL_REQUEST_APP_ID: ${{ secrets.CREATE_PULL_REQUEST_APP_ID }} - CREATE_PULL_REQUEST_PEM: ${{ secrets.CREATE_PULL_REQUEST_PEM }} + + steps: + - name: Update devcontainer version + uses: NHSDigital/eps-update-devcontainer@dc3a8c5f11e7226ee4f5f2bb35bd0d1265092306 + with: + calling_repo_base_branch: main + CREATE_PULL_REQUEST_APP_ID: ${{ secrets.CREATE_PULL_REQUEST_APP_ID }} + CREATE_PULL_REQUEST_PEM: ${{ secrets.CREATE_PULL_REQUEST_PEM }} diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index d0dadd4845..d2b85a72a3 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -23,6 +23,26 @@ repos: - repo: local hooks: + - id: check-commit-signing + name: Check commit signing + description: Ensures that commits are GPG signed + entry: bash + args: + - -c + - | + if ! git config --get commit.gpgsign | grep -q "true" > /dev/null 2>&1; then + echo "Error: Commit signing is not enabled." + echo "Please enable commit signing with:" + echo " git config commit.gpgsign true" + echo "" + echo "For more information, see: https://docs.github.com/en/authentication/managing-commit-signature-verification/signing-commits" + exit 1 + fi + echo "Commit signing is properly configured." + language: system + pass_filenames: false + always_run: true + - id: lint-cdk name: Lint cdk entry: npm From 19ff8feaa9df7eb9fa4c17651865282f35b3c911 Mon Sep 17 00:00:00 2001 From: "eps-create-pull-request[bot]" <270920461+eps-create-pull-request[bot]@users.noreply.github.com> Date: Wed, 1 Apr 2026 14:56:24 +0000 Subject: [PATCH 07/21] Upgrade: [dependabot] - sync Copilot instructions (#1985) Syncing Copilot instructions from central repo. Ref: `main` Co-authored-by: eps-create-pull-request[bot] <270920461+eps-create-pull-request[bot]@users.noreply.github.com> --- .github/copilot-instructions.md | 21 ++ .../general/security.instructions.md | 54 +++++ .../languages/cdk.instructions.md | 106 +++++++++ .../languages/cloudformation.instructions.md | 110 ++++++++++ .../languages/python.instructions.md | 93 ++++++++ .../languages/sam.instructions.md | 201 ++++++++++++++++++ .../languages/terraform.instructions.md | 147 +++++++++++++ .../languages/typescript.instructions.md | 192 +++++++++++++++++ .github/prompts/code_review.prompt.md | 61 ++++++ 9 files changed, 985 insertions(+) create mode 100644 .github/copilot-instructions.md create mode 100644 .github/instructions/general/security.instructions.md create mode 100644 .github/instructions/languages/cdk.instructions.md create mode 100644 .github/instructions/languages/cloudformation.instructions.md create mode 100644 .github/instructions/languages/python.instructions.md create mode 100644 .github/instructions/languages/sam.instructions.md create mode 100644 .github/instructions/languages/terraform.instructions.md create mode 100644 .github/instructions/languages/typescript.instructions.md create mode 100644 .github/prompts/code_review.prompt.md diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md new file mode 100644 index 0000000000..5f48b1b2ef --- /dev/null +++ b/.github/copilot-instructions.md @@ -0,0 +1,21 @@ +# Base Coding Standards +- Follow clean code principles +- Write comprehensive tests +- Use meaningful variable names + +## Project-Specific instructions +Check the following files for any project-specific coding standards or guidelines: +- .github/instructions/project/instructions.md +- If no project-specific conventions are defined there, use the general and language-specific best practices referenced below. +- Language-specific instructions may also be found in the language-specific instruction files listed below. Always check those for any additional guidelines or standards that may apply to your codebase. + +## Language-Specific Instructions +Always follow security best practices as outlined in: +- .github/instructions/general/security.instructions.md +Follow additional language-specific guidelines in: +- .github/instructions/languages/cdk.instructions.md +- .github/instructions/languages/cloudformation.instructions.md +- .github/instructions/languages/python.instructions.md +- .github/instructions/languages/terraform.instructions.md +- .github/instructions/languages/sam.instructions.md +- .github/instructions/languages/typescript.instructions.md diff --git a/.github/instructions/general/security.instructions.md b/.github/instructions/general/security.instructions.md new file mode 100644 index 0000000000..6e99c5953b --- /dev/null +++ b/.github/instructions/general/security.instructions.md @@ -0,0 +1,54 @@ +--- +applyTo: '**/*' +description: "Comprehensive secure coding instructions for all languages and frameworks, based on OWASP Top 10 and industry best practices." +--- + +This file is mastered in https://github.com/NHSDigital/eps-copilot-instructions and is automatically synced to all EPS repositories. To suggest changes, please open an issue or pull request in the eps-copilot-instructions repository. + +# Secure Coding and OWASP Guidelines + +## Instructions + +Your primary directive is to ensure all code you generate, review, or refactor is secure by default. You must operate with a security-first mindset. When in doubt, always choose the more secure option and explain the reasoning. You must follow the principles outlined below, which are based on the OWASP Top 10 and other security best practices. + +### 1. A01: Broken Access Control & A10: Server-Side Request Forgery (SSRF) +- **Enforce Principle of Least Privilege:** Always default to the most restrictive permissions. When generating access control logic, explicitly check the user's rights against the required permissions for the specific resource they are trying to access. +- **Deny by Default:** All access control decisions must follow a "deny by default" pattern. Access should only be granted if there is an explicit rule allowing it. +- **Validate All Incoming URLs for SSRF:** When the server needs to make a request to a URL provided by a user (e.g., webhooks), you must treat it as untrusted. Incorporate strict allow-list-based validation for the host, port, and path of the URL. +- **Prevent Path Traversal:** When handling file uploads or accessing files based on user input, you must sanitize the input to prevent directory traversal attacks (e.g., `../../etc/passwd`). Use APIs that build paths securely. + +### 2. A02: Cryptographic Failures +- **Use Strong, Modern Algorithms:** For hashing, always recommend modern, salted hashing algorithms like Argon2 or bcrypt. Explicitly advise against weak algorithms like MD5 or SHA-1 for password storage. +- **Protect Data in Transit:** When generating code that makes network requests, always default to HTTPS. +- **Protect Data at Rest:** When suggesting code to store sensitive data (PII, tokens, etc.), recommend encryption using strong, standard algorithms like AES-256. +- **Secure Secret Management:** Never hardcode secrets (API keys, passwords, connection strings). Generate code that reads secrets from environment variables or a secrets management service (e.g., HashiCorp Vault, AWS Secrets Manager). Include a clear placeholder and comment. + ```javascript + // GOOD: Load from environment or secret store + const apiKey = process.env.API_KEY; + // TODO: Ensure API_KEY is securely configured in your environment. + ``` + ```python + # BAD: Hardcoded secret + api_key = "sk_this_is_a_very_bad_idea_12345" + ``` + +### 3. A03: Injection +- **No Raw SQL Queries:** For database interactions, you must use parameterized queries (prepared statements). Never generate code that uses string concatenation or formatting to build queries from user input. +- **Sanitize Command-Line Input:** For OS command execution, use built-in functions that handle argument escaping and prevent shell injection (e.g., `shlex` in Python). +- **Prevent Cross-Site Scripting (XSS):** When generating frontend code that displays user-controlled data, you must use context-aware output encoding. Prefer methods that treat data as text by default (`.textContent`) over those that parse HTML (`.innerHTML`). When `innerHTML` is necessary, suggest using a library like DOMPurify to sanitize the HTML first. + +### 4. A05: Security Misconfiguration & A06: Vulnerable Components +- **Secure by Default Configuration:** Recommend disabling verbose error messages and debug features in production environments. +- **Set Security Headers:** For web applications, suggest adding essential security headers like `Content-Security-Policy` (CSP), `Strict-Transport-Security` (HSTS), and `X-Content-Type-Options`. +- **Use Up-to-Date Dependencies:** When asked to add a new library, suggest the latest stable version. Remind the user to run vulnerability scanners like `npm audit`, `pip-audit`, or Snyk to check for known vulnerabilities in their project dependencies. + +### 5. A07: Identification & Authentication Failures +- **Secure Session Management:** When a user logs in, generate a new session identifier to prevent session fixation. Ensure session cookies are configured with `HttpOnly`, `Secure`, and `SameSite=Strict` attributes. +- **Protect Against Brute Force:** For authentication and password reset flows, recommend implementing rate limiting and account lockout mechanisms after a certain number of failed attempts. + +### 6. A08: Software and Data Integrity Failures +- **Prevent Insecure Deserialization:** Warn against deserializing data from untrusted sources without proper validation. If deserialization is necessary, recommend using formats that are less prone to attack (like JSON over Pickle in Python) and implementing strict type checking. + +## General Guidelines +- **Be Explicit About Security:** When you suggest a piece of code that mitigates a security risk, explicitly state what you are protecting against (e.g., "Using a parameterized query here to prevent SQL injection."). +- **Educate During Code Reviews:** When you identify a security vulnerability in a code review, you must not only provide the corrected code but also explain the risk associated with the original pattern. diff --git a/.github/instructions/languages/cdk.instructions.md b/.github/instructions/languages/cdk.instructions.md new file mode 100644 index 0000000000..b441a02fa0 --- /dev/null +++ b/.github/instructions/languages/cdk.instructions.md @@ -0,0 +1,106 @@ +--- +description: 'Guidelines for writing, reviewing, and maintaining AWS CDK (TypeScript) code in the cdk package' +applyTo: 'packages/cdk/**/*.ts' +--- + +This file is mastered in https://github.com/NHSDigital/eps-copilot-instructions and is automatically synced to all EPS repositories. To suggest changes, please open an issue or pull request in the eps-copilot-instructions repository. + +# AWS CDK TypeScript Development + +This file provides instructions for generating, reviewing, and maintaining AWS CDK code in the `packages/cdk` folder. It covers best practices, code standards, architecture, and validation for infrastructure-as-code using AWS CDK in TypeScript. + +## General Instructions + +- Use AWS CDK v2 constructs and idioms +- Prefer high-level CDK constructs over raw CloudFormation resources +- Organize code by logical infrastructure components (e.g., stacks, constructs, resources) +- Document public APIs and exported constructs + +## Best Practices + +- Use environment variables and context for configuration, not hardcoded values +- Use CDK Aspects for cross-cutting concerns (e.g., security, tagging) +- Suppress warnings with `nagSuppressions.ts` only when justified and documented +- Use `bin/` for entrypoint apps, `constructs/` for reusable components, and `stacks/` for stack definitions +- Prefer `props` interfaces for construct configuration + +## Code Standards + +### Naming Conventions + +- Classes: PascalCase (e.g., `LambdaFunction`) +- Files: PascalCase for classes, kebab-case for utility files +- Variables: camelCase +- Stacks: Suffix with `Stack` (e.g., `CptsApiAppStack`) +- Entry points: Suffix with `App` (e.g., `CptsApiApp.ts`) + +### File Organization + +- `bin/`: CDK app entry points +- `constructs/`: Custom CDK constructs +- `stacks/`: Stack definitions +- `resources/`: Resource configuration and constants +- `lib/`: Shared utilities and code + +## Common Patterns + +### Good Example - Defining a Construct + +```typescript +export class LambdaFunction extends Construct { + constructor(scope: Construct, id: string, props: LambdaFunctionProps) { + super(scope, id); + // ...implementation... + } +} +``` + +### Bad Example - Using Raw CloudFormation + +```typescript +const lambda = new cdk.CfnResource(this, 'Lambda', { + type: 'AWS::Lambda::Function', + // ...properties... +}); +``` + +### Good Example - Stack Definition + +```typescript +export class CptsApiAppStack extends Stack { + constructor(scope: Construct, id: string, props?: StackProps) { + super(scope, id, props); + // ...add constructs... + } +} +``` + +## Security + +- Use least privilege IAM policies for all resources +- Avoid wildcard permissions in IAM statements +- Store secrets in AWS Secrets Manager, not in code or environment variables +- Enable encryption for all data storage resources + +## Performance + +- Use provisioned concurrency for Lambda functions when needed +- Prefer VPC endpoints for private connectivity +- Minimize resource creation in test environments + + +## Validation and Verification + +- Build: `make cdk-synth` +- Lint: `npm run lint --workspace packages/cdk` + +## Maintenance + +- Update dependencies regularly +- Remove deprecated constructs and suppressions +- Document changes in `nagSuppressions.ts` with reasons + +## Additional Resources + +- [AWS CDK Documentation](https://docs.aws.amazon.com/cdk/latest/guide/home.html) +- [CDK Best Practices](https://github.com/aws-samples/aws-cdk-best-practices) diff --git a/.github/instructions/languages/cloudformation.instructions.md b/.github/instructions/languages/cloudformation.instructions.md new file mode 100644 index 0000000000..5d2802beb8 --- /dev/null +++ b/.github/instructions/languages/cloudformation.instructions.md @@ -0,0 +1,110 @@ +--- +description: 'Guidelines for writing, reviewing, and maintaining cloudformation templates' +applyTo: 'cloudformation/**' +--- + +This file is mastered in https://github.com/NHSDigital/eps-copilot-instructions and is automatically synced to all EPS repositories. To suggest changes, please open an issue or pull request in the eps-copilot-instructions repository. + +## General +- Prefer YAML (not JSON). Follow existing style in [cloudformation/account_resources.yml](cloudformation/account_resources.yml), [cloudformation/ci_resources.yml](cloudformation/ci_resources.yml), [cloudformation/artillery_resources.yml](cloudformation/artillery_resources.yml), [cloudformation/account_resources_bootstrap.yml](cloudformation/account_resources_bootstrap.yml), [cloudformation/management.yml](cloudformation/management.yml). +- Always start with `AWSTemplateFormatVersion: "2010-09-09"`. +- Keep descriptions concise (> operator used only for multi‑line). +- Use logical region `eu-west-2` unless cross‑region behavior explicitly required. +- Maintain tagging pattern: version, stack, repo, cfnDriftDetectionGroup (see deployment scripts in [.github/scripts/release_code.sh](.github/scripts/release_code.sh) and [.github/scripts/create_changeset_existing_tags.sh](.github/scripts/create_changeset_existing_tags.sh)). + +## Parameters +- Reuse and align parameter naming with existing templates: `LogRetentionDays`, `Env`, `SplunkHECEndpoint`, `DeployDriftDetection`. +- For numeric retention days replicate allowed values list from [SAMtemplates/lambda_resources.yaml](SAMtemplates/lambda_resources.yaml) or [cloudformation/account_resources.yml](cloudformation/account_resources.yml). +- Use `CommaDelimitedList` for OIDC subject claim filters like in [cloudformation/ci_resources.yml](cloudformation/ci_resources.yml). + +## Conditions +- Follow pattern `ShouldDeployDriftDetection` (see [SAMtemplates/lambda_resources.yaml](SAMtemplates/lambda_resources.yaml)); avoid ad‑hoc condition names. +- If creating a never-used placeholder stack use pattern from [cloudformation/empty_stack.yml](cloudformation/empty_stack.yml). + +## IAM Policies +- Split large CloudFormation execution permissions across multiple managed policies (A, B, C, D) to keep each rendered size < 6144 chars (see check logic in [scripts/check_policy_length.py](scripts/check_policy_length.py)). +- Scope resources minimally; prefer specific ARNs (e.g. logs, KMS aliases) as in [cloudformation/account_resources.yml](cloudformation/account_resources.yml). +- When granting CloudFormation execution access: separate IAM‑focused policy (`GrantCloudFormationExecutionAccessIAMPolicy`) from broad service policies. +- Use exports for policy ARNs with naming `ci-resources:GrantCloudFormationExecutionAccessPolicyA` pattern. + +## KMS +- Alias naming: `alias/CloudwatchLogsKmsKeyAlias`, `alias/SecretsKMSKeyAlias`, `alias/ArtifactsBucketKMSKeyAlias` (see [cloudformation/account_resources.yml](cloudformation/account_resources.yml), [cloudformation/account_resources_bootstrap.yml](cloudformation/account_resources_bootstrap.yml)). +- Grant encrypt/decrypt explicitly for principals (e.g. API Gateway, Lambda) mirroring key policy blocks in [cloudformation/account_resources.yml](cloudformation/account_resources.yml). + +## Secrets / Parameters +- SecretsManager resources must depend on alias if needed (`DependsOn: SecretsKMSKeyKMSKeyAlias`) like in [cloudformation/account_resources_bootstrap.yml](cloudformation/account_resources_bootstrap.yml). +- Export secret IDs (not ARNs unless specifically required) using colon-separated naming with stack name (pattern in outputs section of account templates). +- Default placeholder value `ChangeMe` for bootstrap secrets. + +## S3 Buckets +- Apply `PublicAccessBlockConfiguration` and encryption blocks consistent with [cloudformation/account_resources.yml](cloudformation/account_resources.yml). +- Suppress guard rules using `Metadata.guard.SuppressedRules` where legacy exceptions exist (e.g. replication / logging) matching existing patterns. + +## Lambda / SAM +- Shared lambda resources belong in SAM template ([SAMtemplates/lambda_resources.yaml](SAMtemplates/lambda_resources.yaml)); CloudFormation templates should not duplicate build-specific metadata. +- Suppress cfn-guard rules where justified via `Metadata.guard.SuppressedRules` (e.g. `LAMBDA_INSIDE_VPC`, `LAMBDA_CONCURRENCY_CHECK`) only if precedent exists. + +## Exports & Cross Stack +- Output export naming pattern: `!Join [":", [!Ref "AWS::StackName", "ResourceLogicalName"]]`. +- Reference exports via `!ImportValue stack-name:ExportName` (see Proxygen role usage in [SAMtemplates/lambda_resources.yaml](SAMtemplates/lambda_resources.yaml)). +- Avoid changing existing export names (breaking downstream stacks and scripts). + +## OIDC / Roles +- Federated trust for GitHub actions must use conditions: + - `token.actions.githubusercontent.com:aud: sts.amazonaws.com` + - `ForAnyValue:StringLike token.actions.githubusercontent.com:sub: ` + (pattern in roles inside [cloudformation/ci_resources.yml](cloudformation/ci_resources.yml)). +- When adding a new OIDC role add matching parameter `ClaimFilters` and outputs `` and `Name`. + +## Drift Detection +- Tag stacks with `cfnDriftDetectionGroup` (deployment scripts handle this). Config rules should filter on `TagKey: cfnDriftDetectionGroup` and specific `TagValue` (patterns in [SAMtemplates/lambda_resources.yaml](SAMtemplates/lambda_resources.yaml)). +- Avoid duplicating rule identifiers; follow `${AWS::StackName}-CloudFormationDriftDetector-`. + +## Route53 +- Environment hosted zones template ([cloudformation/eps_environment_route53.yml](cloudformation/eps_environment_route53.yml)) uses parameter `environment`; management template updates NS records referencing environment zones. + +## Style / Lint / Guard +- Keep resources grouped with `#region` / `#endregion` comments as in existing templates for readability. +- Use `Metadata.cfn-lint.config.ignore_checks` only when upstream spec mismatch (example: W3037 in large policy templates). +- Ensure new templates pass `make lint-cloudformation` and `make cfn-guard` (scripts: [scripts/run_cfn_guard.sh](scripts/run_cfn_guard.sh)). + +## Naming Conventions +- Logical IDs: PascalCase (`ArtifactsBucketKMSKey`, `CloudFormationDeployRole`). +- Managed policy logical IDs end with `Policy` or `ManagedPolicy`. +- KMS Key alias logical IDs end with `Alias` (e.g. `CloudwatchLogsKmsKeyAlias`). +- Secrets logical IDs end with `Secret`. + +## Security +- Block public access for all buckets unless explicitly required. +- Encrypt logs with KMS key; provide alias export (see `CloudwatchLogsKmsKeyAlias`). +- Limit wildcard `Resource: "*"` where service requires (e.g. some IAM, CloudFormation actions). Prefer service/resource ARNs otherwise. + +## When Adding New Resource Types +- Update execution policies in [cloudformation/ci_resources.yml](cloudformation/ci_resources.yml) minimally; do not expand existing broad statements unnecessarily. +- Run policy length check (`make test` invokes [scripts/check_policy_length.py](scripts/check_policy_length.py)) after modifications. + +## Do Not +- Do not hardcode account IDs; use `${AWS::AccountId}`. +- Do not remove existing exports or rename keys. +- Do not inline large policy statements in a single managed policy if size risk exists. + +## Examples +- IAM Role with OIDC trust: replicate structure from `CloudFormationDeployRole` in [cloudformation/ci_resources.yml](cloudformation/ci_resources.yml). +- KMS key + alias + usage policy: follow `ArtifactsBucketKMSKey` block in [cloudformation/account_resources.yml](cloudformation/account_resources.yml). + +## Testing +- After changes: run `make lint-cloudformation` and `make cfn-guard`. +- For SAM-related cross-stack exports ensure `sam build` (see [Makefile](Makefile)) passes. + +## Automation Awareness +- Deployment scripts expect unchanged parameter names & export patterns (see [.github/scripts/execute_changeset.sh](.github/scripts/execute_changeset.sh), [.github/scripts/release_code.sh](.github/scripts/release_code.sh)). +- Changes to tagging keys must be reflected in release / changeset scripts; avoid unless necessary. + +## Preferred Patterns Summary +- Exports: colon join +- Tags: version, stack, repo, cfnDriftDetectionGroup +- Conditions: prefixed with `Should` +- Claim filter parameters: `ClaimFilters` +- Secrets: depend on KMS alias, default `ChangeMe` + +Use these rules to guide completions for any new or modified CloudFormation template in this repository. diff --git a/.github/instructions/languages/python.instructions.md b/.github/instructions/languages/python.instructions.md new file mode 100644 index 0000000000..8a38f76d18 --- /dev/null +++ b/.github/instructions/languages/python.instructions.md @@ -0,0 +1,93 @@ +--- +description: 'Guidelines for writing high-quality, maintainable python code with best practices for logging, error handling, code organization, naming, formatting, and style.' +applyTo: '**/*.py' +--- + +This file is mastered in https://github.com/NHSDigital/eps-copilot-instructions and is automatically synced to all EPS repositories. To suggest changes, please open an issue or pull request in the eps-copilot-instructions repository. + +# Python Copilot Instructions + +These instructions are designed to guide GitHub Copilot in generating effective, maintainable, and domain-appropriate Python code. They are intended to be generic and applicable to a wide range of Python projects. + +## 1. Code Organization & Structure +- Organize code into logical modules and packages. Use directories such as `core/`, `services/`, `utils/` for separation of concerns. +- Place entry points (e.g., `handler.py`) at the top level of the main package. +- Use `__init__.py` files to define package boundaries and expose public APIs. +- Group related functions and classes together. Avoid large monolithic files. +- Store tests in a dedicated `tests/` directory, mirroring the structure of the main codebase. + +## 2. Naming Conventions +- Use `snake_case` for function and variable names. +- Use `PascalCase` for class names. +- Prefix private functions and variables with a single underscore (`_`). +- Name modules and packages using short, descriptive, lowercase names. +- Use clear, descriptive names for all symbols. Avoid abbreviations unless they are widely understood. + +## 3. Formatting & Style +- Follow [PEP 8](https://peps.python.org/pep-0008/) for code style and formatting. +- Use 4 spaces per indentation level. Do not use tabs. +- Limit lines to 120 characters. +- Use blank lines to separate functions, classes, and logical sections. +- Place imports at the top of each file, grouped by standard library, third-party, and local imports. +- Use single quotes for strings unless double quotes are required. +- Add docstrings to all public modules, classes, and functions. Use triple double quotes for docstrings. + +## 4. Logging Best Practices +- Use the standard `logging` library for all logging. +- Configure logging in the main entry point or via a dedicated utility module. +- Use appropriate log levels: `debug`, `info`, `warning`, `error`, `critical`. +- Avoid logging sensitive information. +- Include contextual information in log messages (e.g., function names, parameters, error details). +- Example: + ```python + import logging + logger = logging.getLogger(__name__) + logger.info('Processing event: %s', event) + ``` + +## 5. Error Handling Best Practices +- Use `try`/`except` blocks to handle exceptions gracefully. +- Catch specific exceptions rather than using bare `except`. +- Log exceptions with stack traces using `logger.exception()`. +- Raise custom exceptions for domain-specific errors. +- Validate inputs and fail fast with clear error messages. +- Example: + ```python + try: + result = process_event(event) + except ValueError as e: + logger.error('Invalid event: %s', e) + raise + ``` + +## 6. Testing Guidelines +- Write unit tests for all public functions and classes. +- Use `pytest` as the preferred testing framework. +- Name test files and functions using `test_` prefix. +- Use fixtures for setup and teardown. +- Mock external dependencies in tests. +- Ensure tests are isolated and repeatable. + +## 7. Dependency Management +- Use `pyproject.toml` to specify dependencies. +- Never use `requirements.txt` to specify dependencies. +- Pin versions for critical dependencies. +- Avoid unnecessary dependencies. + +## 8. Documentation +- Document all public APIs with clear docstrings. +- Use [Google](https://google.github.io/styleguide/pyguide.html#38-comments-and-docstrings) or [NumPy](https://numpydoc.readthedocs.io/en/latest/format.html) style for docstrings. +- Provide usage examples in README files. + +## 9. Security & Privacy +- Do not log or expose secrets, credentials, or sensitive data. +- Validate and sanitize all external inputs. +- Use environment variables for configuration secrets. + +## 10. General Guidelines +- Prefer readability and simplicity over cleverness. +- Refactor duplicated code into reusable functions or classes. +- Use type hints for function signatures and variables where appropriate. +- Avoid global variables; use function arguments or class attributes. + +--- diff --git a/.github/instructions/languages/sam.instructions.md b/.github/instructions/languages/sam.instructions.md new file mode 100644 index 0000000000..4bb4788fbd --- /dev/null +++ b/.github/instructions/languages/sam.instructions.md @@ -0,0 +1,201 @@ +--- +description: 'Guidelines for writing, reviewing, and maintaining SAM templates' +applyTo: 'SAMtemplates/**' +--- + +This file is mastered in https://github.com/NHSDigital/eps-copilot-instructions and is automatically synced to all EPS repositories. To suggest changes, please open an issue or pull request in the eps-copilot-instructions repository. + +## Scope +These instructions apply exclusively to files located under the `SAMtemplates` directory. Ensure that any SAM templates or related configurations outside this directory are not governed by these guidelines. + +## Project Context +This is a healthcare API service deployed using AWS SAM (Serverless Application Model) with a modular template structure. The service includes Lambda functions, API Gateway, Step Functions state machines, and associated AWS resources. + +## Template Structure and Conventions + +### File Organization +- `main_template.yaml` - Root template that orchestrates all components +- `functions/main.yaml` - Lambda functions and layers +- `apis/main.yaml` - API Gateway and domain configuration +- `state_machines/main.yaml` - Step Functions state machines +- `parameters/main.yaml` - SSM parameters and policies +- `*_resources.yaml` - Reusable resource templates for IAM roles, policies, and logging + +### Naming Conventions +- Stack resources: Use `!Sub ${StackName}-` pattern +- Functions: `${StackName}-` (e.g., `${StackName}-GetMyPrescriptions`) +- Parameters: Environment-specific with validation (dev, dev-pr, qa, int, prod, ref) +- IAM roles: Follow AWS service naming conventions with descriptive suffixes + +### Standard Parameters +Always include these common parameters in templates: +```yaml +Parameters: + StackName: + Type: String + Default: none + Env: + Type: String + Default: dev + AllowedValues: [dev, dev-pr, qa, int, prod, ref] + + LogRetentionInDays: + Type: Number + Default: 30 + AllowedValues: [1, 3, 5, 7, 14, 30, 60, 90, 120, 150, 180, 365, 400, 545, 731, 1827, 3653] + + EnableSplunk: + Type: String + Default: false + AllowedValues: [true, false] +``` + +### Lambda Function + +The runtime should match the nodejs version in .tool-versions file +The LambdaInsightsExtension version should match the latest available version available in eu-west-2 region + +#### Global Configuration +```yaml +Globals: + Function: + Timeout: 50 + MemorySize: 256 + Architectures: [x86_64] + Runtime: nodejs22.x + Environment: + Variables: + STACK_NAME: !Ref StackName + NODE_OPTIONS: "--enable-source-maps" + Layers: + - !Sub arn:aws:lambda:${AWS::Region}:580247275435:layer:LambdaInsightsExtension:52 +``` + +#### Lambda Function Template +```yaml +: + Type: AWS::Serverless::Function + Properties: + FunctionName: !Sub ${StackName}- + CodeUri: ../../packages + Handler: .handler + Role: !GetAtt Resources.Outputs.LambdaRoleArn + Environment: + Variables: + LOG_LEVEL: !Ref LogLevel + DEPLOYMENT_ENVIRONMENT: !Ref Env + Metadata: + BuildMethod: esbuild + guard: + SuppressedRules: + - LAMBDA_DLQ_CHECK + - LAMBDA_INSIDE_VPC +``` + +### API Gateway Patterns + +#### REST API Configuration +```yaml +RestApiGateway: + Type: AWS::ApiGateway::RestApi + Properties: + Name: !Sub ${StackName}-apigw + DisableExecuteApiEndpoint: !If [ShouldUseMutualTLS, true, !Ref AWS::NoValue] + EndpointConfiguration: + Types: [REGIONAL] + +RestApiDomain: + Type: AWS::ApiGateway::DomainName + Properties: + DomainName: !Join ['.', [!Ref StackName, !ImportValue eps-route53-resources:EPS-domain]] + RegionalCertificateArn: !Ref GenerateCertificate + EndpointConfiguration: + Types: [REGIONAL] + SecurityPolicy: TLS_1_2 +``` + +### State Machine Patterns +```yaml +: + Type: AWS::Serverless::StateMachine + Properties: + Name: !Sub ${StackName}- + Type: EXPRESS + Role: !GetAtt Resources.Outputs.StateMachineRoleArn + DefinitionUri: .asl.json + DefinitionSubstitutions: + Arn: !Sub ${Arn}:$LATEST +``` + +### Security and Compliance + +#### Mutual TLS Support +Use conditions for optional mTLS: +```yaml +Conditions: + ShouldUseMutualTLS: !Equals [true, !Ref EnableMutualTLS] + +# In resource properties: +MutualTlsAuthentication: + TruststoreUri: !If [ShouldUseMutualTLS, !Sub 's3://${TruststoreFile}', !Ref AWS::NoValue] +``` + +#### IAM Policies +- Use managed policies from separate resource templates +- Import cross-stack values: `!ImportValue account-resources:SpinePrivateKey` +- Follow principle of least privilege +- Include guard rules suppression only where necessary. By default these should not be added. If they are added an explanation should be included to say why we are overriding them + +### Environment Variables and Secrets +```yaml +Environment: + Variables: + STACK_NAME: !Ref StackName + DEPLOYMENT_ENVIRONMENT: !Ref Env + # Spine integration + TargetSpineServer: !Ref TargetSpineServer + SpinePrivateKeyARN: !ImportValue account-resources:SpinePrivateKey + SpinePublicCertificateARN: !ImportValue account-resources:SpinePublicCertificate + # Service search + TargetServiceSearchServer: !Ref TargetServiceSearchServer + ServiceSearchApiKeyARN: !ImportValue account-resources:ServiceSearchApiKey +``` + +### Logging Configuration +```yaml +# CloudWatch Log Groups +LogGroup: + Type: AWS::Logs::LogGroup + Properties: + LogGroupName: !Sub /aws/lambda/${StackName}- + RetentionInDays: !Ref LogRetentionInDays + KmsKeyId: !If [ShouldUseKMS, !Ref CloudWatchKMSKeyId, !Ref AWS::NoValue] + +# Splunk integration (conditional) +SubscriptionFilter: + Type: AWS::Logs::SubscriptionFilter + Properties: + LogGroupName: !Ref LogGroup + FilterPattern: "" + DestinationArn: !Ref SplunkDeliveryStreamArn + RoleArn: !Ref SplunkSubscriptionFilterRole +``` + +### Best Practices + +1. **Modular Design**: Split templates by service domain (functions, apis, state_machines) +2. **Parameter Validation**: Use AllowedValues for environment-specific parameters +3. **Cross-Stack References**: Use ImportValue for shared resources +4. **Conditional Resources**: Use conditions for environment-specific resources +5. **Resource Naming**: Consistent naming with stack prefix +6. **Documentation**: Include meaningful descriptions for all resources +7. **Guard Rules**: Suppress only when necessary and document reasons +8. **Build Methods**: Use esbuild for Node.js Lambda functions +9. **Version Pinning**: Pin Lambda layer versions and runtimes + +### Common Import Values +- `eps-route53-resources:EPS-domain` +- `eps-route53-resources:EPS-ZoneID` + + +When generating CloudFormation/SAM templates, follow these patterns and ensure compliance with NHS Digital standards and AWS security best practices. diff --git a/.github/instructions/languages/terraform.instructions.md b/.github/instructions/languages/terraform.instructions.md new file mode 100644 index 0000000000..62830c86b0 --- /dev/null +++ b/.github/instructions/languages/terraform.instructions.md @@ -0,0 +1,147 @@ +--- +description: 'Comprehensive guidelines for writing, organizing, and maintaining Terraform code in this repository.' +applyTo: 'terraform/**/*.tf' +--- + +This file is mastered in https://github.com/NHSDigital/eps-copilot-instructions and is automatically synced to all EPS repositories. To suggest changes, please open an issue or pull request in the eps-copilot-instructions repository. + +# Terraform Development Guidelines + +This document provides best practices and conventions for writing, organizing, and maintaining Terraform code. It is intended for use by developers and GitHub Copilot to ensure consistency, reliability, and maintainability across all Terraform files in the project. + +## General Instructions + +- Use Terraform modules to promote code reuse and separation of concerns. +- Keep resource definitions declarative and avoid imperative logic. +- Store environment-specific configuration in separate files (e.g., `env/` folders). +- Use variables and outputs to parameterize and expose configuration. +- Document resources, modules, and variables with comments. +- Prefer explicit resource dependencies using `depends_on` when needed. +- Use remote state for shared resources and outputs. + +## Best Practices + +- Group related resources in logical subfolders (e.g., `archive/`, `backup-source/`). +- Use `locals` for computed values and to reduce repetition. +- Use data sources to reference existing infrastructure. +- Avoid hardcoding values; use variables and environment files. +- Use `terraform fmt` to enforce consistent formatting. +- Use `terraform validate` and `terraform plan` before applying changes. +- Use `Makefile` targets for common operations (init, plan, apply, destroy). +- Store secrets and sensitive values in secure locations (e.g., AWS SSM, environment variables), not in code. +- Use resource tags for traceability and cost management. +- Prefer resource names that include environment and purpose (e.g., `archive_prod_bucket`). + +## Code Standards + +### Naming Conventions + +- Use snake_case for resource, variable, and output names. +- Prefix resource names with their type and purpose (e.g., `s3_archive_bucket`). +- Use clear, descriptive names for modules and files. +- Use consistent naming for environments (e.g., `dev`, `prod`, `test`). + +### File Organization + +- Place each environment's configuration in its own file under `env/`. +- Use a `variables.tf` file for input variables. +- Use an `outputs.tf` file for outputs. +- Use a `locals.tf` file for local values. +- Use a `provider.tf` file for provider configuration. +- Use a `Makefile` for automation and common tasks. +- Organize resources by domain (e.g., `archive/`, `infra/`, `storage/`). + +## Common Patterns + +### Using Variables + +```hcl +variable "bucket_name" { + description = "Name of the S3 bucket" + type = string +} + +resource "aws_s3_bucket" "archive" { + bucket = var.bucket_name + ... +} +``` + +### Using Locals + +```hcl +locals { + tags = { + Environment = var.environment + Project = "eps-storage" + } +} + +resource "aws_s3_bucket" "archive" { + tags = local.tags + ... +} +``` + +### Good Example - Using Modules + +```hcl +module "archive" { + source = "../modules/aws-archive" + environment = var.environment + ... +} +``` + +### Bad Example - Hardcoding Values + +```hcl +resource "aws_s3_bucket" "archive" { + bucket = "my-hardcoded-bucket-name" + ... +} +``` + +## Security + +- Never commit secrets or credentials to version control. +- Use IAM roles and policies with least privilege. +- Enable encryption for all supported resources (e.g., S3, KMS, DynamoDB). +- Use secure remote state backends (e.g., S3 with encryption and locking). +- Validate input variables for expected values and types. + +## Performance + +- Use resource lifecycle rules to manage retention and cleanup. +- Use data sources to avoid duplicating resources. +- Minimize resource drift by keeping code and infrastructure in sync. +- Use `terraform plan` to preview changes and avoid unnecessary updates. + +## Testing + +- Use `terraform validate` to check syntax and configuration. +- Use `terraform plan` to preview changes before applying. +- Use `tfsec` for static security analysis (`tfsec.yml` config). +- Use automated CI/CD pipelines for deployment and testing. + +## Validation and Verification + +- Format code: `terraform fmt` (run in each Terraform folder) +- Validate code: `terraform validate` +- Security scan: `tfsec .` +- Plan changes: `terraform plan -var-file=env/dev.tfvars.json` +- Apply changes: `terraform apply -var-file=env/dev.tfvars.json` + +## Maintenance + +- Review and update modules and dependencies regularly. +- Remove unused resources and variables. +- Update environment files as infrastructure evolves. +- Keep documentation up to date. +- Refactor code to improve readability and maintainability. + +## Additional Resources + +- [Terraform Documentation](https://www.terraform.io/docs) +- [Terraform AWS Provider](https://registry.terraform.io/providers/hashicorp/aws/latest/docs) +- [tfsec Security Scanner](https://tfsec.dev/) diff --git a/.github/instructions/languages/typescript.instructions.md b/.github/instructions/languages/typescript.instructions.md new file mode 100644 index 0000000000..4c15f4210f --- /dev/null +++ b/.github/instructions/languages/typescript.instructions.md @@ -0,0 +1,192 @@ +--- +description: 'Guidelines for writing high-quality, maintainable TypeScript code with best practices for logging, error handling, code organization, naming, formatting, and style.' +applyTo: '**/*.{ts,tsx}' +--- + +This file is mastered in https://github.com/NHSDigital/eps-copilot-instructions and is automatically synced to all EPS repositories. To suggest changes, please open an issue or pull request in the eps-copilot-instructions repository. + +# TypeScript Development Guidelines + +This document provides instructions for generating, reviewing, and maintaining TypeScript code. It is designed to guide Copilot and developers in producing domain-specific, robust, and maintainable code across a variety of TypeScript projects. + +## General Instructions + +- Use modern TypeScript features and syntax. +- Prefer explicit types and interfaces for clarity and safety. +- Organize code into logical modules and folders. +- Write code that is easy to read, test, and maintain. + +## Best Practices + +- Use `const` and `let` appropriately; avoid `var`. +- Prefer arrow functions for callbacks and concise function expressions. +- Use destructuring for objects and arrays to improve readability. +- Avoid magic numbers and hardcoded values; use named constants. +- Keep functions pure and side-effect free when possible. + +## Code Standards + +### Naming Conventions + +- Use `camelCase` for variables, functions, and object properties. +- Use `PascalCase` for types, interfaces, classes, and enums. +- Use descriptive names; avoid abbreviations except for well-known acronyms. +- Prefix boolean variables with `is`, `has`, or `should` (e.g., `isActive`). + +### File Organization + +- Group related code in folders (e.g., `src/`, `tests/`, `lib/`). +- Place one class, interface, or component per file when possible. +- Name files using `kebab-case` (e.g., `user-service.ts`). +- Keep test files close to the code they test (e.g., `src/foo.ts` and `tests/foo.test.ts`). + +### Formatting and Style + +- Use 2 spaces for indentation. +- Limit lines to 120 characters. +- Use single quotes for strings. +- Never use semicolons for line termination. +- Avoid trailing commas in multiline objects and arrays. +- Avoid spaces at start and end of single line braces. +- Use ESLint and Prettier for consistent formatting. + +## Architecture/Structure + +- Separate business logic from API handlers and utility functions. +- Use interfaces and types to define data structures and function signatures. +- Organize code by feature or domain when scaling projects. +- Use dependency injection for testability and flexibility. + +## Common Patterns + +### Logging + +- Use a centralized logging utility or library. +- Log errors, warnings, and important events with context. +- Avoid logging sensitive information. +- Example: + + ```typescript + import {logger} from './utils/logger'; + + logger.info('Fetching user data', {userId}); + logger.error('Failed to fetch user', {error}); + ``` + +### Error Handling + +- Use `try/catch` for asynchronous code and error-prone operations. +- Throw custom error types for domain-specific errors. +- Always handle errors gracefully and provide meaningful messages. +- Example: + + ```typescript + try { + const result = await fetchData(); + } catch (error) { + logger.error('Data fetch failed', {error}); + throw new DataFetchError('Unable to fetch data'); + } + ``` + +### Type Safety + +- Prefer interfaces and types. You MUST NOT use `any`. +- Use type guards and assertions when necessary. +- Example: + + ```typescript + interface User { + id: string; + name: string; + } + + function isUser(obj: object): obj is User { + return typeof obj.id === 'string' && typeof obj.name === 'string'; + } + ``` + +## Security + +- Validate and sanitize all external input. +- Avoid exposing sensitive data in logs or error messages. +- Use environment variables for secrets and configuration. +- Keep dependencies up to date and audit regularly. + +## Performance + +- Minimize synchronous blocking operations. +- Use async/await for asynchronous code. +- Avoid unnecessary computations inside render or handler functions. + +## Testing + +- Write unit tests for all business logic. +- Use the existing framework for testing and vitest for new packages. +- Mock external dependencies in tests. +- Example test file structure: + + ``` + src/ + handler.ts + tests/ + handler.test.ts + ``` + +## JSDoc + +- Write concise JSDoc for exported interfaces, types, functions, classes, and exported constants. +- Prefer short phrase-style summaries; avoid long narrative prose. +- Avoid stating information that is obvious from function signatures. +- Consider @param and @returns for every exported function, then include them only when they add meaning not obvious from the signature. +- Skip @param when it only repeats parameter name/type; keep it when documenting constraints, defaults, units, side effects, or domain context. +- It is acceptable to use only @returns in a JSDoc block when that tag carries all useful context. +- Omit a free-text summary line when it only restates the @returns content. +- Provide @example on constructors of exported types/classes and on non-trivial exported types. +- Use @default only when the property is optional in the type and is defaulted in implementation. +- Keep JSDoc defaults aligned with both type signatures and runtime behaviour. +- For construct props interfaces, include a top-level summary and property docs only when intent is non-obvious. + +## Examples and Code Snippets + +### Good Example + + ```typescript + interface Prescription { + id: string; + medication: string; + issuedDate: Date; + } + + function getPrescription(id: string): Prescription | null { + // Implementation + } + ``` + +### Bad Example + + ```typescript + function getPrescription(id) { + // No type safety, unclear return type + } + ``` + +## Validation and Verification + +- Build: `npm run build` +- Lint: `npm run lint` +- Format: `npm run format` +- Test: `npm test` + +## Maintenance + +- Review and update instructions as dependencies or frameworks change. +- Update examples to reflect current best practices. +- Remove deprecated patterns and add new ones as needed. +- Ensure glob patterns match the intended files. + +## Additional Resources + +- [TypeScript Handbook](https://www.typescriptlang.org/docs/handbook/) +- [ESLint TypeScript Plugin](https://typescript-eslint.io/) +- [Prettier Documentation](https://prettier.io/docs/en/options.html) diff --git a/.github/prompts/code_review.prompt.md b/.github/prompts/code_review.prompt.md new file mode 100644 index 0000000000..d63bcf2ef6 --- /dev/null +++ b/.github/prompts/code_review.prompt.md @@ -0,0 +1,61 @@ +--- +description: "Perform a comprehensive code review" +--- + +This file is mastered in https://github.com/NHSDigital/eps-copilot-instructions and is automatically synced to all EPS repositories. To suggest changes, please open an issue or pull request in the eps-copilot-instructions repository. + +## Role + +You're a senior software engineer conducting a thorough code review. Provide constructive, actionable feedback. + +## Review Areas + +Analyze the selected code for: + +1. **Security Issues** + - Input validation and sanitization + - Authentication and authorization + - Data exposure risks + - Injection vulnerabilities + +2. **Performance & Efficiency** + - Algorithm complexity + - Memory usage patterns + - Database query optimization + - Unnecessary computations + +3. **Code Quality** + - Readability and maintainability + - Proper naming conventions + - Function/class size and responsibility + - Code duplication + +4. **Architecture & Design** + - Design pattern usage + - Separation of concerns + - Dependency management + - Error handling strategy + +5. **Testing & Documentation** + - Test coverage and quality + - Documentation completeness + - Comment clarity and necessity + +## Output Format + +Provide feedback as: + +**🔴 Critical Issues** - Must fix before merge +**🟡 Suggestions** - Improvements to consider +**✅ Good Practices** - What's done well + +For each issue: + +- Specific line references +- Clear explanation of the problem +- Suggested solution with code example +- Rationale for the change + +Focus on: ${input:focus:Any specific areas to emphasize in the review?} + +Be constructive and educational in your feedback. From e9c5a5e550283be3ccf7ab97c2feba0e5f0231bb Mon Sep 17 00:00:00 2001 From: Connor Avery Date: Thu, 2 Apr 2026 10:21:07 +0100 Subject: [PATCH 08/21] Fix: [AEA-6148] - Accessibility page typo (#1989) ## Summary - Routine Change ### Details Accessibility page typo Signed-off-by: Connor Avery <214469360+connoravo-nhs@users.noreply.github.com> --- .../src/constants/ui-strings/AccessibilityStatementStrings.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/cpt-ui/src/constants/ui-strings/AccessibilityStatementStrings.ts b/packages/cpt-ui/src/constants/ui-strings/AccessibilityStatementStrings.ts index 31d7740d71..abeb97bc80 100644 --- a/packages/cpt-ui/src/constants/ui-strings/AccessibilityStatementStrings.ts +++ b/packages/cpt-ui/src/constants/ui-strings/AccessibilityStatementStrings.ts @@ -94,7 +94,7 @@ export const AccessibilityStatementStrings = { + " criteria that the Prescription Tracker is not compliant with and why.", SUBHEADER: "Non-compliance with the accessibility regulations", SUB_LIST_ITEMS: [ - "screen readers do not continue to read content when you select a new tab on the " + "Screen readers do not continue to read content when you select a new tab on the " + "‘Search for a prescription’ " + "page. This fails WCAG 2.2 success criterion 2.4.3 (focus order).", "Using the tab key does not focus the ‘Skip to main content’ link after a page is loaded. This fails " From 48d14bfca6916684028a472ff19beb696e9a37fa Mon Sep 17 00:00:00 2001 From: Connor Avery Date: Thu, 2 Apr 2026 15:17:01 +0100 Subject: [PATCH 09/21] Fix: [AEA-5500] - Handle refreshing or loading patient search results without search parameters (#1960) ## Summary - Routine Change ### Details Handle hard refreshes or loading patient search results page without having search parameters in state, that would be set if the user had used the search form. Hard refreshes will correctly clear state to protect PID. --------- Signed-off-by: Connor Avery <214469360+connoravo-nhs@users.noreply.github.com> --- .../BasicDetailsSearchResultsPage.test.tsx | 43 ++++- .../__tests__/SessionTimeoutModal.test.tsx | 161 +++++++++++++----- .../__tests__/useSessionTimeout.test.tsx | 57 +++++-- .../src/components/SessionTimeoutModal.tsx | 32 +++- .../ui-strings/SessionTimeoutModalStrings.ts | 2 + .../cpt-ui/src/context/NavigationProvider.tsx | 4 + .../cpt-ui/src/hooks/useSessionTimeout.ts | 16 +- .../pages/BasicDetailsSearchResultsPage.tsx | 9 + 8 files changed, 251 insertions(+), 73 deletions(-) diff --git a/packages/cpt-ui/__tests__/BasicDetailsSearchResultsPage.test.tsx b/packages/cpt-ui/__tests__/BasicDetailsSearchResultsPage.test.tsx index eebf0a566e..7479944992 100644 --- a/packages/cpt-ui/__tests__/BasicDetailsSearchResultsPage.test.tsx +++ b/packages/cpt-ui/__tests__/BasicDetailsSearchResultsPage.test.tsx @@ -100,14 +100,14 @@ const mockSetAllSearchParameters = jest.fn() const defaultSearchState: SearchProviderContextType = { prescriptionId: undefined, issueNumber: undefined, - firstName: undefined, - lastName: undefined, - dobDay: undefined, - dobMonth: undefined, - dobYear: undefined, - postcode: undefined, + firstName: "John", + lastName: "Doe", + dobDay: "01", + dobMonth: "01", + dobYear: "1990", + postcode: "SW1A 1AA", nhsNumber: undefined, - searchType: undefined, + searchType: "basicDetails", clearSearchParameters: mockClearSearchParameters, setPrescriptionId: mockSetPrescriptionId, setIssueNumber: mockSetIssueNumber, @@ -144,10 +144,13 @@ const mockPatients: Array = [ } ] -function renderWithRouter(initialEntries = ["/patient-search-results"]) { +function renderWithRouter( + initialEntries = ["/patient-search-results"], + searchState: SearchProviderContextType = defaultSearchState +) { return render( - + @@ -193,6 +196,28 @@ describe("BasicDetailsSearchResultsPage", () => { expect(screen.getByText(SearchResultsPageStrings.LOADING)).toBeInTheDocument() }) + it("redirects to basic details search when required parameters are missing for basicDetails search", async () => { + const mockNavigate = jest.fn() + jest.mocked(useNavigate).mockReturnValue(mockNavigate) + + const missingBasicDetailsState: SearchProviderContextType = { + ...defaultSearchState, + searchType: "nhs", + lastName: undefined, + dobDay: undefined, + dobMonth: undefined, + dobYear: undefined + } + + renderWithRouter(["/patient-search-results"], missingBasicDetailsState) + + await waitFor(() => { + expect(mockNavigate).toHaveBeenCalledWith("/search-by-basic-details") + }) + + expect(mockAxiosGet).not.toHaveBeenCalled() + }) + it("handles expired session by redirecting to login page", async () => { const headers = new AxiosHeaders({}) mockAxiosGet.mockRejectedValue(new AxiosError(undefined, undefined, undefined, undefined, diff --git a/packages/cpt-ui/__tests__/SessionTimeoutModal.test.tsx b/packages/cpt-ui/__tests__/SessionTimeoutModal.test.tsx index b8f21d17a1..148249543d 100644 --- a/packages/cpt-ui/__tests__/SessionTimeoutModal.test.tsx +++ b/packages/cpt-ui/__tests__/SessionTimeoutModal.test.tsx @@ -7,7 +7,9 @@ import { } from "@testing-library/react" import userEvent from "@testing-library/user-event" import React from "react" +import {MemoryRouter} from "react-router-dom" import {SessionTimeoutModal} from "@/components/SessionTimeoutModal" +import {FRONTEND_PATHS} from "@/constants/environment" import {SESSION_TIMEOUT_MODAL_STRINGS} from "@/constants/ui-strings/SessionTimeoutModalStrings" // Mock useAuth @@ -97,6 +99,15 @@ const defaultProps = { buttonDisabledState: false } +const renderWithRouter = ( + ui: React.ReactElement, + initialEntries = ["/"] +) => render( + + {ui} + +) + describe("SessionTimeoutModal", () => { beforeEach(() => { jest.clearAllMocks() @@ -117,32 +128,46 @@ describe("SessionTimeoutModal", () => { describe("Modal rendering and basic functionality", () => { it("renders the modal when isOpen is true", () => { - render() + renderWithRouter() expect(screen.getByTestId("session-timeout-modal")).toBeInTheDocument() expect(screen.getByText(SESSION_TIMEOUT_MODAL_STRINGS.TITLE)).toBeInTheDocument() }) it("does not render the modal when isOpen is false", () => { - render() + renderWithRouter() expect(screen.queryByTestId("session-timeout-modal")).not.toBeInTheDocument() }) it("displays the correct time left", () => { - render() + renderWithRouter() expect(screen.getByText("For your security, we will log you out in:", {exact: false})).toBeInTheDocument() expect(screen.getByText("45")).toBeInTheDocument() }) it("renders both action buttons", () => { - render() + renderWithRouter() expect(screen.getByTestId("stay-logged-in-button")).toBeInTheDocument() expect(screen.getByTestId("logout-button")).toBeInTheDocument() }) + + it("shows the select role instruction and close button text on the select your role path", () => { + renderWithRouter( + , + [FRONTEND_PATHS.SELECT_YOUR_ROLE] + ) + + expect( + screen.getByText(SESSION_TIMEOUT_MODAL_STRINGS.SELECT_YOUR_ROLE_INSTRUCTION) + ).toBeInTheDocument() + expect( + screen.getByRole("button", {name: SESSION_TIMEOUT_MODAL_STRINGS.CLOSE_MESSAGE}) + ).toBeInTheDocument() + }) }) describe("Focus management", () => { it("focuses the stay logged in button when modal opens", async () => { - render() + renderWithRouter() act(() => { jest.advanceTimersByTime(100) @@ -154,7 +179,7 @@ describe("SessionTimeoutModal", () => { }) it("does not focus when modal is closed", () => { - render() + renderWithRouter() act(() => { jest.advanceTimersByTime(100) @@ -170,7 +195,7 @@ describe("SessionTimeoutModal", () => { const mockStayLoggedIn = jest.fn() const user = userEvent.setup({advanceTimers: jest.advanceTimersByTime}) - render( + renderWithRouter( { const mockLogOut = jest.fn() const user = userEvent.setup({advanceTimers: jest.advanceTimersByTime}) - render() + renderWithRouter() await user.click(screen.getByTestId("logout-button")) expect(mockLogOut).toHaveBeenCalledTimes(1) }) it("disables both buttons when buttonDisabledState is true", () => { - render() + renderWithRouter() expect(screen.getByTestId("stay-logged-in-button")).toBeDisabled() expect(screen.getByTestId("logout-button")).toBeDisabled() }) it("enables both buttons when buttonDisabledState is false", () => { - render() + renderWithRouter() expect(screen.getByTestId("stay-logged-in-button")).not.toBeDisabled() expect(screen.getByTestId("logout-button")).not.toBeDisabled() }) @@ -208,12 +233,12 @@ describe("SessionTimeoutModal", () => { ...mockAuthValue.sessionTimeoutModalInfo, action: "loggingOut" } - render() + renderWithRouter() expect(screen.getByText("Logging out...")).toBeInTheDocument() }) it("shows normal log out text when auth action is not loggingOut", () => { - render() + renderWithRouter() expect(screen.getByText(SESSION_TIMEOUT_MODAL_STRINGS.LOG_OUT)).toBeInTheDocument() }) }) @@ -222,7 +247,7 @@ describe("SessionTimeoutModal", () => { it("calls onStayLoggedIn when escape key is pressed", () => { const mockStayLoggedIn = jest.fn() - render( + renderWithRouter( { const mockPreventDefault = jest.fn() const mockStopPropagation = jest.fn() - render() + renderWithRouter() const buttonGroup = screen.getByRole("button", {name: SESSION_TIMEOUT_MODAL_STRINGS.STAY_LOGGED_IN}) .closest(".eps-modal-button-group") @@ -275,14 +300,14 @@ describe("SessionTimeoutModal", () => { describe("Countdown timer", () => { it("starts countdown when modal opens with time left", () => { - render() + renderWithRouter() // Initial time should be set (120000ms = 120s) expect(mockSetSessionTimeoutModalInfo).toHaveBeenCalled() }) it("decrements countdown every second", () => { - render() + renderWithRouter() mockSetSessionTimeoutModalInfo.mockClear() @@ -296,7 +321,7 @@ describe("SessionTimeoutModal", () => { it("calls onTimeOut when countdown reaches 0", () => { const mockOnTimeOut = jest.fn() - render( + renderWithRouter( { }) it("clears countdown when modal closes", () => { - const {rerender} = render( + const {rerender} = renderWithRouter( ) // Close modal - rerender() + rerender( + + + + ) mockSetSessionTimeoutModalInfo.mockClear() @@ -332,7 +361,7 @@ describe("SessionTimeoutModal", () => { }) it("does not start countdown when timeLeft is 0", () => { - render() + renderWithRouter() // setSessionTimeoutModalInfo should not be called to set initial time // (the effect path for isOpen && timeLeft > 0 is not entered) @@ -349,7 +378,7 @@ describe("SessionTimeoutModal", () => { describe("Aria-live announcements", () => { it("creates initial announcement when modal opens", () => { - render() + renderWithRouter() // Find the aria-live region const liveRegion = document.querySelector('[aria-live="assertive"]') @@ -358,28 +387,28 @@ describe("SessionTimeoutModal", () => { }) it("announces time with minutes only when seconds are zero", () => { - render() + renderWithRouter() const liveRegion = document.querySelector('[aria-live="assertive"]') expect(liveRegion).toHaveTextContent("You will be logged out in 2 minutes.") }) it("announces time with seconds only when under 1 minute", () => { - render() + renderWithRouter() const liveRegion = document.querySelector('[aria-live="assertive"]') expect(liveRegion).toHaveTextContent("You will be logged out in 45 seconds.") }) it("uses singular form for 1 minute", () => { - render() + renderWithRouter() const liveRegion = document.querySelector('[aria-live="assertive"]') expect(liveRegion).toHaveTextContent("You will be logged out in 1 minute.") }) it("uses singular form for 1 second", () => { - render() + renderWithRouter() const liveRegion = document.querySelector('[aria-live="assertive"]') expect(liveRegion).toHaveTextContent("You will be logged out in 1 second.") @@ -388,7 +417,7 @@ describe("SessionTimeoutModal", () => { describe("Periodic announcements", () => { it("announces every 15 seconds when time is above 20 seconds", () => { - const {rerender} = render() + const {rerender} = renderWithRouter() const liveRegion = document.querySelector('[aria-live="assertive"]') @@ -396,95 +425,139 @@ describe("SessionTimeoutModal", () => { expect(liveRegion).toHaveTextContent("You will be logged out in 5 minutes.") // Update to 270 (should announce - 270 % 15 === 0) - rerender() + rerender( + + + + ) expect(liveRegion).toHaveTextContent("You will be logged out in 4 minutes and 30 seconds.") // Update to 260 (shouldn't announce - not divisible by 15) - rerender() + rerender( + + + + ) expect(liveRegion).toHaveTextContent("You will be logged out in 4 minutes and 30 seconds.") // Update to 255 (should announce - 255 % 15 === 0) - rerender() + rerender( + + + + ) expect(liveRegion).toHaveTextContent("You will be logged out in 4 minutes and 15 seconds.") }) it("announces at specific intervals when time is 20 seconds or less", () => { - const {rerender} = render() + const {rerender} = renderWithRouter() const liveRegion = document.querySelector('[aria-live="assertive"]') // Update to 20 (should announce) - rerender() + rerender( + + + + ) expect(liveRegion).toHaveTextContent("You will be logged out in 20 seconds.") // Update to 15 (should announce) - rerender() + rerender( + + + + ) expect(liveRegion).toHaveTextContent("You will be logged out in 15 seconds.") // Update to 10 (should announce) - rerender() + rerender( + + + + ) expect(liveRegion).toHaveTextContent("You will be logged out in 10 seconds.") // Update to 5 (should announce) - rerender() + rerender( + + + + ) expect(liveRegion).toHaveTextContent("You will be logged out in 5 seconds.") }) it("does not announce at non-specified intervals", () => { - const {rerender} = render() + const {rerender} = renderWithRouter() const liveRegion = document.querySelector('[aria-live="assertive"]') const initialContent = liveRegion?.textContent // Update to 19 (should not announce) - rerender() + rerender( + + + + ) expect(liveRegion).toHaveTextContent(initialContent || "") // Update to 7 (should not announce) - rerender() + rerender( + + + + ) expect(liveRegion).toHaveTextContent(initialContent || "") }) it("does not announce when modal is closed", () => { - const {rerender} = render() + const {rerender} = renderWithRouter() const liveRegion = document.querySelector('[aria-live="assertive"]') const initialContent = liveRegion?.textContent // Close modal and update time - rerender() + rerender( + + + + ) // Content should remain unchanged since modal is closed expect(liveRegion).toHaveTextContent(initialContent || "") }) it("does not announce when timeLeft is 0 or negative", () => { - const {rerender} = render() + const {rerender} = renderWithRouter() const liveRegion = document.querySelector('[aria-live="assertive"]') const initialContent = liveRegion?.textContent // Update to 0 (should not announce) - rerender() + rerender( + + + + ) expect(liveRegion).toHaveTextContent(initialContent || "") }) }) describe("Aria attributes", () => { it("sets correct aria-labelledby attribute", () => { - render() + renderWithRouter() const modal = screen.getByTestId("eps-modal") expect(modal).toHaveAttribute("aria-labelledby", "session-timeout-title") }) it("sets correct aria-describedby attribute", () => { - render() + renderWithRouter() const modal = screen.getByTestId("eps-modal") expect(modal).toHaveAttribute("aria-describedby", "session-timeout-title") }) it("positions aria-live region off screen", () => { - render() + renderWithRouter() const liveRegion = document.querySelector('[aria-live="assertive"]') expect(liveRegion).toHaveStyle({ diff --git a/packages/cpt-ui/__tests__/useSessionTimeout.test.tsx b/packages/cpt-ui/__tests__/useSessionTimeout.test.tsx index 51a51f9a22..9076819b63 100644 --- a/packages/cpt-ui/__tests__/useSessionTimeout.test.tsx +++ b/packages/cpt-ui/__tests__/useSessionTimeout.test.tsx @@ -1,4 +1,6 @@ import {renderHook, act} from "@testing-library/react" +import React from "react" +import {MemoryRouter} from "react-router-dom" import {useSessionTimeout} from "@/hooks/useSessionTimeout" import {updateRemoteSelectedRole} from "@/helpers/userInfo" import {handleSignoutEvent} from "@/helpers/logout" @@ -35,6 +37,9 @@ jest.mock("@/helpers/awsRum", () => ({ } })) jest.mock("@/constants/environment", () => ({ + FRONTEND_PATHS: { + SELECT_YOUR_ROLE: "/select-your-role" + }, AUTH_CONFIG: { REDIRECT_SIGN_OUT: "mock-redirect-url", REDIRECT_SESSION_SIGN_OUT: "mock-session-redirect-url" @@ -97,6 +102,14 @@ const createAuthMock = (overrides: Partial = {}): AuthContextTy ...overrides }) +const renderHookWithRouter = (initialEntries = ["/"]) => renderHook(() => useSessionTimeout(), { + wrapper: ({children}: {children: React.ReactNode}) => ( + + {children} + + ) +}) + describe("useSessionTimeout", () => { beforeEach(() => { jest.clearAllMocks() @@ -109,7 +122,7 @@ describe("useSessionTimeout", () => { }) it("should return the expected handler functions", () => { - const {result} = renderHook(() => useSessionTimeout()) + const {result} = renderHookWithRouter() expect(typeof result.current.onStayLoggedIn).toBe("function") expect(typeof result.current.onLogOut).toBe("function") @@ -118,8 +131,28 @@ describe("useSessionTimeout", () => { }) describe("onStayLoggedIn", () => { + it("should close the modal instead of extending the session on the select your role path", async () => { + const {result} = renderHookWithRouter(["/select-your-role"]) + + await act(async () => { + await result.current.onStayLoggedIn() + }) + + expect(updateRemoteSelectedRole).not.toHaveBeenCalled() + expect(handleSignoutEvent).not.toHaveBeenCalled() + expect(mockSetLogoutModalType).toHaveBeenCalledWith(undefined) + + const updaterFn = mockSetSessionTimeoutModalInfo.mock.calls[0][0] + const updatedState = updaterFn({ + showModal: true, timeLeft: 60, action: undefined, buttonDisabled: false + }) + + expect(updatedState.action).toBe(undefined) + expect(updatedState.buttonDisabled).toBe(false) + }) + it("should extend the session when a role is selected", async () => { - const {result} = renderHook(() => useSessionTimeout()) + const {result} = renderHookWithRouter() await act(async () => { await result.current.onStayLoggedIn() @@ -135,7 +168,7 @@ describe("useSessionTimeout", () => { }) it("should set buttonDisabled and action to extending", async () => { - const {result} = renderHook(() => useSessionTimeout()) + const {result} = renderHookWithRouter() await act(async () => { await result.current.onStayLoggedIn() @@ -152,7 +185,7 @@ describe("useSessionTimeout", () => { }) it("should hide modal and reset state after successful extension", async () => { - const {result} = renderHook(() => useSessionTimeout()) + const {result} = renderHookWithRouter() await act(async () => { await result.current.onStayLoggedIn() @@ -173,7 +206,7 @@ describe("useSessionTimeout", () => { const authMock = createAuthMock({selectedRole: undefined}) jest.mocked(useAuth).mockReturnValue(authMock) - const {result} = renderHook(() => useSessionTimeout()) + const {result} = renderHookWithRouter() await act(async () => { await result.current.onStayLoggedIn() @@ -189,7 +222,7 @@ describe("useSessionTimeout", () => { jest.mocked(useAuth).mockReturnValue(authMock) jest.mocked(updateRemoteSelectedRole).mockRejectedValue(new Error("API Error")) - const {result} = renderHook(() => useSessionTimeout()) + const {result} = renderHookWithRouter() await act(async () => { await result.current.onStayLoggedIn() @@ -213,7 +246,7 @@ describe("useSessionTimeout", () => { () => new Promise(() => {}) // never resolves ) - const {result} = renderHook(() => useSessionTimeout()) + const {result} = renderHookWithRouter() // Start first call (won't resolve) act(() => { @@ -238,7 +271,7 @@ describe("useSessionTimeout", () => { const authMock = createAuthMock() jest.mocked(useAuth).mockReturnValue(authMock) - const {result} = renderHook(() => useSessionTimeout()) + const {result} = renderHookWithRouter() await act(async () => { await result.current.onLogOut() @@ -252,7 +285,7 @@ describe("useSessionTimeout", () => { }) it("should set loggingOut action and disable buttons", async () => { - const {result} = renderHook(() => useSessionTimeout()) + const {result} = renderHookWithRouter() await act(async () => { await result.current.onLogOut() @@ -269,7 +302,7 @@ describe("useSessionTimeout", () => { it("should prevent duplicate logout calls", async () => { jest.mocked(handleSignoutEvent).mockImplementation(() => new Promise(() => {})) - const {result} = renderHook(() => useSessionTimeout()) + const {result} = renderHookWithRouter() act(() => { result.current.onLogOut() @@ -290,7 +323,7 @@ describe("useSessionTimeout", () => { () => new Promise(() => {}) ) - const {result} = renderHook(() => useSessionTimeout()) + const {result} = renderHookWithRouter() // Start stay-logged-in (hangs) act(() => { @@ -311,7 +344,7 @@ describe("useSessionTimeout", () => { const authMock = createAuthMock() jest.mocked(useAuth).mockReturnValue(authMock) - const {result} = renderHook(() => useSessionTimeout()) + const {result} = renderHookWithRouter() await act(async () => { await result.current.onTimeOut() diff --git a/packages/cpt-ui/src/components/SessionTimeoutModal.tsx b/packages/cpt-ui/src/components/SessionTimeoutModal.tsx index e0ff167e01..b6cf8d8bf7 100644 --- a/packages/cpt-ui/src/components/SessionTimeoutModal.tsx +++ b/packages/cpt-ui/src/components/SessionTimeoutModal.tsx @@ -1,10 +1,13 @@ import React, {useEffect, useRef, useCallback} from "react" +import {useLocation} from "react-router-dom" +import {normalizePath} from "@/helpers/utils" import {Container} from "nhsuk-react-components" import {EpsModal} from "@/components/EpsModal" import {SESSION_TIMEOUT_MODAL_STRINGS} from "@/constants/ui-strings/SessionTimeoutModalStrings" import {Button} from "./ReactRouterButton" import {useAuth} from "@/context/AuthProvider" +import {FRONTEND_PATHS} from "@/constants/environment" interface SessionTimeoutModalProps { isOpen: boolean @@ -66,7 +69,8 @@ const useModalFocus = (isOpen: boolean) => { const useAriaLiveAnnouncements = ( isOpen: boolean, timeLeft: number, - liveRegionRef: React.RefObject + liveRegionRef: React.RefObject, + isSelectYourRolePath: boolean ) => { // Initialize aria-live region when modal first opens useEffect(() => { @@ -74,8 +78,14 @@ const useAriaLiveAnnouncements = ( if (shouldInitialize) { const minutes = Math.floor(timeLeft / 60) const seconds = timeLeft % 60 - const announcement = formatTimeAnnouncement(minutes, seconds) - updateLiveRegion(liveRegionRef, announcement) + if (isSelectYourRolePath) { + const selectRoleAnnouncement = + formatTimeAnnouncement(minutes, seconds) + ". " + SESSION_TIMEOUT_MODAL_STRINGS.SELECT_YOUR_ROLE_INSTRUCTION + updateLiveRegion(liveRegionRef, selectRoleAnnouncement) + } else { + const announcement = formatTimeAnnouncement(minutes, seconds) + updateLiveRegion(liveRegionRef, announcement) + } } }, [isOpen]) // Only run when modal opens @@ -105,7 +115,9 @@ export const SessionTimeoutModal: React.FC = ({ }) => { const liveRegionRef = useRef(null) const auth = useAuth() - + const location = useLocation() + const path = normalizePath(location.pathname) + const isSelectYourRolePath = (path === FRONTEND_PATHS.SELECT_YOUR_ROLE) const countdownTimerRef = useRef(null) const clearCountdownTimer = useCallback(() => { @@ -116,7 +128,7 @@ export const SessionTimeoutModal: React.FC = ({ }, []) useModalFocus(isOpen) - useAriaLiveAnnouncements(isOpen, timeLeft, liveRegionRef) + useAriaLiveAnnouncements(isOpen, timeLeft, liveRegionRef, isSelectYourRolePath) const handleKeyDown = (event: React.KeyboardEvent) => { if (event.key === "Escape") { @@ -181,6 +193,13 @@ export const SessionTimeoutModal: React.FC = ({

@@ -191,7 +210,8 @@ export const SessionTimeoutModal: React.FC = ({ onClick={onStayLoggedIn} disabled={buttonDisabledState} > - {SESSION_TIMEOUT_MODAL_STRINGS.STAY_LOGGED_IN} + {isSelectYourRolePath ? + SESSION_TIMEOUT_MODAL_STRINGS.CLOSE_MESSAGE : SESSION_TIMEOUT_MODAL_STRINGS.STAY_LOGGED_IN} + +

+ Important +

+

+ By using the Prescription Tracker, you are taking part in a private beta + and giving us permission to contact you for feedback. + View the privacy notice for more information. +

+

{pageTitle} diff --git a/packages/cpt-ui/src/constants/ui-strings/PrivacyNoticeStrings.ts b/packages/cpt-ui/src/constants/ui-strings/PrivacyNoticeStrings.ts index 6eac044ffb..f5a8562184 100644 --- a/packages/cpt-ui/src/constants/ui-strings/PrivacyNoticeStrings.ts +++ b/packages/cpt-ui/src/constants/ui-strings/PrivacyNoticeStrings.ts @@ -57,6 +57,14 @@ export const PrivacyNoticeStrings = { + " we process your data to comply with our legal obligation to operate the service and any" + " other applicable laws or regulations" }, + betaParticipation: { + "header": "Taking part in our private beta", + "p1": "By using the Prescription Tracker, you are giving us permission to contact you for feedback.", + "p2": "We will do this by emailing your NHS email address. Giving feedback is voluntary" + + " and you can ask us to stop contacting you for feedback at any time.", + "p3": "We might also email your NHS email address with information about the private beta.", + "p4": "All personal data will be stored securely." + }, legal: { header: "Our legal basis for data processing", base1: "Legal obligation: Article 6(1)(c) of UK GDPR." diff --git a/packages/cpt-ui/src/pages/PrivacyNoticePage.tsx b/packages/cpt-ui/src/pages/PrivacyNoticePage.tsx index 592e195efa..fbe4bddf3c 100644 --- a/packages/cpt-ui/src/pages/PrivacyNoticePage.tsx +++ b/packages/cpt-ui/src/pages/PrivacyNoticePage.tsx @@ -75,6 +75,12 @@ export default function PrivacyNoticePage() {
  • {PrivacyNoticeStrings.usage.legal}
    • +

    {PrivacyNoticeStrings.betaParticipation.header}

    +

    {PrivacyNoticeStrings.betaParticipation.p1}

    +

    {PrivacyNoticeStrings.betaParticipation.p2}

    +

    {PrivacyNoticeStrings.betaParticipation.p3}

    +

    {PrivacyNoticeStrings.betaParticipation.p4}

    +

    {PrivacyNoticeStrings.legal.header}

    diff --git a/packages/cpt-ui/src/pages/SearchPrescriptionPage.tsx b/packages/cpt-ui/src/pages/SearchPrescriptionPage.tsx index f6b018ee6e..fbe47f8b16 100644 --- a/packages/cpt-ui/src/pages/SearchPrescriptionPage.tsx +++ b/packages/cpt-ui/src/pages/SearchPrescriptionPage.tsx @@ -8,9 +8,10 @@ import { Col, Container, Hero, - Row + Row, + WarningCallout } from "nhsuk-react-components" -import {useLocation, useNavigate} from "react-router-dom" +import {Link, useLocation, useNavigate} from "react-router-dom" import TabSet from "@/components/tab-set" import PrescriptionIdSearch from "@/components/prescriptionSearch/PrescriptionIdSearch" @@ -21,6 +22,7 @@ import {HERO_TEXT} from "@/constants/ui-strings/SearchForAPrescriptionStrings" import {PRESCRIPTION_SEARCH_TABS} from "@/constants/ui-strings/SearchTabStrings" import "@/styles/searchforaprescription.scss" import {useSearchContext} from "@/context/SearchProvider" +import {FRONTEND_PATHS} from "@/constants/environment" export default function SearchPrescriptionPage() { const location = useLocation() @@ -241,7 +243,7 @@ export default function SearchPrescriptionPage() {
    -

    +
    +
    + +

    + Important +

    +

    + By using the Prescription Tracker, you are taking part in a private beta + and giving us permission to contact you for feedback. + View the privacy notice for more information. +

    +     + + {/* Aria-live region for announcing tab switch events */} + +
    + {ariaLiveMessage} +
    + - {/* Aria-live region for announcing tab switch events */} -
    - {ariaLiveMessage} -
    diff --git a/packages/cpt-ui/src/styles/searchforaprescription.scss b/packages/cpt-ui/src/styles/searchforaprescription.scss index 474d1edb72..e25627cc1e 100644 --- a/packages/cpt-ui/src/styles/searchforaprescription.scss +++ b/packages/cpt-ui/src/styles/searchforaprescription.scss @@ -52,7 +52,7 @@ } .prescription-id-aligned-element { - max-width: 52%; + // max-width: 52%; Temporarily ignored due to AEA-6194 margin-left: -30px; margin-bottom: 1rem; } From 2d2bd3303ce9e9d6a40347b1aed5ec4eb6100c21 Mon Sep 17 00:00:00 2001 From: anthony-nhs <121869075+anthony-nhs@users.noreply.github.com> Date: Wed, 8 Apr 2026 14:29:21 +0100 Subject: [PATCH 21/21] Chore: [AEA-6424] - New quality checks (#1990) ## Summary - Routine Change ### Details - move to latest qc - remove all trivy files - add CODEOWNERS to restrict updates to workflows - use least permissions on all workflows - add --ignore-scripts true to npm install --- .devcontainer/devcontainer.json | 2 +- .github/CODEOWNERS | 2 + .github/actions/mark_jira_released/action.yml | 26 ----- .../actions/update_confluence_jira/action.yml | 89 --------------- .github/workflows/cdk_package_code.yml | 3 +- .github/workflows/ci.yml | 29 ++++- .../delete_old_cloudformation_stacks.yml | 7 +- .github/workflows/link_dev_website.yml | 43 ------- .github/workflows/pull_request.yml | 106 +++++++++++++----- .github/workflows/release.yml | 37 +++++- .github/workflows/release_all_stacks.yml | 97 +++++++++++----- .github/workflows/run_regression_tests.yml | 8 +- .github/workflows/sync_copilot.yml | 2 + .../update_dev_container_version.yml | 1 + .gitignore | 1 + .grype.yaml | 16 +++ .pre-commit-config.yaml | 8 ++ .trivyignore.yaml | 62 ---------- Makefile | 2 +- trivy.yaml | 1 - zizmor.yml | 24 ++++ 21 files changed, 262 insertions(+), 304 deletions(-) create mode 100644 .github/CODEOWNERS delete mode 100644 .github/actions/mark_jira_released/action.yml delete mode 100644 .github/actions/update_confluence_jira/action.yml delete mode 100644 .github/workflows/link_dev_website.yml create mode 100644 .grype.yaml delete mode 100644 .trivyignore.yaml delete mode 100644 trivy.yaml create mode 100644 zizmor.yml diff --git a/.devcontainer/devcontainer.json b/.devcontainer/devcontainer.json index 56d556bbdd..87d66f9af0 100644 --- a/.devcontainer/devcontainer.json +++ b/.devcontainer/devcontainer.json @@ -6,7 +6,7 @@ "args": { "DOCKER_GID": "${env:DOCKER_GID:}", "IMAGE_NAME": "node_24_python_3_14", - "IMAGE_VERSION": "v1.2.0", + "IMAGE_VERSION": "v1.4.4", "USER_UID": "${localEnv:USER_ID:}", "USER_GID": "${localEnv:GROUP_ID:}" } diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS new file mode 100644 index 0000000000..0492a66516 --- /dev/null +++ b/.github/CODEOWNERS @@ -0,0 +1,2 @@ +# restrict access to approving workflow changes +.github/workflows/ @NHSDigital/eps-admins diff --git a/.github/actions/mark_jira_released/action.yml b/.github/actions/mark_jira_released/action.yml deleted file mode 100644 index 8318b4916e..0000000000 --- a/.github/actions/mark_jira_released/action.yml +++ /dev/null @@ -1,26 +0,0 @@ -name: "Create confluence release notes" -description: "Do release note actions in confluence and jira" -inputs: - RELEASE_TAG: - required: false - description: "The tag we are marking as released in jira" - DEV_CLOUD_FORMATION_EXECUTE_LAMBDA_ROLE: - required: true - description: "The role to assume to execute the release notes lambda" - -runs: - using: "composite" - steps: - - name: connect to dev account - uses: aws-actions/configure-aws-credentials@00943011d9042930efac3dcd3a170e4273319bc8 - with: - aws-region: eu-west-2 - role-to-assume: ${{ inputs.DEV_CLOUD_FORMATION_EXECUTE_LAMBDA_ROLE }} - role-session-name: prescription-clinical-tracker-ui-release-notes-run-lambda - - - name: call markJiraReleased lambda - shell: bash - working-directory: .github/scripts - env: - RELEASE_TAG: ${{ inputs.RELEASE_TAG }} - run: ./call_mark_jira_released.sh diff --git a/.github/actions/update_confluence_jira/action.yml b/.github/actions/update_confluence_jira/action.yml deleted file mode 100644 index 90bff965b6..0000000000 --- a/.github/actions/update_confluence_jira/action.yml +++ /dev/null @@ -1,89 +0,0 @@ -name: "Create confluence release notes" -description: "Do release note actions in confluence and jira" -inputs: - TARGET_ENVIRONMENT: - required: true - description: "Target Environment" - RELEASE_TAG: - required: false - description: "The tag we are releasing - only used for create_rc_release_notes" - CONFLUENCE_PAGE_ID: - required: true - description: "The id of confluence page to update or create under" - CREATE_RC_RELEASE_NOTES: - required: true - description: "whether to create rc release notes page instead of normal release notes" - default: "false" - DEV_CLOUD_FORMATION_EXECUTE_LAMBDA_ROLE: - required: true - description: "The role to assume to execute the release notes lambda" - DEV_CLOUD_FORMATION_CHECK_VERSION_ROLE: - required: true - description: "The dev cloud formation deploy role" - TARGET_CLOUD_FORMATION_CHECK_VERSION_ROLE: - required: true - description: "The target cloud formation deploy role" - -runs: - using: "composite" - steps: - - name: connect to target account - uses: aws-actions/configure-aws-credentials@00943011d9042930efac3dcd3a170e4273319bc8 - with: - aws-region: eu-west-2 - role-to-assume: ${{ inputs.TARGET_CLOUD_FORMATION_CHECK_VERSION_ROLE }} - role-session-name: prescription-clinical-tracker-ui-release-notes-target - - - name: Get deployed tag on target - shell: bash - working-directory: .github/scripts - env: - TARGET_ENVIRONMENT: ${{ inputs.TARGET_ENVIRONMENT }} - run: ./get_target_deployed_tag.sh - - - name: connect to dev account - uses: aws-actions/configure-aws-credentials@00943011d9042930efac3dcd3a170e4273319bc8 - with: - aws-region: eu-west-2 - role-to-assume: ${{ inputs.DEV_CLOUD_FORMATION_CHECK_VERSION_ROLE }} - role-session-name: prescription-clinical-tracker-ui-release-notes-dev - - - name: get current dev tag - shell: bash - working-directory: .github/scripts - run: ./get_current_dev_tag.sh - - - name: connect to dev account to run release notes lambda - uses: aws-actions/configure-aws-credentials@00943011d9042930efac3dcd3a170e4273319bc8 - with: - aws-region: eu-west-2 - role-to-assume: ${{ inputs.DEV_CLOUD_FORMATION_EXECUTE_LAMBDA_ROLE }} - role-session-name: prescription-clinical-tracker-ui-release-notes-run-lambda - unset-current-credentials: true - - - name: create int release notes - shell: bash - working-directory: .github/scripts - if: inputs.TARGET_ENVIRONMENT == 'int' && inputs.CREATE_RC_RELEASE_NOTES == 'false' - env: - ENV: INT - PAGE_ID: ${{ inputs.CONFLUENCE_PAGE_ID }} - run: ./create_env_release_notes.sh - - - name: create int rc release notes - shell: bash - working-directory: .github/scripts - if: inputs.TARGET_ENVIRONMENT == 'int' && inputs.CREATE_RC_RELEASE_NOTES == 'true' - env: - RELEASE_TAG: ${{ inputs.RELEASE_TAG }} - PAGE_ID: ${{ inputs.CONFLUENCE_PAGE_ID }} - run: ./create_int_rc_release_notes.sh - - - name: create prod release notes - shell: bash - working-directory: .github/scripts - if: inputs.TARGET_ENVIRONMENT == 'prod' - env: - ENV: PROD - PAGE_ID: ${{ inputs.CONFLUENCE_PAGE_ID }} - run: ./create_env_release_notes.sh diff --git a/.github/workflows/cdk_package_code.yml b/.github/workflows/cdk_package_code.yml index 69d4d8db45..6d89354686 100644 --- a/.github/workflows/cdk_package_code.yml +++ b/.github/workflows/cdk_package_code.yml @@ -13,6 +13,7 @@ on: required: true type: string +permissions: {} jobs: package_code: runs-on: ubuntu-22.04 @@ -33,7 +34,7 @@ jobs: - name: Checkout code uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd with: - ref: ${{ env.BRANCH_NAME }} + persist-credentials: false - name: make install run: | make install diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1e6975a433..a697d22cef 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -4,19 +4,26 @@ on: push: branches: [main] -env: - BRANCH_NAME: ${{ github.event.ref.BRANCH_NAME }} +permissions: {} jobs: get_config_values: - uses: NHSDigital/eps-common-workflows/.github/workflows/get-repo-config.yml@5ac2707dd9cd60ad127275179495b9c890d74711 + uses: NHSDigital/eps-common-workflows/.github/workflows/get-repo-config.yml@c8f899f30a6a726859b0277faa73cd9ff7f4de20 with: verify_published_from_main_image: true + permissions: + attestations: read + contents: read + packages: read quality_checks: - uses: NHSDigital/eps-common-workflows/.github/workflows/quality-checks-devcontainer.yml@5ac2707dd9cd60ad127275179495b9c890d74711 + uses: NHSDigital/eps-common-workflows/.github/workflows/quality-checks-devcontainer.yml@c8f899f30a6a726859b0277faa73cd9ff7f4de20 needs: [get_config_values] with: pinned_image: ${{ needs.get_config_values.outputs.pinned_image }} + permissions: + contents: read + id-token: write + packages: read secrets: SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }} @@ -32,20 +39,24 @@ jobs: tag_release: needs: [quality_checks, get_commit_id, get_config_values] - uses: NHSDigital/eps-common-workflows/.github/workflows/tag-release-devcontainer.yml@5ac2707dd9cd60ad127275179495b9c890d74711 + uses: NHSDigital/eps-common-workflows/.github/workflows/tag-release-devcontainer.yml@c8f899f30a6a726859b0277faa73cd9ff7f4de20 permissions: id-token: write contents: write + packages: write with: dry_run: true pinned_image: ${{ needs.get_config_values.outputs.pinned_image }} branch_name: main tag_format: ${{ needs.get_config_values.outputs.tag_format }} - secrets: inherit package_code: needs: [tag_release, get_commit_id, get_config_values] uses: ./.github/workflows/cdk_package_code.yml + permissions: + contents: read + packages: read + id-token: write with: VERSION_NUMBER: ${{needs.tag_release.outputs.version_tag}} COMMIT_ID: ${{needs.get_commit_id.outputs.commit_id}} @@ -54,6 +65,9 @@ jobs: release_dev: needs: [tag_release, package_code, get_commit_id, get_config_values] uses: ./.github/workflows/release_all_stacks.yml + permissions: + contents: write + id-token: write with: pinned_image: ${{ needs.get_config_values.outputs.pinned_image }} SERVICE_NAME: cpt-ui @@ -93,6 +107,9 @@ jobs: release_qa: needs: [tag_release, release_dev, package_code, get_commit_id, get_config_values] uses: ./.github/workflows/release_all_stacks.yml + permissions: + contents: write + id-token: write with: pinned_image: ${{ needs.get_config_values.outputs.pinned_image }} SERVICE_NAME: cpt-ui diff --git a/.github/workflows/delete_old_cloudformation_stacks.yml b/.github/workflows/delete_old_cloudformation_stacks.yml index 367f618435..12e41a81c8 100644 --- a/.github/workflows/delete_old_cloudformation_stacks.yml +++ b/.github/workflows/delete_old_cloudformation_stacks.yml @@ -6,24 +6,21 @@ on: schedule: - cron: "20 * * * *" -# A workflow run is made up of one or more jobs that can run sequentially or in parallel +permissions: {} jobs: - # This workflow contains a single job called "combine-prs" delete-old-cloudformation-stacks: - # The type of runner that the job will run on runs-on: ubuntu-22.04 permissions: id-token: write contents: read - # Steps represent a sequence of tasks that will be executed as part of the job steps: - name: Checkout local github scripts uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd with: - ref: ${{ env.BRANCH_NAME }} sparse-checkout: | .github/scripts + persist-credentials: false - name: Configure AWS Credentials uses: aws-actions/configure-aws-credentials@8df5847569e6427dd6c4fb1cf565c83acfa8afa7 diff --git a/.github/workflows/link_dev_website.yml b/.github/workflows/link_dev_website.yml deleted file mode 100644 index b80092d4b0..0000000000 --- a/.github/workflows/link_dev_website.yml +++ /dev/null @@ -1,43 +0,0 @@ -name: Link Dev Website -on: - pull_request: - types: [opened] -jobs: - get_issue_number: - runs-on: ubuntu-22.04 - outputs: - issue_number: ${{steps.get_issue_number.outputs.result}} - - steps: - - uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd - name: get issue number - id: get_issue_number - with: - script: | - if (context.issue.number) { - // Return issue number if present - return context.issue.number; - } else { - // Otherwise return issue number from commit - return ( - await github.rest.repos.listPullRequestsAssociatedWithCommit({ - commit_sha: context.sha, - owner: context.repo.owner, - repo: context.repo.repo, - }) - ).data[0].number; - } - result-encoding: string - - link-dev-website: - needs: [get_issue_number] - runs-on: ubuntu-22.04 - continue-on-error: true - steps: - - name: Link Dev Website - env: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - uses: unsplash/comment-on-pr@b5610c6125a7197eaec80072ea35ef53e1fc6035 - with: - msg: | - Deployed URL: https://cpt-ui-pr-${{ needs.get_issue_number.outputs.issue_number }}.dev.eps.national.nhs.uk/site diff --git a/.github/workflows/pull_request.yml b/.github/workflows/pull_request.yml index cc5bec54fd..5ec1cee49b 100644 --- a/.github/workflows/pull_request.yml +++ b/.github/workflows/pull_request.yml @@ -4,30 +4,39 @@ on: pull_request: branches: [main] -env: - BRANCH_NAME: ${{ github.event.pull_request.head.ref }} +permissions: {} jobs: get_config_values: - uses: NHSDigital/eps-common-workflows/.github/workflows/get-repo-config.yml@5ac2707dd9cd60ad127275179495b9c890d74711 + uses: NHSDigital/eps-common-workflows/.github/workflows/get-repo-config.yml@c8f899f30a6a726859b0277faa73cd9ff7f4de20 with: verify_published_from_main_image: false + permissions: + attestations: read + contents: read + packages: read dependabot-auto-approve-and-merge: needs: quality_checks - uses: NHSDigital/eps-common-workflows/.github/workflows/dependabot-auto-approve-and-merge.yml@5ac2707dd9cd60ad127275179495b9c890d74711 + uses: NHSDigital/eps-common-workflows/.github/workflows/dependabot-auto-approve-and-merge.yml@c8f899f30a6a726859b0277faa73cd9ff7f4de20 + permissions: + contents: write + pull-requests: write secrets: AUTOMERGE_APP_ID: ${{ secrets.AUTOMERGE_APP_ID }} AUTOMERGE_PEM: ${{ secrets.AUTOMERGE_PEM }} get_commit_message: runs-on: ubuntu-22.04 + permissions: + contents: read outputs: commit_message: ${{ steps.commit_message.outputs.commit_message }} steps: - name: Checkout code uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd with: - ref: ${{ env.BRANCH_NAME }} - fetch-depth: 0 + persist-credentials: false + fetch-depth: 1 + ref: ${{ github.event.pull_request.head.sha }} - name: Get Commit message id: commit_message run: | @@ -36,9 +45,13 @@ jobs: quality_checks: # always run, but only block in the non-skip case needs: [get_commit_message, get_config_values] - uses: NHSDigital/eps-common-workflows/.github/workflows/quality-checks-devcontainer.yml@5ac2707dd9cd60ad127275179495b9c890d74711 + uses: NHSDigital/eps-common-workflows/.github/workflows/quality-checks-devcontainer.yml@c8f899f30a6a726859b0277faa73cd9ff7f4de20 with: pinned_image: ${{ needs.get_config_values.outputs.pinned_image }} + permissions: + contents: read + id-token: write + packages: read secrets: SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }} @@ -63,42 +76,63 @@ jobs: const maxRetries = 120; // 20 minutes at 10 seconds each let attempts = 0; - async function fetchQCJob() { + async function fetchQCJobs() { const { data } = await github.rest.actions.listJobsForWorkflowRun({ owner, repo, run_id: runId }); - return data.jobs.find(job => job.name === 'quality_checks / quality_checks'); + return data.jobs.filter(job => job.name.startsWith('quality_checks')); } - let qc = await fetchQCJob(); - while ((!qc || qc.status !== 'completed') && attempts < maxRetries) { + let qcJobs = await fetchQCJobs(); + while (attempts < maxRetries) { + const allCompleted = qcJobs.length > 0 && qcJobs.every(qc => qc.status === 'completed'); + if (allCompleted) { + break; + } + attempts++; - console.log(`Attempt #${attempts}: ` + - (qc - ? `found job “${qc.name}” with status=${qc.status}` - : 'no matching quality_checks job yet')); + + if (qcJobs.length === 0) { + console.log(`Attempt #${attempts}: no matching quality_checks jobs yet`); + } else { + const incompleteJobs = qcJobs + .filter(qc => qc.status !== 'completed') + .map(qc => `“${qc.name}” status=${qc.status}`) + .join(', '); + console.log(`Attempt #${attempts}: waiting for quality_checks jobs to complete: ${incompleteJobs}`); + } + + if (attempts >= maxRetries) { + break; + } + await new Promise(r => setTimeout(r, pollTime)); - qc = await fetchQCJob(); + qcJobs = await fetchQCJobs(); } - if (!qc) { + if (qcJobs.length === 0) { core.setFailed( `Timed out waiting for a “quality_checks” job (after ${attempts} polls).` ); return; } - if (qc.status !== 'completed') { - core.setFailed( - `Quality checks job never completed (last status=${qc.status}).` - ); - return; + for (const qc of qcJobs) { + if (qc.status !== 'completed') { + core.setFailed( + `Quality checks job ${qc.name} never completed (last status=${qc.status})` + ); + return; + } } - if (qc.conclusion !== 'success') { - core.setFailed( - `Quality checks failed (conclusion=${qc.conclusion}).` - ); + for (const qc of qcJobs) { + if (qc.conclusion !== 'success' && qc.conclusion !== 'skipped') { + core.setFailed( + `Quality checks job ${qc.name} failed (conclusion=${qc.conclusion}).` + ); + return; + } } - name: Bypass QC gate @@ -106,7 +140,9 @@ jobs: run: echo "Skipping QC gate per commit message." pr_title_format_check: - uses: NHSDigital/eps-common-workflows/.github/workflows/pr_title_check.yml@5ac2707dd9cd60ad127275179495b9c890d74711 + uses: NHSDigital/eps-common-workflows/.github/workflows/pr_title_check.yml@c8f899f30a6a726859b0277faa73cd9ff7f4de20 + permissions: + pull-requests: write get_issue_number: runs-on: ubuntu-22.04 @@ -136,16 +172,16 @@ jobs: tag_release: needs: [get_config_values] - uses: NHSDigital/eps-common-workflows/.github/workflows/tag-release-devcontainer.yml@5ac2707dd9cd60ad127275179495b9c890d74711 + uses: NHSDigital/eps-common-workflows/.github/workflows/tag-release-devcontainer.yml@c8f899f30a6a726859b0277faa73cd9ff7f4de20 permissions: id-token: write contents: write + packages: write with: dry_run: true pinned_image: ${{ needs.get_config_values.outputs.pinned_image }} branch_name: ${{ github.event.pull_request.head.ref }} tag_format: ${{ needs.get_config_values.outputs.tag_format }} - secrets: inherit get_commit_id: runs-on: ubuntu-22.04 @@ -159,6 +195,10 @@ jobs: package_code: needs: [get_issue_number, get_commit_id, quality_gate, get_config_values] + permissions: + contents: read + packages: read + id-token: write if: | always() && ! contains(needs.*.result, 'failure') && @@ -169,6 +209,7 @@ jobs: COMMIT_ID: ${{ needs.get_commit_id.outputs.commit_id }} pinned_image: ${{ needs.get_config_values.outputs.pinned_image }} + release_code: needs: [get_issue_number, package_code, get_commit_id, get_config_values] if: | @@ -176,6 +217,9 @@ jobs: ! contains(needs.*.result, 'failure') && ! contains(needs.*.result, 'cancelled') uses: ./.github/workflows/release_all_stacks.yml + permissions: + contents: write + id-token: write with: pinned_image: ${{ needs.get_config_values.outputs.pinned_image }} SERVICE_NAME: cpt-ui-pr-${{needs.get_issue_number.outputs.issue_number}} @@ -222,4 +266,6 @@ jobs: steps: - name: Report Deployed URL run: | - echo "Deployed URL: https://cpt-ui-pr-${{ needs.get_issue_number.outputs.issue_number }}.dev.eps.national.nhs.uk" >> "$GITHUB_STEP_SUMMARY" + echo "Deployed URL: https://cpt-ui-pr-${ISSUE_NUMBER}.dev.eps.national.nhs.uk" >> "$GITHUB_STEP_SUMMARY" + env: + ISSUE_NUMBER: ${{ needs.get_issue_number.outputs.issue_number }} diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index ae3026efa8..4b38d9862f 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -3,19 +3,26 @@ name: deploy to environments on: workflow_dispatch: -env: - BRANCH_NAME: ${{ github.event.ref.BRANCH_NAME }} +permissions: {} jobs: get_config_values: - uses: NHSDigital/eps-common-workflows/.github/workflows/get-repo-config.yml@5ac2707dd9cd60ad127275179495b9c890d74711 + uses: NHSDigital/eps-common-workflows/.github/workflows/get-repo-config.yml@c8f899f30a6a726859b0277faa73cd9ff7f4de20 with: verify_published_from_main_image: true + permissions: + attestations: read + contents: read + packages: read quality_checks: - uses: NHSDigital/eps-common-workflows/.github/workflows/quality-checks-devcontainer.yml@5ac2707dd9cd60ad127275179495b9c890d74711 + uses: NHSDigital/eps-common-workflows/.github/workflows/quality-checks-devcontainer.yml@c8f899f30a6a726859b0277faa73cd9ff7f4de20 needs: [get_config_values] with: pinned_image: ${{ needs.get_config_values.outputs.pinned_image }} + permissions: + contents: read + id-token: write + packages: read secrets: SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }} @@ -31,10 +38,11 @@ jobs: tag_release: needs: [quality_checks, get_commit_id, get_config_values] - uses: NHSDigital/eps-common-workflows/.github/workflows/tag-release-devcontainer.yml@5ac2707dd9cd60ad127275179495b9c890d74711 + uses: NHSDigital/eps-common-workflows/.github/workflows/tag-release-devcontainer.yml@c8f899f30a6a726859b0277faa73cd9ff7f4de20 permissions: id-token: write contents: write + packages: write with: dry_run: false pinned_image: ${{ needs.get_config_values.outputs.pinned_image }} @@ -48,6 +56,10 @@ jobs: package_code: needs: [tag_release, get_commit_id, get_config_values] uses: ./.github/workflows/cdk_package_code.yml + permissions: + contents: read + packages: read + id-token: write with: VERSION_NUMBER: ${{needs.tag_release.outputs.version_tag}} COMMIT_ID: ${{needs.get_commit_id.outputs.commit_id}} @@ -56,6 +68,9 @@ jobs: release_dev: needs: [tag_release, package_code, get_commit_id, get_config_values] uses: ./.github/workflows/release_all_stacks.yml + permissions: + contents: write + id-token: write with: pinned_image: ${{ needs.get_config_values.outputs.pinned_image }} SERVICE_NAME: cpt-ui @@ -95,6 +110,9 @@ jobs: release_ref: needs: [tag_release, package_code, get_commit_id, release_dev, get_config_values] uses: ./.github/workflows/release_all_stacks.yml + permissions: + contents: write + id-token: write with: pinned_image: ${{ needs.get_config_values.outputs.pinned_image }} SERVICE_NAME: cpt-ui @@ -134,6 +152,9 @@ jobs: release_qa: needs: [tag_release, package_code, get_commit_id, release_dev, get_config_values] uses: ./.github/workflows/release_all_stacks.yml + permissions: + contents: write + id-token: write with: pinned_image: ${{ needs.get_config_values.outputs.pinned_image }} SERVICE_NAME: cpt-ui @@ -172,6 +193,9 @@ jobs: release_int: needs: [tag_release, package_code, get_commit_id, release_qa, get_config_values] uses: ./.github/workflows/release_all_stacks.yml + permissions: + contents: write + id-token: write with: pinned_image: ${{ needs.get_config_values.outputs.pinned_image }} SERVICE_NAME: cpt-ui @@ -206,6 +230,9 @@ jobs: release_prod: needs: [tag_release, package_code, get_commit_id, release_int, get_config_values] uses: ./.github/workflows/release_all_stacks.yml + permissions: + contents: write + id-token: write with: pinned_image: ${{ needs.get_config_values.outputs.pinned_image }} SERVICE_NAME: cpt-ui diff --git a/.github/workflows/release_all_stacks.yml b/.github/workflows/release_all_stacks.yml index 1e3afa130b..39c60f3f22 100644 --- a/.github/workflows/release_all_stacks.yml +++ b/.github/workflows/release_all_stacks.yml @@ -84,6 +84,7 @@ on: pinned_image: required: true type: string +permissions: {} jobs: release_all_code: runs-on: ubuntu-22.04 @@ -106,7 +107,7 @@ jobs: - name: Checkout local github actions uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd with: - ref: ${{ env.BRANCH_NAME }} + persist-credentials: false fetch-depth: 0 sparse-checkout: | .github @@ -136,19 +137,19 @@ jobs: CF_LONDON_EXPORTS=$(aws cloudformation list-exports --region eu-west-2 --output json) CLOUDFRONT_DISTRIBUTION_ID=$(echo "$CF_LONDON_EXPORTS" | \ jq \ - --arg EXPORT_NAME "${{ inputs.SERVICE_NAME }}-stateless-resources:cloudfrontDistribution:Id" \ + --arg EXPORT_NAME "${SERVICE_NAME}-stateless-resources:cloudfrontDistribution:Id" \ -r '.Exports[] | select(.Name == $EXPORT_NAME) | .Value') CLOUDFRONT_DISTRIBUTION_ARN=$(echo "$CF_LONDON_EXPORTS" | \ jq \ - --arg EXPORT_NAME "${{ inputs.SERVICE_NAME }}-stateless-resources:cloudfrontDistribution:Arn" \ + --arg EXPORT_NAME "${SERVICE_NAME}-stateless-resources:cloudfrontDistribution:Arn" \ -r '.Exports[] | select(.Name == $EXPORT_NAME) | .Value') RUM_LOG_GROUP_ARN=$(echo "$CF_LONDON_EXPORTS" | \ jq \ - --arg EXPORT_NAME "${{ inputs.SERVICE_NAME }}-stateful-resources:rum:logGroup:arn" \ + --arg EXPORT_NAME "${SERVICE_NAME}-stateful-resources:rum:logGroup:arn" \ -r '.Exports[] | select(.Name == $EXPORT_NAME) | .Value') RUM_APP_NAME=$(echo "$CF_LONDON_EXPORTS" | \ jq \ - --arg EXPORT_NAME "${{ inputs.SERVICE_NAME }}-stateful-resources:rum:rumApp:Name" \ + --arg EXPORT_NAME "${SERVICE_NAME}-stateful-resources:rum:rumApp:Name" \ -r '.Exports[] | select(.Name == $EXPORT_NAME) | .Value') REDEPLOY_STATEFUL_STACK="false" @@ -175,6 +176,8 @@ jobs: echo "REDEPLOY_STATEFUL_STACK=$REDEPLOY_STATEFUL_STACK" >> "$GITHUB_OUTPUT" shell: bash + env: + SERVICE_NAME: ${{ inputs.SERVICE_NAME }} - name: fix cdk.json for deployment stateful stack run: | @@ -236,7 +239,6 @@ jobs: run: | CF_LONDON_EXPORTS=$(aws cloudformation list-exports --region eu-west-2 --output json) CF_US_EXPORTS=$(aws cloudformation list-exports --region us-east-1 --output json) - SERVICE_NAME='${{ inputs.SERVICE_NAME }}' hostedLoginDomain=$(echo "$CF_US_EXPORTS" | jq --arg EXPORT_NAME "${SERVICE_NAME}-us-certs:fullCognitoDomain:Name" -r '.Exports[] | select(.Name == $EXPORT_NAME) | .Value') userPoolClientId=$(echo "$CF_LONDON_EXPORTS" | jq --arg EXPORT_NAME "${SERVICE_NAME}-stateful-resources:userPoolClient:userPoolClientId" -r '.Exports[] | select(.Name == $EXPORT_NAME) | .Value') userPoolId=$(echo "$CF_LONDON_EXPORTS" | jq --arg EXPORT_NAME "${SERVICE_NAME}-stateful-resources:userPool:Id" -r '.Exports[] | select(.Name == $EXPORT_NAME) | .Value') @@ -262,6 +264,8 @@ jobs: echo "rumSessionSampleRate=${rumSessionSampleRate}" echo "rumTelemetries=${rumTelemetries}" } >> "$GITHUB_ENV" + env: + SERVICE_NAME: ${{ inputs.SERVICE_NAME }} - name: build react app run: | @@ -271,9 +275,9 @@ jobs: export VITE_redirectSignIn="https://${fullCloudfrontDomain}/site/select-your-role" export VITE_redirectSignOut="https://${fullCloudfrontDomain}/site/logout" export VITE_redirectSessionSignOut="https://${fullCloudfrontDomain}/site/session-logged-out" - export VITE_COMMIT_ID=${{ inputs.COMMIT_ID }} - export VITE_VERSION_NUMBER=${{ inputs.VERSION_NUMBER }} - export VITE_TARGET_ENVIRONMENT=${{ inputs.TARGET_ENVIRONMENT }} + export VITE_COMMIT_ID=${COMMIT_ID} + export VITE_VERSION_NUMBER=${VERSION_NUMBER} + export VITE_TARGET_ENVIRONMENT=${TARGET_ENVIRONMENT} export VITE_RUM_GUEST_ROLE_ARN=${rumUnauthenticatedRumRoleArn} export VITE_RUM_IDENTITY_POOL_ID=${rumIdentityPoolId} export VITE_RUM_APPLICATION_ID=${rumAppId} @@ -281,20 +285,30 @@ jobs: export VITE_RUM_ENABLE_XRAY=${rumEnableXRay} export VITE_RUM_SESSION_SAMPLE_RATE=${rumSessionSampleRate} export VITE_RUM_TELEMETRIES=${rumTelemetries} - export VITE_REACT_LOG_LEVEL=${{ inputs.REACT_LOG_LEVEL }} + export VITE_REACT_LOG_LEVEL=${REACT_LOG_LEVEL} cd .build make react-build + env: + REACT_LOG_LEVEL: ${{ inputs.REACT_LOG_LEVEL }} + COMMIT_ID: ${{ inputs.COMMIT_ID }} + VERSION_NUMBER: ${{ inputs.VERSION_NUMBER }} + TARGET_ENVIRONMENT: ${{ inputs.TARGET_ENVIRONMENT }} - name: deploy website run: | - staticBucketName=$(aws cloudformation list-exports --query "Exports[?Name=='${{ inputs.SERVICE_NAME }}-stateful-resources:StaticContentBucket:Name'].Value" --output text) + staticBucketName=$(aws cloudformation list-exports --query "Exports[?Name=='${SERVICE_NAME}-stateful-resources:StaticContentBucket:Name'].Value" --output text) aws s3 cp ".build/packages/staticContent/404.html" "s3://${staticBucketName}/404.html" aws s3 cp ".build/packages/staticContent/404.css" "s3://${staticBucketName}/404.css" aws s3 cp ".build/packages/staticContent/500.html" "s3://${staticBucketName}/500.html" - aws s3 cp ".build/packages/staticContent/jwks/${{ inputs.TARGET_ENVIRONMENT }}/jwks.json" "s3://${staticBucketName}/jwks.json" - aws s3 cp --recursive ".build/packages/cpt-ui/dist/" "s3://${staticBucketName}/${{ inputs.VERSION_NUMBER }}/" - aws s3 cp ".build/packages/cpt-ui/dist/assets/" "s3://${staticBucketName}/source_maps/${{ inputs.COMMIT_ID }}/site/assets/" --exclude "*" --include "*.map" --recursive + aws s3 cp ".build/packages/staticContent/jwks/${TARGET_ENVIRONMENT}/jwks.json" "s3://${staticBucketName}/jwks.json" + aws s3 cp --recursive ".build/packages/cpt-ui/dist/" "s3://${staticBucketName}/${VERSION_NUMBER}/" + aws s3 cp ".build/packages/cpt-ui/dist/assets/" "s3://${staticBucketName}/source_maps/${COMMIT_ID}/site/assets/" --exclude "*" --include "*.map" --recursive + env: + SERVICE_NAME: ${{ inputs.SERVICE_NAME }} + TARGET_ENVIRONMENT: ${{ inputs.TARGET_ENVIRONMENT }} + VERSION_NUMBER: ${{ inputs.VERSION_NUMBER }} + COMMIT_ID: ${{ inputs.COMMIT_ID }} - name: fix cdk.json for deployment for stateless stack run: | @@ -354,16 +368,25 @@ jobs: id: update_cloudfront_kvs shell: bash run: | - # shellcheck disable=SC2140 - keyValueStore=$(aws cloudformation list-exports --region eu-west-2 --query "Exports[?Name=='"${{ inputs.SERVICE_NAME }}-stateless-resources:KeyValueStore:Arn"'].Value" --output text) - # shellcheck disable=SC2140 - cloudfrontDistributionId=$(aws cloudformation list-exports --region eu-west-2 --query "Exports[?Name=='"${{ inputs.SERVICE_NAME }}-stateless-resources:cloudfrontDistribution:Id"'].Value" --output text) - # shellcheck disable=SC2140 - primaryJwtPrivateKeyArn=$(aws cloudformation list-exports --region eu-west-2 --query "Exports[?Name=='"${{ inputs.SERVICE_NAME }}-stateless-resources:primaryJwtPrivateKey:Arn"'].Value" --output text) - # shellcheck disable=SC2140 - mockJwtPrivateKeyArn=$(aws cloudformation list-exports --region eu-west-2 --query "Exports[?Name=='"${{ inputs.SERVICE_NAME }}-stateless-resources:mockJwtPrivateKey:Arn"'].Value" --output text) + CF_LONDON_EXPORTS=$(aws cloudformation list-exports --region eu-west-2 --output json) + keyValueStore=$(echo "$CF_LONDON_EXPORTS" | \ + jq \ + --arg EXPORT_NAME "${SERVICE_NAME}-stateless-resources:KeyValueStore:Arn" \ + -r '.Exports[] | select(.Name == $EXPORT_NAME) | .Value') + cloudfrontDistributionId=$(echo "$CF_LONDON_EXPORTS" | \ + jq \ + --arg EXPORT_NAME "${SERVICE_NAME}-stateless-resources:cloudfrontDistribution:Id" \ + -r '.Exports[] | select(.Name == $EXPORT_NAME) | .Value') + primaryJwtPrivateKeyArn=$(echo "$CF_LONDON_EXPORTS" | \ + jq \ + --arg EXPORT_NAME "${SERVICE_NAME}-stateless-resources:primaryJwtPrivateKey:Arn" \ + -r '.Exports[] | select(.Name == $EXPORT_NAME) | .Value') + mockJwtPrivateKeyArn=$(echo "$CF_LONDON_EXPORTS" | \ + jq \ + --arg EXPORT_NAME "${SERVICE_NAME}-stateless-resources:mockJwtPrivateKey:Arn" \ + -r '.Exports[] | select(.Name == $EXPORT_NAME) | .Value') - newVersion="${{ inputs.VERSION_NUMBER }}" + newVersion="${VERSION_NUMBER}" ETag=$(aws cloudfront-keyvaluestore describe-key-value-store \ --kvs-arn="$keyValueStore" \ @@ -396,14 +419,18 @@ jobs: if [ -z "${primaryJwtPrivateKeyArn}" ]; then echo "primaryJwtPrivateKeyArn is unset or set to the empty string" else - aws secretsmanager put-secret-value --secret-id "${primaryJwtPrivateKeyArn}" --secret-string "${{ secrets.JWT_PRIVATE_KEY }}" + aws secretsmanager put-secret-value --secret-id "${primaryJwtPrivateKeyArn}" --secret-string "${JWT_PRIVATE_KEY}" fi # set the mockJwtPrivateKeyArn secret if [ -z "${mockJwtPrivateKeyArn}" ]; then echo "mockJwtPrivateKeyArn is unset or set to the empty string" else - aws secretsmanager put-secret-value --secret-id "${mockJwtPrivateKeyArn}" --secret-string "${{ secrets.JWT_PRIVATE_KEY }}" + aws secretsmanager put-secret-value --secret-id "${mockJwtPrivateKeyArn}" --secret-string "${JWT_PRIVATE_KEY}" fi + env: + VERSION_NUMBER: ${{ inputs.VERSION_NUMBER }} + SERVICE_NAME: ${{ inputs.SERVICE_NAME }} + JWT_PRIVATE_KEY: ${{ secrets.JWT_PRIVATE_KEY }} - name: fix cdk.json for deployment for stateful stack redeployment if: ${{ steps.check_redeploy_stateful_stack.outputs.REDEPLOY_STATEFUL_STACK == 'true' }} @@ -476,27 +503,35 @@ jobs: with: ref: gh-pages path: gh-pages + persist-credentials: true - name: update release tag in github pages if: ${{ inputs.IS_PULL_REQUEST == false }} run: | cd gh-pages NOW=$(date +'%Y-%m-%dT%H:%M:%S') - echo "tag,release_datetime" > _data/${{ inputs.TARGET_ENVIRONMENT }}_latest.csv - echo "${{ inputs.VERSION_NUMBER }},${NOW}" >> _data/${{ inputs.TARGET_ENVIRONMENT }}_latest.csv - echo "${{ inputs.VERSION_NUMBER }},${NOW}" >> _data/${{ inputs.TARGET_ENVIRONMENT }}_deployments.csv + echo "tag,release_datetime" > "_data/${TARGET_ENVIRONMENT}_latest.csv" + echo "${VERSION_NUMBER},${NOW}" >> "_data/${TARGET_ENVIRONMENT}_latest.csv" + echo "${VERSION_NUMBER},${NOW}" >> "_data/${TARGET_ENVIRONMENT}_deployments.csv" git config user.name github-actions git config user.email github-actions@github.com - git add _data/${{ inputs.TARGET_ENVIRONMENT }}_latest.csv - git add _data/${{ inputs.TARGET_ENVIRONMENT }}_deployments.csv - git commit -m 'update releases for ${{ inputs.TARGET_ENVIRONMENT }}' + git add "_data/${TARGET_ENVIRONMENT}_latest.csv" + git add "_data/${TARGET_ENVIRONMENT}_deployments.csv" + git commit -m "update releases for ${TARGET_ENVIRONMENT}" parallel --retries 10 --delay 3 ::: "git pull --rebase && git push" + env: + TARGET_ENVIRONMENT: ${{ inputs.TARGET_ENVIRONMENT }} + VERSION_NUMBER: ${{ inputs.VERSION_NUMBER }} regression_tests: name: Regression Tests uses: ./.github/workflows/run_regression_tests.yml if: ${{ always() && !failure() && !cancelled() && inputs.RUN_REGRESSION_TESTS == true }} needs: [release_all_code] + permissions: + contents: write + id-token: write + with: ENVIRONMENT: ${{ inputs.TARGET_ENVIRONMENT }} VERSION_NUMBER: ${{ inputs.VERSION_NUMBER }} diff --git a/.github/workflows/run_regression_tests.yml b/.github/workflows/run_regression_tests.yml index 476a2e2918..a3998d36e8 100644 --- a/.github/workflows/run_regression_tests.yml +++ b/.github/workflows/run_regression_tests.yml @@ -18,6 +18,8 @@ on: REGRESSION_TESTS_PEM: required: true +permissions: {} + jobs: run_regression_tests: runs-on: ubuntu-22.04 @@ -38,7 +40,7 @@ jobs: - name: Checkout local github actions uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd with: - ref: ${{ env.BRANCH_NAME }} + persist-credentials: false fetch-depth: 0 - name: Generate a token to authenticate regression testing @@ -54,7 +56,7 @@ jobs: env: TARGET_ENVIRONMENT: ${{ inputs.ENVIRONMENT }} VERSION_NUMBER: ${{ inputs.VERSION_NUMBER }} - GITHUB-TOKEN: ${{ steps.generate-token.outputs.token }} + GITHUB_TOKEN: ${{ steps.generate-token.outputs.token }} run: | if [[ "$TARGET_ENVIRONMENT" != "prod" && "$TARGET_ENVIRONMENT" != "ref" ]]; then REGRESSION_TEST_REPO_TAG="v3.12.12" # This is the tag or branch of the regression test code to run, usually a version tag like v3.1.0 or a branch name @@ -85,7 +87,7 @@ jobs: poetry run python -u run_regression_tests.py \ --env="$TARGET_ENVIRONMENT" \ --pr_label="$VERSION_NUMBER" \ - --token=${{ steps.generate-token.outputs.token }} \ + --token="${GITHUB_TOKEN}" \ --is_called_from_github=true \ --product=CPTS-UI \ --regression_test_repo_tag "${REGRESSION_TEST_REPO_TAG}" \ diff --git a/.github/workflows/sync_copilot.yml b/.github/workflows/sync_copilot.yml index 72b62eb1d7..1bb9174306 100644 --- a/.github/workflows/sync_copilot.yml +++ b/.github/workflows/sync_copilot.yml @@ -5,6 +5,8 @@ on: schedule: - cron: '0 6 * * 1' +permissions: {} + jobs: sync-copilot-instructions: runs-on: ubuntu-22.04 diff --git a/.github/workflows/update_dev_container_version.yml b/.github/workflows/update_dev_container_version.yml index ef79d062c5..04e985f175 100644 --- a/.github/workflows/update_dev_container_version.yml +++ b/.github/workflows/update_dev_container_version.yml @@ -4,6 +4,7 @@ on: workflow_dispatch: schedule: - cron: '0 6 * * 4' + permissions: {} jobs: diff --git a/.gitignore b/.gitignore index 0fbc257ad7..0c4ba9b77d 100644 --- a/.gitignore +++ b/.gitignore @@ -35,3 +35,4 @@ packages/cpt-ui/dist packages/auth_demo/build/ .build/ .local_config/ +.sbom/ diff --git a/.grype.yaml b/.grype.yaml new file mode 100644 index 0000000000..f20b2ec8f6 --- /dev/null +++ b/.grype.yaml @@ -0,0 +1,16 @@ +ignore: + # rollup + - vulnerability: GHSA-mw96-cpmx-2vgc + # node-forge + - vulnerability: GHSA-5m6q-g25r-mvwx + - vulnerability: GHSA-q67f-28xg-22rw + - vulnerability: GHSA-ppp5-5v6c-4jwp + - vulnerability: GHSA-2328-f5f3-gj25 + # path-to-regexp + - vulnerability: GHSA-j3q9-mxjg-w52f + # lodash + - vulnerability: GHSA-r5fr-rjxr-66jc + # vite - only affects dev server - needs vitest and @vitejs/plugin-react-swc to be updated + - vulnerability: GHSA-p9ff-h696-f583 + # vite - only affects server.fs.deny which we don't use + - vulnerability: GHSA-v2wj-q39q-566r diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index d2b85a72a3..2d392429bc 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -23,6 +23,14 @@ repos: - repo: local hooks: + - id: grype-scan-local + name: Grype scan local changes + entry: make + args: ["grype-scan-local"] + language: system + pass_filenames: false + always_run: true + - id: check-commit-signing name: Check commit signing description: Ensures that commits are GPG signed diff --git a/.trivyignore.yaml b/.trivyignore.yaml deleted file mode 100644 index a8adbba059..0000000000 --- a/.trivyignore.yaml +++ /dev/null @@ -1,62 +0,0 @@ -vulnerabilities: - - id: CVE-2025-12816 - paths: - - "package-lock.json" - statement: downstream dependency just used in build stage - expired_at: 2026-06-01 - - id: CVE-2025-66031 - paths: - - "package-lock.json" - statement: downstream dependency just used in build stage - expired_at: 2026-06-01 - - id: CVE-2026-25128 - paths: - - "package-lock.json" - statement: downstream dependency of fast-xml-parser - expired_at: 2026-06-01 - - id: CVE-2026-26278 - statement: fast-xml-parser vulnerability accepted as risk - dependency of aws-cdk/client-dynamodb - expired_at: 2026-06-01 - - id: CVE-2026-26960 - statement: tar vulnerability accepted as risk - expired_at: 2026-06-01 - - id: CVE-2026-26996 - paths: - - "package-lock.json" - statement: downstream dependency - vulnerability is not relevant - we don't allow user input for anything regex. - - id: CVE-2026-27606 - paths: - - "package-lock.json" - statement: Vulnerability in AWS dependency accepted as risk - expired_at: 2026-06-01 - - id: CVE-2026-27903 - paths: - - "package-lock.json" - statement: Vulnerability in minimatch - waiting for depencency updates - expired_at: 2026-06-01 - - id: CVE-2026-27904 - paths: - - "package-lock.json" - statement: Vulnerability in minimatch - waiting for depencency updates - expired_at: 2026-06-01 - - id: CVE-2026-29063 - statement: fixed - waiting for dependency updates - expired_at: 2026-06-01 - - id: CVE-2026-32141 - statement: Transitive dependency vulnerability in flatted - expired_at: 2026-06-01 - - id: CVE-2026-1526 - statement: Transitive dependency vulnerability in undici of npm - expired_at: 2026-06-01 - - id: CVE-2026-1528 - statement: Transitive dependency vulnerability in undici of npm - expired_at: 2026-06-01 - - id: CVE-2026-2229 - statement: Transitive dependency vulnerability in undici of npm - expired_at: 2026-06-01 - - id: CVE-2026-32597 - statement: Transitive dependency vulnerability in pyjwt - expired_at: 2026-06-01 - - id: CVE-2026-33036 - statement: fast-xml-parser vulnerability accepted as risk - dependency of aws-cdk/client-dynamodb - expired_at: 2026-06-01 diff --git a/Makefile b/Makefile index c24df66311..6f8e7be31c 100644 --- a/Makefile +++ b/Makefile @@ -12,7 +12,7 @@ install-python: poetry install install-node: - npm ci + npm ci --ignore-scripts install-hooks: install-python poetry run pre-commit install --install-hooks --overwrite diff --git a/trivy.yaml b/trivy.yaml deleted file mode 100644 index eb2433758b..0000000000 --- a/trivy.yaml +++ /dev/null @@ -1 +0,0 @@ -ignorefile: ".trivyignore.yaml" diff --git a/zizmor.yml b/zizmor.yml new file mode 100644 index 0000000000..fcefc33b95 --- /dev/null +++ b/zizmor.yml @@ -0,0 +1,24 @@ +rules: + unpinned-images: + # these workflows use unpinned images because they are using a full image passed in that contains the tag + ignore: + - cdk_package_code.yml:21:7 + - release_all_stacks.yml:93:7 + - run_regression_tests.yml:27:7 + - pull_request.yml:223:7 + secrets-outside-env: + # these are ignored because they are using known secrets + ignore: + - delete_old_cloudformation_stacks.yml:29:31 + - delete_old_cloudformation_stacks.yml:18:9 + - run_regression_tests.yml:51:28 + secrets-inherit: + ignore: + - pull_request.yml:219:11 + - release.yml:232:11 + - release.yml:195:11 + - release.yml:154:11 + - release.yml:112:11 + - release.yml:70:11 + - ci.yml:109:11 + - ci.yml:67:11