Fix sns.add_permission & remove_permission

IMPLEMENTATION_COVERAGE.md

@@ -700,6 +700,7 @@
 0% implemented
 - [ ] associate_phone_number_with_user
 - [ ] associate_phone_numbers_with_voice_connector
+- [ ] associate_phone_numbers_with_voice_connector_group
 - [ ] batch_delete_phone_number
 - [ ] batch_suspend_user
 - [ ] batch_unsuspend_user
@@ -709,26 +710,34 @@
 - [ ] create_bot
 - [ ] create_phone_number_order
 - [ ] create_voice_connector
+- [ ] create_voice_connector_group
 - [ ] delete_account
 - [ ] delete_events_configuration
 - [ ] delete_phone_number
 - [ ] delete_voice_connector
+- [ ] delete_voice_connector_group
 - [ ] delete_voice_connector_origination
+- [ ] delete_voice_connector_streaming_configuration
 - [ ] delete_voice_connector_termination
 - [ ] delete_voice_connector_termination_credentials
 - [ ] disassociate_phone_number_from_user
 - [ ] disassociate_phone_numbers_from_voice_connector
+- [ ] disassociate_phone_numbers_from_voice_connector_group
 - [ ] get_account
 - [ ] get_account_settings
 - [ ] get_bot
 - [ ] get_events_configuration
 - [ ] get_global_settings
 - [ ] get_phone_number
 - [ ] get_phone_number_order
+- [ ] get_phone_number_settings
 - [ ] get_user
 - [ ] get_user_settings
 - [ ] get_voice_connector
+- [ ] get_voice_connector_group
+- [ ] get_voice_connector_logging_configuration
 - [ ] get_voice_connector_origination
+- [ ] get_voice_connector_streaming_configuration
 - [ ] get_voice_connector_termination
 - [ ] get_voice_connector_termination_health
 - [ ] invite_users
@@ -737,11 +746,14 @@
 - [ ] list_phone_number_orders
 - [ ] list_phone_numbers
 - [ ] list_users
+- [ ] list_voice_connector_groups
 - [ ] list_voice_connector_termination_credentials
 - [ ] list_voice_connectors
 - [ ] logout_user
 - [ ] put_events_configuration
+- [ ] put_voice_connector_logging_configuration
 - [ ] put_voice_connector_origination
+- [ ] put_voice_connector_streaming_configuration
 - [ ] put_voice_connector_termination
 - [ ] put_voice_connector_termination_credentials
 - [ ] regenerate_security_token
@@ -753,9 +765,11 @@
 - [ ] update_bot
 - [ ] update_global_settings
 - [ ] update_phone_number
+- [ ] update_phone_number_settings
 - [ ] update_user
 - [ ] update_user_settings
 - [ ] update_voice_connector
+- [ ] update_voice_connector_group
 
 ## cloud9
 0% implemented
@@ -1525,6 +1539,10 @@
 - [ ] get_current_metric_data
 - [ ] get_federation_token
 - [ ] get_metric_data
+- [ ] list_contact_flows
+- [ ] list_hours_of_operations
+- [ ] list_phone_numbers
+- [ ] list_queues
 - [ ] list_routing_profiles
 - [ ] list_security_profiles
 - [ ] list_user_hierarchy_groups
@@ -3244,7 +3262,7 @@
 - [ ] describe_events
 
 ## iam
-61% implemented
+60% implemented
 - [ ] add_client_id_to_open_id_connect_provider
 - [X] add_role_to_instance_profile
 - [X] add_user_to_group
@@ -6029,8 +6047,8 @@
 - [ ] update_job
 
 ## sns
-57% implemented
-- [ ] add_permission
+63% implemented
+- [X] add_permission
 - [ ] check_if_phone_number_is_opted_out
 - [ ] confirm_subscription
 - [X] create_platform_application
@@ -6053,7 +6071,7 @@
 - [X] list_topics
 - [ ] opt_in_phone_number
 - [X] publish
-- [ ] remove_permission
+- [X] remove_permission
 - [X] set_endpoint_attributes
 - [ ] set_platform_application_attributes
 - [ ] set_sms_attributes

moto/sns/models.py

@@ -34,7 +34,6 @@ def __init__(self, name, sns_backend):
         self.sns_backend = sns_backend
         self.account_id = DEFAULT_ACCOUNT_ID
         self.display_name = ""
-        self.policy = json.dumps(DEFAULT_TOPIC_POLICY)
         self.delivery_policy = ""
         self.effective_delivery_policy = json.dumps(DEFAULT_EFFECTIVE_DELIVERY_POLICY)
         self.arn = make_arn_for_topic(
@@ -44,6 +43,7 @@ def __init__(self, name, sns_backend):
         self.subscriptions_confimed = 0
         self.subscriptions_deleted = 0
 
+        self._policy_json = self._create_default_topic_policy(sns_backend.region_name, self.account_id, name)
         self._tags = {}
 
     def publish(self, message, subject=None, message_attributes=None):
@@ -64,6 +64,14 @@ def get_cfn_attribute(self, attribute_name):
     def physical_resource_id(self):
         return self.arn
 
+    @property
+    def policy(self):
+        return json.dumps(self._policy_json)
+
+    @policy.setter
+    def policy(self, policy):
+        self._policy_json = json.loads(policy)
+
     @classmethod
     def create_from_cloudformation_json(cls, resource_name, cloudformation_json, region_name):
         sns_backend = sns_backends[region_name]
@@ -77,6 +85,37 @@ def create_from_cloudformation_json(cls, resource_name, cloudformation_json, reg
                                   'Endpoint'], subscription['Protocol'])
         return topic
 
+    def _create_default_topic_policy(self, region_name, account_id, name):
+        return {
+            "Version": "2008-10-17",
+            "Id": "__default_policy_ID",
+            "Statement": [{
+                "Effect": "Allow",
+                "Sid": "__default_statement_ID",
+                "Principal": {
+                    "AWS": "*"
+                },
+                "Action": [
+                    "SNS:GetTopicAttributes",
+                    "SNS:SetTopicAttributes",
+                    "SNS:AddPermission",
+                    "SNS:RemovePermission",
+                    "SNS:DeleteTopic",
+                    "SNS:Subscribe",
+                    "SNS:ListSubscriptionsByTopic",
+                    "SNS:Publish",
+                    "SNS:Receive",
+                ],
+                "Resource": make_arn_for_topic(
+                    self.account_id, name, region_name),
+                "Condition": {
+                    "StringEquals": {
+                        "AWS:SourceOwner": str(account_id)
+                    }
+                }
+            }]
+        }
+
 
 class Subscription(BaseModel):
 
@@ -269,7 +308,6 @@ def __init__(self, region_name):
         self.region_name = region_name
         self.sms_attributes = {}
         self.opt_out_numbers = ['+447420500600', '+447420505401', '+447632960543', '+447632960028', '+447700900149', '+447700900550', '+447700900545', '+447700900907']
-        self.permissions = {}
 
     def reset(self):
         region_name = self.region_name
@@ -511,6 +549,43 @@ def _validate_filter_policy(self, value):
 
                 raise SNSInvalidParameter("Invalid parameter: FilterPolicy: Match value must be String, number, true, false, or null")
 
+    def add_permission(self, topic_arn, label, aws_account_ids, action_names):
+        if topic_arn not in self.topics:
+            raise SNSNotFoundError('Topic does not exist')
+
+        policy = self.topics[topic_arn]._policy_json
+        statement = next((statement for statement in policy['Statement'] if statement['Sid'] == label), None)
+
+        if statement:
+            raise SNSInvalidParameter('Statement already exists')
+
+        if any(action_name not in VALID_POLICY_ACTIONS for action_name in action_names):
+            raise SNSInvalidParameter('Policy statement action out of service scope!')
+
+        principals = ['arn:aws:iam::{}:root'.format(account_id) for account_id in aws_account_ids]
+        actions = ['SNS:{}'.format(action_name) for action_name in action_names]
+
+        statement = {
+            'Sid': label,
+            'Effect': 'Allow',
+            'Principal': {
+                'AWS': principals[0] if len(principals) == 1 else principals
+            },
+            'Action': actions[0] if len(actions) == 1 else actions,
+            'Resource': topic_arn
+        }
+
+        self.topics[topic_arn]._policy_json['Statement'].append(statement)
+
+    def remove_permission(self, topic_arn, label):
+        if topic_arn not in self.topics:
+            raise SNSNotFoundError('Topic does not exist')
+
+        statements = self.topics[topic_arn]._policy_json['Statement']
+        statements = [statement for statement in statements if statement['Sid'] != label]
+
+        self.topics[topic_arn]._policy_json['Statement'] = statements
+
     def list_tags_for_resource(self, resource_arn):
         if resource_arn not in self.topics:
             raise ResourceNotFoundError
@@ -542,35 +617,6 @@ def untag_resource(self, resource_arn, tag_keys):
     sns_backends[region] = SNSBackend(region)
 
 
-DEFAULT_TOPIC_POLICY = {
-    "Version": "2008-10-17",
-    "Id": "us-east-1/698519295917/test__default_policy_ID",
-    "Statement": [{
-        "Effect": "Allow",
-        "Sid": "us-east-1/698519295917/test__default_statement_ID",
-        "Principal": {
-            "AWS": "*"
-        },
-        "Action": [
-            "SNS:GetTopicAttributes",
-            "SNS:SetTopicAttributes",
-            "SNS:AddPermission",
-            "SNS:RemovePermission",
-            "SNS:DeleteTopic",
-            "SNS:Subscribe",
-            "SNS:ListSubscriptionsByTopic",
-            "SNS:Publish",
-            "SNS:Receive",
-        ],
-        "Resource": "arn:aws:sns:us-east-1:698519295917:test",
-        "Condition": {
-            "StringLike": {
-                "AWS:SourceArn": "arn:aws:*:*:698519295917:*"
-            }
-        }
-    }]
-}
-
 DEFAULT_EFFECTIVE_DELIVERY_POLICY = {
     'http': {
         'disableSubscriptionOverrides': False,
@@ -585,3 +631,16 @@ def untag_resource(self, resource_arn, tag_keys):
         }
     }
 }
+
+
+VALID_POLICY_ACTIONS = [
+    'GetTopicAttributes',
+    'SetTopicAttributes',
+    'AddPermission',
+    'RemovePermission',
+    'DeleteTopic',
+    'Subscribe',
+    'ListSubscriptionsByTopic',
+    'Publish',
+    'Receive'
+]

moto/sns/responses.py

@@ -639,34 +639,21 @@ def opt_in_phone_number(self):
         return template.render()
 
     def add_permission(self):
-        arn = self._get_param('TopicArn')
+        topic_arn = self._get_param('TopicArn')
         label = self._get_param('Label')
-        accounts = self._get_multi_param('AWSAccountId.member.')
-        action = self._get_multi_param('ActionName.member.')
+        aws_account_ids = self._get_multi_param('AWSAccountId.member.')
+        action_names = self._get_multi_param('ActionName.member.')
 
-        if arn not in self.backend.topics:
-            error_response = self._error('NotFound', 'Topic does not exist')
-            return error_response, dict(status=404)
-
-        key = (arn, label)
-        self.backend.permissions[key] = {'accounts': accounts, 'action': action}
+        self.backend.add_permission(topic_arn, label, aws_account_ids, action_names)
 
         template = self.response_template(ADD_PERMISSION_TEMPLATE)
         return template.render()
 
     def remove_permission(self):
-        arn = self._get_param('TopicArn')
+        topic_arn = self._get_param('TopicArn')
         label = self._get_param('Label')
 
-        if arn not in self.backend.topics:
-            error_response = self._error('NotFound', 'Topic does not exist')
-            return error_response, dict(status=404)
-
-        try:
-            key = (arn, label)
-            del self.backend.permissions[key]
-        except KeyError:
-            pass
+        self.backend.remove_permission(topic_arn, label)
 
         template = self.response_template(DEL_PERMISSION_TEMPLATE)
         return template.render()

tests/test_sns/test_topics.py

@@ -7,7 +7,7 @@
 
 from boto.exception import BotoServerError
 from moto import mock_sns_deprecated
-from moto.sns.models import DEFAULT_TOPIC_POLICY, DEFAULT_EFFECTIVE_DELIVERY_POLICY, DEFAULT_PAGE_SIZE
+from moto.sns.models import DEFAULT_EFFECTIVE_DELIVERY_POLICY, DEFAULT_PAGE_SIZE
 
 
 @mock_sns_deprecated
@@ -76,7 +76,34 @@ def test_topic_attributes():
         .format(conn.region.name)
     )
     attributes["Owner"].should.equal(123456789012)
-    json.loads(attributes["Policy"]).should.equal(DEFAULT_TOPIC_POLICY)
+    json.loads(attributes["Policy"]).should.equal({
+        "Version": "2008-10-17",
+        "Id": "__default_policy_ID",
+        "Statement": [{
+            "Effect": "Allow",
+            "Sid": "__default_statement_ID",
+            "Principal": {
+                "AWS": "*"
+            },
+            "Action": [
+                "SNS:GetTopicAttributes",
+                "SNS:SetTopicAttributes",
+                "SNS:AddPermission",
+                "SNS:RemovePermission",
+                "SNS:DeleteTopic",
+                "SNS:Subscribe",
+                "SNS:ListSubscriptionsByTopic",
+                "SNS:Publish",
+                "SNS:Receive",
+            ],
+            "Resource": "arn:aws:sns:us-east-1:123456789012:some-topic",
+            "Condition": {
+                "StringEquals": {
+                    "AWS:SourceOwner": "123456789012"
+                }
+            }
+        }]
+    })
     attributes["DisplayName"].should.equal("")
     attributes["SubscriptionsPending"].should.equal(0)
     attributes["SubscriptionsConfirmed"].should.equal(0)
@@ -89,11 +116,11 @@ def test_topic_attributes():
     # i.e. unicode on Python 2 -- u"foobar"
     # and bytes on Python 3 -- b"foobar"
     if six.PY2:
-        policy = {b"foo": b"bar"}
+        policy = json.dumps({b"foo": b"bar"})
         displayname = b"My display name"
         delivery = {b"http": {b"defaultHealthyRetryPolicy": {b"numRetries": 5}}}
     else:
-        policy = {u"foo": u"bar"}
+        policy = json.dumps({u"foo": u"bar"})
         displayname = u"My display name"
         delivery = {u"http": {u"defaultHealthyRetryPolicy": {u"numRetries": 5}}}
     conn.set_topic_attributes(topic_arn, "Policy", policy)
@@ -102,7 +129,7 @@ def test_topic_attributes():
 
     attributes = conn.get_topic_attributes(topic_arn)['GetTopicAttributesResponse'][
         'GetTopicAttributesResult']['Attributes']
-    attributes["Policy"].should.equal("{'foo': 'bar'}")
+    attributes["Policy"].should.equal('{"foo": "bar"}')
     attributes["DisplayName"].should.equal("My display name")
     attributes["DeliveryPolicy"].should.equal(
         "{'http': {'defaultHealthyRetryPolicy': {'numRetries': 5}}}")