Merge pull request #2538 from edekadigital/fix-sns-subscribe-sms-validation

Fix sns.subscribe validation for sms numbers

moto/sns/models.py

@@ -29,7 +29,7 @@ from .exceptions import (
     ResourceNotFoundError,
     TagLimitExceededError,
 )
-from .utils import make_arn_for_topic, make_arn_for_subscription
+from .utils import make_arn_for_topic, make_arn_for_subscription, is_e164
 
 DEFAULT_ACCOUNT_ID = 123456789012
 DEFAULT_PAGE_SIZE = 100
@@ -413,6 +413,17 @@ class SNSBackend(BaseBackend):
         setattr(topic, attribute_name, attribute_value)
 
     def subscribe(self, topic_arn, endpoint, protocol):
+        if protocol == "sms":
+            if re.search(r"[./-]{2,}", endpoint) or re.search(
+                r"(^[./-]|[./-]$)", endpoint
+            ):
+                raise SNSInvalidParameter("Invalid SMS endpoint: {}".format(endpoint))
+
+            reduced_endpoint = re.sub(r"[./-]", "", endpoint)
+
+            if not is_e164(reduced_endpoint):
+                raise SNSInvalidParameter("Invalid SMS endpoint: {}".format(endpoint))
+
         # AWS doesn't create duplicates
         old_subscription = self._find_subscription(topic_arn, endpoint, protocol)
         if old_subscription:

moto/sns/responses.py

@@ -211,14 +211,6 @@ class SNSResponse(BaseResponse):
         protocol = self._get_param("Protocol")
         attributes = self._get_attributes()
 
-        if protocol == "sms" and not is_e164(endpoint):
-            return (
-                self._error(
-                    "InvalidParameter", "Phone number does not meet the E164 format"
-                ),
-                dict(status=400),
-            )
-
         subscription = self.backend.subscribe(topic_arn, endpoint, protocol)
 
         if attributes is not None:

tests/test_sns/test_subscriptions_boto3.py

@@ -19,7 +19,10 @@ def test_subscribe_sms():
     arn = resp["TopicArn"]
 
     resp = client.subscribe(TopicArn=arn, Protocol="sms", Endpoint="+15551234567")
-    resp.should.contain("SubscriptionArn")
+    resp.should.have.key("SubscriptionArn")
+
+    resp = client.subscribe(TopicArn=arn, Protocol="sms", Endpoint="+15/55-123.4567")
+    resp.should.have.key("SubscriptionArn")
 
 
 @mock_sns
@@ -51,6 +54,18 @@ def test_subscribe_bad_sms():
     except ClientError as err:
         err.response["Error"]["Code"].should.equal("InvalidParameter")
 
+    client.subscribe.when.called_with(
+        TopicArn=arn, Protocol="sms", Endpoint="+15--551234567"
+    ).should.throw(ClientError, "Invalid SMS endpoint: +15--551234567")
+
+    client.subscribe.when.called_with(
+        TopicArn=arn, Protocol="sms", Endpoint="+15551234567."
+    ).should.throw(ClientError, "Invalid SMS endpoint: +15551234567.")
+
+    client.subscribe.when.called_with(
+        TopicArn=arn, Protocol="sms", Endpoint="/+15551234567"
+    ).should.throw(ClientError, "Invalid SMS endpoint: /+15551234567")
+
 
 @mock_sns
 def test_creating_subscription():