Fix Boolean property parsing

CloudFormation's non-standard YAML parser converts Boolean literal `false` to
string value `"false"` (which is actually truthy). Work around by doing our own
parsing for Boolean properties.

Fixes #10.

CHANGELOG.md

@@ -21,6 +21,11 @@
 * Support using the [`Region`](README.md#region) property to provision an Amazon SES 
   domain in a different region from where you're running your CloudFormation stack.
   (Thanks to @gfodor.)
+
+* Fix incorrect handling of `EnableSend: false` and other potential problems with
+  Boolean properties, by working around CloudFormation's non-standard YAML parsing. 
+  (Thanks to @aajtodd.)
+
 
 ### Features
 

aws_cfn_ses_domain/ses_domain_identity.py

@@ -8,8 +8,7 @@
 from botocore.exceptions import BotoCoreError, ClientError
 
 from .cfnresponse import FAILED, SUCCESS, send
-from .utils import format_arn
-
+from .utils import format_arn, to_bool
 
 logger = logging.getLogger()
 logger.setLevel(os.getenv("LOG_LEVEL", "WARNING"))
@@ -24,6 +23,7 @@
     "TTL": "1800",
     "Region": os.getenv("AWS_REGION"),  # where the stack (lambda fn) is running
 }
+BOOLEAN_PROPERTIES = ("EnableSend", "EnableReceive")
 
 
 def handle_domain_identity_request(event, context):
@@ -52,6 +52,17 @@ def handle_domain_identity_request(event, context):
         resource_type="identity", resource_name=domain,
         defaults_from=event["StackId"])  # current stack's ARN has account and partition
 
+    for prop in BOOLEAN_PROPERTIES:
+        # CloudFormation may convert YAML/JSON bools to strings, so reverse that
+        # https://github.com/medmunds/aws-cfn-ses-domain/issues/10
+        try:
+            properties[prop] = to_bool(properties[prop])
+        except ValueError:
+            return send(event, context, FAILED,
+                        reason=f"The '{prop}' property must be 'true' or 'false',"
+                               f" not '{properties[prop]}'.",
+                        physical_resource_id=domain_arn)
+
     if event["RequestType"] == "Delete" and event["PhysicalResourceId"] == domain:
         # v0.3 backwards compatibility:
         # Earlier versions used just the domain as the PhysicalResourceId.

aws_cfn_ses_domain/utils.py

@@ -25,3 +25,34 @@ def format_arn(partition=None, service=None, region=None, account=None,
         resource = resource if resource is not None else _resource
 
     return f"arn:{partition}:{service}:{region}:{account}:{resource}"
+
+
+def to_bool(val):
+    """Convert val to True or False.
+
+    Converts 'true' (case-insensitive) and 1, '1', or True to True.
+    Converts 'false', 'null' or 'none' (case-insensitive), the empty string '',
+    and 0, '0', or False to False.
+    Raises a ValueError for any other input.
+
+    >>> to_bool('true')
+    True
+    >>> to_bool('False')
+    False
+    >>> to_bool(0)
+    False
+    >>> to_bool('0')
+    False
+    >>> to_bool(None)
+    False
+    >>> to_bool('yes')
+    ValueError("Invalid boolean value 'yes'")
+    """
+    # (Loosely adapted from distutils.util.strtobool)
+    strval = str(val).lower()
+    if strval in ('true', '1'):
+        return True
+    elif strval in ('false', '0', 'null', 'none', ''):
+        return False
+    else:
+        raise ValueError(f"Invalid boolean value {val!r}")

tests/base.py

@@ -1,4 +1,4 @@
-from unittest.case import TestCase
+from unittest import TestCase
 from unittest.mock import patch, ANY as MOCK_ANY
 
 import boto3

tests/test_ses_domain_identity.py

@@ -98,8 +98,8 @@ def test_create_all_options(self):
             "RequestType": "Create",
             "ResourceProperties": {
                 "Domain": "example.com.",
-                "EnableSend": True,
-                "EnableReceive": True,
+                "EnableSend": "true",
+                "EnableReceive": "true",
                 "MailFromSubdomain": "bounce",
                 "CustomDMARC": '"v=DMARC1; p=quarantine; rua=mailto:[email protected];"',
                 "TTL": "300",
@@ -166,8 +166,8 @@ def test_update_receive_only(self):
             "RequestType": "Update",
             "ResourceProperties": {
                 "Domain": "example.com.",
-                "EnableSend": False,
-                "EnableReceive": True,
+                "EnableSend": "false",
+                "EnableReceive": "true",
                 "CustomDMARC": None,
             },
             "StackId": self.mock_stack_id}
@@ -208,8 +208,8 @@ def test_delete(self):
             "PhysicalResourceId": "arn:aws:ses:mock-region:111111111111:identity/example.com",
             "ResourceProperties": {
                 "Domain": "example.com.",
-                "EnableSend": True,
-                "EnableReceive": True,
+                "EnableSend": "true",
+                "EnableReceive": "true",
             },
             "StackId": self.mock_stack_id}
         self.ses_stubber.add_response(
@@ -244,8 +244,8 @@ def test_v0_3_physical_id_change(self):
             "PhysicalResourceId": "example.com",  # old format: just the domain
             "ResourceProperties": {
                 "Domain": "example.com.",
-                "EnableSend": True,
-                "EnableReceive": True,
+                "EnableSend": "true",
+                "EnableReceive": "true",
             },
             "StackId": self.mock_stack_id}
         # self.ses_stubber.nothing: *no* SES ops should occur
@@ -280,3 +280,17 @@ def test_boto_error(self):
             'ERROR:root:Error updating SES: An error occurred (InvalidParameterValue) when'
             ' calling the VerifyDomainIdentity operation: Invalid domain name bad domain name.',
             cm.output[0])
+
+    def test_invalid_boolean_property(self):
+        event = {
+            "RequestType": "Create",
+            "ResourceProperties": {
+                "Domain": "example.com",
+                "EnableSend": "yes",
+            },
+            "StackId": self.mock_stack_id}
+        handle_domain_identity_request(event, self.mock_context)
+        self.assertSentResponse(
+            event, status="FAILED",
+            reason="The 'EnableSend' property must be 'true' or 'false', not 'yes'.",
+            physical_resource_id=MOCK_ANY)

tests/test_utils.py

@@ -0,0 +1,41 @@
+from unittest import TestCase
+
+from aws_cfn_ses_domain.utils import to_bool
+
+
+class TestToBool(TestCase):
+    TRUE_VALUES = (
+        'true', 'True', 'TRUE', 'tRuE',
+        1, '1',
+        True,
+    )
+
+    FALSE_VALUES = (
+        'false', 'False', 'FALSE', 'fAlSe',
+        0, '0',
+        None, 'None',
+        'null',  # JSON's None as a string
+        '',  # empty string
+        False,
+    )
+
+    INVALID_VALUES = (
+        'yes', 'no', 't', 'f', ' ',
+        100, -1, 0.5,
+    )
+
+    def test_true(self):
+        for value in self.TRUE_VALUES:
+            with self.subTest(value=value):
+                self.assertIs(to_bool(value), True)
+
+    def test_false(self):
+        for value in self.FALSE_VALUES:
+            with self.subTest(value=value):
+                self.assertIs(to_bool(value), False)
+
+    def test_invalid(self):
+        for value in self.INVALID_VALUES:
+            with self.subTest(value=value):
+                with self.assertRaises(ValueError):
+                    to_bool(value)