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

CDK Grants into AWS Managed Policy #7448

Closed
2 tasks
0xjjoyy opened this issue Apr 19, 2020 · 10 comments
Closed
2 tasks

CDK Grants into AWS Managed Policy #7448

0xjjoyy opened this issue Apr 19, 2020 · 10 comments
Labels
@aws-cdk/aws-iam Related to AWS Identity and Access Management effort/large Large work item – several weeks of effort feature-request A feature should be added or improved. p2

Comments

@0xjjoyy
Copy link

0xjjoyy commented Apr 19, 2020

CDK Grants option to specify either an existing AWS Managed Policy to to create the Grant as a new AWS Managed Policy

Use Case

AWS IAM Best Practice
Use Customer Managed Policies Instead of Inline Policies

It's easier to manage, version, control, and review AWS Customer Managed Policies compared to Inline policies.

Users should have the option to utilize AWS Customer Managed Policies, rather than only inline policies.

Proposed Solution

Allow the ability to create a new AWS Customer Managed Policy or specify an existing AWS Customer Managed Policy. Rather than the default which is always an Inline Policy.

Other

  • 👋 I may be able to implement this feature request
  • ⚠️ This feature might incur a breaking change

This is a 🚀 Feature Request

@0xjjoyy 0xjjoyy added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Apr 19, 2020
@SomayaB SomayaB added the @aws-cdk/aws-iam Related to AWS Identity and Access Management label Apr 20, 2020
@rix0rrr rix0rrr added the effort/medium Medium work item – several days of effort label Apr 21, 2020
@SomayaB SomayaB removed the needs-triage This issue or PR still needs to be triaged. label May 19, 2020
@rix0rrr rix0rrr added effort/large Large work item – several weeks of effort p2 and removed effort/medium Medium work item – several days of effort labels Aug 12, 2020
@rix0rrr rix0rrr removed their assignment Jun 3, 2021
@athewsey
Copy link

+1: I was surprised recently to learn that ManagedPolicy doesn't implement IGrantable. Any updates on this?

I'd like to build a stack/app that outputs a ManagedPolicy with required permissions for stack resources (specifically things like S3 buckets, SSM param read/write, SFn state machine execution...) - so that this policy can be attached to/removed from roles/users/groups as required.

Today, it seems like I'd need to manually build up lists of PolicyStatements to achieve this, since the high-level resource methods like Bucket.grant_read(...) all require IGrantables so can't work with a ManagedPolicy?

@elacy
Copy link

elacy commented Sep 20, 2021

Here is a quick work around for anyone else that runs into this problem

import cdk = require('monocdk');
import iam = require("monocdk/aws-iam");

export class PrincipalPolicy extends iam.ManagedPolicy implements iam.IPrincipal {
    private principal: iam.IPrincipal;

    constructor(scope: cdk.Construct, id: string, props: iam.ManagedPolicyProps) {
        super(scope, id, props);

        if ((props.users?.length || 0) + (props.roles?.length || 0) + (props.groups?.length || 0) !== 1) {
            throw Error("PrincipalPolicy must have one and only one principal");
        }

        if (props.users?.length === 1) {
            this.principal = props.users[0];
        }

        if (props.roles?.length === 1) {
            this.principal = props.roles[0];
        }

        if (props.groups?.length === 1) {
            this.principal = props.groups[0];
        }
    }

    /**
     * (experimental) When this Principal is used in an AssumeRole policy, the action to use.
     *
     * @experimental
     */
    get assumeRoleAction(): string {
        return this.principal.assumeRoleAction;
    }

    /**
     * (experimental) Return the policy fragment that identifies this principal in a Policy.
     *
     * @experimental
     */
    get policyFragment(): iam.PrincipalPolicyFragment {
        return this.principal.policyFragment;
    }

    /**
     * (experimental) The AWS account ID of this principal.
     *
     * Can be undefined when the account is not known
     * (for example, for service principals).
     * Can be a Token - in that case,
     * it's assumed to be AWS::AccountId.
     *
     * @experimental
     */
    get principalAccount(): string | undefined {
        return this.principal.principalAccount;
    }

    /**
     * (deprecated) Add to the policy of this principal.
     *
     * @returns true if the statement was added, false if the principal in
     * question does not have a policy document to add the statement to.
     * @deprecated Use `addToPrincipalPolicy` instead.
     */
    public addToPolicy(statement: iam.PolicyStatement): boolean {
        super.addStatements(statement);
        return true;
    }

    /**
     * (experimental) Add to the policy of this principal.
     *
     * @experimental
     */
    public addToPrincipalPolicy(statement: iam.PolicyStatement): iam.AddToPrincipalPolicyResult {
        super.addStatements(statement);
        return {
            statementAdded: true,
            policyDependable: this
        };
    }

    /**
     * (experimental) The principal to grant permissions to.
     *
     * @experimental
     */

    get grantPrincipal(): iam.IPrincipal {
        return this;
    }
}

@automartin5000
Copy link

Just ran into the inline policy limit where this could've solved for that. Reported bug here: #18457

@kornicameister
Copy link
Contributor

cdk team, why is that exactly an issue here to be able to grant into managed policies?

@bilalq
Copy link

bilalq commented Aug 16, 2022

The inability to attach a managed policy makes it way harder to grant CodeArtifact access to CDK pipeline synth steps than it should be.

@kornicameister
Copy link
Contributor

I mean, the whole concept of granting is based on iam.IGrantable that has the principal. ManagedPolicies may have multiple policies but perhaps it would be simply sufficient to have iam.IGrantablePolicy and or iam.IGrantablePrincipal interfaces inheriting from iam.IGrantable. That way most of the code could stay put and long story short if we had with iam.IGrantablePrincipal we would simply call add_statements. Needless to say all those statements are anywhere in the code because you actually put them into default policy.

@dan-lind
Copy link
Contributor

Related PR, not sure why it's still a draft though. #22712

@Tietew
Copy link
Contributor

Tietew commented Feb 8, 2023

#22712 has been merged. Policy and ManagedPolicy now implement IGrantable.
It just throw an error when a Policy/ManagedPolicy is added to a resource-based policy.

@khushail
Copy link
Contributor

Hi @Tietew , looks like the issue has been resolved and changes implemented. So I would be marking this issue as closed.

For this new error,- It just throw an error when a Policy/ManagedPolicy is added to a resource-based policy.
you might want to open a new issue to get more traction and help.

@khushail khushail added p1.5 and removed p1 labels May 16, 2023
@github-actions
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

@otaviomacedo otaviomacedo added p2 and removed p1.5 labels May 22, 2023
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 effort/large Large work item – several weeks of effort feature-request A feature should be added or improved. p2
Projects
None yet
Development

No branches or pull requests