aws-sns: `Topic.grantPublish(...)` creates identity policy assuming grantee is local to aws account. #29999

wbertore · 2024-04-29T17:39:34Z

Describe the bug

When creating an external iam user such as with User.fromArn(...) and adding it to a topic resource policy with grantPublish, the underlying constructs will create an identity policy assuming the iam user exists already in the stack.

This fails on cloudformation deployment.

Expected Behavior

It should create a resource policy on the SNS Topic and skip the identity policy if the grantee is from an external aws account.

Current Behavior

Topic.grantPublish, Grant.addToPrincipleOrResource, and User.addtoPrinciplePolicy will create a policy for the iam user, assuming it is part of the stack's aws account.

This fails on cloudformation deployment.

Reproduction Steps

Make a test app and stack:

import { App, Stack, StackProps } from 'aws-cdk-lib';
import { User } from 'aws-cdk-lib/aws-iam';
import { ITopic, Topic } from 'aws-cdk-lib/aws-sns';

const externalIamUser = 'arn:aws:iam::123456789012:user/OthersExternalIamUser';
export class TestSnsExternalIamUserStack extends Stack {
  public readonly myTopic: ITopic;

  constructor(scope: App, props: StackProps) {
    super(scope, 'TestSnsExternalIamUserStack', props);

    this.myTopic = new Topic(this, 'MyTopic');
    this.myTopic.grantPublish(User.fromUserArn(this, `OthersExternalIamUser`, externalIamUser));
  }
}

const app = new App();

// eslint-disable-next-line @typescript-eslint/no-unused-vars
const testSnsExternalIamUserStack = new TestSnsExternalIamUserStack(app, {
  description: 'test stack for aws-cdk bug report',
  env: { account: '234567890123', region: 'us-west-2' },
});

app.synth();

synthesize the stack:

cdk synth

see cloudformation output:

Description: test stack for aws-cdk bug report
Resources:
  MyTopic86869434:
    Type: AWS::SNS::Topic
    Metadata:
      aws:cdk:path: TestSnsExternalIamUserStack/MyTopic/Resource
  MyTopicPolicy12A5EC17:
    Type: AWS::SNS::TopicPolicy
    Properties:
      PolicyDocument:
        Statement:
          - Action: sns:Publish
            Effect: Allow
            Principal:
              AWS: arn:aws:iam::123456789012:user/OthersExternalIamUser
            Resource:
              Ref: MyTopic86869434
            Sid: "0"
        Version: "2012-10-17"
      Topics:
        - Ref: MyTopic86869434
    Metadata:
      aws:cdk:path: TestSnsExternalIamUserStack/MyTopic/Policy/Resource
  OthersExternalIamUserPolicyB3CCA1EB:
    Type: AWS::IAM::Policy
    Properties:
      PolicyDocument:
        Statement:
          - Action: sns:Publish
            Effect: Allow
            Resource:
              Ref: MyTopic86869434
        Version: "2012-10-17"
      PolicyName: OthersExternalIamUserPolicyB3CCA1EB
      Users:
        - OthersExternalIamUser
    Metadata:
      aws:cdk:path: TestSnsExternalIamUserStack/OthersExternalIamUser/Policy/Resource
  CDKMetadata:
    Type: AWS::CDK::Metadata
    Properties:
      Analytics: v2:deflate64:H4sIAAAAAAAA/0WJyw6CMBBFv4X9dKSsYO0PGHRvylCS4dEaBjSm6b/7KOjm3nPPLbDSmGfmIYraQY3cYDgvhgZ4q2sQJxgu/sYEx84l+ObJj0zPn0wzApsJw//bdITail9nsh+5cwTnW4u9HO66xCJHnfXCrObVLTxZrFO/AJt/vFuiAAAA
    Metadata:
      aws:cdk:path: TestSnsExternalIamUserStack/CDKMetadata/Default
Parameters:
  BootstrapVersion:
    Type: AWS::SSM::Parameter::Value<String>
    Default: /cdk-bootstrap/hnb659fds/version
    Description: Version of the CDK Bootstrap resources in this environment, automatically retrieved from SSM Parameter Store. [cdk:skip]
Rules:
  CheckBootstrapVersion:
    Assertions:
      - Assert:
          Fn::Not:
            - Fn::Contains:
                - - "1"
                  - "2"
                  - "3"
                  - "4"
                  - "5"
                - Ref: BootstrapVersion
        AssertDescription: CDK bootstrap stack version 6 required. Please run 'cdk bootstrap' with a recent version of the CDK CLI.

Notice that CDK generates a Policy that references a user that doesn't exist in the cloudformation stack.

Possible Solution

Add intelligence to the grantPublish procedure or underlying calls in Grant or User to compare the stack aws account against the user aws account to skip the identity policy creation.

Additional Information/Context

I am using an internal version of cdk for my company and cannot upgrade to the latest due to company library dependencies. It's possible this is fixed in the latest version (but unlikely after reading the source code and revision history in aws-cdk).

CDK CLI Version

2.77.0

Framework Version

No response

Node.js Version

18

OS

MacOS Sonoma 14.4.1

Language

TypeScript

Language Version

5.0.4

Other information

No response

The text was updated successfully, but these errors were encountered:

khushail · 2024-04-30T20:38:21Z

Hi @wbertore , Thanks for reaching out. It is highly suggested to use the latest CDK version. Neverthelss, Looks like with the latest cdk version 2.139 and with cdk 2.77, I am able to successfully synth and deploy the code with external user policy created .

export class GrantPublishStack extends cdk.Stack {
  public readonly mytopic : ITopic;
  constructor(scope: Construct, id: string, props?: cdk.StackProps) {
    super(scope, id, props);

    this.mytopic = new Topic(this, 'MyTopic', {
      displayName: 'MyTopic',
    });

    this.mytopic.grantPublish(User.fromUserArn(this, 'OtherExternaluser', 'arn:aws:iam::55**********:user/admin'));
  }
}

This is the synth template which has the policy for the external user mentioned in the code -


{
 "Resources": {
  "MyTopic86869434": {
   "Type": "AWS::SNS::Topic",
   "Properties": {
    "DisplayName": "MyTopic"
   },
   "Metadata": {
    "aws:cdk:path": "GrantPublishStack/MyTopic/Resource"
   }
  },
  "OtherExternaluserPolicyCD96E322": {
   "Type": "AWS::IAM::Policy",
   "Properties": {
    "PolicyDocument": {
     "Statement": [
      {
       "Action": "sns:Publish",
       "Effect": "Allow",
       "Resource": {
        "Ref": "MyTopic86869434"
       }
      }
     ],
     "Version": "2012-10-17"
    },
    "PolicyName": "OtherExternaluserPolicyCD96E322",
    "Users": [
     "admin"
    ]
   },
   "Metadata": {
    "aws:cdk:path": "GrantPublishStack/OtherExternaluser/Policy/Resource"
   }
  },
  "CDKMetadata": {
   "Type": "AWS::CDK::Metadata",
   "Properties": {
    "Analytics": "v2:deflate64:H4sIAAAAAAAA/03IPQ7CMAxA4bN0TwwpC8y9ACrdUXCC5P7YqE5BKMrdoXRhep9eDe5wgn3lX2oxDHakG+RL8jiY77pmZYXcyYPQNHf+oRjyE+SzjITv9W4qxbRRZZkxrvPfjXCgRMLFsIQIve6e7giuBlf1SmTnhRNNEdqtH+S0s/2VAAAA"
   },
   "Metadata": {
    "aws:cdk:path": "GrantPublishStack/CDKMetadata/Default"
   },
   "Condition": "CDKMetadataAvailable"
  }
 },

Snippet for the successful deployment with CDK 2.77 -

However I see some policy missing in the console, despite being successful. Diving deep to get to the root cause of what could be going wrong.

pahud · 2024-04-30T22:55:35Z

This is because iam.User.fromUserArn() does not return the principalAccount correctly.

PoC

export class DummyStack extends Stack {
  constructor(scope: Construct, id: string, props: StackProps) {
    super(scope, id, props);

    const externalIamUser = 'arn:aws:iam::123456789012:user/OthersExternalIamUser';
    const externalIamRole = 'arn:aws:iam::123456789012:role/OthersExternalIamUser';
    const granteeUser = iam.User.fromUserArn(this, 'OthersExternalIamUser', externalIamUser)
    const granteeRole = iam.Role.fromRoleArn(this, 'OthersExternalIamRole', externalIamRole)
    new CfnOutput(this, 'principalAccountUser', { value: granteeUser.grantPrincipal.principalAccount! })
    new CfnOutput(this, 'principalAccountRole', { value: granteeRole.grantPrincipal.principalAccount! })
  }
}

Outputs:
dummy-stack2.principalAccountRole = 123456789012
dummy-stack2.principalAccountUser = <your_own_account>

see the different implementation between fromUserArn() and fromRoleArn().

We are getting the pricipal account with the Aws.ACCOUNT_ID which presumes always the same account and that is the root cause of this bug.

Making this a p1 bug.

pahud · 2024-04-30T23:03:00Z

By the way, it's generally not recommended using IAM user like that but IAM role is always recommended. While this is a bug we need to fix, is there any reason you have to use iam.User rather than iam.Role?

### Issue # (if applicable) Closes #29999 ### Reason for this change As described in the issue [comment](#29999 (comment)). ### Description of changes ### Description of how you validated changes 1. added more unit tests. 2. added a new integ test 3. I have deployed this in my AWS account ```ts import { App, Stack, CfnParameter, aws_iam as iam, CfnOutput, } from 'aws-cdk-lib'; const app = new App(); const stack = new Stack(app, 'dummy-stack'); const userArn = 'arn:aws:iam::123456789012:user/OthersExternalIamUser'; const userparam = new CfnParameter(stack, 'UserParameter', { default: userArn, }); const imported = iam.User.fromUserArn(stack, 'imported-user', userArn); const imported2 = iam.User.fromUserArn(stack, 'imported-user2', userparam.valueAsString ); new CfnOutput(stack, 'User', { value: imported.principalAccount! }); new CfnOutput(stack, 'User2', { value: imported2.principalAccount! }); ``` And the output is correct: ``` Outputs: dummy-stack.User = 123456789012 dummy-stack.User2 = 123456789012 ``` ### Checklist - [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md) ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

github-actions · 2024-05-09T18:41:30Z

⚠️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.

wbertore added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Apr 29, 2024

github-actions bot added the @aws-cdk/aws-sns Related to Amazon Simple Notification Service label Apr 29, 2024

khushail added the needs-reproduction This issue needs reproduction. label Apr 29, 2024

khushail self-assigned this Apr 29, 2024

khushail added investigating This issue is being investigated and/or work is in progress to resolve the issue. and removed needs-triage This issue or PR still needs to be triaged. needs-reproduction This issue needs reproduction. labels Apr 29, 2024

pahud added p1 effort/medium Medium work item – several days of effort and removed p2 investigating This issue is being investigated and/or work is in progress to resolve the issue. labels Apr 30, 2024

pahud mentioned this issue Apr 30, 2024

fix(iam): fromUserArn returns incorrect principalAccount #30023

Merged

1 task

khushail removed their assignment Apr 30, 2024

This was referenced May 1, 2024

Monthly issue metrics report #30025

Closed

Weekly issue metrics report #30071

Closed

Weekly issue metrics report #30072

Closed

mergify bot closed this as completed in #30023 May 9, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

aws-sns: `Topic.grantPublish(...)` creates identity policy assuming grantee is local to aws account. #29999

aws-sns: `Topic.grantPublish(...)` creates identity policy assuming grantee is local to aws account. #29999

wbertore commented Apr 29, 2024 •

edited

khushail commented Apr 30, 2024 •

edited

pahud commented Apr 30, 2024 •

edited

pahud commented Apr 30, 2024

github-actions bot commented May 9, 2024

aws-sns: Topic.grantPublish(...) creates identity policy assuming grantee is local to aws account. #29999

aws-sns: Topic.grantPublish(...) creates identity policy assuming grantee is local to aws account. #29999

Comments

wbertore commented Apr 29, 2024 • edited

Describe the bug

Expected Behavior

Current Behavior

Reproduction Steps

Possible Solution

Additional Information/Context

CDK CLI Version

Framework Version

Node.js Version

OS

Language

Language Version

Other information

khushail commented Apr 30, 2024 • edited

pahud commented Apr 30, 2024 • edited

pahud commented Apr 30, 2024

github-actions bot commented May 9, 2024

⚠️COMMENT VISIBILITY WARNING⚠️

aws-sns: `Topic.grantPublish(...)` creates identity policy assuming grantee is local to aws account. #29999

aws-sns: `Topic.grantPublish(...)` creates identity policy assuming grantee is local to aws account. #29999

wbertore commented Apr 29, 2024 •

edited

khushail commented Apr 30, 2024 •

edited

pahud commented Apr 30, 2024 •

edited