Skip to content

Commit

Permalink
feat(cb2-11294): move get zip to use signed URLs (#67)
Browse files Browse the repository at this point in the history
  • Loading branch information
@naathanbrown
naathanbrown authored Mar 19, 2024
1 parent 236868c commit 12d8ebd
Show file tree
Hide file tree
Showing 5 changed files with 54 additions and 115 deletions.
13 changes: 7 additions & 6 deletions src/domain/getZip.ts
View file
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
/* eslint-disable @typescript-eslint/no-unsafe-argument, @typescript-eslint/no-base-to-string */

import { S3, AWSError } from 'aws-sdk';
import { APIGatewayProxyResult } from 'aws-lambda';
import getObjectFromS3 from '../infrastructure/s3/s3ZipService';
import NoBodyError from '../errors/NoBodyError';
import MissingBucketNameError from '../errors/MissingBucketNameError';
import { AWSError, S3 } from 'aws-sdk';
import FileNameError from '../errors/FileNameError';
import IncorrectFileTypeError from '../errors/IncorrectFileTypeError';
import MissingBucketNameError from '../errors/MissingBucketNameError';
import MissingFolderNameError from '../errors/MissingFolderNameError';
import FileNameError from '../errors/FileNameError';
import NoBodyError from '../errors/NoBodyError';
import getObjectFromS3 from '../infrastructure/s3/s3ZipService';
import ZipDetails from '../interfaces/ZipDetails';

function isAWSError(error: Error | AWSError): error is AWSError {
Expand Down Expand Up @@ -35,11 +35,12 @@ export default async (
throw new FileNameError();
}

// This is actually a unsigned URL that can be used to retrieve the file contents
const file = await getObjectFromS3(s3, bucketName, folder, event.adrDocumentId);
const response = file.toString('base64');

const headers = {
'Content-type': 'application/zip',
'Content-type': 'application/json',
'Access-Control-Allow-Origin': '*',
'Access-Control-Allow-Credentials': true,
'X-Content-Type-Options': 'nosniff',
Expand Down
12 changes: 5 additions & 7 deletions src/handler.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,8 @@ import serverless from 'serverless-http';
import { Context, APIGatewayEvent, APIGatewayProxyStructuredResultV2 } from 'aws-lambda';
import { app } from './infrastructure/api';

const handler = async (event: APIGatewayEvent, context: Context): Promise<APIGatewayProxyStructuredResultV2> => {
return serverless(app, {
/**
const handler = async (event: APIGatewayEvent, context: Context): Promise<APIGatewayProxyStructuredResultV2> => serverless(app, {
/**
* We proxy requests from / as <stage> is handled in APIG when we deploy.
* With with serverless-offline we proxy requests from /v<x> from the client -
* The package.json version as single source of truth to be the app basePath with stage
Expand All @@ -28,9 +27,8 @@ const handler = async (event: APIGatewayEvent, context: Context): Promise<APIGat
*
* We use express Router to proxy redirect requests from /v<x>/
*/
// basePath: `${AWS_PROVIDER_STAGE}/${MAJOR_VERSION}`,
basePath: 'v1',
})(event, context);
};
// basePath: `${AWS_PROVIDER_STAGE}/${MAJOR_VERSION}`,
basePath: 'v1',
})(event, context);

export { handler };
19 changes: 4 additions & 15 deletions src/infrastructure/s3/s3ZipService.ts
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { S3 } from 'aws-sdk';
import IncorrectFileTypeError from '../../errors/IncorrectFileTypeError';
import NoBodyError from '../../errors/NoBodyError';

export default async (
Expand All @@ -13,21 +12,11 @@ export default async (
console.info(`Bucket name: ${bucket}`);
console.info(`Item key: ${key}`);

const response = await s3
.getObject({
Bucket: bucket,
Key: key,
})
.promise();
const params = { Bucket: bucket, Key: key };
const response = await s3.getSignedUrlPromise('getObject', params);

if (response.ContentType !== 'application/octet-stream' && response.ContentType !== 'application/zip') {
// TODO: Cover this in case we encrypt the zip as octet stream or some other form
console.error(`Incorrect content-type: ${response.ContentType}`);
throw new IncorrectFileTypeError();
}

if (response.Body) {
return response.Body;
if (response) {
return response;
}

throw new NoBodyError();
Expand Down
62 changes: 15 additions & 47 deletions tests/unit/domain/getZip.test.ts
View file
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
import { S3 } from 'aws-sdk';
import MissingBucketNameError from '../../../src/errors/MissingBucketNameError';
import NoBodyError from '../../../src/errors/NoBodyError';
import IncorrectFileTypeError from '../../../src/errors/IncorrectFileTypeError';
import MissingFolderNameError from '../../../src/errors/MissingFolderNameError';
import getZip from '../../../src/domain/getZip';
import FileNameError from '../../../src/errors/FileNameError';
import MissingBucketNameError from '../../../src/errors/MissingBucketNameError';
import MissingFolderNameError from '../../../src/errors/MissingFolderNameError';
import NoBodyError from '../../../src/errors/NoBodyError';
import ZipDetails from '../../../src/interfaces/ZipDetails';

describe('getZip', () => {
Expand Down Expand Up @@ -37,10 +36,9 @@ describe('getZip', () => {

it('returns an internal server error if there is no Body in the S3 request', async () => {
const mockS3 = ({} as unknown) as S3;
const mockPromise = jest.fn().mockReturnValue(Promise.resolve({ ContentType: 'application/octet-stream' }));
const mockGetObject = jest.fn().mockReturnValue({ promise: mockPromise });
const mockGetSignedUrlPromise = jest.fn().mockReturnValue(undefined);

mockS3.getObject = mockGetObject;
mockS3.getSignedUrlPromise = mockGetSignedUrlPromise;

const event: ZipDetails = {
adrDocumentId: '1234',
Expand All @@ -52,31 +50,11 @@ describe('getZip', () => {
expect(response.body).toEqual(error.message);
});

it('returns an 404 if the stored file is not a ZIP', async () => {
const mockS3 = ({} as unknown) as S3;
const mockPromise = jest
.fn()
.mockReturnValue(Promise.resolve({ Body: 'This is an image', ContentType: 'image/jpg' }));
const mockGetObject = jest.fn().mockReturnValue({ promise: mockPromise });

mockS3.getObject = mockGetObject;

const event: ZipDetails = {
adrDocumentId: '1234',
};
const response = await getZip(event, mockS3, 'bucket', 'folder', 'test');
const error = new IncorrectFileTypeError();

expect(response.statusCode).toBe(404);
expect(response.body).toEqual(error.message);
});

it('returns a not found error if the file is not found', async () => {
const mockS3 = ({} as unknown) as S3;
const mockPromise = jest.fn().mockReturnValue(Promise.reject(({ code: 'NoSuchKey' } as unknown) as Error)); // eslint-disable-line prefer-promise-reject-errors
const mockGetObject = jest.fn().mockReturnValue({ promise: mockPromise });
const mockGetSignedUrlPromise = jest.fn().mockRejectedValue({ code: 'NoSuchKey' } as unknown as Error);

mockS3.getObject = mockGetObject;
mockS3.getSignedUrlPromise = mockGetSignedUrlPromise;

const event: ZipDetails = {
adrDocumentId: '1234',
Expand All @@ -89,10 +67,9 @@ describe('getZip', () => {

it('returns an internal server error if the S3 get fails for any other reason', async () => {
const mockS3 = ({} as unknown) as S3;
const mockPromise = jest.fn().mockReturnValue(Promise.reject(({ code: 'Generic Error' } as unknown) as Error)); // eslint-disable-line prefer-promise-reject-errors
const mockGetObject = jest.fn().mockReturnValue({ promise: mockPromise });
const mockGetSignedUrlPromise = jest.fn().mockRejectedValue(({ code: 'Generic Error' } as unknown as Error));

mockS3.getObject = mockGetObject;
mockS3.getSignedUrlPromise = mockGetSignedUrlPromise;

const event: ZipDetails = {
adrDocumentId: '1234',
Expand All @@ -105,12 +82,9 @@ describe('getZip', () => {

it('returns a successful response if everything works', async () => {
const mockS3 = ({} as unknown) as S3;
const mockPromise = jest
.fn()
.mockReturnValue(Promise.resolve({ Body: 'Certificate Content', ContentType: 'application/octet-stream' }));
const mockGetObject = jest.fn().mockReturnValue({ promise: mockPromise });
const mockGetSignedUrlPromise = jest.fn().mockReturnValue('Certificate Content');

mockS3.getObject = mockGetObject;
mockS3.getSignedUrlPromise = mockGetSignedUrlPromise;

const event: ZipDetails = {
adrDocumentId: '1234',
Expand All @@ -124,12 +98,9 @@ describe('getZip', () => {
it('base64 encodes the response', async () => {
const mockS3 = ({} as unknown) as S3;
const body = Buffer.from('Certificate Content');
const mockPromise = jest
.fn()
.mockReturnValue(Promise.resolve({ Body: body, ContentType: 'application/octet-stream' }));
const mockGetObject = jest.fn().mockReturnValue({ promise: mockPromise });
const mockGetSignedUrlPromise = jest.fn().mockReturnValue(body);

mockS3.getObject = mockGetObject;
mockS3.getSignedUrlPromise = mockGetSignedUrlPromise;

const event: ZipDetails = {
adrDocumentId: '1234',
Expand All @@ -142,12 +113,9 @@ describe('getZip', () => {

it('ignores the folder check if the current environment is "local". Required for local testing', async () => {
const mockS3 = ({} as unknown) as S3;
const mockPromise = jest
.fn()
.mockReturnValue(Promise.resolve({ Body: 'Zip Content', ContentType: 'application/octet-stream' }));
const mockGetObject = jest.fn().mockReturnValue({ promise: mockPromise });
const mockGetSignedUrlPromise = jest.fn().mockReturnValue('Zip Content');

mockS3.getObject = mockGetObject;
mockS3.getSignedUrlPromise = mockGetSignedUrlPromise;

const event: ZipDetails = {
adrDocumentId: '1234',
Expand Down
63 changes: 23 additions & 40 deletions tests/unit/infrastructure/s3/s3ZipService.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,100 +6,83 @@ describe('S3 Zip Service', () => {
jest.resetAllMocks().restoreAllMocks();
});

it('passes the expected key to getObject if folder is defined', async () => {
it('passes the expected key to getSignedUrl if folder is defined', async () => {
const mockS3 = ({} as unknown) as S3;
const bucket = 'bucket';
const folder = 'folder';
const adrDocumentId = '1234';
const mockPromise = jest
.fn()
.mockResolvedValue({ Body: 'Success!', ContentType: 'application/octet-stream' });
const mockGetObject = jest.fn().mockReturnValue({ promise: mockPromise });
.mockResolvedValue({ Body: 'Success!', ContentType: 'application/json' });
const mockGetSignedUrlPromise = jest.fn().mockReturnValue({ promise: mockPromise });

mockS3.getObject = mockGetObject;
mockS3.getSignedUrlPromise = mockGetSignedUrlPromise;
await getFromS3(mockS3, bucket, folder, adrDocumentId).catch(() => {});

const firstCall = mockGetObject.mock.calls[0] as S3.GetObjectRequest[];
const firstCall = mockGetSignedUrlPromise.mock.calls[0] as S3.GetObjectRequest[];
const firstArg = firstCall[0];

expect(firstArg.Key).toBe(`${folder}/adr-documents/${adrDocumentId}.zip`);
expect(firstArg).toBe('getObject');
});

it('passes the expected key to getObject if folder is undefined', async () => {
it('passes the expected key to getSignedUrl if folder is undefined', async () => {
const mockS3 = ({} as unknown) as S3;
const bucket = 'bucket';
const folder: string | undefined = undefined;
const adrDocumentId = '1234';
const mockPromise = jest
.fn()
.mockReturnValue(Promise.resolve({ Body: 'Success!', ContentType: 'application/octet-stream' }));
const mockGetObject = jest.fn().mockReturnValue({ promise: mockPromise });
.mockReturnValue(Promise.resolve({ Body: 'Success!', ContentType: 'application/json' }));
const mockGetSignedUrlPromise = jest.fn().mockReturnValue({ promise: mockPromise });

mockS3.getObject = mockGetObject;
mockS3.getSignedUrlPromise = mockGetSignedUrlPromise;
await getFromS3(mockS3, bucket, folder, adrDocumentId).catch(() => {});

const firstCall = mockGetObject.mock.calls[0] as S3.GetObjectRequest[];
const firstCall = mockGetSignedUrlPromise.mock.calls[0] as S3.GetObjectRequest[];
const firstArg = firstCall[0];

expect(firstArg.Key).toBe(`/adr-documents/${adrDocumentId}.zip`);
expect(firstArg).toBe('getObject');
});

it('passes the bucket to getObject', () => {
it('passes the bucket to getSignedUrl', () => {
const mockS3 = ({} as unknown) as S3;
const bucket = 'bucket';
const folder = 'folder';
const adrDocumentId = '1234';
const mockPromise = jest
.fn()
.mockReturnValue(Promise.resolve({ Body: 'Success!', ContentType: 'application/octet-stream' }));
const mockGetObject = jest.fn().mockReturnValue({ promise: mockPromise });
.mockReturnValue(Promise.resolve({ Body: 'Success!', ContentType: 'application/json' }));
const mockGetSignedUrlPromise = jest.fn().mockReturnValue({ promise: mockPromise });

mockS3.getObject = mockGetObject;
mockS3.getSignedUrlPromise = mockGetSignedUrlPromise;
getFromS3(mockS3, bucket, folder, adrDocumentId).catch(() => {});

const firstCall = mockGetObject.mock.calls[0] as S3.GetObjectRequest[];
const firstArg = firstCall[0];
const firstCall = mockGetSignedUrlPromise.mock.calls[0] as S3.GetObjectRequest[];
const secondArg = firstCall[1];

expect(firstArg.Bucket).toEqual(bucket);
expect(secondArg.Bucket).toEqual(bucket);
});

it('returns the expected output', async () => {
const mockS3 = ({} as unknown) as S3;
const bucket = 'bucket';
const folder = 'folder';
const adrDocumentId = '1234';
const mockPromise = jest
.fn()
.mockReturnValue(Promise.resolve({ Body: 'Success!', ContentType: 'application/octet-stream' }));
const mockGetObject = jest.fn().mockReturnValue({ promise: mockPromise });
const mockGetSignedUrlPromise = jest.fn().mockReturnValue('Success!');

mockS3.getObject = mockGetObject;
mockS3.getSignedUrlPromise = mockGetSignedUrlPromise;

expect(await getFromS3(mockS3, bucket, folder, adrDocumentId)).toBe('Success!');
});

it('throws an error if the response is not a ZIP', async () => {
const mockS3 = ({} as unknown) as S3;
const bucket = 'bucket';
const folder = 'folder';
const adrDocumentId = '1234';
const mockPromise = jest.fn().mockReturnValue(Promise.resolve({ Body: 'Success!', ContentType: 'image/jpg' }));
const mockGetObject = jest.fn().mockReturnValue({ promise: mockPromise });

mockS3.getObject = mockGetObject;

await expect(getFromS3(mockS3, bucket, folder, adrDocumentId)).rejects.toThrow();
});

it('throws an error if there is no body in the response', async () => {
const mockS3 = ({} as unknown) as S3;
const bucket = 'bucket';
const folder = 'folder';
const adrDocumentId = '1234';
const mockPromise = jest.fn().mockReturnValue(Promise.resolve({ ContentType: 'application/octet-stream' }));
const mockGetObject = jest.fn().mockReturnValue({ promise: mockPromise });
const mockGetSignedUrlPromise = jest.fn().mockReturnValue(undefined);

mockS3.getObject = mockGetObject;
mockS3.getSignedUrlPromise = mockGetSignedUrlPromise;

await expect(getFromS3(mockS3, bucket, folder, adrDocumentId)).rejects.toThrow();
});
Expand Down

0 comments on commit 12d8ebd

Please sign in to comment.