From 2d75ab924bea6da2504d680497be08e9aab36384 Mon Sep 17 00:00:00 2001 From: Sam Liu Date: Wed, 14 Dec 2022 16:47:44 -0800 Subject: [PATCH] fix: Raise correct exception when rest api auth is not a dict --- samtranslator/model/eventsources/push.py | 14 ++++++---- samtranslator/swagger/swagger.py | 23 ++++++++-------- .../eventsources/test_api_event_source.py | 12 ++++----- tests/openapi/test_openapi.py | 9 ++++--- tests/swagger/test_swagger.py | 11 ++++---- ...nvalid_auth_while_method_auth_is_none.yaml | 27 +++++++++++++++++++ ...nvalid_auth_while_method_auth_is_none.json | 2 +- 7 files changed, 65 insertions(+), 33 deletions(-) diff --git a/samtranslator/model/eventsources/push.py b/samtranslator/model/eventsources/push.py index 9b767a9e5..b73f161ee 100644 --- a/samtranslator/model/eventsources/push.py +++ b/samtranslator/model/eventsources/push.py @@ -640,7 +640,7 @@ def resources_to_link(self, resources): # type: ignore[no-untyped-def] "RestApiId property of Api event must reference a valid resource in the same template.", ) - return {"explicit_api": explicit_api, "explicit_api_stage": {"suffix": stage_suffix}} + return {"explicit_api": explicit_api, "api_id": rest_api_id, "explicit_api_stage": {"suffix": stage_suffix}} @cw_timer(prefix=FUNCTION_EVETSOURCE_METRIC_PREFIX) def to_cloudformation(self, **kwargs): # type: ignore[no-untyped-def] @@ -668,8 +668,9 @@ def to_cloudformation(self, **kwargs): # type: ignore[no-untyped-def] resources.extend(self._get_permissions(kwargs)) # type: ignore[no-untyped-call] explicit_api = kwargs["explicit_api"] + api_id = kwargs["api_id"] if explicit_api.get("__MANAGE_SWAGGER"): - self._add_swagger_integration(explicit_api, function, intrinsics_resolver) # type: ignore[no-untyped-call] + self._add_swagger_integration(explicit_api, api_id, function, intrinsics_resolver) # type: ignore[no-untyped-call] return resources @@ -712,7 +713,7 @@ def _get_permission(self, resources_to_link, stage, suffix): # type: ignore[no- return self._construct_permission(resources_to_link["function"], source_arn=source_arn, suffix=suffix) # type: ignore[no-untyped-call] - def _add_swagger_integration(self, api, function, intrinsics_resolver): # type: ignore[no-untyped-def] + def _add_swagger_integration(self, api, api_id, function, intrinsics_resolver): # type: ignore[no-untyped-def] # pylint: disable=duplicate-code """Adds the path and method for this Api event source to the Swagger body for the provided RestApi. @@ -740,10 +741,13 @@ def _add_swagger_integration(self, api, function, intrinsics_resolver): # type: if CONDITION in function.resource_attributes: condition = function.resource_attributes[CONDITION] - editor.add_lambda_integration(self.Path, self.Method, uri, self.Auth, api.get("Auth"), condition=condition) # type: ignore[attr-defined, attr-defined, no-untyped-call] + method_auth = self.Auth or Py27Dict() # type: ignore[attr-defined] + sam_expect(method_auth, self.relative_id, "Auth", is_sam_event=True).to_be_a_map() + api_auth = api.get("Auth") or Py27Dict() + sam_expect(api_auth, api_id, "Auth").to_be_a_map() + editor.add_lambda_integration(self.Path, self.Method, uri, method_auth, api_auth, condition=condition) # type: ignore[attr-defined, attr-defined] if self.Auth: # type: ignore[attr-defined] - sam_expect(self.Auth, self.relative_id, "Auth", is_sam_event=True).to_be_a_map() # type: ignore[attr-defined] method_authorizer = self.Auth.get("Authorizer") # type: ignore[attr-defined] api_auth = api.get("Auth") api_auth = intrinsics_resolver.resolve_parameter_refs(api_auth) diff --git a/samtranslator/swagger/swagger.py b/samtranslator/swagger/swagger.py index 2d3a14120..745df4842 100644 --- a/samtranslator/swagger/swagger.py +++ b/samtranslator/swagger/swagger.py @@ -93,15 +93,17 @@ def add_disable_execute_api_endpoint_extension(self, disable_execute_api_endpoin else: SwaggerEditor._update_dict(self._doc, self._X_ENDPOINT_CONFIG, set_disable_api_endpoint) - def add_lambda_integration( # type: ignore[no-untyped-def] - self, path, method, integration_uri, method_auth_config=None, api_auth_config=None, condition=None - ): + def add_lambda_integration( + self, + path: str, + method: str, + integration_uri: str, + method_auth_config: Dict[str, Any], + api_auth_config: Dict[str, Any], + condition: Optional[str] = None, + ) -> None: """ Adds aws_proxy APIGW integration to the given path+method. - - :param string path: Path name - :param string method: HTTP Method - :param string integration_uri: URI for the integration. """ method = self._normalize_method_name(method) @@ -118,8 +120,7 @@ def add_lambda_integration( # type: ignore[no-untyped-def] # Wrap the integration_uri in a Condition if one exists on that function # This is necessary so CFN doesn't try to resolve the integration reference. - if condition: - integration_uri = make_conditional(condition, integration_uri) + _integration_uri = make_conditional(condition, integration_uri) if condition else integration_uri for path_item in self.get_conditional_contents(self.paths.get(path)): BaseEditor.validate_path_item_is_dict(path_item, path) @@ -127,10 +128,8 @@ def add_lambda_integration( # type: ignore[no-untyped-def] # insert key one by one to preserce input order path_item[method][self._X_APIGW_INTEGRATION]["type"] = "aws_proxy" path_item[method][self._X_APIGW_INTEGRATION]["httpMethod"] = "POST" - path_item[method][self._X_APIGW_INTEGRATION]["uri"] = integration_uri + path_item[method][self._X_APIGW_INTEGRATION]["uri"] = _integration_uri - method_auth_config = method_auth_config or Py27Dict() - api_auth_config = api_auth_config or Py27Dict() if ( method_auth_config.get("Authorizer") == "AWS_IAM" or api_auth_config.get("DefaultAuthorizer") == "AWS_IAM" diff --git a/tests/model/eventsources/test_api_event_source.py b/tests/model/eventsources/test_api_event_source.py index bb8bf11e8..6e8caaaff 100644 --- a/tests/model/eventsources/test_api_event_source.py +++ b/tests/model/eventsources/test_api_event_source.py @@ -24,7 +24,7 @@ def setUp(self): @patch("boto3.session.Session.region_name", "eu-west-2") def test_get_permission_without_trailing_slash(self): - cfn = self.api_event_source.to_cloudformation(function=self.func, explicit_api={}) + cfn = self.api_event_source.to_cloudformation(function=self.func, explicit_api={}, api_id="RestApi") perm = cfn[0] self.assertIsInstance(perm, LambdaPermission) @@ -39,7 +39,7 @@ def test_get_permission_without_trailing_slash(self): @patch("boto3.session.Session.region_name", "eu-west-2") def test_get_permission_with_trailing_slash(self): self.api_event_source.Path = "/foo/" - cfn = self.api_event_source.to_cloudformation(function=self.func, explicit_api={}) + cfn = self.api_event_source.to_cloudformation(function=self.func, explicit_api={}, api_id="RestApi") perm = cfn[0] self.assertIsInstance(perm, LambdaPermission) @@ -54,7 +54,7 @@ def test_get_permission_with_trailing_slash(self): @patch("boto3.session.Session.region_name", "eu-west-2") def test_get_permission_with_path_parameter_to_any_path(self): self.api_event_source.Path = "/foo/{userId+}" - cfn = self.api_event_source.to_cloudformation(function=self.func, explicit_api={}) + cfn = self.api_event_source.to_cloudformation(function=self.func, explicit_api={}, api_id="RestApi") perm = cfn[0] self.assertIsInstance(perm, LambdaPermission) @@ -71,7 +71,7 @@ def test_get_permission_with_path_parameter_to_any_path(self): @patch("boto3.session.Session.region_name", "eu-west-2") def test_get_permission_with_path_parameter(self): self.api_event_source.Path = "/foo/{userId}/bar" - cfn = self.api_event_source.to_cloudformation(function=self.func, explicit_api={}) + cfn = self.api_event_source.to_cloudformation(function=self.func, explicit_api={}, api_id="RestApi") perm = cfn[0] self.assertIsInstance(perm, LambdaPermission) @@ -88,7 +88,7 @@ def test_get_permission_with_path_parameter(self): @patch("boto3.session.Session.region_name", "eu-west-2") def test_get_permission_with_proxy_resource(self): self.api_event_source.Path = "/foo/{proxy+}" - cfn = self.api_event_source.to_cloudformation(function=self.func, explicit_api={}) + cfn = self.api_event_source.to_cloudformation(function=self.func, explicit_api={}, api_id="RestApi") perm = cfn[0] self.assertIsInstance(perm, LambdaPermission) @@ -105,7 +105,7 @@ def test_get_permission_with_proxy_resource(self): @patch("boto3.session.Session.region_name", "eu-west-2") def test_get_permission_with_just_slash(self): self.api_event_source.Path = "/" - cfn = self.api_event_source.to_cloudformation(function=self.func, explicit_api={}) + cfn = self.api_event_source.to_cloudformation(function=self.func, explicit_api={}, api_id="RestApi") perm = cfn[0] self.assertIsInstance(perm, LambdaPermission) diff --git a/tests/openapi/test_openapi.py b/tests/openapi/test_openapi.py index 23675e031..9d78c6a06 100644 --- a/tests/openapi/test_openapi.py +++ b/tests/openapi/test_openapi.py @@ -5,6 +5,7 @@ from samtranslator.open_api.open_api import OpenApiEditor from samtranslator.model.exceptions import InvalidDocumentException +from samtranslator.utils.py27hash_fix import Py27Dict _X_INTEGRATION = "x-amazon-apigateway-integration" _X_ANY_METHOD = "x-amazon-apigateway-any-method" @@ -223,7 +224,7 @@ def test_must_override_null_path(self): }, } - self.editor.add_lambda_integration(path, method, integration_uri) + self.editor.add_lambda_integration(path, method, integration_uri, Py27Dict(), Py27Dict()) self.assertTrue(self.editor.has_path(path, method)) actual = self.editor.openapi["paths"][path][method] @@ -243,7 +244,7 @@ def test_must_add_new_integration_to_new_path(self): }, } - self.editor.add_lambda_integration(path, method, integration_uri) + self.editor.add_lambda_integration(path, method, integration_uri, Py27Dict(), Py27Dict()) self.assertTrue(self.editor.has_path(path, method)) actual = self.editor.openapi["paths"][path][method] @@ -270,7 +271,7 @@ def test_must_add_new_integration_with_conditions_to_new_path(self): ] } - self.editor.add_lambda_integration(path, method, integration_uri, condition=condition) + self.editor.add_lambda_integration(path, method, integration_uri, Py27Dict(), Py27Dict(), condition=condition) self.assertTrue(self.editor.has_path(path, method)) actual = self.editor.openapi["paths"][path][method] @@ -297,7 +298,7 @@ def test_must_add_new_integration_to_existing_path(self): # Just make sure test is working on an existing path self.assertTrue(self.editor.has_path(path, method)) - self.editor.add_lambda_integration(path, method, integration_uri) + self.editor.add_lambda_integration(path, method, integration_uri, Py27Dict(), Py27Dict()) actual = self.editor.openapi["paths"][path][method] self.assertEqual(expected, actual) diff --git a/tests/swagger/test_swagger.py b/tests/swagger/test_swagger.py index 60964b562..b41b777e0 100644 --- a/tests/swagger/test_swagger.py +++ b/tests/swagger/test_swagger.py @@ -6,6 +6,7 @@ from samtranslator.swagger.swagger import SwaggerEditor from samtranslator.model.exceptions import InvalidDocumentException, InvalidTemplateException +from samtranslator.utils.py27hash_fix import Py27Dict from tests.translator.test_translator import deep_sort_lists _X_INTEGRATION = "x-amazon-apigateway-integration" @@ -206,7 +207,7 @@ def test_must_add_new_integration_to_new_path(self): _X_INTEGRATION: {"type": "aws_proxy", "httpMethod": "POST", "uri": integration_uri}, } - self.editor.add_lambda_integration(path, method, integration_uri) + self.editor.add_lambda_integration(path, method, integration_uri, Py27Dict(), Py27Dict()) self.assertTrue(self.editor.has_path(path, method)) actual = self.editor.swagger["paths"][path][method] @@ -232,7 +233,7 @@ def test_must_add_new_integration_with_conditions_to_new_path(self): ] } - self.editor.add_lambda_integration(path, method, integration_uri, condition=condition) + self.editor.add_lambda_integration(path, method, integration_uri, Py27Dict(), Py27Dict(), condition=condition) self.assertTrue(self.editor.has_path(path, method)) actual = self.editor.swagger["paths"][path][method] @@ -254,7 +255,7 @@ def test_must_add_new_integration_to_existing_path(self): # Just make sure test is working on an existing path self.assertTrue(self.editor.has_path(path, method)) - self.editor.add_lambda_integration(path, method, integration_uri) + self.editor.add_lambda_integration(path, method, integration_uri, Py27Dict(), Py27Dict()) actual = self.editor.swagger["paths"][path][method] self.assertEqual(expected, actual) @@ -262,7 +263,7 @@ def test_must_add_new_integration_to_existing_path(self): def test_must_raise_on_existing_integration(self): with self.assertRaises(InvalidDocumentException): - self.editor.add_lambda_integration("/bar", "get", "integrationUri") + self.editor.add_lambda_integration("/bar", "get", "integrationUri", Py27Dict(), Py27Dict()) def test_must_add_credentials_to_the_integration(self): path = "/newpath" @@ -271,7 +272,7 @@ def test_must_add_credentials_to_the_integration(self): expected = "arn:aws:iam::*:user/*" api_auth_config = {"DefaultAuthorizer": "AWS_IAM", "InvokeRole": "CALLER_CREDENTIALS"} - self.editor.add_lambda_integration(path, method, integration_uri, None, api_auth_config) + self.editor.add_lambda_integration(path, method, integration_uri, Py27Dict(), api_auth_config) actual = self.editor.swagger["paths"][path][method][_X_INTEGRATION]["credentials"] self.assertEqual(expected, actual) diff --git a/tests/translator/input/error_http_api_with_invalid_auth_while_method_auth_is_none.yaml b/tests/translator/input/error_http_api_with_invalid_auth_while_method_auth_is_none.yaml index 9ef9206cd..df215c343 100644 --- a/tests/translator/input/error_http_api_with_invalid_auth_while_method_auth_is_none.yaml +++ b/tests/translator/input/error_http_api_with_invalid_auth_while_method_auth_is_none.yaml @@ -17,7 +17,34 @@ Resources: Ref: MyApi Method: GET Path: /get + + MyLambdaFunction2: + Type: AWS::Serverless::Function + Properties: + Handler: index.handler + Runtime: python3.7 + InlineCode: | + def handler(event, context): + return {'body': 'Hello World!', 'statusCode': 200} + Events: + GetRestApi: + Type: Api + Properties: + Auth: + Authorizer: NONE + RestApiId: + Ref: MyRestApi + Method: GET + Path: /get + MyApi: Type: AWS::Serverless::HttpApi Properties: Auth: + + MyRestApi: + Type: AWS::Serverless::Api + Properties: + StageName: Prod + Auth: + - I am a list diff --git a/tests/translator/output/error_http_api_with_invalid_auth_while_method_auth_is_none.json b/tests/translator/output/error_http_api_with_invalid_auth_while_method_auth_is_none.json index cc85b13aa..41a1e537a 100644 --- a/tests/translator/output/error_http_api_with_invalid_auth_while_method_auth_is_none.json +++ b/tests/translator/output/error_http_api_with_invalid_auth_while_method_auth_is_none.json @@ -1,3 +1,3 @@ { - "errorMessage": "Invalid Serverless Application Specification document. Number of errors found: 1. Resource with id [MyApi] is invalid. Property 'Auth' should be a map." + "errorMessage": "Invalid Serverless Application Specification document. Number of errors found: 3. Resource with id [MyApi] is invalid. Property 'Auth' should be a map. Resource with id [MyRestApi] is invalid. Property 'Auth' should be a map. Resource with id [MyRestApi] is invalid. Type of property 'Auth' is invalid." }