Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

INCIDEN-922: further access control #906

Merged
merged 11 commits into from
Sep 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions backend/api/api.template.yml
Original file line number Diff line number Diff line change
Expand Up @@ -915,9 +915,14 @@ Resources:
- Effect: Allow
Action: states:StartSyncExecution
Resource: !Ref UpdateClientStepFunction
- DynamoDBReadPolicy:
TableName:
Fn::ImportValue: !Sub ${DeploymentName}-DynamoDB-DataTableName
Environment:
Variables:
STATE_MACHINE_ARN: !Ref UpdateClientStepFunction
TABLE:
Fn::ImportValue: !Sub ${DeploymentName}-DynamoDB-DataTableName
Events:
Api:
Type: Api
Expand Down
2 changes: 1 addition & 1 deletion backend/api/src/dynamodb-client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ export default class DynamoDbClient {
return this.update("service", serviceId, "client", clientId, updates);
}

async updateUser(userId: string, cognitoUserId: string, updates: Updates): Promise<UpdateItemCommandOutput> {
async updateUser(userId: string, updates: Updates): Promise<UpdateItemCommandOutput> {
return this.update("user", userId, "user", userId, updates);
}

Expand Down
63 changes: 53 additions & 10 deletions backend/api/src/handlers/dynamodb/dynamo-db-service.ts
Original file line number Diff line number Diff line change
@@ -1,19 +1,30 @@
import DynamoDbClient from "../../dynamodb-client";
import console from "console";
import {APIGatewayProxyEvent, APIGatewayProxyResult} from "aws-lambda";
import {validateAuthorisationHeader} from "../helper/validate-authorisation-header";

const dynamoDBClient = new DynamoDbClient();

export const getDynamoDBEntriesHandler = async (event: APIGatewayProxyEvent): Promise<APIGatewayProxyResult> => {
//This endpoint is unauthorised because it is called in controllers
//where the user may not have an access token present in their session
//such as in the forgot password flow before login
console.log("dynamo-db-inspector - In getDynamoDBEntriesHandler");

const response = {statusCode: 404, body: JSON.stringify("")};
const userEmail = event.pathParameters?.userEmail;

if (!userEmail) {
return {
statusCode: 400,
body: "Missing userEmail parameter in request"
};
}

const result = await dynamoDBClient.getListOfClients();

if (userEmail == null || result == null || result.Items == null) {
throw new Error("Unable to check DynamoDB for Items of User Email =>" + userEmail);
if (result == null || result.Items == null) {
throw new Error("Unable to check DynamoDB for Items of User Email");
} else {
response.statusCode = 200; // Need to set Response here because List might be empty (i.e. non-existent account).

Expand All @@ -31,35 +42,67 @@ export const getDynamoDBEntriesHandler = async (event: APIGatewayProxyEvent): Pr

export const deleteDynamoDBClientEntriesHandler = async (event: APIGatewayProxyEvent): Promise<APIGatewayProxyResult> => {
console.log("dynamo-db-inspector - In deleteDynamoDBClientEntriesHandler");
const authHeader = event.headers.Authorization;

const authorisationHeaderValidation = validateAuthorisationHeader(authHeader);

if (!authorisationHeaderValidation.valid) {
return authorisationHeaderValidation.errorResponse;
}

const response = {statusCode: 500, body: JSON.stringify("")};
const payload = event?.body ? JSON.parse(event.body as string) : event;
const userID = payload.userId;
const userId = payload.userId;
const serviceID = payload.serviceId;

if (userID == null || serviceID == null) {
if (userId == null || serviceID == null) {
throw new Error("No details provided for DeleteDynamoDBClientEntries");
} else {
await dynamoDBClient.deleteDynamoDBClientEntries(userID, serviceID);
response.statusCode = 200;
}

const isUserAuthorised = await dynamoDBClient.checkServiceUserExists(serviceID, authorisationHeaderValidation.userId);

const userIdWithoutPrefix = userId.includes("user#") ? userId.substring("user#".length) : userId;

if (!isUserAuthorised || userIdWithoutPrefix !== authorisationHeaderValidation.userId) {
return {
statusCode: 403,
body: "Forbidden"
};
}

await dynamoDBClient.deleteDynamoDBClientEntries(userId, serviceID);
response.statusCode = 200;

return response;
};

export const deleteDynamoDBServiceEntriesHandler = async (event: APIGatewayProxyEvent): Promise<APIGatewayProxyResult> => {
console.log("dynamo-db-inspector - In deleteDynamoDBServiceEntriesHandler");
const authHeader = event.headers.Authorization;

const authorisationHeaderValidation = validateAuthorisationHeader(authHeader);

if (!authorisationHeaderValidation.valid) {
return authorisationHeaderValidation.errorResponse;
}

const response = {statusCode: 500, body: JSON.stringify("")};
const payload = event?.body ? JSON.parse(event.body as string) : event;
const serviceID = payload.serviceId;

if (serviceID == null) {
throw new Error("No Service ID provided for DeleteDynamoDBServiceEntries");
} else {
await dynamoDBClient.deleteDynamoDBServiceEntries(serviceID);
response.statusCode = 200;
}
const isUserAuthorised = await dynamoDBClient.checkServiceUserExists(serviceID, authorisationHeaderValidation.userId);

if (!isUserAuthorised) {
return {
statusCode: 403,
body: "Forbidden"
};
}
await dynamoDBClient.deleteDynamoDBServiceEntries(serviceID);
response.statusCode = 200;

return response;
};
17 changes: 17 additions & 0 deletions backend/api/src/handlers/dynamodb/get-services.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import {APIGatewayProxyEvent, APIGatewayProxyResult} from "aws-lambda";
import DynamoDbClient from "../../dynamodb-client";
import {validateAuthorisationHeader} from "../helper/validate-authorisation-header";

const client = new DynamoDbClient();

Expand All @@ -9,6 +10,22 @@ export const getServicesHandler = async (event: APIGatewayProxyEvent): Promise<A
return noUserIdResponse;
}

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"
};
}

const response = {statusCode: 200, body: JSON.stringify(userId)};
await client
.getServices(userId)
Expand Down
17 changes: 17 additions & 0 deletions backend/api/src/handlers/dynamodb/get-user.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,27 @@
import {APIGatewayProxyEvent, APIGatewayProxyResult} from "aws-lambda";
import DynamoDbClient from "../../dynamodb-client";
import {validateAuthorisationHeader} from "../helper/validate-authorisation-header";

const client = new DynamoDbClient();

export const getUserHandler = async (event: APIGatewayProxyEvent): Promise<APIGatewayProxyResult> => {
const cognitoId = JSON.parse(event.body as string);
const authHeader = event.headers.Authorization;

const authorisationHeaderValidation = validateAuthorisationHeader(authHeader);

if (!authorisationHeaderValidation.valid) {
return authorisationHeaderValidation.errorResponse;
}
const userIdWithoutPrefix = cognitoId.includes("user#") ? cognitoId.substring("user#".length) : cognitoId;

if (authorisationHeaderValidation.userId !== userIdWithoutPrefix) {
return {
statusCode: 403,
body: "Forbidden"
};
}

const response = {statusCode: 200, body: JSON.stringify("OK")};

await client
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@ export type clientRegistryUpdateResponse = {
clientId: string;
updates: Updates;
};

//This lambda is called by the step function after updating the Client registry
//Given we authorise the user before we call the step function, there is no need
//to implement additional authorisation here
export const updateServiceClientHandler = async (event: handlerInvokeEvent): Promise<{statusCode: number; body: string}> => {
const payload: clientRegistryUpdateResponse = JSON.parse(event.body);

Expand Down
3 changes: 3 additions & 0 deletions backend/api/src/handlers/dynamodb/update-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@ import {handlerInvokeEvent} from "../handler-utils";

const client = new DynamoDbClient();

//This lambda is called by the step function after updating the Client registry
//Given we authorise the user before we call the step function, there is no need
//to implement additional authorisation here
export const updateServiceHandler = async (event: handlerInvokeEvent): Promise<{statusCode: number; body: string}> => {
const body = JSON.parse(event.body as string);
const response = {statusCode: 200, body: JSON.stringify("OK")};
Expand Down
31 changes: 28 additions & 3 deletions backend/api/src/handlers/dynamodb/update-user.ts
Original file line number Diff line number Diff line change
@@ -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);
Expand Down
34 changes: 34 additions & 0 deletions backend/api/src/handlers/step-functions/update-client.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,40 @@
import {APIGatewayProxyEvent, APIGatewayProxyResult, Context} from "aws-lambda";
import {stepFunctionHandler} from "./step-function-handler";
import {validateAuthorisationHeader} from "../helper/validate-authorisation-header";
import DynamoDbClient from "../../dynamodb-client";

const client = new DynamoDbClient();

export const doUpdateClientHandler = async (event: APIGatewayProxyEvent, context: Context): Promise<APIGatewayProxyResult> => {
if (!event.body) {
return {statusCode: 400, body: "Invalid Request, missing body"};
}

const updateClientBody: {
serviceId: string;
updates: Record<string, string>;
} = JSON.parse(event.body);

const serviceId = updateClientBody.serviceId;

if (!serviceId) {
return {statusCode: 400, body: "Invalid Request, missing serviceId"};
}

const authHeaderValidationResult = validateAuthorisationHeader(event.headers.Authorization);

if (!authHeaderValidationResult.valid) {
return authHeaderValidationResult.errorResponse;
}

const isUserAuthorised = await client.checkServiceUserExists(serviceId, authHeaderValidationResult.userId);

if (!isUserAuthorised) {
return {
statusCode: 403,
body: "Forbidden"
};
}

return stepFunctionHandler(event, context);
};
2 changes: 1 addition & 1 deletion backend/api/tests/dynamodb-client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ describe("DynamoDB client", () => {
});

it("should invoke updateUser successfully", async () => {
await client.updateUser("423423ada-32123892", "c-id-0001", {Name: "Robert"});
await client.updateUser("423423ada-32123892", {Name: "Robert"});
expect(updateItemCommand).toBeCalledTimes(1);
});

Expand Down
Loading
Loading