Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(lambda): grant function permissions to an AWS organization #19975

Merged
merged 24 commits into from
Jun 28, 2022
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
fca493b
update readme to reflect new feature
kaizencc Apr 18, 2022
34f5cae
add new prop and change docs for principal
kaizencc Apr 18, 2022
d2122d2
more doc changes
kaizencc Apr 19, 2022
71e6bd6
main commit with changes to permission handling and testing
kaizencc Apr 19, 2022
3b4311f
add integ test
kaizencc Apr 19, 2022
545dc73
fix snippets
kaizencc Apr 20, 2022
1d89444
Merge branch 'master' into conroy/lambda-perms
kaizencc Apr 20, 2022
fb5a9d2
Merge branch 'master' into conroy/lambda-perms
kaizencc Apr 27, 2022
d4c2904
update readme to reflect potential convenience method in iam
kaizencc Apr 27, 2022
60022d7
remove instanceOf in favor of parsing the conditions
kaizencc Apr 27, 2022
b6769bb
some changes from pr feedback
kaizencc Apr 28, 2022
da48f1d
Merge branch 'master' into conroy/lambda-perms
kaizencc Apr 28, 2022
4ad0b40
Merge branch 'conroy/lambda-perms' of https://github.com/aws/aws-cdk …
kaizencc Apr 28, 2022
c26d3ad
update integ test and fix bug in isPrincipalWithConditions
kaizencc Apr 28, 2022
b5f1e1b
Merge branch 'master' into conroy/lambda-perms
kaizencc Apr 29, 2022
2f64196
fix(lambda): grant function permissions does not accept sourceArn con…
kaizencc Apr 29, 2022
8afe9e7
pr feedback
kaizencc May 31, 2022
0e2202f
Merge branch 'main' into conroy/lambda-perms
kaizencc Jun 7, 2022
d453215
Merge branch 'main' into conroy/lambda-perms
kaizencc Jun 15, 2022
48488eb
update permissions integ snapshot
kaizencc Jun 27, 2022
9b8f92e
Merge branch 'main' into conroy/lambda-perms
kaizencc Jun 27, 2022
c432d65
refactor org principal logic
kaizencc Jun 27, 2022
99d9d5a
Merge branch 'main' into conroy/lambda-perms
mergify[bot] Jun 28, 2022
bbc7420
Merge branch 'main' into conroy/lambda-perms
mergify[bot] Jun 28, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 55 additions & 13 deletions packages/@aws-cdk/aws-lambda/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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');

Expand All @@ -172,10 +173,58 @@ 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 org = new iam.OrganizationPrincipal('o-xxxxxxxxxx');

fn.grantInvoke(org);
```

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;
const account = new iam.AccountPrincipal('123456789012');

fn.grantInvoke(account.inOrganization('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 = new iam.AccountPrincipal('123456789012');

fn.grantInvoke(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
Expand All @@ -198,13 +247,6 @@ const servicePrincipalWithConditions = servicePrincipal.withConditions({
});

fn.grantInvoke(servicePrincipalWithConditions);

// Equivalent to:
fn.addPermission('my-service Invocation', {
principal: servicePrincipal,
sourceArn: sourceArn,
sourceAccount: sourceAccount,
});
```

## Versions
Expand Down
74 changes: 61 additions & 13 deletions packages/@aws-cdk/aws-lambda/lib/function-base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -345,8 +345,10 @@ 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) ?? {};

const action = permission.action ?? 'lambda:InvokeFunction';
const scope = permission.scope ?? this;

Expand All @@ -359,6 +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,
functionUrlAuthType: permission.functionUrlAuthType,
});
}
Expand Down Expand Up @@ -548,7 +551,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'];
Expand All @@ -566,6 +568,11 @@ export abstract class FunctionBase extends Resource implements IFunction, ec2.IC
return (principal as iam.ArnPrincipal).arn;
}

if ('organizationId' in principal) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feels nasty, and should not be necessary? OrganizationPrincipals fragment looks like this:

  public get policyFragment(): PrincipalPolicyFragment {
    return new PrincipalPolicyFragment(
      { AWS: ['*'] },
      { StringEquals: { 'aws:PrincipalOrgID': this.organizationId } },
    );
  }

So it should already fall properly into the cases below, where we parse the output of policyFragment and parse the AWS key out?

(Honestly -- that should be the only thing we do. The duck typing here above is 🤮)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'm down to move it out, but the case below has the comment

// The principal cannot have conditions and must have a single { AWS: [arn] } entry.

OrganizationPrincipal has the condition StringEquals, so it fails. @rix0rrr since you were the one who introduced that comment, can you explain why the principal cannot have conditions? maybe we can loosen the logic to allow for something like OrganizationPrincipal to be parsed, but until then, the duck typing stays :).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

    // 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.
    //
    // The principal cannot have conditions and must have a single { AWS: [arn] } entry.
    const json = principal.policyFragment.principalJson;
    if (Object.keys(principal.policyFragment.conditions).length === 0 && json.AWS) {
      if (typeof json.AWS === 'string') { return json.AWS; }
      if (Array.isArray(json.AWS) && json.AWS.length === 1 && typeof json.AWS[0] === 'string') {
        return json.AWS[0];
      }
    }

the parsing logic, for reference.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In that case, I'd still prefer to see something like:

const stringEquals = matchSingleKey('StringEquals', principal.policyFragment.conditions));
if (stringEquals) {
  const orgId = matchSingleKey('aws:PrincipalOrgID', stringEquals);
  if (orgId) {
    return '*';
  }
}

Right?

What I'm trying to get at is that we look at the existing AWS principal and conditions, and parse and convert those to Lambda Permissions.

Instead of, what the code is historically doing, is looking at what classes you used and using "inside" knowledge of what those classes represent to generate the right output.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've done this for org principal. I wanted to refactor the others to follow the same approach, but I couldn't get it to work. The approach works for org principal because we can identify the org principal from its policy fragment. That isn't the case for something like ArnPrincipal and AccountPrincipal, since both their policy fragments have just an arn value.

// 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
// classes, but are simple enough that they will fit into the Permission model. Main target
// here: imported Roles, Users, Groups.
Expand All @@ -580,17 +587,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(
Expand All @@ -599,21 +650,18 @@ 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 {
return 'conditions' in principal;
return Object.keys(principal.policyFragment.conditions).length > 0;
Copy link
Contributor Author

@kaizencc kaizencc Apr 28, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a bug to me. OrganizationPrincipal does not have a conditions property but its policy fragment does have a condition.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right. But watch out! The return type of this function is a type assertion, which is now no longer true! You need to fix the return type and callsites as well.

}
}

Expand Down
30 changes: 23 additions & 7 deletions packages/@aws-cdk/aws-lambda/lib/permission.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,19 +26,22 @@ 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, an ArnPrincipal, a ServicePrincipal,
* or an OrganizationPrincipal.
*/
readonly principal: iam.IPrincipal;

Expand Down Expand Up @@ -70,6 +73,19 @@ 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. 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
*/
readonly principalOrg?: iam.OrganizationPrincipal;
kaizencc marked this conversation as resolved.
Show resolved Hide resolved

/**
* The authType for the function URL that you are granting permissions for.
*
Expand Down
Loading