feat: some changes according to PR comments

packages/amplify-category-api/src/provider-utils/awscloudformation/containers-handler.ts

@@ -6,7 +6,7 @@ import { DEPLOYMENT_MECHANISM } from './base-api-stack';
 import { GitHubSourceActionInfo } from './pipeline-with-awaiter';
 import { API_TYPE, IMAGE_SOURCE_TYPE, ResourceDependency, ServiceConfiguration } from './service-walkthroughs/containers-walkthrough';
 import { ApiResource, generateContainersArtifacts } from './utils/containers-artifacts';
-import { stateManager } from 'amplify-cli-core';
+import {addCustomPoliciesFile} from "amplify-cli-core";
 
 export const addResource = async (
   serviceWalkthroughPromise: Promise<ServiceConfiguration>,
@@ -97,7 +97,7 @@ export const addResource = async (
 
   }
 
-  stateManager.addCustomPoliciesFile(category, resourceName);
+  addCustomPoliciesFile(category, resourceName);
 
   context.print.success(`Successfully added resource ${resourceName} locally.`);
   context.print.info('');

packages/amplify-category-function/src/provider-utils/awscloudformation/index.ts

@@ -30,6 +30,7 @@ import {
   saveMutableState,
   updateLayerArtifacts,
 } from './utils/storeResources';
+import {addCustomPoliciesFile} from "amplify-cli-core";
 
 /**
  * Entry point for creating a new function
@@ -112,7 +113,7 @@ export async function addFunctionResource(
 
   await createFunctionResources(context, completeParams);
 
-  stateManager.addCustomPoliciesFile(category, completeParams.resourceName);
+  addCustomPoliciesFile(category, completeParams.resourceName);
 
   if (!completeParams.skipEdit) {
     await openEditor(context, category, completeParams.resourceName, completeParams.functionTemplate);

packages/amplify-cli-core/src/cfnUtilities.ts

@@ -231,7 +231,7 @@ const CF_SCHEMA = yaml.JSON_SCHEMA.extend([
   }),
 ]);
 
-function isJsonFileContent(fileContent: string): boolean {
+export function isJsonFileContent(fileContent: string): boolean {
   // We use the first character to determine if the content is json or yaml because historically the CLI could
   // have emitted JSON with YML extension, so we can't rely on filename extension.
   return fileContent?.trim()[0] === '{'; // CFN templates are always objects, never arrays

packages/amplify-cli-core/src/customPoliciesUtils.ts

@@ -1,4 +1,6 @@
 import { Fn, IAM } from "cloudform-types";
+import * as iam from '@aws-cdk/aws-iam';
+import { JSONUtilities, pathManager } from ".";
 
 export type CustomIAMPolicies = {
     policies:  CustomIAMPolicy[];
@@ -23,8 +25,8 @@ export const CustomIAMPoliciesSchema = {
 export const CustomIAMPolicySchema = {
         type: "object",
         properties: {
-            Action: {type: "array", items: {type: "string"}, nullable: false},
-            Effect: {type: "string", items: {type: "string"}, nullable: true, default: "Allow"},
+            Action: {type: "array", items: {type: "string"}, minItems: 1, nullable: false},
+            Effect: {type: "string", nullable: true, default: iam.Effect.ALLOW},
             Resource: {type: "array", items: {type: "string"}, minItems: 1, nullable: false},
         },
         required: ["Resource", "Action"],
@@ -53,5 +55,18 @@ export const customExecutionPolicyForContainer = new IAM.Policy({
     ]
   });
 
+export function addCustomPoliciesFile(categoryName: string, resourceName: string) {
+  const customPoliciesPath = pathManager.getCustomPoliciesPath(categoryName, resourceName);
+  const defaultCustomPolicies = {
+      policies: [
+        {
+          Action: [],
+          Resource: []
+        }
+      ]
+  }
+  JSONUtilities.writeJson(customPoliciesPath, defaultCustomPolicies);
+}
+
 
 

packages/amplify-cli-core/src/state-manager/pathManager.ts

@@ -42,7 +42,7 @@ export const PathConstants = {
 
   CfnFileName: (resourceName: string) => `${resourceName}-awscloudformation-template.json`,
 
-  CustomPoliciesFilename: 'custom-iam-policy-documents.json',
+  CustomPoliciesFilename: 'custom-policies.json',
 };
 
 export class PathManager {

packages/amplify-cli-core/src/state-manager/stateManager.ts

@@ -6,6 +6,7 @@ import _ from 'lodash';
 import { SecretFileMode } from '../cliConstants';
 import { HydrateTags, ReadTags, Tag } from '../tags';
 import { CustomIAMPolicies } from '../customPoliciesUtils';
+import { isJsonFileContent} from '../cfnUtilities'
 
 export type GetOptions<T> = {
   throwIfNotExist?: boolean;
@@ -76,30 +77,13 @@ export class StateManager {
     return this.getData<$TSTeamProviderInfo>(filePath, mergedOptions);
   };
 
-  getCustomPolicies = (service: string, categoryName: string, resourceName: string): CustomIAMPolicies | undefined => {
-    if (!(service === 'Lambda' || service === 'ElasticContainer')) {
-      return undefined;
-    }
+  getCustomPolicies = (categoryName: string, resourceName: string): CustomIAMPolicies | undefined => {
     const filePath = pathManager.getCustomPoliciesPath(categoryName, resourceName);
-    if (!filePath) {
+    if (!(fs.existsSync(filePath)) || !isJsonFileContent(fs.readFileSync(filePath, 'utf8'))) {
       return undefined;
     }
-    return JSONUtilities.readJson<CustomIAMPolicies>(filePath, {throwIfNotExist : false});
-  };
-
-  addCustomPoliciesFile = (categoryName: string, resourceName: string): void => {
-    const customPoliciesPath = pathManager.getCustomPoliciesPath(categoryName, resourceName);
-    const defaultCustomPolicies = {
-        policies: [
-          {
-            Effect: 'Allow',
-            Action: [],
-            Resource: []
-          }
-        ]
-    }
-    JSONUtilities.writeJson(customPoliciesPath, defaultCustomPolicies);
-  }
+    return JSONUtilities.readJson<CustomIAMPolicies>(filePath);
+  };
 
   localEnvInfoExists = (projectPath?: string): boolean => this.doesExist(pathManager.getLocalEnvFilePath, projectPath);
 

packages/amplify-e2e-tests/src/__tests__/custom_policies_container.test.ts

@@ -5,14 +5,15 @@ import {
   getCustomPoliciesPath, 
   amplifyPushWithoutCodegen,
   readJsonFile,
-  addRestContainerApiForCustomPolicies
+  addRestContainerApiForCustomPolicies,
+  amplifyConfigureProject,
+  createNewProjectDir,
+  deleteProjectDir 
 } from 'amplify-e2e-core';
-import { createNewProjectDir, deleteProjectDir } from 'amplify-e2e-core';
 import _ from 'lodash';
 import { JSONUtilities } from 'amplify-cli-core';
 import AWS from 'aws-sdk';
 import path from 'path';
-import { amplifyConfigureProject } from 'amplify-e2e-core';
 
 const customIAMPolicy: CustomIAMPolicy = {
             Effect: 'Allow',
@@ -55,7 +56,7 @@ it(`should init and deploy a api container, attach custom policies to the Fargat
   const { Region: region } = meta?.providers?.awscloudformation;
 
   // Put SSM parameter
-  let ssmClient = new AWS.SSM({ region });
+  const ssmClient = new AWS.SSM({ region });
   await ssmClient.putParameter({
     Name: 'testCustomPolicies',
     Value: 'testCustomPoliciesValue',

packages/amplify-e2e-tests/src/__tests__/custom_policies_function.test.ts

@@ -5,11 +5,13 @@ import {
   getProjectMeta, 
   getCustomPoliciesPath, 
   overrideFunctionCodeNode ,
-  invokeFunction
+  invokeFunction,
+  addFunction,
+  addLambdaTrigger,
+  addSimpleDDB,
+  createNewProjectDir,
+  deleteProjectDir
 } from 'amplify-e2e-core';
-import { addFunction, addLambdaTrigger } from 'amplify-e2e-core';
-import { addSimpleDDB } from 'amplify-e2e-core';
-import { createNewProjectDir, deleteProjectDir } from 'amplify-e2e-core';
 import _ from 'lodash';
 import { JSONUtilities } from 'amplify-cli-core';
 import AWS from 'aws-sdk';
@@ -58,7 +60,7 @@ it(`should init and deploy storage DynamoDB + Lambda trigger, attach custom poli
   const { Region: region } = meta?.providers?.awscloudformation;
 
   // Put SSM parameter
-  let ssmClient = new AWS.SSM({ region });
+  const ssmClient = new AWS.SSM({ region });
   await ssmClient.putParameter({
     Name: 'testCustomPolicies',
     Value: 'testCustomPoliciesValue',

packages/amplify-provider-awscloudformation/src/pre-push-cfn-processor/cfn-pre-processor.ts

@@ -15,6 +15,7 @@ import { prePushCfnTemplateModifier } from './pre-push-cfn-modifier';
 import { Fn, Template } from 'cloudform-types';
 import { printer } from 'amplify-prompts';
 import Ajv from "ajv";
+import * as iam from '@aws-cdk/aws-iam';
 
 
 const buildDir = 'build';
@@ -48,21 +49,33 @@ export async function writeCustomPoliciesToCFNTemplate(
   category: string,
   filePath: string
 ) {
+  if (!(service === 'Lambda' || service === 'ElasticContainer')) {
+    return;
+  }
   const { templateFormat, cfnTemplate } = await readCFNTemplate(path.join(resourceDir, cfnFile));
-  const customPolicies = stateManager.getCustomPolicies(service, category, resourceName);
+  const customPolicies = stateManager.getCustomPolicies(category, resourceName);
   if (!validateExistCustomPolicies(customPolicies)) {
+    if (cfnTemplate.Resources.CustomLambdaExecutionPolicy) {
+      delete cfnTemplate.Resources.CustomLambdaExecutionPolicy;
+      await writeCFNTemplate(cfnTemplate, filePath, { templateFormat });
+    }
+    if (cfnTemplate.Resources.CustomLambdaExecutionPolicy) {
+      delete cfnTemplate.Resources.CustomExecutionPolicyForContainer;
+      await writeCFNTemplate(cfnTemplate, filePath, { templateFormat });
+    }
     return;
   }
   await validateCustomPoliciesSchema(customPolicies, category, resourceName);
 
-  await addCustomPoliciesToCFNTemplate(service, customPolicies.policies, cfnTemplate, filePath, resourceName, {templateFormat} );
+  await addCustomPoliciesToCFNTemplate(service, category, customPolicies.policies, cfnTemplate, filePath, resourceName, {templateFormat} );
 
 }
 
 //merge the custom IAM polciies to CFN template for lambda and API container
 
 async function addCustomPoliciesToCFNTemplate(
   service: string,
+  category: string,
   customPolicies: CustomIAMPolicy[],
   cfnTemplate: Template,
   filePath: string,
@@ -84,9 +97,9 @@ async function addCustomPoliciesToCFNTemplate(
   warnWildcardCustomPolicies(customPolicies, resourceName);
 
   for (const customPolicy of customPolicies) {
-    validateCustomPolicy(customPolicy, resourceName);
+    validateCustomPolicy(customPolicy, category, resourceName);
     const policyWithEnv = await replaceEnvForCustomPolicies(customPolicy, envName);
-    if (!policyWithEnv.Effect) policyWithEnv.Effect = 'Allow';
+    if (!policyWithEnv.Effect) policyWithEnv.Effect = iam.Effect.ALLOW;
     customExecutionPolicy.Properties.PolicyDocument.Statement.push(policyWithEnv);
   }
 
@@ -102,17 +115,17 @@ async function addCustomPoliciesToCFNTemplate(
 
 
 //validate the format of actions and ARNs for custom IAM policies
-function validateCustomPolicy(customPolicy: CustomIAMPolicy, resourceName: string) {
+function validateCustomPolicy(customPolicy: CustomIAMPolicy, category: string, resourceName: string) {
   const resources = customPolicy.Resource;
   const actions = customPolicy.Action;
-  let resourceRegex = new RegExp('arn:(aws[a-zA-Z0-9-]*):([a-zA-Z0-9\\-])+:([a-z]{2}(-gov)?-[a-z]+-\\d{1})?:(\\d{12})?:(.*)');
-  let actionRegex = new RegExp('[a-zA-Z0-9]+:[a-z|A-Z|0-9|*]+');
-  let wrongResourcesRegex = [];
-  let wrongActionsRegex = [];
+  const resourceRegex = new RegExp('arn:(aws[a-zA-Z0-9-]*):([a-zA-Z0-9\\-])+:([a-z]{2}(-gov)?-[a-z]+-\\d{1})?:(\\d{12})?:(.*)');
+  const actionRegex = new RegExp('[a-zA-Z0-9]+:[a-z|A-Z|0-9|*]+');
+  const wrongResourcesRegex = [];
+  const wrongActionsRegex = [];
   let errorMessage = "";
 
   for (const resource of resources) {
-    if (!resourceRegex.test(resource) && !(resource === '*')) {
+    if (!(resourceRegex.test(resource) && resource === '*')) {
       wrongResourcesRegex.push(resource);
     }
   }
@@ -123,11 +136,13 @@ function validateCustomPolicy(customPolicy: CustomIAMPolicy, resourceName: strin
     }
   }
 
+  const customPoliciesPath = pathManager.getCustomPoliciesPath(category, resourceName);
+
   if (wrongResourcesRegex.length > 0) {
-    errorMessage += `\nInvalid ARN format for custom IAM policies in ${resourceName}:\n${wrongResourcesRegex.toString()}\n`;
+    errorMessage += `Invalid custom IAM policy for {function name}. Incorrect "Resource": ${wrongResourcesRegex.toString()}\n Edit ${customPoliciesPath} to fix`;
   }
   if (wrongActionsRegex.length > 0) {
-    errorMessage += `\nInvalid actions format for custom IAM policies in ${resourceName}:\n${wrongActionsRegex.toString()}\n`;
+    errorMessage += `Invalid custom IAM policy for {function name}. Incorrect "Action": ${wrongActionsRegex.toString()}\n Edit ${customPoliciesPath} to fix`;
   }
 
   if (errorMessage.length > 0) {
@@ -155,10 +170,10 @@ function validateExistCustomPolicies(customPolicies: CustomIAMPolicies) : Boolea
 
 //replace or add env parameter in the front of the resource customers enter to the current env
 async function replaceEnvForCustomPolicies(policy: CustomIAMPolicy, currentEnv: string) : Promise<CustomIAMPolicy> {
-  let resourceWithEnv = [];
-  let action = policy.Action;
-  let effect = policy.Effect;
-  let customIAMpolicy: CustomIAMPolicy = {
+  const resourceWithEnv = [];
+  const action = policy.Action;
+  const effect = policy.Effect;
+  const customIAMpolicy: CustomIAMPolicy = {
     Action: action,
     Effect: effect,
     Resource: []
@@ -178,7 +193,9 @@ async function validateCustomPoliciesSchema(data: CustomIAMPolicies, categoryNam
 
   for(const policy of data.policies) {
     if(!validatePolicy(policy)) {
-      let errorMessage = `The format of custom IAM policies in the ${categoryName} ${resourceName} is not valid\n`;
+      let errorMessage = `Invalid custom IAM policies in the ${resourceName} ${categoryName} is invalid.\n
+      Edit <project-dir>/amplify/backend/function/socialmediademoea2a770a/custom-policies.json to fix
+      Learn more about custom IAM policies for ${categoryName}: https://docs.amplify.aws/function/custom-policies`;
       validatePolicy.errors.forEach(error => errorMessage += error.message + "\n");
       throw new CustomPoliciesFormatError(errorMessage);
     }
@@ -187,8 +204,6 @@ async function validateCustomPoliciesSchema(data: CustomIAMPolicies, categoryNam
 
 function warnWildcardCustomPolicies(customPolicies: CustomIAMPolicy[], resourceName: string) {
   customPolicies
-  .map(policy => policy.Resource)
-    .forEach(resources => resources
-      .filter(resource => resource === '*')
-    .forEach( wildcardResources =>printer.warn(`Warning: You've specified "*" as the resource in a custom IAM policy for ${resourceName}.\n This will give ${resourceName} access to ALL resources in the AWS Account.`)))
+    .filter(policy => policy.Resource.includes("*"))
+    .forEach( policy =>printer.warn(`Warning: You've specified "*" as the "Resource" in ${resourceName}'s custom IAM policy.\n This will grant ${resourceName} the ability to perform ${policy.Action} on ALL resources in this AWS Account.`))
 }