From 61ebc51a014a59786d72fbee0e6a0e97f693ef20 Mon Sep 17 00:00:00 2001 From: ashika112 Date: Mon, 23 Sep 2024 21:36:12 -0700 Subject: [PATCH 01/19] first draft poc --- packages/core/src/parseAmplifyOutputs.ts | 55 +++++++++++++++---- .../src/singleton/AmplifyOutputs/types.ts | 1 + packages/core/src/singleton/Storage/types.ts | 3 + .../createAmplifyAuthConfigAdapter.ts | 30 ++++++++++ 4 files changed, 77 insertions(+), 12 deletions(-) create mode 100644 packages/storage/src/storageBrowser/amplifyAuthConfigAdapter/createAmplifyAuthConfigAdapter.ts diff --git a/packages/core/src/parseAmplifyOutputs.ts b/packages/core/src/parseAmplifyOutputs.ts index 3a9265720e1..94295ae47ef 100644 --- a/packages/core/src/parseAmplifyOutputs.ts +++ b/packages/core/src/parseAmplifyOutputs.ts @@ -65,6 +65,11 @@ function parseStorage( bucket: bucket_name, region: aws_region, buckets: buckets && createBucketInfoMap(buckets), + paths: + buckets && + buckets.map(bucket => ({ + [bucket.name]: parseAccessRules(bucket.paths), + })), }, }; } @@ -342,18 +347,44 @@ function createBucketInfoMap( ): Record { const mappedBuckets: Record = {}; - buckets.forEach(({ name, bucket_name: bucketName, aws_region: region }) => { - if (name in mappedBuckets) { - throw new Error( - `Duplicate friendly name found: ${name}. Name must be unique.`, - ); - } - - mappedBuckets[name] = { - bucketName, - region, - }; - }); + buckets.forEach( + ({ name, bucket_name: bucketName, aws_region: region, paths }) => { + if (name in mappedBuckets) { + throw new Error( + `Duplicate friendly name found: ${name}. Name must be unique.`, + ); + } + + mappedBuckets[name] = { + bucketName, + region, + paths, + }; + }, + ); return mappedBuckets; } + +const parseAccessRules = ( + paths: AmplifyOutputsStorageBucketProperties['paths'], +): Record => { + const output: Record = {}; + + for (const [path, roles] of Object.entries(paths)) { + for (const [role, permissions] of Object.entries(roles)) { + if (!output[role]) { + output[role] = []; + } + if (role === 'authenticated' || role === 'guest') { + output[role].push({ [path]: permissions }); + } else if (role.startsWith('groups') || role.startsWith('entity')) { + if (!path.includes('cognito-identity.amazonaws.com')) { + output[role].push({ [path]: permissions }); + } + } + } + } + + return output; +}; diff --git a/packages/core/src/singleton/AmplifyOutputs/types.ts b/packages/core/src/singleton/AmplifyOutputs/types.ts index 2968955158f..b9de72a657c 100644 --- a/packages/core/src/singleton/AmplifyOutputs/types.ts +++ b/packages/core/src/singleton/AmplifyOutputs/types.ts @@ -50,6 +50,7 @@ export interface AmplifyOutputsStorageBucketProperties { bucket_name: string; /** Region for the bucket */ aws_region: string; + paths: Record>; } export interface AmplifyOutputsStorageProperties { /** Default region for Storage */ diff --git a/packages/core/src/singleton/Storage/types.ts b/packages/core/src/singleton/Storage/types.ts index 5bca120c9b3..cd82ad76433 100644 --- a/packages/core/src/singleton/Storage/types.ts +++ b/packages/core/src/singleton/Storage/types.ts @@ -12,6 +12,8 @@ export interface BucketInfo { bucketName: string; /** Region of the bucket */ region: string; + /** Path to object provisioned */ + paths: Record>; } export interface S3ProviderConfig { S3: { @@ -25,6 +27,7 @@ export interface S3ProviderConfig { dangerouslyConnectToHttpEndpointForTesting?: string; /** Map of friendly name for bucket to its information */ buckets?: Record; + paths?: any; }; } diff --git a/packages/storage/src/storageBrowser/amplifyAuthConfigAdapter/createAmplifyAuthConfigAdapter.ts b/packages/storage/src/storageBrowser/amplifyAuthConfigAdapter/createAmplifyAuthConfigAdapter.ts new file mode 100644 index 00000000000..a805f89bec8 --- /dev/null +++ b/packages/storage/src/storageBrowser/amplifyAuthConfigAdapter/createAmplifyAuthConfigAdapter.ts @@ -0,0 +1,30 @@ +// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 +import { Amplify } from '@aws-amplify/core'; + +import { GetLocationCredentials, ListLocations } from '../types'; + +export interface AuthConfigAdapter { + /** outputs Scope(path), permission (read/write/readwrite), type(prefix/bucket/object) */ + listLocations: ListLocations; + /** basically fetch auth session */ + getLocationCredentials?: GetLocationCredentials; +} + +export const createAmplifyAuthConfigAdapter = (): AuthConfigAdapter => { + const listLocations = createAmplifyListLocationsHandler(); + + return { listLocations }; +}; + +const createAmplifyListLocationsHandler = (): ListLocations => { + // eslint-disable-next-line unused-imports/no-unused-vars + return async function listLocations(input = {}) { + const { Storage } = Amplify.getConfig(); + const { paths } = Storage!.S3!; + + // Parse Amplify storage buckets to get location + + return { locations: paths ?? [] }; + }; +}; From a834fcc3114ae96617c44b3330d311cff8c93b36 Mon Sep 17 00:00:00 2001 From: ashika112 Date: Wed, 25 Sep 2024 16:38:56 -0700 Subject: [PATCH 02/19] upadtes --- packages/storage/src/internals/index.ts | 1 + .../createAmplifyAuthConfigAdapter.ts | 59 +++++++++++++++++-- .../createListLocationsHandler.ts | 21 +++++++ 3 files changed, 77 insertions(+), 4 deletions(-) create mode 100644 packages/storage/src/storageBrowser/amplifyAuthConfigAdapter/createListLocationsHandler.ts diff --git a/packages/storage/src/internals/index.ts b/packages/storage/src/internals/index.ts index 279ef2afd3b..878248bdfea 100644 --- a/packages/storage/src/internals/index.ts +++ b/packages/storage/src/internals/index.ts @@ -30,6 +30,7 @@ export { createManagedAuthConfigAdapter, CreateManagedAuthConfigAdapterInput, } from './managedAuthConfigAdapter'; +export { createAmplifyAuthConfigAdapter } from './amplifyAuthConfigAdapter/createAmplifyAuthConfigAdapter'; export { GetLocationCredentials, ListLocations, diff --git a/packages/storage/src/storageBrowser/amplifyAuthConfigAdapter/createAmplifyAuthConfigAdapter.ts b/packages/storage/src/storageBrowser/amplifyAuthConfigAdapter/createAmplifyAuthConfigAdapter.ts index a805f89bec8..1138bb0e0d4 100644 --- a/packages/storage/src/storageBrowser/amplifyAuthConfigAdapter/createAmplifyAuthConfigAdapter.ts +++ b/packages/storage/src/storageBrowser/amplifyAuthConfigAdapter/createAmplifyAuthConfigAdapter.ts @@ -2,7 +2,12 @@ // SPDX-License-Identifier: Apache-2.0 import { Amplify } from '@aws-amplify/core'; -import { GetLocationCredentials, ListLocations } from '../types'; +import { + GetLocationCredentials, + ListLocations, + ListLocationsInput, + ListLocationsOutput, +} from '../types'; export interface AuthConfigAdapter { /** outputs Scope(path), permission (read/write/readwrite), type(prefix/bucket/object) */ @@ -18,13 +23,59 @@ export const createAmplifyAuthConfigAdapter = (): AuthConfigAdapter => { }; const createAmplifyListLocationsHandler = (): ListLocations => { - // eslint-disable-next-line unused-imports/no-unused-vars return async function listLocations(input = {}) { const { Storage } = Amplify.getConfig(); const { paths } = Storage!.S3!; - // Parse Amplify storage buckets to get location + const { pageSize, nextToken } = input; - return { locations: paths ?? [] }; + if (pageSize) { + if (nextToken) { + const start = -nextToken; + const end = start > pageSize ? start + pageSize : undefined; + + return { + locations: paths.slice(start, end), + nextToken: end ? `${end}` : undefined, + }; + } + + return { + locations: paths.slice(0, pageSize), + nextToken: `${pageSize}`, + }; + } + + return { + locations: paths, + }; + }; +}; + +// eslint-disable-next-line unused-imports/no-unused-vars +const listPathsForUser = (input: ListLocationsInput): ListLocationsOutput => { + const { Storage } = Amplify.getConfig(); + const { paths } = Storage!.S3!; + const { pageSize, nextToken } = input; + + if (pageSize) { + if (nextToken) { + const start = -nextToken; + const end = start > pageSize ? start + pageSize : undefined; + + return { + locations: paths.slice(start, end), + nextToken: end ? `${end}` : undefined, + }; + } + + return { + locations: paths.slice(0, pageSize), + nextToken: `${pageSize}`, + }; + } + + return { + locations: paths, }; }; diff --git a/packages/storage/src/storageBrowser/amplifyAuthConfigAdapter/createListLocationsHandler.ts b/packages/storage/src/storageBrowser/amplifyAuthConfigAdapter/createListLocationsHandler.ts new file mode 100644 index 00000000000..e246a2bdc27 --- /dev/null +++ b/packages/storage/src/storageBrowser/amplifyAuthConfigAdapter/createListLocationsHandler.ts @@ -0,0 +1,21 @@ +// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { CredentialsProvider, ListLocations } from '../types'; +import { listCallerAccessGrants } from '../apis/listCallerAccessGrants'; + +interface CreateListLocationsHandlerInput { + accountId: string; + credentialsProvider: CredentialsProvider; + region: string; +} + +export const createListLocationsHandler = ( + handlerInput: CreateListLocationsHandlerInput, +): ListLocations => { + return async function listLocations(input = {}) { + const result = await listCallerAccessGrants({ ...input, ...handlerInput }); + + return result; + }; +}; From 337068b5a3312dfe49836064b52d2704fbc76471 Mon Sep 17 00:00:00 2001 From: ashika112 Date: Mon, 30 Sep 2024 14:01:40 -0700 Subject: [PATCH 03/19] add listPaths API --- packages/core/src/parseAmplifyOutputs.ts | 10 +- packages/core/src/singleton/Storage/types.ts | 1 - .../storage/src/providers/s3/types/options.ts | 1 + .../createAmplifyAuthConfigAdapter.ts | 138 ++++++++++++------ 4 files changed, 94 insertions(+), 56 deletions(-) diff --git a/packages/core/src/parseAmplifyOutputs.ts b/packages/core/src/parseAmplifyOutputs.ts index 94295ae47ef..0c58d42a120 100644 --- a/packages/core/src/parseAmplifyOutputs.ts +++ b/packages/core/src/parseAmplifyOutputs.ts @@ -65,11 +65,6 @@ function parseStorage( bucket: bucket_name, region: aws_region, buckets: buckets && createBucketInfoMap(buckets), - paths: - buckets && - buckets.map(bucket => ({ - [bucket.name]: parseAccessRules(bucket.paths), - })), }, }; } @@ -366,6 +361,7 @@ function createBucketInfoMap( return mappedBuckets; } +// eslint-disable-next-line unused-imports/no-unused-vars const parseAccessRules = ( paths: AmplifyOutputsStorageBucketProperties['paths'], ): Record => { @@ -379,9 +375,7 @@ const parseAccessRules = ( if (role === 'authenticated' || role === 'guest') { output[role].push({ [path]: permissions }); } else if (role.startsWith('groups') || role.startsWith('entity')) { - if (!path.includes('cognito-identity.amazonaws.com')) { - output[role].push({ [path]: permissions }); - } + output[role].push({ [path]: permissions }); } } } diff --git a/packages/core/src/singleton/Storage/types.ts b/packages/core/src/singleton/Storage/types.ts index cd82ad76433..57830ba5782 100644 --- a/packages/core/src/singleton/Storage/types.ts +++ b/packages/core/src/singleton/Storage/types.ts @@ -27,7 +27,6 @@ export interface S3ProviderConfig { dangerouslyConnectToHttpEndpointForTesting?: string; /** Map of friendly name for bucket to its information */ buckets?: Record; - paths?: any; }; } diff --git a/packages/storage/src/providers/s3/types/options.ts b/packages/storage/src/providers/s3/types/options.ts index 33e3b8e1c14..c2c625b5af5 100644 --- a/packages/storage/src/providers/s3/types/options.ts +++ b/packages/storage/src/providers/s3/types/options.ts @@ -35,6 +35,7 @@ export type LocationCredentialsProvider = ( export interface BucketInfo { bucketName: string; region: string; + paths: Record>; } export type StorageBucket = string | BucketInfo; diff --git a/packages/storage/src/storageBrowser/amplifyAuthConfigAdapter/createAmplifyAuthConfigAdapter.ts b/packages/storage/src/storageBrowser/amplifyAuthConfigAdapter/createAmplifyAuthConfigAdapter.ts index 1138bb0e0d4..df6297b0e6d 100644 --- a/packages/storage/src/storageBrowser/amplifyAuthConfigAdapter/createAmplifyAuthConfigAdapter.ts +++ b/packages/storage/src/storageBrowser/amplifyAuthConfigAdapter/createAmplifyAuthConfigAdapter.ts @@ -1,17 +1,22 @@ // Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 -import { Amplify } from '@aws-amplify/core'; +import { Amplify, fetchAuthSession } from '@aws-amplify/core'; -import { - GetLocationCredentials, - ListLocations, - ListLocationsInput, - ListLocationsOutput, -} from '../types'; +import { GetLocationCredentials, ListPaths } from '../types'; +import { BucketInfo } from '../../providers/s3/types/options'; + +interface Location { + type: 'PREFIX'; + permission: string[]; + scope: { + bucketName: string; + path: string; + }; +} export interface AuthConfigAdapter { /** outputs Scope(path), permission (read/write/readwrite), type(prefix/bucket/object) */ - listLocations: ListLocations; + listLocations: ListPaths; /** basically fetch auth session */ getLocationCredentials?: GetLocationCredentials; } @@ -22,60 +27,99 @@ export const createAmplifyAuthConfigAdapter = (): AuthConfigAdapter => { return { listLocations }; }; -const createAmplifyListLocationsHandler = (): ListLocations => { +const createAmplifyListLocationsHandler = (): ListPaths => { return async function listLocations(input = {}) { - const { Storage } = Amplify.getConfig(); - const { paths } = Storage!.S3!; + const { buckets } = Amplify.getConfig().Storage!.S3!; + + if (!buckets) { + return { locations: [] }; + } + // eslint-disable-next-line no-debugger + debugger; + + const { tokens, identityId } = await fetchAuthSession(); + const userGroups = tokens?.accessToken.payload['cognito:groups']; + // eslint-disable-next-line unused-imports/no-unused-vars const { pageSize, nextToken } = input; - if (pageSize) { - if (nextToken) { - const start = -nextToken; - const end = start > pageSize ? start + pageSize : undefined; + const locations = parseBuckets({ + buckets, + tokens: !!tokens, + identityId: identityId!, + userGroup: (userGroups as any)[0], // TODO: fix this edge case + }); - return { - locations: paths.slice(start, end), - nextToken: end ? `${end}` : undefined, - }; - } + return { locations }; + }; +}; - return { - locations: paths.slice(0, pageSize), - nextToken: `${pageSize}`, - }; - } +const getPermissions = ( + accessRule: Record, + token: boolean, + groups?: string, +) => { + if (!token) { + return { + permission: accessRule.guest, + }; + } + if (groups) { + const selectedKey = Object.keys(accessRule).find(access => + access.includes(groups), + ); return { - locations: paths, + permission: selectedKey + ? accessRule[selectedKey] + : accessRule.authenticated, }; + } + + return { + permission: accessRule.authenticated, }; }; -// eslint-disable-next-line unused-imports/no-unused-vars -const listPathsForUser = (input: ListLocationsInput): ListLocationsOutput => { - const { Storage } = Amplify.getConfig(); - const { paths } = Storage!.S3!; - const { pageSize, nextToken } = input; - - if (pageSize) { - if (nextToken) { - const start = -nextToken; - const end = start > pageSize ? start + pageSize : undefined; +const parseBuckets = ({ + buckets, + tokens, + identityId, + userGroup, +}: { + buckets: Record; + tokens: boolean; + identityId: string; + userGroup: string; +}): Location[] => { + const locations: Location[] = []; - return { - locations: paths.slice(start, end), - nextToken: end ? `${end}` : undefined, + for (const [, bucketInfo] of Object.entries(buckets)) { + const { bucketName, paths } = bucketInfo; + for (const [path, accessRules] of Object.entries(paths)) { + // eslint-disable-next-line no-template-curly-in-string + if (tokens && path.includes('${cognito-identity.amazonaws.com:sub}')) { + locations.push({ + type: 'PREFIX', // TODO: update logic to include prefix/object + permission: accessRules.entityidentity, + scope: { + bucketName, + path: path.replace( + // eslint-disable-next-line no-template-curly-in-string + '${cognito-identity.amazonaws.com:sub}', + identityId, + ), + }, + }); + } + const location: Location = { + type: 'PREFIX', + ...getPermissions(accessRules, tokens, userGroup), + scope: { bucketName, path }, }; + if (location.permission) locations.push(location); } - - return { - locations: paths.slice(0, pageSize), - nextToken: `${pageSize}`, - }; } - return { - locations: paths, - }; + return locations; }; From 9d209e17b4ec21d1d8a530f79febbcffc29b0a3f Mon Sep 17 00:00:00 2001 From: ashika112 Date: Mon, 30 Sep 2024 14:19:20 -0700 Subject: [PATCH 04/19] update new file structure --- .../createAmplifyAuthConfigAdapter.ts | 2 +- .../createListLocationsHandler.ts | 2 +- .../storage/src/internals/types/credentials.ts | 15 +++++++++++++++ 3 files changed, 17 insertions(+), 2 deletions(-) rename packages/storage/src/{storageBrowser => internals}/amplifyAuthConfigAdapter/createAmplifyAuthConfigAdapter.ts (97%) rename packages/storage/src/{storageBrowser => internals}/amplifyAuthConfigAdapter/createListLocationsHandler.ts (88%) diff --git a/packages/storage/src/storageBrowser/amplifyAuthConfigAdapter/createAmplifyAuthConfigAdapter.ts b/packages/storage/src/internals/amplifyAuthConfigAdapter/createAmplifyAuthConfigAdapter.ts similarity index 97% rename from packages/storage/src/storageBrowser/amplifyAuthConfigAdapter/createAmplifyAuthConfigAdapter.ts rename to packages/storage/src/internals/amplifyAuthConfigAdapter/createAmplifyAuthConfigAdapter.ts index df6297b0e6d..3026d156594 100644 --- a/packages/storage/src/storageBrowser/amplifyAuthConfigAdapter/createAmplifyAuthConfigAdapter.ts +++ b/packages/storage/src/internals/amplifyAuthConfigAdapter/createAmplifyAuthConfigAdapter.ts @@ -2,7 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 import { Amplify, fetchAuthSession } from '@aws-amplify/core'; -import { GetLocationCredentials, ListPaths } from '../types'; +import { GetLocationCredentials, ListPaths } from '../types/credentials'; import { BucketInfo } from '../../providers/s3/types/options'; interface Location { diff --git a/packages/storage/src/storageBrowser/amplifyAuthConfigAdapter/createListLocationsHandler.ts b/packages/storage/src/internals/amplifyAuthConfigAdapter/createListLocationsHandler.ts similarity index 88% rename from packages/storage/src/storageBrowser/amplifyAuthConfigAdapter/createListLocationsHandler.ts rename to packages/storage/src/internals/amplifyAuthConfigAdapter/createListLocationsHandler.ts index e246a2bdc27..b34dff9848c 100644 --- a/packages/storage/src/storageBrowser/amplifyAuthConfigAdapter/createListLocationsHandler.ts +++ b/packages/storage/src/internals/amplifyAuthConfigAdapter/createListLocationsHandler.ts @@ -1,7 +1,7 @@ // Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 -import { CredentialsProvider, ListLocations } from '../types'; +import { CredentialsProvider, ListLocations } from '../types/credentials'; import { listCallerAccessGrants } from '../apis/listCallerAccessGrants'; interface CreateListLocationsHandlerInput { diff --git a/packages/storage/src/internals/types/credentials.ts b/packages/storage/src/internals/types/credentials.ts index 95d6de8a00e..7846106a82c 100644 --- a/packages/storage/src/internals/types/credentials.ts +++ b/packages/storage/src/internals/types/credentials.ts @@ -69,6 +69,21 @@ export type ListLocations = ( input?: ListLocationsInput, ) => Promise; +/** + * @internal + */ +export type ListPaths = (input?: ListLocationsInput) => Promise<{ + locations: { + type: 'PREFIX'; + permission: string[]; + scope: { + bucketName: string; + path: string; + }; + }[]; + nextToken?: string; +}>; + /** * @internal */ From 537275c08a1d603d5c7c898129b1e22ee7cf889f Mon Sep 17 00:00:00 2001 From: ashika112 Date: Wed, 2 Oct 2024 13:53:09 -0700 Subject: [PATCH 05/19] fix types --- packages/core/src/parseAmplifyOutputs.ts | 22 ------------------- .../src/singleton/AmplifyOutputs/types.ts | 2 +- packages/core/src/singleton/Storage/types.ts | 2 +- .../createAmplifyAuthConfigAdapter.ts | 4 ++++ .../storage/src/providers/s3/types/options.ts | 2 +- 5 files changed, 7 insertions(+), 25 deletions(-) diff --git a/packages/core/src/parseAmplifyOutputs.ts b/packages/core/src/parseAmplifyOutputs.ts index 0c58d42a120..21e52752812 100644 --- a/packages/core/src/parseAmplifyOutputs.ts +++ b/packages/core/src/parseAmplifyOutputs.ts @@ -360,25 +360,3 @@ function createBucketInfoMap( return mappedBuckets; } - -// eslint-disable-next-line unused-imports/no-unused-vars -const parseAccessRules = ( - paths: AmplifyOutputsStorageBucketProperties['paths'], -): Record => { - const output: Record = {}; - - for (const [path, roles] of Object.entries(paths)) { - for (const [role, permissions] of Object.entries(roles)) { - if (!output[role]) { - output[role] = []; - } - if (role === 'authenticated' || role === 'guest') { - output[role].push({ [path]: permissions }); - } else if (role.startsWith('groups') || role.startsWith('entity')) { - output[role].push({ [path]: permissions }); - } - } - } - - return output; -}; diff --git a/packages/core/src/singleton/AmplifyOutputs/types.ts b/packages/core/src/singleton/AmplifyOutputs/types.ts index b9de72a657c..f1290999e7e 100644 --- a/packages/core/src/singleton/AmplifyOutputs/types.ts +++ b/packages/core/src/singleton/AmplifyOutputs/types.ts @@ -50,7 +50,7 @@ export interface AmplifyOutputsStorageBucketProperties { bucket_name: string; /** Region for the bucket */ aws_region: string; - paths: Record>; + paths?: Record>; } export interface AmplifyOutputsStorageProperties { /** Default region for Storage */ diff --git a/packages/core/src/singleton/Storage/types.ts b/packages/core/src/singleton/Storage/types.ts index 57830ba5782..ef4c625de7a 100644 --- a/packages/core/src/singleton/Storage/types.ts +++ b/packages/core/src/singleton/Storage/types.ts @@ -13,7 +13,7 @@ export interface BucketInfo { /** Region of the bucket */ region: string; /** Path to object provisioned */ - paths: Record>; + paths?: Record>; } export interface S3ProviderConfig { S3: { diff --git a/packages/storage/src/internals/amplifyAuthConfigAdapter/createAmplifyAuthConfigAdapter.ts b/packages/storage/src/internals/amplifyAuthConfigAdapter/createAmplifyAuthConfigAdapter.ts index 3026d156594..8920cef851a 100644 --- a/packages/storage/src/internals/amplifyAuthConfigAdapter/createAmplifyAuthConfigAdapter.ts +++ b/packages/storage/src/internals/amplifyAuthConfigAdapter/createAmplifyAuthConfigAdapter.ts @@ -96,6 +96,10 @@ const parseBuckets = ({ for (const [, bucketInfo] of Object.entries(buckets)) { const { bucketName, paths } = bucketInfo; + if (!paths) { + // Todo: Verify behavior + return locations; + } for (const [path, accessRules] of Object.entries(paths)) { // eslint-disable-next-line no-template-curly-in-string if (tokens && path.includes('${cognito-identity.amazonaws.com:sub}')) { diff --git a/packages/storage/src/providers/s3/types/options.ts b/packages/storage/src/providers/s3/types/options.ts index c2c625b5af5..d5b4c9501ec 100644 --- a/packages/storage/src/providers/s3/types/options.ts +++ b/packages/storage/src/providers/s3/types/options.ts @@ -35,7 +35,7 @@ export type LocationCredentialsProvider = ( export interface BucketInfo { bucketName: string; region: string; - paths: Record>; + paths?: Record>; } export type StorageBucket = string | BucketInfo; From 631dd7fcacfacac286ebe29e3b4833b73a9064cf Mon Sep 17 00:00:00 2001 From: ashika112 Date: Thu, 3 Oct 2024 15:55:33 -0700 Subject: [PATCH 06/19] refactor types and utils --- .../__tests__/parseAmplifyOutputs.test.ts | 53 ++++++++++ .../src/singleton/AmplifyOutputs/types.ts | 1 + packages/core/src/singleton/Storage/types.ts | 2 +- .../createAmplifyAuthConfigAdapter.ts | 100 +----------------- .../generateLocationsFromPaths.ts | 75 +++++++++++++ .../storage/src/internals/types/common.ts | 5 + .../src/internals/types/credentials.ts | 51 ++++++--- .../storage/src/internals/utils/constants.ts | 3 + 8 files changed, 178 insertions(+), 112 deletions(-) create mode 100644 packages/storage/src/internals/amplifyAuthConfigAdapter/generateLocationsFromPaths.ts diff --git a/packages/core/__tests__/parseAmplifyOutputs.test.ts b/packages/core/__tests__/parseAmplifyOutputs.test.ts index bb93d12116c..38a8fa141c4 100644 --- a/packages/core/__tests__/parseAmplifyOutputs.test.ts +++ b/packages/core/__tests__/parseAmplifyOutputs.test.ts @@ -294,6 +294,59 @@ describe('parseAmplifyOutputs tests', () => { expect(() => parseAmplifyOutputs(amplifyOutputs)).toThrow(); }); + it('should parse storage bucket with paths', () => { + const amplifyOutputs: AmplifyOutputs = { + version: '1.2', + storage: { + aws_region: 'us-west-2', + bucket_name: 'storage-bucket-test', + buckets: [ + { + name: 'default-bucket', + bucket_name: 'storage-bucket-test', + aws_region: 'us-west-2', + paths: { + 'other/*': { + guest: ['get', 'list'], + authenticated: ['get', 'list', 'write'], + }, + 'admin/*': { + groupsauditor: ['get', 'list'], + groupsadmin: ['get', 'list', 'write', 'delete'], + }, + }, + }, + ], + }, + }; + + const result = parseAmplifyOutputs(amplifyOutputs); + + expect(result).toEqual({ + Storage: { + S3: { + bucket: 'storage-bucket-test', + region: 'us-west-2', + buckets: { + 'default-bucket': { + bucketName: 'storage-bucket-test', + region: 'us-west-2', + paths: { + 'other/*': { + guest: ['get', 'list'], + authenticated: ['get', 'list', 'write'], + }, + 'admin/*': { + groupsauditor: ['get', 'list'], + groupsadmin: ['get', 'list', 'write', 'delete'], + }, + }, + }, + }, + }, + }, + }); + }); }); describe('analytics tests', () => { diff --git a/packages/core/src/singleton/AmplifyOutputs/types.ts b/packages/core/src/singleton/AmplifyOutputs/types.ts index f1290999e7e..367a1961559 100644 --- a/packages/core/src/singleton/AmplifyOutputs/types.ts +++ b/packages/core/src/singleton/AmplifyOutputs/types.ts @@ -50,6 +50,7 @@ export interface AmplifyOutputsStorageBucketProperties { bucket_name: string; /** Region for the bucket */ aws_region: string; + /** Paths to object with access permissions */ paths?: Record>; } export interface AmplifyOutputsStorageProperties { diff --git a/packages/core/src/singleton/Storage/types.ts b/packages/core/src/singleton/Storage/types.ts index ef4c625de7a..5c0e8a9bcfd 100644 --- a/packages/core/src/singleton/Storage/types.ts +++ b/packages/core/src/singleton/Storage/types.ts @@ -12,7 +12,7 @@ export interface BucketInfo { bucketName: string; /** Region of the bucket */ region: string; - /** Path to object provisioned */ + /** Paths to object with access permissions */ paths?: Record>; } export interface S3ProviderConfig { diff --git a/packages/storage/src/internals/amplifyAuthConfigAdapter/createAmplifyAuthConfigAdapter.ts b/packages/storage/src/internals/amplifyAuthConfigAdapter/createAmplifyAuthConfigAdapter.ts index 8920cef851a..94304eaa070 100644 --- a/packages/storage/src/internals/amplifyAuthConfigAdapter/createAmplifyAuthConfigAdapter.ts +++ b/packages/storage/src/internals/amplifyAuthConfigAdapter/createAmplifyAuthConfigAdapter.ts @@ -2,23 +2,12 @@ // SPDX-License-Identifier: Apache-2.0 import { Amplify, fetchAuthSession } from '@aws-amplify/core'; -import { GetLocationCredentials, ListPaths } from '../types/credentials'; -import { BucketInfo } from '../../providers/s3/types/options'; +import { ListPaths } from '../types/credentials'; -interface Location { - type: 'PREFIX'; - permission: string[]; - scope: { - bucketName: string; - path: string; - }; -} +import { generateLocationsFromPaths } from './generateLocationsFromPaths'; export interface AuthConfigAdapter { - /** outputs Scope(path), permission (read/write/readwrite), type(prefix/bucket/object) */ listLocations: ListPaths; - /** basically fetch auth session */ - getLocationCredentials?: GetLocationCredentials; } export const createAmplifyAuthConfigAdapter = (): AuthConfigAdapter => { @@ -28,22 +17,17 @@ export const createAmplifyAuthConfigAdapter = (): AuthConfigAdapter => { }; const createAmplifyListLocationsHandler = (): ListPaths => { - return async function listLocations(input = {}) { - const { buckets } = Amplify.getConfig().Storage!.S3!; + const { buckets } = Amplify.getConfig().Storage!.S3!; + return async function listLocations() { if (!buckets) { return { locations: [] }; } - // eslint-disable-next-line no-debugger - debugger; const { tokens, identityId } = await fetchAuthSession(); const userGroups = tokens?.accessToken.payload['cognito:groups']; - // eslint-disable-next-line unused-imports/no-unused-vars - const { pageSize, nextToken } = input; - - const locations = parseBuckets({ + const locations = generateLocationsFromPaths({ buckets, tokens: !!tokens, identityId: identityId!, @@ -53,77 +37,3 @@ const createAmplifyListLocationsHandler = (): ListPaths => { return { locations }; }; }; - -const getPermissions = ( - accessRule: Record, - token: boolean, - groups?: string, -) => { - if (!token) { - return { - permission: accessRule.guest, - }; - } - if (groups) { - const selectedKey = Object.keys(accessRule).find(access => - access.includes(groups), - ); - - return { - permission: selectedKey - ? accessRule[selectedKey] - : accessRule.authenticated, - }; - } - - return { - permission: accessRule.authenticated, - }; -}; - -const parseBuckets = ({ - buckets, - tokens, - identityId, - userGroup, -}: { - buckets: Record; - tokens: boolean; - identityId: string; - userGroup: string; -}): Location[] => { - const locations: Location[] = []; - - for (const [, bucketInfo] of Object.entries(buckets)) { - const { bucketName, paths } = bucketInfo; - if (!paths) { - // Todo: Verify behavior - return locations; - } - for (const [path, accessRules] of Object.entries(paths)) { - // eslint-disable-next-line no-template-curly-in-string - if (tokens && path.includes('${cognito-identity.amazonaws.com:sub}')) { - locations.push({ - type: 'PREFIX', // TODO: update logic to include prefix/object - permission: accessRules.entityidentity, - scope: { - bucketName, - path: path.replace( - // eslint-disable-next-line no-template-curly-in-string - '${cognito-identity.amazonaws.com:sub}', - identityId, - ), - }, - }); - } - const location: Location = { - type: 'PREFIX', - ...getPermissions(accessRules, tokens, userGroup), - scope: { bucketName, path }, - }; - if (location.permission) locations.push(location); - } - } - - return locations; -}; diff --git a/packages/storage/src/internals/amplifyAuthConfigAdapter/generateLocationsFromPaths.ts b/packages/storage/src/internals/amplifyAuthConfigAdapter/generateLocationsFromPaths.ts new file mode 100644 index 00000000000..145ddcdeb70 --- /dev/null +++ b/packages/storage/src/internals/amplifyAuthConfigAdapter/generateLocationsFromPaths.ts @@ -0,0 +1,75 @@ +// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 +import { PathAccess } from '../types/credentials'; +import { BucketInfo } from '../../providers/s3/types/options'; +import { ENTITY_IDENTITY_URL } from '../utils/constants'; +import { StorageAccess } from '../types/common'; + +const resolvePermissions = ( + accessRule: Record, + token: boolean, + groups?: string, +) => { + if (!token) { + return { + permission: accessRule.guest, + }; + } + if (groups) { + const selectedKey = Object.keys(accessRule).find(access => + access.includes(groups), + ); + + return { + permission: selectedKey + ? accessRule[selectedKey] + : accessRule.authenticated, + }; + } + + return { + permission: accessRule.authenticated, + }; +}; + +export const generateLocationsFromPaths = ({ + buckets, + tokens, + identityId, + userGroup, +}: { + buckets: Record; + tokens: boolean; + identityId: string; + userGroup: string; +}): PathAccess[] => { + const locations: PathAccess[] = []; + + for (const [, bucketInfo] of Object.entries(buckets)) { + const { bucketName, paths } = bucketInfo; + if (!paths) { + // Todo: Verify behavior + return locations; + } + for (const [path, accessRules] of Object.entries(paths)) { + if (tokens && path.includes(ENTITY_IDENTITY_URL)) { + locations.push({ + type: 'PREFIX', + permission: accessRules.entityidentity as StorageAccess[], + scope: { + bucketName, + path: path.replace(ENTITY_IDENTITY_URL, identityId), + }, + }); + } + const location = { + type: 'PREFIX', + ...resolvePermissions(accessRules, tokens, userGroup), + scope: { bucketName, path }, + }; + if (location.permission) locations.push(location as PathAccess); + } + } + + return locations; +}; diff --git a/packages/storage/src/internals/types/common.ts b/packages/storage/src/internals/types/common.ts index 2fd32ca6d8c..32671db5660 100644 --- a/packages/storage/src/internals/types/common.ts +++ b/packages/storage/src/internals/types/common.ts @@ -20,3 +20,8 @@ export type Privilege = 'Default' | 'Minimal'; * @internal */ export type PrefixType = 'Object'; + +/** + * @internal + */ +export type StorageAccess = 'read' | 'get' | 'list' | 'write' | 'delete'; diff --git a/packages/storage/src/internals/types/credentials.ts b/packages/storage/src/internals/types/credentials.ts index 7846106a82c..56a6058ac5d 100644 --- a/packages/storage/src/internals/types/credentials.ts +++ b/packages/storage/src/internals/types/credentials.ts @@ -6,7 +6,7 @@ import { LocationCredentialsProvider, } from '../../providers/s3/types/options'; -import { LocationType, Permission } from './common'; +import { LocationType, Permission, StorageAccess } from './common'; /** * @internal @@ -69,21 +69,6 @@ export type ListLocations = ( input?: ListLocationsInput, ) => Promise; -/** - * @internal - */ -export type ListPaths = (input?: ListLocationsInput) => Promise<{ - locations: { - type: 'PREFIX'; - permission: string[]; - scope: { - bucketName: string; - path: string; - }; - }[]; - nextToken?: string; -}>; - /** * @internal */ @@ -122,3 +107,37 @@ export interface LocationAccess extends CredentialsLocation { */ readonly type: LocationType; } + +/** + * @internal + */ +export interface PathAccess { + /** The Amplify backend mandates that all paths conclude with '/*', + * which means the only applicable type in this context is 'PREFIX'. */ + type: 'PREFIX'; + permission: StorageAccess[]; + scope: { + bucketName: string; + path: string; + }; +} +/** + * @internal + */ +export interface ListPathsInput { + pageSize?: number; + nextToken?: string; +} + +/** + * @internal + */ +export interface ListPathsOutput { + locations: PathAccess[]; + nextToken?: string; +} + +/** + * @internal + */ +export type ListPaths = (input?: ListPathsInput) => Promise; diff --git a/packages/storage/src/internals/utils/constants.ts b/packages/storage/src/internals/utils/constants.ts index 4c322de94f1..1bc620efb95 100644 --- a/packages/storage/src/internals/utils/constants.ts +++ b/packages/storage/src/internals/utils/constants.ts @@ -3,3 +3,6 @@ export const DEFAULT_CRED_TTL = 15 * 60; // 15 minutes export const MAX_PAGE_SIZE = 1000; + +// eslint-disable-next-line no-template-curly-in-string +export const ENTITY_IDENTITY_URL = '${cognito-identity.amazonaws.com:sub}'; From ee5dcebfe2c523491d7e1e826e8de961646b7f27 Mon Sep 17 00:00:00 2001 From: ashika112 Date: Fri, 4 Oct 2024 11:46:34 -0700 Subject: [PATCH 07/19] update tests --- .../createAmplifyAuthConfigAdapter.test.ts | 122 +++++++++++++++ .../generateLocationsFromPaths.test.ts | 141 ++++++++++++++++++ .../createAmplifyAuthConfigAdapter.ts | 2 +- .../generateLocationsFromPaths.ts | 14 +- 4 files changed, 270 insertions(+), 9 deletions(-) create mode 100644 packages/storage/__tests__/internals/amplifyAuthAdapter/createAmplifyAuthConfigAdapter.test.ts create mode 100644 packages/storage/__tests__/internals/amplifyAuthAdapter/generateLocationsFromPaths.test.ts diff --git a/packages/storage/__tests__/internals/amplifyAuthAdapter/createAmplifyAuthConfigAdapter.test.ts b/packages/storage/__tests__/internals/amplifyAuthAdapter/createAmplifyAuthConfigAdapter.test.ts new file mode 100644 index 00000000000..59c752739bd --- /dev/null +++ b/packages/storage/__tests__/internals/amplifyAuthAdapter/createAmplifyAuthConfigAdapter.test.ts @@ -0,0 +1,122 @@ +// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { Amplify, fetchAuthSession } from '@aws-amplify/core'; + +import { generateLocationsFromPaths } from '../../../src/internals/amplifyAuthConfigAdapter/generateLocationsFromPaths'; +import { createAmplifyAuthConfigAdapter } from '../../../src/internals'; + +jest.mock('@aws-amplify/core', () => ({ + ConsoleLogger: jest.fn(), + Amplify: { + getConfig: jest.fn(), + Auth: { + getConfig: jest.fn(), + fetchAuthSession: jest.fn(), + }, + }, +})); +jest.mock( + '../../../src/internals/amplifyAuthConfigAdapter/generateLocationsFromPaths', +); + +const credentials = { + accessKeyId: 'accessKeyId', + sessionToken: 'sessionToken', + secretAccessKey: 'secretAccessKey', +}; +const identityId = 'identityId'; + +const mockGetConfig = jest.mocked(Amplify.getConfig); +const mockFetchAuthSession = fetchAuthSession as jest.Mock; +const mockGenerateLocationsFromPaths = generateLocationsFromPaths as jest.Mock; + +describe('createAmplifyAuthConfigAdapter', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + mockGetConfig.mockReturnValue({ + Storage: { + S3: { + bucket: 'bucket1', + region: 'region1', + buckets: { + 'bucket-1': { + bucketName: 'bucket-1', + region: 'region1', + paths: {}, + }, + }, + }, + }, + }); + mockFetchAuthSession.mockResolvedValue({ + credentials, + identityId, + token: {}, + }); + + it.only('should return an AuthConfigAdapter with listLocations function', async () => { + const adapter = createAmplifyAuthConfigAdapter(); + expect(adapter).toHaveProperty('listLocations'); + const { listLocations } = adapter; + const locations = await listLocations(); + console.log(locations); + // expect(mockFetchAuthSession).toHaveBeenCalled(); + }); + + it('should return empty locations when buckets are not defined', async () => { + mockGetConfig.mockReturnValue({ Storage: { S3: { buckets: undefined } } }); + + const adapter = createAmplifyAuthConfigAdapter(); + const result = await adapter.listLocations(); + + expect(result).toEqual({ locations: [] }); + }); + + it.skip('should generate locations correctly when buckets are defined', async () => { + const mockBuckets = { + bucket1: { + bucketName: 'bucket1', + region: 'region1', + paths: { + '/path1': { + entityidentity: ['read', 'write'], + groupsadmin: ['read'], + }, + }, + }, + }; + + mockGetConfig.mockReturnValue({ + Storage: { S3: { buckets: mockBuckets } }, + }); + mockGenerateLocationsFromPaths.mockReturnValue([ + { + type: 'PREFIX', + permission: ['read', 'write'], + scope: { + bucketName: 'bucket1', + path: '/path1', + }, + }, + ]); + + const adapter = createAmplifyAuthConfigAdapter(); + const result = await adapter.listLocations(); + + expect(result).toEqual({ + locations: [ + { + type: 'PREFIX', + permission: ['read', 'write'], + scope: { + bucketName: 'bucket1', + path: '/path1', + }, + }, + ], + }); + }); +}); diff --git a/packages/storage/__tests__/internals/amplifyAuthAdapter/generateLocationsFromPaths.test.ts b/packages/storage/__tests__/internals/amplifyAuthAdapter/generateLocationsFromPaths.test.ts new file mode 100644 index 00000000000..4dd15d7aa98 --- /dev/null +++ b/packages/storage/__tests__/internals/amplifyAuthAdapter/generateLocationsFromPaths.test.ts @@ -0,0 +1,141 @@ +import { generateLocationsFromPaths } from '../../../src/internals/amplifyAuthConfigAdapter/generateLocationsFromPaths'; +import { BucketInfo } from '../../../src/providers/s3/types/options'; + +describe('generateLocationsFromPaths', () => { + const mockBuckets: Record = { + bucket1: { + bucketName: 'bucket1', + region: 'region1', + paths: { + 'path1/*': { + guest: ['get', 'list'], + authenticated: ['get', 'list', 'write'], + }, + 'path2/*': { + groupsauditor: ['get', 'list'], + groupsadmin: ['get', 'list', 'write', 'delete'], + }, + // eslint-disable-next-line no-template-curly-in-string + 'profile-pictures/${cognito-identity.amazonaws.com:sub}/*': { + entityidentity: ['get', 'list', 'write', 'delete'], + }, + }, + }, + bucket2: { + bucketName: 'bucket2', + region: 'region1', + paths: { + 'path3/*': { + guest: ['read'], + }, + }, + }, + }; + + it('should generate locations correctly when tokens are true', () => { + const result = generateLocationsFromPaths({ + buckets: mockBuckets, + tokens: true, + identityId: '12345', + userGroup: 'admin', + }); + + expect(result).toEqual([ + { + type: 'PREFIX', + permission: ['get', 'list', 'write'], + scope: { + bucketName: 'bucket1', + path: 'path1/*', + }, + }, + { + type: 'PREFIX', + permission: ['get', 'list', 'write', 'delete'], + scope: { + bucketName: 'bucket1', + path: 'path2/*', + }, + }, + { + type: 'PREFIX', + permission: ['get', 'list', 'write', 'delete'], + scope: { + bucketName: 'bucket1', + path: 'profile-pictures/12345/*', + }, + }, + ]); + }); + + it('should generate locations correctly when tokens are true & bad userGroup', () => { + const result = generateLocationsFromPaths({ + buckets: mockBuckets, + tokens: true, + identityId: '12345', + userGroup: 'editor', + }); + + expect(result).toEqual([ + { + type: 'PREFIX', + permission: ['get', 'list', 'write'], + scope: { + bucketName: 'bucket1', + path: 'path1/*', + }, + }, + { + type: 'PREFIX', + permission: ['get', 'list', 'write', 'delete'], + scope: { + bucketName: 'bucket1', + path: 'profile-pictures/12345/*', + }, + }, + ]); + }); + + it('should return empty array when paths are not defined', () => { + const result = generateLocationsFromPaths({ + buckets: { + bucket1: { + bucketName: 'bucket1', + region: 'region1', + paths: undefined, + }, + }, + tokens: true, + identityId: '12345', + userGroup: 'admin', + }); + + expect(result).toEqual([]); + }); + + it('should generate locations correctly when tokens are false', () => { + const result = generateLocationsFromPaths({ + buckets: mockBuckets, + tokens: false, + }); + + expect(result).toEqual([ + { + type: 'PREFIX', + permission: ['get', 'list'], + scope: { + bucketName: 'bucket1', + path: 'path1/*', + }, + }, + { + type: 'PREFIX', + permission: ['read'], + scope: { + bucketName: 'bucket2', + path: 'path3/*', + }, + }, + ]); + }); +}); diff --git a/packages/storage/src/internals/amplifyAuthConfigAdapter/createAmplifyAuthConfigAdapter.ts b/packages/storage/src/internals/amplifyAuthConfigAdapter/createAmplifyAuthConfigAdapter.ts index 94304eaa070..4586033f45f 100644 --- a/packages/storage/src/internals/amplifyAuthConfigAdapter/createAmplifyAuthConfigAdapter.ts +++ b/packages/storage/src/internals/amplifyAuthConfigAdapter/createAmplifyAuthConfigAdapter.ts @@ -30,7 +30,7 @@ const createAmplifyListLocationsHandler = (): ListPaths => { const locations = generateLocationsFromPaths({ buckets, tokens: !!tokens, - identityId: identityId!, + identityId, userGroup: (userGroups as any)[0], // TODO: fix this edge case }); diff --git a/packages/storage/src/internals/amplifyAuthConfigAdapter/generateLocationsFromPaths.ts b/packages/storage/src/internals/amplifyAuthConfigAdapter/generateLocationsFromPaths.ts index 145ddcdeb70..fcd612f3416 100644 --- a/packages/storage/src/internals/amplifyAuthConfigAdapter/generateLocationsFromPaths.ts +++ b/packages/storage/src/internals/amplifyAuthConfigAdapter/generateLocationsFromPaths.ts @@ -16,14 +16,12 @@ const resolvePermissions = ( }; } if (groups) { - const selectedKey = Object.keys(accessRule).find(access => - access.includes(groups), + const selectedKey = Object.keys(accessRule).find( + access => access.includes(groups) || access.includes('authenticated'), ); return { - permission: selectedKey - ? accessRule[selectedKey] - : accessRule.authenticated, + permission: selectedKey ? accessRule[selectedKey] : undefined, }; } @@ -40,8 +38,8 @@ export const generateLocationsFromPaths = ({ }: { buckets: Record; tokens: boolean; - identityId: string; - userGroup: string; + identityId?: string; + userGroup?: string; }): PathAccess[] => { const locations: PathAccess[] = []; @@ -52,7 +50,7 @@ export const generateLocationsFromPaths = ({ return locations; } for (const [path, accessRules] of Object.entries(paths)) { - if (tokens && path.includes(ENTITY_IDENTITY_URL)) { + if (tokens && identityId && path.includes(ENTITY_IDENTITY_URL)) { locations.push({ type: 'PREFIX', permission: accessRules.entityidentity as StorageAccess[], From 70882a7d8e14ad2ddab657202384107d35e53dc4 Mon Sep 17 00:00:00 2001 From: ashika112 Date: Fri, 4 Oct 2024 12:05:55 -0700 Subject: [PATCH 08/19] fix test --- packages/aws-amplify/package.json | 2 +- .../createAmplifyAuthConfigAdapter.test.ts | 14 ++++++++------ .../createAmplifyAuthConfigAdapter.ts | 2 +- 3 files changed, 10 insertions(+), 8 deletions(-) diff --git a/packages/aws-amplify/package.json b/packages/aws-amplify/package.json index 41e506e36f0..28599abba07 100644 --- a/packages/aws-amplify/package.json +++ b/packages/aws-amplify/package.json @@ -497,7 +497,7 @@ "name": "[Storage] uploadData (S3)", "path": "./dist/esm/storage/index.mjs", "import": "{ uploadData }", - "limit": "22.01 kB" + "limit": "22.03 kB" } ] } diff --git a/packages/storage/__tests__/internals/amplifyAuthAdapter/createAmplifyAuthConfigAdapter.test.ts b/packages/storage/__tests__/internals/amplifyAuthAdapter/createAmplifyAuthConfigAdapter.test.ts index 59c752739bd..67765512fba 100644 --- a/packages/storage/__tests__/internals/amplifyAuthAdapter/createAmplifyAuthConfigAdapter.test.ts +++ b/packages/storage/__tests__/internals/amplifyAuthAdapter/createAmplifyAuthConfigAdapter.test.ts @@ -15,6 +15,7 @@ jest.mock('@aws-amplify/core', () => ({ fetchAuthSession: jest.fn(), }, }, + fetchAuthSession: jest.fn(), })); jest.mock( '../../../src/internals/amplifyAuthConfigAdapter/generateLocationsFromPaths', @@ -54,16 +55,17 @@ describe('createAmplifyAuthConfigAdapter', () => { mockFetchAuthSession.mockResolvedValue({ credentials, identityId, - token: {}, + tokens: { + accessToken: { payload: {} }, + }, }); - it.only('should return an AuthConfigAdapter with listLocations function', async () => { + it('should return an AuthConfigAdapter with listLocations function', async () => { const adapter = createAmplifyAuthConfigAdapter(); expect(adapter).toHaveProperty('listLocations'); const { listLocations } = adapter; - const locations = await listLocations(); - console.log(locations); - // expect(mockFetchAuthSession).toHaveBeenCalled(); + await listLocations(); + expect(mockFetchAuthSession).toHaveBeenCalled(); }); it('should return empty locations when buckets are not defined', async () => { @@ -75,7 +77,7 @@ describe('createAmplifyAuthConfigAdapter', () => { expect(result).toEqual({ locations: [] }); }); - it.skip('should generate locations correctly when buckets are defined', async () => { + it('should generate locations correctly when buckets are defined', async () => { const mockBuckets = { bucket1: { bucketName: 'bucket1', diff --git a/packages/storage/src/internals/amplifyAuthConfigAdapter/createAmplifyAuthConfigAdapter.ts b/packages/storage/src/internals/amplifyAuthConfigAdapter/createAmplifyAuthConfigAdapter.ts index 4586033f45f..c6957b808da 100644 --- a/packages/storage/src/internals/amplifyAuthConfigAdapter/createAmplifyAuthConfigAdapter.ts +++ b/packages/storage/src/internals/amplifyAuthConfigAdapter/createAmplifyAuthConfigAdapter.ts @@ -31,7 +31,7 @@ const createAmplifyListLocationsHandler = (): ListPaths => { buckets, tokens: !!tokens, identityId, - userGroup: (userGroups as any)[0], // TODO: fix this edge case + userGroup: userGroups && (userGroups as any)[0], // TODO: fix this edge case }); return { locations }; From 19468f2ad8159027addebfe206872a50a3d4e4a0 Mon Sep 17 00:00:00 2001 From: ashika112 Date: Fri, 4 Oct 2024 12:30:59 -0700 Subject: [PATCH 09/19] fix bundle size test --- packages/aws-amplify/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/aws-amplify/package.json b/packages/aws-amplify/package.json index 1cede57f7ad..0ce7e85c564 100644 --- a/packages/aws-amplify/package.json +++ b/packages/aws-amplify/package.json @@ -497,7 +497,7 @@ "name": "[Storage] uploadData (S3)", "path": "./dist/esm/storage/index.mjs", "import": "{ uploadData }", - "limit": "22.07 kB" + "limit": "22.08 kB" } ] } From 9cbc71d8a78ce8944db9642d7222efaec55f99e1 Mon Sep 17 00:00:00 2001 From: ashika112 Date: Fri, 4 Oct 2024 12:41:26 -0700 Subject: [PATCH 10/19] update the listLocation handler --- .../createAmplifyAuthConfigAdapter.ts | 25 +--------------- .../createAmplifyListLocationsHandler.ts | 30 +++++++++++++++++++ .../createListLocationsHandler.ts | 21 ------------- 3 files changed, 31 insertions(+), 45 deletions(-) create mode 100644 packages/storage/src/internals/amplifyAuthConfigAdapter/createAmplifyListLocationsHandler.ts delete mode 100644 packages/storage/src/internals/amplifyAuthConfigAdapter/createListLocationsHandler.ts diff --git a/packages/storage/src/internals/amplifyAuthConfigAdapter/createAmplifyAuthConfigAdapter.ts b/packages/storage/src/internals/amplifyAuthConfigAdapter/createAmplifyAuthConfigAdapter.ts index c6957b808da..39ab73de29f 100644 --- a/packages/storage/src/internals/amplifyAuthConfigAdapter/createAmplifyAuthConfigAdapter.ts +++ b/packages/storage/src/internals/amplifyAuthConfigAdapter/createAmplifyAuthConfigAdapter.ts @@ -1,10 +1,9 @@ // Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 -import { Amplify, fetchAuthSession } from '@aws-amplify/core'; import { ListPaths } from '../types/credentials'; -import { generateLocationsFromPaths } from './generateLocationsFromPaths'; +import { createAmplifyListLocationsHandler } from './createAmplifyListLocationsHandler'; export interface AuthConfigAdapter { listLocations: ListPaths; @@ -15,25 +14,3 @@ export const createAmplifyAuthConfigAdapter = (): AuthConfigAdapter => { return { listLocations }; }; - -const createAmplifyListLocationsHandler = (): ListPaths => { - const { buckets } = Amplify.getConfig().Storage!.S3!; - - return async function listLocations() { - if (!buckets) { - return { locations: [] }; - } - - const { tokens, identityId } = await fetchAuthSession(); - const userGroups = tokens?.accessToken.payload['cognito:groups']; - - const locations = generateLocationsFromPaths({ - buckets, - tokens: !!tokens, - identityId, - userGroup: userGroups && (userGroups as any)[0], // TODO: fix this edge case - }); - - return { locations }; - }; -}; diff --git a/packages/storage/src/internals/amplifyAuthConfigAdapter/createAmplifyListLocationsHandler.ts b/packages/storage/src/internals/amplifyAuthConfigAdapter/createAmplifyListLocationsHandler.ts new file mode 100644 index 00000000000..a07ca1dd25e --- /dev/null +++ b/packages/storage/src/internals/amplifyAuthConfigAdapter/createAmplifyListLocationsHandler.ts @@ -0,0 +1,30 @@ +// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { Amplify, fetchAuthSession } from '@aws-amplify/core'; + +import { ListPaths } from '../types/credentials'; + +import { generateLocationsFromPaths } from './generateLocationsFromPaths'; + +export const createAmplifyListLocationsHandler = (): ListPaths => { + const { buckets } = Amplify.getConfig().Storage!.S3!; + + return async function listLocations() { + if (!buckets) { + return { locations: [] }; + } + + const { tokens, identityId } = await fetchAuthSession(); + const userGroups = tokens?.accessToken.payload['cognito:groups']; + + const locations = generateLocationsFromPaths({ + buckets, + tokens: !!tokens, + identityId, + userGroup: userGroups && (userGroups as any)[0], // TODO: fix this edge case + }); + + return { locations }; + }; +}; diff --git a/packages/storage/src/internals/amplifyAuthConfigAdapter/createListLocationsHandler.ts b/packages/storage/src/internals/amplifyAuthConfigAdapter/createListLocationsHandler.ts deleted file mode 100644 index b34dff9848c..00000000000 --- a/packages/storage/src/internals/amplifyAuthConfigAdapter/createListLocationsHandler.ts +++ /dev/null @@ -1,21 +0,0 @@ -// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. -// SPDX-License-Identifier: Apache-2.0 - -import { CredentialsProvider, ListLocations } from '../types/credentials'; -import { listCallerAccessGrants } from '../apis/listCallerAccessGrants'; - -interface CreateListLocationsHandlerInput { - accountId: string; - credentialsProvider: CredentialsProvider; - region: string; -} - -export const createListLocationsHandler = ( - handlerInput: CreateListLocationsHandlerInput, -): ListLocations => { - return async function listLocations(input = {}) { - const result = await listCallerAccessGrants({ ...input, ...handlerInput }); - - return result; - }; -}; From 2eae7b5a398d536edbd68ab0eca039cc83e816c5 Mon Sep 17 00:00:00 2001 From: ashika112 Date: Fri, 4 Oct 2024 16:28:11 -0700 Subject: [PATCH 11/19] implement memoization --- .../createAmplifyListLocationsHandler.ts | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/packages/storage/src/internals/amplifyAuthConfigAdapter/createAmplifyListLocationsHandler.ts b/packages/storage/src/internals/amplifyAuthConfigAdapter/createAmplifyListLocationsHandler.ts index a07ca1dd25e..1eb751ac5aa 100644 --- a/packages/storage/src/internals/amplifyAuthConfigAdapter/createAmplifyListLocationsHandler.ts +++ b/packages/storage/src/internals/amplifyAuthConfigAdapter/createAmplifyListLocationsHandler.ts @@ -3,12 +3,15 @@ import { Amplify, fetchAuthSession } from '@aws-amplify/core'; -import { ListPaths } from '../types/credentials'; +import { ListPaths, ListPathsOutput } from '../types/credentials'; import { generateLocationsFromPaths } from './generateLocationsFromPaths'; export const createAmplifyListLocationsHandler = (): ListPaths => { const { buckets } = Amplify.getConfig().Storage!.S3!; + // eslint-disable-next-line no-debugger + debugger; + let cachedResult: Record | null = null; return async function listLocations() { if (!buckets) { @@ -17,6 +20,11 @@ export const createAmplifyListLocationsHandler = (): ListPaths => { const { tokens, identityId } = await fetchAuthSession(); const userGroups = tokens?.accessToken.payload['cognito:groups']; + const cacheKey = JSON.stringify({ identityId, userGroups }) + `${!!tokens}`; + + if (cachedResult && cachedResult[cacheKey]) return cachedResult[cacheKey]; + + cachedResult = {}; const locations = generateLocationsFromPaths({ buckets, @@ -25,6 +33,8 @@ export const createAmplifyListLocationsHandler = (): ListPaths => { userGroup: userGroups && (userGroups as any)[0], // TODO: fix this edge case }); - return { locations }; + cachedResult[cacheKey] = { locations }; + + return cachedResult[cacheKey]; }; }; From c8af4548de19860107d18fc02e8d859ad0d9ee45 Mon Sep 17 00:00:00 2001 From: ashika112 Date: Mon, 7 Oct 2024 14:29:22 -0700 Subject: [PATCH 12/19] add pagination logic --- .../createAmplifyListLocationsHandler.ts | 54 +++++++++++++++++-- 1 file changed, 49 insertions(+), 5 deletions(-) diff --git a/packages/storage/src/internals/amplifyAuthConfigAdapter/createAmplifyListLocationsHandler.ts b/packages/storage/src/internals/amplifyAuthConfigAdapter/createAmplifyListLocationsHandler.ts index 1eb751ac5aa..7f8f49f1271 100644 --- a/packages/storage/src/internals/amplifyAuthConfigAdapter/createAmplifyListLocationsHandler.ts +++ b/packages/storage/src/internals/amplifyAuthConfigAdapter/createAmplifyListLocationsHandler.ts @@ -3,7 +3,7 @@ import { Amplify, fetchAuthSession } from '@aws-amplify/core'; -import { ListPaths, ListPathsOutput } from '../types/credentials'; +import { ListPaths, PathAccess } from '../types/credentials'; import { generateLocationsFromPaths } from './generateLocationsFromPaths'; @@ -11,18 +11,24 @@ export const createAmplifyListLocationsHandler = (): ListPaths => { const { buckets } = Amplify.getConfig().Storage!.S3!; // eslint-disable-next-line no-debugger debugger; - let cachedResult: Record | null = null; + let cachedResult: Record | null = null; - return async function listLocations() { + return async function listLocations(input = {}) { if (!buckets) { return { locations: [] }; } + const { pageSize, nextToken } = input; const { tokens, identityId } = await fetchAuthSession(); const userGroups = tokens?.accessToken.payload['cognito:groups']; const cacheKey = JSON.stringify({ identityId, userGroups }) + `${!!tokens}`; - if (cachedResult && cachedResult[cacheKey]) return cachedResult[cacheKey]; + if (cachedResult && cachedResult[cacheKey]) + return getPaginatedResult({ + locations: cachedResult[cacheKey].locations, + pageSize, + nextToken, + }); cachedResult = {}; @@ -35,6 +41,44 @@ export const createAmplifyListLocationsHandler = (): ListPaths => { cachedResult[cacheKey] = { locations }; - return cachedResult[cacheKey]; + return getPaginatedResult({ + locations: cachedResult[cacheKey].locations, + pageSize, + nextToken, + }); + }; +}; + +const getPaginatedResult = ({ + locations, + pageSize, + nextToken, +}: { + locations: PathAccess[]; + pageSize?: number; + nextToken?: string; +}) => { + if (pageSize) { + if (nextToken) { + const start = -nextToken; + const end = start > pageSize ? start + pageSize : undefined; + + return { + locations: locations.slice(start, end), + nextToken: end ? `${end}` : undefined, + }; + } + + return { + locations: locations.slice(0, pageSize), + nextToken: + locations.length > pageSize + ? `${locations.length - pageSize}` + : undefined, + }; + } + + return { + locations, }; }; From 76da9040da7905b6e2410eccff33506c33dd127e Mon Sep 17 00:00:00 2001 From: ashika112 Date: Mon, 7 Oct 2024 17:22:46 -0700 Subject: [PATCH 13/19] upadte usergroup logic & test --- .../generateLocationsFromPaths.test.ts | 42 +++++++++++++++++-- .../createAmplifyListLocationsHandler.ts | 2 +- .../generateLocationsFromPaths.ts | 26 ++++++------ 3 files changed, 53 insertions(+), 17 deletions(-) diff --git a/packages/storage/__tests__/internals/amplifyAuthAdapter/generateLocationsFromPaths.test.ts b/packages/storage/__tests__/internals/amplifyAuthAdapter/generateLocationsFromPaths.test.ts index 4dd15d7aa98..7d4d23c0ddb 100644 --- a/packages/storage/__tests__/internals/amplifyAuthAdapter/generateLocationsFromPaths.test.ts +++ b/packages/storage/__tests__/internals/amplifyAuthAdapter/generateLocationsFromPaths.test.ts @@ -37,7 +37,7 @@ describe('generateLocationsFromPaths', () => { buckets: mockBuckets, tokens: true, identityId: '12345', - userGroup: 'admin', + userGroup: ['admin'], }); expect(result).toEqual([ @@ -68,12 +68,48 @@ describe('generateLocationsFromPaths', () => { ]); }); + it('should generate locations correctly when tokens are true and usergroup order is given', () => { + const result = generateLocationsFromPaths({ + buckets: mockBuckets, + tokens: true, + identityId: '12345', + userGroup: ['editor', 'auditor'], + }); + + expect(result).toEqual([ + { + type: 'PREFIX', + permission: ['get', 'list', 'write'], + scope: { + bucketName: 'bucket1', + path: 'path1/*', + }, + }, + { + type: 'PREFIX', + permission: ['get', 'list'], + scope: { + bucketName: 'bucket1', + path: 'path2/*', + }, + }, + { + type: 'PREFIX', + permission: ['get', 'list', 'write', 'delete'], + scope: { + bucketName: 'bucket1', + path: 'profile-pictures/12345/*', + }, + }, + ]); + }); + it('should generate locations correctly when tokens are true & bad userGroup', () => { const result = generateLocationsFromPaths({ buckets: mockBuckets, tokens: true, identityId: '12345', - userGroup: 'editor', + userGroup: ['editor'], }); expect(result).toEqual([ @@ -107,7 +143,7 @@ describe('generateLocationsFromPaths', () => { }, tokens: true, identityId: '12345', - userGroup: 'admin', + userGroup: ['admin'], }); expect(result).toEqual([]); diff --git a/packages/storage/src/internals/amplifyAuthConfigAdapter/createAmplifyListLocationsHandler.ts b/packages/storage/src/internals/amplifyAuthConfigAdapter/createAmplifyListLocationsHandler.ts index 7f8f49f1271..cdd905c5085 100644 --- a/packages/storage/src/internals/amplifyAuthConfigAdapter/createAmplifyListLocationsHandler.ts +++ b/packages/storage/src/internals/amplifyAuthConfigAdapter/createAmplifyListLocationsHandler.ts @@ -36,7 +36,7 @@ export const createAmplifyListLocationsHandler = (): ListPaths => { buckets, tokens: !!tokens, identityId, - userGroup: userGroups && (userGroups as any)[0], // TODO: fix this edge case + userGroup: userGroups as any, // TODO: fix this edge case }); cachedResult[cacheKey] = { locations }; diff --git a/packages/storage/src/internals/amplifyAuthConfigAdapter/generateLocationsFromPaths.ts b/packages/storage/src/internals/amplifyAuthConfigAdapter/generateLocationsFromPaths.ts index fcd612f3416..aad0e011c3e 100644 --- a/packages/storage/src/internals/amplifyAuthConfigAdapter/generateLocationsFromPaths.ts +++ b/packages/storage/src/internals/amplifyAuthConfigAdapter/generateLocationsFromPaths.ts @@ -8,26 +8,26 @@ import { StorageAccess } from '../types/common'; const resolvePermissions = ( accessRule: Record, token: boolean, - groups?: string, + groups: string[] = [], ) => { if (!token) { - return { - permission: accessRule.guest, - }; + return { permission: accessRule.guest }; } - if (groups) { + + const matchingGroup = groups.find(group => + Object.keys(accessRule).some(key => key.includes(group)), + ); + + if (matchingGroup) { const selectedKey = Object.keys(accessRule).find( - access => access.includes(groups) || access.includes('authenticated'), + access => + access.includes(matchingGroup) || access.includes('authenticated'), ); - return { - permission: selectedKey ? accessRule[selectedKey] : undefined, - }; + return { permission: selectedKey ? accessRule[selectedKey] : undefined }; } - return { - permission: accessRule.authenticated, - }; + return { permission: accessRule.authenticated }; }; export const generateLocationsFromPaths = ({ @@ -39,7 +39,7 @@ export const generateLocationsFromPaths = ({ buckets: Record; tokens: boolean; identityId?: string; - userGroup?: string; + userGroup?: string[]; }): PathAccess[] => { const locations: PathAccess[] = []; From 87e87667d60d21cd58a2414d71f0cad84336ecba Mon Sep 17 00:00:00 2001 From: ashika112 Date: Mon, 7 Oct 2024 22:35:22 -0700 Subject: [PATCH 14/19] update getPaginated Locations --- .../getPaginatedLocations.test.ts | 87 +++++++++++++++++++ .../createAmplifyListLocationsHandler.ts | 42 +-------- .../getPaginatedLocations.ts | 41 +++++++++ 3 files changed, 132 insertions(+), 38 deletions(-) create mode 100644 packages/storage/__tests__/internals/amplifyAuthAdapter/getPaginatedLocations.test.ts create mode 100644 packages/storage/src/internals/amplifyAuthConfigAdapter/getPaginatedLocations.ts diff --git a/packages/storage/__tests__/internals/amplifyAuthAdapter/getPaginatedLocations.test.ts b/packages/storage/__tests__/internals/amplifyAuthAdapter/getPaginatedLocations.test.ts new file mode 100644 index 00000000000..600af6ae4d7 --- /dev/null +++ b/packages/storage/__tests__/internals/amplifyAuthAdapter/getPaginatedLocations.test.ts @@ -0,0 +1,87 @@ +import { getPaginatedLocations } from '../../../src/internals/amplifyAuthConfigAdapter/getPaginatedLocations'; +import { PathAccess } from '../../../src/internals/types/credentials'; + +describe('getPaginatedLocations', () => { + const mockLocations: PathAccess[] = [ + { + type: 'PREFIX', + permission: ['read'], + scope: { bucketName: 'bucket1', path: 'path1/' }, + }, + { + type: 'PREFIX', + permission: ['write'], + scope: { bucketName: 'bucket2', path: 'path2/' }, + }, + { + type: 'PREFIX', + permission: ['read', 'write'], + scope: { bucketName: 'bucket3', path: 'path3/' }, + }, + ]; + + it('should return all locations when no pagination is specified', () => { + const result = getPaginatedLocations({ locations: mockLocations }); + expect(result).toEqual({ locations: mockLocations }); + }); + + it('should return paginated locations when pageSize is specified', () => { + const result = getPaginatedLocations({ + locations: mockLocations, + pageSize: 2, + }); + expect(result).toEqual({ + locations: mockLocations.slice(0, 2), + nextToken: '1', + }); + }); + + it('should return paginated locations when pageSize and nextToken are specified', () => { + const result = getPaginatedLocations({ + locations: mockLocations, + pageSize: 1, + nextToken: '2', + }); + expect(result).toEqual({ + locations: mockLocations.slice(1, 2), + nextToken: '1', + }); + }); + + it('should return empty locations when locations array is empty', () => { + const result = getPaginatedLocations({ locations: [], pageSize: 2 }); + expect(result).toEqual({ locations: [] }); + }); + + it('should return empty location when nextToken is beyond array length', () => { + const result = getPaginatedLocations({ + locations: mockLocations, + pageSize: 2, + nextToken: '5', + }); + expect(result).toEqual({ locations: [], nextToken: undefined }); + }); + + it('should return all remaining location when page size is greater than remaining locations length', () => { + const result = getPaginatedLocations({ + locations: mockLocations, + pageSize: 5, + nextToken: '2', + }); + expect(result).toEqual({ + locations: mockLocations.slice(-2), + nextToken: undefined, + }); + }); + + it('should return undefined nextToken when end of array is reached', () => { + const result = getPaginatedLocations({ + locations: mockLocations, + pageSize: 5, + }); + expect(result).toEqual({ + locations: mockLocations.slice(0, 3), + nextToken: undefined, + }); + }); +}); diff --git a/packages/storage/src/internals/amplifyAuthConfigAdapter/createAmplifyListLocationsHandler.ts b/packages/storage/src/internals/amplifyAuthConfigAdapter/createAmplifyListLocationsHandler.ts index cdd905c5085..8ae3f260f4c 100644 --- a/packages/storage/src/internals/amplifyAuthConfigAdapter/createAmplifyListLocationsHandler.ts +++ b/packages/storage/src/internals/amplifyAuthConfigAdapter/createAmplifyListLocationsHandler.ts @@ -6,11 +6,11 @@ import { Amplify, fetchAuthSession } from '@aws-amplify/core'; import { ListPaths, PathAccess } from '../types/credentials'; import { generateLocationsFromPaths } from './generateLocationsFromPaths'; +import { getPaginatedLocations } from './getPaginatedLocations'; export const createAmplifyListLocationsHandler = (): ListPaths => { const { buckets } = Amplify.getConfig().Storage!.S3!; - // eslint-disable-next-line no-debugger - debugger; + let cachedResult: Record | null = null; return async function listLocations(input = {}) { @@ -24,7 +24,7 @@ export const createAmplifyListLocationsHandler = (): ListPaths => { const cacheKey = JSON.stringify({ identityId, userGroups }) + `${!!tokens}`; if (cachedResult && cachedResult[cacheKey]) - return getPaginatedResult({ + return getPaginatedLocations({ locations: cachedResult[cacheKey].locations, pageSize, nextToken, @@ -41,44 +41,10 @@ export const createAmplifyListLocationsHandler = (): ListPaths => { cachedResult[cacheKey] = { locations }; - return getPaginatedResult({ + return getPaginatedLocations({ locations: cachedResult[cacheKey].locations, pageSize, nextToken, }); }; }; - -const getPaginatedResult = ({ - locations, - pageSize, - nextToken, -}: { - locations: PathAccess[]; - pageSize?: number; - nextToken?: string; -}) => { - if (pageSize) { - if (nextToken) { - const start = -nextToken; - const end = start > pageSize ? start + pageSize : undefined; - - return { - locations: locations.slice(start, end), - nextToken: end ? `${end}` : undefined, - }; - } - - return { - locations: locations.slice(0, pageSize), - nextToken: - locations.length > pageSize - ? `${locations.length - pageSize}` - : undefined, - }; - } - - return { - locations, - }; -}; diff --git a/packages/storage/src/internals/amplifyAuthConfigAdapter/getPaginatedLocations.ts b/packages/storage/src/internals/amplifyAuthConfigAdapter/getPaginatedLocations.ts new file mode 100644 index 00000000000..93c501a92dd --- /dev/null +++ b/packages/storage/src/internals/amplifyAuthConfigAdapter/getPaginatedLocations.ts @@ -0,0 +1,41 @@ +// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { PathAccess } from '../types/credentials'; + +export const getPaginatedLocations = ({ + locations, + pageSize, + nextToken, +}: { + locations: PathAccess[]; + pageSize?: number; + nextToken?: string; +}) => { + if (pageSize) { + if (nextToken) { + if (Number(nextToken) > locations.length) { + return { locations: [], nextToken: undefined }; + } + const start = -nextToken; + const end = start + pageSize < 0 ? start + pageSize : undefined; + + return { + locations: locations.slice(start, end), + nextToken: end ? `${-end}` : undefined, + }; + } + + return { + locations: locations.slice(0, pageSize), + nextToken: + locations.length > pageSize + ? `${locations.length - pageSize}` + : undefined, + }; + } + + return { + locations, + }; +}; From 6f8b96814754f431ae30265e59bd523d71e8ebf1 Mon Sep 17 00:00:00 2001 From: ashika112 Date: Wed, 23 Oct 2024 10:21:08 -0700 Subject: [PATCH 15/19] fix failing test --- .../amplifyAuthAdapter/getPaginatedLocations.test.ts | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/packages/storage/__tests__/internals/amplifyAuthAdapter/getPaginatedLocations.test.ts b/packages/storage/__tests__/internals/amplifyAuthAdapter/getPaginatedLocations.test.ts index 600af6ae4d7..15809d88085 100644 --- a/packages/storage/__tests__/internals/amplifyAuthAdapter/getPaginatedLocations.test.ts +++ b/packages/storage/__tests__/internals/amplifyAuthAdapter/getPaginatedLocations.test.ts @@ -6,17 +6,20 @@ describe('getPaginatedLocations', () => { { type: 'PREFIX', permission: ['read'], - scope: { bucketName: 'bucket1', path: 'path1/' }, + bucket: 'bucket1', + prefix: 'path1/', }, { type: 'PREFIX', permission: ['write'], - scope: { bucketName: 'bucket2', path: 'path2/' }, + bucket: 'bucket2', + prefix: 'path2/', }, { type: 'PREFIX', permission: ['read', 'write'], - scope: { bucketName: 'bucket3', path: 'path3/' }, + bucket: 'bucket3', + prefix: 'path3/', }, ]; From 22dd0c2afbc1df86316487edf1258a722000a3ad Mon Sep 17 00:00:00 2001 From: ashika112 Date: Thu, 24 Oct 2024 11:53:32 -0700 Subject: [PATCH 16/19] update userGroup logic and tests --- packages/core/src/parseAmplifyOutputs.ts | 3 +- .../src/singleton/AmplifyOutputs/types.ts | 4 +- packages/core/src/singleton/Auth/types.ts | 5 ++ .../createAmplifyAuthConfigAdapter.test.ts | 30 +++++++--- .../getHighestPrecedenceUserGroup.test.ts | 57 +++++++++++++++++++ .../resolveLocationsForCurrentSession.test.ts | 31 +++++----- .../createAmplifyListLocationsHandler.ts | 17 +++++- .../getHighestPrecedenceUserGroup.ts | 43 ++++++++++++++ .../resolveLocationsForCurrentSession.ts | 11 +++- 9 files changed, 167 insertions(+), 34 deletions(-) create mode 100644 packages/storage/__tests__/internals/amplifyAuthAdapter/getHighestPrecedenceUserGroup.test.ts create mode 100644 packages/storage/src/internals/amplifyAuthConfigAdapter/getHighestPrecedenceUserGroup.ts diff --git a/packages/core/src/parseAmplifyOutputs.ts b/packages/core/src/parseAmplifyOutputs.ts index 21e52752812..22ef98d1ffe 100644 --- a/packages/core/src/parseAmplifyOutputs.ts +++ b/packages/core/src/parseAmplifyOutputs.ts @@ -5,7 +5,6 @@ /* eslint-disable camelcase */ /* Does not like exhaustive checks */ -/* eslint-disable no-case-declarations */ import { APIConfig, @@ -87,12 +86,14 @@ function parseAuth( oauth, username_attributes, standard_required_attributes, + groups, } = amplifyOutputsAuthProperties; const authConfig = { Cognito: { userPoolId: user_pool_id, userPoolClientId: user_pool_client_id, + groups, }, } as AuthConfig; diff --git a/packages/core/src/singleton/AmplifyOutputs/types.ts b/packages/core/src/singleton/AmplifyOutputs/types.ts index 244c58a9451..d154f04c418 100644 --- a/packages/core/src/singleton/AmplifyOutputs/types.ts +++ b/packages/core/src/singleton/AmplifyOutputs/types.ts @@ -13,7 +13,8 @@ export type AmplifyOutputsAuthMFAConfiguration = | 'NONE'; export type AmplifyOutputsAuthMFAMethod = 'SMS' | 'TOTP'; - +type UserGroupName = string; +type UserGroupPrecedence = Record; export interface AmplifyOutputsAuthProperties { aws_region: string; authentication_flow_type?: 'USER_SRP_AUTH' | 'CUSTOM_AUTH'; @@ -41,6 +42,7 @@ export interface AmplifyOutputsAuthProperties { unauthenticated_identities_enabled?: boolean; mfa_configuration?: string; mfa_methods?: string[]; + groups?: Record[]; } export interface AmplifyOutputsStorageBucketProperties { diff --git a/packages/core/src/singleton/Auth/types.ts b/packages/core/src/singleton/Auth/types.ts index fd7bc788472..70d784a91e3 100644 --- a/packages/core/src/singleton/Auth/types.ts +++ b/packages/core/src/singleton/Auth/types.ts @@ -108,6 +108,9 @@ export type LegacyUserAttributeKey = Uppercase; export type AuthVerifiableAttributeKey = 'email' | 'phone_number'; +type UserGroupName = string; +type UserGroupPrecedence = Record; + export type AuthConfigUserAttributes = Partial< Record >; @@ -130,6 +133,7 @@ export interface AuthIdentityPoolConfig { userAttributes?: never; mfa?: never; passwordFormat?: never; + groups?: never; }; } @@ -171,6 +175,7 @@ export interface CognitoUserPoolConfig { requireNumbers?: boolean; requireSpecialCharacters?: boolean; }; + groups?: Record[]; } export interface OAuthConfig { diff --git a/packages/storage/__tests__/internals/amplifyAuthAdapter/createAmplifyAuthConfigAdapter.test.ts b/packages/storage/__tests__/internals/amplifyAuthAdapter/createAmplifyAuthConfigAdapter.test.ts index 5f2a4bbb446..171210f8599 100644 --- a/packages/storage/__tests__/internals/amplifyAuthAdapter/createAmplifyAuthConfigAdapter.test.ts +++ b/packages/storage/__tests__/internals/amplifyAuthAdapter/createAmplifyAuthConfigAdapter.test.ts @@ -33,12 +33,24 @@ const mockFetchAuthSession = fetchAuthSession as jest.Mock; const mockResolveLocationsFromCurrentSession = resolveLocationsForCurrentSession as jest.Mock; +const mockAuthConfig = { + Auth: { + Cognito: { + userPoolClientId: 'userPoolClientId', + userPoolId: 'userPoolId', + identityPoolId: 'identityPoolId', + groups: [{ admin: { precedence: 0 } }], + }, + }, +}; + describe('createAmplifyAuthConfigAdapter', () => { beforeEach(() => { jest.clearAllMocks(); }); mockGetConfig.mockReturnValue({ + ...mockAuthConfig, Storage: { S3: { bucket: 'bucket1', @@ -70,7 +82,10 @@ describe('createAmplifyAuthConfigAdapter', () => { }); it('should return empty locations when buckets are not defined', async () => { - mockGetConfig.mockReturnValue({ Storage: { S3: { buckets: undefined } } }); + mockGetConfig.mockReturnValue({ + ...mockAuthConfig, + Storage: { S3: { buckets: undefined } }, + }); const adapter = createAmplifyAuthConfigAdapter(); const result = await adapter.listLocations(); @@ -93,16 +108,15 @@ describe('createAmplifyAuthConfigAdapter', () => { }; mockGetConfig.mockReturnValue({ + ...mockAuthConfig, Storage: { S3: { buckets: mockBuckets } }, }); mockResolveLocationsFromCurrentSession.mockReturnValue([ { type: 'PREFIX', permission: ['read', 'write'], - scope: { - bucketName: 'bucket1', - path: '/path1', - }, + bucket: 'bucket1', + prefix: '/path1', }, ]); @@ -114,10 +128,8 @@ describe('createAmplifyAuthConfigAdapter', () => { { type: 'PREFIX', permission: ['read', 'write'], - scope: { - bucketName: 'bucket1', - path: '/path1', - }, + bucket: 'bucket1', + prefix: '/path1', }, ], }); diff --git a/packages/storage/__tests__/internals/amplifyAuthAdapter/getHighestPrecedenceUserGroup.test.ts b/packages/storage/__tests__/internals/amplifyAuthAdapter/getHighestPrecedenceUserGroup.test.ts new file mode 100644 index 00000000000..bf1055797a0 --- /dev/null +++ b/packages/storage/__tests__/internals/amplifyAuthAdapter/getHighestPrecedenceUserGroup.test.ts @@ -0,0 +1,57 @@ +// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { + UserGroupConfig, + getHighestPrecedenceUserGroup, +} from '../../../src/internals/amplifyAuthConfigAdapter/getHighestPrecedenceUserGroup'; + +const userGroupsFromConfig: UserGroupConfig = [ + { + editor: { + precedence: 0, + }, + }, + { + admin: { + precedence: 1, + }, + }, + { + auditor: { + precedence: 2, + }, + }, +]; +const currentUserGroups = ['guest', 'user', 'admin']; + +describe('getHighestPrecedenceUserGroup', () => { + it('should return the user group with the highest precedence', () => { + const result = getHighestPrecedenceUserGroup( + userGroupsFromConfig, + currentUserGroups, + ); + expect(result).toBe('admin'); + }); + + it('should return undefined if userGroupsFromConfig is undefined', () => { + const result = getHighestPrecedenceUserGroup(undefined, currentUserGroups); + expect(result).toBeUndefined(); + }); + + it('should return undefined if currentUserGroups is undefined', () => { + const result = getHighestPrecedenceUserGroup( + userGroupsFromConfig, + undefined, + ); + expect(result).toBeUndefined(); + }); + + it('should handle currentUserGroups containing groups not present in userGroupsFromConfig', () => { + const result = getHighestPrecedenceUserGroup(userGroupsFromConfig, [ + 'unknown', + 'user', + ]); + expect(result).toBe(undefined); + }); +}); diff --git a/packages/storage/__tests__/internals/amplifyAuthAdapter/resolveLocationsForCurrentSession.test.ts b/packages/storage/__tests__/internals/amplifyAuthAdapter/resolveLocationsForCurrentSession.test.ts index bebfa85ef77..14119e9e80b 100644 --- a/packages/storage/__tests__/internals/amplifyAuthAdapter/resolveLocationsForCurrentSession.test.ts +++ b/packages/storage/__tests__/internals/amplifyAuthAdapter/resolveLocationsForCurrentSession.test.ts @@ -37,7 +37,6 @@ describe('resolveLocationsForCurrentSession', () => { buckets: mockBuckets, isAuthenticated: true, identityId: '12345', - userGroup: 'admin', }); expect(result).toEqual([ @@ -47,12 +46,6 @@ describe('resolveLocationsForCurrentSession', () => { bucket: 'bucket1', prefix: 'path1/*', }, - { - type: 'PREFIX', - permission: ['get', 'list', 'write', 'delete'], - bucket: 'bucket1', - prefix: 'path2/*', - }, { type: 'PREFIX', permission: ['get', 'list', 'write', 'delete'], @@ -62,30 +55,35 @@ describe('resolveLocationsForCurrentSession', () => { ]); }); - it('should generate locations correctly when tokens are true & bad userGroup', () => { + it('should generate locations correctly when tokens are true & userGroup', () => { const result = resolveLocationsForCurrentSession({ buckets: mockBuckets, isAuthenticated: true, identityId: '12345', - userGroup: 'editor', + userGroup: 'admin', }); expect(result).toEqual([ - { - type: 'PREFIX', - permission: ['get', 'list', 'write'], - bucket: 'bucket1', - prefix: 'path1/*', - }, { type: 'PREFIX', permission: ['get', 'list', 'write', 'delete'], bucket: 'bucket1', - prefix: 'profile-pictures/12345/*', + prefix: 'path2/*', }, ]); }); + it('should return empty locations when tokens are true & bad userGroup', () => { + const result = resolveLocationsForCurrentSession({ + buckets: mockBuckets, + isAuthenticated: true, + identityId: '12345', + userGroup: 'editor', + }); + + expect(result).toEqual([]); + }); + it('should continue to next bucket when paths are not defined', () => { const result = resolveLocationsForCurrentSession({ buckets: { @@ -107,7 +105,6 @@ describe('resolveLocationsForCurrentSession', () => { }, isAuthenticated: true, identityId: '12345', - userGroup: 'admin', }); expect(result).toEqual([ diff --git a/packages/storage/src/internals/amplifyAuthConfigAdapter/createAmplifyListLocationsHandler.ts b/packages/storage/src/internals/amplifyAuthConfigAdapter/createAmplifyListLocationsHandler.ts index 5cfcf96997b..f8150a34d04 100644 --- a/packages/storage/src/internals/amplifyAuthConfigAdapter/createAmplifyListLocationsHandler.ts +++ b/packages/storage/src/internals/amplifyAuthConfigAdapter/createAmplifyListLocationsHandler.ts @@ -7,9 +7,11 @@ import { ListPaths, PathAccess } from '../types/credentials'; import { getPaginatedLocations } from './getPaginatedLocations'; import { resolveLocationsForCurrentSession } from './resolveLocationsForCurrentSession'; +import { getHighestPrecedenceUserGroup } from './getHighestPrecedenceUserGroup'; export const createAmplifyListLocationsHandler = (): ListPaths => { const { buckets } = Amplify.getConfig().Storage!.S3!; + const { groups } = Amplify.getConfig().Auth!.Cognito; let cachedResult: Record | null = null; @@ -20,8 +22,17 @@ export const createAmplifyListLocationsHandler = (): ListPaths => { const { pageSize, nextToken } = input; const { tokens, identityId } = await fetchAuthSession(); - const userGroups = tokens?.accessToken.payload['cognito:groups']; - const cacheKey = JSON.stringify({ identityId, userGroups }) + `${!!tokens}`; + const currentUserGroups = tokens?.accessToken.payload['cognito:groups'] as + | string[] + | undefined; + + const userGroupToUse = getHighestPrecedenceUserGroup( + groups, + currentUserGroups, + ); + + const cacheKey = + JSON.stringify({ identityId, userGroup: userGroupToUse }) + `${!!tokens}`; if (cachedResult && cachedResult[cacheKey]) return getPaginatedLocations({ @@ -36,7 +47,7 @@ export const createAmplifyListLocationsHandler = (): ListPaths => { buckets, isAuthenticated: !!tokens, identityId, - userGroup: userGroups as any, // TODO: fix this edge case + userGroup: userGroupToUse, }); cachedResult[cacheKey] = { locations }; diff --git a/packages/storage/src/internals/amplifyAuthConfigAdapter/getHighestPrecedenceUserGroup.ts b/packages/storage/src/internals/amplifyAuthConfigAdapter/getHighestPrecedenceUserGroup.ts new file mode 100644 index 00000000000..f5927aea30d --- /dev/null +++ b/packages/storage/src/internals/amplifyAuthConfigAdapter/getHighestPrecedenceUserGroup.ts @@ -0,0 +1,43 @@ +// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +export type UserGroupConfig = Record>[]; + +/** + * Given a user group configuration and a list of current user groups, + * this function returns the user group with the highest precedence. + * + * @param {UserGroupConfig} userGroupsFromConfig - User groups with their precedence values based on Amplify outputs. + * @param {string[]} currentUserGroups - The list of current user's groups. + * @returns {string | undefined} - The user group with the highest precedence (0), or undefined if no matching group is found. + */ +export const getHighestPrecedenceUserGroup = ( + userGroupsFromConfig?: UserGroupConfig, + currentUserGroups?: string[], +): string | undefined => { + if (userGroupsFromConfig && currentUserGroups) { + const precedenceMap = userGroupsFromConfig.reduce( + (acc, group) => { + Object.entries(group).forEach(([key, value]) => { + acc[key] = value.precedence; + }); + + return acc; + }, + {} as Record, + ); + + const sortedUserGroup = currentUserGroups + .filter(group => + Object.prototype.hasOwnProperty.call(precedenceMap, group), + ) + .sort( + (a, b) => + (precedenceMap[a] ?? Infinity) - (precedenceMap[b] ?? Infinity), + ); + + return sortedUserGroup[0]; + } + + return undefined; +}; diff --git a/packages/storage/src/internals/amplifyAuthConfigAdapter/resolveLocationsForCurrentSession.ts b/packages/storage/src/internals/amplifyAuthConfigAdapter/resolveLocationsForCurrentSession.ts index 16e63066f1e..adfa63ece13 100644 --- a/packages/storage/src/internals/amplifyAuthConfigAdapter/resolveLocationsForCurrentSession.ts +++ b/packages/storage/src/internals/amplifyAuthConfigAdapter/resolveLocationsForCurrentSession.ts @@ -16,8 +16,8 @@ const resolvePermissions = ( }; } if (groups) { - const selectedKey = Object.keys(accessRule).find( - access => access.includes(groups) || access.includes('authenticated'), + const selectedKey = Object.keys(accessRule).find(access => + access.includes(groups), ); return { @@ -50,7 +50,12 @@ export const resolveLocationsForCurrentSession = ({ } for (const [path, accessRules] of Object.entries(paths)) { - if (path.includes(ENTITY_IDENTITY_URL) && isAuthenticated && identityId) { + if ( + !userGroup && + path.includes(ENTITY_IDENTITY_URL) && + isAuthenticated && + identityId + ) { locations.push({ type: 'PREFIX', permission: accessRules.entityidentity as StorageAccess[], From 55967423e906dbea4d96c122a7311f43806f7136 Mon Sep 17 00:00:00 2001 From: ashika112 Date: Thu, 24 Oct 2024 11:58:04 -0700 Subject: [PATCH 17/19] update for readbility --- .../resolveLocationsForCurrentSession.ts | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/packages/storage/src/internals/amplifyAuthConfigAdapter/resolveLocationsForCurrentSession.ts b/packages/storage/src/internals/amplifyAuthConfigAdapter/resolveLocationsForCurrentSession.ts index adfa63ece13..9071c2386ae 100644 --- a/packages/storage/src/internals/amplifyAuthConfigAdapter/resolveLocationsForCurrentSession.ts +++ b/packages/storage/src/internals/amplifyAuthConfigAdapter/resolveLocationsForCurrentSession.ts @@ -50,12 +50,13 @@ export const resolveLocationsForCurrentSession = ({ } for (const [path, accessRules] of Object.entries(paths)) { - if ( + const shouldIncludeEntityIdPath = !userGroup && path.includes(ENTITY_IDENTITY_URL) && isAuthenticated && - identityId - ) { + identityId; + + if (shouldIncludeEntityIdPath) { locations.push({ type: 'PREFIX', permission: accessRules.entityidentity as StorageAccess[], @@ -63,12 +64,14 @@ export const resolveLocationsForCurrentSession = ({ prefix: path.replace(ENTITY_IDENTITY_URL, identityId), }); } + const location = { type: 'PREFIX', ...resolvePermissions(accessRules, isAuthenticated, userGroup), bucket: bucketName, prefix: path, }; + if (location.permission) locations.push(location as PathAccess); } } From ffd3ff8493dd4b2f42f55e4c7edea8ba072fd607 Mon Sep 17 00:00:00 2001 From: ashika112 Date: Thu, 24 Oct 2024 12:36:26 -0700 Subject: [PATCH 18/19] update bundle size --- packages/aws-amplify/package.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/aws-amplify/package.json b/packages/aws-amplify/package.json index a629f3d5a51..da94bb7e6f3 100644 --- a/packages/aws-amplify/package.json +++ b/packages/aws-amplify/package.json @@ -461,7 +461,7 @@ "name": "[Storage] copy (S3)", "path": "./dist/esm/storage/index.mjs", "import": "{ copy }", - "limit": "16.05 kB" + "limit": "16.08 kB" }, { "name": "[Storage] downloadData (S3)", @@ -491,7 +491,7 @@ "name": "[Storage] remove (S3)", "path": "./dist/esm/storage/index.mjs", "import": "{ remove }", - "limit": "15.50 kB" + "limit": "15.52 kB" }, { "name": "[Storage] uploadData (S3)", From 3fcd481bd059383ebcf58984e262f4e285ee66b9 Mon Sep 17 00:00:00 2001 From: ashika112 Date: Thu, 24 Oct 2024 15:50:28 -0700 Subject: [PATCH 19/19] address feedbacks --- .../getHighestPrecedenceUserGroup.ts | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/packages/storage/src/internals/amplifyAuthConfigAdapter/getHighestPrecedenceUserGroup.ts b/packages/storage/src/internals/amplifyAuthConfigAdapter/getHighestPrecedenceUserGroup.ts index f5927aea30d..82303a9d0d8 100644 --- a/packages/storage/src/internals/amplifyAuthConfigAdapter/getHighestPrecedenceUserGroup.ts +++ b/packages/storage/src/internals/amplifyAuthConfigAdapter/getHighestPrecedenceUserGroup.ts @@ -4,8 +4,10 @@ export type UserGroupConfig = Record>[]; /** - * Given a user group configuration and a list of current user groups, - * this function returns the user group with the highest precedence. + * Given the Cognito user groups associated to current user session + * and all the user group configurations defined by backend. + * This function returns the user group with the highest precedence. + * Reference: https://docs.aws.amazon.com/cognito/latest/developerguide/cognito-user-pools-user-groups.html#assigning-precedence-values-to-groups * * @param {UserGroupConfig} userGroupsFromConfig - User groups with their precedence values based on Amplify outputs. * @param {string[]} currentUserGroups - The list of current user's groups. @@ -31,10 +33,7 @@ export const getHighestPrecedenceUserGroup = ( .filter(group => Object.prototype.hasOwnProperty.call(precedenceMap, group), ) - .sort( - (a, b) => - (precedenceMap[a] ?? Infinity) - (precedenceMap[b] ?? Infinity), - ); + .sort((a, b) => precedenceMap[a] - precedenceMap[b]); return sortedUserGroup[0]; }