Skip to content

Commit

Permalink
feat(storage): Add API support for Expected Bucket Owner (#13914)
Browse files Browse the repository at this point in the history 
* Update Top/Internal API for expected bucket owner feat

---------

Co-authored-by: JoonWon Choi <joonwonc@amazon.com>
  • Loading branch information
@joon-won
joon-won and JoonWon Choi authored Oct 15, 2024
1 parent 2cc870c commit 96c2dc7
Show file tree
Hide file tree
Showing 32 changed files with 653 additions and 21 deletions.
14 changes: 7 additions & 7 deletions packages/aws-amplify/package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -461,43 +461,43 @@
"name": "[Storage] copy (S3)",
"path": "./dist/esm/storage/index.mjs",
"import": "{ copy }",
"limit": "15.90 kB"
"limit": "16.05 kB"
},
{
"name": "[Storage] downloadData (S3)",
"path": "./dist/esm/storage/index.mjs",
"import": "{ downloadData }",
"limit": "16.27 kB"
"limit": "16.40 kB"
},
{
"name": "[Storage] getProperties (S3)",
"path": "./dist/esm/storage/index.mjs",
"import": "{ getProperties }",
"limit": "15.50 kB"
"limit": "15.65 kB"
},
{
"name": "[Storage] getUrl (S3)",
"path": "./dist/esm/storage/index.mjs",
"import": "{ getUrl }",
"limit": "16.74 kB"
"limit": "16.90 kB"
},
{
"name": "[Storage] list (S3)",
"path": "./dist/esm/storage/index.mjs",
"import": "{ list }",
"limit": "16.26 kB"
"limit": "16.35 kB"
},
{
"name": "[Storage] remove (S3)",
"path": "./dist/esm/storage/index.mjs",
"import": "{ remove }",
"limit": "15.37 kB"
"limit": "15.50 kB"
},
{
"name": "[Storage] uploadData (S3)",
"path": "./dist/esm/storage/index.mjs",
"import": "{ uploadData }",
"limit": "22.19 kB"
"limit": "22.35 kB"
}
]
}
51 changes: 51 additions & 0 deletions packages/storage/__tests__/providers/s3/apis/copy.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ const bucket = 'bucket';
const region = 'region';
const targetIdentityId = 'targetIdentityId';
const defaultIdentityId = 'defaultIdentityId';
const validBucketOwner = '111122223333';
const validBucketOwner2 = '123456789012';
const credentials: AWSCredentials = {
accessKeyId: 'accessKeyId',
sessionToken: 'sessionToken',
Expand Down Expand Up @@ -264,6 +266,31 @@ describe('copy API', () => {
}),
);
});

describe('ExpectedBucketOwner passed in options', () => {
it('should include expectedBucketOwner in headers when provided', async () => {
const mockEtag = 'mock-etag';
await copyWrapper({
source: {
key: 'sourceKey',
eTag: mockEtag,
expectedBucketOwner: validBucketOwner,
},
destination: {
key: 'destinationKey',
expectedBucketOwner: validBucketOwner2,
},
});
expect(copyObject).toHaveBeenCalledTimes(1);
expect(copyObject).toHaveBeenCalledWith(
expect.anything(),
expect.objectContaining({
ExpectedSourceBucketOwner: validBucketOwner,
ExpectedBucketOwner: validBucketOwner2,
}),
);
});
});
});

describe('With path', () => {
Expand Down Expand Up @@ -382,6 +409,30 @@ describe('copy API', () => {
}),
);
});
describe('ExpectedBucketOwner passed in options', () => {
it('should include expectedBucketOwner in headers when provided', async () => {
const mockEtag = 'mock-etag';
await copyWrapper({
source: {
path: 'public/sourceKey',
eTag: mockEtag,
expectedBucketOwner: validBucketOwner,
},
destination: {
path: 'public/destinationKey',
expectedBucketOwner: validBucketOwner2,
},
});
expect(copyObject).toHaveBeenCalledTimes(1);
expect(copyObject).toHaveBeenCalledWith(
expect.anything(),
expect.objectContaining({
ExpectedSourceBucketOwner: validBucketOwner,
ExpectedBucketOwner: validBucketOwner2,
}),
);
});
});
});
});

Expand Down
47 changes: 47 additions & 0 deletions packages/storage/__tests__/providers/s3/apis/downloadData.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ const bucket = 'bucket';
const region = 'region';
const targetIdentityId = 'targetIdentityId';
const defaultIdentityId = 'defaultIdentityId';
const validBucketOwner = '111122223333';
const mockDownloadResultBase = {
body: 'body',
lastModified: 'lastModified',
Expand Down Expand Up @@ -286,6 +287,29 @@ describe('downloadData with key', () => {
);
});
});

describe('ExpectedBucketOwner passed in options', () => {
it('should include expectedBucketOwner in headers when provided', async () => {
(getObject as jest.Mock).mockResolvedValueOnce({ Body: 'body' });
downloadData({
key: inputKey,
options: {
expectedBucketOwner: validBucketOwner,
},
});

const { job } = mockCreateDownloadTask.mock.calls[0][0];
await job();

expect(getObject).toHaveBeenCalledTimes(1);
await expect(getObject).toBeLastCalledWithConfigAndInput(
expect.any(Object),
expect.objectContaining({
ExpectedBucketOwner: validBucketOwner,
}),
);
});
});
});

describe('downloadData with path', () => {
Expand Down Expand Up @@ -497,4 +521,27 @@ describe('downloadData with path', () => {
);
});
});

describe('ExpectedBucketOwner passed in options', () => {
it('should include expectedBucketOwner in headers when provided', async () => {
(getObject as jest.Mock).mockResolvedValueOnce({ Body: 'body' });
downloadData({
path: inputKey,
options: {
expectedBucketOwner: validBucketOwner,
},
});

const { job } = mockCreateDownloadTask.mock.calls[0][0];
await job();

expect(getObject).toHaveBeenCalledTimes(1);
await expect(getObject).toBeLastCalledWithConfigAndInput(
expect.any(Object),
expect.objectContaining({
ExpectedBucketOwner: validBucketOwner,
}),
);
});
});
});
89 changes: 89 additions & 0 deletions packages/storage/__tests__/providers/s3/apis/getProperties.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ const inputKey = 'key';
const inputPath = 'path';
const targetIdentityId = 'targetIdentityId';
const defaultIdentityId = 'defaultIdentityId';
const validBucketOwner = '111122223333';
const invalidBucketOwner = '123';

const expectedResult = {
size: 100,
Expand Down Expand Up @@ -412,3 +414,90 @@ describe('Happy cases: With path', () => {
});
});
});

describe(`getProperties with path and Expected Bucket Owner`, () => {
const getPropertiesWrapper = (
input: GetPropertiesWithPathInput,
): Promise<GetPropertiesWithPathOutput> => getProperties(input);
beforeAll(() => {
mockFetchAuthSession.mockResolvedValue({
credentials,
identityId: defaultIdentityId,
});
mockGetConfig.mockReturnValue({
Storage: {
S3: {
bucket,
region,
buckets: { 'default-bucket': { bucketName: bucket, region } },
},
},
});
});

afterEach(() => {
jest.clearAllMocks();
});

it('should pass expectedBucketOwner to headObject', async () => {
const path = 'public/expectedbucketowner_test';

await getPropertiesWrapper({
path,
options: {
expectedBucketOwner: validBucketOwner,
},
});

expect(headObject).toHaveBeenCalledTimes(1);
await expect(headObject).toBeLastCalledWithConfigAndInput(
{
credentials,
region,
userAgentValue: expect.any(String),
},
{
Bucket: bucket,
ExpectedBucketOwner: validBucketOwner,
Key: path,
},
);
});

it('headObject should not expose expectedBucketOwner when not provided', async () => {
const path = 'public/expectedbucketowner_test';

await getPropertiesWrapper({
path,
options: {},
});

expect(headObject).toHaveBeenCalledTimes(1);
await expect(headObject).toBeLastCalledWithConfigAndInput(
{
credentials,
region,
userAgentValue: expect.any(String),
},
{
Bucket: bucket,
Key: path,
},
);
});

it('should throw error on invalid bucket owner id', async () => {
const path = 'public/expectedbucketowner_test';

await expect(
getPropertiesWrapper({
path,
options: {
expectedBucketOwner: invalidBucketOwner,
},
}),
).rejects.toThrow('Invalid AWS account ID was provided.');

expect(headObject).not.toHaveBeenCalled();
});
});
92 changes: 92 additions & 0 deletions packages/storage/__tests__/providers/s3/apis/getUrl.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ const credentials: AWSCredentials = {
const targetIdentityId = 'targetIdentityId';
const defaultIdentityId = 'defaultIdentityId';
const mockURL = new URL('https://google.com');
const validBucketOwner = '111122223333';
const invalidBucketOwner = '123';

describe('getUrl test with key', () => {
const getUrlWrapper = (input: GetUrlInput): Promise<GetUrlOutput> =>
Expand Down Expand Up @@ -488,3 +490,93 @@ describe('getUrl test with path', () => {
});
});
});

describe(`getURL with path and Expected Bucket Owner`, () => {
const getUrlWrapper = (
input: GetUrlWithPathInput,
): Promise<GetUrlWithPathOutput> => getUrl(input);
beforeAll(() => {
mockFetchAuthSession.mockResolvedValue({
credentials,
identityId: defaultIdentityId,
});
mockGetConfig.mockReturnValue({
Storage: {
S3: {
bucket,
region,
buckets: { 'default-bucket': { bucketName: bucket, region } },
},
},
});
});

afterEach(() => {
jest.clearAllMocks();
});

it('should pass expectedBucketOwner to getPresignedGetObjectUrl', async () => {
const path = 'public/expectedbucketowner_test';

await getUrlWrapper({
path,
options: {
expiresIn: 300,
expectedBucketOwner: validBucketOwner,
},
});

expect(getPresignedGetObjectUrl).toHaveBeenCalledTimes(1);
await expect(getPresignedGetObjectUrl).toBeLastCalledWithConfigAndInput(
{
credentials,
region,
expiration: expect.any(Number),
},
{
Bucket: bucket,
ExpectedBucketOwner: validBucketOwner,
Key: path,
},
);
});

it('getPresignedGetObjectUrl should not expose expectedBucketOwner when not provided', async () => {
const path = 'public/expectedbucketowner_test';

await getUrlWrapper({
path,
options: {
expiresIn: 300,
},
});

expect(getPresignedGetObjectUrl).toHaveBeenCalledTimes(1);
await expect(getPresignedGetObjectUrl).toBeLastCalledWithConfigAndInput(
{
credentials,
region,
expiration: expect.any(Number),
},
{
Bucket: bucket,
Key: path,
},
);
});

it('should throw error on invalid bucket owner id', async () => {
const path = 'public/expectedbucketowner_test';

await expect(
getUrlWrapper({
path,
options: {
expectedBucketOwner: invalidBucketOwner,
},
}),
).rejects.toThrow('Invalid AWS account ID was provided.');

expect(getPresignedGetObjectUrl).not.toHaveBeenCalled();
});
});
Loading

0 comments on commit 96c2dc7

Please sign in to comment.