diff --git a/packages/amplify-category-api/src/provider-utils/awscloudformation/containers-handler.ts b/packages/amplify-category-api/src/provider-utils/awscloudformation/containers-handler.ts index e9eb4e7b5c3..982bc5723c5 100644 --- a/packages/amplify-category-api/src/provider-utils/awscloudformation/containers-handler.ts +++ b/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, @@ -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(''); diff --git a/packages/amplify-category-function/src/provider-utils/awscloudformation/index.ts b/packages/amplify-category-function/src/provider-utils/awscloudformation/index.ts index 7f3203ecdff..5706a966041 100644 --- a/packages/amplify-category-function/src/provider-utils/awscloudformation/index.ts +++ b/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); diff --git a/packages/amplify-cli-core/src/cfnUtilities.ts b/packages/amplify-cli-core/src/cfnUtilities.ts index 171edbea06a..c9dde10f420 100644 --- a/packages/amplify-cli-core/src/cfnUtilities.ts +++ b/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 diff --git a/packages/amplify-cli-core/src/customPoliciesUtils.ts b/packages/amplify-cli-core/src/customPoliciesUtils.ts index f877e7dd09b..df992ca386d 100644 --- a/packages/amplify-cli-core/src/customPoliciesUtils.ts +++ b/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); +} + diff --git a/packages/amplify-cli-core/src/state-manager/pathManager.ts b/packages/amplify-cli-core/src/state-manager/pathManager.ts index 7b299aec51e..4c5853222cf 100644 --- a/packages/amplify-cli-core/src/state-manager/pathManager.ts +++ b/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 { diff --git a/packages/amplify-cli-core/src/state-manager/stateManager.ts b/packages/amplify-cli-core/src/state-manager/stateManager.ts index 334530e9e88..796ae50a09e 100644 --- a/packages/amplify-cli-core/src/state-manager/stateManager.ts +++ b/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 = { 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(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(filePath); + }; localEnvInfoExists = (projectPath?: string): boolean => this.doesExist(pathManager.getLocalEnvFilePath, projectPath); diff --git a/packages/amplify-e2e-tests/src/__tests__/custom_policies_container.test.ts b/packages/amplify-e2e-tests/src/__tests__/custom_policies_container.test.ts index 9f38b084d3f..1685b57747b 100644 --- a/packages/amplify-e2e-tests/src/__tests__/custom_policies_container.test.ts +++ b/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', diff --git a/packages/amplify-e2e-tests/src/__tests__/custom_policies_function.test.ts b/packages/amplify-e2e-tests/src/__tests__/custom_policies_function.test.ts index e361dc3ea04..d6eec5a2f2f 100644 --- a/packages/amplify-e2e-tests/src/__tests__/custom_policies_function.test.ts +++ b/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', diff --git a/packages/amplify-provider-awscloudformation/src/pre-push-cfn-processor/cfn-pre-processor.ts b/packages/amplify-provider-awscloudformation/src/pre-push-cfn-processor/cfn-pre-processor.ts index 44abbf9ead1..52b31c4cddd 100644 --- a/packages/amplify-provider-awscloudformation/src/pre-push-cfn-processor/cfn-pre-processor.ts +++ b/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,14 +49,25 @@ 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} ); } @@ -63,6 +75,7 @@ export async function writeCustomPoliciesToCFNTemplate( 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 { - 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 /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.`)) } \ No newline at end of file