(aws-s3): bucket.grantRead to an organization principal grants public read access

[ehiggins0]

Describe the bug

When using bucket.grantRead(org), the generated policy allows access to the bucket for all AWS accounts without a condition.

Regression Issue

• Select this option if this issue appears to be a regression.

Last Known Working CDK Version

No response

Expected Behavior

The policy should have a condition:

  "bucketPolicy": {
   "Type": "AWS::S3::BucketPolicy",
   "Properties": {
    "Bucket": {
     "Ref": "bucket"
    },
    "PolicyDocument": {
     "Statement": [
      {
       "Action": [
        "s3:GetBucket*",
        "s3:GetObject*",
        "s3:List*"
       ],
       "Condition": {
           "StringEquals": {
               "aws:PrincipalOrgID": "o-yyyyyyyyyy"
           },
       },
       "Effect": "Allow",
       "Principal": {
        "AWS": "*"
       },
       "Resource": [
        {
         "Fn::GetAtt": [
          "bucket",
          "Arn"
         ]
        },
        {
         "Fn::Join": [
          "",
          [
           {
            "Fn::GetAtt": [
             "bucket",
             "Arn"
            ]
           },
           "/*"
          ]
         ]
        }
       ]
      }
     ],
     "Version": "2012-10-17"
    }
   },
   "Metadata": {
    "aws:cdk:path": "Stack/bucket/Policy/Resource"
   }
  }

Current Behavior

This policy gets generated:

  "bucketPolicy": {
   "Type": "AWS::S3::BucketPolicy",
   "Properties": {
    "Bucket": {
     "Ref": "bucket"
    },
    "PolicyDocument": {
     "Statement": [
      {
       "Action": [
        "s3:GetBucket*",
        "s3:GetObject*",
        "s3:List*"
       ],
       "Condition": {
        "StringEquals": {}
       },
       "Effect": "Allow",
       "Principal": {
        "AWS": "*"
       },
       "Resource": [
        {
         "Fn::GetAtt": [
          "bucket",
          "Arn"
         ]
        },
        {
         "Fn::Join": [
          "",
          [
           {
            "Fn::GetAtt": [
             "bucket",
             "Arn"
            ]
           },
           "/*"
          ]
         ]
        }
       ]
      }
     ],
     "Version": "2012-10-17"
    }
   },
   "Metadata": {
    "aws:cdk:path": "Stack/bucket/Policy/Resource"
   }
  }

Reproduction Steps

    const org = new iam.OrganizationPrincipal(orgName);
    const bucket = new s3.Bucket(this, "bucket", {...});
    bucket.grantRead(org);

Possible Solution

No response

Additional Information/Context

No response

CDK CLI Version

2.150.0

Framework Version

No response

Node.js Version

18.18.2

OS

Ubuntu 24.04

Language

TypeScript

Language Version

No response

Other information

No response

[ehiggins0]

Turns out while trying to come up with a workaround that this is not an issue with aws-s3, but is instead a silent failure when trying to add permissions to iam.OrganizationPrincipal(undefined). It still grants the "*" access, but the condition is not added due to the orgId being undefined. Not sure if this should be addressed as it could cause security issues to deployed S3 resources, but feel free to close if not.

[khushail]

@ehiggins0 , thanks for reporting this. I see that the implementation of iam.OrgranizationPrincipal -

aws-cdk/packages/aws-cdk-lib/aws-iam/lib/principals.ts

Line 607 in add7c6b

export class OrganizationPrincipal extends PrincipalBase {

and this is how grant() is implemented invoked by grantRead(..)

aws-cdk/packages/aws-cdk-lib/aws-s3/lib/bucket.ts

Line 770 in add7c6b

public grantRead(identity: iam.IGrantable, objectsKeyPattern: any = '*') {

aws-cdk/packages/aws-cdk-lib/aws-iam/lib/principals.ts

Line 627 in add7c6b

public dedupeString(): string | undefined {

aws-cdk/packages/aws-cdk-lib/aws-s3/lib/bucket.ts

Line 968 in add7c6b

private grant(

Looks like there should be a check added before granting access to * in case of organization principal.