Skip to content

Commit

Permalink
fix: Raise correct exception when rest api auth is not a dict
Browse files Browse the repository at this point in the history
  • Loading branch information
aahung committed Dec 15, 2022
1 parent 9101c7a commit 2fb1fde
Show file tree
Hide file tree
Showing 6 changed files with 58 additions and 27 deletions.
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]
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
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
10 changes: 5 additions & 5 deletions tests/swagger/test_swagger.py
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,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 +232,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 +254,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 +271,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."
}

0 comments on commit 2fb1fde

Please sign in to comment.