Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Raise correct exception when rest api auth is not a dict #2732

Merged
merged 1 commit into from
Dec 15, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 9 additions & 5 deletions samtranslator/model/eventsources/push.py
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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.

Expand Down Expand Up @@ -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)
Expand Down
23 changes: 11 additions & 12 deletions samtranslator/swagger/swagger.py
Original file line number Diff line number Diff line change
Expand Up @@ -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]
aahung marked this conversation as resolved.
Show resolved Hide resolved
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)
Expand All @@ -118,19 +120,16 @@ 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)
path_item[method][self._X_APIGW_INTEGRATION] = Py27Dict()
# 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"
Expand Down
12 changes: 6 additions & 6 deletions tests/model/eventsources/test_api_event_source.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand Down
9 changes: 5 additions & 4 deletions tests/openapi/test_openapi.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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]
Expand All @@ -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]
Expand All @@ -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]
Expand All @@ -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)
Expand Down
11 changes: 6 additions & 5 deletions tests/swagger/test_swagger.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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]
Expand All @@ -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]
Expand All @@ -254,15 +255,15 @@ 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)

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"
Expand All @@ -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)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Original file line number Diff line number Diff line change
@@ -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."
aahung marked this conversation as resolved.
Show resolved Hide resolved
}