Skip to content

Commit

Permalink
fix: resolve conflicts
Browse files Browse the repository at this point in the history
  • Loading branch information
Sridhar committed Aug 15, 2023
2 parents 0b7b3da + 1348b0a commit 8ddd6b6
Show file tree
Hide file tree
Showing 16 changed files with 148 additions and 89 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/codeql.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ on:
push:
branches: ['*']
pull_request:
branches: ['main']
branches: ['main', 'next']
schedule:
# Run every Tuesday at midnight GMT
- cron: '0 0 * * 2'
Expand Down
8 changes: 0 additions & 8 deletions packages/storage/__tests__/AwsClients/S3/cases/getObject.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,14 +66,6 @@ const getObjectHappyCase: ApiFunctionalTestCase<typeof getObject> = [
{
Bucket: 'bucket',
Key: 'key',
ResponseCacheControl: 'ResponseCacheControl',
ResponseContentDisposition: 'ResponseContentDisposition',
ResponseContentEncoding: 'ResponseContentEncoding',
ResponseContentLanguage: 'ResponseContentLanguage',
ResponseContentType: 'ResponseContentType',
SSECustomerAlgorithm: 'SSECustomerAlgorithm',
SSECustomerKey: 'SSECustomerKey',
SSECustomerKeyMD5: 'SSECustomerKeyMD5',
},
expect.objectContaining({
url: expect.objectContaining({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ describe('serializeGetObjectRequest', () => {
Key: 'key',
}
);
const actualUrl = new URL(actual);
const actualUrl = actual;
expect(actualUrl.hostname).toEqual(
`bucket.s3.${defaultConfig.region}.amazonaws.com`
);
Expand Down
8 changes: 0 additions & 8 deletions packages/storage/__tests__/Storage-unit-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -657,17 +657,9 @@ describe('Storage', () => {
test('get object with all available config', async () => {
await storage.get('key', {
download: false,
contentType: 'text/plain',
contentDisposition: 'contentDisposition',
contentLanguage: 'contentLanguage',
contentEncoding: 'contentEncoding',
cacheControl: 'cacheControl',
identityId: 'identityId',
expires: 100,
progressCallback: () => {},
SSECustomerAlgorithm: 'aes256',
SSECustomerKey: 'key',
SSECustomerKeyMD5: 'md5',
customPrefix: {
public: 'public',
protected: 'protected',
Expand Down
48 changes: 48 additions & 0 deletions packages/storage/__tests__/providers/s3/getUrl.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
import { getProperties, getUrl } from '../../../src/providers/s3/apis';
jest.mock('../../../src/AwsClients/S3');
const headObject = jest.fn();

const getPresignedGetObjectUrl = jest.fn();
describe('getUrl happy path case', () => {
test.skip('get presigned url happy case', async () => {
// TODO[kvramya] test credentials
headObject.mockImplementation(() => {
return {
Key: 'key',
ContentLength: '100',
ContentType: 'text/plain',
ETag: 'etag',
LastModified: 'last-modified',
Metadata: { key: 'value' },
};
});
getPresignedGetObjectUrl.mockReturnValueOnce({ url: new URL('url') });
expect(getPresignedGetObjectUrl).toBeCalledTimes(1);
const result = await getUrl({ key: 'key' });
expect(result.url).toEqual({
url: new URL('url'),
});
});
});

describe('getUrl error path case', () => {
test.skip('Should return not found error when the object is not found', async () => {
expect.assertions(2)
// TODO[kvramya] test credentials
headObject.mockImplementation(() =>
Object.assign(new Error(), {
$metadata: { httpStatusCode: 404 },
name: 'NotFound',
})
);
try {
await getUrl({
key: 'invalid_key',
options: { validateObjectExistence: true },
});
} catch (error) {
expect(getProperties).toBeCalledTimes(1);
expect(error.$metadata?.httpStatusCode).toBe(404);
}
});
});
23 changes: 4 additions & 19 deletions packages/storage/src/AwsClients/S3/getObject.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,33 +27,18 @@ import {
map,
parseXmlError,
s3TransferHandler,
serializeObjectSsecOptionsToHeaders,
serializePathnameObjectKey,
CONTENT_SHA256_HEADER,
} from './utils';

export type GetObjectInput = Pick<
GetObjectCommandInput,
| 'Bucket'
| 'Key'
| 'ResponseCacheControl'
| 'ResponseContentDisposition'
| 'ResponseContentEncoding'
| 'ResponseContentLanguage'
| 'ResponseContentType'
| 'SSECustomerAlgorithm'
| 'SSECustomerKey'
// TODO(AllanZhengYP): remove in V6.
| 'SSECustomerKeyMD5'
>;
export type GetObjectInput = Pick<GetObjectCommandInput, 'Bucket' | 'Key'>;

export type GetObjectOutput = GetObjectCommandOutput;

const getObjectSerializer = async (
input: GetObjectInput,
endpoint: Endpoint
): Promise<HttpRequest> => {
const headers = await serializeObjectSsecOptionsToHeaders(input);
const query = map(input, {
'response-cache-control': 'ResponseCacheControl',
'response-content-disposition': 'ResponseContentDisposition',
Expand All @@ -66,7 +51,7 @@ const getObjectSerializer = async (
url.search = new URLSearchParams(query).toString();
return {
method: 'GET',
headers,
headers: {},
url,
};
};
Expand Down Expand Up @@ -145,7 +130,7 @@ export const getObject = composeServiceApi(
export const getPresignedGetObjectUrl = async (
config: UserAgentOptions & PresignUrlOptions & S3EndpointResolverOptions,
input: GetObjectInput
): Promise<string> => {
): Promise<URL> => {
const endpoint = defaultConfig.endpointResolver(config, input);
const { url, headers, method } = await getObjectSerializer(input, endpoint);

Expand All @@ -169,5 +154,5 @@ export const getPresignedGetObjectUrl = async (
...defaultConfig,
...config,
}
).toString();
);
};
14 changes: 9 additions & 5 deletions packages/storage/src/errors/types/validation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,23 +9,27 @@ export enum StorageValidationErrorCode {
NoKey = 'NoKey',
NoBucket = 'NoBucket',
NoRegion = 'NoRegion',
UrlExpirationMaxLimitExceed = 'UrlExpirationMaxLimitExceed',
}

export const validationErrorMap: AmplifyErrorMap<StorageValidationErrorCode> = {
[StorageValidationErrorCode.NoCredentials]: {
message: 'Credentials should not be empty',
message: 'Credentials should not be empty.',
},
[StorageValidationErrorCode.NoIdentityId]: {
message:
'Missing identity ID when accessing objects in protected or private access level',
'Missing identity ID when accessing objects in protected or private access level.',
},
[StorageValidationErrorCode.NoKey]: {
message: 'Missing key in getProperties api call',
message: 'Missing key in getProperties api call.',
},
[StorageValidationErrorCode.NoBucket]: {
message: 'Missing bucket name while accessing object',
message: 'Missing bucket name while accessing object.',
},
[StorageValidationErrorCode.NoRegion]: {
message: 'Missing region while accessing object',
message: 'Missing region while accessing object.',
},
[StorageValidationErrorCode.UrlExpirationMaxLimitExceed]: {
message: 'Url Expiration can not be greater than 7 Days.',
},
};
27 changes: 1 addition & 26 deletions packages/storage/src/providers/AWSS3Provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -371,16 +371,8 @@ export class AWSS3Provider implements StorageProvider {
const opt = Object.assign({}, this._config, config);
const {
download,
cacheControl,
contentDisposition,
contentEncoding,
contentLanguage,
contentType,
expires,
track,
SSECustomerAlgorithm,
SSECustomerKey,
SSECustomerKeyMD5,
progressCallback,
validateObjectExistence = false,
} = opt;
Expand All @@ -398,23 +390,6 @@ export class AWSS3Provider implements StorageProvider {
Bucket: bucket,
Key: final_key,
};
// See: https://docs.aws.amazon.com/AWSJavaScriptSDK/latest/AWS/S3.html#getObject-property
if (cacheControl) params.ResponseCacheControl = cacheControl;
if (contentDisposition)
params.ResponseContentDisposition = contentDisposition;
if (contentEncoding) params.ResponseContentEncoding = contentEncoding;
if (contentLanguage) params.ResponseContentLanguage = contentLanguage;
if (contentType) params.ResponseContentType = contentType;
if (SSECustomerAlgorithm) {
params.SSECustomerAlgorithm = SSECustomerAlgorithm;
}
if (SSECustomerKey) {
params.SSECustomerKey = SSECustomerKey;
}
if (SSECustomerKeyMD5) {
params.SSECustomerKeyMD5 = SSECustomerKeyMD5;
}

if (download === true) {
try {
if (progressCallback) {
Expand Down Expand Up @@ -492,7 +467,7 @@ export class AWSS3Provider implements StorageProvider {
null,
`Signed URL: ${url}`
);
return url;
return url.toString();
} catch (error) {
logger.warn('get signed url error', error);
dispatchStorageEvent(
Expand Down
4 changes: 2 additions & 2 deletions packages/storage/src/providers/s3/apis/getProperties.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { headObject } from '../../../AwsClients/S3';
import { StorageOptions, StorageOperationRequest } from '../../../types';
import { assertValidationError } from '../../../errors/utils/assertValidationError';
import { StorageValidationErrorCode } from '../../../errors/types/validation';
import { StorageException, S3GetPropertiesResult } from '../types';
import { S3Exception, S3GetPropertiesResult } from '../types';
import {
resolveStorageConfig,
getKeyWithPrefix,
Expand All @@ -18,7 +18,7 @@ import {
*
* @param {StorageOperationRequest} req The request to make an API call.
* @returns {Promise<S3GetPropertiesResult>} A promise that resolves the properties.
* @throws A {@link StorageException} when the underlying S3 service returned error.
* @throws A {@link S3Exception} when the underlying S3 service returned error.
* @throws A {@link StorageValidationErrorCode} when API call parameters are invalid.
*/
export const getProperties = async function (
Expand Down
74 changes: 70 additions & 4 deletions packages/storage/src/providers/s3/apis/getUrl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,74 @@

import { StorageDownloadDataRequest } from '../../../types';
import { S3GetUrlOptions, S3GetUrlResult } from '../types';
import { StorageValidationErrorCode } from '../../../errors/types/validation';
import {
SERVICE_NAME as S3_SERVICE_NAME,
GetObjectInput,
getPresignedGetObjectUrl,
} from '../../../AwsClients/S3';
import { getProperties } from './getProperties';
import { S3Exception } from '../types/errors';
import {
getKeyWithPrefix,
resolveCredentials,
resolveStorageConfig,
} from '../utils';
import { assertValidationError } from '../../../errors/utils/assertValidationError';
const DEFAULT_PRESIGN_EXPIRATION = 900;
const MAX_URL_EXPIRATION = 7 * 24 * 60 * 60 * 1000;

// TODO: pending implementation
export declare const getUrl: (
params: StorageDownloadDataRequest<S3GetUrlOptions>
) => Promise<S3GetUrlResult>;
/**
* Get Presigned url of the object
*
* @param {StorageDownloadDataRequest<S3GetUrlOptions>} The request object
* @return {Promise<S3GetUrlResult>} url of the object
* @throws service: {@link S3Exception} - thrown when checking for existence of the object
* @throws validation: {@link StorageValidationErrorCode } - Validation errors
* thrown either username or key are not defined.
*
* TODO: add config errors
*
*/

export const getUrl = async function (
req: StorageDownloadDataRequest<S3GetUrlOptions>
): Promise<S3GetUrlResult> {
const options = req?.options;
const { credentials, identityId } = await resolveCredentials();
const { defaultAccessLevel, bucket, region } = resolveStorageConfig();
const { key, options: { accessLevel = defaultAccessLevel } = {} } = req;
assertValidationError(!!key, StorageValidationErrorCode.NoKey);
if (options?.validateObjectExistence) {
await getProperties({ key });
}
const finalKey = getKeyWithPrefix(accessLevel, identityId, key);
const getUrlParams: GetObjectInput = {
Bucket: bucket,
Key: finalKey,
};
const getUrlOptions = {
accessLevel,
credentials,
expiration: options?.expiration ?? DEFAULT_PRESIGN_EXPIRATION,
signingRegion: region,
region,
signingService: S3_SERVICE_NAME,
};

let urlExpiration = options?.expiration ?? DEFAULT_PRESIGN_EXPIRATION;
assertValidationError(
urlExpiration > MAX_URL_EXPIRATION,
StorageValidationErrorCode.UrlExpirationMaxLimitExceed
);
const awsCredExpiration = credentials?.expiration;
// expiresAt is the minimum of credential expiration and url expiration
urlExpiration =
urlExpiration < awsCredExpiration.getTime()
? urlExpiration
: awsCredExpiration.getTime();
return {
url: await getPresignedGetObjectUrl(getUrlOptions, getUrlParams),
expiresAt: new Date(Date.now() + urlExpiration),
};
};
1 change: 1 addition & 0 deletions packages/storage/src/providers/s3/apis/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,4 @@ export { remove } from './remove';
export { list } from './list';
export { getProperties } from './getProperties';
export { copy } from './copy';
export { getUrl } from './getUrl';
6 changes: 3 additions & 3 deletions packages/storage/src/providers/s3/apis/list.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import {
} from '../../../types';
import {
S3ListOutputItem,
StorageException,
S3Exception,
S3ListAllResult,
S3ListPaginateResult,
} from '../types';
Expand All @@ -28,7 +28,7 @@ type S3ListApi = {
* List all bucket objects
* @param {StorageListRequest<StorageListAllOptions>} req - The request object
* @return {Promise<S3ListAllResult>} - Promise resolves to list of keys and metadata for all objects in path
* @throws service: {@link StorageException} - S3 service errors thrown while getting properties
* @throws service: {@link S3Exception} - S3 service errors thrown while getting properties
* @throws validation: {@link StorageValidationErrorCode } - Validation errors thrown
*/
(req: StorageListRequest<StorageListAllOptions>): Promise<S3ListAllResult>;
Expand All @@ -37,7 +37,7 @@ type S3ListApi = {
* @param {StorageListRequest<StorageListPaginateOptions>} req - The request object
* @return {Promise<S3ListPaginateResult>} - Promise resolves to list of keys and metadata for all objects in path
* additionally the result will include a nextToken if there are more items to retrieve
* @throws service: {@link StorageException} - S3 service errors thrown while getting properties
* @throws service: {@link S3Exception} - S3 service errors thrown while getting properties
* @throws validation: {@link StorageValidationErrorCode } - Validation errors thrown
*/
(
Expand Down
2 changes: 1 addition & 1 deletion packages/storage/src/providers/s3/types/errors.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0

export enum StorageException {
export enum S3Exception {
NotFoundException = 'NotFoundException',
ForbiddenException = 'ForbiddenException',
BadRequestException = 'BadRequestException',
Expand Down
2 changes: 1 addition & 1 deletion packages/storage/src/providers/s3/types/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,4 @@ export {
S3ListPaginateResult,
S3GetPropertiesResult,
} from './results';
export { StorageException } from './errors';
export { S3Exception } from './errors';
Loading

0 comments on commit 8ddd6b6

Please sign in to comment.