-
Notifications
You must be signed in to change notification settings - Fork 6
Create "raw" managed policy using existing class props #2822
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
Open
aug24
wants to merge
13
commits into
main
Choose a base branch
from
raw-managed-policies
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+219
−0
Open
Changes from all commits
Commits
Show all changes
13 commits
Select commit
Hold shift + click to select a range
8de29e8
Create "raw" managed policy using existing class props
aug24 62c6674
Add 'raw' managed policy with existing props
aug24 3a25978
Apply suggestion from @kelvin-chappell
aug24 867175d
Apply suggestion from @kelvin-chappell
aug24 fea14e0
Commit tests
aug24 fc32b64
Lint
aug24 3c01d58
Requested changes
aug24 cb13db0
Prune props harder
aug24 2d95c9d
Remove managed policy name as it is dangerous. Add documentation
aug24 4276e77
Rename after discussions
aug24 b5108fa
Add tests for permissions on *
aug24 0f33823
Add better explanation to change set
aug24 29190f3
Lint
aug24 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| --- | ||
| "@guardian/cdk": minor | ||
| --- | ||
|
|
||
| Add a class for safe instantiation of managed policies with a specific structure of path | ||
| which enables them to be discoverable. | ||
|
|
||
| This enables teams to define sets of permissions which are re-usable and can be used to | ||
| create credentials suitable to approach a given workload, consistent with the Principle | ||
| of Least Privilege. This is preferred to existing workflows where a wide-ranging | ||
| developer role is used. | ||
|
|
||
| These can be reused in multiple locations, so, for example, an EC2 instance can be given | ||
| a specific set of permissions which are also identically available for a support task. | ||
| Changing one would then change the other, ensuring encapsulation of requirements in a | ||
| single place. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,96 @@ | ||
| import { Template } from "aws-cdk-lib/assertions"; | ||
| import { simpleGuStackForTesting } from "../../../utils/test"; | ||
| import { GuWorkloadPolicy } from "./workload-policy"; | ||
|
|
||
| describe("GuDeveloperPolicy", () => { | ||
| test("if a single action is provided, the resulting Workload Policy resource's statement will have a single item", () => { | ||
| const stack = simpleGuStackForTesting(); | ||
| new GuWorkloadPolicy(stack, "AllowS3GetObject", { | ||
| allow: [ | ||
| { | ||
| actions: ["s3:GetObject"], | ||
| resources: ["s3:///log-bucket"], | ||
| }, | ||
| ], | ||
| permission: "test123", | ||
| }); | ||
|
|
||
| Template.fromStack(stack).hasResourceProperties("AWS::IAM::ManagedPolicy", { | ||
| Path: "/workload-policy/test123/", | ||
| PolicyDocument: { | ||
| Version: "2012-10-17", | ||
| Statement: [ | ||
| { | ||
| Action: "s3:GetObject", | ||
| Effect: "Allow", | ||
| Resource: "s3:///log-bucket", | ||
| }, | ||
| ], | ||
| }, | ||
| }); | ||
| }); | ||
|
|
||
| test("throws an error if a wide-open permissions is requested", () => { | ||
| const stack = simpleGuStackForTesting(); | ||
| expect(() => { | ||
| new GuWorkloadPolicy(stack, "AllowS3GetObject", { | ||
| allow: [ | ||
| { | ||
| actions: ["s3:GetObject"], | ||
| resources: ["*"], | ||
| }, | ||
| ], | ||
| permission: "test123", | ||
| }); | ||
| }).toThrow("Overly broad permission present, see annotations for details"); | ||
| }); | ||
|
|
||
| test("if multiple actions are provided, the resulting Managed Policy resource's action will container all items", () => { | ||
| const stack = simpleGuStackForTesting(); | ||
| new GuWorkloadPolicy(stack, "AllowS3GetObject", { | ||
| allow: [ | ||
| { | ||
| actions: ["s3:GetObject"], | ||
| resources: ["arn:aws:s3:::mybucket/mypath"], | ||
| }, | ||
| { | ||
| actions: ["s3:GetObject"], | ||
| resources: ["arn:aws:s3:::mybucket/myotherpath"], | ||
| }, | ||
| ], | ||
| deny: [ | ||
| { | ||
| actions: ["s3:GetObject"], | ||
| resources: ["arn:aws:s3:::mybucket/mypath/butnotthispath"], | ||
| }, | ||
| ], | ||
| permission: "test321", | ||
| description: "testtesttest", | ||
| }); | ||
|
|
||
| Template.fromStack(stack).hasResourceProperties("AWS::IAM::ManagedPolicy", { | ||
| Description: "testtesttest", | ||
| Path: "/workload-policy/test321/", | ||
| PolicyDocument: { | ||
| Version: "2012-10-17", | ||
| Statement: [ | ||
| { | ||
| Action: "s3:GetObject", | ||
| Effect: "Allow", | ||
| Resource: "arn:aws:s3:::mybucket/mypath", | ||
| }, | ||
| { | ||
| Action: "s3:GetObject", | ||
| Effect: "Allow", | ||
| Resource: "arn:aws:s3:::mybucket/myotherpath", | ||
| }, | ||
| { | ||
| Action: "s3:GetObject", | ||
| Effect: "Deny", | ||
| Resource: "arn:aws:s3:::mybucket/mypath/butnotthispath", | ||
| }, | ||
| ], | ||
| }, | ||
| }); | ||
| }); | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,106 @@ | ||
| import { Annotations } from "aws-cdk-lib"; | ||
| import { Effect, ManagedPolicy, PolicyStatement } from "aws-cdk-lib/aws-iam"; | ||
| import type { GuStack } from "../../core"; | ||
| import type { GuAllowPolicyProps, GuDenyPolicyProps } from "./base-policy"; | ||
|
|
||
| export type GuWorkloadPolicyProps = { | ||
| /** | ||
| * List of explicitly allowed permissions given by this policy. | ||
| */ | ||
| allow: GuAllowPolicyProps[]; | ||
| /** | ||
| * List of explicitly denied permissions which can be used to fine tune this policy by pruning the allow permissions. | ||
| */ | ||
| deny?: GuDenyPolicyProps[]; | ||
| /** | ||
| * The unique identifier of the policy, which will be displayed when creating credentials. | ||
| */ | ||
| permission: string; | ||
| /** | ||
| * An optional description of the policy which will be displayed if present. | ||
| */ | ||
| description?: string; | ||
| }; | ||
|
|
||
| /** | ||
| * Creates a structured `AWS::IAM::ManagedPolicy` resource to manage arbitrary permissions on general account | ||
| * resources which can then be used to create limited permission credentials for use with specific activities. | ||
| * | ||
| * The permission scope is not controlled. This class should be used with care to create minimal permissions. | ||
| * To that end, broad ALLOW permissions can pruned with narrower optional DENY permissions. | ||
| * | ||
| * `permission` is prefixed with `/developer-policy/` and postfixed with `/` to construct the path. This will | ||
| * be used for discovery and display. It is restricted to the same character set as AWS `path`. | ||
| * | ||
| * `description` is optionally used to construct the AWS Managed Policy description and used for display. | ||
| * | ||
| * ```yaml | ||
| * TestingECAE2E87: | ||
| * Type: AWS::IAM::ManagedPolicy | ||
| * Properties: | ||
| * Description: This is testing stuff | ||
| * Path: /developer-policy/read-from-mybucket-under-mypath/ | ||
| * PolicyDocument: | ||
| * Statement: | ||
| * - Action: s3:GetObject | ||
| * Effect: Allow | ||
| * Resource: arn:aws:s3:::mybucket/mypath | ||
| * - Action: s3:GetObject | ||
| * Effect: Deny | ||
| * Resource: arn:aws:s3:::mybucket/mypath/butnotthispath | ||
| * Version: "2012-10-17" | ||
| * Metadata: | ||
| * aws:cdk:path: janus-resources-for-testing-managed-policy-tagging/justin-testing/Resource* ``` | ||
| * ``` | ||
| */ | ||
| export class GuWorkloadPolicy extends ManagedPolicy { | ||
| constructor(scope: GuStack, id: string, props: GuWorkloadPolicyProps) { | ||
| super(scope, id, { | ||
| description: `${props.permission} developer policy`, | ||
| ...props, | ||
| path: `/workload-policy/${props.permission}/`, | ||
| }); | ||
|
|
||
| let valid = true; | ||
|
|
||
| for (const allowed of props.allow) { | ||
| // validity checks | ||
| const name = allowed.policyName ?? allowed.actions.join(",") + " on " + allowed.resources.join(","); | ||
| for (const resource of allowed.resources) { | ||
| if (resource === "*") { | ||
| Annotations.of(this).addError(`Resource of '*' found in ${name} ALLOW permission`); | ||
| valid = false; | ||
| } | ||
| } | ||
| for (const action of allowed.actions) { | ||
| if (action === "*") { | ||
| const name = allowed.policyName ?? allowed.actions.join(","); | ||
| Annotations.of(this).addError(`Action of '*' found in ${name} ALLOW permission`); | ||
| valid = false; | ||
| } | ||
| } | ||
| this.addStatements( | ||
| new PolicyStatement({ | ||
| effect: Effect.ALLOW, | ||
| resources: allowed.resources, | ||
| actions: allowed.actions, | ||
| }), | ||
| ); | ||
| } | ||
|
|
||
| if (!valid) { | ||
| throw new Error("Overly broad permission present, see annotations for details"); | ||
| } | ||
|
|
||
| const { deny = [] } = props; | ||
| for (const denied of deny) { | ||
| this.addStatements( | ||
| new PolicyStatement({ | ||
| effect: Effect.DENY, | ||
| resources: denied.resources, | ||
| actions: denied.actions, | ||
| }), | ||
| ); | ||
| } | ||
| } | ||
| } | ||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we document these properties please? TypeDoc strings make their way to the documentation site.