From 55002b6f91db290267d43981ff940353b6580ee4 Mon Sep 17 00:00:00 2001 From: Yathi <511386+yuth@users.noreply.github.com> Date: Wed, 26 Jan 2022 14:29:26 -0800 Subject: [PATCH] test: add tests --- .../package.json | 1 + .../utils/consolidate-apigw-policies.test.ts | 284 ++++++++++++++++++ .../src/utils/consolidate-apigw-policies.ts | 30 +- yarn.lock | 19 +- 4 files changed, 326 insertions(+), 8 deletions(-) create mode 100644 packages/amplify-provider-awscloudformation/src/__tests__/utils/consolidate-apigw-policies.test.ts diff --git a/packages/amplify-provider-awscloudformation/package.json b/packages/amplify-provider-awscloudformation/package.json index d8cee4fee3b..50a41c891dc 100644 --- a/packages/amplify-provider-awscloudformation/package.json +++ b/packages/amplify-provider-awscloudformation/package.json @@ -127,6 +127,7 @@ "xstate": "^4.14.0" }, "devDependencies": { + "@aws-cdk/assertions": "~1.124.0", "@types/columnify": "^1.5.0", "@types/deep-diff": "^1.0.0", "@types/folder-hash": "^4.0.0", diff --git a/packages/amplify-provider-awscloudformation/src/__tests__/utils/consolidate-apigw-policies.test.ts b/packages/amplify-provider-awscloudformation/src/__tests__/utils/consolidate-apigw-policies.test.ts new file mode 100644 index 00000000000..a94c4570ab5 --- /dev/null +++ b/packages/amplify-provider-awscloudformation/src/__tests__/utils/consolidate-apigw-policies.test.ts @@ -0,0 +1,284 @@ +import { Capture, Template, Match } from '@aws-cdk/assertions'; +import * as cdk from '@aws-cdk/core'; +import { cloneDeep } from 'lodash'; + +import { ApiGatewayAuthStack, CrudOperation } from '../../utils/consolidate-apigw-policies'; + +const generatePolicyDoc = (roleName: string, policy: any, assertionType: 'Presence' | 'Absence' = 'Presence') => { + return { + Roles: [ + { + Ref: roleName, + }, + ], + PolicyDocument: Match.objectLike({ + Statement: Match.arrayWith([ + Match.objectLike({ + Effect: 'Allow', + Action: ['execute-api:Invoke'], + Resource: (assertionType === 'Presence' ? Match.arrayWith : Match.not)([policy]), + }), + ]), + }), + }; +}; + +describe('ApiGatewayAuthStack', () => { + let policyDocTemplate; + + beforeEach(() => { + policyDocTemplate = { + 'Fn::Join': [ + '', + [ + 'arn:aws:execute-api:', + { + Ref: 'AWS::Region', + }, + ':', + { + Ref: 'AWS::AccountId', + }, + ':', + { + Ref: 'restApi', + }, + '/', + { + 'Fn::If': [ + 'ShouldNotCreateEnvResources', + 'Prod', + { + Ref: 'env', + }, + ], + }, + ], + ], + }; + }); + + it('should generate policy that collapses to * when all HTTP verbs are allowed', () => { + const app = new cdk.App(); + const apiAuthStack = new ApiGatewayAuthStack(app, 'authStack', { + envName: 'dev', + stackName: 'authStack', + description: 'authStackForAPI', + apiGateways: [ + { + resourceName: 'restApi', + params: { + paths: { + '/items': { + lambdaFunction: 'myFn1', + name: '/items', + permissions: { + settings: 'protected', + auth: [CrudOperation.CREATE, CrudOperation.DELETE, CrudOperation.READ, CrudOperation.UPDATE], + }, + }, + }, + }, + }, + ], + }); + + const template = Template.fromStack(apiAuthStack); + // Auth policy collapsed to * HTTP_VERB + // Proxy auth policy + const policyDocProxy = cloneDeep(policyDocTemplate); + policyDocProxy['Fn::Join'][1].push('/*/items/*'); + template.hasResourceProperties('AWS::IAM::ManagedPolicy', generatePolicyDoc('authRoleName', policyDocProxy)); + + const policyDoc = cloneDeep(policyDocTemplate); + policyDoc['Fn::Join'][1].push('/*/items'); + template.hasResourceProperties('AWS::IAM::ManagedPolicy', generatePolicyDoc('authRoleName', policyDoc)); + }); + + it('should not generate authPolicy when authRole rules are missing', () => { + const app = new cdk.App(); + const apiAuthStack = new ApiGatewayAuthStack(app, 'authStack', { + envName: 'dev', + stackName: 'authStack', + description: 'authStackForAPI', + apiGateways: [ + { + resourceName: 'restApi', + params: { + paths: { + '/items': { + lambdaFunction: 'myFn1', + name: '/items', + permissions: { + settings: 'protected', + guest: [CrudOperation.CREATE], + }, + }, + }, + }, + }, + ], + }); + const template = Template.fromStack(apiAuthStack); + template.hasResourceProperties( + 'AWS::IAM::ManagedPolicy', + Match.not({ + Roles: [ + { + Ref: 'authRoleName', + }, + ], + }), + ); + }); + + it('should not generate unAuthPolicy when unAuthRole rules are missing', () => { + const app = new cdk.App(); + const apiAuthStack = new ApiGatewayAuthStack(app, 'authStack', { + envName: 'dev', + stackName: 'authStack', + description: 'authStackForAPI', + apiGateways: [ + { + resourceName: 'restApi', + params: { + paths: { + '/items': { + lambdaFunction: 'myFn1', + name: '/items', + permissions: { + settings: 'protected', + auth: [CrudOperation.CREATE], + }, + }, + }, + }, + }, + ], + }); + const template = Template.fromStack(apiAuthStack); + template.hasResourceProperties( + 'AWS::IAM::ManagedPolicy', + Match.not({ + Roles: [ + { + Ref: 'unauthRoleName', + }, + ], + }), + ); + }); + + it('should generate policy that allows only allowed HTTP verb', () => { + const app = new cdk.App(); + const apiAuthStack = new ApiGatewayAuthStack(app, 'authStack', { + envName: 'dev', + stackName: 'authStack', + description: 'authStackForAPI', + apiGateways: [ + { + resourceName: 'restApi', + params: { + paths: { + '/items': { + lambdaFunction: 'myFn1', + name: '/items', + permissions: { + settings: 'protected', + guest: [CrudOperation.CREATE], + }, + }, + }, + }, + }, + ], + }); + const template = Template.fromStack(apiAuthStack); + + // UnAuth policy CREATE crud option should create POST + const unAuthPolicyProxy = cloneDeep(policyDocTemplate); + unAuthPolicyProxy['Fn::Join'][1].push('/POST/items/*'); + + template.hasResourceProperties('AWS::IAM::ManagedPolicy', generatePolicyDoc('unauthRoleName', unAuthPolicyProxy)); + + const unAuthPolicy = cloneDeep(policyDocTemplate); + unAuthPolicy['Fn::Join'][1].push('/POST/items'); + template.hasResourceProperties('AWS::IAM::ManagedPolicy', generatePolicyDoc('unauthRoleName', unAuthPolicy)); + + // Check for other HTTP verbs absence + const unAuthGet = cloneDeep(policyDocTemplate); + unAuthGet['Fn::Join'][1].push('/GET/items'); + template.hasResourceProperties('AWS::IAM::ManagedPolicy', generatePolicyDoc('unauthRoleName', unAuthGet, 'Absence')); + + const unAuthGetProxy = cloneDeep(policyDocTemplate); + unAuthGetProxy['Fn::Join'][1].push('/GET/items/*'); + template.hasResourceProperties('AWS::IAM::ManagedPolicy', generatePolicyDoc('unauthRoleName', unAuthGetProxy, 'Absence')); + + const unAuthPut = cloneDeep(policyDocTemplate); + unAuthGet['Fn::Join'][1].push('/PUT/items'); + template.hasResourceProperties('AWS::IAM::ManagedPolicy', generatePolicyDoc('unauthRoleName', unAuthPut, 'Absence')); + + const unAuthPutProxy = cloneDeep(policyDocTemplate); + unAuthPutProxy['Fn::Join'][1].push('/PUT/items/*'); + template.hasResourceProperties('AWS::IAM::ManagedPolicy', generatePolicyDoc('unauthRoleName', unAuthPutProxy, 'Absence')); + }); + + // The test needs CDK to be updated to 1.140.0 so it can use capture.next. Skipping it for now + it('should slice managed role when the size of the policy document exceeds 6K', () => { + // create 100 paths + const paths = new Array(100).fill(1).reduce((acc, _, idx) => { + return { + ...acc, + [`/items${idx}`]: { + lambdaFunction: 'myFn1', + name: `/items${idx}`, + permissions: { + settings: 'protected', + auth: [CrudOperation.CREATE, CrudOperation.UPDATE, CrudOperation.DELETE, CrudOperation.READ], + guest: [CrudOperation.READ], + }, + }, + }; + }, {}); + + const app = new cdk.App(); + + const apiAuthStack = new ApiGatewayAuthStack(app, 'authStack', { + envName: 'dev', + stackName: 'authStack', + description: 'authStackForAPI', + apiGateways: [ + { + resourceName: 'restApi', + params: { + paths, + }, + }, + ], + }); + + const template = Template.fromStack(apiAuthStack); + template.resourceCountIs('AWS::IAM::ManagedPolicy', 6); + + const policyStatements = new Capture(); + template.hasResourceProperties('AWS::IAM::ManagedPolicy', { + PolicyDocument: Match.objectLike({ + Version: '2012-10-17', + Statement: Match.arrayWith([ + { + Effect: 'Allow', + Action: ['execute-api:Invoke'], + Resource: policyStatements, + }, + ]), + }), + Roles: Match.arrayEquals([Match.anyValue()]), + }); + + // Skipping this test for now as the CDK version needs to be updated to use `policyStatements.next()` + // do { + // const policy = JSON.stringify(policyStatements.asArray()); + // expect(policy.length).toBeLessThanOrEqual(6_144); + // } while (policyStatements.next()); + }); +}); diff --git a/packages/amplify-provider-awscloudformation/src/utils/consolidate-apigw-policies.ts b/packages/amplify-provider-awscloudformation/src/utils/consolidate-apigw-policies.ts index 066f67e473f..14ff938ead1 100644 --- a/packages/amplify-provider-awscloudformation/src/utils/consolidate-apigw-policies.ts +++ b/packages/amplify-provider-awscloudformation/src/utils/consolidate-apigw-policies.ts @@ -16,10 +16,30 @@ import * as fs from 'fs-extra'; import * as path from 'path'; import { ProviderName } from '../constants'; +// type information from input schema located at packages/amplify-category-api/resources/schemas/aPIGateway/APIGatewayCLIInputs.schema.json +type APIGatewayPermissionSetting = 'open' | 'private' | 'protected'; +type APIGateway = { + resourceName: string; + params?: { + paths?: Record< + string, + { + name?: string; + lambdaFunction?: string; + permissions?: { + settings?: APIGatewayPermissionSetting; + auth?: CrudOperation[]; + guest?: CrudOperation[]; + }; + } + >; + }; +}; + type ApiGatewayAuthStackProps = Readonly<{ description: string; stackName: string; - apiGateways: $TSAny[]; + apiGateways: APIGateway[]; envName: string; }>; @@ -44,7 +64,7 @@ const S3_UPLOAD_PATH = `${AmplifyCategories.API}/${APIGW_AUTH_STACK_LOGICAL_ID}. const AUTH_ROLE_NAME = 'authRoleName'; const UNAUTH_ROLE_NAME = 'unauthRoleName'; -class ApiGatewayAuthStack extends cdk.Stack { +export class ApiGatewayAuthStack extends cdk.Stack { constructor(scope: cdk.Construct, id: string, props: ApiGatewayAuthStackProps) { super(scope, id, props); this.templateOptions.templateFormatVersion = CFN_TEMPLATE_FORMAT_VERSION; @@ -215,7 +235,7 @@ export async function consolidateApiGatewayPolicies(context: $TSContext, stackNa return { APIGatewayAuthURL: createApiGatewayAuthResources(stackName, apiGateways, envInfo.envName) }; } -enum CrudOperation { +export enum CrudOperation { CREATE = 'create', READ = 'read', UPDATE = 'update', @@ -229,7 +249,9 @@ function convertCrudOperationsToPermissions(crudOps: CrudOperation[]) { [CrudOperation.UPDATE]: ['/PUT', '/PATCH'], [CrudOperation.DELETE]: ['/DELETE'], }; - return crudOps.flatMap(op => opMap[op]); + const possibleMethods = Object.values(opMap).flat(); + const methods = crudOps.flatMap(op => opMap[op]); + return possibleMethods.every(m => methods.includes(m)) ? ['/*'] : methods; } function createApiGatewayAuthResources(stackName: string, apiGateways: $TSAny, envName: string): string | undefined { diff --git a/yarn.lock b/yarn.lock index 998836de4eb..b2a2dd14f85 100644 --- a/yarn.lock +++ b/yarn.lock @@ -16466,9 +16466,9 @@ json-buffer@3.0.0: integrity sha1-Wx85evx11ne96Lz8Dkfh+aPZqJg= json-diff@^0.5.4: - version "0.5.4" - resolved "https://registry.npmjs.org/json-diff/-/json-diff-0.5.4.tgz#7bc8198c441756632aab66c7d9189d365a7a035a" - integrity sha512-q5Xmx9QXNOzOzIlMoYtLrLiu4Jl/Ce2bn0CNcv54PhyH89CI4GWlGVDye8ei2Ijt9R3U+vsWPsXpLUNob8bs8Q== + version "0.5.5" + resolved "https://registry.npmjs.org/json-diff/-/json-diff-0.5.5.tgz#24658ad200dbdd64ae8a56baf4d87b2b33d7196e" + integrity sha512-B2RSfPv8Y5iqm6/9aKC3cOhXPzjYupKDpGuqT5py9NRulL8J0UoB/zKXUo70xBsuxPcIFgtsGgEdXLrNp0GL7w== dependencies: cli-color "~0.1.6" difflib "~0.2.1" @@ -22989,7 +22989,7 @@ sync-fetch@^0.3.1: buffer "^5.7.0" node-fetch "^2.6.1" -table@^6.0.9, table@^6.7.1: +table@^6.0.9: version "6.7.5" resolved "https://registry.npmjs.org/table/-/table-6.7.5.tgz#f04478c351ef3d8c7904f0e8be90a1b62417d238" integrity sha512-LFNeryOqiQHqCVKzhkymKwt6ozeRhlm8IL1mE8rNUurkir4heF6PzMyRgaTa4tlyPTGGgXuvVOF/OLWiH09Lqw== @@ -23000,6 +23000,17 @@ table@^6.0.9, table@^6.7.1: string-width "^4.2.3" strip-ansi "^6.0.1" +table@^6.7.1: + version "6.8.0" + resolved "https://registry.npmjs.org/table/-/table-6.8.0.tgz#87e28f14fa4321c3377ba286f07b79b281a3b3ca" + integrity sha512-s/fitrbVeEyHKFa7mFdkuQMWlH1Wgw/yEXMt5xACT4ZpzWFluehAxRtUUQKPuWhaLAWhFcVx6w3oC8VKaUfPGA== + dependencies: + ajv "^8.0.1" + lodash.truncate "^4.4.2" + slice-ansi "^4.0.0" + string-width "^4.2.3" + strip-ansi "^6.0.1" + tapable@^1.0.0, tapable@^1.1.3: version "1.1.3" resolved "https://registry.npmjs.org/tapable/-/tapable-1.1.3.tgz#a1fccc06b58db61fd7a45da2da44f5f3a3e67ba2"