pr feedback

aws-amplify · Sep 23, 2024 · 69dbc7b · 69dbc7b
1 parent 12db2ca
commit 69dbc7b
Show file tree

Hide file tree

Showing 7 changed files with 68 additions and 17 deletions.
diff --git 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
@@ -100,8 +100,8 @@ export class StorageAccessOrchestrator {
       ([s3Prefix, accessPermissions]) => {
         const uniqueDefinitionIdSet = new Set<string>();
         // 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
    */