aws_sns.topic: grant_publish create invalid access policy for S3 event notifications.

[Kirizan]

Describe the bug

When granting publish permission to an external account, the access policy being created is invalid for use with S3 event notification.

When you use the two stacks in the reproduction steps, the stack in account B will produce the following error:

You will get the following error:

Received response status [FAILED] from custom resource. Message returned: Error: An error occurred (InvalidArgument) when calling the PutBucketNotificationConfiguration operation: Unable to validate the following destination configurations. See the details in CloudWatch Log Stream

The Account B code will deploy properly.

Expected Behavior

This access policy should be created on the SNS topic:

{
  "Version": "2012-10-17",
  "Statement": [
    {
      "Sid": "0",
      "Effect": "Allow",
      "Principal": {
        "AWS": "*"
      },
      "Action": "sns:publish",
      "Resource": "arn:aws:sns:us-east-1:AccountAId:trigger_testing",
      "Condition": {
        "StringEquals": {
          "AWS:SourceAccount": "AccountBId"
        }
      }
    }
  ]
}

Current Behavior

The following access policy is created:

{
  "Version": "2012-10-17",
  "Statement": [
    {
      "Sid": "0",
      "Effect": "Allow",
      "Principal": {
        "AWS": "arn:aws:iam::AccountBId:root"
      },
      "Action": "sns:Publish",
      "Resource": "arn:aws:sns:us-east-1:AccountAId:test_topic"
    }
  ]
}

This policy does not allow the S3 event notification to be created.

Reproduction Steps

Code for AccountA

from aws_cdk import (
  aws_sns as _sns,
  aws_iam as _iam,
  Stack
)
from constructs import Construct

class TestSNSAccountA(Stack):
  def __init__(self, scope: Construct, construct_id: str, account_b_id: str, **kwargs) -> None:
    super().__init__(scope, construct_id, **kwargs)
    
    topic = _sns.Topic(
      self, "TestTopic",
      topic_name="test_topic",
      display_name="test_topic"
    )
    
    topic.grant_publish(_iam.AccountPrincipal(f"{account_b_id}"))

Code for AccountB

from aws_cdk import (
  Arn, ArnComponents, RemovalPolicy,
  aws_sns as _sns,
  aws_s3 as _s3,
  aws_s3_notifications as _s3n,
  aws_iam as _iam,
  Stack
)

from constructs import Construct
class TestS3AccountB(Stack):
  def __init__(self, scope: Construct, construct_id: str, account_a_id: str, **kwargs) -> None:
    super().__init__(scope, construct_id, **kwargs)

    bucket = _s3.Bucket(
      self, "Bucket",
      auto_delete_objects=True,
      removal_policy=RemovalPolicy.DESTROY
    )
    
    topic = _sns.Topic.from_topic_arn(
      self, "CrossAccountTopic",
      Arn.format(ArnComponents(
          partition="aws",
          service="sns",
          region="us-east-1",
          account=account_a_id,
          resource="test_topic",
        )
      )
    )
    
    bucket.add_event_notification(
      _s3.EventType.OBJECT_CREATED,
      _s3n.SnsDestination(topic),
      _s3.NotificationKeyFilter(
        prefix="user/"
      )
    )

Possible Solution

Based on the code here: https://github.com/aws/aws-cdk/blob/0b4ab1d0ba11b3536a2f7b02b537966de6ac0493/packages/aws-cdk-lib/aws-sns/lib/topic-base.ts#L143C1-L150C4

The policy generation needs to be updated to the working format.

To do this manually, remote the line with grant_publish add the following code to the Account A code

    topic.add_to_resource_policy(_iam.PolicyStatement(
      effect=_iam.Effect.ALLOW,
      principals=[_iam.AnyPrincipal()],
      actions=["sns:publish"],
      resources=[
        Arn.format(ArnComponents(
          partition="aws",
          service="sns",
          region="us-east-1",
          account=AccountAId,
          resource="trigger_testing",
        )
        )
      ],
      conditions={
        "StringEquals": {
          "AWS:SourceAccount": "AccountBId"
        }
      }
    ))

Additional Information/Context

No response

CDK CLI Version

2.114.1

Framework Version

No response

Node.js Version

v21.4.0

OS

MacOS 14.2

Language

Python

Language Version

3.11.5

Other information

No response

[pahud]

Yes looks like we still need to do something manually like that for cross-account grant.