Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
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
Prev Previous commit
Next Next commit
Refactor to queryStringParams
  • Loading branch information
markuscolourbox committed Mar 25, 2022
commit 38671a53e643f654b7561589718fab6345cca913
27 changes: 22 additions & 5 deletions source/image-handler/image-request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ type OriginalImageInfo = Partial<{
export class ImageRequest {
private static readonly DEFAULT_REDUCTION_EFFORT = 4;

constructor(private readonly s3Client: S3, private readonly secretProvider: SecretProvider) {}
constructor(private readonly s3Client: S3, private readonly secretProvider: SecretProvider) { }

/**
* Initializer function for creating a new image request, used by the image handler to perform image modifications.
Expand All @@ -29,6 +29,7 @@ export class ImageRequest {
public async setup(event: ImageHandlerEvent): Promise<ImageRequestInfo> {
try {
await this.validateRequestSignature(event);
this.validateRequestExpires(event);

let imageRequestInfo: ImageRequestInfo = <ImageRequestInfo>{};

Expand All @@ -41,7 +42,6 @@ export class ImageRequest {
imageRequestInfo = { ...imageRequestInfo, ...originalImage };

imageRequestInfo.headers = this.parseImageHeaders(event, imageRequestInfo.requestType);
this.validateRequestExpires(imageRequestInfo);

// If the original image is SVG file and it has any edits but no output format, change the format to WebP.
if (imageRequestInfo.contentType === 'image/svg+xml' && imageRequestInfo.edits && Object.keys(imageRequestInfo.edits).length > 0 && !imageRequestInfo.edits.toFormat) {
Expand Down Expand Up @@ -407,6 +407,19 @@ export class ImageRequest {
}
}

/**
* Creates a query string similar to API Gateway 2.0 payload's $.rawQueryString
* @param queryStringParameters Request's query parameters
* @returns URL encoded queryString
*/
private recreateQueryString(queryStringParameters: ImageHandlerEvent['queryStringParameters']): string {
return Object
.entries(queryStringParameters)
.filter(([key]) => key !== 'signature')
.map(([key, value]) => [key, value].join('='))
.join('&');
}

/**
* Validates the request's signature.
* @param event Lambda request body.
Expand All @@ -424,11 +437,14 @@ export class ImageRequest {
throw new ImageHandlerError(StatusCodes.BAD_REQUEST, 'AuthorizationQueryParametersError', 'Query-string requires the signature parameter.');
}

const queryString = this.recreateQueryString(queryStringParameters);

try {
const { signature } = queryStringParameters;
const secret = JSON.parse(await this.secretProvider.getSecret(SECRETS_MANAGER));
const key = secret[SECRET_KEY];
const hash = createHmac('sha256', key).update(path).digest('hex');
const stringToSign = queryString !== '' ? [path, queryString].join('?') : path;
const hash = createHmac('sha256', key).update(stringToSign).digest('hex');

// Signature should be made with the full path.
if (signature !== hash) {
Expand All @@ -445,9 +461,10 @@ export class ImageRequest {
}
}

private validateRequestExpires(requestInfo: ImageRequestInfo): void {
private validateRequestExpires(event: ImageHandlerEvent): void {
try {
const expires = requestInfo.headers?.expires;
const { queryStringParameters } = event;
const expires = queryStringParameters?.expires;
if (expires !== undefined) {
const parsedDate = new Date(expires);
if (isNaN(parsedDate.getTime())) {
Expand Down
3 changes: 2 additions & 1 deletion source/image-handler/lib/interfaces.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ import { Headers, ImageEdits } from './types';
export interface ImageHandlerEvent {
path?: string;
queryStringParameters?: {
signature: string;
signature?: string;
expires?: string;
};
requestContext?: {
elb?: unknown;
Expand Down
69 changes: 46 additions & 23 deletions source/image-handler/test/image-request.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import SecretsManager from 'aws-sdk/clients/secretsmanager';
import S3 from 'aws-sdk/clients/s3';

import { ImageRequest } from '../image-request';
import { ImageHandlerError, RequestTypes, StatusCodes } from '../lib';
import { ImageHandlerError, ImageHandlerEvent, RequestTypes, StatusCodes } from '../lib';
import { SecretProvider } from '../secret-provider';

describe('setup()', () => {
Expand Down Expand Up @@ -434,6 +434,22 @@ describe('setup()', () => {
expect(imageRequestInfo).toEqual(expectedResult);
});

it('Should pass when the image signature is correct with expires query string', async () => {
jest.useFakeTimers('modern').setSystemTime(0);

// TODO write test

expect(true).toBe(true);
});

it('Should throw when the image signature is incorrect with expires query string', async () => {
jest.useFakeTimers('modern').setSystemTime(0);

// TODO write test

expect(true).toBe(true);
});

it('Should throw an error when queryStringParameters are missing', async () => {
// Arrange
const event = {
Expand Down Expand Up @@ -855,43 +871,50 @@ describe('setup()', () => {
});

describe('011/expiryDate', () => {
const baseRequest = {
bucket: 'test',
requestType: 'Default',
key: 'test.png',
};
const path = `/${Buffer.from(JSON.stringify(baseRequest)).toString('base64')}`;
it.each([
{
path: '/eyJidWNrZXQiOiJ0ZXN0IiwicmVxdWVzdFR5cGUiOiJEZWZhdWx0Iiwia2V5IjoidGVzdC5wbmciLCJoZWFkZXJzIjp7ImV4cGlyZXMiOiJUaHUsIDAxIEphbiAxOTcwIDAwOjAwOjAwIEdNVCJ9fQ==',
expires: 'Thu, 01 Jan 1970 00:00:00 GMT',
error: {
code: 'ImageRequestExpired',
message: 'Request has expired.',
status: StatusCodes.FORBIDDEN,
},
},
{
path: '/eyJidWNrZXQiOiJ0ZXN0IiwicmVxdWVzdFR5cGUiOiJEZWZhdWx0Iiwia2V5IjoidGVzdC5wbmciLCJoZWFkZXJzIjp7ImV4cGlyZXMiOiJpbnZhbGlkS2V5In19',
expires: 'invalidKey',
error: {
code: 'ImageRequestExpiryFormat',
message: 'Request has invalid expiry date.',
status: StatusCodes.BAD_REQUEST,
}
}
])("Should throw an error when $error.message", (async ({ path, error: expectedError }) => {
// Arrange
const event = {
path,
};
// Mock
mockAwsS3.getObject.mockImplementationOnce(() => ({
promise() {
return Promise.resolve({ Body: Buffer.from('SampleImageContent\n') });
}
}));
// Act
const imageRequest = new ImageRequest(s3Client, secretProvider);
try {
await imageRequest.setup(event);
} catch (error) {
// Assert
expect(error).toMatchObject(expectedError);
}
}));
] as { expires: ImageHandlerEvent['queryStringParameters']['expires'], error: object, }[])(
"Should throw an error when $error.message",
(async ({ error: expectedError, expires }) => {
// Arrange
const event: ImageHandlerEvent = {
path,
queryStringParameters: {
expires,
},
};
// Mock
mockAwsS3.getObject.mockImplementationOnce(() => ({
promise() {
return Promise.resolve({ Body: Buffer.from('SampleImageContent\n') });
}
}));
// Act
const imageRequest = new ImageRequest(s3Client, secretProvider);
await expect(imageRequest.setup(event)).rejects.toMatchObject(expectedError);
})
);
});
});

Expand Down