fix: Crash when using an invalid method in open api (aws#2001)

When customers use auth and define an invalid method in the open api definition, SAM would return a 'server error'. This was actually due to SAM attempting to get the method from the path. If the method was not a supported method and non-lowercase, SAM would attempt to fetch the lower case method and crash with a KeyError. This PR addresses that by checking for the valid methods supported. Co-authored-by: Jacob Fuss <[email protected]>
qingchm · May 13, 2021 · 6cc9add · 6cc9add
1 parent a1fe09b
commit 6cc9add
Show file tree

Hide file tree

Showing 6 changed files with 100 additions and 2 deletions.
diff --git a/samtranslator/swagger/swagger.py b/samtranslator/swagger/swagger.py
@@ -531,6 +531,19 @@ def set_path_default_authorizer(
             if add_default_auth_to_preflight or normalized_method_name != "options":
                 normalized_method_name = self._normalize_method_name(method_name)
                 # It is possible that the method could have two definitions in a Fn::If block.
+
+                # check for valid methods
+                if normalized_method_name.upper() not in self._ALL_HTTP_METHODS:
+                    raise InvalidDocumentException(
+                        [
+                            InvalidTemplateException(
+                                "Path '{}' contains method '{}' which is not a supported method {}".format(
+                                    path, method_name, self._ALL_HTTP_METHODS
+                                )
+                            )
+                        ]
+                    )
+
                 for method_definition in self.get_method_contents(self.get_path(path)[normalized_method_name]):
 
                     # If no integration given, then we don't need to process this definition (could be AWS::NoValue)

diff --git a/tests/swagger/test_swagger.py b/tests/swagger/test_swagger.py
@@ -1456,3 +1456,39 @@ def test_should_include_none_if_default_is_overwritte(self):
 
         self.editor.add_auth_to_method("/cognito", "get", auth, self.api)
         self.assertEqual([{"NONE": []}], self.editor.swagger["paths"]["/cognito"]["get"]["security"])
+
+
+class TestSwaggerEditor_set_path_default_authorizer(TestCase):
+    def setUp(self):
+        self.api = api = {
+            "Auth": {
+                "Authorizers": {"MyOtherCognitoAuth": {}, "MyCognitoAuth": {}},
+                "DefaultAuthorizer": "MyCognitoAuth",
+            }
+        }
+        self.editor = SwaggerEditor(
+            {
+                "swagger": "2.0",
+                "paths": {
+                    "/cognito": {
+                        "nonMethod": {
+                            "x-amazon-apigateway-integration": {
+                                "httpMethod": "POST",
+                                "type": "aws_proxy",
+                                "uri": {
+                                    "Fn::Sub": "arn:aws:apigateway:${AWS::Region}:lambda:path/2015-03-31/functions/${MyFn.Arn}/invocations"
+                                },
+                            },
+                            "security": [],
+                            "responses": {},
+                        }
+                    }
+                },
+            }
+        )
+
+    def test_should_fail_when_path_methods_are_invalid(self):
+        with self.assertRaises(InvalidDocumentException):
+            self.editor.set_path_default_authorizer(
+                "/cognito", "MyCognitoAuth", {"MyOtherCognitoAuth": {}, "MyCognitoAuth": {}}
+            )
diff --git a/tests/translator/input/error_api_with_invalid_path_object.yaml b/tests/translator/input/error_api_with_invalid_path_object.yaml
@@ -0,0 +1,47 @@
+Globals:
+  Api:
+    Name: "some api"
+    Variables:
+      SomeVar: Value
+    Auth:
+      DefaultAuthorizer: MyCognitoAuth
+      Authorizers:
+        MyCognitoAuth:
+          UserPoolArn: !GetAtt MyUserPool.Arn
+
+Resources:
+  ImplicitApiFunction:
+    Type: AWS::Serverless::Function
+    Properties:
+      CodeUri: s3://sam-demo-bucket/member_portal.zip
+      Handler: index.gethtml
+      Runtime: nodejs12.x
+
+  ExplicitApi:
+    Type: AWS::Serverless::Api
+    Properties:
+      StageName: SomeStage
+      DefinitionBody:
+        swagger: 2.0
+        paths:
+          "/a":
+            SomeInvalidKey:
+              x-amazon-apigateway-integration:
+                httpMethod: POST
+                type: aws_proxy
+                uri: !Sub arn:aws:apigateway:${AWS::Region}:lambda:path/2015-03-31/functions/${ImplicitApiFunction.Arn}/invocations
+              responses: {}
+
+  MyUserPool:
+    Type: AWS::Cognito::UserPool
+    Properties:
+      UserPoolName: UserPoolName
+      Policies:
+        PasswordPolicy:
+          MinimumLength: 8
+      UsernameAttributes:
+        - email
+      Schema:
+        - AttributeDataType: String
+          Name: email
+          Required: false
diff --git a/tests/translator/input/error_invalid_method_definition.yaml b/tests/translator/input/error_invalid_method_definition.yaml
@@ -42,7 +42,7 @@ Resources:
                 description: Application domain
                 type: string
                 required: true
-            tags:
+            options:
               - InvalidMethodDefinition
             get:
               x-amazon-apigateway-integration:

diff --git a/tests/translator/output/error_api_with_invalid_path_object.json b/tests/translator/output/error_api_with_invalid_path_object.json
@@ -0,0 +1,3 @@
+{
+  "errorMessage": "Invalid Serverless Application Specification document. Number of errors found: 1. Path '/a' contains method 'SomeInvalidKey' which is not a supported method ['OPTIONS', 'GET', 'HEAD', 'POST', 'PUT', 'DELETE', 'PATCH']"
+}
diff --git a/tests/translator/test_translator.py b/tests/translator/test_translator.py
@@ -413,7 +413,6 @@ def test_transform_success(self, testcase, partition_with_region):
             ],  # Run all the above tests against each of the list of partitions to test against
         )
     )
-    @pytest.mark.slow
     @patch(
         "samtranslator.plugins.application.serverless_app_plugin.ServerlessAppPlugin._sar_service_call",
         mock_sar_service_call,