Skip to content

Commit

Permalink
INCIDEN-922: Restricting listClient API to specified user
Browse files Browse the repository at this point in the history
We're now sending the access token from the frontend to the
backend, so we can now enforce some access control on listing
a client. This will restrict the user to only seeing the service
they created, so if they navigated to another user's service
the backend now returns a 403 - Forbidden. [deploy]
  • Loading branch information
Ryan-Andrews99 committed Sep 10, 2024
1 parent 4c76e96 commit d1b3827
Show file tree
Hide file tree
Showing 5 changed files with 148 additions and 12 deletions.
11 changes: 11 additions & 0 deletions backend/api/src/dynamodb-client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
82 changes: 78 additions & 4 deletions backend/api/src/handlers/dynamodb/get-service-clients.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)};
Expand All @@ -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
};
};
1 change: 1 addition & 0 deletions backend/api/tests/handlers/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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")}.`;
57 changes: 53 additions & 4 deletions backend/api/tests/handlers/dynamodb/get-service-clients.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
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";

describe("getServiceClientsHandler tests", () => {
Expand All @@ -14,20 +14,64 @@ 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 testApiEvent = constructTestApiGatewayEvent({
body: "",
pathParameters: {serviceId: TEST_SERVICE_ID},
headers: {Authorization: "Bearer " + TEST_ACCESS_TOKEN}
});
const getServiceClientsResponse = await getServiceClientsHandler(testApiEvent);
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 testApiEvent = constructTestApiGatewayEvent({
body: "",
pathParameters: {serviceId: TEST_SERVICE_ID},
headers: {Authorization: "Bearer " + TEST_ACCESS_TOKEN}
});
const getServiceClientsResponse = await getServiceClientsHandler(testApiEvent);

expect(getServiceClientsResponse).toStrictEqual({
Expand All @@ -39,9 +83,14 @@ 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 testApiEvent = constructTestApiGatewayEvent({
body: "",
pathParameters: {serviceId: TEST_SERVICE_ID},
headers: {Authorization: "Bearer " + TEST_ACCESS_TOKEN}
});

const getServiceClientsResponse = await getServiceClientsHandler(testApiEvent);

Expand Down
9 changes: 5 additions & 4 deletions backend/api/tests/handlers/utils.ts
Original file line number Diff line number Diff line change
@@ -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",
Expand Down Expand Up @@ -52,7 +52,8 @@ export const constructTestApiGatewayEvent = (params = {body: "", pathParameters:
stage: "dev"
},
resource: "",
stageVariables: {}
stageVariables: {},
...params
});

export const mockLambdaContext: Context = {
Expand Down

0 comments on commit d1b3827

Please sign in to comment.