From fca493b5c039ce3e9e2fd8e368594df471d6dad2 Mon Sep 17 00:00:00 2001 From: kaizen3031593 Date: Mon, 18 Apr 2022 17:52:33 -0400 Subject: [PATCH 01/14] update readme to reflect new feature --- packages/@aws-cdk/aws-lambda/README.md | 79 ++++++++++++++++++++++++-- 1 file changed, 73 insertions(+), 6 deletions(-) diff --git a/packages/@aws-cdk/aws-lambda/README.md b/packages/@aws-cdk/aws-lambda/README.md index 573d469bfccd6..db368943d21bf 100644 --- a/packages/@aws-cdk/aws-lambda/README.md +++ b/packages/@aws-cdk/aws-lambda/README.md @@ -155,12 +155,13 @@ if (fn.timeout) { AWS Lambda supports resource-based policies for controlling access to Lambda functions and layers on a per-resource basis. In particular, this allows you to -give permission to AWS services and other AWS accounts to modify and invoke your -functions. You can also restrict permissions given to AWS services by providing -a source account or ARN (representing the account and identifier of the resource -that accesses the function or layer). +give permission to AWS services, AWS Organizations, or other AWS accounts to +modify and invoke your functions. + +### Grant function access to AWS services ```ts +// Grant permissions to a service declare const fn: lambda.Function; const principal = new iam.ServicePrincipal('my-service'); @@ -172,10 +173,76 @@ fn.addPermission('my-service Invocation', { }); ``` -For more information, see [Resource-based -policies](https://docs.aws.amazon.com/lambda/latest/dg/access-control-resource-based.html) +You can also restrict permissions given to AWS services by providing +a source account or ARN (representing the account and identifier of the resource +that accesses the function or layer). + +For more information, see +[Granting function access to AWS services](https://docs.aws.amazon.com/lambda/latest/dg/access-control-resource-based.html#permissions-resource-serviceinvoke) in the AWS Lambda Developer Guide. +### Grant function access to an AWS Organization + +```ts +// Grant permissions to an entire AWS organization +declare const fn: lambda.Function; +const orgPrincipal = new iam.OrganizationPrincipal('o-xxxxxxxxxx'); + +fn.grantInvoke(orgPrincipal); + +// Equivalent to: +fn.addPermission('grant to entire org', { + principal: orgPrincipal, +}); + +// Equivalent to: +fn.addPermission('grant to entire org', { + principal: '*', + principalOrg: orgPrincipal, +}) +``` + +In the above example, the `principal` will be `*` and all users in the +organization `o-xxxxxxxxxx` will get function invocation permissions. + +You can restrict permissions given to the organization by specifying an +AWS account or role as the `principal`: + +```ts +// Grant permission to an account ONLY IF they are part of the organization +declare const fn: lambda.Function; + +fn.addPermission('grant to account in org', { + principal: iam.AccountPrincipal('123456789012'); + principalOrg: iam.OrganizationPrincipal('o-xxxxxxxxxx'), +}); +``` + +For more information, see +[Granting function access to an organization](https://docs.aws.amazon.com/lambda/latest/dg/access-control-resource-based.html#permissions-resource-xorginvoke) +in the AWS Lambda Developer Guide. + +### Grant function access to other AWS accounts + +```ts +// Grant permission to other AWS account +declare const fn: lambda.Function; +const account = iam.AccountPrincipal('123456789012'); + +fn.grantInvoke(account); + +// Equivalent to: +fn.addPermission('grant to account in org', { + principal: account, +}); +``` + +For more information, see +[Granting function access to other accounts](https://docs.aws.amazon.com/lambda/latest/dg/access-control-resource-based.html#permissions-resource-xaccountinvoke) +in the AWS Lambda Developer Guide. + +### Grant function access to unowned principals + Providing an unowned principal (such as account principals, generic ARN principals, service principals, and principals in other accounts) to a call to `fn.grantInvoke` will result in a resource-based policy being created. If the From 34f5cae2174bd4cef0fa9c2ddf1377daa67ed0bc Mon Sep 17 00:00:00 2001 From: kaizen3031593 Date: Mon, 18 Apr 2022 17:52:46 -0400 Subject: [PATCH 02/14] add new prop and change docs for principal --- .../@aws-cdk/aws-lambda/lib/permission.ts | 26 ++++++++++++++----- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/packages/@aws-cdk/aws-lambda/lib/permission.ts b/packages/@aws-cdk/aws-lambda/lib/permission.ts index 2808bab77ff65..4c93fdca34df3 100644 --- a/packages/@aws-cdk/aws-lambda/lib/permission.ts +++ b/packages/@aws-cdk/aws-lambda/lib/permission.ts @@ -26,19 +26,21 @@ export interface Permission { * A unique token that must be supplied by the principal invoking the * function. * - * @default The caller would not need to present a token. + * @default - The caller would not need to present a token. */ readonly eventSourceToken?: string; /** * The entity for which you are granting permission to invoke the Lambda - * function. This entity can be any valid AWS service principal, such as - * s3.amazonaws.com or sns.amazonaws.com, or, if you are granting - * cross-account permission, an AWS account ID. For example, you might want - * to allow a custom application in another AWS account to push events to - * Lambda by invoking your function. + * function. This entity can be any of the following: * - * The principal can be either an AccountPrincipal or a ServicePrincipal. + * - a valid AWS service principal, such as `s3.amazonaws.com` or `sns.amazonaws.com` + * - an AWS account ID for cross-account permissions. For example, you might want + * to allow a custom application in another AWS account to push events to + * Lambda by invoking your function. + * - an AWS organization principal to grant permissions to an entire organization. + * + * The principal can be an AccountPrincipal, a ServicePrincipal, or an OrganizationPrincipal. */ readonly principal: iam.IPrincipal; @@ -70,6 +72,16 @@ export interface Permission { */ readonly sourceArn?: string; + /** + * The organization you want to grant permissions to. Use this ONLY if you + * need to grant permissions to a subset of the organization. In most cases, + * sending the organization principal through the `principal` property will + * suffice. + * + * @default - No principalOrg + */ + readonly principalOrg?: iam.OrganizationPrincipal; + /** * The authType for the function URL that you are granting permissions for. * From d2122d23b9c0296de492b62dfc66727e8f07fa9d Mon Sep 17 00:00:00 2001 From: kaizen3031593 Date: Tue, 19 Apr 2022 13:32:14 -0400 Subject: [PATCH 03/14] more doc changes --- packages/@aws-cdk/aws-lambda/README.md | 2 +- packages/@aws-cdk/aws-lambda/lib/permission.ts | 12 ++++++++---- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/packages/@aws-cdk/aws-lambda/README.md b/packages/@aws-cdk/aws-lambda/README.md index db368943d21bf..b217a380fe3c2 100644 --- a/packages/@aws-cdk/aws-lambda/README.md +++ b/packages/@aws-cdk/aws-lambda/README.md @@ -197,7 +197,7 @@ fn.addPermission('grant to entire org', { // Equivalent to: fn.addPermission('grant to entire org', { - principal: '*', + principal: iam.StarPrincipal(), principalOrg: orgPrincipal, }) ``` diff --git a/packages/@aws-cdk/aws-lambda/lib/permission.ts b/packages/@aws-cdk/aws-lambda/lib/permission.ts index 4c93fdca34df3..3c5b2b1e83c65 100644 --- a/packages/@aws-cdk/aws-lambda/lib/permission.ts +++ b/packages/@aws-cdk/aws-lambda/lib/permission.ts @@ -40,7 +40,8 @@ export interface Permission { * Lambda by invoking your function. * - an AWS organization principal to grant permissions to an entire organization. * - * The principal can be an AccountPrincipal, a ServicePrincipal, or an OrganizationPrincipal. + * The principal can be an AccountPrincipal, an ArnPrincipal, a ServicePrincipal, + * or an OrganizationPrincipal. */ readonly principal: iam.IPrincipal; @@ -74,9 +75,12 @@ export interface Permission { /** * The organization you want to grant permissions to. Use this ONLY if you - * need to grant permissions to a subset of the organization. In most cases, - * sending the organization principal through the `principal` property will - * suffice. + * need to grant permissions to a subset of the organization. If you want to + * grant permissions to the entire organization, sending the organization principal + * through the `principal` property will suffice. + * + * You can use this property to ensure that all source principals are owned by + * a specific organization. * * @default - No principalOrg */ From 71e6bd6d9d7cc7f65756dff6e357412ea8a40ee1 Mon Sep 17 00:00:00 2001 From: kaizen3031593 Date: Tue, 19 Apr 2022 13:32:38 -0400 Subject: [PATCH 04/14] main commit with changes to permission handling and testing --- .../@aws-cdk/aws-lambda/lib/function-base.ts | 82 +++++++++++-- .../@aws-cdk/aws-lambda/test/function.test.ts | 108 +++++++++++++++++- 2 files changed, 177 insertions(+), 13 deletions(-) diff --git a/packages/@aws-cdk/aws-lambda/lib/function-base.ts b/packages/@aws-cdk/aws-lambda/lib/function-base.ts index f53bc54a18012..21a677ba3b15e 100644 --- a/packages/@aws-cdk/aws-lambda/lib/function-base.ts +++ b/packages/@aws-cdk/aws-lambda/lib/function-base.ts @@ -345,8 +345,17 @@ export abstract class FunctionBase extends Resource implements IFunction, ec2.IC return; } - const principal = this.parsePermissionPrincipal(permission.principal); - const { sourceAccount, sourceArn } = this.parseConditions(permission.principal) ?? {}; + let principal = this.parsePermissionPrincipal(permission.principal); + + let { sourceArn, sourceAccount, principalOrgID } = this.validateConditionCombinations(permission.principal) ?? {}; + + // Lambda does not actually accept an organization principal here. + // so the principal becomes '*' and the organization ID will be sent to + // the 'principalOrgID' property. + if (permission.principal instanceof iam.OrganizationPrincipal) { + principalOrgID = permission.principal.organizationId; + } + const action = permission.action ?? 'lambda:InvokeFunction'; const scope = permission.scope ?? this; @@ -359,6 +368,7 @@ export abstract class FunctionBase extends Resource implements IFunction, ec2.IC eventSourceToken: permission.eventSourceToken, sourceAccount: permission.sourceAccount ?? sourceAccount, sourceArn: permission.sourceArn ?? sourceArn, + principalOrgId: permission.principalOrg?.organizationId ?? principalOrgID, functionUrlAuthType: permission.functionUrlAuthType, }); } @@ -566,6 +576,15 @@ export abstract class FunctionBase extends Resource implements IFunction, ec2.IC return (principal as iam.ArnPrincipal).arn; } + if (principal instanceof iam.OrganizationPrincipal) { + // we will move the organization id to the `principalOrgId` property of `Permissions`. + return '*'; + } + + if (principal instanceof iam.StarPrincipal) { + return '*'; + } + // Try a best-effort approach to support simple principals that are not any of the predefined // classes, but are simple enough that they will fit into the Permission model. Main target // here: imported Roles, Users, Groups. @@ -580,17 +599,61 @@ export abstract class FunctionBase extends Resource implements IFunction, ec2.IC } throw new Error(`Invalid principal type for Lambda permission statement: ${principal.constructor.name}. ` + - 'Supported: AccountPrincipal, ArnPrincipal, ServicePrincipal'); + 'Supported: AccountPrincipal, ArnPrincipal, ServicePrincipal, OrganizationPrincipal'); + } + + private validateConditionCombinations(principal: iam.IPrincipal): { + sourceArn: string | undefined, + sourceAccount: string | undefined, + principalOrgID: string | undefined, + } | undefined { + const conditions = this.validateConditions(principal); + + if (!conditions) { return undefined; } + + const sourceArn = conditions.ArnLike ? conditions.ArnLike['aws:SourceArn'] : undefined; + const sourceAccount = conditions.StringEquals ? conditions.StringEquals['aws:SourceAccount'] : undefined; + const principalOrgID = conditions.StringEquals ? conditions.StringEquals['aws:PrincipalOrgID'] : undefined; + + // PrincipalOrgID cannot be combined with any other conditions + if (principalOrgID && (sourceArn || sourceAccount)) { + throw new Error('PrincipalWithConditions had unsupported condition combinations for Lambda permission statement: principalOrgID cannot be set with other conditions.'); + } + + // SourceArn and SourceAccount must be set together + if ((sourceArn && !sourceAccount) || (!sourceArn && sourceAccount)) { + throw new Error('PrincipalWithConditions had unsupported condition combinations for Lambda permission statement: you cannot set sourceAccount without sourceArn and vice versa'); + } + + return { + sourceArn, + sourceAccount, + principalOrgID, + }; } - private parseConditions(principal: iam.IPrincipal): { sourceAccount: string, sourceArn: string } | null { + private validateConditions(principal: iam.IPrincipal): iam.Conditions | undefined { if (this.isPrincipalWithConditions(principal)) { const conditions: iam.Conditions = principal.policyFragment.conditions; const conditionPairs = flatMap( Object.entries(conditions), ([operator, conditionObjs]) => Object.keys(conditionObjs as object).map(key => { return { operator, key }; }), ); - const supportedPrincipalConditions = [{ operator: 'ArnLike', key: 'aws:SourceArn' }, { operator: 'StringEquals', key: 'aws:SourceAccount' }]; + + // These are all the supported conditions. Some combinations are not supported, + // like only 'aws:SourceArn' or 'aws:PrincipalOrgID' and 'aws:SourceAccount'. + // These will be validated through `this.validateConditionCombinations`. + const supportedPrincipalConditions = [{ + operator: 'ArnLike', + key: 'aws:SourceArn', + }, + { + operator: 'StringEquals', + key: 'aws:SourceAccount', + }, { + operator: 'StringEquals', + key: 'aws:PrincipalOrgID', + }]; const unsupportedConditions = conditionPairs.filter( (condition) => !supportedPrincipalConditions.some( @@ -599,17 +662,14 @@ export abstract class FunctionBase extends Resource implements IFunction, ec2.IC ); if (unsupportedConditions.length == 0) { - return { - sourceAccount: conditions.StringEquals['aws:SourceAccount'], - sourceArn: conditions.ArnLike['aws:SourceArn'], - }; + return conditions; } else { throw new Error(`PrincipalWithConditions had unsupported conditions for Lambda permission statement: ${JSON.stringify(unsupportedConditions)}. ` + `Supported operator/condition pairs: ${JSON.stringify(supportedPrincipalConditions)}`); } - } else { - return null; } + + return undefined; } private isPrincipalWithConditions(principal: iam.IPrincipal): principal is iam.PrincipalWithConditions { diff --git a/packages/@aws-cdk/aws-lambda/test/function.test.ts b/packages/@aws-cdk/aws-lambda/test/function.test.ts index 9cadfd03afab9..535cb411f1125 100644 --- a/packages/@aws-cdk/aws-lambda/test/function.test.ts +++ b/packages/@aws-cdk/aws-lambda/test/function.test.ts @@ -181,16 +181,42 @@ describe('function', () => { }); }); - test('fails if the principal is not a service, account or arn principal', () => { + test('can supply principalOrgID via permission property', () => { const stack = new cdk.Stack(); const fn = newTestLambda(stack); + const org = new iam.OrganizationPrincipal('o-xxxxxxxxxx'); + const account = new iam.AccountPrincipal('123456789012'); + + fn.addPermission('S3Permission', { + action: 'lambda:*', + principal: account, + principalOrg: org, + }); - expect(() => fn.addPermission('F1', { principal: new iam.OrganizationPrincipal('org') })) + Template.fromStack(stack).hasResourceProperties('AWS::Lambda::Permission', { + Action: 'lambda:*', + FunctionName: { + 'Fn::GetAtt': [ + 'MyLambdaCCE802FB', + 'Arn', + ], + }, + Principal: account.accountId, + PrincipalOrgID: org.organizationId, + }); + }); + + test('fails if the principal is not a service, account, arn, or organization principal', () => { + const stack = new cdk.Stack(); + const fn = newTestLambda(stack); + + expect(() => fn.addPermission('F1', { principal: new iam.CanonicalUserPrincipal('org') })) .toThrow(/Invalid principal type for Lambda permission statement/); fn.addPermission('S1', { principal: new iam.ServicePrincipal('my-service') }); fn.addPermission('S2', { principal: new iam.AccountPrincipal('account') }); fn.addPermission('S3', { principal: new iam.ArnPrincipal('my:arn') }); + fn.addPermission('S4', { principal: new iam.OrganizationPrincipal('my:org') }); }); test('applies source account/ARN conditions if the principal has conditions', () => { @@ -224,6 +250,32 @@ describe('function', () => { }); }); + test('applies principal org id conditions if the principal has conditions', () => { + const stack = new cdk.Stack(); + const fn = newTestLambda(stack); + const principalOrgId = 'org-xxxxxxxxxx'; + const service = 'my-service'; + const principal = new iam.PrincipalWithConditions(new iam.ServicePrincipal(service), { + StringEquals: { + 'aws:PrincipalOrgID': principalOrgId, + }, + }); + + fn.addPermission('S1', { principal: principal }); + + Template.fromStack(stack).hasResourceProperties('AWS::Lambda::Permission', { + Action: 'lambda:InvokeFunction', + FunctionName: { + 'Fn::GetAtt': [ + 'MyLambdaCCE802FB', + 'Arn', + ], + }, + Principal: service, + PrincipalOrgID: principalOrgId, + }); + }); + test('fails if the principal has conditions that are not supported', () => { const stack = new cdk.Stack(); const fn = newTestLambda(stack); @@ -251,6 +303,31 @@ describe('function', () => { })).toThrow(/PrincipalWithConditions had unsupported conditions for Lambda permission statement/); }); + test('fails if the principal has condition combinations that are not supported', () => { + const stack = new cdk.Stack(); + const fn = newTestLambda(stack); + + expect(() => fn.addPermission('F1', { + principal: new iam.PrincipalWithConditions(new iam.ServicePrincipal('my-service'), { + StringEquals: { + 'aws:SourceAccount': 'source-account', + }, + }), + })).toThrow(/PrincipalWithConditions had unsupported condition combinations for Lambda permission statement/); + + expect(() => fn.addPermission('F2', { + principal: new iam.PrincipalWithConditions(new iam.ServicePrincipal('my-service'), { + StringEquals: { + 'aws:SourceAccount': 'source-account', + 'aws:PrincipalOrgID': 'principal-org-id', + }, + ArnLike: { + 'aws:SourceArn': 'source-arn', + }, + }), + })).toThrow(/PrincipalWithConditions had unsupported condition combinations for Lambda permission statement/); + }); + test('BYORole', () => { // GIVEN const stack = new cdk.Stack(); @@ -1220,6 +1297,33 @@ describe('function', () => { }); }); + test('with an organization principal', () => { + // GIVEN + const stack = new cdk.Stack(); + const fn = new lambda.Function(stack, 'Function', { + code: lambda.Code.fromInline('xxx'), + handler: 'index.handler', + runtime: lambda.Runtime.NODEJS_14_X, + }); + const org = new iam.OrganizationPrincipal('my-org-id'); + + // WHEN + fn.grantInvoke(org); + + // THEN + Template.fromStack(stack).hasResourceProperties('AWS::Lambda::Permission', { + Action: 'lambda:InvokeFunction', + FunctionName: { + 'Fn::GetAtt': [ + 'Function76856677', + 'Arn', + ], + }, + Principal: '*', + PrincipalOrgID: 'my-org-id', + }); + }); + test('can be called twice for the same service principal', () => { // GIVEN const stack = new cdk.Stack(); From 3b4311f07d5d9c617d9031691fa926f9bcd885c0 Mon Sep 17 00:00:00 2001 From: kaizen3031593 Date: Tue, 19 Apr 2022 14:02:03 -0400 Subject: [PATCH 05/14] add integ test --- .../aws-lambda/test/integ.permissions.ts | 30 +++ .../test/permissions.integ.snapshot/cdk.out | 1 + .../lambda-permissions.template.json | 96 ++++++++++ .../permissions.integ.snapshot/manifest.json | 52 +++++ .../test/permissions.integ.snapshot/tree.json | 180 ++++++++++++++++++ 5 files changed, 359 insertions(+) create mode 100644 packages/@aws-cdk/aws-lambda/test/integ.permissions.ts create mode 100644 packages/@aws-cdk/aws-lambda/test/permissions.integ.snapshot/cdk.out create mode 100644 packages/@aws-cdk/aws-lambda/test/permissions.integ.snapshot/lambda-permissions.template.json create mode 100644 packages/@aws-cdk/aws-lambda/test/permissions.integ.snapshot/manifest.json create mode 100644 packages/@aws-cdk/aws-lambda/test/permissions.integ.snapshot/tree.json diff --git a/packages/@aws-cdk/aws-lambda/test/integ.permissions.ts b/packages/@aws-cdk/aws-lambda/test/integ.permissions.ts new file mode 100644 index 0000000000000..361e40f1a0d26 --- /dev/null +++ b/packages/@aws-cdk/aws-lambda/test/integ.permissions.ts @@ -0,0 +1,30 @@ +import * as iam from '@aws-cdk/aws-iam'; +import * as cdk from '@aws-cdk/core'; +import * as lambda from '../lib'; + +const app = new cdk.App(); + +const stack = new cdk.Stack(app, 'lambda-permissions'); + +const fn = new lambda.Function(stack, 'MyLambda', { + code: new lambda.InlineCode('foo'), + handler: 'index.handler', + runtime: lambda.Runtime.NODEJS_14_X, +}); + +// 3 different ways to configure the same permission +fn.addPermission('org', { + principal: new iam.OrganizationPrincipal('o-xxxxxxxxxx'), +}); + +fn.addPermission('other-org', { + principal: new iam.StarPrincipal(), + principalOrg: new iam.OrganizationPrincipal('o-zzzzzzzzzz'), +}); + +fn.grantInvoke(new iam.AnyPrincipal().withConditions({ + StringEquals: { + 'aws:PrincipalOrgID': 'o-yyyyyyyyyy', + }, +})); + diff --git a/packages/@aws-cdk/aws-lambda/test/permissions.integ.snapshot/cdk.out b/packages/@aws-cdk/aws-lambda/test/permissions.integ.snapshot/cdk.out new file mode 100644 index 0000000000000..90bef2e09ad39 --- /dev/null +++ b/packages/@aws-cdk/aws-lambda/test/permissions.integ.snapshot/cdk.out @@ -0,0 +1 @@ +{"version":"17.0.0"} \ No newline at end of file diff --git a/packages/@aws-cdk/aws-lambda/test/permissions.integ.snapshot/lambda-permissions.template.json b/packages/@aws-cdk/aws-lambda/test/permissions.integ.snapshot/lambda-permissions.template.json new file mode 100644 index 0000000000000..9634c6e2a19a2 --- /dev/null +++ b/packages/@aws-cdk/aws-lambda/test/permissions.integ.snapshot/lambda-permissions.template.json @@ -0,0 +1,96 @@ +{ + "Resources": { + "MyLambdaServiceRole4539ECB6": { + "Type": "AWS::IAM::Role", + "Properties": { + "AssumeRolePolicyDocument": { + "Statement": [ + { + "Action": "sts:AssumeRole", + "Effect": "Allow", + "Principal": { + "Service": "lambda.amazonaws.com" + } + } + ], + "Version": "2012-10-17" + }, + "ManagedPolicyArns": [ + { + "Fn::Join": [ + "", + [ + "arn:", + { + "Ref": "AWS::Partition" + }, + ":iam::aws:policy/service-role/AWSLambdaBasicExecutionRole" + ] + ] + } + ] + } + }, + "MyLambdaCCE802FB": { + "Type": "AWS::Lambda::Function", + "Properties": { + "Code": { + "ZipFile": "foo" + }, + "Role": { + "Fn::GetAtt": [ + "MyLambdaServiceRole4539ECB6", + "Arn" + ] + }, + "Handler": "index.handler", + "Runtime": "nodejs14.x" + }, + "DependsOn": [ + "MyLambdaServiceRole4539ECB6" + ] + }, + "MyLambdaorgF85788CF": { + "Type": "AWS::Lambda::Permission", + "Properties": { + "Action": "lambda:InvokeFunction", + "FunctionName": { + "Fn::GetAtt": [ + "MyLambdaCCE802FB", + "Arn" + ] + }, + "Principal": "*", + "PrincipalOrgID": "o-xxxxxxxxxx" + } + }, + "MyLambdaotherorg9E5CFB3F": { + "Type": "AWS::Lambda::Permission", + "Properties": { + "Action": "lambda:InvokeFunction", + "FunctionName": { + "Fn::GetAtt": [ + "MyLambdaCCE802FB", + "Arn" + ] + }, + "Principal": "*", + "PrincipalOrgID": "o-zzzzzzzzzz" + } + }, + "MyLambdaInvokeAnyPrincipal256ECDE1": { + "Type": "AWS::Lambda::Permission", + "Properties": { + "Action": "lambda:InvokeFunction", + "FunctionName": { + "Fn::GetAtt": [ + "MyLambdaCCE802FB", + "Arn" + ] + }, + "Principal": "*", + "PrincipalOrgID": "o-yyyyyyyyyy" + } + } + } +} \ No newline at end of file diff --git a/packages/@aws-cdk/aws-lambda/test/permissions.integ.snapshot/manifest.json b/packages/@aws-cdk/aws-lambda/test/permissions.integ.snapshot/manifest.json new file mode 100644 index 0000000000000..154b59d6368bb --- /dev/null +++ b/packages/@aws-cdk/aws-lambda/test/permissions.integ.snapshot/manifest.json @@ -0,0 +1,52 @@ +{ + "version": "17.0.0", + "artifacts": { + "Tree": { + "type": "cdk:tree", + "properties": { + "file": "tree.json" + } + }, + "lambda-permissions": { + "type": "aws:cloudformation:stack", + "environment": "aws://unknown-account/unknown-region", + "properties": { + "templateFile": "lambda-permissions.template.json", + "validateOnSynth": false + }, + "metadata": { + "/lambda-permissions/MyLambda/ServiceRole/Resource": [ + { + "type": "aws:cdk:logicalId", + "data": "MyLambdaServiceRole4539ECB6" + } + ], + "/lambda-permissions/MyLambda/Resource": [ + { + "type": "aws:cdk:logicalId", + "data": "MyLambdaCCE802FB" + } + ], + "/lambda-permissions/MyLambda/org": [ + { + "type": "aws:cdk:logicalId", + "data": "MyLambdaorgF85788CF" + } + ], + "/lambda-permissions/MyLambda/other-org": [ + { + "type": "aws:cdk:logicalId", + "data": "MyLambdaotherorg9E5CFB3F" + } + ], + "/lambda-permissions/MyLambda/InvokeAnyPrincipal()": [ + { + "type": "aws:cdk:logicalId", + "data": "MyLambdaInvokeAnyPrincipal256ECDE1" + } + ] + }, + "displayName": "lambda-permissions" + } + } +} \ No newline at end of file diff --git a/packages/@aws-cdk/aws-lambda/test/permissions.integ.snapshot/tree.json b/packages/@aws-cdk/aws-lambda/test/permissions.integ.snapshot/tree.json new file mode 100644 index 0000000000000..4e2de168ef500 --- /dev/null +++ b/packages/@aws-cdk/aws-lambda/test/permissions.integ.snapshot/tree.json @@ -0,0 +1,180 @@ +{ + "version": "tree-0.1", + "tree": { + "id": "App", + "path": "", + "children": { + "Tree": { + "id": "Tree", + "path": "Tree", + "constructInfo": { + "fqn": "@aws-cdk/core.Construct", + "version": "0.0.0" + } + }, + "lambda-permissions": { + "id": "lambda-permissions", + "path": "lambda-permissions", + "children": { + "MyLambda": { + "id": "MyLambda", + "path": "lambda-permissions/MyLambda", + "children": { + "ServiceRole": { + "id": "ServiceRole", + "path": "lambda-permissions/MyLambda/ServiceRole", + "children": { + "Resource": { + "id": "Resource", + "path": "lambda-permissions/MyLambda/ServiceRole/Resource", + "attributes": { + "aws:cdk:cloudformation:type": "AWS::IAM::Role", + "aws:cdk:cloudformation:props": { + "assumeRolePolicyDocument": { + "Statement": [ + { + "Action": "sts:AssumeRole", + "Effect": "Allow", + "Principal": { + "Service": "lambda.amazonaws.com" + } + } + ], + "Version": "2012-10-17" + }, + "managedPolicyArns": [ + { + "Fn::Join": [ + "", + [ + "arn:", + { + "Ref": "AWS::Partition" + }, + ":iam::aws:policy/service-role/AWSLambdaBasicExecutionRole" + ] + ] + } + ] + } + }, + "constructInfo": { + "fqn": "@aws-cdk/aws-iam.CfnRole", + "version": "0.0.0" + } + } + }, + "constructInfo": { + "fqn": "@aws-cdk/aws-iam.Role", + "version": "0.0.0" + } + }, + "Resource": { + "id": "Resource", + "path": "lambda-permissions/MyLambda/Resource", + "attributes": { + "aws:cdk:cloudformation:type": "AWS::Lambda::Function", + "aws:cdk:cloudformation:props": { + "code": { + "zipFile": "foo" + }, + "role": { + "Fn::GetAtt": [ + "MyLambdaServiceRole4539ECB6", + "Arn" + ] + }, + "handler": "index.handler", + "runtime": "nodejs14.x" + } + }, + "constructInfo": { + "fqn": "@aws-cdk/aws-lambda.CfnFunction", + "version": "0.0.0" + } + }, + "org": { + "id": "org", + "path": "lambda-permissions/MyLambda/org", + "attributes": { + "aws:cdk:cloudformation:type": "AWS::Lambda::Permission", + "aws:cdk:cloudformation:props": { + "action": "lambda:InvokeFunction", + "functionName": { + "Fn::GetAtt": [ + "MyLambdaCCE802FB", + "Arn" + ] + }, + "principal": "*", + "principalOrgId": "o-xxxxxxxxxx" + } + }, + "constructInfo": { + "fqn": "@aws-cdk/aws-lambda.CfnPermission", + "version": "0.0.0" + } + }, + "other-org": { + "id": "other-org", + "path": "lambda-permissions/MyLambda/other-org", + "attributes": { + "aws:cdk:cloudformation:type": "AWS::Lambda::Permission", + "aws:cdk:cloudformation:props": { + "action": "lambda:InvokeFunction", + "functionName": { + "Fn::GetAtt": [ + "MyLambdaCCE802FB", + "Arn" + ] + }, + "principal": "*", + "principalOrgId": "o-zzzzzzzzzz" + } + }, + "constructInfo": { + "fqn": "@aws-cdk/aws-lambda.CfnPermission", + "version": "0.0.0" + } + }, + "InvokeAnyPrincipal()": { + "id": "InvokeAnyPrincipal()", + "path": "lambda-permissions/MyLambda/InvokeAnyPrincipal()", + "attributes": { + "aws:cdk:cloudformation:type": "AWS::Lambda::Permission", + "aws:cdk:cloudformation:props": { + "action": "lambda:InvokeFunction", + "functionName": { + "Fn::GetAtt": [ + "MyLambdaCCE802FB", + "Arn" + ] + }, + "principal": "*", + "principalOrgId": "o-yyyyyyyyyy" + } + }, + "constructInfo": { + "fqn": "@aws-cdk/aws-lambda.CfnPermission", + "version": "0.0.0" + } + } + }, + "constructInfo": { + "fqn": "@aws-cdk/aws-lambda.Function", + "version": "0.0.0" + } + } + }, + "constructInfo": { + "fqn": "@aws-cdk/core.Stack", + "version": "0.0.0" + } + } + }, + "constructInfo": { + "fqn": "@aws-cdk/core.App", + "version": "0.0.0" + } + } +} \ No newline at end of file From 545dc73e480f96c70a6226827990a914ba0d5fdd Mon Sep 17 00:00:00 2001 From: kaizen3031593 Date: Wed, 20 Apr 2022 10:46:28 -0400 Subject: [PATCH 06/14] fix snippets --- packages/@aws-cdk/aws-lambda/README.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/@aws-cdk/aws-lambda/README.md b/packages/@aws-cdk/aws-lambda/README.md index b217a380fe3c2..bfa2e5aef13c0 100644 --- a/packages/@aws-cdk/aws-lambda/README.md +++ b/packages/@aws-cdk/aws-lambda/README.md @@ -197,7 +197,7 @@ fn.addPermission('grant to entire org', { // Equivalent to: fn.addPermission('grant to entire org', { - principal: iam.StarPrincipal(), + principal: new iam.StarPrincipal(), principalOrg: orgPrincipal, }) ``` @@ -213,8 +213,8 @@ AWS account or role as the `principal`: declare const fn: lambda.Function; fn.addPermission('grant to account in org', { - principal: iam.AccountPrincipal('123456789012'); - principalOrg: iam.OrganizationPrincipal('o-xxxxxxxxxx'), + principal: new iam.AccountPrincipal('123456789012'), + principalOrg: new iam.OrganizationPrincipal('o-xxxxxxxxxx'), }); ``` @@ -227,7 +227,7 @@ in the AWS Lambda Developer Guide. ```ts // Grant permission to other AWS account declare const fn: lambda.Function; -const account = iam.AccountPrincipal('123456789012'); +const account = new iam.AccountPrincipal('123456789012'); fn.grantInvoke(account); From d4c2904a0ed3733163e3752f8c12f34e520c855a Mon Sep 17 00:00:00 2001 From: kaizen3031593 Date: Wed, 27 Apr 2022 17:46:31 -0400 Subject: [PATCH 07/14] update readme to reflect potential convenience method in iam --- packages/@aws-cdk/aws-lambda/README.md | 33 ++++---------------------- 1 file changed, 4 insertions(+), 29 deletions(-) diff --git a/packages/@aws-cdk/aws-lambda/README.md b/packages/@aws-cdk/aws-lambda/README.md index bfa2e5aef13c0..21f9e55912532 100644 --- a/packages/@aws-cdk/aws-lambda/README.md +++ b/packages/@aws-cdk/aws-lambda/README.md @@ -186,20 +186,9 @@ in the AWS Lambda Developer Guide. ```ts // Grant permissions to an entire AWS organization declare const fn: lambda.Function; -const orgPrincipal = new iam.OrganizationPrincipal('o-xxxxxxxxxx'); +const org = new iam.OrganizationPrincipal('o-xxxxxxxxxx'); -fn.grantInvoke(orgPrincipal); - -// Equivalent to: -fn.addPermission('grant to entire org', { - principal: orgPrincipal, -}); - -// Equivalent to: -fn.addPermission('grant to entire org', { - principal: new iam.StarPrincipal(), - principalOrg: orgPrincipal, -}) +fn.grantInvoke(org); ``` In the above example, the `principal` will be `*` and all users in the @@ -211,11 +200,9 @@ AWS account or role as the `principal`: ```ts // Grant permission to an account ONLY IF they are part of the organization declare const fn: lambda.Function; +const account = new iam.AccountPrincipal('123456789012'); -fn.addPermission('grant to account in org', { - principal: new iam.AccountPrincipal('123456789012'), - principalOrg: new iam.OrganizationPrincipal('o-xxxxxxxxxx'), -}); +fn.grantInvoke(account.inOrganization('o-xxxxxxxxxx')); ``` For more information, see @@ -230,11 +217,6 @@ declare const fn: lambda.Function; const account = new iam.AccountPrincipal('123456789012'); fn.grantInvoke(account); - -// Equivalent to: -fn.addPermission('grant to account in org', { - principal: account, -}); ``` For more information, see @@ -265,13 +247,6 @@ const servicePrincipalWithConditions = servicePrincipal.withConditions({ }); fn.grantInvoke(servicePrincipalWithConditions); - -// Equivalent to: -fn.addPermission('my-service Invocation', { - principal: servicePrincipal, - sourceArn: sourceArn, - sourceAccount: sourceAccount, -}); ``` ## Versions From 60022d71b23115a365fcc92ecb222ebd1a43d737 Mon Sep 17 00:00:00 2001 From: kaizen3031593 Date: Wed, 27 Apr 2022 18:00:35 -0400 Subject: [PATCH 08/14] remove instanceOf in favor of parsing the conditions --- packages/@aws-cdk/aws-lambda/lib/function-base.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/@aws-cdk/aws-lambda/lib/function-base.ts b/packages/@aws-cdk/aws-lambda/lib/function-base.ts index 21a677ba3b15e..0d6df7d9b91f2 100644 --- a/packages/@aws-cdk/aws-lambda/lib/function-base.ts +++ b/packages/@aws-cdk/aws-lambda/lib/function-base.ts @@ -352,8 +352,8 @@ export abstract class FunctionBase extends Resource implements IFunction, ec2.IC // Lambda does not actually accept an organization principal here. // so the principal becomes '*' and the organization ID will be sent to // the 'principalOrgID' property. - if (permission.principal instanceof iam.OrganizationPrincipal) { - principalOrgID = permission.principal.organizationId; + if (permission.principal.toString().startsWith('OrganizationPrincipal')) { + principalOrgID = permission.principal.policyFragment.conditions.StringEquals['aws:PrincipalOrgID']; } const action = permission.action ?? 'lambda:InvokeFunction'; From b6769bbf714d158c3fe0cb2519a0b9dae35626dc Mon Sep 17 00:00:00 2001 From: kaizen3031593 Date: Thu, 28 Apr 2022 14:00:35 -0400 Subject: [PATCH 09/14] some changes from pr feedback --- .../@aws-cdk/aws-lambda/lib/function-base.ts | 7 +------ .../aws-lambda/test/integ.permissions.ts | 17 ++--------------- 2 files changed, 3 insertions(+), 21 deletions(-) diff --git a/packages/@aws-cdk/aws-lambda/lib/function-base.ts b/packages/@aws-cdk/aws-lambda/lib/function-base.ts index 0d6df7d9b91f2..a3547b664491d 100644 --- a/packages/@aws-cdk/aws-lambda/lib/function-base.ts +++ b/packages/@aws-cdk/aws-lambda/lib/function-base.ts @@ -558,7 +558,6 @@ export abstract class FunctionBase extends Resource implements IFunction, ec2.IC private parsePermissionPrincipal(principal: iam.IPrincipal) { // Try some specific common classes first. // use duck-typing, not instance of - // @deprecated: after v2, we can change these to 'instanceof' if ('wrapped' in principal) { // eslint-disable-next-line dot-notation principal = principal['wrapped']; @@ -576,15 +575,11 @@ export abstract class FunctionBase extends Resource implements IFunction, ec2.IC return (principal as iam.ArnPrincipal).arn; } - if (principal instanceof iam.OrganizationPrincipal) { + if ('organizationId' in principal) { // we will move the organization id to the `principalOrgId` property of `Permissions`. return '*'; } - if (principal instanceof iam.StarPrincipal) { - return '*'; - } - // Try a best-effort approach to support simple principals that are not any of the predefined // classes, but are simple enough that they will fit into the Permission model. Main target // here: imported Roles, Users, Groups. diff --git a/packages/@aws-cdk/aws-lambda/test/integ.permissions.ts b/packages/@aws-cdk/aws-lambda/test/integ.permissions.ts index 361e40f1a0d26..414112e84232b 100644 --- a/packages/@aws-cdk/aws-lambda/test/integ.permissions.ts +++ b/packages/@aws-cdk/aws-lambda/test/integ.permissions.ts @@ -12,19 +12,6 @@ const fn = new lambda.Function(stack, 'MyLambda', { runtime: lambda.Runtime.NODEJS_14_X, }); -// 3 different ways to configure the same permission -fn.addPermission('org', { - principal: new iam.OrganizationPrincipal('o-xxxxxxxxxx'), -}); - -fn.addPermission('other-org', { - principal: new iam.StarPrincipal(), - principalOrg: new iam.OrganizationPrincipal('o-zzzzzzzzzz'), -}); - -fn.grantInvoke(new iam.AnyPrincipal().withConditions({ - StringEquals: { - 'aws:PrincipalOrgID': 'o-yyyyyyyyyy', - }, -})); +fn.grantInvoke(new iam.AnyPrincipal()); +fn.grantInvoke(new iam.OrganizationPrincipal('o-xxxxxxxxxx')); From c26d3adc4dc736055843dd8fbdbb238d3c5222d5 Mon Sep 17 00:00:00 2001 From: kaizen3031593 Date: Thu, 28 Apr 2022 14:57:34 -0400 Subject: [PATCH 10/14] update integ test and fix bug in isPrincipalWithConditions --- .../@aws-cdk/aws-lambda/lib/function-base.ts | 9 +---- .../aws-lambda/test/integ.permissions.ts | 2 +- .../test/permissions.integ.snapshot/cdk.out | 2 +- .../permissions.integ.snapshot/integ.json | 14 +++++++ .../lambda-permissions.template.json | 22 ++--------- .../permissions.integ.snapshot/manifest.json | 26 +++++++++---- .../test/permissions.integ.snapshot/tree.json | 38 ++++--------------- 7 files changed, 48 insertions(+), 65 deletions(-) create mode 100644 packages/@aws-cdk/aws-lambda/test/permissions.integ.snapshot/integ.json diff --git a/packages/@aws-cdk/aws-lambda/lib/function-base.ts b/packages/@aws-cdk/aws-lambda/lib/function-base.ts index a3547b664491d..15eafa29d26dd 100644 --- a/packages/@aws-cdk/aws-lambda/lib/function-base.ts +++ b/packages/@aws-cdk/aws-lambda/lib/function-base.ts @@ -349,13 +349,6 @@ export abstract class FunctionBase extends Resource implements IFunction, ec2.IC let { sourceArn, sourceAccount, principalOrgID } = this.validateConditionCombinations(permission.principal) ?? {}; - // Lambda does not actually accept an organization principal here. - // so the principal becomes '*' and the organization ID will be sent to - // the 'principalOrgID' property. - if (permission.principal.toString().startsWith('OrganizationPrincipal')) { - principalOrgID = permission.principal.policyFragment.conditions.StringEquals['aws:PrincipalOrgID']; - } - const action = permission.action ?? 'lambda:InvokeFunction'; const scope = permission.scope ?? this; @@ -668,7 +661,7 @@ export abstract class FunctionBase extends Resource implements IFunction, ec2.IC } private isPrincipalWithConditions(principal: iam.IPrincipal): principal is iam.PrincipalWithConditions { - return 'conditions' in principal; + return Object.keys(principal.policyFragment.conditions).length > 0; } } diff --git a/packages/@aws-cdk/aws-lambda/test/integ.permissions.ts b/packages/@aws-cdk/aws-lambda/test/integ.permissions.ts index 414112e84232b..92a9964dd76d2 100644 --- a/packages/@aws-cdk/aws-lambda/test/integ.permissions.ts +++ b/packages/@aws-cdk/aws-lambda/test/integ.permissions.ts @@ -12,6 +12,6 @@ const fn = new lambda.Function(stack, 'MyLambda', { runtime: lambda.Runtime.NODEJS_14_X, }); -fn.grantInvoke(new iam.AnyPrincipal()); +fn.grantInvoke(new iam.AnyPrincipal().inOrganization('o-yyyyyyyyyy')); fn.grantInvoke(new iam.OrganizationPrincipal('o-xxxxxxxxxx')); diff --git a/packages/@aws-cdk/aws-lambda/test/permissions.integ.snapshot/cdk.out b/packages/@aws-cdk/aws-lambda/test/permissions.integ.snapshot/cdk.out index 90bef2e09ad39..2efc89439fab8 100644 --- a/packages/@aws-cdk/aws-lambda/test/permissions.integ.snapshot/cdk.out +++ b/packages/@aws-cdk/aws-lambda/test/permissions.integ.snapshot/cdk.out @@ -1 +1 @@ -{"version":"17.0.0"} \ No newline at end of file +{"version":"18.0.0"} \ No newline at end of file diff --git a/packages/@aws-cdk/aws-lambda/test/permissions.integ.snapshot/integ.json b/packages/@aws-cdk/aws-lambda/test/permissions.integ.snapshot/integ.json new file mode 100644 index 0000000000000..6c62ccc64f4ba --- /dev/null +++ b/packages/@aws-cdk/aws-lambda/test/permissions.integ.snapshot/integ.json @@ -0,0 +1,14 @@ +{ + "version": "18.0.0", + "testCases": { + "integ.permissions": { + "stacks": [ + "lambda-permissions" + ], + "diffAssets": false, + "stackUpdateWorkflow": true + } + }, + "synthContext": {}, + "enableLookups": false +} \ No newline at end of file diff --git a/packages/@aws-cdk/aws-lambda/test/permissions.integ.snapshot/lambda-permissions.template.json b/packages/@aws-cdk/aws-lambda/test/permissions.integ.snapshot/lambda-permissions.template.json index 9634c6e2a19a2..53e848c5ba649 100644 --- a/packages/@aws-cdk/aws-lambda/test/permissions.integ.snapshot/lambda-permissions.template.json +++ b/packages/@aws-cdk/aws-lambda/test/permissions.integ.snapshot/lambda-permissions.template.json @@ -50,21 +50,7 @@ "MyLambdaServiceRole4539ECB6" ] }, - "MyLambdaorgF85788CF": { - "Type": "AWS::Lambda::Permission", - "Properties": { - "Action": "lambda:InvokeFunction", - "FunctionName": { - "Fn::GetAtt": [ - "MyLambdaCCE802FB", - "Arn" - ] - }, - "Principal": "*", - "PrincipalOrgID": "o-xxxxxxxxxx" - } - }, - "MyLambdaotherorg9E5CFB3F": { + "MyLambdaInvokeAnyPrincipal256ECDE1": { "Type": "AWS::Lambda::Permission", "Properties": { "Action": "lambda:InvokeFunction", @@ -75,10 +61,10 @@ ] }, "Principal": "*", - "PrincipalOrgID": "o-zzzzzzzzzz" + "PrincipalOrgID": "o-yyyyyyyyyy" } }, - "MyLambdaInvokeAnyPrincipal256ECDE1": { + "MyLambdaInvokeOrganizationPrincipaloxxxxxxxxxxEB282AD1": { "Type": "AWS::Lambda::Permission", "Properties": { "Action": "lambda:InvokeFunction", @@ -89,7 +75,7 @@ ] }, "Principal": "*", - "PrincipalOrgID": "o-yyyyyyyyyy" + "PrincipalOrgID": "o-xxxxxxxxxx" } } } diff --git a/packages/@aws-cdk/aws-lambda/test/permissions.integ.snapshot/manifest.json b/packages/@aws-cdk/aws-lambda/test/permissions.integ.snapshot/manifest.json index 154b59d6368bb..07f1891ce4f22 100644 --- a/packages/@aws-cdk/aws-lambda/test/permissions.integ.snapshot/manifest.json +++ b/packages/@aws-cdk/aws-lambda/test/permissions.integ.snapshot/manifest.json @@ -1,5 +1,5 @@ { - "version": "17.0.0", + "version": "18.0.0", "artifacts": { "Tree": { "type": "cdk:tree", @@ -27,22 +27,34 @@ "data": "MyLambdaCCE802FB" } ], - "/lambda-permissions/MyLambda/org": [ + "/lambda-permissions/MyLambda/InvokeAnyPrincipal()": [ { "type": "aws:cdk:logicalId", - "data": "MyLambdaorgF85788CF" + "data": "MyLambdaInvokeAnyPrincipal256ECDE1" } ], - "/lambda-permissions/MyLambda/other-org": [ + "/lambda-permissions/MyLambda/InvokeOrganizationPrincipal(o-xxxxxxxxxx)": [ { "type": "aws:cdk:logicalId", - "data": "MyLambdaotherorg9E5CFB3F" + "data": "MyLambdaInvokeOrganizationPrincipaloxxxxxxxxxxEB282AD1" } ], - "/lambda-permissions/MyLambda/InvokeAnyPrincipal()": [ + "MyLambdaorgF85788CF": [ { "type": "aws:cdk:logicalId", - "data": "MyLambdaInvokeAnyPrincipal256ECDE1" + "data": "MyLambdaorgF85788CF", + "trace": [ + "!!DESTRUCTIVE_CHANGES: WILL_DESTROY" + ] + } + ], + "MyLambdaotherorg9E5CFB3F": [ + { + "type": "aws:cdk:logicalId", + "data": "MyLambdaotherorg9E5CFB3F", + "trace": [ + "!!DESTRUCTIVE_CHANGES: WILL_DESTROY" + ] } ] }, diff --git a/packages/@aws-cdk/aws-lambda/test/permissions.integ.snapshot/tree.json b/packages/@aws-cdk/aws-lambda/test/permissions.integ.snapshot/tree.json index 4e2de168ef500..b25b654f1d68a 100644 --- a/packages/@aws-cdk/aws-lambda/test/permissions.integ.snapshot/tree.json +++ b/packages/@aws-cdk/aws-lambda/test/permissions.integ.snapshot/tree.json @@ -93,31 +93,9 @@ "version": "0.0.0" } }, - "org": { - "id": "org", - "path": "lambda-permissions/MyLambda/org", - "attributes": { - "aws:cdk:cloudformation:type": "AWS::Lambda::Permission", - "aws:cdk:cloudformation:props": { - "action": "lambda:InvokeFunction", - "functionName": { - "Fn::GetAtt": [ - "MyLambdaCCE802FB", - "Arn" - ] - }, - "principal": "*", - "principalOrgId": "o-xxxxxxxxxx" - } - }, - "constructInfo": { - "fqn": "@aws-cdk/aws-lambda.CfnPermission", - "version": "0.0.0" - } - }, - "other-org": { - "id": "other-org", - "path": "lambda-permissions/MyLambda/other-org", + "InvokeAnyPrincipal()": { + "id": "InvokeAnyPrincipal()", + "path": "lambda-permissions/MyLambda/InvokeAnyPrincipal()", "attributes": { "aws:cdk:cloudformation:type": "AWS::Lambda::Permission", "aws:cdk:cloudformation:props": { @@ -129,7 +107,7 @@ ] }, "principal": "*", - "principalOrgId": "o-zzzzzzzzzz" + "principalOrgId": "o-yyyyyyyyyy" } }, "constructInfo": { @@ -137,9 +115,9 @@ "version": "0.0.0" } }, - "InvokeAnyPrincipal()": { - "id": "InvokeAnyPrincipal()", - "path": "lambda-permissions/MyLambda/InvokeAnyPrincipal()", + "InvokeOrganizationPrincipal(o-xxxxxxxxxx)": { + "id": "InvokeOrganizationPrincipal(o-xxxxxxxxxx)", + "path": "lambda-permissions/MyLambda/InvokeOrganizationPrincipal(o-xxxxxxxxxx)", "attributes": { "aws:cdk:cloudformation:type": "AWS::Lambda::Permission", "aws:cdk:cloudformation:props": { @@ -151,7 +129,7 @@ ] }, "principal": "*", - "principalOrgId": "o-yyyyyyyyyy" + "principalOrgId": "o-xxxxxxxxxx" } }, "constructInfo": { From 2f641961f32a2b44cd19cf7a7f1a2fb142330220 Mon Sep 17 00:00:00 2001 From: kaizen3031593 Date: Fri, 29 Apr 2022 13:48:46 -0400 Subject: [PATCH 11/14] fix(lambda): grant function permissions does not accept sourceArn condition only --- .../@aws-cdk/aws-lambda/lib/function-base.ts | 5 --- .../@aws-cdk/aws-lambda/test/function.test.ts | 36 ++++++++++++++----- 2 files changed, 27 insertions(+), 14 deletions(-) diff --git a/packages/@aws-cdk/aws-lambda/lib/function-base.ts b/packages/@aws-cdk/aws-lambda/lib/function-base.ts index 15eafa29d26dd..009f292df18a3 100644 --- a/packages/@aws-cdk/aws-lambda/lib/function-base.ts +++ b/packages/@aws-cdk/aws-lambda/lib/function-base.ts @@ -608,11 +608,6 @@ export abstract class FunctionBase extends Resource implements IFunction, ec2.IC throw new Error('PrincipalWithConditions had unsupported condition combinations for Lambda permission statement: principalOrgID cannot be set with other conditions.'); } - // SourceArn and SourceAccount must be set together - if ((sourceArn && !sourceAccount) || (!sourceArn && sourceAccount)) { - throw new Error('PrincipalWithConditions had unsupported condition combinations for Lambda permission statement: you cannot set sourceAccount without sourceArn and vice versa'); - } - return { sourceArn, sourceAccount, diff --git a/packages/@aws-cdk/aws-lambda/test/function.test.ts b/packages/@aws-cdk/aws-lambda/test/function.test.ts index 9223c62a326d2..6f3a14387a24b 100644 --- a/packages/@aws-cdk/aws-lambda/test/function.test.ts +++ b/packages/@aws-cdk/aws-lambda/test/function.test.ts @@ -116,7 +116,7 @@ describe('function', () => { })).toThrow(); }); - describe('addToResourcePolicy', () => { + describe('addPermissions', () => { test('can be used to add permissions to the Lambda function', () => { const stack = new cdk.Stack(); const fn = newTestLambda(stack); @@ -250,6 +250,32 @@ describe('function', () => { }); }); + test('applies source arn condition if principal has conditions', () => { + const stack = new cdk.Stack(); + const fn = newTestLambda(stack); + const sourceArn = 'some-arn'; + const service = 'my-service'; + const principal = new iam.PrincipalWithConditions(new iam.ServicePrincipal(service), { + ArnLike: { + 'aws:SourceArn': sourceArn, + }, + }); + + fn.addPermission('S1', { principal: principal }); + + Template.fromStack(stack).hasResourceProperties('AWS::Lambda::Permission', { + Action: 'lambda:InvokeFunction', + FunctionName: { + 'Fn::GetAtt': [ + 'MyLambdaCCE802FB', + 'Arn', + ], + }, + Principal: service, + SourceArn: sourceArn, + }); + }); + test('applies principal org id conditions if the principal has conditions', () => { const stack = new cdk.Stack(); const fn = newTestLambda(stack); @@ -307,14 +333,6 @@ describe('function', () => { const stack = new cdk.Stack(); const fn = newTestLambda(stack); - expect(() => fn.addPermission('F1', { - principal: new iam.PrincipalWithConditions(new iam.ServicePrincipal('my-service'), { - StringEquals: { - 'aws:SourceAccount': 'source-account', - }, - }), - })).toThrow(/PrincipalWithConditions had unsupported condition combinations for Lambda permission statement/); - expect(() => fn.addPermission('F2', { principal: new iam.PrincipalWithConditions(new iam.ServicePrincipal('my-service'), { StringEquals: { From 8afe9e7df1ea73753c26f2d4b3f6737c2b38ca65 Mon Sep 17 00:00:00 2001 From: Kaizen Conroy Date: Tue, 31 May 2022 14:01:48 -0400 Subject: [PATCH 12/14] pr feedback --- packages/@aws-cdk/aws-lambda/lib/function-base.ts | 4 ++-- packages/@aws-cdk/aws-lambda/lib/function.ts | 2 +- packages/@aws-cdk/aws-lambda/lib/permission.ts | 4 ++-- packages/@aws-cdk/aws-lambda/test/function.test.ts | 2 +- packages/@aws-cdk/aws-lambda/test/lambda-version.test.ts | 2 +- packages/@aws-cdk/aws-lambda/test/singleton-lambda.test.ts | 2 +- 6 files changed, 8 insertions(+), 8 deletions(-) diff --git a/packages/@aws-cdk/aws-lambda/lib/function-base.ts b/packages/@aws-cdk/aws-lambda/lib/function-base.ts index 009f292df18a3..9f7f70222fe24 100644 --- a/packages/@aws-cdk/aws-lambda/lib/function-base.ts +++ b/packages/@aws-cdk/aws-lambda/lib/function-base.ts @@ -361,7 +361,7 @@ export abstract class FunctionBase extends Resource implements IFunction, ec2.IC eventSourceToken: permission.eventSourceToken, sourceAccount: permission.sourceAccount ?? sourceAccount, sourceArn: permission.sourceArn ?? sourceArn, - principalOrgId: permission.principalOrg?.organizationId ?? principalOrgID, + principalOrgId: permission.organizationId ?? principalOrgID, functionUrlAuthType: permission.functionUrlAuthType, }); } @@ -655,7 +655,7 @@ export abstract class FunctionBase extends Resource implements IFunction, ec2.IC return undefined; } - private isPrincipalWithConditions(principal: iam.IPrincipal): principal is iam.PrincipalWithConditions { + private isPrincipalWithConditions(principal: iam.IPrincipal): boolean { return Object.keys(principal.policyFragment.conditions).length > 0; } } diff --git a/packages/@aws-cdk/aws-lambda/lib/function.ts b/packages/@aws-cdk/aws-lambda/lib/function.ts index 68002c22c5b4b..81412f5930505 100644 --- a/packages/@aws-cdk/aws-lambda/lib/function.ts +++ b/packages/@aws-cdk/aws-lambda/lib/function.ts @@ -8,6 +8,7 @@ import * as sns from '@aws-cdk/aws-sns'; import * as sqs from '@aws-cdk/aws-sqs'; import { Annotations, ArnFormat, CfnResource, Duration, Fn, Lazy, Names, Size, Stack, Token } from '@aws-cdk/core'; import { Construct } from 'constructs'; +import { AliasOptions, Alias } from './alias'; import { Architecture } from './architecture'; import { Code, CodeConfig } from './code'; import { ICodeSigningConfig } from './code-signing-config'; @@ -26,7 +27,6 @@ import { Runtime } from './runtime'; // keep this import separate from other imports to reduce chance for merge conflicts with v2-main // eslint-disable-next-line import { LogRetentionRetryOptions } from './log-retention'; -import { AliasOptions, Alias } from './alias'; import { addAlias } from './util'; /** diff --git a/packages/@aws-cdk/aws-lambda/lib/permission.ts b/packages/@aws-cdk/aws-lambda/lib/permission.ts index 3c5b2b1e83c65..4a3b049795e0a 100644 --- a/packages/@aws-cdk/aws-lambda/lib/permission.ts +++ b/packages/@aws-cdk/aws-lambda/lib/permission.ts @@ -82,9 +82,9 @@ export interface Permission { * You can use this property to ensure that all source principals are owned by * a specific organization. * - * @default - No principalOrg + * @default - No organizationId */ - readonly principalOrg?: iam.OrganizationPrincipal; + readonly organizationId?: string; /** * The authType for the function URL that you are granting permissions for. diff --git a/packages/@aws-cdk/aws-lambda/test/function.test.ts b/packages/@aws-cdk/aws-lambda/test/function.test.ts index 6f3a14387a24b..276958df657c7 100644 --- a/packages/@aws-cdk/aws-lambda/test/function.test.ts +++ b/packages/@aws-cdk/aws-lambda/test/function.test.ts @@ -190,7 +190,7 @@ describe('function', () => { fn.addPermission('S3Permission', { action: 'lambda:*', principal: account, - principalOrg: org, + organizationId: org.organizationId, }); Template.fromStack(stack).hasResourceProperties('AWS::Lambda::Permission', { diff --git a/packages/@aws-cdk/aws-lambda/test/lambda-version.test.ts b/packages/@aws-cdk/aws-lambda/test/lambda-version.test.ts index 80a540fa5c6c8..d18e5301d971d 100644 --- a/packages/@aws-cdk/aws-lambda/test/lambda-version.test.ts +++ b/packages/@aws-cdk/aws-lambda/test/lambda-version.test.ts @@ -1,7 +1,7 @@ import { Template } from '@aws-cdk/assertions'; +import { testDeprecated } from '@aws-cdk/cdk-build-tools'; import * as cdk from '@aws-cdk/core'; import * as lambda from '../lib'; -import { testDeprecated } from '@aws-cdk/cdk-build-tools'; describe('lambda version', () => { test('can import a Lambda version by ARN', () => { diff --git a/packages/@aws-cdk/aws-lambda/test/singleton-lambda.test.ts b/packages/@aws-cdk/aws-lambda/test/singleton-lambda.test.ts index bd40f612558e1..fd65c207882ee 100644 --- a/packages/@aws-cdk/aws-lambda/test/singleton-lambda.test.ts +++ b/packages/@aws-cdk/aws-lambda/test/singleton-lambda.test.ts @@ -2,9 +2,9 @@ import { Template } from '@aws-cdk/assertions'; import * as ec2 from '@aws-cdk/aws-ec2'; import * as iam from '@aws-cdk/aws-iam'; import * as s3 from '@aws-cdk/aws-s3'; +import { testDeprecated } from '@aws-cdk/cdk-build-tools'; import * as cdk from '@aws-cdk/core'; import * as lambda from '../lib'; -import { testDeprecated } from '@aws-cdk/cdk-build-tools'; describe('singleton lambda', () => { test('can add same singleton Lambda multiple times, only instantiated once in template', () => { From 48488eb7552011264a5f66c487bca932eab1d7f3 Mon Sep 17 00:00:00 2001 From: Kaizen Conroy Date: Mon, 27 Jun 2022 18:10:49 -0400 Subject: [PATCH 13/14] update permissions integ snapshot --- .../test/permissions.integ.snapshot/cdk.out | 2 +- .../permissions.integ.snapshot/integ.json | 2 +- .../lambda-permissions.assets.json | 19 +++++++++++++++++++ .../lambda-permissions.template.json | 4 ++-- .../permissions.integ.snapshot/manifest.json | 18 +++++++++--------- .../test/permissions.integ.snapshot/tree.json | 16 ++++++++-------- 6 files changed, 40 insertions(+), 21 deletions(-) create mode 100644 packages/@aws-cdk/aws-lambda/test/permissions.integ.snapshot/lambda-permissions.assets.json diff --git a/packages/@aws-cdk/aws-lambda/test/permissions.integ.snapshot/cdk.out b/packages/@aws-cdk/aws-lambda/test/permissions.integ.snapshot/cdk.out index 2efc89439fab8..588d7b269d34f 100644 --- a/packages/@aws-cdk/aws-lambda/test/permissions.integ.snapshot/cdk.out +++ b/packages/@aws-cdk/aws-lambda/test/permissions.integ.snapshot/cdk.out @@ -1 +1 @@ -{"version":"18.0.0"} \ No newline at end of file +{"version":"20.0.0"} \ No newline at end of file diff --git a/packages/@aws-cdk/aws-lambda/test/permissions.integ.snapshot/integ.json b/packages/@aws-cdk/aws-lambda/test/permissions.integ.snapshot/integ.json index 6c62ccc64f4ba..2e8ba8922ac72 100644 --- a/packages/@aws-cdk/aws-lambda/test/permissions.integ.snapshot/integ.json +++ b/packages/@aws-cdk/aws-lambda/test/permissions.integ.snapshot/integ.json @@ -1,5 +1,5 @@ { - "version": "18.0.0", + "version": "20.0.0", "testCases": { "integ.permissions": { "stacks": [ diff --git a/packages/@aws-cdk/aws-lambda/test/permissions.integ.snapshot/lambda-permissions.assets.json b/packages/@aws-cdk/aws-lambda/test/permissions.integ.snapshot/lambda-permissions.assets.json new file mode 100644 index 0000000000000..1a37de91e4acd --- /dev/null +++ b/packages/@aws-cdk/aws-lambda/test/permissions.integ.snapshot/lambda-permissions.assets.json @@ -0,0 +1,19 @@ +{ + "version": "20.0.0", + "files": { + "f189cb5ef6ca8751f98c7dbbfdfb7dac59c18d9bb287a62e9b01d4fb43156f7e": { + "source": { + "path": "lambda-permissions.template.json", + "packaging": "file" + }, + "destinations": { + "current_account-current_region": { + "bucketName": "cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}", + "objectKey": "f189cb5ef6ca8751f98c7dbbfdfb7dac59c18d9bb287a62e9b01d4fb43156f7e.json", + "assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-file-publishing-role-${AWS::AccountId}-${AWS::Region}" + } + } + } + }, + "dockerImages": {} +} \ No newline at end of file diff --git a/packages/@aws-cdk/aws-lambda/test/permissions.integ.snapshot/lambda-permissions.template.json b/packages/@aws-cdk/aws-lambda/test/permissions.integ.snapshot/lambda-permissions.template.json index 53e848c5ba649..74c24026d0f44 100644 --- a/packages/@aws-cdk/aws-lambda/test/permissions.integ.snapshot/lambda-permissions.template.json +++ b/packages/@aws-cdk/aws-lambda/test/permissions.integ.snapshot/lambda-permissions.template.json @@ -50,7 +50,7 @@ "MyLambdaServiceRole4539ECB6" ] }, - "MyLambdaInvokeAnyPrincipal256ECDE1": { + "MyLambdaInvokehlab6Vr41INt1IUXIhhCesB4gzNedP5IURKNgciwD9D5EABD": { "Type": "AWS::Lambda::Permission", "Properties": { "Action": "lambda:InvokeFunction", @@ -64,7 +64,7 @@ "PrincipalOrgID": "o-yyyyyyyyyy" } }, - "MyLambdaInvokeOrganizationPrincipaloxxxxxxxxxxEB282AD1": { + "MyLambdaInvoke138AF9IJcZORjZNKCKShZMMuVQwCnUkbFqMoQf5of0C1F7DFD8": { "Type": "AWS::Lambda::Permission", "Properties": { "Action": "lambda:InvokeFunction", diff --git a/packages/@aws-cdk/aws-lambda/test/permissions.integ.snapshot/manifest.json b/packages/@aws-cdk/aws-lambda/test/permissions.integ.snapshot/manifest.json index 07f1891ce4f22..b1a86d7f35697 100644 --- a/packages/@aws-cdk/aws-lambda/test/permissions.integ.snapshot/manifest.json +++ b/packages/@aws-cdk/aws-lambda/test/permissions.integ.snapshot/manifest.json @@ -1,5 +1,5 @@ { - "version": "18.0.0", + "version": "20.0.0", "artifacts": { "Tree": { "type": "cdk:tree", @@ -27,31 +27,31 @@ "data": "MyLambdaCCE802FB" } ], - "/lambda-permissions/MyLambda/InvokeAnyPrincipal()": [ + "/lambda-permissions/MyLambda/Invokehl--ab6+Vr41INt1IUX--IhhCesB4gzNedP5IURKNgciw=": [ { "type": "aws:cdk:logicalId", - "data": "MyLambdaInvokeAnyPrincipal256ECDE1" + "data": "MyLambdaInvokehlab6Vr41INt1IUXIhhCesB4gzNedP5IURKNgciwD9D5EABD" } ], - "/lambda-permissions/MyLambda/InvokeOrganizationPrincipal(o-xxxxxxxxxx)": [ + "/lambda-permissions/MyLambda/Invoke138AF9IJcZORjZ--NKCKShZMMuVQwCnUkbFqMoQf5of0=": [ { "type": "aws:cdk:logicalId", - "data": "MyLambdaInvokeOrganizationPrincipaloxxxxxxxxxxEB282AD1" + "data": "MyLambdaInvoke138AF9IJcZORjZNKCKShZMMuVQwCnUkbFqMoQf5of0C1F7DFD8" } ], - "MyLambdaorgF85788CF": [ + "MyLambdaInvokeAnyPrincipal256ECDE1": [ { "type": "aws:cdk:logicalId", - "data": "MyLambdaorgF85788CF", + "data": "MyLambdaInvokeAnyPrincipal256ECDE1", "trace": [ "!!DESTRUCTIVE_CHANGES: WILL_DESTROY" ] } ], - "MyLambdaotherorg9E5CFB3F": [ + "MyLambdaInvokeOrganizationPrincipaloxxxxxxxxxxEB282AD1": [ { "type": "aws:cdk:logicalId", - "data": "MyLambdaotherorg9E5CFB3F", + "data": "MyLambdaInvokeOrganizationPrincipaloxxxxxxxxxxEB282AD1", "trace": [ "!!DESTRUCTIVE_CHANGES: WILL_DESTROY" ] diff --git a/packages/@aws-cdk/aws-lambda/test/permissions.integ.snapshot/tree.json b/packages/@aws-cdk/aws-lambda/test/permissions.integ.snapshot/tree.json index b25b654f1d68a..51fbde7100a28 100644 --- a/packages/@aws-cdk/aws-lambda/test/permissions.integ.snapshot/tree.json +++ b/packages/@aws-cdk/aws-lambda/test/permissions.integ.snapshot/tree.json @@ -8,8 +8,8 @@ "id": "Tree", "path": "Tree", "constructInfo": { - "fqn": "@aws-cdk/core.Construct", - "version": "0.0.0" + "fqn": "constructs.Construct", + "version": "10.1.33" } }, "lambda-permissions": { @@ -93,9 +93,9 @@ "version": "0.0.0" } }, - "InvokeAnyPrincipal()": { - "id": "InvokeAnyPrincipal()", - "path": "lambda-permissions/MyLambda/InvokeAnyPrincipal()", + "Invokehl--ab6+Vr41INt1IUX--IhhCesB4gzNedP5IURKNgciw=": { + "id": "Invokehl--ab6+Vr41INt1IUX--IhhCesB4gzNedP5IURKNgciw=", + "path": "lambda-permissions/MyLambda/Invokehl--ab6+Vr41INt1IUX--IhhCesB4gzNedP5IURKNgciw=", "attributes": { "aws:cdk:cloudformation:type": "AWS::Lambda::Permission", "aws:cdk:cloudformation:props": { @@ -115,9 +115,9 @@ "version": "0.0.0" } }, - "InvokeOrganizationPrincipal(o-xxxxxxxxxx)": { - "id": "InvokeOrganizationPrincipal(o-xxxxxxxxxx)", - "path": "lambda-permissions/MyLambda/InvokeOrganizationPrincipal(o-xxxxxxxxxx)", + "Invoke138AF9IJcZORjZ--NKCKShZMMuVQwCnUkbFqMoQf5of0=": { + "id": "Invoke138AF9IJcZORjZ--NKCKShZMMuVQwCnUkbFqMoQf5of0=", + "path": "lambda-permissions/MyLambda/Invoke138AF9IJcZORjZ--NKCKShZMMuVQwCnUkbFqMoQf5of0=", "attributes": { "aws:cdk:cloudformation:type": "AWS::Lambda::Permission", "aws:cdk:cloudformation:props": { From c432d650542a5ff7f72c0309e7b1b0fa504287d4 Mon Sep 17 00:00:00 2001 From: Kaizen Conroy Date: Mon, 27 Jun 2022 18:45:15 -0400 Subject: [PATCH 14/14] refactor org principal logic --- .../@aws-cdk/aws-lambda/lib/function-base.ts | 21 ++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/packages/@aws-cdk/aws-lambda/lib/function-base.ts b/packages/@aws-cdk/aws-lambda/lib/function-base.ts index 1ac3670745c58..14376c3b32909 100644 --- a/packages/@aws-cdk/aws-lambda/lib/function-base.ts +++ b/packages/@aws-cdk/aws-lambda/lib/function-base.ts @@ -572,9 +572,13 @@ export abstract class FunctionBase extends Resource implements IFunction, ec2.IC return (principal as iam.ArnPrincipal).arn; } - if ('organizationId' in principal) { - // we will move the organization id to the `principalOrgId` property of `Permissions`. - return '*'; + const stringEquals = matchSingleKey('StringEquals', principal.policyFragment.conditions); + if (stringEquals) { + const orgId = matchSingleKey('aws:PrincipalOrgID', stringEquals); + if (orgId) { + // we will move the organization id to the `principalOrgId` property of `Permissions`. + return '*'; + } } // Try a best-effort approach to support simple principals that are not any of the predefined @@ -592,6 +596,17 @@ export abstract class FunctionBase extends Resource implements IFunction, ec2.IC throw new Error(`Invalid principal type for Lambda permission statement: ${principal.constructor.name}. ` + 'Supported: AccountPrincipal, ArnPrincipal, ServicePrincipal, OrganizationPrincipal'); + + /** + * Returns the value at the key if the object contains the key and nothing else. Otherwise, + * returns undefined. + */ + function matchSingleKey(key: string, obj: Record): any | undefined { + if (Object.keys(obj).length !== 1) { return undefined; } + + return obj[key]; + } + } private validateConditionCombinations(principal: iam.IPrincipal): {