aws - policy modify - handle statements without sids

c7n/actions/policy.py

@@ -1,5 +1,6 @@
 # Copyright The Cloud Custodian Authors.
 # SPDX-License-Identifier: Apache-2.0
+import hashlib
 
 from .core import BaseAction
 from c7n import utils
@@ -41,6 +42,11 @@ def remove_statements(match_ids, statements, matched=()):
     return statements, found
 
 
+def statement_id(s):
+    # for statements without a sid, use a checksum for identity
+    return hashlib.sha224(utils.dumps(s, indent=0).encode('utf8')).hexdigest()
+
+
 class ModifyPolicyBase(BaseAction):
     """Action to modify resource IAM policy statements.
 
@@ -97,8 +103,8 @@ def __init__(self, data=None, manager=None):
         self.manager = manager
 
     def add_statements(self, policy_statements):
-        current = {s['Sid']: s for s in policy_statements}
-        additional = {s['Sid']: s for s in self.data.get('add-statements', [])}
+        current = {s.get('Sid', statement_id(s)): s for s in policy_statements}
+        additional = {s.get('Sid', statement_id(s)): s for s in self.data.get('add-statements', [])}
         current.update(additional)
         return list(current.values()), bool(additional)
 

tests/data/placebo/test_sns_modify_add_policy_without_sid/sns.CreateTopic_1.json

@@ -0,0 +1,7 @@
+{
+    "status_code": 200,
+    "data": {
+        "TopicArn": "arn:aws:sns:us-east-1:644160558196:c7n-test-rbp-no-sid",
+        "ResponseMetadata": {}
+    }
+}

tests/data/placebo/test_sns_modify_add_policy_without_sid/sns.DeleteTopic_1.json

@@ -0,0 +1,6 @@
+{
+    "status_code": 200,
+    "data": {
+        "ResponseMetadata": {}
+    }
+}

tests/data/placebo/test_sns_modify_add_policy_without_sid/sns.GetTopicAttributes_1.json

@@ -0,0 +1,16 @@
+{
+    "status_code": 200,
+    "data": {
+        "Attributes": {
+            "Policy": "{\"Version\":\"2012-10-17\",\"Statement\":[{\"Effect\":\"Allow\",\"Principal\":\"*\",\"Action\":\"SNS:Subscribe\",\"Resource\":\"arn:aws:sns:us-east-1:644160558196:c7n-test-rbp-no-sid\"}]}",
+            "Owner": "644160558196",
+            "SubscriptionsPending": "0",
+            "TopicArn": "arn:aws:sns:us-east-1:644160558196:c7n-test-rbp-no-sid",
+            "EffectiveDeliveryPolicy": "{\"http\":{\"defaultHealthyRetryPolicy\":{\"minDelayTarget\":20,\"maxDelayTarget\":20,\"numRetries\":3,\"numMaxDelayRetries\":0,\"numNoDelayRetries\":0,\"numMinDelayRetries\":0,\"backoffFunction\":\"linear\"},\"disableSubscriptionOverrides\":false,\"defaultRequestPolicy\":{\"headerContentType\":\"text/plain; charset=UTF-8\"}}}",
+            "SubscriptionsConfirmed": "0",
+            "DisplayName": "",
+            "SubscriptionsDeleted": "0"
+        },
+        "ResponseMetadata": {}
+    }
+}

tests/data/placebo/test_sns_modify_add_policy_without_sid/sns.GetTopicAttributes_2.json

@@ -0,0 +1,22 @@
+{
+    "status_code": 200,
+    "data": {
+        "Attributes": {
+            "Policy": "{\"Version\":\"2008-10-17\",\"Id\":\"__default_policy_ID\",\"Statement\":[{\"Effect\":\"Allow\",\"Principal\":{\"AWS\":\"*\"},\"Action\":[\"SNS:GetTopicAttributes\",\"SNS:SetTopicAttributes\",\"SNS:AddPermission\",\"SNS:RemovePermission\",\"SNS:DeleteTopic\",\"SNS:Subscribe\",\"SNS:ListSubscriptionsByTopic\",\"SNS:Publish\"],\"Resource\":\"arn:aws:sns:us-east-1:644160558196:test123\",\"Condition\":{\"StringEquals\":{\"AWS:SourceOwner\":\"644160558196\"}}},{\"Sid\":\"AddMe\",\"Effect\":\"Allow\",\"Principal\":\"*\",\"Action\":\"SNS:GetTopicAttributes\",\"Resource\":\"arn:aws:sns:us-east-1:644160558196:test123\"}]}",
+            "LambdaSuccessFeedbackSampleRate": "0",
+            "Owner": "644160558196",
+            "SubscriptionsPending": "0",
+            "KmsMasterKeyId": "arn:aws:kms:us-east-1:644160558196:key/082cd05f-96d1-49f6-a5ac-32093d2cfe38",
+            "TopicArn": "arn:aws:sns:us-east-1:644160558196:test123",
+            "EffectiveDeliveryPolicy": "{\"http\":{\"defaultHealthyRetryPolicy\":{\"minDelayTarget\":20,\"maxDelayTarget\":20,\"numRetries\":3,\"numMaxDelayRetries\":0,\"numNoDelayRetries\":0,\"numMinDelayRetries\":0,\"backoffFunction\":\"linear\"},\"disableSubscriptionOverrides\":false,\"defaultRequestPolicy\":{\"headerContentType\":\"text/plain; charset=UTF-8\"}}}",
+            "FirehoseSuccessFeedbackSampleRate": "0",
+            "SubscriptionsConfirmed": "0",
+            "SQSSuccessFeedbackSampleRate": "0",
+            "HTTPSuccessFeedbackSampleRate": "0",
+            "ApplicationSuccessFeedbackSampleRate": "0",
+            "DisplayName": "",
+            "SubscriptionsDeleted": "0"
+        },
+        "ResponseMetadata": {}
+    }
+}

tests/data/placebo/test_sns_modify_add_policy_without_sid/sns.GetTopicAttributes_3.json

@@ -0,0 +1,16 @@
+{
+    "status_code": 200,
+    "data": {
+        "Attributes": {
+            "Policy": "{\"Version\":\"2012-10-17\",\"Statement\":[{\"Effect\":\"Allow\",\"Principal\":\"*\",\"Action\":\"SNS:Subscribe\",\"Resource\":\"arn:aws:sns:us-east-1:644160558196:c7n-test-rbp-no-sid\"},{\"Sid\":\"AddMe\",\"Effect\":\"Allow\",\"Principal\":\"*\",\"Action\":\"SNS:GetTopicAttributes\",\"Resource\":\"arn:aws:sns:us-east-1:644160558196:c7n-test-rbp-no-sid\"}]}",
+            "Owner": "644160558196",
+            "SubscriptionsPending": "0",
+            "TopicArn": "arn:aws:sns:us-east-1:644160558196:c7n-test-rbp-no-sid",
+            "EffectiveDeliveryPolicy": "{\"http\":{\"defaultHealthyRetryPolicy\":{\"minDelayTarget\":20,\"maxDelayTarget\":20,\"numRetries\":3,\"numMaxDelayRetries\":0,\"numNoDelayRetries\":0,\"numMinDelayRetries\":0,\"backoffFunction\":\"linear\"},\"disableSubscriptionOverrides\":false,\"defaultRequestPolicy\":{\"headerContentType\":\"text/plain; charset=UTF-8\"}}}",
+            "SubscriptionsConfirmed": "0",
+            "DisplayName": "",
+            "SubscriptionsDeleted": "0"
+        },
+        "ResponseMetadata": {}
+    }
+}

tests/data/placebo/test_sns_modify_add_policy_without_sid/sns.ListTopics_1.json

@@ -0,0 +1,14 @@
+{
+    "status_code": 200,
+    "data": {
+        "Topics": [
+            {
+                "TopicArn": "arn:aws:sns:us-east-1:644160558196:c7n-test-rbp-no-sid"
+            },
+            {
+                "TopicArn": "arn:aws:sns:us-east-1:644160558196:test123"
+            }
+        ],
+        "ResponseMetadata": {}
+    }
+}

tests/data/placebo/test_sns_modify_add_policy_without_sid/sns.SetTopicAttributes_1.json

@@ -0,0 +1,6 @@
+{
+    "status_code": 200,
+    "data": {
+        "ResponseMetadata": {}
+    }
+}

tests/data/placebo/test_sns_modify_add_policy_without_sid/sns.SetTopicAttributes_2.json

@@ -0,0 +1,6 @@
+{
+    "status_code": 200,
+    "data": {
+        "ResponseMetadata": {}
+    }
+}

tests/data/placebo/test_sns_modify_add_policy_without_sid/tagging.GetResources_1.json

@@ -0,0 +1,8 @@
+{
+    "status_code": 200,
+    "data": {
+        "PaginationToken": "",
+        "ResourceTagMappingList": [],
+        "ResponseMetadata": {}
+    }
+}

tests/test_sns.py

@@ -472,6 +472,68 @@ def test_sns_modify_add_and_remove_policy(self):
         self.assertTrue("RemoveMe" not in statement_ids)
         self.assertTrue("SpecificAllow" in statement_ids)
 
+    def test_sns_modify_add_policy_without_sid(self):
+        session_factory = self.replay_flight_data("test_sns_modify_add_policy_without_sid")
+        client = session_factory().client("sns")
+        name = "c7n-test-rbp-no-sid"
+        topic_arn = client.create_topic(Name=name)["TopicArn"]
+        self.addCleanup(client.delete_topic, TopicArn=topic_arn)
+
+        client.set_topic_attributes(
+            TopicArn=topic_arn,
+            AttributeName="Policy",
+            AttributeValue=json.dumps(
+                {
+                    "Version": "2012-10-17",
+                    "Statement": [
+                        {
+                            "Effect": "Allow",
+                            "Principal": "*",
+                            "Action": ["SNS:Subscribe"],
+                            "Resource": topic_arn,
+                        }
+                    ],
+                }
+            ),
+        )
+
+        p = self.load_policy(
+            {
+                "name": "sns-modify-add-policy-without-sid",
+                "resource": "sns",
+                "filters": [{"TopicArn": topic_arn}],
+                "actions": [
+                    {
+                        "type": "modify-policy",
+                        "add-statements": [
+                            {
+                                "Sid": "AddMe",
+                                "Effect": "Allow",
+                                "Principal": "*",
+                                "Action": ["SNS:GetTopicAttributes"],
+                                "Resource": topic_arn,
+                            }
+                        ],
+                        "remove-statements": [],
+                    }
+                ],
+            },
+            session_factory=session_factory,
+        )
+
+        resources = p.run()
+        self.assertEqual(len(resources), 1)
+
+        data = json.loads(
+            client.get_topic_attributes(TopicArn=resources[0]["TopicArn"])[
+                "Attributes"
+            ][
+                "Policy"
+            ]
+        )
+        self.assertEqual(len(data.get('Statement')), 2)
+        self.assertTrue("AddMe" in [s.get("Sid") for s in data.get("Statement", ())])
+
     def test_sns_topic_encryption(self):
         session_factory = self.replay_flight_data('test_sns_kms_related_filter_test')
         kms = session_factory().client('kms', region_name='ap-northeast-2')