diff --git a/package-lock.json b/package-lock.json index ebcc1e0aee..f55a2ef27b 100644 --- a/package-lock.json +++ b/package-lock.json @@ -26607,7 +26607,7 @@ "version": "2.0.0", "license": "MIT" }, - "node_modules/selected-role-lambda": { + "node_modules/selectedRoleLambda": { "resolved": "packages/selectedRoleLambda", "link": true }, @@ -32647,7 +32647,6 @@ } }, "packages/selectedRoleLambda": { - "name": "selected-role-lambda", "version": "1.0.0", "license": "MIT", "dependencies": { diff --git a/packages/cpt-ui/__tests__/ChangeRolePage.test.tsx b/packages/cpt-ui/__tests__/ChangeRolePage.test.tsx index c539b1989d..c25d7516e6 100644 --- a/packages/cpt-ui/__tests__/ChangeRolePage.test.tsx +++ b/packages/cpt-ui/__tests__/ChangeRolePage.test.tsx @@ -16,8 +16,8 @@ jest.mock("@/constants/ui-strings/CardStrings", () => { "You are currently logged in at GREENE'S PHARMACY (ODS: FG419) with Health Professional Access Role.", }, confirmButton: { - text: "Confirm and continue to find a prescription", - link: "tracker-presc-no", + text: "Continue to find a prescription", + link: "searchforaprescription", }, alternativeMessage: "Alternatively, you can choose a new role below.", organisation: "Organisation", @@ -280,4 +280,58 @@ describe("ChangeRolePage", () => { expect(mockPush).toHaveBeenCalledWith("/searchforaprescription") }) }) + + it("renders loading state when waiting for API response", async () => { + mockFetch.mockImplementation(() => new Promise(() => {})) + renderWithAuth() + expect(screen.getByText("Loading...")).toBeInTheDocument() + }) + + it("redirects when a single role is available", async () => { + (useRouter as jest.Mock).mockReturnValue({ + push: jest.fn() + }) + + const mockUserInfo = { + roles_with_access: [{ + role_name: "Pharmacist", + org_name: "Test Pharmacy", + org_code: "ORG123", + site_address: "123 Test St" + }], + roles_without_access: [] + } + + mockFetch.mockResolvedValue({ + status: 200, + json: async () => ({userInfo: mockUserInfo}) + }) + + renderWithAuth({isSignedIn: true, idToken: {toString: jest.fn().mockReturnValue("mock-id-token")}}) + + await waitFor(() => { + expect(useRouter().push).toHaveBeenCalledWith("/searchforaprescription") + }) + }) + + it("does not fetch user roles if user is not signed in", async () => { + const mockFetch = jest.fn() + global.fetch = mockFetch + + renderWithAuth({isSignedIn: false}) // Simulating a user who is not signed in + + expect(mockFetch).not.toHaveBeenCalled() + }) + + it("displays an error when the API request fails", async () => { + mockFetch.mockRejectedValue(new Error("Failed to fetch user roles")) + + renderWithAuth({isSignedIn: true, idToken: {toString: jest.fn().mockReturnValue("mock-id-token")}}) + + await waitFor(() => { + const errorSummary = screen.getByRole("heading", {name: "Error during role selection"}) + expect(errorSummary).toBeInTheDocument() + expect(screen.getByText("Failed to fetch CPT user info")).toBeInTheDocument() + }) + }) }) diff --git a/packages/cpt-ui/__tests__/RBACBanner.test.tsx b/packages/cpt-ui/__tests__/RBACBanner.test.tsx index 496ec22d50..6420e1a477 100644 --- a/packages/cpt-ui/__tests__/RBACBanner.test.tsx +++ b/packages/cpt-ui/__tests__/RBACBanner.test.tsx @@ -1,8 +1,8 @@ import "@testing-library/jest-dom" -import { render, screen } from "@testing-library/react" -import { useRouter } from "next/navigation" +import {render, screen} from "@testing-library/react" +import {useRouter} from "next/navigation" import React from "react" -import { JWT } from "aws-amplify/auth" +import {JWT} from "aws-amplify/auth" import RBACBanner from "@/components/RBACBanner" @@ -19,10 +19,10 @@ jest.mock("@/constants/ui-strings/RBACBannerStrings", () => { LOCUM_NAME: "Locum pharmacy" } - return { RBAC_BANNER_STRINGS } + return {RBAC_BANNER_STRINGS} }) -const { RBAC_BANNER_STRINGS } = require("@/constants/ui-strings/RBACBannerStrings") +const {RBAC_BANNER_STRINGS} = require("@/constants/ui-strings/RBACBannerStrings") // Mock `next/navigation` jest.mock("next/navigation", () => ({ @@ -46,8 +46,8 @@ jest.mock("@/context/AccessProvider", () => { org_name: "org name" }, userDetails: { - given_name: "Jane", - family_name: "Doe" + given_name: "JaNe", + family_name: "DoE" }, setUserDetails: jest.fn(), setSelectedRole: jest.fn(), @@ -57,7 +57,7 @@ jest.mock("@/context/AccessProvider", () => { const useAccess = () => React.useContext(MockAccessContext) const __setMockContextValue = (newValue: any) => { - mockContextValue = { ...mockContextValue, ...newValue } + mockContextValue = {...mockContextValue, ...newValue} // Reassign the context’s defaultValue so subsequent consumers get new values MockAccessContext._currentValue = mockContextValue MockAccessContext._currentValue2 = mockContextValue @@ -70,7 +70,7 @@ jest.mock("@/context/AccessProvider", () => { __setMockContextValue } }) -const { __setMockContextValue } = require("@/context/AccessProvider") +const {__setMockContextValue} = require("@/context/AccessProvider") // Mock an AuthContext const AuthContext = React.createContext(null) @@ -90,7 +90,7 @@ const defaultAuthContext = { } export const renderWithAuth = (authOverrides = {}) => { - const authValue = { ...defaultAuthContext, ...authOverrides } + const authValue = {...defaultAuthContext, ...authOverrides} return render( @@ -112,7 +112,7 @@ describe("RBACBanner", () => { org_name: "org name" }, userDetails: { - family_name: "Doe", + family_name: "DoE", given_name: "Jane" } }) @@ -145,7 +145,7 @@ describe("RBACBanner", () => { expect(bannerText).toBeInTheDocument() // Check that placeholders are properly replaced - const expectedText = `CONFIDENTIAL: PERSONAL PATIENT DATA accessed by Doe, Jane - Role Name - org name (ODS: deadbeef)` + const expectedText = `CONFIDENTIAL: PERSONAL PATIENT DATA accessed by DOE, Jane - Role Name - org name (ODS: deadbeef)` expect(bannerText).toHaveTextContent(expectedText) }) @@ -165,7 +165,7 @@ describe("RBACBanner", () => { const bannerText = screen.getByTestId("rbac-banner-text") // Locum pharmacy name should appear expect(bannerText).toHaveTextContent( - `CONFIDENTIAL: PERSONAL PATIENT DATA accessed by Doe, Jane - Role Name - Locum pharmacy (ODS: FFFFF)` + `CONFIDENTIAL: PERSONAL PATIENT DATA accessed by DOE, Jane - Role Name - Locum pharmacy (ODS: FFFFF)` ) }) @@ -198,7 +198,25 @@ describe("RBACBanner", () => { const bannerText = screen.getByTestId("rbac-banner-text") // Notice fallback values: NO_ROLE_NAME, NO_ORG_NAME, NO_ODS_CODE expect(bannerText).toHaveTextContent( - `CONFIDENTIAL: PERSONAL PATIENT DATA accessed by Doe, Jane - NO_ROLE_NAME - NO_ORG_NAME (ODS: NO_ODS_CODE)` + `CONFIDENTIAL: PERSONAL PATIENT DATA accessed by DOE, Jane - NO_ROLE_NAME - NO_ORG_NAME (ODS: NO_ODS_CODE)` + ) + }) + + it("should fallback to NO_ORG_NAME if org_name is missing", () => { + __setMockContextValue({ + selectedRole: { + role_name: "Role Name", + role_id: "role-id", + org_code: "deadbeef" + // org_name is missing + } + }) + + renderWithAuth() + + const bannerText = screen.getByTestId("rbac-banner-text") + expect(bannerText).toHaveTextContent( + `CONFIDENTIAL: PERSONAL PATIENT DATA accessed by DOE, Jane - Role Name - NO_ORG_NAME (ODS: deadbeef)` ) }) diff --git a/packages/cpt-ui/__tests__/SelectYourRolePage.test.tsx b/packages/cpt-ui/__tests__/SelectYourRolePage.test.tsx index bcccad6d68..ee1f770ac1 100644 --- a/packages/cpt-ui/__tests__/SelectYourRolePage.test.tsx +++ b/packages/cpt-ui/__tests__/SelectYourRolePage.test.tsx @@ -20,8 +20,8 @@ jest.mock("@/constants/ui-strings/CardStrings", () => { "You are currently logged in at GREENE'S PHARMACY (ODS: FG419) with Health Professional Access Role.", }, confirmButton: { - text: "Confirm and continue to find a prescription", - link: "tracker-presc-no", + text: "Continue to find a prescription", + link: "searchforaprescription", }, alternativeMessage: "Alternatively, you can choose a new role below.", organisation: "Organisation", @@ -321,4 +321,25 @@ describe("SelectYourRolePage", () => { expect(useRouter().push).toHaveBeenCalledWith("/searchforaprescription") }) }) + + it("does not fetch user roles if user is not signed in", async () => { + const mockFetch = jest.fn() + global.fetch = mockFetch + + renderWithAuth({isSignedIn: false}) // Simulating a user who is not signed in + + expect(mockFetch).not.toHaveBeenCalled() + }) + + it("displays an error when the API request fails", async () => { + mockFetch.mockRejectedValue(new Error("Failed to fetch user roles")) + + renderWithAuth({isSignedIn: true, idToken: {toString: jest.fn().mockReturnValue("mock-id-token")}}) + + await waitFor(() => { + const errorSummary = screen.getByRole("heading", {name: "Error during role selection"}) + expect(errorSummary).toBeInTheDocument() + expect(screen.getByText("Failed to fetch CPT user info")).toBeInTheDocument() + }) + }) }) diff --git a/packages/cpt-ui/__tests__/YourSelectedRole.test.tsx b/packages/cpt-ui/__tests__/YourSelectedRole.test.tsx index 1b699d44f9..57dc1c34c0 100644 --- a/packages/cpt-ui/__tests__/YourSelectedRole.test.tsx +++ b/packages/cpt-ui/__tests__/YourSelectedRole.test.tsx @@ -15,7 +15,7 @@ jest.mock("@/constants/ui-strings/YourSelectedRoleStrings", () => { roleLabel: "Role", orgLabel: "Organisation", changeLinkText: "Change", - confirmButtonText: "Confirm and continue to find a prescription", + confirmButtonText: "Continue to find a prescription", noODSCode: "NO ODS CODE", noRoleName: "NO ROLE NAME", noOrgName: "NO ORG NAME" diff --git a/packages/cpt-ui/app/confirmrole/page.tsx b/packages/cpt-ui/app/confirmrole/page.tsx index 7534870e52..901335690f 100644 --- a/packages/cpt-ui/app/confirmrole/page.tsx +++ b/packages/cpt-ui/app/confirmrole/page.tsx @@ -1,7 +1,7 @@ 'use client' -import React from "react"; +import React from "react" -import { Button, Card, Container, Col, InsetText, Row } from "nhsuk-react-components"; +import {Button, Card, Container, Col, InsetText, Row} from "nhsuk-react-components" export default function Page() { return (
@@ -26,7 +26,7 @@ export default function Page() {

Alternatively, you can choose a new role below.

diff --git a/packages/cpt-ui/app/logout/page.tsx b/packages/cpt-ui/app/logout/page.tsx index d8b235a3a6..19e89ab0b7 100644 --- a/packages/cpt-ui/app/logout/page.tsx +++ b/packages/cpt-ui/app/logout/page.tsx @@ -1,54 +1,53 @@ 'use client' -import React, { useContext, useEffect } from "react"; -import { Container } from "nhsuk-react-components" -import Link from "next/link"; +import React, {useContext, useEffect} from "react" +import {Container} from "nhsuk-react-components" +import Link from "next/link" -import { AuthContext } from "@/context/AuthProvider"; -import { useAccess } from "@/context/AccessProvider" -import EpsSpinner from "@/components/EpsSpinner"; -import { EpsLogoutStrings } from "@/constants/ui-strings/EpsLogoutPageStrings"; +import {AuthContext} from "@/context/AuthProvider" +import {useAccess} from "@/context/AccessProvider" +import EpsSpinner from "@/components/EpsSpinner" +import {EpsLogoutStrings} from "@/constants/ui-strings/EpsLogoutPageStrings" export default function LogoutPage() { - - const auth = useContext(AuthContext); - const { clear } = useAccess(); + const auth = useContext(AuthContext) + const {clear} = useAccess() // Log out on page load useEffect(() => { const signOut = async () => { - console.log("Signing out", auth); + console.log("Signing out", auth) + + await auth?.cognitoSignOut() - await auth?.cognitoSignOut(); - console.log("Signed out: ", auth); + // Ensure user details & roles are cleared from local storage + clear() + console.log("Signed out and cleared session data") } if (auth?.isSignedIn) { - signOut(); - clear(); + signOut() } else { - console.log("Cannot sign out - not signed in"); + console.log("Cannot sign out - not signed in") + clear() // Clear data even if not signed in } - }, [auth, clear]); + }, [auth, clear]) return (
- {auth?.isSignedIn ? ( + {!auth?.isSignedIn ? ( <> -

{EpsLogoutStrings.loading}

- +

{EpsLogoutStrings.title}

+

{EpsLogoutStrings.body}

+ {EpsLogoutStrings.login_link} ) : ( <> -

{EpsLogoutStrings.title}

-
{EpsLogoutStrings.body}
-

- - {EpsLogoutStrings.login_link} - +

{EpsLogoutStrings.loading}

+ )}
- ); + ) } diff --git a/packages/cpt-ui/components/EpsRoleSelectionPage.tsx b/packages/cpt-ui/components/EpsRoleSelectionPage.tsx index e6373a9403..03dc919adf 100644 --- a/packages/cpt-ui/components/EpsRoleSelectionPage.tsx +++ b/packages/cpt-ui/components/EpsRoleSelectionPage.tsx @@ -133,12 +133,20 @@ export default function RoleSelectionPage({contentText}: RoleSelectionPageProps) const rolesWithAccess = userInfo.roles_with_access || [] const rolesWithoutAccess = userInfo.roles_without_access || [] - const selectedRole = userInfo?.currently_selected_role - ? { - ...userInfo?.currently_selected_role, - uuid: `selected_role_0` - } - : undefined + // Check if the user info object and currently_selected_role exist + const selectedRole = + userInfo?.currently_selected_role && Object.keys(userInfo.currently_selected_role).length > 0 + ? { + // If currently_selected_role is not empty, spread its properties + ...userInfo.currently_selected_role, + // Add uuid only if the selected role is not an empty object + uuid: `selected_role_0` + } + // If currently_selected_role is an empty object `{}`, set selectedRole to undefined + : undefined + + console.log("Selected role:", selectedRole) + setSelectedRole(selectedRole) // Populate the EPS card props setRolesWithAccess( @@ -158,7 +166,6 @@ export default function RoleSelectionPage({contentText}: RoleSelectionPageProps) })) ) - setSelectedRole(selectedRole) setNoAccess(rolesWithAccess.length === 0) setSingleAccess(rolesWithAccess.length === 1) diff --git a/packages/cpt-ui/components/RBACBanner.tsx b/packages/cpt-ui/components/RBACBanner.tsx index 899f85b7fe..37286ff21b 100644 --- a/packages/cpt-ui/components/RBACBanner.tsx +++ b/packages/cpt-ui/components/RBACBanner.tsx @@ -1,62 +1,67 @@ -"use client"; -import React, { useEffect, useState } from "react"; +"use client" +import React, {useEffect, useState} from "react" -import { Row } from "nhsuk-react-components"; +import {Row} from "nhsuk-react-components" import "@/assets/styles/rbacBanner.scss" -import { RBAC_BANNER_STRINGS } from "@/constants/ui-strings/RBACBannerStrings"; -import { useAccess } from "@/context/AccessProvider"; +import {RBAC_BANNER_STRINGS} from "@/constants/ui-strings/RBACBannerStrings" +import {useAccess} from "@/context/AccessProvider" export default function RBACBanner() { const [bannerText, setBannerText] = useState("") - - const { selectedRole, userDetails } = useAccess(); + const {selectedRole, userDetails} = useAccess() useEffect(() => { - let orgName - if (selectedRole) { - // Locums have a specific ODS code, and need to be shouted out - if (selectedRole.org_code === "FFFFF") { - orgName = RBAC_BANNER_STRINGS.LOCUM_NAME - } else if (selectedRole.org_name) { - orgName = selectedRole.org_name - } + if (!selectedRole || !userDetails) { + console.log("No selected role or user details - hiding RBAC banner.") + return } - setBannerText(RBAC_BANNER_STRINGS.CONFIDENTIAL_DATA - .replace("{lastName}", userDetails?.family_name ?? RBAC_BANNER_STRINGS.NO_FAMILY_NAME) - .replace("{firstName}", userDetails?.given_name ?? RBAC_BANNER_STRINGS.NO_GIVEN_NAME) - .replace("{roleName}", selectedRole?.role_name ?? RBAC_BANNER_STRINGS.NO_ROLE_NAME) - .replace("{orgName}", orgName ?? RBAC_BANNER_STRINGS.NO_ORG_NAME) - .replace("{odsCode}", selectedRole?.org_code ?? RBAC_BANNER_STRINGS.NO_ODS_CODE) + /** + * The RBAC (Role-Based Access Control) User Profile Banner follows these patterns: + * + * Standard User: + * CONFIDENTIAL: PERSONAL PATIENT DATA accessed by LAST NAME, First Name - RBAC Role - Site Name (ODS: ODS) + * + * Locum User (org_code === 'FFFFF'): + * CONFIDENTIAL: PERSONAL PATIENT DATA accessed by LAST NAME, First Name - RBAC Role - Locum pharmacy (ODS: FFFFF) - Site Name (ODS: ODS Code) + */ + + // Determine the organization name (use "Locum pharmacy" for locum users) + const orgName = selectedRole.org_code === "FFFFF" + ? RBAC_BANNER_STRINGS.LOCUM_NAME + : selectedRole.org_name || RBAC_BANNER_STRINGS.NO_ORG_NAME + + // Format the last name in uppercase + const lastName = userDetails.family_name?.toUpperCase() || RBAC_BANNER_STRINGS.NO_FAMILY_NAME + + // Construct the final banner text using template replacement + setBannerText( + RBAC_BANNER_STRINGS.CONFIDENTIAL_DATA + .replace("{lastName}", lastName) + .replace("{firstName}", userDetails.given_name || RBAC_BANNER_STRINGS.NO_GIVEN_NAME) + .replace("{roleName}", selectedRole.role_name || RBAC_BANNER_STRINGS.NO_ROLE_NAME) + .replace("{orgName}", orgName) + .replace("{odsCode}", selectedRole.org_code || RBAC_BANNER_STRINGS.NO_ODS_CODE) ) - }, [ - selectedRole, - userDetails - ]) + }, [selectedRole, userDetails]) - // Render only after the user selects a role. - // This has to come after all the logic, or we get a hydration error! + /** + * 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) { - return (null) + return null } return ( - <> - {/* I can't find a component for a banner, so I've cribbed the CSS from the prototype here. */} -
- -

- {bannerText} -

- -
- - ); +
+ +

+ {bannerText} +

+ +
+ ) } diff --git a/packages/cpt-ui/constants/ui-strings/CardStrings.ts b/packages/cpt-ui/constants/ui-strings/CardStrings.ts index 7e6978deee..919ca57673 100644 --- a/packages/cpt-ui/constants/ui-strings/CardStrings.ts +++ b/packages/cpt-ui/constants/ui-strings/CardStrings.ts @@ -12,8 +12,8 @@ export const SELECT_YOUR_ROLE_PAGE_TEXT = { "You are currently logged in at GREENE'S PHARMACY (ODS: FG419) with Health Professional Access Role." }, confirmButton: { - text: "Confirm and continue to find a prescription", - link: "tracker-presc-no" + text: "Continue to find a prescription", + link: "searchforaprescription" }, alternativeMessage: "Alternatively, you can choose a new role below.", organisation: "Organisation", diff --git a/packages/cpt-ui/constants/ui-strings/ChangeRolePageStrings.ts b/packages/cpt-ui/constants/ui-strings/ChangeRolePageStrings.ts index c31c930950..4133810198 100644 --- a/packages/cpt-ui/constants/ui-strings/ChangeRolePageStrings.ts +++ b/packages/cpt-ui/constants/ui-strings/ChangeRolePageStrings.ts @@ -8,11 +8,11 @@ export const CHANGE_YOUR_ROLE_PAGE_TEXT = { insetText: { visuallyHidden: "Information: ", message: - "You are currently logged in at GREENE'S PHARMACY (ODS: FG419) with Health Professional Access Role." + "You are currently logged in at GREENE'S PHARMACY (ODS: FG419) with Health Professional Access Role." }, confirmButton: { - text: "Confirm and continue to find a prescription", - link: "tracker-presc-no" + text: "Continue to find a prescription", + link: "searchforaprescription" }, alternativeMessage: "Alternatively, you can choose a new role below.", organisation: "Organisation", diff --git a/packages/cpt-ui/constants/ui-strings/YourSelectedRoleStrings.ts b/packages/cpt-ui/constants/ui-strings/YourSelectedRoleStrings.ts index 7502d89e5e..6078861c4d 100644 --- a/packages/cpt-ui/constants/ui-strings/YourSelectedRoleStrings.ts +++ b/packages/cpt-ui/constants/ui-strings/YourSelectedRoleStrings.ts @@ -2,10 +2,10 @@ export const YOUR_SELECTED_ROLE_STRINGS = { heading: "Your selected role", subheading: "The following role is now active.", tableTitle: "Current role Details", - roleLabel:"Role", + roleLabel: "Role", orgLabel: "Organisation", changeLinkText: "Change", - confirmButtonText: "Confirm and continue to find a prescription", + confirmButtonText: "Continue to find a prescription", noODSCode: "NO ODS CODE", noRoleName: "NO ROLE NAME", noOrgName: "NO ORG NAME" diff --git a/packages/cpt-ui/context/AccessProvider.tsx b/packages/cpt-ui/context/AccessProvider.tsx index 02e24d4ee7..dea7b6f1db 100644 --- a/packages/cpt-ui/context/AccessProvider.tsx +++ b/packages/cpt-ui/context/AccessProvider.tsx @@ -31,11 +31,17 @@ export const AccessProvider = ({children}: {children: ReactNode}) => { const auth = useContext(AuthContext) const clear = () => { - console.warn("Clearing access context.") + console.log("Clearing access context and local storage...") setNoAccess(false) setSingleAccess(false) setSelectedRole(undefined) setUserDetails(undefined) + + // Clear from localStorage to ensure RBAC Banner is removed + localStorage.removeItem("access") + localStorage.removeItem("selectedRole") + localStorage.removeItem("userDetails") + console.log("Local storage cleared.") } diff --git a/packages/selectedRoleLambda/package.json b/packages/selectedRoleLambda/package.json index d6d65583de..c19490d451 100644 --- a/packages/selectedRoleLambda/package.json +++ b/packages/selectedRoleLambda/package.json @@ -1,7 +1,7 @@ { - "name": "selected-role-lambda", + "name": "selectedRoleLambda", "version": "1.0.0", - "description": "Lambda to handle selectedRole endpoint", + "description": "Lambda function handling user role selection in the Clinical Prescription Tracking Service", "main": "handler.ts", "author": "NHS Digital", "license": "MIT", diff --git a/packages/selectedRoleLambda/src/handler.ts b/packages/selectedRoleLambda/src/handler.ts index a4429cb9b9..7dd7bef678 100644 --- a/packages/selectedRoleLambda/src/handler.ts +++ b/packages/selectedRoleLambda/src/handler.ts @@ -7,9 +7,9 @@ import middy from "@middy/core" import inputOutputLogger from "@middy/input-output-logger" import {MiddyErrorHandler} from "@cpt-ui-common/middyErrorHandler" import {getUsernameFromEvent} from "@cpt-ui-common/authFunctions" -import {updateDynamoTable} from "./selectedRoleHelpers" +import {updateDynamoTable, fetchUserRolesFromDynamoDB} from "./selectedRoleHelpers" -/* +/** * Lambda function for updating the selected role in the DynamoDB table. * This function handles incoming API Gateway requests, extracts the username, * parses the request body, and updates the user's role in the database. @@ -26,16 +26,13 @@ const documentClient = DynamoDBDocumentClient.from(dynamoClient) const tokenMappingTableName = process.env["TokenMappingTableName"] ?? "" // Default error response body for internal system errors -const errorResponseBody = { - message: "A system error has occurred" -} +const errorResponseBody = {message: "A system error has occurred"} // Custom error handler for handling unexpected errors in the Lambda function const middyErrorHandler = new MiddyErrorHandler(errorResponseBody) /** - * The main handler function for processing API Gateway events. - * Handles parsing, validation, and updates the selected role in the DynamoDB table. + * Lambda function handler for updating a user's selected role. */ const lambdaHandler = async (event: APIGatewayProxyEvent): Promise => { logger.appendKeys({"apigw-request-id": event.requestContext?.requestId}) @@ -46,7 +43,7 @@ const lambdaHandler = async (event: APIGatewayProxyEvent): Promise role.role_id === userSelectedRoleId) + + // Log extracted role details + logger.info("Extracted role data", { + username, + rolesWithAccessCount: rolesWithAccess.length, + rolesWithAccess: rolesWithAccess.map(role => ({ + role_id: role.role_id, + role_name: role.role_name, + org_code: role.org_code + })), + previousSelectedRole: currentSelectedRole + ? { + role_id: currentSelectedRole.role_id, + role_name: currentSelectedRole.role_name, + org_code: currentSelectedRole.org_code + } + : "No previous role selected", + newSelectedRole: newSelectedRole + ? { + role_id: newSelectedRole.role_id, + role_name: newSelectedRole.role_name, + org_code: newSelectedRole.org_code + } + : "Role not found in rolesWithAccess" + }) - // Call helper function to update the selected role in the database - await updateDynamoTable(username, userInfoSelectedRole, documentClient, logger, tokenMappingTableName) + // Construct updated roles list + const updatedRolesWithAccess = [ + ...rolesWithAccess.filter(role => role.role_id !== userSelectedRoleId), // Remove the new selected role + + // Move the previously selected role back into rolesWithAccess, but only if it was set + ...(currentSelectedRole && Object.keys(currentSelectedRole).length > 0 + ? [currentSelectedRole] + : []) + ] + + logger.info("Updated roles list before database update", { + username, + newSelectedRole: newSelectedRole + ? { + role_id: newSelectedRole.role_id, + role_name: newSelectedRole.role_name, + org_code: newSelectedRole.org_code + } + : "No role selected", + returningRoleToAccessList: currentSelectedRole + ? { + role_id: currentSelectedRole.role_id, + role_name: currentSelectedRole.role_name, + org_code: currentSelectedRole.org_code + } + : "No previous role to return", + updatedRolesWithAccessCount: updatedRolesWithAccess.length, + updatedRolesWithAccess: updatedRolesWithAccess.map(role => ({ + role_id: role.role_id, + role_name: role.role_name, + org_code: role.org_code + })) + }) + + // Prepare the updated user info to be stored in DynamoDB + const updatedUserInfo = { + currentlySelectedRole: newSelectedRole || undefined, // If no role is found, store `undefined` + rolesWithAccess: updatedRolesWithAccess, + selectedRoleId: userSelectedRoleId + } + + logger.info("Updating user role in DynamoDB", { + username, + updatedUserInfo + }) + + // Persist changes to DynamoDB + await updateDynamoTable(username, updatedUserInfo, documentClient, logger, tokenMappingTableName) return { statusCode: 200, body: JSON.stringify({ message: "Selected role data has been updated successfully", - userInfo: userInfoSelectedRole + userInfo: updatedUserInfo }) } - } export const handler = middy(lambdaHandler) diff --git a/packages/selectedRoleLambda/src/selectedRoleHelpers.ts b/packages/selectedRoleLambda/src/selectedRoleHelpers.ts index df8cf2c2c6..d5e40ec7cf 100644 --- a/packages/selectedRoleLambda/src/selectedRoleHelpers.ts +++ b/packages/selectedRoleLambda/src/selectedRoleHelpers.ts @@ -1,57 +1,60 @@ import {Logger} from "@aws-lambda-powertools/logger" -import {DynamoDBDocumentClient, UpdateCommand} from "@aws-sdk/lib-dynamodb" -import {RoleDetails, TrackerUserInfo} from "./selectedRoleTypes" +import {DynamoDBDocumentClient, GetCommand, UpdateCommand} from "@aws-sdk/lib-dynamodb" +import {RoleDetails, SelectedRole} from "./selectedRoleTypes" /** - * Update the user currentlySelectedRole and selectedRoleId in the DynamoDB table. - * @param username - The username of the user. - * @param data - The TrackerUserInfo object containing user role information. - * @param documentClient - The DynamoDBDocumentClient instance. - * @param logger - The Logger instance for logging. - * @param tokenMappingTableName - The name of the DynamoDB table. + * **Updates the user's selected role in DynamoDB** + * + * - Stores the `currentlySelectedRole` and `selectedRoleId` in the table. + * - Removes the selected role from `rolesWithAccess` to avoid duplication. */ export const updateDynamoTable = async ( username: string, - data: TrackerUserInfo, + updatedUserInfo: SelectedRole, documentClient: DynamoDBDocumentClient, logger: Logger, tokenMappingTableName: string ) => { - // Check if the token mapping table name is provided + // Validate if the table name is provided in environment variables if (!tokenMappingTableName) { - logger.error("Token mapping table name not set") + logger.error("Token mapping table name is not set.") throw new Error("Token mapping table name not set") } logger.debug("Starting DynamoDB update process", { username, table: tokenMappingTableName, - receivedData: data + receivedData: updatedUserInfo }) - // DyanamoDB cannot allow undefined values. We need to scrub any undefined values from the data objects - const currentlySelectedRole: RoleDetails = data.currently_selected_role ? data.currently_selected_role : {} + // Ensure no undefined values are stored in DynamoDB + const currentlySelectedRole: RoleDetails = updatedUserInfo.currentlySelectedRole || {} + const rolesWithAccess: Array = updatedUserInfo.rolesWithAccess || [] + const selectedRoleId: string = updatedUserInfo.selectedRoleId || "" - // Ensure selectedRoleId is never undefined by providing a fallback value - const selectedRoleId: string = currentlySelectedRole.role_id || "UNSPECIFIED_ROLE_ID" - - // Since RoleDetails has a bunch of possibly undefined fields, we need to scrub those out. - // Convert everything to strings, then convert back to a generic object. + // Remove `undefined` properties from the objects before updating const scrubbedCurrentlySelectedRole = JSON.parse(JSON.stringify(currentlySelectedRole)) + const scrubbedRolesWithAccess = rolesWithAccess.map((role) => JSON.parse(JSON.stringify(role))) - logger.info("Prepared data for DynamoDB update", { + logger.info("Prepared role data for DynamoDB update", { currentlySelectedRole: scrubbedCurrentlySelectedRole, + rolesWithAccess: scrubbedRolesWithAccess, selectedRoleId }) try { - // Create the update command for DynamoDB + // Construct the UpdateCommand to modify user role data in DynamoDB const updateCommand = new UpdateCommand({ TableName: tokenMappingTableName, Key: {username}, - UpdateExpression: "SET currentlySelectedRole = :currentlySelectedRole, selectedRoleId = :selectedRoleId", + UpdateExpression: ` + SET currentlySelectedRole = :currentlySelectedRole, + selectedRoleId = :selectedRoleId, + rolesWithAccess = :rolesWithAccess + `, ExpressionAttributeValues: { ":currentlySelectedRole": scrubbedCurrentlySelectedRole, + ":rolesWithAccess": scrubbedRolesWithAccess, ":selectedRoleId": selectedRoleId }, ReturnValues: "ALL_NEW" @@ -59,9 +62,8 @@ export const updateDynamoTable = async ( logger.debug("Executing DynamoDB update command", {updateCommand}) - // Send the update command to DynamoDB + // Execute the update operation const response = await documentClient.send(updateCommand) - logger.info("DynamoDB update successful", {response}) } catch (error) { @@ -71,12 +73,59 @@ export const updateDynamoTable = async ( errorMessage: error.message, errorStack: error.stack }) + throw error // Ensure the function actually throws the error } else { logger.error("Unknown error type while updating user's selected role", { username, - error + error: JSON.stringify(error) // Ensure the error is stringified }) + throw new Error("Unexpected error occurred while updating user's selected role") } - throw error + } +} + +/** + * **Fetches user roles from DynamoDB** + * + * - Retrieves `rolesWithAccess` (available roles for the user). + * - Retrieves `currentlySelectedRole` (the role currently chosen by the user). + * - Ensures both fields are always returned, even if no data exists. + */ +export const fetchUserRolesFromDynamoDB = async ( + username: string, + documentClient: DynamoDBDocumentClient, + logger: Logger, + tokenMappingTableName: string +): Promise => { + logger.info("Fetching user role information from DynamoDB", {username}) + + try { + // Execute a GetCommand to fetch user role details + const response = await documentClient.send( + new GetCommand({ + TableName: tokenMappingTableName, + Key: {username} + }) + ) + + // Handle case where no user data is found + if (!response.Item) { + logger.warn("No user info found in DynamoDB", {username}) + return {rolesWithAccess: [], currentlySelectedRole: undefined} // Ensure both fields are always returned + } + + // Extract rolesWithAccess and currentlySelectedRole safely + const retrievedRolesWithAccess: SelectedRole = { + rolesWithAccess: response.Item.rolesWithAccess || [], + currentlySelectedRole: response.Item.currentlySelectedRole || undefined // Retrieve currentlySelectedRole + } + + logger.info("Roles and selected role successfully retrieved from DynamoDB", { + data: retrievedRolesWithAccess + }) + return retrievedRolesWithAccess + } catch (error) { + logger.error("Error fetching user info from DynamoDB", {error}) + throw new Error("Failed to retrieve user info from cache") } } diff --git a/packages/selectedRoleLambda/src/selectedRoleTypes.ts b/packages/selectedRoleLambda/src/selectedRoleTypes.ts index 7abafb4729..392bc8f9bc 100644 --- a/packages/selectedRoleLambda/src/selectedRoleTypes.ts +++ b/packages/selectedRoleLambda/src/selectedRoleTypes.ts @@ -8,8 +8,8 @@ export type RoleDetails = { uuid?: string } -export type TrackerUserInfo = { - roles_with_access: Array - roles_without_access: Array - currently_selected_role?: RoleDetails +export type SelectedRole = { + rolesWithAccess: Array + currentlySelectedRole?: RoleDetails + selectedRoleId?: string } diff --git a/packages/selectedRoleLambda/tests/test_handler.mockDisabled.test.ts b/packages/selectedRoleLambda/tests/test_handler.mockDisabled.test.ts deleted file mode 100644 index 2df4ebc03d..0000000000 --- a/packages/selectedRoleLambda/tests/test_handler.mockDisabled.test.ts +++ /dev/null @@ -1,68 +0,0 @@ -import {jest} from "@jest/globals" - -// Mock environment variables -process.env.MOCK_MODE_ENABLED = "false" - -// Mocked functions -const mockGetUsernameFromEvent = jest.fn() -const mockUpdateDynamoTable = jest.fn() - -jest.unstable_mockModule("@cpt-ui-common/authFunctions", () => { - return { - getUsernameFromEvent: mockGetUsernameFromEvent.mockImplementation(() => "Primary_JoeBloggs") - } -}) - -jest.unstable_mockModule("@/selectedRoleHelpers", () => { - return { - updateDynamoTable: mockUpdateDynamoTable.mockImplementation(() => Promise.resolve()) - } -}) - -const {handler} = await import("@/handler") -import {mockContext, mockAPIGatewayProxyEvent} from "./mockObjects" - -describe("Lambda Handler Tests with mock disabled", () => { - let event = {...mockAPIGatewayProxyEvent, body: JSON.stringify({role_id: "123", org_code: "XYZ"})} - let context = {...mockContext} - - beforeEach(() => { - jest.clearAllMocks() - }) - - it("should return a successful response when called normally", async () => { - const response = await handler(event, context) - - expect(mockGetUsernameFromEvent).toHaveBeenCalled() - expect(mockUpdateDynamoTable).toHaveBeenCalledWith( - "Primary_JoeBloggs", - {role_id: "123", org_code: "XYZ"}, - expect.any(Object), - expect.any(Object), - expect.any(String) - ) - expect(response).toEqual({ - statusCode: 200, - body: JSON.stringify({ - message: "Selected role data has been updated successfully", - userInfo: JSON.parse(event.body) - }) - }) - }) - - it("should return 400 when request body is missing", async () => { - event.body = "" as unknown as string - const response = await handler(event, context) - - expect(response.statusCode).toBe(400) - expect(response.body).toContain("Request body is required") - }) - - it("should return 400 when request body is invalid JSON", async () => { - event.body = "Invalid JSON" - const response = await handler(event, context) - - expect(response.statusCode).toBe(400) - expect(response.body).toContain("Invalid JSON format in request body") - }) -}) diff --git a/packages/selectedRoleLambda/tests/test_handler.mockEnabled.test.ts b/packages/selectedRoleLambda/tests/test_handler.mockEnabled.test.ts deleted file mode 100644 index e1891e950e..0000000000 --- a/packages/selectedRoleLambda/tests/test_handler.mockEnabled.test.ts +++ /dev/null @@ -1,65 +0,0 @@ -import {jest} from "@jest/globals" - -// Mocked functions -const mockGetUsernameFromEvent = jest.fn() -const mockUpdateDynamoTable = jest.fn() - -jest.unstable_mockModule("@cpt-ui-common/authFunctions", () => { - return { - getUsernameFromEvent: mockGetUsernameFromEvent.mockImplementation(() => "Mock_JoeBloggs") - } -}) - -jest.unstable_mockModule("@/selectedRoleHelpers", () => { - return { - updateDynamoTable: mockUpdateDynamoTable.mockImplementation(() => Promise.resolve()) - } -}) - -const {handler} = await import("@/handler") -import {mockContext, mockAPIGatewayProxyEvent} from "./mockObjects" - -describe("Lambda Handler Tests with mock enabled", () => { - let event = {...mockAPIGatewayProxyEvent, body: JSON.stringify({role_id: "123", org_code: "XYZ"})} - let context = {...mockContext} - - beforeEach(() => { - jest.clearAllMocks() - }) - - it("should return a successful response with mock username", async () => { - const response = await handler(event, context) - - expect(mockGetUsernameFromEvent).toHaveBeenCalled() - expect(mockUpdateDynamoTable).toHaveBeenCalledWith( - "Mock_JoeBloggs", - {role_id: "123", org_code: "XYZ"}, - expect.any(Object), - expect.any(Object), - expect.any(String) - ) - expect(response).toEqual({ - statusCode: 200, - body: JSON.stringify({ - message: "Selected role data has been updated successfully", - userInfo: JSON.parse(event.body) - }) - }) - }) - - it("should return 400 when request body is missing", async () => { - event.body = "" as unknown as string - const response = await handler(event, context) - - expect(response.statusCode).toBe(400) - expect(response.body).toContain("Request body is required") - }) - - it("should return 400 when request body is invalid JSON", async () => { - event.body = "Invalid JSON" - const response = await handler(event, context) - - expect(response.statusCode).toBe(400) - expect(response.body).toContain("Invalid JSON format in request body") - }) -}) diff --git a/packages/selectedRoleLambda/tests/test_handler.test.ts b/packages/selectedRoleLambda/tests/test_handler.test.ts new file mode 100644 index 0000000000..05fc9b02a6 --- /dev/null +++ b/packages/selectedRoleLambda/tests/test_handler.test.ts @@ -0,0 +1,348 @@ +import {jest} from "@jest/globals" + +// Mocked functions from authFunctions +const mockGetUsernameFromEvent = jest.fn() + +jest.unstable_mockModule("@cpt-ui-common/authFunctions", () => { + const getUsernameFromEvent = mockGetUsernameFromEvent.mockImplementation(() => "Mock_JoeBloggs") + + return { + getUsernameFromEvent + } +}) + +// Mocked functions from selectedRoleHelpers +const mockFetchUserRolesFromDynamoDB = jest.fn() +const mockUpdateDynamoTable = jest.fn() + +jest.unstable_mockModule("@/selectedRoleHelpers", () => { + const fetchUserRolesFromDynamoDB = mockFetchUserRolesFromDynamoDB.mockImplementation(() => { + return { + rolesWithAccess: [ + {role_id: "123", org_code: "XYZ", role_name: "MockRole_1"}, + {role_id: "456", org_code: "ABC", role_name: "MockRole_2"} + ], + currentlySelectedRole: undefined // Initially no role is selected + } + }) + + const updateDynamoTable = mockUpdateDynamoTable.mockImplementation(() => {}) + + return { + fetchUserRolesFromDynamoDB, + updateDynamoTable + } +}) + +const {handler} = await import("@/handler") +import {mockContext, mockAPIGatewayProxyEvent} from "./mockObjects" +import {Logger} from "@aws-lambda-powertools/logger" + +describe("Lambda Handler Tests", () => { + let event = { + ...mockAPIGatewayProxyEvent, + body: JSON.stringify({ + currently_selected_role: { + role_id: "123", + org_code: "XYZ", + role_name: "MockRole_1" + } + }) + } + let context = {...mockContext} + + beforeAll(() => { + jest.clearAllMocks() + }) + + it("should return a successful response when called", async () => { + const response = await handler(event, context) + expect(mockUpdateDynamoTable).toHaveBeenCalled() + + expect(response).toBeDefined() + expect(response).toHaveProperty("statusCode", 200) + expect(response).toHaveProperty("body") + + const body = JSON.parse(response.body) + expect(body).toHaveProperty("message", "Selected role data has been updated successfully") + expect(body).toHaveProperty("userInfo") + }) + + it( + "should call updateDynamoTable and move the selected role from rolesWithAccess to currentlySelectedRole", + async () => { + const testUsername = "Mock_JoeBloggs" + const updatedUserInfo = { + // The selected role has been moved from rolesWithAccess to currentlySelectedRole + currentlySelectedRole: { + role_id: "123", + org_code: "XYZ", + role_name: "MockRole_1" + }, + rolesWithAccess: [ + { + role_id: "456", + org_code: "ABC", + role_name: "MockRole_2" + } + ], + selectedRoleId: "123" + } + + const response = await handler(event, context) + + expect(mockGetUsernameFromEvent).toHaveBeenCalled() + expect(mockUpdateDynamoTable).toHaveBeenCalledWith( + testUsername, + updatedUserInfo, + expect.any(Object), + expect.any(Object), + expect.any(String) + ) + expect(response).toEqual({ + statusCode: 200, + body: JSON.stringify({ + message: "Selected role data has been updated successfully", + userInfo: { + currentlySelectedRole: { + role_id: "123", + org_code: "XYZ", + role_name: "MockRole_1" + }, + rolesWithAccess: [ + { + role_id: "456", + org_code: "ABC", + role_name: "MockRole_2" + } + ], + selectedRoleId: "123" + } + }) + }) + }) + + it( + "should swap currentlySelectedRole with the new selected role and move the old one back to rolesWithAccess", + async () => { + // Initial rolesWithAccess contains multiple roles, currentlySelectedRole is undefined + mockFetchUserRolesFromDynamoDB.mockImplementation(() => { + return { + rolesWithAccess: [ + {role_id: "123", org_code: "XYZ", role_name: "MockRole_1"}, + {role_id: "456", org_code: "ABC", role_name: "MockRole_2"}, + {role_id: "789", org_code: "DEF", role_name: "MockRole_3"} + ], + currentlySelectedRole: undefined // Initially no role is selected + } + }) + + // User selects "MockRole_1" (role_id: 123) + let event = { + ...mockAPIGatewayProxyEvent, + body: JSON.stringify({ + currently_selected_role: { + role_id: "123", + org_code: "XYZ", + role_name: "MockRole_1" + } + }) + } + + let response = await handler(event, mockContext) + let firstResponseBody = JSON.parse(response.body) + + expect(firstResponseBody.userInfo).toEqual({ + currentlySelectedRole: {role_id: "123", org_code: "XYZ", role_name: "MockRole_1"}, + rolesWithAccess: [ + {role_id: "456", org_code: "ABC", role_name: "MockRole_2"}, + {role_id: "789", org_code: "DEF", role_name: "MockRole_3"} + ], + selectedRoleId: "123" + }) + + // User selects "MockRole_2" (role_id: 456) + event = { + ...mockAPIGatewayProxyEvent, + body: JSON.stringify({ + currently_selected_role: { + role_id: "456", + org_code: "ABC", + role_name: "MockRole_2" + } + }) + } + + response = await handler(event, mockContext) + let secondResponseBody = JSON.parse(response.body) + + expect(secondResponseBody.userInfo).toEqual({ + currentlySelectedRole: {role_id: "456", org_code: "ABC", role_name: "MockRole_2"}, + rolesWithAccess: [ + {role_id: "123", org_code: "XYZ", role_name: "MockRole_1"}, // Previously selected role moved back + {role_id: "789", org_code: "DEF", role_name: "MockRole_3"} + ], + selectedRoleId: "456" + }) + + // User selects "MockRole_3" (role_id: 789) + event = { + ...mockAPIGatewayProxyEvent, + body: JSON.stringify({ + currently_selected_role: { + role_id: "789", + org_code: "DEF", + role_name: "MockRole_3" + } + }) + } + + response = await handler(event, mockContext) + let thirdResponseBody = JSON.parse(response.body) + + expect(thirdResponseBody.userInfo).toEqual({ + currentlySelectedRole: {role_id: "789", org_code: "DEF", role_name: "MockRole_3"}, + rolesWithAccess: [ + {role_id: "123", org_code: "XYZ", role_name: "MockRole_1"}, + {role_id: "456", org_code: "ABC", role_name: "MockRole_2"} // Previously selected role moved back + ], + selectedRoleId: "789" + }) + } + ) + + it( + "should swap initially selected role with the new selected role and move the old one back to rolesWithAccess", + async () => { + // Initial database state with a previously selected role + mockFetchUserRolesFromDynamoDB.mockImplementation(() => { + return { + rolesWithAccess: [ + {role_id: "123", org_code: "XYZ", role_name: "MockRole_1"}, + {role_id: "456", org_code: "ABC", role_name: "MockRole_2"}, + {role_id: "789", org_code: "DEF", role_name: "MockRole_3"} + ], + currentlySelectedRole: {role_id: "555", org_code: "GHI", role_name: "MockRole_4"} // Initially selected role + } + }) + + // First role selection: Change to "MockRole_1" + const firstEvent = { + ...mockAPIGatewayProxyEvent, + body: JSON.stringify({ + currently_selected_role: { + role_id: "123", + org_code: "XYZ", + role_name: "MockRole_1" + } + }) + } + + const firstResponse = await handler(firstEvent, mockContext) + const firstResponseBody = JSON.parse(firstResponse.body) + + expect(mockUpdateDynamoTable).toHaveBeenCalledWith( + "Mock_JoeBloggs", + { + currentlySelectedRole: { + role_id: "123", + org_code: "XYZ", + role_name: "MockRole_1" + }, + rolesWithAccess: [ + {role_id: "456", org_code: "ABC", role_name: "MockRole_2"}, + {role_id: "789", org_code: "DEF", role_name: "MockRole_3"}, + {role_id: "555", org_code: "GHI", role_name: "MockRole_4"} // Old role moved back + ], + selectedRoleId: "123" + }, + expect.any(Object), + expect.any(Object), + expect.any(String) + ) + + // Now simulate switching to another role + mockFetchUserRolesFromDynamoDB.mockImplementation(() => firstResponseBody.userInfo) + + const secondEvent = { + ...mockAPIGatewayProxyEvent, + body: JSON.stringify({ + currently_selected_role: { + role_id: "789", + org_code: "DEF", + role_name: "MockRole_3" + } + }) + } + + const secondResponse = await handler(secondEvent, mockContext) + const secondResponseBody = JSON.parse(secondResponse.body) + + expect(secondResponseBody).toEqual({ + message: "Selected role data has been updated successfully", + userInfo: { + currentlySelectedRole: { + role_id: "789", + org_code: "DEF", + role_name: "MockRole_3" + }, + rolesWithAccess: [ + {role_id: "456", org_code: "ABC", role_name: "MockRole_2"}, + {role_id: "555", org_code: "GHI", role_name: "MockRole_4"}, + {role_id: "123", org_code: "XYZ", role_name: "MockRole_1"} // Previously selected role moved back + ], + selectedRoleId: "789" + } + }) + }) + + it("should return 500 and log error when updateDynamoTable throws an error", async () => { + const error = new Error("Dynamo update failed") + const loggerSpy = jest.spyOn(Logger.prototype, "error") + mockUpdateDynamoTable.mockImplementation(() => { + throw error + }) + + const response = await handler(event, context) + expect(response).toMatchObject({ + message: "A system error has occurred" + }) + expect(loggerSpy).toHaveBeenCalledWith( + expect.any(Object), + "Error: Dynamo update failed" + ) + }) + + it("should handle unexpected error types gracefully", async () => { + mockUpdateDynamoTable.mockImplementation(() => { + throw new Error("Unexpected error string") + }) + const loggerSpy = jest.spyOn(Logger.prototype, "error") + + const response = await handler(event, context) + expect(response).toMatchObject({ + message: "A system error has occurred" + }) + expect(loggerSpy).toHaveBeenCalledWith( + expect.any(Object), + "Error: Unexpected error string" + ) + }) + + it("should return 400 when request body is missing", async () => { + event.body = "" as unknown as string + const response = await handler(event, context) + + expect(response.statusCode).toBe(400) + expect(response.body).toContain("Request body is required") + }) + + it("should return 400 when request body is invalid JSON", async () => { + event.body = "Invalid JSON" + const response = await handler(event, context) + + expect(response.statusCode).toBe(400) + expect(response.body).toContain("Invalid JSON format in request body") + }) + +}) diff --git a/packages/selectedRoleLambda/tests/test_selectedRoleHelpers.test.ts b/packages/selectedRoleLambda/tests/test_selectedRoleHelpers.test.ts index 055a6b4df8..c88fb9ac3c 100644 --- a/packages/selectedRoleLambda/tests/test_selectedRoleHelpers.test.ts +++ b/packages/selectedRoleLambda/tests/test_selectedRoleHelpers.test.ts @@ -1,13 +1,14 @@ import {jest} from "@jest/globals" import {Logger} from "@aws-lambda-powertools/logger" import {DynamoDBClient} from "@aws-sdk/client-dynamodb" -import {DynamoDBDocumentClient} from "@aws-sdk/lib-dynamodb" -import {updateDynamoTable} from "@/selectedRoleHelpers" -import {TrackerUserInfo} from "@/selectedRoleTypes" +import {DynamoDBDocumentClient, GetCommand, UpdateCommand} from "@aws-sdk/lib-dynamodb" +import {updateDynamoTable, fetchUserRolesFromDynamoDB} from "@/selectedRoleHelpers" +import {SelectedRole} from "@/selectedRoleTypes" // Mock Logger const logger = new Logger() jest.spyOn(logger, "info") +jest.spyOn(logger, "warn") jest.spyOn(logger, "error") // Properly initializing the DynamoDB client @@ -18,8 +19,8 @@ jest.spyOn(dynamoDBClient, "send").mockImplementation(() => Promise.resolve({})) const username = "testUser" const tokenMappingTableName = "dummyTable" -const mockRoleData: TrackerUserInfo = { - roles_with_access: [ +const mockRoleData: SelectedRole = { + rolesWithAccess: [ { role_name: "Doctor", role_id: "123", @@ -27,31 +28,15 @@ const mockRoleData: TrackerUserInfo = { org_name: "Test Hospital" } ], - roles_without_access: [], - currently_selected_role: { + currentlySelectedRole: { role_name: "Doctor", role_id: "123", org_code: "ABC", org_name: "Test Hospital" - } + }, + selectedRoleId: "123" } -// Mock OIDC Token Verification -const mockVerifyIdToken = jest.fn() - -jest.unstable_mockModule("@cpt-ui-common/authFunctions", async () => { - const verifyIdToken = mockVerifyIdToken.mockImplementation(async () => { - return { - selected_roleid: "123" - } - }) - - return { - __esModule: true, - verifyIdToken: verifyIdToken - } -}) - describe("updateDynamoTable", () => { beforeEach(() => { jest.clearAllMocks() @@ -59,6 +44,38 @@ describe("updateDynamoTable", () => { it("should update DynamoDB with user selected role successfully", async () => { await updateDynamoTable(username, mockRoleData, dynamoDBClient, logger, tokenMappingTableName) + + expect(dynamoDBClient.send).toHaveBeenCalledWith(expect.any(UpdateCommand)) + expect(logger.info).toHaveBeenCalledWith( + expect.stringContaining("DynamoDB update successful"), + expect.any(Object) + ) + }) + + it("should ensure no undefined values are stored in DynamoDB", async () => { + const incompleteRoleData: SelectedRole = { + // Ensuring an empty array instead of undefined + rolesWithAccess: undefined as unknown as [], + // Ensuring an empty object instead of undefined + currentlySelectedRole: undefined as unknown as Record, + // Ensuring an empty string instead of undefined + selectedRoleId: undefined as unknown as string + } + + // Call the function with incomplete role data + await updateDynamoTable(username, incompleteRoleData, dynamoDBClient, logger, tokenMappingTableName) + + // Verify that the logger captured the correct transformation of data + expect(logger.info).toHaveBeenCalledWith( + expect.stringContaining("Prepared role data for DynamoDB update"), + expect.objectContaining({ + currentlySelectedRole: {}, // Should be set to an empty object + rolesWithAccess: [], // Should be set to an empty array + selectedRoleId: "" // Should be set to an empty string + }) + ) + + // Verify that the function actually sent the update command to DynamoDB expect(dynamoDBClient.send).toHaveBeenCalled() }) @@ -86,4 +103,79 @@ describe("updateDynamoTable", () => { }) ) }) + + it("should handle unknown errors gracefully", async () => { + const unexpectedError = {customMessage: "Unexpected error"} // Simulating a non-Error object + jest.spyOn(dynamoDBClient, "send").mockRejectedValueOnce(unexpectedError as never) + + await expect( + updateDynamoTable(username, mockRoleData, dynamoDBClient, logger, tokenMappingTableName) + ).rejects.toThrow("Unexpected error occurred while updating user's selected role") // Expect our new custom error + + expect(logger.error).toHaveBeenCalledWith( + "Unknown error type while updating user's selected role", + expect.objectContaining({ + username, + error: JSON.stringify(unexpectedError) + }) + ) + }) +}) + +describe("fetchUserRolesFromDynamoDB", () => { + beforeEach(() => { + jest.clearAllMocks() + jest.spyOn(logger, "info").mockImplementation(() => {}) + jest.spyOn(logger, "warn").mockImplementation(() => {}) + jest.spyOn(logger, "error").mockImplementation(() => {}) + }) + + it("should successfully fetch roles with access from DynamoDB", async () => { + // Mock a successful DynamoDB response + const mockDynamoResponse = { + Item: { + rolesWithAccess: [ + {role_name: "Nurse", role_id: "456", org_code: "XYZ", org_name: "Clinic A"}, + {role_name: "Surgeon", role_id: "789", org_code: "DEF", org_name: "Hospital B"} + ] + } + } + + jest.spyOn(dynamoDBClient, "send").mockResolvedValueOnce(mockDynamoResponse as never) + + const result = await fetchUserRolesFromDynamoDB(username, dynamoDBClient, logger, tokenMappingTableName) + + expect(dynamoDBClient.send).toHaveBeenCalledWith(expect.any(GetCommand)) + expect(logger.info).toHaveBeenCalledWith( + "Roles and selected role successfully retrieved from DynamoDB", + expect.objectContaining({data: expect.any(Object)}) + ) + + expect(result).toEqual({ + rolesWithAccess: mockDynamoResponse.Item.rolesWithAccess + }) + }) + + it("should return an empty array if no user data is found in DynamoDB", async () => { + const mockEmptyResponse: {Item: undefined} = {Item: undefined} + jest.spyOn(dynamoDBClient, "send").mockResolvedValueOnce(mockEmptyResponse as never) + + const result = await fetchUserRolesFromDynamoDB(username, dynamoDBClient, logger, tokenMappingTableName) + + expect(logger.warn).toHaveBeenCalledWith("No user info found in DynamoDB", {username}) + expect(result).toEqual({rolesWithAccess: []}) + }) + + it("should throw an error if DynamoDB request fails", async () => { + jest.spyOn(dynamoDBClient, "send").mockRejectedValueOnce(new Error("DynamoDB fetch error") as never) + + await expect( + fetchUserRolesFromDynamoDB(username, dynamoDBClient, logger, tokenMappingTableName) + ).rejects.toThrow("Failed to retrieve user info from cache") + + expect(logger.error).toHaveBeenCalledWith( + "Error fetching user info from DynamoDB", + expect.objectContaining({error: expect.any(Error)}) + ) + }) }) diff --git a/packages/trackerUserInfoLambda/src/handler.ts b/packages/trackerUserInfoLambda/src/handler.ts index b7ce468fbd..ebabf295f0 100644 --- a/packages/trackerUserInfoLambda/src/handler.ts +++ b/packages/trackerUserInfoLambda/src/handler.ts @@ -66,7 +66,7 @@ const lambdaHandler = async (event: APIGatewayProxyEvent): Promise { if (!roleString) { return undefined @@ -19,13 +23,16 @@ export const removeRoleCategories = (roleString: string | undefined) => { return chunk.replace(/"/g, "") } -// Fetch user info from the OIDC UserInfo endpoint -// The access token is used to identify the user, and fetch their roles. -// This populates three lists: -// - rolesWithAccess: roles that have access to the CPT -// - rolesWithoutAccess: roles that don't have access to the CPT -// - [OPTIONAL] currentlySelectedRole: the role that is currently selected by the user -// Each list contains information on the roles, such as the role name, role ID, ODS code, and organization name. +/** + * **Fetches user information from the OIDC UserInfo endpoint.** + * + * - Uses the provided access token to retrieve user roles and details. + * - Organizes roles into three categories: + * - `rolesWithAccess`: Roles that have CPT access. + * - `rolesWithoutAccess`: Roles that do not have CPT access. + * - `currentlySelectedRole`: The user's currently selected role, if applicable. + * - Extracts basic user details (`family_name` and `given_name`). + */ export const fetchUserInfo = async ( cis2AccessToken: string, cis2IdToken: string, @@ -64,6 +71,7 @@ export const fetchUserInfo = async ( const rolesWithoutAccess: Array = [] let currentlySelectedRole: RoleDetails | undefined = undefined + // Extract user details const userDetails: UserDetails = { family_name: data.family_name, given_name: data.given_name @@ -72,10 +80,19 @@ export const fetchUserInfo = async ( // Get roles from the user info response const roles = data.nhsid_nrbac_roles || [] + /** + * Processes user roles to determine their access level and selection status. + * + * - If a role has access (`hasAccess`), it is added to `rolesWithAccess`. + * - If a role does not have access, it is added to `rolesWithoutAccess`. + * - If a role has access and matches the `selectedRoleId`, it is set as `currentlySelectedRole`. + * + */ roles.forEach((role) => { logger.debug("Processing role", {role}) - const activityCodes = role.activity_codes || [] + // Extract activity codes and check if any match the accepted access codes + const activityCodes = role.activity_codes || [] const hasAccess = activityCodes.some((code: string) => accepted_access_codes.includes(code)) logger.debug("Role CPT access?", {hasAccess}) @@ -86,7 +103,7 @@ export const fetchUserInfo = async ( org_name: getOrgNameFromOrgCode(data, role.org_code, logger) } - // Ensure the role has at least one of the required fields + // Ensure the role has at least one of the required fields to be processed if (!(roleInfo.role_name || roleInfo.role_id || roleInfo.org_code || roleInfo.org_name)) { // Skip roles that don't meet the minimum field requirements logger.warn("Role does not meet minimum field requirements", {roleInfo}) @@ -94,24 +111,19 @@ export const fetchUserInfo = async ( } if (hasAccess) { - rolesWithAccess.push(roleInfo) - logger.debug("Role has access; adding to rolesWithAccess", {roleInfo}) - } else { - rolesWithoutAccess.push(roleInfo) - logger.debug("Role does not have access; adding to rolesWithoutAccess", {roleInfo}) - } - - // Determine the currently selected role - logger.debug("Checking if role is currently selected", {selectedRoleId, role_id: role.person_roleid, roleInfo}) - if (selectedRoleId && role.person_roleid === selectedRoleId) { - logger.debug("Role is currently selected", {role_id: role.person_roleid, roleInfo}) - if (hasAccess) { - logger.debug("Role has access; setting as currently selected", {roleInfo}) + if (selectedRoleId && role.person_roleid === selectedRoleId) { + // If the role has access and matches the selectedRoleId, set it as currently selected + logger.debug("Role has access and matches selectedRoleId; setting as currently selected", {roleInfo}) currentlySelectedRole = roleInfo } else { - logger.debug("Role does not have access; unsetting currently selected role", {roleInfo}) - currentlySelectedRole = undefined + // Add the role to rolesWithAccess array only if it is NOT the selectedRoleId + rolesWithAccess.push(roleInfo) + logger.debug("Role has access; adding to rolesWithAccess", {roleInfo}) } + } else { + // If role lacks access, add it to rolesWithoutAccess array + rolesWithoutAccess.push(roleInfo) + logger.debug("Role does not have access; adding to rolesWithoutAccess", {roleInfo}) } }) @@ -130,7 +142,9 @@ export const fetchUserInfo = async ( } } -// Helper function to get organization name from org_code +/** + * **Fetches the organization name for a given organization code.** + */ function getOrgNameFromOrgCode(data: UserInfoResponse, org_code: string, logger: Logger): string | undefined { logger.info("Getting org name from org code", {org_code}) const orgs = data.nhsid_user_orgs || [] @@ -139,8 +153,12 @@ function getOrgNameFromOrgCode(data: UserInfoResponse, org_code: string, logger: return org ? org.org_name : undefined } -// Add the user currentlySelectedRole, rolesWithAccess and rolesWithoutAccess to the dynamoDB document, -// keyed by this user's username. +/** + * **Updates the user information in DynamoDB.** + * + * - Stores `currentlySelectedRole`, `rolesWithAccess`, `rolesWithoutAccess`, and `userDetails`. + * - Ensures no undefined values are stored in DynamoDB. + */ export const updateDynamoTable = async ( username: string, data: TrackerUserInfo, @@ -154,31 +172,35 @@ export const updateDynamoTable = async ( logger.debug("Adding user roles to DynamoDB", {username, data}) - // Add the user roles to the DynamoDB table - - // Dyanamo cannot allow undefined values. We need to scrub any undefined values from the data objects + // Dynamo cannot allow undefined values. We need to scrub any undefined values from the data objects const currentlySelectedRole: RoleDetails = data.currently_selected_role ? data.currently_selected_role : {} const rolesWithAccess: Array = data.roles_with_access ? data.roles_with_access : [] const rolesWithoutAccess: Array = data.roles_without_access ? data.roles_without_access : [] + const userDetails: UserDetails = data.user_details || {family_name: "", given_name: ""} // Since RoleDetails has a bunch of possibly undefined fields, we need to scrub those out. // Convert everything to strings, then convert back to a generic object. const scrubbedCurrentlySelectedRole = JSON.parse(JSON.stringify(currentlySelectedRole)) const scrubbedRolesWithAccess = rolesWithAccess.map((role) => JSON.parse(JSON.stringify(role))) const scrubbedRolesWithoutAccess = rolesWithoutAccess.map((role) => JSON.parse(JSON.stringify(role))) + const scrubbedUserDetails = JSON.parse(JSON.stringify(userDetails)) try { await documentClient.send( new UpdateCommand({ TableName: tokenMappingTableName, Key: {username}, - UpdateExpression: - // eslint-disable-next-line max-len - "SET rolesWithAccess = :rolesWithAccess, rolesWithoutAccess = :rolesWithoutAccess, currentlySelectedRole = :currentlySelectedRole", + UpdateExpression: ` + SET rolesWithAccess = :rolesWithAccess, + rolesWithoutAccess = :rolesWithoutAccess, + currentlySelectedRole = :currentlySelectedRole, + userDetails = :userDetails + `, ExpressionAttributeValues: { ":rolesWithAccess": scrubbedRolesWithAccess, ":rolesWithoutAccess": scrubbedRolesWithoutAccess, - ":currentlySelectedRole": scrubbedCurrentlySelectedRole + ":currentlySelectedRole": scrubbedCurrentlySelectedRole, + ":userDetails": scrubbedUserDetails } }) ) @@ -189,7 +211,12 @@ export const updateDynamoTable = async ( } } -// Fetch user info from DynamoDB +/** + * **Fetches user information from DynamoDB.** + * + * - Retrieves `rolesWithAccess`, `rolesWithoutAccess`, `currentlySelectedRole`, and `userDetails`. + * - Returns default values for missing attributes. + */ export const fetchDynamoTable = async ( username: string, documentClient: DynamoDBDocumentClient, @@ -216,12 +243,10 @@ export const fetchDynamoTable = async ( roles_with_access: response.Item.rolesWithAccess || [], roles_without_access: response.Item.rolesWithoutAccess || [], currently_selected_role: response.Item.currentlySelectedRole || undefined, - user_details: response.Item.userDetails || {family_name: "", given_name: ""} // Default empty user details + user_details: response.Item.userDetails || {family_name: "", given_name: ""} } - logger.info("User info successfully retrieved from DynamoDB", { - data: mappedUserInfo - }) + logger.info("User info successfully retrieved from DynamoDB", {data: mappedUserInfo}) return mappedUserInfo } catch (error) { logger.error("Error fetching user info from DynamoDB", {error}) diff --git a/packages/trackerUserInfoLambda/tests/test_handler.mockDisabled.test.ts b/packages/trackerUserInfoLambda/tests/test_handler.mockDisabled.test.ts index a3ec4c9907..df778f82c6 100644 --- a/packages/trackerUserInfoLambda/tests/test_handler.mockDisabled.test.ts +++ b/packages/trackerUserInfoLambda/tests/test_handler.mockDisabled.test.ts @@ -133,7 +133,7 @@ describe("Lambda Handler Tests with mock disabled", () => { expect(response).toHaveProperty("body") const body = JSON.parse(response.body) - expect(body).toHaveProperty("message", "UserInfo fetched successfully") + expect(body).toHaveProperty("message", "UserInfo fetched successfully from the OIDC endpoint") expect(body).toHaveProperty("userInfo") }) diff --git a/packages/trackerUserInfoLambda/tests/test_handler.mockEnabled.test.ts b/packages/trackerUserInfoLambda/tests/test_handler.mockEnabled.test.ts index 98bbb944eb..3f616679f3 100644 --- a/packages/trackerUserInfoLambda/tests/test_handler.mockEnabled.test.ts +++ b/packages/trackerUserInfoLambda/tests/test_handler.mockEnabled.test.ts @@ -160,7 +160,7 @@ describe("Lambda Handler Tests with mock enabled", () => { expect(response).toHaveProperty("body") const body = JSON.parse(response.body) - expect(body).toHaveProperty("message", "UserInfo fetched successfully") + expect(body).toHaveProperty("message", "UserInfo fetched successfully from the OIDC endpoint") expect(body).toHaveProperty("userInfo") }) @@ -362,7 +362,7 @@ describe("Lambda Handler Tests with mock enabled", () => { ) expect(response.statusCode).toBe(200) - expect(response.body).toContain("UserInfo fetched successfully from cache") + expect(response.body).toContain("UserInfo fetched successfully from DynamoDB") const responseBody = JSON.parse(response.body) expect(responseBody.userInfo).toEqual(userInfoMock) @@ -398,7 +398,7 @@ describe("Lambda Handler Tests with mock enabled", () => { ) expect(response.statusCode).toBe(200) - expect(response.body).toContain("UserInfo fetched successfully from cache") + expect(response.body).toContain("UserInfo fetched successfully from DynamoDB") const responseBody = JSON.parse(response.body) expect(responseBody.userInfo).toEqual(userInfoMock) @@ -434,7 +434,7 @@ describe("Lambda Handler Tests with mock enabled", () => { ) expect(response.statusCode).toBe(200) - expect(response.body).toContain("UserInfo fetched successfully from cache") + expect(response.body).toContain("UserInfo fetched successfully from DynamoDB") const responseBody = JSON.parse(response.body) expect(responseBody.userInfo).toEqual(userInfoMock) diff --git a/packages/trackerUserInfoLambda/tests/test_userInfoHelpers.test.ts b/packages/trackerUserInfoLambda/tests/test_userInfoHelpers.test.ts index aa555f4e9b..ab944eb6f5 100644 --- a/packages/trackerUserInfoLambda/tests/test_userInfoHelpers.test.ts +++ b/packages/trackerUserInfoLambda/tests/test_userInfoHelpers.test.ts @@ -92,6 +92,14 @@ describe("fetchUserInfo", () => { activity_codes: ["OTHER_CODE"], person_orgid: "org-id-2", role_code: "role-code-2" + }, + { + role_name: "Doctor", + person_roleid: "role-id-3", + org_code: "ORG3", + activity_codes: ["CPT_CODE"], + person_orgid: "org-id-3", + role_code: "role-code-3" } ], nhsid_user_orgs: [ @@ -102,6 +110,10 @@ describe("fetchUserInfo", () => { { org_code: "ORG2", org_name: "Organization Two" + }, + { + org_code: "ORG3", + org_name: "Organization Three" } ] } @@ -126,9 +138,9 @@ describe("fetchUserInfo", () => { roles_with_access: [ { role_name: "Doctor", - role_id: "role-id-1", - org_code: "ORG1", - org_name: "Organization One" + role_id: "role-id-3", + org_code: "ORG3", + org_name: "Organization Three" } ], roles_without_access: [ @@ -338,6 +350,34 @@ describe("fetchDynamoTable", () => { jest.clearAllMocks() }) + it("should fetch user roles and user details from DynamoDB", async () => { + const mockResponse = { + Item: { + rolesWithAccess: [ + {role_name: "Doctor", role_id: "123", org_code: "ABC", org_name: "Test Hospital"} + ], + rolesWithoutAccess: [], + currentlySelectedRole: undefined, + userDetails: {family_name: "Doe", given_name: "John"} + } + } + + mockSend.mockResolvedValueOnce(mockResponse as never) + + const result = await fetchDynamoTable(username, documentClient, logger, tokenMappingTableName) + + expect(result).toEqual({ + roles_with_access: [ + {role_name: "Doctor", role_id: "123", org_code: "ABC", org_name: "Test Hospital"} + ], + roles_without_access: [], + currently_selected_role: undefined, + user_details: {family_name: "Doe", given_name: "John"} + }) + + expect(mockSend).toHaveBeenCalled() + }) + it("should throw an error when DynamoDB retrieval fails", async () => { mockSend.mockRejectedValue(new Error("DynamoDB error")) @@ -373,4 +413,28 @@ describe("fetchDynamoTable", () => { user_details: {family_name: "Doe", given_name: "John"} }) }) + + it("should return default user details if missing in DynamoDB", async () => { + const mockResponse = { + Item: { + rolesWithAccess: [], + rolesWithoutAccess: [], + currentlySelectedRole: undefined + // userDetails is missing + } + } + + mockSend.mockResolvedValueOnce(mockResponse as never) + + const result = await fetchDynamoTable(username, documentClient, logger, tokenMappingTableName) + + expect(result).toEqual({ + roles_with_access: [], + roles_without_access: [], + currently_selected_role: undefined, + user_details: {family_name: "", given_name: ""} + }) + + expect(mockSend).toHaveBeenCalled() + }) }) diff --git a/scripts/run_regression_tests.py b/scripts/run_regression_tests.py index f377312b09..9506d712f4 100644 --- a/scripts/run_regression_tests.py +++ b/scripts/run_regression_tests.py @@ -13,7 +13,7 @@ from requests.auth import HTTPBasicAuth # This should be set to a known good version of regression test repo -REGRESSION_TESTS_REPO_TAG = "v2.11.0" +REGRESSION_TESTS_REPO_TAG = "v2.11.4" GITHUB_API_URL = "https://api.github.com/repos/NHSDigital/electronic-prescription-service-api-regression-tests/actions" GITHUB_RUN_URL = "https://github.com/NHSDigital/electronic-prescription-service-api-regression-tests/actions/runs"