Skip to content

Commit

Permalink
feat(storage): simplify the location cred provider option input (#13601)
Browse files Browse the repository at this point in the history
Remove the un-essetnial validation of per-API's location credentials provider
input scope and permission for now.
  • Loading branch information
AllanZhengYP authored Jul 16, 2024
1 parent dcba3d7 commit 06c093b
Show file tree
Hide file tree
Showing 8 changed files with 21 additions and 453 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,13 @@ import {
getValue,
removeStore,
} from '../../../src/storageBrowser/locationCredentialsStore/registry';
import { validateCredentialsProviderLocation } from '../../../src/storageBrowser/locationCredentialsStore/validators';
import { LocationCredentialsStore } from '../../../src/storageBrowser/types';
import {
StorageValidationErrorCode,
validationErrorMap,
} from '../../../src/errors/types/validation';

jest.mock('../../../src/storageBrowser/locationCredentialsStore/registry');
jest.mock('../../../src/storageBrowser/locationCredentialsStore/validators');

const mockedCredentials = 'MOCK_CREDS' as any as AWSCredentials;

Expand Down Expand Up @@ -53,10 +51,7 @@ describe('createLocationCredentialsStore', () => {
scope: 's3://bucket/path/*',
permission: 'READ',
});
const { credentials } = await locationCredentialsProvider({
locations: [{ bucket: 'bucket', path: 'path/to/object' }],
permission: 'READ',
});
const { credentials } = await locationCredentialsProvider();
expect(credentials).toEqual(mockedCredentials);
expect(getValue).toHaveBeenCalledWith(
expect.objectContaining({
Expand All @@ -69,55 +64,6 @@ describe('createLocationCredentialsStore', () => {
);
});

it('should validate credentials location with resolved common scope', async () => {
expect.assertions(1);
jest
.mocked(getValue)
.mockResolvedValue({ credentials: mockedCredentials });
const locationCredentialsProvider = store.getProvider({
scope: 's3://bucket/path/*',
permission: 'READWRITE',
});
await locationCredentialsProvider({
locations: [
{ bucket: 'bucket', path: 'path/to/object' },
{ bucket: 'bucket', path: 'path/to/object2' },
{ bucket: 'bucket', path: 'path/folder' },
],
permission: 'READ',
});
expect(validateCredentialsProviderLocation).toHaveBeenCalledWith(
{ bucket: 'bucket', path: 'path/', permission: 'READ' },
{ bucket: 'bucket', path: 'path/*', permission: 'READWRITE' },
);
});

it('should throw if action requires cross-bucket permission', async () => {
expect.assertions(1);
jest
.mocked(getValue)
.mockResolvedValue({ credentials: mockedCredentials });
const locationCredentialsProvider = store.getProvider({
scope: 's3://bucket/path/*',
permission: 'READWRITE',
});
try {
await locationCredentialsProvider({
locations: [
{ bucket: 'bucket-1', path: 'path/to/object' },
{ bucket: 'bucket-2', path: 'path/to/object2' },
],
permission: 'READ',
});
} catch (e: any) {
expect(e.message).toEqual(
validationErrorMap[
StorageValidationErrorCode.LocationCredentialsCrossBucket
].message,
);
}
});

it.each(['invalid-s3-uri', 's3://', 's3:///'])(
'should throw if location credentials provider scope is not a valid S3 URI "%s"',
async invalidScope => {
Expand All @@ -130,10 +76,7 @@ describe('createLocationCredentialsStore', () => {
permission: 'READWRITE',
});
try {
await locationCredentialsProvider({
locations: [{ bucket: 'XXXXXXXX', path: 'path/to/object' }],
permission: 'READ',
});
await locationCredentialsProvider();
} catch (e: any) {
expect(e.message).toEqual(
validationErrorMap[StorageValidationErrorCode.InvalidS3Uri]
Expand Down

This file was deleted.

22 changes: 1 addition & 21 deletions packages/storage/src/errors/types/validation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,17 +21,9 @@ export enum StorageValidationErrorCode {
UrlExpirationMaxLimitExceed = 'UrlExpirationMaxLimitExceed',
InvalidLocationCredentialsCacheSize = 'InvalidLocationCredentialsCacheSize',
LocationCredentialsStoreDestroyed = 'LocationCredentialsStoreDestroyed',
LocationCredentialsBucketMismatch = 'LocationCredentialsBucketMismatch',
LocationCredentialsCrossBucket = 'LocationCredentialsCrossBucket',
LocationCredentialsPathMismatch = 'LocationCredentialsPathMismatch',
LocationCredentialsPermissionMismatch = 'LocationCredentialsPermissionMismatch',
InvalidS3Uri = 'InvalidS3Uri',
}

// Common error message strings to save some bytes
const LOCATION_SPECIFIC_CREDENTIALS = 'Location-specific credentials';
const DOES_NOT_MATCH = 'does not match that required for the API call';

export const validationErrorMap: AmplifyErrorMap<StorageValidationErrorCode> = {
[StorageValidationErrorCode.NoCredentials]: {
message: 'Credentials should not be empty.',
Expand Down Expand Up @@ -85,21 +77,9 @@ export const validationErrorMap: AmplifyErrorMap<StorageValidationErrorCode> = {
message: 'locationCredentialsCacheSize must be a positive integer.',
},
[StorageValidationErrorCode.LocationCredentialsStoreDestroyed]: {
message: `${LOCATION_SPECIFIC_CREDENTIALS} store has been destroyed.`,
message: `Location-specific credentials store has been destroyed.`,
},
[StorageValidationErrorCode.InvalidS3Uri]: {
message: 'Invalid S3 URI.',
},
[StorageValidationErrorCode.LocationCredentialsCrossBucket]: {
message: `${LOCATION_SPECIFIC_CREDENTIALS} cannot be used across buckets.`,
},
[StorageValidationErrorCode.LocationCredentialsBucketMismatch]: {
message: `${LOCATION_SPECIFIC_CREDENTIALS} bucket ${DOES_NOT_MATCH}.`,
},
[StorageValidationErrorCode.LocationCredentialsPathMismatch]: {
message: `${LOCATION_SPECIFIC_CREDENTIALS} path ${DOES_NOT_MATCH}.`,
},
[StorageValidationErrorCode.LocationCredentialsPermissionMismatch]: {
message: `${LOCATION_SPECIFIC_CREDENTIALS} permission ${DOES_NOT_MATCH}.`,
},
};
17 changes: 1 addition & 16 deletions packages/storage/src/providers/s3/types/options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,23 +14,8 @@ import {
/**
* @internal
*/
export type Permission = 'READ' | 'READWRITE' | 'WRITE';

/**
* @internal
*/
export interface BucketLocation {
bucket: string;
path: string;
}

/**
* @internal
*/
export type LocationCredentialsProvider = (options: {
export type LocationCredentialsProvider = (options?: {
forceRefresh?: boolean;
locations: BucketLocation[];
permission: Permission;
}) => Promise<{ credentials: AWSCredentials }>;

interface CommonOptions {
Expand Down
Loading

0 comments on commit 06c093b

Please sign in to comment.