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,15 +710,19 @@
 - [ ] 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
@@ -725,10 +730,14 @@
 - [ ] 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
+63% implemented
-- [ ] add_permission
+- [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 @@ class Topic(BaseModel):
         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 @@ class Topic(BaseModel):
         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 @@ class Topic(BaseModel):
     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 @@ class Topic(BaseModel):
                                   '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 @@ class SNSBackend(BaseBackend):
         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 @@ class SNSBackend(BaseBackend):
 
                 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 @@ for region in Session().get_available_regions('sns'):
     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 @@ DEFAULT_EFFECTIVE_DELIVERY_POLICY = {
         }
     }
 }
+
+
+VALID_POLICY_ACTIONS = [
+    'GetTopicAttributes',
+    'SetTopicAttributes',
+    'AddPermission',
+    'RemovePermission',
+    'DeleteTopic',
+    'Subscribe',
+    'ListSubscriptionsByTopic',
+    'Publish',
+    'Receive'
+]

moto/sns/responses.py

@@ -639,34 +639,21 @@ class SNSResponse(BaseResponse):
         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.')
+        aws_account_ids = self._get_multi_param('AWSAccountId.member.')
-        action = self._get_multi_param('ActionName.member.')
+        action_names = self._get_multi_param('ActionName.member.')
 
-        if arn not in self.backend.topics:
+        self.backend.add_permission(topic_arn, label, aws_account_ids, action_names)
-            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}
 
         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:
+        self.backend.remove_permission(topic_arn, label)
-            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
 
         template = self.response_template(DEL_PERMISSION_TEMPLATE)
         return template.render()

tests/test_sns/test_topics.py

@@ -7,7 +7,7 @@ import sure  # noqa
 
 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}}}")

tests/test_sns/test_topics_boto3.py

@@ -7,7 +7,7 @@ import sure  # noqa
 
 from botocore.exceptions import ClientError
 from moto import mock_sns
-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
@@ -156,7 +156,34 @@ def test_topic_attributes():
         .format(conn._client_config.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')
@@ -217,18 +244,190 @@ def test_topic_paging():
 
 @mock_sns
 def test_add_remove_permissions():
-    conn = boto3.client('sns', region_name='us-east-1')
+    client = boto3.client('sns', region_name='us-east-1')
-    response = conn.create_topic(Name='testpermissions')
+    topic_arn = client.create_topic(Name='test-permissions')['TopicArn']
 
-    conn.add_permission(
+    client.add_permission(
-        TopicArn=response['TopicArn'],
+        TopicArn=topic_arn,
-        Label='Test1234',
+        Label='test',
+        AWSAccountId=['999999999999'],
+        ActionName=['Publish']
+    )
+
+    response = client.get_topic_attributes(TopicArn=topic_arn)
+    json.loads(response['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:test-permissions',
+                'Condition': {
+                    'StringEquals': {
+                        'AWS:SourceOwner': '123456789012'
+                    }
+                }
+            },
+            {
+                'Sid': 'test',
+                'Effect': 'Allow',
+                'Principal': {
+                    'AWS': 'arn:aws:iam::999999999999:root'
+                },
+                'Action': 'SNS:Publish',
+                'Resource': 'arn:aws:sns:us-east-1:123456789012:test-permissions'
+            }
+        ]
+    })
+
+    client.remove_permission(
+        TopicArn=topic_arn,
+        Label='test'
+    )
+
+    response = client.get_topic_attributes(TopicArn=topic_arn)
+    json.loads(response['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:test-permissions',
+                'Condition': {
+                    'StringEquals': {
+                        'AWS:SourceOwner': '123456789012'
+                    }
+                }
+            }
+        ]
+    })
+
+    client.add_permission(
+        TopicArn=topic_arn,
+        Label='test',
+        AWSAccountId=[
+            '888888888888',
+            '999999999999'
+        ],
+        ActionName=[
+            'Publish',
+            'Subscribe'
+        ]
+    )
+
+    response = client.get_topic_attributes(TopicArn=topic_arn)
+    json.loads(response['Attributes']['Policy'])['Statement'][1].should.equal({
+        'Sid': 'test',
+        'Effect': 'Allow',
+        'Principal': {
+            'AWS': [
+                'arn:aws:iam::888888888888:root',
+                'arn:aws:iam::999999999999:root'
+            ]
+        },
+        'Action': [
+            'SNS:Publish',
+            'SNS:Subscribe'
+        ],
+        'Resource': 'arn:aws:sns:us-east-1:123456789012:test-permissions'
+    })
+
+    # deleting non existing permission should be successful
+    client.remove_permission(
+        TopicArn=topic_arn,
+        Label='non-existing'
+    )
+
+
+@mock_sns
+def test_add_permission_errors():
+    client = boto3.client('sns', region_name='us-east-1')
+    topic_arn = client.create_topic(Name='test-permissions')['TopicArn']
+    client.add_permission(
+        TopicArn=topic_arn,
+        Label='test',
+        AWSAccountId=['999999999999'],
+        ActionName=['Publish']
+    )
+
+    client.add_permission.when.called_with(
+        TopicArn=topic_arn,
+        Label='test',
         AWSAccountId=['999999999999'],
         ActionName=['AddPermission']
+    ).should.throw(
+        ClientError,
+        'Statement already exists'
     )
-    conn.remove_permission(
+
-        TopicArn=response['TopicArn'],
+    client.add_permission.when.called_with(
-        Label='Test1234'
+        TopicArn=topic_arn + '-not-existing',
+        Label='test-2',
+        AWSAccountId=['999999999999'],
+        ActionName=['AddPermission']
+    ).should.throw(
+        ClientError,
+        'Topic does not exist'
+    )
+
+    client.add_permission.when.called_with(
+        TopicArn=topic_arn,
+        Label='test-2',
+        AWSAccountId=['999999999999'],
+        ActionName=['NotExistingAction']
+    ).should.throw(
+        ClientError,
+        'Policy statement action out of service scope!'
+    )
+
+
+@mock_sns
+def test_remove_permission_errors():
+    client = boto3.client('sns', region_name='us-east-1')
+    topic_arn = client.create_topic(Name='test-permissions')['TopicArn']
+    client.add_permission(
+        TopicArn=topic_arn,
+        Label='test',
+        AWSAccountId=['999999999999'],
+        ActionName=['Publish']
+    )
+
+    client.remove_permission.when.called_with(
+        TopicArn=topic_arn + '-not-existing',
+        Label='test',
+    ).should.throw(
+        ClientError,
+        'Topic does not exist'
     )