fix: Defining CORS when ApiKeyRequired is true results in an OPTIONS …

…method that requires an API key (#2981) Co-authored-by: Christoffer Rehn <[email protected]>
aws · Mar 10, 2023 · 089859e · 089859e
1 parent d6c0468
commit 089859e
Show file tree

Hide file tree

Showing 16 changed files with 1,812 additions and 5 deletions.
diff --git a/integration/combination/test_api_with_cors.py b/integration/combination/test_api_with_cors.py
@@ -15,6 +15,7 @@ class TestApiWithCors(BaseTest):
         [
             "combination/api_with_cors",
             "combination/api_with_cors_openapi",
+            "combination/api_with_cors_and_apikey",
         ]
     )
     def test_cors(self, file_name):

diff --git a/integration/resources/expected/combination/api_with_cors_and_apikey.json b/integration/resources/expected/combination/api_with_cors_and_apikey.json
@@ -0,0 +1,26 @@
+[
+  {
+    "LogicalResourceId": "MyApi",
+    "ResourceType": "AWS::ApiGateway::RestApi"
+  },
+  {
+    "LogicalResourceId": "MyApiDeployment",
+    "ResourceType": "AWS::ApiGateway::Deployment"
+  },
+  {
+    "LogicalResourceId": "MyApidevStage",
+    "ResourceType": "AWS::ApiGateway::Stage"
+  },
+  {
+    "LogicalResourceId": "ApiGatewayLambdaRole",
+    "ResourceType": "AWS::IAM::Role"
+  },
+  {
+    "LogicalResourceId": "MyFunction",
+    "ResourceType": "AWS::Lambda::Function"
+  },
+  {
+    "LogicalResourceId": "MyFunctionRole",
+    "ResourceType": "AWS::IAM::Role"
+  }
+]
diff --git a/integration/resources/templates/combination/api_with_cors_and_apikey.yaml b/integration/resources/templates/combination/api_with_cors_and_apikey.yaml
@@ -0,0 +1,87 @@
+AWSTemplateFormatVersion: '2010-09-09'
+
+Transform:
+- AWS::Serverless-2016-10-31
+
+Globals:
+  Api:
+    Auth:
+      ApiKeyRequired: true
+      AddApiKeyRequiredToCorsPreflight: false
+
+Resources:
+
+  MyFunction:
+    Type: AWS::Serverless::Function
+    Properties:
+      Handler: index.handler
+      InlineCode: |
+        exports.handler = async function (event) {
+          return {
+            statusCode: 200,
+            body: JSON.stringify({ message: "Hello, SAM!" }),
+          }
+        }
+      Runtime: nodejs16.x
+
+  ApiGatewayLambdaRole:
+    Type: AWS::IAM::Role
+    Properties:
+      AssumeRolePolicyDocument:
+        Version: '2012-10-17'
+        Statement:
+        - Effect: Allow
+          Principal: {Service: apigateway.amazonaws.com}
+          Action: sts:AssumeRole
+      Policies:
+      - PolicyName: AllowInvokeLambdaFunctions
+        PolicyDocument:
+          Version: '2012-10-17'
+          Statement:
+          - Effect: Allow
+            Action: lambda:InvokeFunction
+            Resource: '*'
+
+  MyApi:
+    Type: AWS::Serverless::Api
+    Properties:
+      Cors:
+        AllowMethods: "'methods'"
+        AllowHeaders: "'headers'"
+        AllowOrigin: "'origins'"
+        MaxAge: "'600'"
+      Auth:
+        ApiKeyRequired: true
+      StageName: dev
+      DefinitionBody:
+        openapi: 3.0.1
+        paths:
+          /apione:
+            get:
+              x-amazon-apigateway-integration:
+                credentials:
+                  Fn::Sub: ${ApiGatewayLambdaRole.Arn}
+                uri:
+                  Fn::Sub: arn:aws:apigateway:${AWS::Region}:lambda:path/2015-03-31/functions/${MyFunction.Arn}/invocations
+                passthroughBehavior: when_no_match
+                httpMethod: POST
+                type: aws_proxy
+          /apitwo:
+            get:
+              x-amazon-apigateway-integration:
+                credentials:
+                  Fn::Sub: ${ApiGatewayLambdaRole.Arn}
+                uri:
+                  Fn::Sub: arn:aws:apigateway:${AWS::Region}:lambda:path/2015-03-31/functions/${MyFunction.Arn}/invocations
+                passthroughBehavior: when_no_match
+                httpMethod: POST
+                type: aws_proxy
+
+
+
+Outputs:
+  ApiUrl:
+    Description: URL of your API endpoint
+    Value: !Sub "https://${MyApi}.execute-api.${AWS::Region}.amazonaws.com/dev/"
+Metadata:
+  SamTransformTest: true
diff --git a/samtranslator/internal/schema_source/aws_serverless_api.py b/samtranslator/internal/schema_source/aws_serverless_api.py
@@ -100,6 +100,7 @@ class UsagePlan(BaseModel):
 
 class Auth(BaseModel):
     AddDefaultAuthorizerToCorsPreflight: Optional[bool] = auth("AddDefaultAuthorizerToCorsPreflight")
+    AddApiKeyRequiredToCorsPreflight: Optional[bool]  # TODO Add Docs
     ApiKeyRequired: Optional[bool] = auth("ApiKeyRequired")
     Authorizers: Optional[
         Dict[

diff --git a/samtranslator/model/api/api_generator.py b/samtranslator/model/api/api_generator.py
@@ -52,12 +52,13 @@
         "DefaultAuthorizer",
         "InvokeRole",
         "AddDefaultAuthorizerToCorsPreflight",
+        "AddApiKeyRequiredToCorsPreflight",
         "ApiKeyRequired",
         "ResourcePolicy",
         "UsagePlan",
     ],
 )
-AuthProperties.__new__.__defaults__ = (None, None, None, True, None, None, None)
+AuthProperties.__new__.__defaults__ = (None, None, None, True, True, None, None, None)
 UsagePlanProperties = namedtuple(
     "UsagePlanProperties", ["CreateUsagePlan", "Description", "Quota", "Tags", "Throttle", "UsagePlanName"]
 )
@@ -752,7 +753,7 @@ def _add_auth(self) -> None:
 
         if auth_properties.ApiKeyRequired:
             swagger_editor.add_apikey_security_definition()
-            self._set_default_apikey_required(swagger_editor)
+            self._set_default_apikey_required(swagger_editor, auth_properties.AddApiKeyRequiredToCorsPreflight)
 
         if auth_properties.ResourcePolicy:
             SwaggerEditor.validate_is_dict(
@@ -1224,9 +1225,9 @@ def _set_default_authorizer(
                 add_default_auth_to_preflight=add_default_auth_to_preflight,
             )
 
-    def _set_default_apikey_required(self, swagger_editor: SwaggerEditor) -> None:
+    def _set_default_apikey_required(self, swagger_editor: SwaggerEditor, required_options_api_key: bool) -> None:
         for path in swagger_editor.iter_on_path():
-            swagger_editor.set_path_default_apikey_required(path)
+            swagger_editor.set_path_default_apikey_required(path, required_options_api_key)
 
     def _set_endpoint_configuration(self, rest_api: ApiGatewayRestApi, value: Union[str, Dict[str, Any]]) -> None:
         """

diff --git a/samtranslator/schema/schema.json b/samtranslator/schema/schema.json
@@ -197274,6 +197274,10 @@
     "samtranslator__internal__schema_source__aws_serverless_api__Auth": {
       "additionalProperties": false,
       "properties": {
+        "AddApiKeyRequiredToCorsPreflight": {
+          "title": "Addapikeyrequiredtocorspreflight",
+          "type": "boolean"
+        },
         "AddDefaultAuthorizerToCorsPreflight": {
           "description": "If the `DefaultAuthorizer` and `Cors` properties are set, then setting `AddDefaultAuthorizerToCorsPreflight` will cause the default authorizer to be added to the `Options` property in the OpenAPI section\\.  \n*Type*: Boolean  \n*Required*: No  \n*Default*: True  \n*AWS CloudFormation compatibility*: This property is unique to AWS SAM and doesn't have an AWS CloudFormation equivalent\\.",
           "markdownDescription": "If the `DefaultAuthorizer` and `Cors` properties are set, then setting `AddDefaultAuthorizerToCorsPreflight` will cause the default authorizer to be added to the `Options` property in the OpenAPI section\\.  \n*Type*: Boolean  \n*Required*: No  \n*Default*: True  \n*AWS CloudFormation compatibility*: This property is unique to AWS SAM and doesn't have an AWS CloudFormation equivalent\\.",

diff --git a/samtranslator/swagger/swagger.py b/samtranslator/swagger/swagger.py
@@ -612,14 +612,16 @@ def set_path_default_authorizer(  # noqa: too-many-branches
                 if "AWS_IAM" in method_definition["security"][0]:
                     self.add_awsiam_security_definition()
 
-    def set_path_default_apikey_required(self, path: str) -> None:
+    def set_path_default_apikey_required(self, path: str, required_options_api_key: bool = True) -> None:
         """
         Add the ApiKey security as required for each method on this path unless ApiKeyRequired
         was defined at the Function/Path/Method level. This is intended to be used to set the
         apikey security restriction for all api methods based upon the default configured in the
         Serverless API.
 
         :param string path: Path name
+        :param bool required_options_api_key: Bool of whether to add the ApiKeyRequired
+         to OPTIONS preflight requests.
         """
 
         for method_name, method_definition in self.iter_on_all_methods_for_path(path):  # type: ignore[no-untyped-call]
@@ -673,6 +675,9 @@ def set_path_default_apikey_required(self, path: str) -> None:
 
             security = existing_non_apikey_security + apikey_security
 
+            if method_name == "options" and not required_options_api_key:
+                security = existing_non_apikey_security
+
             if security != existing_security:
                 method_definition["security"] = security
 
@@ -691,10 +696,12 @@ def add_auth_to_method(self, path: str, method_name: str, auth: Dict[str, Any],
         method_scopes = auth and auth.get("AuthorizationScopes")
         api_auth = api and api.get("Auth")
         authorizers = api_auth and api_auth.get("Authorizers")
+
         if method_authorizer:
             self._set_method_authorizer(path, method_name, method_authorizer, authorizers, method_scopes)  # type: ignore[no-untyped-call]
 
         method_apikey_required = auth and auth.get("ApiKeyRequired")
+
         if method_apikey_required is not None:
             self._set_method_apikey_handling(path, method_name, method_apikey_required)  # type: ignore[no-untyped-call]
 

diff --git a/schema_source/sam.schema.json b/schema_source/sam.schema.json
@@ -3015,6 +3015,10 @@
     "samtranslator__internal__schema_source__aws_serverless_api__Auth": {
       "additionalProperties": false,
       "properties": {
+        "AddApiKeyRequiredToCorsPreflight": {
+          "title": "Addapikeyrequiredtocorspreflight",
+          "type": "boolean"
+        },
         "AddDefaultAuthorizerToCorsPreflight": {
           "description": "If the `DefaultAuthorizer` and `Cors` properties are set, then setting `AddDefaultAuthorizerToCorsPreflight` will cause the default authorizer to be added to the `Options` property in the OpenAPI section\\.  \n*Type*: Boolean  \n*Required*: No  \n*Default*: True  \n*AWS CloudFormation compatibility*: This property is unique to AWS SAM and doesn't have an AWS CloudFormation equivalent\\.",
           "markdownDescription": "If the `DefaultAuthorizer` and `Cors` properties are set, then setting `AddDefaultAuthorizerToCorsPreflight` will cause the default authorizer to be added to the `Options` property in the OpenAPI section\\.  \n*Type*: Boolean  \n*Required*: No  \n*Default*: True  \n*AWS CloudFormation compatibility*: This property is unique to AWS SAM and doesn't have an AWS CloudFormation equivalent\\.",

diff --git a/tests/translator/input/api_with_cors_and_apikey.yaml b/tests/translator/input/api_with_cors_and_apikey.yaml
@@ -0,0 +1,87 @@
+AWSTemplateFormatVersion: '2010-09-09'
+
+Transform:
+- AWS::Serverless-2016-10-31
+
+Globals:
+  Api:
+    Auth:
+      ApiKeyRequired: true
+      AddApiKeyRequiredToCorsPreflight: false
+
+Resources:
+
+  MyFunction:
+    Type: AWS::Serverless::Function
+    Properties:
+      Handler: index.handler
+      InlineCode: |
+        exports.handler = async function (event) {
+          return {
+            statusCode: 200,
+            body: JSON.stringify({ message: "Hello, SAM!" }),
+          }
+        }
+      Runtime: nodejs16.x
+
+  ApiGatewayLambdaRole:
+    Type: AWS::IAM::Role
+    Properties:
+      AssumeRolePolicyDocument:
+        Version: '2012-10-17'
+        Statement:
+        - Effect: Allow
+          Principal: {Service: apigateway.amazonaws.com}
+          Action: sts:AssumeRole
+      Policies:
+      - PolicyName: AllowInvokeLambdaFunctions
+        PolicyDocument:
+          Version: '2012-10-17'
+          Statement:
+          - Effect: Allow
+            Action: lambda:InvokeFunction
+            Resource: '*'
+
+  MyApi:
+    Type: AWS::Serverless::Api
+    Properties:
+      Cors:
+        AllowMethods: "'methods'"
+        AllowHeaders: "'headers'"
+        AllowOrigin: "'origins'"
+        MaxAge: "'600'"
+      Auth:
+        ApiKeyRequired: true
+      StageName: dev
+      DefinitionBody:
+        openapi: 3.0.1
+        paths:
+          /apione:
+            get:
+              x-amazon-apigateway-integration:
+                credentials:
+                  Fn::Sub: ${ApiGatewayLambdaRole.Arn}
+                uri:
+                  Fn::Sub: arn:aws:apigateway:${AWS::Region}:lambda:path/2015-03-31/functions/${MyFunction.Arn}/invocations
+                passthroughBehavior: when_no_match
+                httpMethod: POST
+                type: aws_proxy
+          /apitwo:
+            get:
+              x-amazon-apigateway-integration:
+                credentials:
+                  Fn::Sub: ${ApiGatewayLambdaRole.Arn}
+                uri:
+                  Fn::Sub: arn:aws:apigateway:${AWS::Region}:lambda:path/2015-03-31/functions/${MyFunction.Arn}/invocations
+                passthroughBehavior: when_no_match
+                httpMethod: POST
+                type: aws_proxy
+
+
+
+Outputs:
+  ApiUrl:
+    Description: URL of your API endpoint
+    Value: !Sub "https://${MyApi}.execute-api.${AWS::Region}.amazonaws.com/dev/"
+Metadata:
+  SamTransformTest: true
diff --git a/tests/translator/input/api_with_cors_and_apikey_defined_at_api_level.yaml b/tests/translator/input/api_with_cors_and_apikey_defined_at_api_level.yaml
@@ -0,0 +1,66 @@
+AWSTemplateFormatVersion: '2010-09-09'
+
+Transform:
+- AWS::Serverless-2016-10-31
+
+Resources:
+
+  MyFunction:
+    Type: AWS::Serverless::Function
+    Properties:
+      Handler: index.handler
+      InlineCode: |
+        exports.handler = async function (event) {
+          return {
+            statusCode: 200,
+            body: JSON.stringify({ message: "Hello, SAM!" }),
+          }
+        }
+      Runtime: nodejs16.x
+
+  ApiGatewayLambdaRole:
+    Type: AWS::IAM::Role
+    Properties:
+      AssumeRolePolicyDocument:
+        Version: '2012-10-17'
+        Statement:
+        - Effect: Allow
+          Principal: {Service: apigateway.amazonaws.com}
+          Action: sts:AssumeRole
+      Policies:
+      - PolicyName: AllowInvokeLambdaFunctions
+        PolicyDocument:
+          Version: '2012-10-17'
+          Statement:
+          - Effect: Allow
+            Action: lambda:InvokeFunction
+            Resource: '*'
+
+  MyApi:
+    Type: AWS::Serverless::Api
+    Properties:
+      Cors: "'*'"
+      Auth:
+        ApiKeyRequired: true
+        AddApiKeyRequiredToCorsPreflight: false
+      StageName: dev
+      DefinitionBody:
+        openapi: 3.0.1
+        paths:
+          /hello:
+            get:
+              x-amazon-apigateway-integration:
+                credentials:
+                  Fn::Sub: ${ApiGatewayLambdaRole.Arn}
+                uri:
+                  Fn::Sub: arn:aws:apigateway:${AWS::Region}:lambda:path/2015-03-31/functions/${MyFunction.Arn}/invocations
+                passthroughBehavior: when_no_match
+                httpMethod: POST
+                type: aws_proxy
+
+
+
+Outputs:
+  WebEndpoint:
+    Description: API Gateway endpoint URL
+    Value: !Sub "https://${MyApi}.execute-api.${AWS::Region}.amazonaws.com/dev/hello"