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

fix(storage): credentials expires after 1 hour #13329

Merged
merged 2 commits into from
May 16, 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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@
"eslint-plugin-prettier": "^5.1.3",
"eslint-plugin-promise": "^6.1.1",
"eslint-plugin-unused-imports": "^3.0.0",
"expect": "^29.7.0",
"glob": "^10.3.10",
"husky": "^9.0.11",
"jest": "^29.7.0",
Expand Down
40 changes: 25 additions & 15 deletions packages/storage/__tests__/providers/s3/apis/copy.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
CopyWithPathInput,
CopyWithPathOutput,
} from '../../../../src/providers/s3/types';
import './testUtils';

jest.mock('../../../../src/providers/s3/utils/client');
jest.mock('@aws-amplify/core', () => ({
Expand Down Expand Up @@ -186,11 +187,14 @@ describe('copy API', () => {
});
expect(key).toEqual(destinationKey);
expect(copyObject).toHaveBeenCalledTimes(1);
expect(copyObject).toHaveBeenCalledWith(copyObjectClientConfig, {
...copyObjectClientBaseParams,
CopySource: expectedSourceKey,
Key: expectedDestinationKey,
});
await expect(copyObject).toBeLastCalledWithConfigAndInput(
copyObjectClientConfig,
{
...copyObjectClientBaseParams,
CopySource: expectedSourceKey,
Key: expectedDestinationKey,
},
);
});
},
);
Expand Down Expand Up @@ -239,11 +243,14 @@ describe('copy API', () => {
});
expect(path).toEqual(expectedDestinationPath);
expect(copyObject).toHaveBeenCalledTimes(1);
expect(copyObject).toHaveBeenCalledWith(copyObjectClientConfig, {
...copyObjectClientBaseParams,
CopySource: `${bucket}/${expectedSourcePath}`,
Key: expectedDestinationPath,
});
await expect(copyObject).toBeLastCalledWithConfigAndInput(
copyObjectClientConfig,
{
...copyObjectClientBaseParams,
CopySource: `${bucket}/${expectedSourcePath}`,
Key: expectedDestinationPath,
},
);
},
);
});
Expand All @@ -269,11 +276,14 @@ describe('copy API', () => {
});
} catch (error: any) {
expect(copyObject).toHaveBeenCalledTimes(1);
expect(copyObject).toHaveBeenCalledWith(copyObjectClientConfig, {
...copyObjectClientBaseParams,
CopySource: `${bucket}/public/${missingSourceKey}`,
Key: `public/${destinationKey}`,
});
await expect(copyObject).toBeLastCalledWithConfigAndInput(
copyObjectClientConfig,
{
...copyObjectClientBaseParams,
CopySource: `${bucket}/public/${missingSourceKey}`,
Key: `public/${destinationKey}`,
},
);
expect(error.$metadata.httpStatusCode).toBe(404);
}
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import {
ItemWithKey,
ItemWithPath,
} from '../../../../src/providers/s3/types/outputs';
import './testUtils';

jest.mock('../../../../src/providers/s3/utils/client');
jest.mock('../../../../src/providers/s3/utils');
Expand Down Expand Up @@ -142,7 +143,7 @@ describe('downloadData with key', () => {
body: 'body',
});
expect(getObject).toHaveBeenCalledTimes(1);
expect(getObject).toHaveBeenCalledWith(
await expect(getObject).toBeLastCalledWithConfigAndInput(
{
credentials,
region,
Expand Down Expand Up @@ -288,7 +289,7 @@ describe('downloadData with path', () => {
body: 'body',
});
expect(getObject).toHaveBeenCalledTimes(1);
expect(getObject).toHaveBeenCalledWith(
await expect(getObject).toBeLastCalledWithConfigAndInput(
{
credentials,
region,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
GetPropertiesWithPathInput,
GetPropertiesWithPathOutput,
} from '../../../../src/providers/s3/types';
import './testUtils';

jest.mock('../../../../src/providers/s3/utils/client');
jest.mock('@aws-amplify/core', () => ({
Expand Down Expand Up @@ -145,7 +146,10 @@ describe('getProperties with key', () => {
...expectedResult,
});
expect(headObject).toHaveBeenCalledTimes(1);
expect(headObject).toHaveBeenCalledWith(config, headObjectOptions);
await expect(headObject).toBeLastCalledWithConfigAndInput(
config,
headObjectOptions,
);
},
);
});
Expand All @@ -166,7 +170,7 @@ describe('getProperties with key', () => {
await getPropertiesWrapper({ key: inputKey });
} catch (error: any) {
expect(headObject).toHaveBeenCalledTimes(1);
expect(headObject).toHaveBeenCalledWith(
await expect(headObject).toBeLastCalledWithConfigAndInput(
{
credentials,
region: 'region',
Expand Down Expand Up @@ -265,7 +269,10 @@ describe('Happy cases: With path', () => {
...expectedResult,
});
expect(headObject).toHaveBeenCalledTimes(1);
expect(headObject).toHaveBeenCalledWith(config, headObjectOptions);
await expect(headObject).toBeLastCalledWithConfigAndInput(
config,
headObjectOptions,
);
},
);
});
Expand All @@ -286,7 +293,7 @@ describe('Happy cases: With path', () => {
await getPropertiesWrapper({ path: inputPath });
} catch (error: any) {
expect(headObject).toHaveBeenCalledTimes(1);
expect(headObject).toHaveBeenCalledWith(
await expect(headObject).toBeLastCalledWithConfigAndInput(
{
credentials,
region: 'region',
Expand Down
31 changes: 15 additions & 16 deletions packages/storage/__tests__/providers/s3/apis/getUrl.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
GetUrlWithPathInput,
GetUrlWithPathOutput,
} from '../../../../src/providers/s3/types';
import './testUtils';

jest.mock('../../../../src/providers/s3/utils/client');
jest.mock('@aws-amplify/core', () => ({
Expand All @@ -31,8 +32,8 @@ jest.mock('@aws-amplify/core', () => ({

const bucket = 'bucket';
const region = 'region';
const mockFetchAuthSession = Amplify.Auth.fetchAuthSession as jest.Mock;
const mockGetConfig = Amplify.getConfig as jest.Mock;
const mockFetchAuthSession = jest.mocked(Amplify.Auth.fetchAuthSession);
const mockGetConfig = jest.mocked(Amplify.getConfig);
const credentials: AWSCredentials = {
accessKeyId: 'accessKeyId',
sessionToken: 'sessionToken',
Expand Down Expand Up @@ -68,19 +69,15 @@ describe('getUrl test with key', () => {
};
const key = 'key';
beforeEach(() => {
(headObject as jest.MockedFunction<typeof headObject>).mockResolvedValue({
jest.mocked(headObject).mockResolvedValue({
ashika112 marked this conversation as resolved.
Show resolved Hide resolved
ContentLength: 100,
ContentType: 'text/plain',
ETag: 'etag',
LastModified: new Date('01-01-1980'),
Metadata: { meta: 'value' },
$metadata: {} as any,
});
(
getPresignedGetObjectUrl as jest.MockedFunction<
typeof getPresignedGetObjectUrl
>
).mockResolvedValue(mockURL);
jest.mocked(getPresignedGetObjectUrl).mockResolvedValue(mockURL);
});
afterEach(() => {
jest.clearAllMocks();
Expand Down Expand Up @@ -131,7 +128,10 @@ describe('getUrl test with key', () => {
};
expect(getPresignedGetObjectUrl).toHaveBeenCalledTimes(1);
expect(headObject).toHaveBeenCalledTimes(1);
expect(headObject).toHaveBeenCalledWith(config, headObjectOptions);
await expect(headObject).toBeLastCalledWithConfigAndInput(
config,
headObjectOptions,
);
expect({ url, expiresAt }).toEqual(expectedResult);
},
);
Expand Down Expand Up @@ -187,19 +187,15 @@ describe('getUrl test with path', () => {
userAgentValue: expect.any(String),
};
beforeEach(() => {
(headObject as jest.MockedFunction<typeof headObject>).mockResolvedValue({
jest.mocked(headObject).mockResolvedValue({
ContentLength: 100,
ContentType: 'text/plain',
ETag: 'etag',
LastModified: new Date('01-01-1980'),
Metadata: { meta: 'value' },
$metadata: {} as any,
});
(
getPresignedGetObjectUrl as jest.MockedFunction<
typeof getPresignedGetObjectUrl
>
).mockResolvedValue(mockURL);
jest.mocked(getPresignedGetObjectUrl).mockResolvedValue(mockURL);
});
afterEach(() => {
jest.clearAllMocks();
Expand Down Expand Up @@ -229,7 +225,10 @@ describe('getUrl test with path', () => {
});
expect(getPresignedGetObjectUrl).toHaveBeenCalledTimes(1);
expect(headObject).toHaveBeenCalledTimes(1);
expect(headObject).toHaveBeenCalledWith(config, headObjectOptions);
await expect(headObject).toBeLastCalledWithConfigAndInput(
config,
headObjectOptions,
);
expect({ url, expiresAt }).toEqual({
url: mockURL,
expiresAt: expect.any(Date),
Expand Down