INCIDEN-922: Adds authorisation to update user handler

This checks that the userId in the access token matches the userId in the update request body, ensuring a user can only update their own entries
govuk-one-login · Sep 18, 2024 · 5000b37 · 5000b37
1 parent 1ab610e
commit 5000b37
Show file tree

Hide file tree

Showing 2 changed files with 98 additions and 18 deletions.
diff --git a/backend/api/src/handlers/dynamodb/update-user.ts b/backend/api/src/handlers/dynamodb/update-user.ts
@@ -1,14 +1,39 @@
+import {APIGatewayEvent} from "aws-lambda";
 import DynamoDbClient from "../../dynamodb-client";
-import {handlerInvokeEvent} from "../handler-utils";
+import {validateAuthorisationHeader} from "../helper/validate-authorisation-header";
 
 const client = new DynamoDbClient();
 
-export const updateUserHandler = async (event: handlerInvokeEvent): Promise<{statusCode: number; body: string}> => {
+export const updateUserHandler = async (event: APIGatewayEvent): Promise<{statusCode: number; body: string}> => {
     const body = JSON.parse(event.body as string);
     const response = {statusCode: 200, body: JSON.stringify("OK")};
+    const userId = body.userId;
+
+    if (!userId) {
+        return {
+            statusCode: 400,
+            body: "No userId provided in request body"
+        };
+    }
+
+    const authHeader = event.headers.Authorization;
+    const authorisationHeaderValidation = validateAuthorisationHeader(authHeader);
+
+    if (!authorisationHeaderValidation.valid) {
+        return authorisationHeaderValidation.errorResponse;
+    }
+
+    const userIdWithoutPrefix = userId.includes("user#") ? userId.substring("user#".length) : userId;
+
+    if (userIdWithoutPrefix !== authorisationHeaderValidation.userId) {
+        return {
+            statusCode: 403,
+            body: "Forbidden"
+        };
+    }
 
     await client
-        .updateUser(body.userId, body.cognitoUserId, body.updates)
+        .updateUser(body.userId, body.updates)
         .then(updateItemCommandOutput => {
             response.statusCode = 200;
             response.body = JSON.stringify(updateItemCommandOutput);

diff --git a/backend/api/tests/handlers/dynamodb/update-user.test.ts b/backend/api/tests/handlers/dynamodb/update-user.test.ts
@@ -1,34 +1,83 @@
 import {updateUserHandler} from "../../../src/handlers/dynamodb/update-user";
 import DynamoDbClient from "../../../src/dynamodb-client";
-import {TEST_COGNITO_USER_ID, TEST_SERVICE_NAME, TEST_USER_ID} from "../constants";
+import {TEST_ACCESS_TOKEN, TEST_COGNITO_USER_ID, TEST_SERVICE_NAME, TEST_USER_ID, TEST_USER_PHONE_NUMBER} from "../constants";
 import {UpdateItemCommandOutput} from "@aws-sdk/client-dynamodb";
-import {handlerInvokeEvent} from "../../../src/handlers/handler-utils";
+import {constructTestApiGatewayEvent} from "../utils";
 
 const TEST_USER_UPDATES = {
     userId: TEST_USER_ID,
-    cognitoUserId: TEST_COGNITO_USER_ID,
     updates: {
-        serviceName: TEST_SERVICE_NAME
+        phone: TEST_USER_PHONE_NUMBER
     }
 };
-
-const TEST_UPDATE_USER_EVENT: handlerInvokeEvent = {
-    statusCode: 200,
-    body: JSON.stringify(TEST_USER_UPDATES)
-};
-
-describe("handlerName tests", () => {
+describe("updateUserHandler tests", () => {
     beforeEach(() => {
         jest.clearAllMocks();
     });
 
+    it("returns a  400 for no UserId in the update body", async () => {
+        const updateUserResponse: UpdateItemCommandOutput = {$metadata: {httpStatusCode: 200}};
+        const updateUserSpy = jest.spyOn(DynamoDbClient.prototype, "updateUser").mockResolvedValue(updateUserResponse);
+
+        const response = await updateUserHandler(
+            constructTestApiGatewayEvent({
+                body: JSON.stringify({
+                    cognitoUserId: TEST_COGNITO_USER_ID,
+                    updates: {
+                        serviceName: TEST_SERVICE_NAME
+                    }
+                }),
+                pathParameters: {}
+            })
+        );
+
+        expect(updateUserSpy).not.toHaveBeenCalled();
+        expect(response).toStrictEqual({
+            statusCode: 400,
+            body: "No userId provided in request body"
+        });
+    });
+
+    it("returns a 403 if the userId in the request body does not match the access token", async () => {
+        const updateUserResponse: UpdateItemCommandOutput = {$metadata: {httpStatusCode: 200}};
+        const updateUserSpy = jest.spyOn(DynamoDbClient.prototype, "updateUser").mockResolvedValue(updateUserResponse);
+
+        const response = await updateUserHandler(
+            constructTestApiGatewayEvent({
+                body: JSON.stringify({
+                    cognitoUserId: TEST_COGNITO_USER_ID,
+                    updates: {
+                        serviceId: "aDifferentUserId",
+                        serviceName: TEST_SERVICE_NAME
+                    }
+                }),
+                pathParameters: {},
+                headers: {Authorization: `Bearer ${TEST_ACCESS_TOKEN}`}
+            })
+        );
+
+        expect(updateUserSpy).not.toHaveBeenCalled();
+        expect(response).toStrictEqual({
+            statusCode: 400,
+            body: "No userId provided in request body"
+        });
+    });
+
     it("returns a 200 and the stringified record in the response when the update is successful", async () => {
         const updateUserResponse: UpdateItemCommandOutput = {$metadata: {httpStatusCode: 200}};
         const updateUserSpy = jest.spyOn(DynamoDbClient.prototype, "updateUser").mockResolvedValue(updateUserResponse);
 
-        const response = await updateUserHandler(TEST_UPDATE_USER_EVENT);
+        const response = await updateUserHandler(
+            constructTestApiGatewayEvent({
+                body: JSON.stringify(TEST_USER_UPDATES),
+                pathParameters: {},
+                headers: {Authorization: `Bearer ${TEST_ACCESS_TOKEN}`}
+            })
+        );
 
-        expect(updateUserSpy).toHaveBeenCalledWith(TEST_USER_UPDATES.userId, TEST_USER_UPDATES.cognitoUserId, TEST_USER_UPDATES.updates);
+        expect(updateUserSpy).toHaveBeenCalledWith(TEST_USER_ID, {
+            phone: TEST_USER_PHONE_NUMBER
+        });
         expect(response).toStrictEqual({
             statusCode: 200,
             body: JSON.stringify(updateUserResponse)
@@ -39,9 +88,15 @@ describe("handlerName tests", () => {
         const dynamoErr = "SomeDynamoErr";
         const updateUserSpy = jest.spyOn(DynamoDbClient.prototype, "updateUser").mockRejectedValue(dynamoErr);
 
-        const response = await updateUserHandler(TEST_UPDATE_USER_EVENT);
+        const response = await updateUserHandler(
+            constructTestApiGatewayEvent({
+                body: JSON.stringify(TEST_USER_UPDATES),
+                pathParameters: {},
+                headers: {Authorization: `Bearer ${TEST_ACCESS_TOKEN}`}
+            })
+        );
 
-        expect(updateUserSpy).toHaveBeenCalledWith(TEST_USER_UPDATES.userId, TEST_USER_UPDATES.cognitoUserId, TEST_USER_UPDATES.updates);
+        expect(updateUserSpy).toHaveBeenCalledWith(TEST_USER_UPDATES.userId, TEST_USER_UPDATES.updates);
         expect(response).toStrictEqual({
             statusCode: 500,
             body: JSON.stringify(dynamoErr)