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(iam): Permissions Boundaries #12777

Merged
merged 5 commits into from
Feb 1, 2021
Merged

Conversation

rix0rrr
Copy link
Contributor

@rix0rrr rix0rrr commented Jan 29, 2021

Allow configuring Permissions Boundaries for an entire subtree using
Aspects, add a sample policy which can be used to reduce future
misconfiguration risk for untrusted CodeBuild projects as an example.

Addresses one part of aws/aws-cdk-rfcs#5.

Fixes #3242.

ALSO IN THIS COMMIT:

Fix a bug in the assert library, where haveResource() would never match
any resource that didn't have a Properties block (even if we tested for no property
in particular, or the absence of properties). This fix caused two ECS tests to fail,
which were asserting the wrong thing anyway (both were asserting notTo(haveResource(...))
where they actually meant to assert to(haveResource()).


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

Allow configuring Permissions Boundaries for an entire subtree using
Aspects, add a sample policy which can be used to reduce future
misconfiguration risk for untrusted CodeBuild projects as an example.

Fixes #3242.
@rix0rrr rix0rrr requested a review from a team January 29, 2021 15:52
@rix0rrr rix0rrr self-assigned this Jan 29, 2021
@gitpod-io
Copy link

gitpod-io bot commented Jan 29, 2021

@github-actions github-actions bot added the @aws-cdk/aws-iam Related to AWS Identity and Access Management label Jan 29, 2021
@mergify mergify bot added the contribution/core This is a PR that came from AWS. label Jan 29, 2021
@robertd
Copy link
Contributor

robertd commented Jan 30, 2021

Copy link
Contributor

@eladb eladb left a comment

Choose a reason for hiding this comment

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

Looks awesome! People are going to love this!

Add a note in the PR description that this is related to aws/aws-cdk-rfcs#5

'ec2:DescribeDhcpOptions',
'ec2:DescribeVpcs',

// NOTABLY MISSING:
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it make sense to add a "forbid" statements for these?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not really. It does the same thing.

tree all at once:

```ts
// This imports an existing policy. You can also create a new one.
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't we also want to show how to define a new permission boundary using the CDK?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The 99.9% use case is that a PB will have been created for you by an administrator. It only makes sense in very select cases to create one yourself. But sure.

@eladb eladb added the pr/do-not-merge This PR should not be merged at this time. label Jan 31, 2021
@eladb
Copy link
Contributor

eladb commented Jan 31, 2021

Minor notes, added do-not-merge

@rix0rrr rix0rrr removed the pr/do-not-merge This PR should not be merged at this time. label Feb 1, 2021
@mergify
Copy link
Contributor

mergify bot commented Feb 1, 2021

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

@mergify
Copy link
Contributor

mergify bot commented Feb 1, 2021

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

@mergify mergify bot merged commit 415eb86 into master Feb 1, 2021
@mergify mergify bot deleted the huijbers/permissions-boundary branch February 1, 2021 13:40
@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
  • Commit ID: 2720039
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

NovakGu pushed a commit to NovakGu/aws-cdk that referenced this pull request Feb 18, 2021
Allow configuring Permissions Boundaries for an entire subtree using
Aspects, add a sample policy which can be used to reduce future
misconfiguration risk for untrusted CodeBuild projects as an example.

Addresses one part of aws/aws-cdk-rfcs#5.

Fixes aws#3242.

ALSO IN THIS COMMIT:

Fix a bug in the `assert` library, where `haveResource()` would *never* match
any resource that didn't have a `Properties` block (even if we tested for no property
in particular, or the absence of properties). This fix caused two ECS tests to fail,
which were asserting the wrong thing anyway (both were asserting `notTo(haveResource(...))`
where they actually meant to assert `to(haveResource())`.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-iam Related to AWS Identity and Access Management contribution/core This is a PR that came from AWS.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Applying permissions boundary to aws-cdk globally
4 participants