fix: Fail when Intrinsics are in SourceVPC list instead of IntrinsicsSourceVPC (#1999)

Author: jfuss

samtranslator/swagger/swagger.py

@@ -892,6 +892,8 @@ def add_resource_policy(self, resource_policy, path, api_id, stage):
         ip_range_blacklist = resource_policy.get("IpRangeBlacklist")
         source_vpc_whitelist = resource_policy.get("SourceVpcWhitelist")
         source_vpc_blacklist = resource_policy.get("SourceVpcBlacklist")
+
+        # Intrinsic's supported in these properties
         source_vpc_intrinsic_whitelist = resource_policy.get("IntrinsicVpcWhitelist")
         source_vpce_intrinsic_whitelist = resource_policy.get("IntrinsicVpceWhitelist")
         source_vpc_intrinsic_blacklist = resource_policy.get("IntrinsicVpcBlacklist")
@@ -913,31 +915,38 @@ def add_resource_policy(self, resource_policy, path, api_id, stage):
             resource_list = self._get_method_path_uri_list(path, api_id, stage)
             self._add_ip_resource_policy_for_method(ip_range_blacklist, "IpAddress", resource_list)
 
-        if (
-            (source_vpc_blacklist is not None)
-            or (source_vpc_intrinsic_blacklist is not None)
-            or (source_vpce_intrinsic_blacklist is not None)
-        ):
-            blacklist_dict = {
-                "StringEndpointList": source_vpc_blacklist,
-                "IntrinsicVpcList": source_vpc_intrinsic_blacklist,
-                "IntrinsicVpceList": source_vpce_intrinsic_blacklist,
-            }
-            resource_list = self._get_method_path_uri_list(path, api_id, stage)
-            self._add_vpc_resource_policy_for_method(blacklist_dict, "StringEquals", resource_list)
+        if not SwaggerEditor._validate_list_property_is_resolved(source_vpc_blacklist):
+            raise InvalidDocumentException(
+                [
+                    InvalidTemplateException(
+                        "SourceVpcBlacklist must be a list of strings. Use IntrinsicVpcBlacklist instead for values that use Intrinsic Functions"
+                    )
+                ]
+            )
 
-        if (
-            (source_vpc_whitelist is not None)
-            or (source_vpc_intrinsic_whitelist is not None)
-            or (source_vpce_intrinsic_whitelist is not None)
-        ):
-            whitelist_dict = {
-                "StringEndpointList": source_vpc_whitelist,
-                "IntrinsicVpcList": source_vpc_intrinsic_whitelist,
-                "IntrinsicVpceList": source_vpce_intrinsic_whitelist,
-            }
-            resource_list = self._get_method_path_uri_list(path, api_id, stage)
-            self._add_vpc_resource_policy_for_method(whitelist_dict, "StringNotEquals", resource_list)
+        blacklist_dict = {
+            "StringEndpointList": source_vpc_blacklist,
+            "IntrinsicVpcList": source_vpc_intrinsic_blacklist,
+            "IntrinsicVpceList": source_vpce_intrinsic_blacklist,
+        }
+        resource_list = self._get_method_path_uri_list(path, api_id, stage)
+        self._add_vpc_resource_policy_for_method(blacklist_dict, "StringEquals", resource_list)
+
+        if not SwaggerEditor._validate_list_property_is_resolved(source_vpc_whitelist):
+            raise InvalidDocumentException(
+                [
+                    InvalidTemplateException(
+                        "SourceVpcWhitelist must be a list of strings. Use IntrinsicVpcWhitelist instead for values that use Intrinsic Functions"
+                    )
+                ]
+            )
+
+        whitelist_dict = {
+            "StringEndpointList": source_vpc_whitelist,
+            "IntrinsicVpcList": source_vpc_intrinsic_whitelist,
+            "IntrinsicVpceList": source_vpce_intrinsic_whitelist,
+        }
+        self._add_vpc_resource_policy_for_method(whitelist_dict, "StringNotEquals", resource_list)
 
         self._doc[self._X_APIGW_POLICY] = self.resource_policy
 
@@ -1268,3 +1277,17 @@ def safe_compare_regex_with_string(regex, data):
     def get_path_without_trailing_slash(path):
         # convert greedy paths to such as {greedy+}, {proxy+} to "*"
         return re.sub(r"{([a-zA-Z0-9._-]+|[a-zA-Z0-9._-]+\+|proxy\+)}", "*", path)
+
+    @staticmethod
+    def _validate_list_property_is_resolved(property_list):
+        """
+        Validate if the values of a Property List are all of type string
+
+        :param property_list: Value of a Property List
+        :return bool: True if the property_list is all of type string otherwise False
+        """
+
+        if property_list is not None and not all(isinstance(x, string_types) for x in property_list):
+            return False
+
+        return True

tests/swagger/test_swagger.py

@@ -1181,6 +1181,18 @@ def test_must_add_vpc_allow_string_only(self):
 
         self.assertEqual(deep_sort_lists(expected), deep_sort_lists(self.editor.swagger[_X_POLICY]))
 
+    @parameterized.expand(
+        [
+            param("SourceVpcWhitelist"),
+            param("SourceVpcBlacklist"),
+        ]
+    )
+    def test_must_fail_when_vpc_whitelist_is_non_string(self, resource_policy_key):
+        resource_policy = {resource_policy_key: [{"sub": "somevalue"}]}
+
+        with self.assertRaises(InvalidDocumentException):
+            self.editor.add_resource_policy(resource_policy, "/foo", "123", "prod")
+
     def test_must_add_vpc_deny_string_only(self):
 
         resourcePolicy = {

tests/translator/input/error_api_invalid_source_vpc_blacklist.yaml

@@ -0,0 +1,31 @@
+Globals:
+  Api:
+    Auth:
+      ResourcePolicy:
+        SourceVpcBlacklist: [{"Ref":"SomeParameter"}]
+
+Parameters:
+  SomeParameter:
+    Type: String
+    Default: param
+
+Resources:
+  MyFunction:
+    Type: AWS::Serverless::Function
+    Properties:
+      InlineCode: |
+        exports.handler = async (event) => {
+          const response = {
+            statusCode: 200,
+            body: JSON.stringify('Hello from Lambda!'),
+          };
+          return response;
+        };
+      Handler: index.handler
+      Runtime: nodejs12.x
+      Events:
+        Api:
+          Type: Api
+          Properties:
+            Method: Put
+            Path: /get

tests/translator/input/error_api_invalid_source_vpc_whitelist.yaml

@@ -0,0 +1,31 @@
+Globals:
+  Api:
+    Auth:
+      ResourcePolicy:
+        SourceVpcWhitelist: [{"Ref":"SomeParameter"}]
+
+Parameters:
+  SomeParameter:
+    Type: String
+    Default: param
+
+Resources:
+  MyFunction:
+    Type: AWS::Serverless::Function
+    Properties:
+      InlineCode: |
+        exports.handler = async (event) => {
+          const response = {
+            statusCode: 200,
+            body: JSON.stringify('Hello from Lambda!'),
+          };
+          return response;
+        };
+      Handler: index.handler
+      Runtime: nodejs12.x
+      Events:
+        Api:
+          Type: Api
+          Properties:
+            Method: Put
+            Path: /get

tests/translator/output/error_api_invalid_source_vpc_blacklist.json

@@ -0,0 +1,3 @@
+{
+  "errorMessage": "Invalid Serverless Application Specification document. Number of errors found: 1. SourceVpcBlacklist must be a list of strings. Use IntrinsicVpcBlacklist instead for values that use Intrinsic Functions"
+}

tests/translator/output/error_api_invalid_source_vpc_whitelist.json

@@ -0,0 +1,3 @@
+{
+  "errorMessage": "Invalid Serverless Application Specification document. Number of errors found: 1. SourceVpcWhitelist must be a list of strings. Use IntrinsicVpcWhitelist instead for values that use Intrinsic Functions"
+}