From 69dbc7b5f0a0ed01c8b8a312de31e96533f17ca2 Mon Sep 17 00:00:00 2001 From: Roshane Pascual Date: Mon, 23 Sep 2024 16:13:34 -0700 Subject: [PATCH] pr feedback --- .../src/storage_access_orchestrator.test.ts | 2 - .../src/storage_access_orchestrator.ts | 4 +- .../src/storage_outputs_aspect.test.ts | 44 +++++++++++++++++++ .../src/storage_outputs_aspect.ts | 19 +++++--- packages/client-config/API.md | 2 +- .../client_config_contributor_v1.test.ts | 6 ++- .../client_config_v1.2.ts | 8 ++-- 7 files changed, 68 insertions(+), 17 deletions(-) diff --git a/packages/backend-storage/src/storage_access_orchestrator.test.ts b/packages/backend-storage/src/storage_access_orchestrator.test.ts index 72fd70cd4d..75cecc30ba 100644 --- a/packages/backend-storage/src/storage_access_orchestrator.test.ts +++ b/packages/backend-storage/src/storage_access_orchestrator.test.ts @@ -500,7 +500,6 @@ void describe('StorageAccessOrchestrator', () => { ); assert.deepStrictEqual(storageAccessDefinitionOutput, { 'foo/*': { - 'auth-with-id': ['write', 'delete'], auth: ['get'], }, [`foo/${entityIdSubstitution}/*`]: { @@ -657,7 +656,6 @@ void describe('StorageAccessOrchestrator', () => { }, 'other/*': { stub1: ['get'], - stub2: ['get', 'write', 'delete'], }, [`other/${entityIdSubstitution}/*`]: { stub2: ['get', 'write', 'delete'], diff --git a/packages/backend-storage/src/storage_access_orchestrator.ts b/packages/backend-storage/src/storage_access_orchestrator.ts index ba6322f61e..35d2553ab4 100644 --- a/packages/backend-storage/src/storage_access_orchestrator.ts +++ b/packages/backend-storage/src/storage_access_orchestrator.ts @@ -100,8 +100,8 @@ export class StorageAccessOrchestrator { ([s3Prefix, accessPermissions]) => { const uniqueDefinitionIdSet = new Set(); // iterate over all of the access definitions for a given prefix - const accessConfig: StorageAccessConfig = {}; accessPermissions.forEach((permission) => { + const accessConfig: StorageAccessConfig = {}; // replace "read" with "get" and "list" in actions const replaceReadWithGetAndList = permission.actions.flatMap( (action) => (action === 'read' ? ['get', 'list'] : [action]) @@ -291,7 +291,7 @@ const placeholderSubstitution = ( idSubstitution ) as StoragePath; - // for auth/unauth roles where prefix ends with '/*/*' remove the last wildcard + // for owner paths where prefix ends with '/*/*' remove the last wildcard if (prefix.endsWith('/*/*')) { return prefix.slice(0, -2) as StoragePath; } diff --git a/packages/backend-storage/src/storage_outputs_aspect.test.ts b/packages/backend-storage/src/storage_outputs_aspect.test.ts index 7487f0864d..cccf5ba94c 100644 --- a/packages/backend-storage/src/storage_outputs_aspect.test.ts +++ b/packages/backend-storage/src/storage_outputs_aspect.test.ts @@ -7,6 +7,7 @@ import { StorageOutput } from '@aws-amplify/backend-output-schemas'; import { App, Stack } from 'aws-cdk-lib'; import { IConstruct } from 'constructs'; import { AmplifyUserError } from '@aws-amplify/platform-core'; +import { StorageAccessDefinitionOutput } from './private_types.js'; void describe('StorageOutputsAspect', () => { let app: App; @@ -129,6 +130,49 @@ void describe('StorageOutputsAspect', () => { }) ); }); + + void it('should add access paths if the storage has access rules configured', () => { + const accessDefinition = { + 'path/*': { + authenticated: ['get', 'list', 'write', 'delete'], + guest: ['get', 'list'], + }, + }; + const node = new AmplifyStorage(stack, 'test', { name: 'testName' }); + node.addAccessDefinition( + accessDefinition as StorageAccessDefinitionOutput + ); + aspect = new StorageOutputsAspect(outputStorageStrategy); + aspect.visit(node); + + assert.equal( + addBackendOutputEntryMock.mock.calls[0].arguments[0], + 'AWS::Amplify::Storage' + ); + assert.equal( + addBackendOutputEntryMock.mock.calls[0].arguments[1].payload + .storageRegion, + Stack.of(node).region + ); + assert.equal( + addBackendOutputEntryMock.mock.calls[0].arguments[1].payload.bucketName, + node.resources.bucket.bucketName + ); + assert.equal( + appendToBackendOutputListMock.mock.calls[0].arguments[0], + 'AWS::Amplify::Storage' + ); + assert.equal( + appendToBackendOutputListMock.mock.calls[0].arguments[1].payload + .buckets, + JSON.stringify({ + name: node.name, + bucketName: node.resources.bucket.bucketName, + storageRegion: Stack.of(node).region, + paths: accessDefinition, + }) + ); + }); }); void describe('Validate', () => { diff --git a/packages/backend-storage/src/storage_outputs_aspect.ts b/packages/backend-storage/src/storage_outputs_aspect.ts index 39c400b92c..dd448937a5 100644 --- a/packages/backend-storage/src/storage_outputs_aspect.ts +++ b/packages/backend-storage/src/storage_outputs_aspect.ts @@ -7,6 +7,7 @@ import { BackendOutputStorageStrategy } from '@aws-amplify/plugin-types'; import { IAspect, Stack } from 'aws-cdk-lib'; import { IConstruct } from 'constructs'; import { AmplifyStorage } from './construct.js'; +import { StorageAccessDefinitionOutput } from './private_types.js'; /** * Aspect to store the storage outputs in the backend @@ -95,16 +96,22 @@ export class StorageOutputsAspect implements IAspect { }, }); } + const bucketsPayload: Record< + string, + string | StorageAccessDefinitionOutput + > = { + name: node.name, + bucketName: node.resources.bucket.bucketName, + storageRegion: Stack.of(node).region, + }; + if (node.accessDefinition) { + bucketsPayload.paths = node.accessDefinition; + } // both default and non-default buckets should have the name, bucket name, and storage region stored in `buckets` field outputStorageStrategy.appendToBackendOutputList(storageOutputKey, { version: '1', payload: { - buckets: JSON.stringify({ - name: node.name, - bucketName: node.resources.bucket.bucketName, - storageRegion: Stack.of(node).region, - paths: node.accessDefinition, - }), + buckets: JSON.stringify(bucketsPayload), }, }); }; diff --git a/packages/client-config/API.md b/packages/client-config/API.md index ea25f09a2d..55c083d3e7 100644 --- a/packages/client-config/API.md +++ b/packages/client-config/API.md @@ -21,6 +21,7 @@ type AmazonCognitoStandardAttributes_3 = 'address' | 'birthdate' | 'email' | 'fa // @public interface AmazonLocationServiceConfig { + name?: string; style?: string; } @@ -128,7 +129,6 @@ export type AuthClientConfig = { // @public interface AWSAmplifyBackendOutputs { - $schema?: string; analytics?: { amazon_pinpoint?: { aws_region: string; diff --git a/packages/client-config/src/client-config-contributor/client_config_contributor_v1.test.ts b/packages/client-config/src/client-config-contributor/client_config_contributor_v1.test.ts index 92532b36fc..022bea9dd4 100644 --- a/packages/client-config/src/client-config-contributor/client_config_contributor_v1.test.ts +++ b/packages/client-config/src/client-config-contributor/client_config_contributor_v1.test.ts @@ -542,7 +542,8 @@ void describe('storage client config contributor v1', () => { storageRegion: 'testRegion', paths: { 'path/*': { - guest: ['get'], + guest: ['get', 'list'], + authenticated: ['read', 'write', 'delete'], }, }, }), @@ -569,7 +570,8 @@ void describe('storage client config contributor v1', () => { aws_region: 'testRegion', paths: { 'path/*': { - guest: ['get'], + guest: ['get', 'list'], + authenticated: ['read', 'write', 'delete'], }, }, }, diff --git a/packages/client-config/src/client-config-schema/client_config_v1.2.ts b/packages/client-config/src/client-config-schema/client_config_v1.2.ts index 629894f90f..f9aa397d97 100644 --- a/packages/client-config/src/client-config-schema/client_config_v1.2.ts +++ b/packages/client-config/src/client-config-schema/client_config_v1.2.ts @@ -57,10 +57,6 @@ export type AmplifyStorageAccessActions = * Config format for Amplify Gen 2 client libraries to communicate with backend services. */ export interface AWSAmplifyBackendOutputs { - /** - * JSON schema - */ - $schema?: string; /** * Version of this schema */ @@ -246,6 +242,10 @@ export interface AWSAmplifyBackendOutputs { * via the `patternProperty` ".*". */ export interface AmazonLocationServiceConfig { + /** + * Map resource name + */ + name?: string; /** * Map style */