Merge pull request #897 from govuk-one-login/inciden-922-enfore-acces…

…s-token

INCIDEN-922: Restricting list client API action to specified user

backend/api/src/dynamodb-client.ts

@@ -160,6 +160,17 @@ export default class DynamoDbClient {
         return this.dynamodb.send(command);
     }
 
+    async checkServiceUserExists(serviceId: string, userId: string): Promise<boolean> {
+        const params = {
+            TableName: this.tableName,
+            Key: marshall({pk: `service#${serviceId}`, sk: `user#${userId}`})
+        };
+
+        const command = new GetItemCommand(params);
+        const result = await this.dynamodb.send(command);
+        return result.Item !== undefined;
+    }
+
     async updateClient(serviceId: string, clientId: string, updates: Updates): Promise<UpdateItemCommandOutput> {
         return this.update("service", serviceId, "client", clientId, updates);
     }

backend/api/src/handlers/dynamodb/get-service-clients.ts

@@ -6,7 +6,25 @@ const client = new DynamoDbClient();
 export const getServiceClientsHandler = async (event: APIGatewayProxyEvent): Promise<APIGatewayProxyResult> => {
     const serviceId = event.pathParameters?.serviceId;
     if (serviceId === undefined) {
-        return noUserIdResponse;
+        return {
+            statusCode: 400,
+            body: "No serviceId request parameter supplied"
+        };
+    }
+
+    const authHeaderValidationResult = validateAuthorizationHeader(event.headers.Authorization);
+
+    if (!authHeaderValidationResult.valid) {
+        return authHeaderValidationResult.errorResponse;
+    }
+
+    const isUserAuthorised = await client.checkServiceUserExists(serviceId, authHeaderValidationResult.userId);
+
+    if (!isUserAuthorised) {
+        return {
+            statusCode: 403,
+            body: "Forbidden"
+        };
     }
 
     const response = {statusCode: 200, body: JSON.stringify(serviceId)};
@@ -25,7 +43,63 @@ export const getServiceClientsHandler = async (event: APIGatewayProxyEvent): Pro
     return response;
 };
 
-const noUserIdResponse = {
-    statusCode: 400,
-    body: JSON.stringify("No userId request parameter supplied")
+const validateAuthorizationHeader = (
+    authHeader: string | undefined
+):
+    | {
+          valid: true;
+          userId: string;
+      }
+    | {
+          valid: false;
+          errorResponse: {
+              statusCode: number;
+              body: string;
+          };
+      } => {
+    if (!authHeader || !authHeader.startsWith("Bearer ")) {
+        return {
+            valid: false,
+            errorResponse: {
+                statusCode: 401,
+                body: "Missing access token"
+            }
+        };
+    }
+
+    const authToken = authHeader.substring(7);
+    //We trust the signature as this token is attached from
+    //the frontend which does the validation
+    const tokenParts = authToken.split(".");
+
+    if (tokenParts.length !== 3) {
+        return {
+            valid: false,
+            errorResponse: {
+                statusCode: 400,
+                body: "Invalid access token"
+            }
+        };
+    }
+
+    const encodedPayload = tokenParts[1];
+    const decodedPayload = Buffer.from(encodedPayload, "base64url").toString("utf-8");
+    const parsedPayload = JSON.parse(decodedPayload);
+
+    const subjectClaim = parsedPayload.sub;
+
+    if (typeof subjectClaim !== "string") {
+        return {
+            valid: false,
+            errorResponse: {
+                statusCode: 400,
+                body: "Invalid access token"
+            }
+        };
+    }
+
+    return {
+        valid: true,
+        userId: subjectClaim
+    };
 };

backend/api/tests/handlers/constants.ts

@@ -117,3 +117,4 @@ export const TEST_DATA_TABLE_ITEM = {
 };
 
 export const TEST_MESSAGE_ID = "1234";
+export const TEST_ACCESS_TOKEN = `.${Buffer.from(JSON.stringify({sub: TEST_USER_ID})).toString("base64url")}.`;

backend/api/tests/handlers/dynamodb/get-service-clients.test.ts

@@ -1,8 +1,14 @@
 import DynamoDbClient from "../../../src/dynamodb-client";
 import {getServiceClientsHandler} from "../../../src/handlers/dynamodb/get-service-clients";
-import {TEST_SERVICE_ID} from "../constants";
+import {TEST_ACCESS_TOKEN, TEST_SERVICE_ID, TEST_USER_ID} from "../constants";
 import {constructTestApiGatewayEvent} from "../utils";
 
+const VALID_GET_CLIENT_REQUEST = constructTestApiGatewayEvent({
+    body: "",
+    pathParameters: {serviceId: TEST_SERVICE_ID},
+    headers: {Authorization: "Bearer " + TEST_ACCESS_TOKEN}
+});
+
 describe("getServiceClientsHandler tests", () => {
     beforeEach(() => {
         jest.clearAllMocks();
@@ -14,21 +20,56 @@ describe("getServiceClientsHandler tests", () => {
 
         expect(getServiceClientsResponse).toStrictEqual({
             statusCode: 400,
-            body: JSON.stringify("No userId request parameter supplied")
+            body: "No serviceId request parameter supplied"
         });
 
         expect(getClientsSpy).not.toHaveBeenCalled();
     });
 
+    it("returns a 401 and Missing access token when no authorization header is present", async () => {
+        const testApiEvent = constructTestApiGatewayEvent({body: "", pathParameters: {serviceId: TEST_SERVICE_ID}});
+        const getServiceClientsResponse = await getServiceClientsHandler(testApiEvent);
+
+        expect(getServiceClientsResponse).toStrictEqual({
+            statusCode: 401,
+            body: "Missing access token"
+        });
+    });
+
+    it("returns a 400 when the authorization header is invalid", async () => {
+        const testApiEvent = constructTestApiGatewayEvent({
+            body: "",
+            pathParameters: {serviceId: TEST_SERVICE_ID},
+            headers: {Authorization: "Bearer invalid"}
+        });
+        const getServiceClientsResponse = await getServiceClientsHandler(testApiEvent);
+
+        expect(getServiceClientsResponse).toStrictEqual({
+            statusCode: 400,
+            body: "Invalid access token"
+        });
+    });
+
+    it("returns 403 when the user is not authorised", async () => {
+        const checkServiceUserExistsSpy = jest.spyOn(DynamoDbClient.prototype, "checkServiceUserExists").mockResolvedValue(false);
+
+        const getServiceClientsResponse = await getServiceClientsHandler(VALID_GET_CLIENT_REQUEST);
+        expect(checkServiceUserExistsSpy).toHaveBeenCalledWith(TEST_SERVICE_ID, TEST_USER_ID);
+        expect(getServiceClientsResponse).toStrictEqual({
+            statusCode: 403,
+            body: "Forbidden"
+        });
+    });
+
     it("returns the clients when they are returned from dynamo", async () => {
+        jest.spyOn(DynamoDbClient.prototype, "checkServiceUserExists").mockResolvedValue(true);
         const testDynamoResponse = {
             $metadata: {},
             Items: []
         };
         const getClientsSpy = jest.spyOn(DynamoDbClient.prototype, "getClients").mockResolvedValue(testDynamoResponse);
 
-        const testApiEvent = constructTestApiGatewayEvent({body: "", pathParameters: {serviceId: TEST_SERVICE_ID}});
-        const getServiceClientsResponse = await getServiceClientsHandler(testApiEvent);
+        const getServiceClientsResponse = await getServiceClientsHandler(VALID_GET_CLIENT_REQUEST);
 
         expect(getServiceClientsResponse).toStrictEqual({
             statusCode: 200,
@@ -39,11 +80,11 @@ describe("getServiceClientsHandler tests", () => {
     });
 
     it("returns an error response when the dynamo query throws an error", async () => {
+        jest.spyOn(DynamoDbClient.prototype, "checkServiceUserExists").mockResolvedValue(true);
         const dynamoErr = "someDynamoError";
         const getClientsSpy = jest.spyOn(DynamoDbClient.prototype, "getClients").mockRejectedValue(dynamoErr);
-        const testApiEvent = constructTestApiGatewayEvent({body: "", pathParameters: {serviceId: TEST_SERVICE_ID}});
 
-        const getServiceClientsResponse = await getServiceClientsHandler(testApiEvent);
+        const getServiceClientsResponse = await getServiceClientsHandler(VALID_GET_CLIENT_REQUEST);
 
         expect(getServiceClientsResponse).toStrictEqual({
             statusCode: 500,

backend/api/tests/handlers/utils.ts

@@ -1,15 +1,15 @@
 import {APIGatewayProxyEvent} from "aws-lambda";
 import {Context} from "aws-lambda";
 
-export const constructTestApiGatewayEvent = (params = {body: "", pathParameters: {}}): APIGatewayProxyEvent => ({
+export const constructTestApiGatewayEvent = (
+    params: Partial<APIGatewayProxyEvent> & Pick<APIGatewayProxyEvent, "body" | "pathParameters"> = {body: "", pathParameters: {}}
+): APIGatewayProxyEvent => ({
     httpMethod: "get",
-    body: params.body,
     headers: {},
     isBase64Encoded: false,
     multiValueHeaders: {},
     multiValueQueryStringParameters: {},
     path: "/",
-    pathParameters: params.pathParameters,
     queryStringParameters: {},
     requestContext: {
         accountId: "123456789012",
@@ -52,7 +52,8 @@ export const constructTestApiGatewayEvent = (params = {body: "", pathParameters:
         stage: "dev"
     },
     resource: "",
-    stageVariables: {}
+    stageVariables: {},
+    ...params
 });
 
 export const mockLambdaContext: Context = {