From 35162b8482b6986ec0d1ce18192d13aa4268bfbb Mon Sep 17 00:00:00 2001 From: James Saryerwinnie Date: Tue, 6 Jun 2017 12:47:44 -0700 Subject: [PATCH 01/11] Add support for builtin API Gateway authorizers Chalice currently supports IAM, Cognito, and Custom authorizers. The custom authorizer allows you to define a lambda function which contains custom authorization logic specific for your use case. Previously, you'd have to create your lambda authorizer function out of band from your chalice app. We just provided an API to associated your custom authorizer with certain views. This change adds support for a "built in authorizer", which allows you to create custom authorizers that are defined within your chalice app. When you run "chalice deploy" we'll automatically create and associate the authorizer for you, but we'll also create the lambda function associated with the authorizer. This introduces new concepts in chalice including multi lambda function support. We now have an API handler (the existing lambda function) as well as auth handlers. There's still a few missing pieces that need to get ironed out: * Need to work out how we want to configure the auth functions in config.json (timeouts, memory, etc) * Need to work out how to deal with "chalice package". It appears SAM doesn't support giving implicit permission to invoke the lambda function in the same way the event type "Api" works. * Docs (in progress, should be coming shortly). --- chalice/__init__.py | 3 +- chalice/app.py | 137 ++++++++++++++++++++ chalice/app.pyi | 32 +++++ chalice/awsclient.py | 26 ++++ chalice/config.py | 8 +- chalice/deploy/deployer.py | 97 ++++++++++++-- chalice/deploy/swagger.py | 36 +++++- tests/functional/cli/test_cli.py | 6 +- tests/functional/test_awsclient.py | 121 +++++++++++++----- tests/integration/test_features.py | 31 +++++ tests/integration/testapp/app.py | 34 ++++- tests/unit/conftest.py | 16 +++ tests/unit/deploy/test_deployer.py | 118 +++++++++++++++-- tests/unit/deploy/test_swagger.py | 42 +++++- tests/unit/test_app.py | 197 +++++++++++++++++++++++++++++ tests/unit/test_config.py | 2 + 16 files changed, 836 insertions(+), 70 deletions(-) diff --git a/chalice/__init__.py b/chalice/__init__.py index 25aa219ab..d0eec037d 100644 --- a/chalice/__init__.py +++ b/chalice/__init__.py @@ -2,7 +2,8 @@ from chalice.app import ( ChaliceViewError, BadRequestError, UnauthorizedError, ForbiddenError, NotFoundError, ConflictError, TooManyRequestsError, Response, CORSConfig, - CustomAuthorizer, CognitoUserPoolAuthorizer, IAMAuthorizer + CustomAuthorizer, CognitoUserPoolAuthorizer, IAMAuthorizer, + AuthResponse, AuthRoute ) __version__ = '0.9.0' diff --git a/chalice/app.py b/chalice/app.py index c450b81b4..95afd1c03 100644 --- a/chalice/app.py +++ b/chalice/app.py @@ -417,6 +417,7 @@ def __init__(self, app_name, configure_logs=True): self.configure_logs = configure_logs self.log = logging.getLogger(self.app_name) self._authorizers = {} + self.builtin_auth_handlers = [] if self.configure_logs: self._configure_logging() @@ -461,6 +462,25 @@ def define_authorizer(self, name, header, auth_type, provider_arns=None): 'provider_arns': provider_arns, } + def authorizer(self, name=None, **kwargs): + def _register_authorizer(auth_func): + auth_name = name + if auth_name is None: + auth_name = auth_func.__name__ + ttl_seconds = kwargs.pop('ttl_seconds', None) + execution_role = kwargs.pop('execution_role', None) + auth_config = BuiltinAuthConfig( + name=auth_name, + handler_string='app.%s' % auth_func.__name__, + ttl_seconds=ttl_seconds, + execution_role=execution_role, + ) + # Do we even need the builtin_auth_handlers attr? See if we can + # remove this and calculate this list on demand. + self.builtin_auth_handlers.append(auth_config) + return ChaliceAuthorizer(name, auth_func, auth_config) + return _register_authorizer + def route(self, path, **kwargs): def _register_view(view_func): self._add_route(path, view_func, **kwargs) @@ -626,3 +646,120 @@ def _add_cors_headers(self, response, cors): for name, value in cors.get_access_control_headers().items(): if name not in response.headers: response.headers[name] = value + + +class BuiltinAuthConfig(object): + def __init__(self, name, handler_string, ttl_seconds=None, + execution_role=None): + # We'd also support all the misc config options you can set. + self.name = name + self.handler_string = handler_string + self.ttl_seconds = ttl_seconds + self.execution_role = execution_role + + +class ChaliceAuthorizer(object): + def __init__(self, name, func, config): + self.name = name + self.func = func + self.config = config + + def __call__(self, event, content): + auth_request = self._transform_event(event) + result = self.func(auth_request) + if isinstance(result, AuthResponse): + return result.to_dict(auth_request) + return result + + def _transform_event(self, event): + return AuthRequest(event['type'], + event['authorizationToken'], + event['methodArn']) + + +class AuthRequest(object): + def __init__(self, auth_type, token, method_arn): + self.auth_type = auth_type + self.token = token + self.method_arn = method_arn + + +class AuthResponse(object): + ALL_HTTP_METHODS = ['DELETE', 'HEAD', 'OPTIONS', + 'PATCH', 'POST', 'PUT', 'GET'] + + def __init__(self, routes, principal_id, context=None): + self.routes = routes + self.principal_id = principal_id + # The request is used to generate full qualified ARNs + # that we need for the resource portion of the returned + # policy. + if context is None: + context = {} + self.context = context + + def to_dict(self, request): + return { + 'context': self.context, + 'principalId': self.principal_id, + 'policyDocument': self._generate_policy(request), + } + + def _generate_policy(self, request): + allowed_resources = self._generate_allowed_resources(request) + return { + 'Version': '2012-10-17', + 'Statement': [ + { + 'Action': 'execute-api:Invoke', + 'Effect': 'Allow', + 'Resource': allowed_resources, + } + ] + } + + def _generate_allowed_resources(self, request): + allowed_resources = [] + for route in self.routes: + if isinstance(route, AuthRoute): + methods = route.methods + path = route.path + else: + # If 'route' is just a string, then they've + # opted not to use the AuthRoute(), so we'll + # generate a policy that allows all HTTP methods. + methods = self.ALL_HTTP_METHODS + path = route + for method in methods: + allowed_resources.append( + self._generate_arn(path, request, method)) + return allowed_resources + + def _generate_arn(self, route, request, method='*'): + incoming_arn = request.method_arn + parts = incoming_arn.rsplit(':', 1) + # "arn:aws:execute-api:us-west-2:123:rest-api-id/dev/GET/needs/auth" + # Then we pull out the rest-api-id and stage, such that: + # base = ['rest-api-id', 'stage'] + base = parts[-1].split('/')[:2] + # Now we add in the path components and rejoin everything + # back together to make a full arn. + # We're also assuming all HTTP methods (via '*') for now. + # To support per HTTP method routes the API will need to be updated. + # We also need to strip off the leading ``/`` so it can be + # '/'.join(...)'d properly. + base.extend([method, route[1:]]) + last_arn_segment = '/'.join(base) + if route == '/': + # We have to special case the '/' case. For whatever + # reason, API gateway adds an extra '/' to the method_arn + # of the auth request, so we need to do the same thing. + last_arn_segment += '/' + final_arn = '%s:%s' % (parts[0], last_arn_segment) + return final_arn + + +class AuthRoute(object): + def __init__(self, path, methods): + self.path = path + self.methods = methods diff --git a/chalice/app.pyi b/chalice/app.pyi index 3a01e348d..4aaae88bb 100644 --- a/chalice/app.pyi +++ b/chalice/app.pyi @@ -13,6 +13,8 @@ class TooManyRequestsError(ChaliceViewError): ... ALL_ERRORS = ... # type: List[ChaliceViewError] +_BUILTIN_AUTH_FUNC = Callable[ + [AuthRequest], Union[AuthResponse, Dict[str, Any]]] class Authorizer: @@ -107,6 +109,7 @@ class Chalice(object): current_request = ... # type: Request debug = ... # type: bool authorizers = ... # type: Dict[str, Dict[str, Any]] + builtin_auth_handlers = ... # type: List[BuiltinAuthConfig] def __init__(self, app_name: str) -> None: ... @@ -116,3 +119,32 @@ class Chalice(object): def _get_view_function_response(self, view_function: Callable[..., Any], function_args: List[Any]) -> Response: ... + + +class ChaliceAuthorizer(object): + name = ... # type: str + func = ... # type: _BUILTIN_AUTH_FUNC + config = ... # type: BuiltinAuthConfig + + +class BuiltinAuthConfig(object): + name = ... # type: str + handler_string = ... # type: str + + +class AuthRequest(object): + auth_type = ... # type: str + token = ... # type: str + method_arn = ... # type: str + + +class AuthRoute(object): + path = ... # type: str + methods = ... # type: List[str] + + +class AuthResponse(object): + ALL_HTTP_METHODS = ... # type: List[str] + routes = ... # type: Union[str, AuthRoute] + principal_id = ... # type: str + context = ... # type: Optional[Dict[str, str]] diff --git a/chalice/awsclient.py b/chalice/awsclient.py index 8cdecbbcb..a0908a846 100644 --- a/chalice/awsclient.py +++ b/chalice/awsclient.py @@ -564,3 +564,29 @@ def _client(self, service_name): self._client_cache[service_name] = self._session.create_client( service_name) return self._client_cache[service_name] + + def add_permission_for_authorizer(self, rest_api_id, function_arn, + random_id): + # type: (str, str, str) -> None + client = self._client('apigateway') + authorizers = client.get_authorizers(restApiId=rest_api_id) + for authorizer in authorizers['items']: + if function_arn in authorizer['authorizerUri']: + authorizer_id = authorizer['id'] + break + else: + raise ValueError("Unable to find authorizer associated " + "with function ARN: %s" % function_arn) + parts = function_arn.split(':') + region_name = parts[3] + account_id = parts[4] + function_name = parts[-1] + source_arn = ("arn:aws:execute-api:%s:%s:%s/authorizers/%s" % + (region_name, account_id, rest_api_id, authorizer_id)) + self._client('lambda').add_permission( + Action='lambda:InvokeFunction', + FunctionName=function_name, + StatementId=random_id, + Principal='apigateway.amazonaws.com', + SourceArn=source_arn, + ) diff --git a/chalice/config.py b/chalice/config.py index 81fa15aaf..93c999107 100644 --- a/chalice/config.py +++ b/chalice/config.py @@ -262,8 +262,8 @@ def deployed_resources(self, chalice_stage_name): class DeployedResources(object): def __init__(self, backend, api_handler_arn, api_handler_name, rest_api_id, api_gateway_stage, - region, chalice_version): - # type: (str, str, str, str, str, str, str) -> None + region, chalice_version, lambda_functions): + # type: (str, str, str, str, str, str, str, StrMap) -> None self.backend = backend self.api_handler_arn = api_handler_arn self.api_handler_name = api_handler_name @@ -271,10 +271,11 @@ def __init__(self, backend, api_handler_arn, self.api_gateway_stage = api_gateway_stage self.region = region self.chalice_version = chalice_version + self.lambda_functions = lambda_functions @classmethod def from_dict(cls, data): - # type: (Dict[str, str]) -> DeployedResources + # type: (Dict[str, Any]) -> DeployedResources return cls( data['backend'], data['api_handler_arn'], @@ -283,4 +284,5 @@ def from_dict(cls, data): data['api_gateway_stage'], data['region'], data['chalice_version'], + data['lambda_functions'], ) diff --git a/chalice/deploy/deployer.py b/chalice/deploy/deployer.py index 2fbe92260..13a18ff32 100644 --- a/chalice/deploy/deployer.py +++ b/chalice/deploy/deployer.py @@ -375,13 +375,10 @@ def __init__(self, def delete(self, existing_resources): # type: (DeployedResources) -> None - handler_name = existing_resources.api_handler_name - role_arn = self._get_lambda_role_arn(handler_name) - print('Deleting lambda function %s' % handler_name) - try: - self._aws_client.delete_function(handler_name) - except ResourceDoesNotExistError as e: - print('No lambda function named %s found.' % e) + self._delete_api_handler(existing_resources) + self._delete_auth_handlers(existing_resources) + role_arn = self._get_lambda_role_arn( + existing_resources.api_handler_name) if role_arn is not None: role_name = role_arn.split('/')[1] if self._prompter.confirm( @@ -390,9 +387,40 @@ def delete(self, existing_resources): print('Deleting role name %s' % role_name) self._aws_client.delete_role(role_name) + def _delete_api_handler(self, existing_resources): + # type: (DeployedResources) -> None + handler_name = existing_resources.api_handler_name + print('Deleting lambda function %s' % handler_name) + try: + self._aws_client.delete_function(handler_name) + except ResourceDoesNotExistError as e: + print('No lambda function named %s found.' % e) + + def _delete_auth_handlers(self, existing_resources): + # type: (DeployedResources) -> None + if not existing_resources.lambda_functions: + return + for function_arn in existing_resources.lambda_functions.values(): + # We could use the key names, but we're using the + # Lambda ARNs to ensure we have the right lambda + # function. + try: + self._aws_client.delete_function(function_arn) + except ResourceDoesNotExistError as e: + print('No lambda function named %s found.' % e) + def deploy(self, config, existing_resources, stage_name): # type: (Config, OPT_RESOURCES, str) -> Dict[str, Any] - deployed_values = {} + deployed_values = {} # type: Dict[str, Any] + self._deploy_api_handler(config, existing_resources, stage_name, + deployed_values) + self._deploy_auth_handlers(config, existing_resources, stage_name, + deployed_values) + return deployed_values + + def _deploy_api_handler(self, config, existing_resources, stage_name, + deployed_values): + # type: (Config, OPT_RESOURCES, str, Dict[str, Any]) -> None if existing_resources is not None and \ self._aws_client.lambda_function_exists( existing_resources.api_handler_name): @@ -408,7 +436,48 @@ def deploy(self, config, existing_resources, stage_name): config, function_name, stage_name) deployed_values['api_handler_name'] = function_name deployed_values['api_handler_arn'] = function_arn - return deployed_values + + def _deploy_auth_handlers(self, config, existing_resources, stage_name, + deployed_values): + # type: (Config, OPT_RESOURCES, str, Dict[str, Any]) -> None + # The method makes the assumption that _deploy_api_handler + # has already been called. As a result, it reused portions of that + # functions configuration: + auth_handlers = config.chalice_app.builtin_auth_handlers + if not auth_handlers: + return + for auth_config in auth_handlers: + self._deploy_auth_handler( + config, auth_config, stage_name, deployed_values) + + def _deploy_auth_handler(self, config, auth_config, + stage_name, deployed_values): + # type: (Config, app.BuiltinAuthConfig, str, Dict[str, Any]) -> None + api_handler_name = deployed_values['api_handler_name'] + role_arn = self._get_or_create_lambda_role_arn( + config, api_handler_name) + zip_contents = self._osutils.get_file_contents( + self._packager.deployment_package_filename(config.project_dir), + binary=True) + function_name = api_handler_name + '-' + auth_config.name + if self._aws_client.lambda_function_exists(function_name): + response = self._update_lambda_function( + config, function_name, stage_name) + function_arn = response['FunctionArn'] + else: + function_arn = self._aws_client.create_function( + function_name=function_name, + role_arn=role_arn, + zip_contents=zip_contents, + environment_variables=config.environment_variables, + runtime=config.lambda_python_version, + handler=auth_config.handler_string, + tags=config.tags, + timeout=self._get_lambda_timeout(config), + memory_size=self._get_lambda_memory_size(config), + ) + deployed_values.setdefault( + 'lambda_functions', {})[function_name] = function_arn def _confirm_any_runtime_changes(self, config, handler_name): # type: (Config, str) -> None @@ -511,7 +580,7 @@ def _get_lambda_memory_size(self, config): return config.lambda_memory_size def _update_lambda_function(self, config, lambda_name, stage_name): - # type: (Config, str, str) -> None + # type: (Config, str, str) -> Dict[str, Any] print("Updating lambda function...") project_dir = config.project_dir packager = self._packager @@ -527,7 +596,7 @@ def _update_lambda_function(self, config, lambda_name, stage_name): deployment_package_filename, binary=True) role_arn = self._get_or_create_lambda_role_arn(config, lambda_name) print("Sending changes to lambda.") - self._aws_client.update_function( + return self._aws_client.update_function( function_name=lambda_name, zip_contents=zip_contents, runtime=config.lambda_python_version, @@ -634,6 +703,12 @@ def _deploy_api_to_stage(self, rest_api_id, api_gateway_stage, rest_api_id, str(uuid.uuid4()), ) + lambda_functions = deployed_resources.get('lambda_functions', {}) + if lambda_functions: + # Assuming these are just authorizers for now. + for function_arn in lambda_functions.values(): + self._aws_client.add_permission_for_authorizer( + rest_api_id, function_arn, str(uuid.uuid4())) class ApplicationPolicyHandler(object): diff --git a/chalice/deploy/swagger.py b/chalice/deploy/swagger.py index 011f005d5..7730aeda1 100644 --- a/chalice/deploy/swagger.py +++ b/chalice/deploy/swagger.py @@ -1,8 +1,9 @@ import copy -from typing import Any, List, Dict # noqa +from typing import Any, List, Dict, Optional # noqa from chalice.app import Chalice, RouteEntry, Authorizer, CORSConfig # noqa +from chalice.app import ChaliceAuthorizer class SwaggerGenerator(object): @@ -67,7 +68,27 @@ def _add_route_paths(self, api, app): def _generate_security_from_auth_obj(self, api_config, authorizer): # type: (Dict[str, Any], Authorizer) -> None - config = authorizer.to_swagger() + if isinstance(authorizer, ChaliceAuthorizer): + function_name = '%s-%s' % ( + self._deployed_resources['api_handler_name'], + authorizer.config.name + ) + arn = self._deployed_resources['lambda_functions'][function_name] + auth_config = authorizer.config + config = { + 'in': 'header', + 'type': 'apiKey', + 'name': 'Authorization', + 'x-amazon-apigateway-authtype': 'custom', + 'x-amazon-apigateway-authorizer': { + 'type': 'token', + 'authorizerCredentials': auth_config.execution_role, + 'authorizerUri': self._uri(arn), + 'authorizerResultTtlInSeconds': auth_config.ttl_seconds, + } + } + else: + config = authorizer.to_swagger() api_config.setdefault( 'securityDefinitions', {})[authorizer.name] = config @@ -156,9 +177,10 @@ def _generate_precanned_responses(self): } return responses - def _uri(self): - # type: () -> Any - lambda_arn = self._deployed_resources['api_handler_arn'] + def _uri(self, lambda_arn=None): + # type: (Optional[str]) -> Any + if lambda_arn is None: + lambda_arn = self._deployed_resources['api_handler_arn'] return ('arn:aws:apigateway:{region}:lambda:path/2015-03-31' '/functions/{lambda_arn}/invocations').format( region=self._region, lambda_arn=lambda_arn) @@ -230,8 +252,8 @@ def _add_preflight_request(self, cors, methods, swagger_for_path): class CFNSwaggerGenerator(SwaggerGenerator): - def _uri(self): - # type: () -> Any + def _uri(self, lambda_arn=None): + # type: (Optional[str]) -> Any # TODO: Does this have to be return type Any? return { 'Fn::Sub': ( diff --git a/tests/functional/cli/test_cli.py b/tests/functional/cli/test_cli.py index 820db7beb..331233c26 100644 --- a/tests/functional/cli/test_cli.py +++ b/tests/functional/cli/test_cli.py @@ -251,7 +251,8 @@ def test_can_retrieve_url(runner, mock_cli_factory): "backend": "api", "api_handler_name": "helloworld-dev", "api_handler_arn": "arn:...", - "api_gateway_stage": "dev-apig" + "api_gateway_stage": "dev-apig", + "lambda_functions": {}, }, "prod": { "rest_api_id": "rest_api_id_prod", @@ -260,7 +261,8 @@ def test_can_retrieve_url(runner, mock_cli_factory): "backend": "api", "api_handler_name": "helloworld-dev", "api_handler_arn": "arn:...", - "api_gateway_stage": "prod-apig" + "api_gateway_stage": "prod-apig", + "lambda_functions": {}, }, } with runner.isolated_filesystem(): diff --git a/tests/functional/test_awsclient.py b/tests/functional/test_awsclient.py index 2baec36d0..d08e3cc73 100644 --- a/tests/functional/test_awsclient.py +++ b/tests/functional/test_awsclient.py @@ -927,43 +927,100 @@ def test_can_add_permission_when_policy_does_not_exist(self, stubbed_session): 'name', 'us-west-2', '123', 'rest-api-id', 'random-id') stubbed_session.verify_stubs() - def test_get_sdk(self, stubbed_session): - apig = stubbed_session.stub('apigateway') - apig.get_sdk( - restApiId='rest-api-id', - stageName='dev', - sdkType='javascript').returns({'body': 'foo'}) + +class TestAddPermissionsForAuthorizer(object): + def test_can_add_permission_for_authorizer(self, stubbed_session): + apigateway = stubbed_session.stub('apigateway') + function_arn =( + 'arn:aws:lambda:us-west-2:1:function:app-dev-name' + ) + good_arn = ( + 'arn:aws:apigateway:us-west-2:lambda:path/2015-03-31/functions/' + '%s/invocations' % function_arn + ) + apigateway.get_authorizers(restApiId='rest-api-id').returns({ + 'items': [ + {'authorizerUri': 'not:arn', 'id': 'bad'}, + {'authorizerUri': good_arn, 'id': 'good'}, + ] + }) + source_arn = 'james' + source_arn = ( + 'arn:aws:execute-api:us-west-2:1:rest-api-id/authorizers/good' + ) + # We should call the appropriate add_permission call. + lambda_client = stubbed_session.stub('lambda') + lambda_client.add_permission( + Action='lambda:InvokeFunction', + FunctionName='app-dev-name', + StatementId='random-id', + Principal='apigateway.amazonaws.com', + SourceArn=source_arn + ).returns({}) stubbed_session.activate_stubs() - awsclient = TypedAWSClient(stubbed_session) - response = awsclient.get_sdk_download_stream( - 'rest-api-id', 'dev', 'javascript') - stubbed_session.verify_stubs() - assert response == 'foo' - def test_import_rest_api(self, stubbed_session): - apig = stubbed_session.stub('apigateway') - swagger_doc = {'swagger': 'doc'} - apig.import_rest_api( - body=json.dumps(swagger_doc, indent=2)).returns( - {'id': 'rest_api_id'}) + TypedAWSClient(stubbed_session).add_permission_for_authorizer( + 'rest-api-id', function_arn, 'random-id' + ) + stubbed_session.verify_stubs() + def test_value_error_raised_for_unknown_function(self, stubbed_session): + apigateway = stubbed_session.stub('apigateway') + apigateway.get_authorizers(restApiId='rest-api-id').returns({ + 'items': [ + {'authorizerUri': 'not:arn', 'id': 'bad'}, + {'authorizerUri': 'also-not:arn', 'id': 'alsobad'}, + ] + }) stubbed_session.activate_stubs() - awsclient = TypedAWSClient(stubbed_session) - rest_api_id = awsclient.import_rest_api(swagger_doc) + + unknown_function_arn = 'function:arn' + with pytest.raises(ValueError): + TypedAWSClient(stubbed_session).add_permission_for_authorizer( + 'rest-api-id', unknown_function_arn, 'random-id' + ) stubbed_session.verify_stubs() - assert rest_api_id == 'rest_api_id' - def test_update_api_from_swagger(self, stubbed_session): - apig = stubbed_session.stub('apigateway') - swagger_doc = {'swagger': 'doc'} - apig.put_rest_api( - restApiId='rest_api_id', - mode='overwrite', - body=json.dumps(swagger_doc, indent=2)).returns({}) - stubbed_session.activate_stubs() - awsclient = TypedAWSClient(stubbed_session) +def test_get_sdk(stubbed_session): + apig = stubbed_session.stub('apigateway') + apig.get_sdk( + restApiId='rest-api-id', + stageName='dev', + sdkType='javascript').returns({'body': 'foo'}) + stubbed_session.activate_stubs() + awsclient = TypedAWSClient(stubbed_session) + response = awsclient.get_sdk_download_stream( + 'rest-api-id', 'dev', 'javascript') + stubbed_session.verify_stubs() + assert response == 'foo' - awsclient.update_api_from_swagger('rest_api_id', - swagger_doc) - stubbed_session.verify_stubs() + +def test_import_rest_api(stubbed_session): + apig = stubbed_session.stub('apigateway') + swagger_doc = {'swagger': 'doc'} + apig.import_rest_api( + body=json.dumps(swagger_doc, indent=2)).returns( + {'id': 'rest_api_id'}) + + stubbed_session.activate_stubs() + awsclient = TypedAWSClient(stubbed_session) + rest_api_id = awsclient.import_rest_api(swagger_doc) + stubbed_session.verify_stubs() + assert rest_api_id == 'rest_api_id' + + +def test_update_api_from_swagger(stubbed_session): + apig = stubbed_session.stub('apigateway') + swagger_doc = {'swagger': 'doc'} + apig.put_rest_api( + restApiId='rest_api_id', + mode='overwrite', + body=json.dumps(swagger_doc, indent=2)).returns({}) + + stubbed_session.activate_stubs() + awsclient = TypedAWSClient(stubbed_session) + + awsclient.update_api_from_swagger('rest_api_id', + swagger_doc) + stubbed_session.verify_stubs() diff --git a/tests/integration/test_features.py b/tests/integration/test_features.py index f7f9c81d3..86c0d39b2 100644 --- a/tests/integration/test_features.py +++ b/tests/integration/test_features.py @@ -327,6 +327,37 @@ def test_can_handle_charset(smoke_test_app): assert response.status_code == 200 +def test_can_use_builtin_custom_auth(smoke_test_app): + url = smoke_test_app.url + '/builtin-auth' + # First time without an Auth header, we should fail. + response = requests.get(url) + assert response.status_code == 401 + # Now with the proper auth header, things should work. + response = requests.get(url, headers={'Authorization': 'yes'}) + assert response.status_code == 200 + context = response.json()['context'] + assert 'authorizer' in context + # The keyval context we added shuld also be in the authorizer + # dict. + assert context['authorizer']['foo'] == 'bar' + + +def test_can_use_shared_auth(smoke_test_app): + url = smoke_test_app.url + '/fake-profile' + response = requests.get(url) + # GETs are allowed + assert response.status_code == 200 + # However, POSTs require auth. + # This has the same auth config as /builtin-auth, + # so we're testing the auth handler can be shared. + assert requests.post(url).status_code == 401 + response = requests.post(url, headers={'Authorization': 'yes'}) + assert response.status_code == 200 + context = response.json()['context'] + assert 'authorizer' in context + assert context['authorizer']['foo'] == 'bar' + + @pytest.mark.on_redeploy def test_redeploy_no_change_view(smoke_test_app): smoke_test_app.redeploy_once() diff --git a/tests/integration/testapp/app.py b/tests/integration/testapp/app.py index a2da72ba0..d12205acf 100644 --- a/tests/integration/testapp/app.py +++ b/tests/integration/testapp/app.py @@ -1,5 +1,5 @@ from chalice import Chalice, BadRequestError, NotFoundError, Response,\ - CORSConfig + CORSConfig, UnauthorizedError, AuthResponse, AuthRoute try: from urllib.parse import parse_qs @@ -14,6 +14,19 @@ app.api.binary_types.append('application/binary') +@app.authorizer(ttl_seconds=300) +def dummy_auth(auth_request): + if auth_request.token == 'yes': + return AuthResponse( + routes=['/builtin-auth', + AuthRoute('/fake-profile', methods=['POST'])], + context={'foo': 'bar'}, + principal_id='foo' + ) + else: + raise UnauthorizedError('Authorization failed') + + @app.route('/') def index(): return {'hello': 'world'} @@ -148,3 +161,22 @@ def shared_get(): @app.route('/shared', methods=['POST']) def shared_post(): return {'method': 'POST'} + + +@app.route('/builtin-auth', authorizer=dummy_auth) +def builtin_auth(): + return {'success': True, 'context': app.current_request.context} + + +# Testing a common use case where you can have read only GET access +# but you need to be auth'd to POST. + +@app.route('/fake-profile', methods=['GET']) +def fake_profile_read_only(): + return {'success': True, 'context': app.current_request.context} + + +@app.route('/fake-profile', authorizer=dummy_auth, + methods=['POST']) +def fake_profile_post(): + return {'success': True, 'context': app.current_request.context} diff --git a/tests/unit/conftest.py b/tests/unit/conftest.py index 20a81126f..6c087dd74 100644 --- a/tests/unit/conftest.py +++ b/tests/unit/conftest.py @@ -17,3 +17,19 @@ def foo(): return {} return app + + +@fixture +def sample_app_with_auth(): + app = Chalice('sampleauth') + + @app.authorizer('myauth') + def myauth(auth_request): + pass + + + @app.route('/', authorizer=myauth) + def foo(): + return {} + + return app diff --git a/tests/unit/deploy/test_deployer.py b/tests/unit/deploy/test_deployer.py index 8e824ff7f..202d98951 100644 --- a/tests/unit/deploy/test_deployer.py +++ b/tests/unit/deploy/test_deployer.py @@ -143,7 +143,7 @@ def test_api_gateway_deployer_redeploy_api(config_obj): # The rest_api_id does not exist which will trigger # the initial import deployed = DeployedResources( - None, None, None, 'existing-id', 'dev', None, None) + None, None, None, 'existing-id', 'dev', None, None, None) aws_client.rest_api_exists.return_value = True lambda_arn = 'arn:aws:lambda:us-west-2:account-id:function:func-name' @@ -167,7 +167,7 @@ def test_api_gateway_deployer_delete(config_obj): rest_api_id = 'abcdef1234' deployed = DeployedResources( - None, None, None, rest_api_id, 'dev', None, None) + None, None, None, rest_api_id, 'dev', None, None, None) aws_client.rest_api_exists.return_value = True d = APIGatewayDeployer(aws_client) @@ -181,7 +181,7 @@ def test_api_gateway_deployer_delete_already_deleted(capsys): aws_client.delete_rest_api.side_effect = ResourceDoesNotExistError( rest_api_id) deployed = DeployedResources( - None, None, None, rest_api_id, 'dev', None, None) + None, None, None, rest_api_id, 'dev', None, None, None) aws_client.rest_api_exists.return_value = True d = APIGatewayDeployer(aws_client) d.delete(deployed) @@ -546,6 +546,7 @@ def test_deployer_returns_deployed_resources(self, sample_app): } } + def test_deployer_delete_calls_deletes(self): # Check that athe deployer class calls other deployer classes delete # methods. @@ -560,6 +561,31 @@ def test_deployer_delete_calls_deletes(self): 'api_gateway_stage': 'dev', 'region': 'us-west-2', 'chalice_version': '0', + 'lambda_functions': {}, + }) + cfg.deployed_resources.return_value = deployed_resources + + d = Deployer(apig_deploy, lambda_deploy) + d.delete(cfg) + + lambda_deploy.delete.assert_called_with(deployed_resources) + apig_deploy.delete.assert_called_with(deployed_resources) + + def test_deployer_delete_calls_deletes(self): + # Check that the deployer class calls other deployer classes delete + # methods. + lambda_deploy = mock.Mock(spec=LambdaDeployer) + apig_deploy = mock.Mock(spec=APIGatewayDeployer) + cfg = mock.Mock(spec=Config) + deployed_resources = DeployedResources.from_dict({ + 'backend': 'api', + 'api_handler_arn': 'lambda_arn', + 'api_handler_name': 'lambda_name', + 'rest_api_id': 'rest_id', + 'api_gateway_stage': 'dev', + 'region': 'us-west-2', + 'chalice_version': '0', + 'lambda_functions': {}, }) cfg.deployed_resources.return_value = deployed_resources @@ -700,7 +726,7 @@ def test_lambda_deployer_repeated_deploy(app_policy, sample_app): lambda_function_name = 'lambda_function_name' deployed = DeployedResources( 'api', 'api_handler_arn', lambda_function_name, - None, 'dev', None, None) + None, 'dev', None, None, None) d.deploy(cfg, deployed, 'dev') # Should result in injecting the latest app code. @@ -726,16 +752,19 @@ def test_lambda_deployer_repeated_deploy(app_policy, sample_app): def test_lambda_deployer_delete(): aws_client = mock.Mock(spec=TypedAWSClient) aws_client.get_role_arn_for_name.return_value = 'arn_prefix/role_name' - lambda_function_name = 'lambda_name' + lambda_function_name = 'api-handler' deployed = DeployedResources( 'api', 'api_handler_arn/lambda_name', lambda_function_name, - None, 'dev', None, None) + None, 'dev', None, None, {'name': 'auth-arn'}) d = LambdaDeployer( aws_client, None, CustomConfirmPrompt(True), None, None) d.delete(deployed) aws_client.get_role_arn_for_name.assert_called_with(lambda_function_name) - aws_client.delete_function.assert_called_with(lambda_function_name) + assert aws_client.delete_function.call_args_list == [ + mock.call('api-handler'), + mock.call('auth-arn'), + ] aws_client.delete_role.assert_called_with('role_name') @@ -747,7 +776,7 @@ def test_lambda_deployer_delete_already_deleted(capsys): lambda_function_name) deployed = DeployedResources( 'api', 'api_handler_arn/lambda_name', lambda_function_name, - None, 'dev', None, None) + None, 'dev', None, None, None) d = LambdaDeployer( aws_client, None, NoPrompt(), None, None) d.delete(deployed) @@ -785,7 +814,7 @@ def test_prompted_on_runtime_change_can_reject_change(app_policy, sample_app): lambda_function_name = 'lambda_function_name' deployed = DeployedResources( 'api', 'api_handler_arn', lambda_function_name, - None, 'dev', None, None) + None, 'dev', None, None, None) with pytest.raises(RuntimeError): d.deploy(cfg, deployed, 'dev') @@ -955,6 +984,36 @@ def index(): sample_app.api.binary_types) is None +class TestAuthHandlersAreAuthorized(object): + def tests_apigateway_adds_auth_handler_policy(self, sample_app_with_auth): + # When we create authorizers in API gateway, we also need to + # give the authorizers permission to invoke the lambda functions + # we've created. + aws_client = mock.Mock(spec=TypedAWSClient, region_name='us-west-2') + cfg = Config.create( + chalice_stage='dev', app_name='myapp', + chalice_app=sample_app_with_auth, + manage_iam_role=False, iam_role_arn='role-arn', + project_dir='.' + ) + d = APIGatewayDeployer(aws_client) + deployed_resources = { + 'api_handler_arn': ( + 'arn:aws:lambda:us-west-2:1:function:myapp-dev' + ), + 'api_handler_name': 'myapp-dev', + 'lambda_functions': { + 'myapp-dev-myauth': 'myauth:arn', + }, + } + aws_client.import_rest_api.return_value = 'rest-api-id' + d.deploy(cfg, None, deployed_resources) + # We should have add permission for the authorizer to invoke + # the auth lambda function. + aws_client.add_permission_for_authorizer.assert_called_with( + 'rest-api-id', 'myauth:arn', mock.ANY) + + class TestLambdaInitialDeploymentWithConfigurations(object): @fixture(autouse=True) def setup_deployer_dependencies(self, app_policy): @@ -971,18 +1030,53 @@ def setup_deployer_dependencies(self, app_policy): self.osutils = InMemoryOSUtils( {self.package_name: self.package_contents}) self.aws_client = mock.Mock(spec=TypedAWSClient) - self.aws_client.create_function.return_value = self.lambda_arn + self.aws_client.create_function.side_effect = [self.lambda_arn] self.packager = mock.Mock(LambdaDeploymentPackager) self.packager.create_deployment_package.return_value =\ self.package_name + self.packager.deployment_package_filename.return_value =\ + self.package_name self.app_policy = app_policy - def test_lambda_deployer_defaults(self, sample_app): + def create_config_obj(self, sample_app): cfg = Config.create( chalice_stage='dev', app_name='myapp', chalice_app=sample_app, manage_iam_role=False, iam_role_arn='role-arn', project_dir='.' ) + return cfg + + def test_can_create_auth_handlers(self, sample_app_with_auth): + config = self.create_config_obj(sample_app_with_auth) + deployer = LambdaDeployer( + self.aws_client, self.packager, None, self.osutils, + self.app_policy) + self.aws_client.lambda_function_exists.return_value = False + self.aws_client.create_function.side_effect = [ + self.lambda_arn, 'arn:auth-function'] + deployed = deployer.deploy(config, None, stage_name='dev') + assert 'lambda_functions' in deployed + assert deployed['lambda_functions'] == { + 'myapp-dev-myauth': 'arn:auth-function', + } + + def test_can_update_auth_handlers(self, sample_app_with_auth): + config = self.create_config_obj(sample_app_with_auth) + deployer = LambdaDeployer( + self.aws_client, self.packager, None, self.osutils, + self.app_policy) + self.aws_client.lambda_function_exists.return_value = True + self.aws_client.update_function.return_value = { + 'FunctionArn': 'arn:auth-function' + } + deployed = deployer.deploy(config, None, stage_name='dev') + assert 'lambda_functions' in deployed + assert deployed['lambda_functions'] == { + 'myapp-dev-myauth': 'arn:auth-function', + } + + def test_lambda_deployer_defaults(self, sample_app): + cfg = self.create_config_obj(sample_app) deployer = LambdaDeployer( self.aws_client, self.packager, None, self.osutils, self.app_policy) @@ -1136,7 +1230,7 @@ def setup_deployer_dependencies(self, app_policy): self.deployed_resources = DeployedResources( 'api', 'api_handler_arn', self.lambda_function_name, - None, 'dev', None, None) + None, 'dev', None, None, None) def test_lambda_deployer_defaults(self, sample_app): cfg = Config.create( diff --git a/tests/unit/deploy/test_swagger.py b/tests/unit/deploy/test_swagger.py index 1101b57af..3f34e3d71 100644 --- a/tests/unit/deploy/test_swagger.py +++ b/tests/unit/deploy/test_swagger.py @@ -1,6 +1,7 @@ from chalice.deploy.swagger import SwaggerGenerator from chalice import CORSConfig -from chalice.app import CustomAuthorizer, CognitoUserPoolAuthorizer, IAMAuthorizer, Chalice +from chalice.app import CustomAuthorizer, CognitoUserPoolAuthorizer +from chalice.app import IAMAuthorizer, Chalice import mock import pytest @@ -438,3 +439,42 @@ def bar(): doc = swagger_gen.generate_swagger(sample_app) assert 'securityDefinitions' in doc assert len(doc['securityDefinitions']) == 1 + + +def test_builtin_auth(sample_app): + swagger_gen = SwaggerGenerator( + region='us-west-2', + deployed_resources={ + 'api_handler_arn': 'lambda_arn', + 'api_handler_name': 'api-dev', + 'lambda_functions': { + 'api-dev-myauth': 'auth_arn', + } + } + ) + + @sample_app.authorizer(name='myauth', + ttl_seconds=10, + execution_role='arn:role') + def auth(auth_request): + pass + + @sample_app.route('/auth', authorizer=auth) + def foo(): + pass + + doc = swagger_gen.generate_swagger(sample_app) + assert 'securityDefinitions' in doc + assert doc['securityDefinitions']['myauth'] == { + 'in': 'header', + 'name': 'Authorization', + 'type': 'apiKey', + 'x-amazon-apigateway-authtype': 'custom', + 'x-amazon-apigateway-authorizer': { + 'type': 'token', + 'authorizerCredentials': 'arn:role', + 'authorizerResultTtlInSeconds': 10, + 'authorizerUri': ('arn:aws:apigateway:us-west-2:lambda:path' + '/2015-03-31/functions/auth_arn/invocations'), + } + } diff --git a/tests/unit/test_app.py b/tests/unit/test_app.py index c6d89ca0b..46807736b 100644 --- a/tests/unit/test_app.py +++ b/tests/unit/test_app.py @@ -80,6 +80,14 @@ def name(name): return demo +@fixture +def auth_request(): + method_arn = ( + "arn:aws:execute-api:us-west-2:123:rest-api-id/dev/GET/needs/auth") + request = app.AuthRequest('TOKEN', 'authtoken', method_arn) + return request + + @pytest.mark.skipif(sys.version[0] == '2', reason=('Test is irrelevant under python 2, since str and ' 'bytes are interchangable.')) @@ -773,3 +781,192 @@ def test_eq_non_default_configurations(self): allow_credentials=True ) assert custom_cors == same_custom_cors + + +def test_can_handle_builtin_auth(): + demo = app.Chalice('builtin-auth') + + @demo.authorizer() + def my_auth(auth_request): + pass + + + @demo.route('/', authorizer=my_auth) + def index_view(): + return {} + + assert len(demo.builtin_auth_handlers) == 1 + authorizer = demo.builtin_auth_handlers[0] + assert isinstance(authorizer, app.BuiltinAuthConfig) + assert authorizer.name == 'my_auth' + assert authorizer.handler_string == 'app.my_auth' + + +def test_builtin_auth_can_transform_event(): + event = { + 'type': 'TOKEN', + 'authorizationToken': 'authtoken', + 'methodArn': 'arn:aws:execute-api:...:foo', + } + auth_app = app.Chalice('builtin-auth') + + request = [] + + @auth_app.authorizer() + def builtin_auth(auth_request): + request.append(auth_request) + + builtin_auth(event, None) + + assert len(request) == 1 + transformed = request[0] + assert transformed.auth_type == 'TOKEN' + assert transformed.token == 'authtoken' + assert transformed.method_arn == 'arn:aws:execute-api:...:foo' + + +def test_can_return_auth_dict_directly(): + # A user can bypass our AuthResponse and return the auth response + # dict that API gateway expects. + event = { + 'type': 'TOKEN', + 'authorizationToken': 'authtoken', + 'methodArn': 'arn:aws:execute-api:...:foo', + } + auth_app = app.Chalice('builtin-auth') + + response = { + 'context': {'foo': 'bar'}, + 'principalId': 'user', + 'policyDocument': { + 'Version': '2012-10-17', + 'Statement': [] + } + } + + @auth_app.authorizer() + def builtin_auth(auth_request): + return response + + actual = builtin_auth(event, None) + assert actual == response + + +def test_can_specify_extra_auth_attributes(): + auth_app = app.Chalice('builtin-auth') + + @auth_app.authorizer(ttl_seconds=10, execution_role='arn:my-role') + def builtin_auth(auth_request): + pass + + handler = auth_app.builtin_auth_handlers[0] + assert handler.ttl_seconds == 10 + assert handler.execution_role == 'arn:my-role' + + +def test_can_return_auth_response(): + event = { + 'type': 'TOKEN', + 'authorizationToken': 'authtoken', + 'methodArn': 'arn:aws:execute-api:us-west-2:1:id/dev/GET/a', + } + auth_app = app.Chalice('builtin-auth') + + response = { + 'context': {}, + 'principalId': 'principal', + 'policyDocument': { + 'Version': '2012-10-17', + 'Statement': [ + {'Action': 'execute-api:Invoke', + 'Effect': 'Allow', + 'Resource': [ + 'arn:aws:execute-api:us-west-2:1:id/dev/%s/a' % + method for method in app.AuthResponse.ALL_HTTP_METHODS + ]} + ] + } + } + + @auth_app.authorizer() + def builtin_auth(auth_request): + return app.AuthResponse(['/a'], 'principal') + + actual = builtin_auth(event, None) + assert actual == response + + +def test_auth_response_serialization(): + method_arn = ( + "arn:aws:execute-api:us-west-2:123:rest-api-id/dev/GET/needs/auth") + request = app.AuthRequest('TOKEN', 'authtoken', method_arn) + response = app.AuthResponse(routes=['/needs/auth'], principal_id='foo') + response_dict = response.to_dict(request) + expected = [ + method_arn.replace('GET', method) + for method in app.AuthResponse.ALL_HTTP_METHODS + ] + assert response_dict == { + 'policyDocument': { + 'Version': '2012-10-17', + 'Statement': [ + { + 'Action': 'execute-api:Invoke', + 'Resource': expected, + 'Effect': 'Allow' + } + ] + }, + 'context': {}, + 'principalId': 'foo', + } + + +def test_auth_response_can_include_context(auth_request): + response = app.AuthResponse(['/foo'], 'principal', {'foo': 'bar'}) + serialized = response.to_dict(auth_request) + assert serialized['context'] == {'foo': 'bar'} + + +def test_can_use_auth_routes_instead_of_strings(auth_request): + expected = [ + "arn:aws:execute-api:us-west-2:123:rest-api-id/dev/GET/a", + "arn:aws:execute-api:us-west-2:123:rest-api-id/dev/GET/a/b", + "arn:aws:execute-api:us-west-2:123:rest-api-id/dev/POST/a/b", + ] + response = app.AuthResponse( + [app.AuthRoute('/a', ['GET']), + app.AuthRoute('/a/b', ['GET', 'POST'])], + 'principal') + serialized = response.to_dict(auth_request) + assert serialized['policyDocument'] == { + 'Version': '2012-10-17', + 'Statement': [{ + 'Action': 'execute-api:Invoke', + 'Effect': 'Allow', + 'Resource': expected, + }] + } + + +def test_special_cased_root_resource(auth_request): + # Not sure why, but API gateway uses `//` for the root + # resource. I've confirmed it doesn't do this for non-root + # URLs. We don't to let that leak out to the APIs we expose. + auth_request.method_arn = ( + "arn:aws:execute-api:us-west-2:123:rest-api-id/dev/GET//") + expected = [ + "arn:aws:execute-api:us-west-2:123:rest-api-id/dev/GET//" + ] + response = app.AuthResponse( + [app.AuthRoute('/', ['GET'])], + 'principal') + serialized = response.to_dict(auth_request) + assert serialized['policyDocument'] == { + 'Version': '2012-10-17', + 'Statement': [{ + 'Action': 'execute-api:Invoke', + 'Effect': 'Allow', + 'Resource': expected, + }] + } diff --git a/tests/unit/test_config.py b/tests/unit/test_config.py index a4fba9cd3..961814072 100644 --- a/tests/unit/test_config.py +++ b/tests/unit/test_config.py @@ -98,6 +98,7 @@ def test_can_create_deployed_resource_from_dict(): 'api_gateway_stage': 'stage', 'region': 'region', 'chalice_version': '1.0.0', + 'lambda_functions': {}, }) assert d.backend == 'api' assert d.api_handler_arn == 'arn' @@ -106,6 +107,7 @@ def test_can_create_deployed_resource_from_dict(): assert d.api_gateway_stage == 'stage' assert d.region == 'region' assert d.chalice_version == '1.0.0' + assert d.lambda_functions == {} def test_environment_from_top_level(): From 33cf604af91b25bd4c074ed314e39bffc26344b8 Mon Sep 17 00:00:00 2001 From: James Saryerwinnie Date: Mon, 19 Jun 2017 13:13:37 -0700 Subject: [PATCH 02/11] Incorporate review feedback * Use ResourceDoesNotExistError * Provide defaults for random_id param in awsclient * Add note about API gateway authorizer pagination * Validate unknown kwargs to authorizer() * Test expected kwargs passed to create/update function * Add test for mixed string and AuthRoutes in AuthResponse --- chalice/app.py | 6 ++- chalice/awsclient.py | 28 ++++++++++---- chalice/deploy/deployer.py | 22 ++++++----- tests/functional/test_awsclient.py | 62 +++++++++++++++++++++++------- tests/unit/deploy/test_deployer.py | 51 ++++++++++++------------ tests/unit/test_app.py | 34 ++++++++++++++++ 6 files changed, 146 insertions(+), 57 deletions(-) diff --git a/chalice/app.py b/chalice/app.py index 95afd1c03..151329b8d 100644 --- a/chalice/app.py +++ b/chalice/app.py @@ -469,14 +469,16 @@ def _register_authorizer(auth_func): auth_name = auth_func.__name__ ttl_seconds = kwargs.pop('ttl_seconds', None) execution_role = kwargs.pop('execution_role', None) + if kwargs: + raise TypeError( + 'TypeError: authorizer() got unexpected keyword ' + 'arguments: %s' % ', '.join(list(kwargs))) auth_config = BuiltinAuthConfig( name=auth_name, handler_string='app.%s' % auth_func.__name__, ttl_seconds=ttl_seconds, execution_role=execution_role, ) - # Do we even need the builtin_auth_handlers attr? See if we can - # remove this and calculate this list on demand. self.builtin_auth_handlers.append(auth_config) return ChaliceAuthorizer(name, auth_func, auth_config) return _register_authorizer diff --git a/chalice/awsclient.py b/chalice/awsclient.py index a0908a846..71ef56ade 100644 --- a/chalice/awsclient.py +++ b/chalice/awsclient.py @@ -21,6 +21,7 @@ import shutil import json import re +import uuid import botocore.session # noqa from botocore.exceptions import ClientError @@ -275,7 +276,7 @@ def get_role_arn_for_name(self, name): try: role = client.get_role(RoleName=name) except client.exceptions.NoSuchEntityException: - raise ValueError("No role ARN found for: %s" % name) + raise ResourceDoesNotExistError("No role ARN found for: %s" % name) return role['Role']['Arn'] def delete_role_policy(self, role_name, policy_name): @@ -507,12 +508,14 @@ def get_sdk_download_stream(self, rest_api_id, return response['body'] def add_permission_for_apigateway(self, function_name, region_name, - account_id, rest_api_id, random_id): - # type: (str, str, str, str, str) -> None + account_id, rest_api_id, random_id=None): + # type: (str, str, str, str, Optional[str]) -> None """Authorize API gateway to invoke a lambda function.""" client = self._client('lambda') source_arn = self._build_source_arn_str(region_name, account_id, rest_api_id) + if random_id is None: + random_id = self._random_id() client.add_permission( Action='lambda:InvokeFunction', FunctionName=function_name, @@ -566,23 +569,30 @@ def _client(self, service_name): return self._client_cache[service_name] def add_permission_for_authorizer(self, rest_api_id, function_arn, - random_id): - # type: (str, str, str) -> None + random_id=None): + # type: (str, str, Optional[str]) -> None client = self._client('apigateway') + # This is actually a paginated operation, but botocore does not + # support this style of pagination right now. The max authorizers + # for an API is 10, so we're ok for now. We will need to circle + # back on this eventually. authorizers = client.get_authorizers(restApiId=rest_api_id) for authorizer in authorizers['items']: if function_arn in authorizer['authorizerUri']: authorizer_id = authorizer['id'] break else: - raise ValueError("Unable to find authorizer associated " - "with function ARN: %s" % function_arn) + raise ResourceDoesNotExistError( + "Unable to find authorizer associated " + "with function ARN: %s" % function_arn) parts = function_arn.split(':') region_name = parts[3] account_id = parts[4] function_name = parts[-1] source_arn = ("arn:aws:execute-api:%s:%s:%s/authorizers/%s" % (region_name, account_id, rest_api_id, authorizer_id)) + if random_id is None: + random_id = self._random_id() self._client('lambda').add_permission( Action='lambda:InvokeFunction', FunctionName=function_name, @@ -590,3 +600,7 @@ def add_permission_for_authorizer(self, rest_api_id, function_arn, Principal='apigateway.amazonaws.com', SourceArn=source_arn, ) + + def _random_id(self): + # type: () -> str + return str(uuid.uuid4()) diff --git a/chalice/deploy/deployer.py b/chalice/deploy/deployer.py index 13a18ff32..1f72ad4d8 100644 --- a/chalice/deploy/deployer.py +++ b/chalice/deploy/deployer.py @@ -391,10 +391,7 @@ def _delete_api_handler(self, existing_resources): # type: (DeployedResources) -> None handler_name = existing_resources.api_handler_name print('Deleting lambda function %s' % handler_name) - try: - self._aws_client.delete_function(handler_name) - except ResourceDoesNotExistError as e: - print('No lambda function named %s found.' % e) + self._delete_lambda_function(handler_name) def _delete_auth_handlers(self, existing_resources): # type: (DeployedResources) -> None @@ -404,10 +401,15 @@ def _delete_auth_handlers(self, existing_resources): # We could use the key names, but we're using the # Lambda ARNs to ensure we have the right lambda # function. - try: - self._aws_client.delete_function(function_arn) - except ResourceDoesNotExistError as e: - print('No lambda function named %s found.' % e) + self._delete_lambda_function(function_arn) + + def _delete_lambda_function(self, function_name_or_arn): + # type: (str) -> None + # Deletes a function and prints an error if deletion fails. + try: + self._aws_client.delete_function(function_name_or_arn) + except ResourceDoesNotExistError as e: + print('No lambda function named %s found.' % e) def deploy(self, config, existing_resources, stage_name): # type: (Config, OPT_RESOURCES, str) -> Dict[str, Any] @@ -497,7 +499,7 @@ def _get_lambda_role_arn(self, role_name): try: role_arn = self._aws_client.get_role_arn_for_name(role_name) return role_arn - except ValueError: + except ResourceDoesNotExistError: return None def _get_or_create_lambda_role_arn(self, config, role_name): @@ -512,7 +514,7 @@ def _get_or_create_lambda_role_arn(self, config, role_name): # We're using the lambda function_name as the role_name. role_arn = self._aws_client.get_role_arn_for_name(role_name) self._update_role_with_latest_policy(role_name, config) - except ValueError: + except ResourceDoesNotExistError: print("Creating role") role_arn = self._create_role_from_source_code(config, role_name) return role_arn diff --git a/tests/functional/test_awsclient.py b/tests/functional/test_awsclient.py index d08e3cc73..2f3e98146 100644 --- a/tests/functional/test_awsclient.py +++ b/tests/functional/test_awsclient.py @@ -5,9 +5,9 @@ import pytest import mock import botocore.exceptions -import botocore.session from botocore.vendored.requests import ConnectionError as \ RequestsConnectionError +from botocore import stub from chalice.awsclient import TypedAWSClient from chalice.awsclient import ResourceDoesNotExistError @@ -249,7 +249,7 @@ def test_got_role_arn_not_found_raises_value_error(self, stubbed_session): message='Foo') stubbed_session.activate_stubs() awsclient = TypedAWSClient(stubbed_session) - with pytest.raises(ValueError): + with pytest.raises(ResourceDoesNotExistError): awsclient.get_role_arn_for_name(name='Yes') stubbed_session.verify_stubs() @@ -869,6 +869,20 @@ def test_can_add_permission_for_apigateway(self, stubbed_session): 'function_name', 'us-west-2', '123', 'rest-api-id', 'random-id') stubbed_session.verify_stubs() + def test_random_id_can_be_omitted(self, stubbed_session): + stubbed_session.stub('lambda').add_permission( + Action='lambda:InvokeFunction', + FunctionName='function_name', + StatementId=stub.ANY, + Principal='apigateway.amazonaws.com', + SourceArn='arn:aws:execute-api:us-west-2:123:rest-api-id/*', + ).returns({}) + stubbed_session.activate_stubs() + TypedAWSClient(stubbed_session).add_permission_for_apigateway( + # random_id is omitted here. + 'function_name', 'us-west-2', '123', 'rest-api-id') + stubbed_session.verify_stubs() + def should_call_add_permission(self, lambda_stub): lambda_stub.add_permission( Action='lambda:InvokeFunction', @@ -929,22 +943,22 @@ def test_can_add_permission_when_policy_does_not_exist(self, stubbed_session): class TestAddPermissionsForAuthorizer(object): + FUNCTION_ARN =( + 'arn:aws:lambda:us-west-2:1:function:app-dev-name' + ) + GOOD_ARN = ( + 'arn:aws:apigateway:us-west-2:lambda:path/2015-03-31/functions/' + '%s/invocations' % FUNCTION_ARN + ) + def test_can_add_permission_for_authorizer(self, stubbed_session): apigateway = stubbed_session.stub('apigateway') - function_arn =( - 'arn:aws:lambda:us-west-2:1:function:app-dev-name' - ) - good_arn = ( - 'arn:aws:apigateway:us-west-2:lambda:path/2015-03-31/functions/' - '%s/invocations' % function_arn - ) apigateway.get_authorizers(restApiId='rest-api-id').returns({ 'items': [ {'authorizerUri': 'not:arn', 'id': 'bad'}, - {'authorizerUri': good_arn, 'id': 'good'}, + {'authorizerUri': self.GOOD_ARN, 'id': 'good'}, ] }) - source_arn = 'james' source_arn = ( 'arn:aws:execute-api:us-west-2:1:rest-api-id/authorizers/good' ) @@ -960,7 +974,29 @@ def test_can_add_permission_for_authorizer(self, stubbed_session): stubbed_session.activate_stubs() TypedAWSClient(stubbed_session).add_permission_for_authorizer( - 'rest-api-id', function_arn, 'random-id' + 'rest-api-id', self.FUNCTION_ARN, 'random-id' + ) + stubbed_session.verify_stubs() + + def test_random_id_can_be_omitted(self, stubbed_session): + stubbed_session.stub('apigateway').get_authorizers( + restApiId='rest-api-id').returns({ + 'items': [{'authorizerUri': self.GOOD_ARN, 'id': 'good'}]}) + source_arn = ( + 'arn:aws:execute-api:us-west-2:1:rest-api-id/authorizers/good' + ) + stubbed_session.stub('lambda').add_permission( + Action='lambda:InvokeFunction', + FunctionName='app-dev-name', + # Autogenerated value here. + StatementId=stub.ANY, + Principal='apigateway.amazonaws.com', + SourceArn=source_arn + ).returns({}) + stubbed_session.activate_stubs() + # Note the omission of the random id. + TypedAWSClient(stubbed_session).add_permission_for_authorizer( + 'rest-api-id', self.FUNCTION_ARN ) stubbed_session.verify_stubs() @@ -975,7 +1011,7 @@ def test_value_error_raised_for_unknown_function(self, stubbed_session): stubbed_session.activate_stubs() unknown_function_arn = 'function:arn' - with pytest.raises(ValueError): + with pytest.raises(ResourceDoesNotExistError): TypedAWSClient(stubbed_session).add_permission_for_authorizer( 'rest-api-id', unknown_function_arn, 'random-id' ) diff --git a/tests/unit/deploy/test_deployer.py b/tests/unit/deploy/test_deployer.py index 202d98951..92712cf71 100644 --- a/tests/unit/deploy/test_deployer.py +++ b/tests/unit/deploy/test_deployer.py @@ -22,6 +22,7 @@ from chalice.config import Config, DeployedResources from chalice.policy import AppPolicyGenerator from chalice.deploy.deployer import ChaliceDeploymentError +from chalice import constants from chalice.deploy.deployer import APIGatewayDeployer from chalice.deploy.deployer import ApplicationPolicyHandler from chalice.deploy.deployer import Deployer @@ -546,31 +547,6 @@ def test_deployer_returns_deployed_resources(self, sample_app): } } - - def test_deployer_delete_calls_deletes(self): - # Check that athe deployer class calls other deployer classes delete - # methods. - lambda_deploy = mock.Mock(spec=LambdaDeployer) - apig_deploy = mock.Mock(spec=APIGatewayDeployer) - cfg = mock.Mock(spec=Config) - deployed_resources = DeployedResources.from_dict({ - 'backend': 'api', - 'api_handler_arn': 'lambda_arn', - 'api_handler_name': 'lambda_name', - 'rest_api_id': 'rest_id', - 'api_gateway_stage': 'dev', - 'region': 'us-west-2', - 'chalice_version': '0', - 'lambda_functions': {}, - }) - cfg.deployed_resources.return_value = deployed_resources - - d = Deployer(apig_deploy, lambda_deploy) - d.delete(cfg) - - lambda_deploy.delete.assert_called_with(deployed_resources) - apig_deploy.delete.assert_called_with(deployed_resources) - def test_deployer_delete_calls_deletes(self): # Check that the deployer class calls other deployer classes delete # methods. @@ -1059,6 +1035,19 @@ def test_can_create_auth_handlers(self, sample_app_with_auth): assert deployed['lambda_functions'] == { 'myapp-dev-myauth': 'arn:auth-function', } + self.aws_client.create_function.assert_called_with( + environment_variables={}, + function_name='myapp-dev-myauth', + handler='app.myauth', + memory_size=constants.DEFAULT_LAMBDA_MEMORY_SIZE, + role_arn='role-arn', + # The python runtime versions are tested elsewhere. + runtime=mock.ANY, + # The tag format is tested elsewhere. + tags=mock.ANY, + timeout=constants.DEFAULT_LAMBDA_TIMEOUT, + zip_contents='package contents', + ) def test_can_update_auth_handlers(self, sample_app_with_auth): config = self.create_config_obj(sample_app_with_auth) @@ -1074,6 +1063,18 @@ def test_can_update_auth_handlers(self, sample_app_with_auth): assert deployed['lambda_functions'] == { 'myapp-dev-myauth': 'arn:auth-function', } + self.aws_client.update_function.assert_called_with( + environment_variables={}, + function_name='myapp-dev-myauth', + memory_size=constants.DEFAULT_LAMBDA_MEMORY_SIZE, + role_arn='role-arn', + # The python runtime versions are tested elsewhere. + runtime=mock.ANY, + # The tag format is tested elsewhere. + tags=mock.ANY, + timeout=constants.DEFAULT_LAMBDA_TIMEOUT, + zip_contents='package contents', + ) def test_lambda_deployer_defaults(self, sample_app): cfg = self.create_config_obj(sample_app) diff --git a/tests/unit/test_app.py b/tests/unit/test_app.py index 46807736b..9536f9955 100644 --- a/tests/unit/test_app.py +++ b/tests/unit/test_app.py @@ -864,6 +864,14 @@ def builtin_auth(auth_request): assert handler.execution_role == 'arn:my-role' +def test_validation_raised_on_unknown_kwargs(): + auth_app = app.Chalice('builtin-auth') + + with pytest.raises(TypeError): + @auth_app.authorizer(this_is_an_unknown_kwarg=True) + def builtin_auth(auth_request): + pass + def test_can_return_auth_response(): event = { 'type': 'TOKEN', @@ -949,6 +957,32 @@ def test_can_use_auth_routes_instead_of_strings(auth_request): } +def test_can_mix_auth_routes_and_strings(auth_request): + expected = [ + 'arn:aws:execute-api:us-west-2:123:rest-api-id/dev/DELETE/a', + 'arn:aws:execute-api:us-west-2:123:rest-api-id/dev/HEAD/a', + 'arn:aws:execute-api:us-west-2:123:rest-api-id/dev/OPTIONS/a', + 'arn:aws:execute-api:us-west-2:123:rest-api-id/dev/PATCH/a', + 'arn:aws:execute-api:us-west-2:123:rest-api-id/dev/POST/a', + 'arn:aws:execute-api:us-west-2:123:rest-api-id/dev/PUT/a', + 'arn:aws:execute-api:us-west-2:123:rest-api-id/dev/GET/a', + 'arn:aws:execute-api:us-west-2:123:rest-api-id/dev/GET/a/b', + ] + response = app.AuthResponse( + ['/a', app.AuthRoute('/a/b', ['GET'])], + 'principal') + serialized = response.to_dict(auth_request) + assert serialized['policyDocument'] == { + 'Version': '2012-10-17', + 'Statement': [{ + 'Action': 'execute-api:Invoke', + 'Effect': 'Allow', + 'Resource': expected, + }] + } + + + def test_special_cased_root_resource(auth_request): # Not sure why, but API gateway uses `//` for the root # resource. I've confirmed it doesn't do this for non-root From 8ff56f6f6083a9c175a207db8b1aa31269e8e322 Mon Sep 17 00:00:00 2001 From: James Saryerwinnie Date: Mon, 19 Jun 2017 14:18:42 -0700 Subject: [PATCH 03/11] Fix py3 test failures --- tests/unit/deploy/test_deployer.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/unit/deploy/test_deployer.py b/tests/unit/deploy/test_deployer.py index 92712cf71..470421601 100644 --- a/tests/unit/deploy/test_deployer.py +++ b/tests/unit/deploy/test_deployer.py @@ -1046,7 +1046,7 @@ def test_can_create_auth_handlers(self, sample_app_with_auth): # The tag format is tested elsewhere. tags=mock.ANY, timeout=constants.DEFAULT_LAMBDA_TIMEOUT, - zip_contents='package contents', + zip_contents=b'package contents', ) def test_can_update_auth_handlers(self, sample_app_with_auth): @@ -1073,7 +1073,7 @@ def test_can_update_auth_handlers(self, sample_app_with_auth): # The tag format is tested elsewhere. tags=mock.ANY, timeout=constants.DEFAULT_LAMBDA_TIMEOUT, - zip_contents='package contents', + zip_contents=b'package contents', ) def test_lambda_deployer_defaults(self, sample_app): From cdc975b2fceeb412a0afa962b11d646aed53fed0 Mon Sep 17 00:00:00 2001 From: James Saryerwinnie Date: Mon, 19 Jun 2017 16:31:54 -0700 Subject: [PATCH 04/11] Fail when using built in auths with the package command --- chalice/package.py | 20 ++++++++++++++++++++ tests/unit/test_package.py | 15 +++++++++++++++ 2 files changed, 35 insertions(+) diff --git a/chalice/package.py b/chalice/package.py index 4d612a204..d81610f0d 100644 --- a/chalice/package.py +++ b/chalice/package.py @@ -35,6 +35,10 @@ def create_app_packager(config): ) +class UnsupportedFeatureError(Exception): + pass + + class PreconfiguredPolicyGenerator(object): def __init__(self, config, policy_gen): # type: (Config, ApplicationPolicyHandler) -> None @@ -81,6 +85,7 @@ def __init__(self, swagger_generator, policy_generator): def generate_sam_template(self, config, code_uri=''): # type: (Config, str) -> Dict[str, Any] + self._check_for_unsupported_features(config) template = copy.deepcopy(self._BASE_TEMPLATE) resources = { 'APIHandler': self._generate_serverless_function(config, code_uri), @@ -91,6 +96,21 @@ def generate_sam_template(self, config, code_uri=''): self._update_endpoint_url_output(template, config) return template + def _check_for_unsupported_features(self, config): + # type: (Config) -> None + if config.chalice_app.builtin_auth_handlers: + # It doesn't look like SAM templates support everything + # we need to fully support built in authorizers. + # See: awslabs/serverless-application-model#49 + # and: https://forums.aws.amazon.com/thread.jspa?messageID=787920 + # + # We might need to switch to low level cfn to fix this. + raise UnsupportedFeatureError( + "SAM templates do not currently support these " + "built-in auth handlers: %s" % ', '.join( + [c.name for c in + config.chalice_app.builtin_auth_handlers])) + def _update_endpoint_url_output(self, template, config): # type: (Dict[str, Any], Config) -> None url = template['Outputs']['EndpointURL']['Value']['Fn::Sub'] diff --git a/tests/unit/test_package.py b/tests/unit/test_package.py index 4ed435e0e..68ac37141 100644 --- a/tests/unit/test_package.py +++ b/tests/unit/test_package.py @@ -258,3 +258,18 @@ def test_role_arn_added_to_function(sample_app, properties = template['Resources']['APIHandler']['Properties'] assert properties['Role'] == 'role-arn' assert 'Policies' not in properties + + +def test_fails_with_custom_auth(sample_app_with_auth, + mock_swagger_generator, + mock_policy_generator): + p = package.SAMTemplateGenerator( + mock_swagger_generator, mock_policy_generator) + mock_swagger_generator.generate_swagger.return_value = { + 'swagger': 'document' + } + config = Config.create( + chalice_app=sample_app_with_auth, api_gateway_stage='dev', app_name='myapp', + manage_iam_role=False, iam_role_arn='role-arn') + with pytest.raises(package.UnsupportedFeatureError): + p.generate_sam_template(config) From 2caaff827c2a5ebba830e56d6d808e7765c9df88 Mon Sep 17 00:00:00 2001 From: James Saryerwinnie Date: Tue, 20 Jun 2017 10:41:11 -0700 Subject: [PATCH 05/11] Delete unreferenced lambda functions on redeploys --- chalice/deploy/deployer.py | 13 +++++++++++ tests/unit/deploy/test_deployer.py | 35 ++++++++++++++++++++++++++++-- 2 files changed, 46 insertions(+), 2 deletions(-) diff --git a/chalice/deploy/deployer.py b/chalice/deploy/deployer.py index 1f72ad4d8..2ecac3241 100644 --- a/chalice/deploy/deployer.py +++ b/chalice/deploy/deployer.py @@ -418,8 +418,20 @@ def deploy(self, config, existing_resources, stage_name): deployed_values) self._deploy_auth_handlers(config, existing_resources, stage_name, deployed_values) + if existing_resources is not None: + self._cleanup_unreferenced_functions(existing_resources, + deployed_values) return deployed_values + def _cleanup_unreferenced_functions(self, existing_resources, + deployed_values): + # type: (DeployedResources, Dict[str, Any]) -> None + unreferenced = ( + set(existing_resources.lambda_functions.values()) - + set(deployed_values['lambda_functions'].values())) + for function_arn in unreferenced: + self._delete_lambda_function(function_arn) + def _deploy_api_handler(self, config, existing_resources, stage_name, deployed_values): # type: (Config, OPT_RESOURCES, str, Dict[str, Any]) -> None @@ -447,6 +459,7 @@ def _deploy_auth_handlers(self, config, existing_resources, stage_name, # functions configuration: auth_handlers = config.chalice_app.builtin_auth_handlers if not auth_handlers: + deployed_values['lambda_functions'] = {} return for auth_config in auth_handlers: self._deploy_auth_handler( diff --git a/tests/unit/deploy/test_deployer.py b/tests/unit/deploy/test_deployer.py index 470421601..9052398ea 100644 --- a/tests/unit/deploy/test_deployer.py +++ b/tests/unit/deploy/test_deployer.py @@ -702,7 +702,7 @@ def test_lambda_deployer_repeated_deploy(app_policy, sample_app): lambda_function_name = 'lambda_function_name' deployed = DeployedResources( 'api', 'api_handler_arn', lambda_function_name, - None, 'dev', None, None, None) + None, 'dev', None, None, {}) d.deploy(cfg, deployed, 'dev') # Should result in injecting the latest app code. @@ -825,6 +825,7 @@ def test_lambda_deployer_initial_deploy(app_policy, sample_app): assert deployed == { 'api_handler_arn': 'lambda-arn', 'api_handler_name': 'myapp-dev', + 'lambda_functions': {}, } aws_client.create_function.assert_called_with( function_name='myapp-dev', role_arn='role-arn', @@ -1076,6 +1077,36 @@ def test_can_update_auth_handlers(self, sample_app_with_auth): zip_contents=b'package contents', ) + def test_unreferenced_functions_are_deleted(self, sample_app_with_auth): + # Existing resources is the set of resources that have + # *previously* been deployed. + existing_lambda_functions = { + 'old-function': 'arn:not-referenced-anymore', + } + existing = DeployedResources( + 'api', 'api-handler-arn', 'api-handler-name', + 'existing-id', 'dev', None, None, + existing_lambda_functions) + self.aws_client.lambda_function_exists.return_value = True + self.aws_client.update_function.return_value = { + 'FunctionArn': 'arn:new-auth-function' + } + config = self.create_config_obj(sample_app_with_auth) + self.aws_client.get_function_configuration.return_value = { + 'Runtime': config.lambda_python_version, + } + deployer = LambdaDeployer( + self.aws_client, self.packager, None, self.osutils, + self.app_policy) + deployed = deployer.deploy(config, existing, stage_name='dev') + # Because the "old-function" was not referenced in the update + # function calls, we should expect that it was deleted. + self.aws_client.delete_function.assert_called_with( + 'arn:not-referenced-anymore') + # And the old-arn is not in the deployed resources + assert deployed['lambda_functions'] == { + 'api-handler-name-myauth': 'arn:new-auth-function'} + def test_lambda_deployer_defaults(self, sample_app): cfg = self.create_config_obj(sample_app) deployer = LambdaDeployer( @@ -1231,7 +1262,7 @@ def setup_deployer_dependencies(self, app_policy): self.deployed_resources = DeployedResources( 'api', 'api_handler_arn', self.lambda_function_name, - None, 'dev', None, None, None) + None, 'dev', None, None, {}) def test_lambda_deployer_defaults(self, sample_app): cfg = Config.create( From 2cdde29f39b1d18dadebed1611d3a9fcbfdf83f5 Mon Sep 17 00:00:00 2001 From: James Saryerwinnie Date: Tue, 20 Jun 2017 13:36:52 -0700 Subject: [PATCH 06/11] Add per-function granularity in config As part of this change, I updated the pylintrc to permit the scope() method to assign attributes for an instance. --- .pylintrc | 4 +- chalice/cli/factory.py | 6 +- chalice/config.py | 103 +++++++++++++++++--------- chalice/constants.py | 4 ++ tests/unit/test_config.py | 147 +++++++++++++++++++++++++++++++++++--- 5 files changed, 219 insertions(+), 45 deletions(-) diff --git a/.pylintrc b/.pylintrc index 4230edc65..ff6524204 100644 --- a/.pylintrc +++ b/.pylintrc @@ -294,7 +294,9 @@ callbacks=cb_,_cb [CLASSES] # List of method names used to declare (i.e. assign) instance attributes. -defining-attr-methods=__init__,__new__,setUp +# scope() is from the config obj, which is allowed to define the __dict__ +# attr as part of its implementation. +defining-attr-methods=__init__,__new__,setUp,scope # List of valid names for the first argument in a class method. valid-classmethod-first-arg=cls diff --git a/chalice/cli/factory.py b/chalice/cli/factory.py index eedcd1a5a..f569c0954 100644 --- a/chalice/cli/factory.py +++ b/chalice/cli/factory.py @@ -104,8 +104,10 @@ def create_config_obj(self, chalice_stage_name=DEFAULT_STAGE_NAME, user_provided_params['profile'] = self.profile if api_gateway_stage is not None: user_provided_params['api_gateway_stage'] = api_gateway_stage - config = Config(chalice_stage_name, user_provided_params, - config_from_disk, default_params) + config = Config(chalice_stage=chalice_stage_name, + user_provided_params=user_provided_params, + config_from_disk=config_from_disk, + default_params=default_params) return config def _validate_config_from_disk(self, config): diff --git a/chalice/config.py b/chalice/config.py index 93c999107..a266c4517 100644 --- a/chalice/config.py +++ b/chalice/config.py @@ -1,11 +1,13 @@ import os import sys import json +import copy from typing import Dict, Any, Optional # noqa from chalice import __version__ as current_chalice_version from chalice.app import Chalice # noqa from chalice.constants import DEFAULT_STAGE_NAME +from chalice.constants import DEFAULT_HANDLER_NAME StrMap = Dict[str, Any] @@ -79,13 +81,15 @@ class Config(object): def __init__(self, chalice_stage=DEFAULT_STAGE_NAME, + function_name=DEFAULT_HANDLER_NAME, user_provided_params=None, config_from_disk=None, default_params=None): - # type: (str, StrMap, StrMap, StrMap) -> None + # type: (str, str, StrMap, StrMap, StrMap) -> None #: Params that a user provided explicitly, #: typically via the command line. self.chalice_stage = chalice_stage + self.function_name = function_name if user_provided_params is None: user_provided_params = {} self._user_provided_params = user_provided_params @@ -98,8 +102,10 @@ def __init__(self, self._default_params = default_params @classmethod - def create(cls, chalice_stage=DEFAULT_STAGE_NAME, **kwargs): - # type: (str, **Any) -> Config + def create(cls, chalice_stage=DEFAULT_STAGE_NAME, + function_name=DEFAULT_HANDLER_NAME, + **kwargs): + # type: (str, str, **Any) -> Config return cls(chalice_stage=chalice_stage, user_provided_params=kwargs.copy()) @@ -128,12 +134,6 @@ def config_from_disk(self): # type: () -> StrMap return self._config_from_disk - @property - def iam_policy_file(self): - # type: () -> str - return self._chain_lookup('iam_policy_file', - varies_per_chalice_stage=True) - @property def lambda_python_version(self): # type: () -> str @@ -142,29 +142,15 @@ def lambda_python_version(self): # supported by lambda. return self._PYTHON_VERSIONS[sys.version_info[0]] - @property - def lambda_memory_size(self): - # type: () -> int - return self._chain_lookup( - 'lambda_memory_size', varies_per_chalice_stage=True) - - @property - def lambda_timeout(self): - # type: () -> int - return self._chain_lookup( - 'lambda_timeout', varies_per_chalice_stage=True) - - @property - def tags(self): - # type: () -> Dict[str, str] - tags = self._chain_merge('tags') - tags['aws-chalice'] = 'version=%s:stage=%s:app=%s' % ( - current_chalice_version, self.chalice_stage, self.app_name) - return tags - - def _chain_lookup(self, name, varies_per_chalice_stage=False): - # type: (str, bool) -> Any + def _chain_lookup(self, name, varies_per_chalice_stage=False, + varies_per_function=False): + # type: (str, bool, bool) -> Any search_dicts = [self._user_provided_params] + if varies_per_function: + search_dicts.append( + self._config_from_disk.get('stages', {}).get( + self.chalice_stage, {}).get('lambda_functions', {}).get( + self.function_name, {})) if varies_per_chalice_stage: search_dicts.append( self._config_from_disk.get('stages', {}).get( @@ -184,6 +170,9 @@ def _chain_merge(self, name): self._config_from_disk, self._config_from_disk.get('stages', {}).get( self.chalice_stage, {}), + self._config_from_disk.get('stages', {}).get( + self.chalice_stage, {}).get('lambda_functions', {}).get( + self.function_name, {}), self._user_provided_params, ] final = {} @@ -207,17 +196,40 @@ def api_gateway_stage(self): return self._chain_lookup('api_gateway_stage', varies_per_chalice_stage=True) + @property + def iam_policy_file(self): + # type: () -> str + return self._chain_lookup('iam_policy_file', + varies_per_chalice_stage=True, + varies_per_function=True) + + @property + def lambda_memory_size(self): + # type: () -> int + return self._chain_lookup('lambda_memory_size', + varies_per_chalice_stage=True, + varies_per_function=True) + + @property + def lambda_timeout(self): + # type: () -> int + return self._chain_lookup('lambda_timeout', + varies_per_chalice_stage=True, + varies_per_function=True) + @property def iam_role_arn(self): # type: () -> str return self._chain_lookup('iam_role_arn', - varies_per_chalice_stage=True) + varies_per_chalice_stage=True, + varies_per_function=True) @property def manage_iam_role(self): # type: () -> bool result = self._chain_lookup('manage_iam_role', - varies_per_chalice_stage=True) + varies_per_chalice_stage=True, + varies_per_function=True) if result is None: # To simplify downstream code, if manage_iam_role # is None (indicating the user hasn't configured/specified this @@ -231,13 +243,36 @@ def manage_iam_role(self): def autogen_policy(self): # type: () -> bool return self._chain_lookup('autogen_policy', - varies_per_chalice_stage=True) + varies_per_chalice_stage=True, + varies_per_function=True) @property def environment_variables(self): # type: () -> Dict[str, str] return self._chain_merge('environment_variables') + @property + def tags(self): + # type: () -> Dict[str, str] + tags = self._chain_merge('tags') + tags['aws-chalice'] = 'version=%s:stage=%s:app=%s' % ( + current_chalice_version, self.chalice_stage, self.app_name) + return tags + + def scope(self, chalice_stage, function_name): + # type: (str, str) -> Config + # Used to create a new config object that's scoped to a different + # stage and/or function. This creates a completely separate copy. + # This is preferred over mutating the existing config obj. + # We technically don't need to do a copy here, but this avoids + # any possible issues if we ever mutate the config values. + instance_data = copy.deepcopy(vars(self)) + clone = self.__class__() + clone.__dict__ = instance_data + clone.chalice_stage = chalice_stage + clone.function_name = function_name + return clone + def deployed_resources(self, chalice_stage_name): # type: (str) -> Optional[DeployedResources] """Return resources associated with a given stage. diff --git a/chalice/constants.py b/chalice/constants.py index edfd0a3a5..27168a60a 100644 --- a/chalice/constants.py +++ b/chalice/constants.py @@ -51,6 +51,10 @@ def index(): DEFAULT_LAMBDA_TIMEOUT = 60 DEFAULT_LAMBDA_MEMORY_SIZE = 128 MAX_LAMBDA_DEPLOYMENT_SIZE = 50 * (1024 ** 2) +# This is the name of the main handler used to +# handle API gateway requests. This is used as a key +# in the config module. +DEFAULT_HANDLER_NAME = 'api_handler' LAMBDA_TRUST_POLICY = { diff --git a/tests/unit/test_config.py b/tests/unit/test_config.py index 961814072..ed9206761 100644 --- a/tests/unit/test_config.py +++ b/tests/unit/test_config.py @@ -1,4 +1,5 @@ import sys +import pytest from chalice import __version__ as chalice_version from chalice.config import Config, DeployedResources @@ -50,7 +51,10 @@ def test_can_chain_lookup(): 'project_dir': 'default_params', } - c = Config('dev', user_provided_params, config_from_disk, default_params) + c = Config(chalice_stage='dev', + user_provided_params=user_provided_params, + config_from_disk=config_from_disk, + default_params=default_params) assert c.api_gateway_stage == 'user_provided_params' assert c.app_name == 'config_from_disk' assert c.project_dir == 'default_params' @@ -89,6 +93,109 @@ def test_can_chain_chalice_stage_values(): assert not prod.manage_iam_role +def test_can_chain_function_values(): + disk_config = { + 'lambda_timeout': 10, + 'stages': { + 'dev': { + 'lambda_timeout': 20, + 'lambda_functions': { + 'api_handler': { + 'lambda_timeout': 30, + } + } + } + } + } + c = Config(chalice_stage='dev', + config_from_disk=disk_config) + assert c.lambda_timeout == 30 + + +def test_can_create_scope_obj_with_new_function(): + disk_config = { + 'lambda_timeout': 10, + 'stages': { + 'dev': { + 'lambda_timeout': 20, + 'lambda_functions': { + 'api_handler': { + 'lambda_timeout': 30, + }, + 'myauth': { + 'lambda_timeout': 40, + } + } + } + } + } + c = Config(chalice_stage='dev', config_from_disk=disk_config) + new_config = c.scope(chalice_stage='dev', + function_name='myauth') + assert new_config.lambda_timeout == 40 + + +@pytest.mark.parametrize('stage_name,function_name,expected', [ + ('dev', 'api_handler', 'dev-api-handler'), + ('dev', 'myauth', 'dev-myauth'), + ('beta', 'api_handler', 'beta-api-handler'), + ('beta', 'myauth', 'beta-myauth'), + ('prod', 'api_handler', 'prod-stage'), + ('prod', 'myauth', 'prod-stage'), + ('foostage', 'api_handler', 'global'), + ('foostage', 'myauth', 'global'), +]) +def test_can_create_scope_new_stage_and_function(stage_name, function_name, + expected): + disk_config = { + 'environment_variables': {'from': 'global'}, + 'stages': { + 'dev': { + 'environment_variables': {'from': 'dev-stage'}, + 'lambda_functions': { + 'api_handler': { + 'environment_variables': { + 'from': 'dev-api-handler', + } + }, + 'myauth': { + 'environment_variables': { + 'from': 'dev-myauth', + } + } + } + }, + 'beta': { + 'environment_variables': {'from': 'beta-stage'}, + 'lambda_functions': { + 'api_handler': { + 'environment_variables': { + 'from': 'beta-api-handler', + } + }, + 'myauth': { + 'environment_variables': { + 'from': 'beta-myauth', + } + } + } + }, + 'prod': { + 'environment_variables': {'from': 'prod-stage'}, + } + } + } + c = Config(chalice_stage='dev', config_from_disk=disk_config) + new_config = c.scope(chalice_stage=stage_name, + function_name=function_name) + assert new_config.environment_variables == {'from': expected} + + + +def test_can_create_scope_new_stage_only(): + pass + + def test_can_create_deployed_resource_from_dict(): d = DeployedResources.from_dict({ 'backend': 'api', @@ -116,7 +223,7 @@ def test_environment_from_top_level(): assert c.environment_variables == config_from_disk['environment_variables'] -def test_environment_from_stage_leve(): +def test_environment_from_stage_level(): config_from_disk = { 'stages': { 'prod': { @@ -133,13 +240,23 @@ def test_env_vars_chain_merge(): config_from_disk = { 'environment_variables': { 'top_level': 'foo', - 'shared_key': 'from-top', + 'shared_stage_key': 'from-top', + 'shared_stage': 'from-top', }, 'stages': { 'prod': { 'environment_variables': { 'stage_var': 'bar', - 'shared_key': 'from-stage', + 'shared_stage_key': 'from-stage', + 'shared_stage': 'from-stage', + }, + 'lambda_functions': { + 'api_handler': { + 'environment_variables': { + 'function_key': 'from-function', + 'shared_stage': 'from-function', + } + } } } } @@ -149,7 +266,9 @@ def test_env_vars_chain_merge(): assert resolved == { 'top_level': 'foo', 'stage_var': 'bar', - 'shared_key': 'from-stage', + 'shared_stage': 'from-function', + 'function_key': 'from-function', + 'shared_stage_key': 'from-stage', } @@ -272,13 +391,23 @@ def test_tags_merge(self): 'app_name': 'myapp', 'tags': { 'onlyglobalkey': 'globalvalue', - 'sharedkey': 'globalvalue' + 'sharedkey': 'globalvalue', + 'sharedstage': 'globalvalue', }, 'stages': { 'dev': { 'tags': { 'sharedkey': 'stagevalue', - 'onlystagekey': 'stagevalue' + 'sharedstage': 'stagevalue', + 'onlystagekey': 'stagevalue', + }, + 'lambda_functions': { + 'api_handler': { + 'tags': { + 'sharedkey': 'functionvalue', + 'onlyfunctionkey': 'functionvalue', + } + } } } } @@ -286,8 +415,10 @@ def test_tags_merge(self): c = Config('dev', config_from_disk=config_from_disk) assert c.tags == { 'onlyglobalkey': 'globalvalue', - 'sharedkey': 'stagevalue', 'onlystagekey': 'stagevalue', + 'onlyfunctionkey': 'functionvalue', + 'sharedstage': 'stagevalue', + 'sharedkey': 'functionvalue', 'aws-chalice': 'version=%s:stage=dev:app=myapp' % chalice_version } From 67893225121ce720214c701d3d6cc8f2f8e76efa Mon Sep 17 00:00:00 2001 From: James Saryerwinnie Date: Tue, 20 Jun 2017 17:06:57 -0700 Subject: [PATCH 07/11] Use per-function config when deploying auth handlers --- chalice/config.py | 4 +-- chalice/deploy/deployer.py | 4 ++- tests/unit/deploy/test_deployer.py | 46 ++++++++++++++++++++++++++++++ 3 files changed, 50 insertions(+), 4 deletions(-) diff --git a/chalice/config.py b/chalice/config.py index a266c4517..182483686 100644 --- a/chalice/config.py +++ b/chalice/config.py @@ -1,7 +1,6 @@ import os import sys import json -import copy from typing import Dict, Any, Optional # noqa from chalice import __version__ as current_chalice_version @@ -266,9 +265,8 @@ def scope(self, chalice_stage, function_name): # This is preferred over mutating the existing config obj. # We technically don't need to do a copy here, but this avoids # any possible issues if we ever mutate the config values. - instance_data = copy.deepcopy(vars(self)) clone = self.__class__() - clone.__dict__ = instance_data + clone.__dict__ = vars(self) clone.chalice_stage = chalice_stage clone.function_name = function_name return clone diff --git a/chalice/deploy/deployer.py b/chalice/deploy/deployer.py index 2ecac3241..93142c2d8 100644 --- a/chalice/deploy/deployer.py +++ b/chalice/deploy/deployer.py @@ -462,8 +462,10 @@ def _deploy_auth_handlers(self, config, existing_resources, stage_name, deployed_values['lambda_functions'] = {} return for auth_config in auth_handlers: + new_config = config.scope(chalice_stage=config.chalice_stage, + function_name=auth_config.name) self._deploy_auth_handler( - config, auth_config, stage_name, deployed_values) + new_config, auth_config, stage_name, deployed_values) def _deploy_auth_handler(self, config, auth_config, stage_name, deployed_values): diff --git a/tests/unit/deploy/test_deployer.py b/tests/unit/deploy/test_deployer.py index 9052398ea..59765a1bf 100644 --- a/tests/unit/deploy/test_deployer.py +++ b/tests/unit/deploy/test_deployer.py @@ -1077,6 +1077,52 @@ def test_can_update_auth_handlers(self, sample_app_with_auth): zip_contents=b'package contents', ) + def test_can_create_auth_with_different_config(self, sample_app_with_auth): + # We're notusing create_config_obj because we want to approximate + # loading config from disk which contains per-lambda configuration. + disk_config = { + 'app_name': 'myapp', + 'iam_role_arn': 'role-arn', + 'manage_iam_role': False, + 'stages': { + 'dev': { + 'lamba_timeout': 10, + 'lambda_functions': { + 'myauth': { + 'lambda_timeout': 20, + 'lambda_memory_size': 512, + } + } + } + } + } + config = Config( + 'dev', + config_from_disk=disk_config, + user_provided_params={'chalice_app': sample_app_with_auth, + 'project_dir': '.'} + ) + deployer = LambdaDeployer( + self.aws_client, self.packager, None, self.osutils, + self.app_policy) + self.aws_client.lambda_function_exists.return_value = False + self.aws_client.create_function.side_effect = [ + self.lambda_arn, 'arn:auth-function'] + deployer.deploy(config, None, stage_name='dev') + self.aws_client.create_function.assert_called_with( + environment_variables={}, + function_name='myapp-dev-myauth', + handler='app.myauth', + role_arn='role-arn', + runtime=mock.ANY, + tags=mock.ANY, + zip_contents=b'package contents', + # These come from the 'lambda_functions.myauth' section + # in the config above. + timeout=20, + memory_size=512, + ) + def test_unreferenced_functions_are_deleted(self, sample_app_with_auth): # Existing resources is the set of resources that have # *previously* been deployed. From 107c07ef5ec4e5242567ad84ecb29148dab48a46 Mon Sep 17 00:00:00 2001 From: James Saryerwinnie Date: Wed, 21 Jun 2017 12:37:39 -0700 Subject: [PATCH 08/11] Incorporate review feedback --- .pylintrc | 4 +-- chalice/config.py | 11 ++++--- tests/unit/deploy/test_deployer.py | 46 ++++++++++++++++++++---------- tests/unit/test_config.py | 42 +++++++++++++++++++++++---- 4 files changed, 76 insertions(+), 27 deletions(-) diff --git a/.pylintrc b/.pylintrc index ff6524204..4230edc65 100644 --- a/.pylintrc +++ b/.pylintrc @@ -294,9 +294,7 @@ callbacks=cb_,_cb [CLASSES] # List of method names used to declare (i.e. assign) instance attributes. -# scope() is from the config obj, which is allowed to define the __dict__ -# attr as part of its implementation. -defining-attr-methods=__init__,__new__,setUp,scope +defining-attr-methods=__init__,__new__,setUp # List of valid names for the first argument in a class method. valid-classmethod-first-arg=cls diff --git a/chalice/config.py b/chalice/config.py index 182483686..81dbf6f1d 100644 --- a/chalice/config.py +++ b/chalice/config.py @@ -265,10 +265,13 @@ def scope(self, chalice_stage, function_name): # This is preferred over mutating the existing config obj. # We technically don't need to do a copy here, but this avoids # any possible issues if we ever mutate the config values. - clone = self.__class__() - clone.__dict__ = vars(self) - clone.chalice_stage = chalice_stage - clone.function_name = function_name + clone = self.__class__( + chalice_stage=chalice_stage, + function_name=function_name, + user_provided_params=self._user_provided_params, + config_from_disk=self._config_from_disk, + default_params=self._default_params, + ) return clone def deployed_resources(self, chalice_stage_name): diff --git a/tests/unit/deploy/test_deployer.py b/tests/unit/deploy/test_deployer.py index 59765a1bf..cc68bb2a8 100644 --- a/tests/unit/deploy/test_deployer.py +++ b/tests/unit/deploy/test_deployer.py @@ -1078,7 +1078,7 @@ def test_can_update_auth_handlers(self, sample_app_with_auth): ) def test_can_create_auth_with_different_config(self, sample_app_with_auth): - # We're notusing create_config_obj because we want to approximate + # We're not using create_config_obj because we want to approximate # loading config from disk which contains per-lambda configuration. disk_config = { 'app_name': 'myapp', @@ -1086,7 +1086,8 @@ def test_can_create_auth_with_different_config(self, sample_app_with_auth): 'manage_iam_role': False, 'stages': { 'dev': { - 'lamba_timeout': 10, + 'lambda_timeout': 10, + 'lambda_memory_size': 128, 'lambda_functions': { 'myauth': { 'lambda_timeout': 20, @@ -1109,19 +1110,34 @@ def test_can_create_auth_with_different_config(self, sample_app_with_auth): self.aws_client.create_function.side_effect = [ self.lambda_arn, 'arn:auth-function'] deployer.deploy(config, None, stage_name='dev') - self.aws_client.create_function.assert_called_with( - environment_variables={}, - function_name='myapp-dev-myauth', - handler='app.myauth', - role_arn='role-arn', - runtime=mock.ANY, - tags=mock.ANY, - zip_contents=b'package contents', - # These come from the 'lambda_functions.myauth' section - # in the config above. - timeout=20, - memory_size=512, - ) + create_function_calls = self.aws_client.create_function.call_args_list + assert create_function_calls == [ + mock.call( + environment_variables={}, + function_name='myapp-dev', + handler='app.app', + role_arn='role-arn', + runtime=mock.ANY, + tags=mock.ANY, + zip_contents=b'package contents', + # These come frmo the stage level config above. + timeout=10, + memory_size=128, + ), + mock.call( + environment_variables={}, + function_name='myapp-dev-myauth', + handler='app.myauth', + role_arn='role-arn', + runtime=mock.ANY, + tags=mock.ANY, + zip_contents=b'package contents', + # These come from the 'lambda_functions.myauth' section + # in the config above. + timeout=20, + memory_size=512, + ) + ] def test_unreferenced_functions_are_deleted(self, sample_app_with_auth): # Existing resources is the set of resources that have diff --git a/tests/unit/test_config.py b/tests/unit/test_config.py index ed9206761..c2b5aa6eb 100644 --- a/tests/unit/test_config.py +++ b/tests/unit/test_config.py @@ -117,13 +117,31 @@ def test_can_create_scope_obj_with_new_function(): 'lambda_timeout': 10, 'stages': { 'dev': { - 'lambda_timeout': 20, + 'manage_iam_role': True, + 'iam_role_arn': 'role-arn', + 'autogen_policy': True, + 'iam_policy_file': 'policy.json', + 'environment_variables': {'env': 'stage'}, + 'lambda_timeout': 1, + 'lambda_memory_size': 1, + 'tags': {'tag': 'stage'}, 'lambda_functions': { 'api_handler': { 'lambda_timeout': 30, }, 'myauth': { - 'lambda_timeout': 40, + # We're purposefully using different + # values for everything in the stage + # level config to ensure we can pull + # from function scoped config properly. + 'manage_iam_role': True, + 'iam_role_arn': 'auth-role-arn', + 'autogen_policy': True, + 'iam_policy_file': 'function.json', + 'environment_variables': {'env': 'function'}, + 'lambda_timeout': 2, + 'lambda_memory_size': 2, + 'tags': {'tag': 'function'}, } } } @@ -132,7 +150,14 @@ def test_can_create_scope_obj_with_new_function(): c = Config(chalice_stage='dev', config_from_disk=disk_config) new_config = c.scope(chalice_stage='dev', function_name='myauth') - assert new_config.lambda_timeout == 40 + assert new_config.manage_iam_role == True + assert new_config.iam_role_arn == 'auth-role-arn' + assert new_config.autogen_policy == True + assert new_config.iam_policy_file == 'function.json' + assert new_config.environment_variables == {'env': 'function'} + assert new_config.lambda_timeout == 2 + assert new_config.lambda_memory_size == 2 + assert new_config.tags['tag'] == 'function' @pytest.mark.parametrize('stage_name,function_name,expected', [ @@ -191,9 +216,16 @@ def test_can_create_scope_new_stage_and_function(stage_name, function_name, assert new_config.environment_variables == {'from': expected} +def test_new_scope_config_is_separate_copy(): + original = Config(chalice_stage='dev', function_name='foo') + new_config = original.scope(chalice_stage='prod', function_name='bar') + + # The original should not have been mutated. + assert original.chalice_stage == 'dev' + assert original.function_name == 'foo' -def test_can_create_scope_new_stage_only(): - pass + assert new_config.chalice_stage == 'prod' + assert new_config.function_name == 'bar' def test_can_create_deployed_resource_from_dict(): From a234e45953a2b7ffadc5d99c15a9e1afadd18780 Mon Sep 17 00:00:00 2001 From: James Saryerwinnie Date: Wed, 21 Jun 2017 15:23:35 -0700 Subject: [PATCH 09/11] Fix APIGateway documentation to include class That way refs to the :class: work as expected. --- docs/source/api.rst | 64 ++++++++++++++++++++++++++------------------- 1 file changed, 37 insertions(+), 27 deletions(-) diff --git a/docs/source/api.rst b/docs/source/api.rst index f0d3b59c3..e36a66190 100644 --- a/docs/source/api.rst +++ b/docs/source/api.rst @@ -260,42 +260,52 @@ for an ``@app.route(authorizer=...)`` call: APIGateway ========== -There is a single instance of :class:`APIGateway` attached to each -:class:`Chalice` object under the ``api`` attribute. +.. class:: APIGateway() -.. attribute:: default_binary_types + This class is used to control + how API Gateway interprets ``Content-Type`` headers in both requests and + responses. - The value of ``default_binary_types`` are the ``Content-Types`` that are - considered binary by default. This value should not be changed, instead you - should modify the ``binary_types`` list to change the behavior of a content - type. Its value is: ``application/octet-stream``, ``application/x-tar``, ``application/zip``, ``audio/basic``, ``audio/ogg``, ``audio/mp4``, ``audio/mpeg``, ``audio/wav``, ``audio/webm``, ``image/png``, ``image/jpg``, ``image/gif``, ``video/ogg``, ``video/mpeg``, ``video/webm``. + There is a single instance of this class attached to each + :class:`Chalice` object under the ``api`` attribute. + .. attribute:: default_binary_types -.. attribute:: binary_types + The value of ``default_binary_types`` are the ``Content-Types`` that are + considered binary by default. This value should not be changed, instead you + should modify the ``binary_types`` list to change the behavior of a content + type. Its value is: ``application/octet-stream``, ``application/x-tar``, + ``application/zip``, ``audio/basic``, ``audio/ogg``, ``audio/mp4``, + ``audio/mpeg``, ``audio/wav``, ``audio/webm``, ``image/png``, + ``image/jpg``, ``image/gif``, ``video/ogg``, ``video/mpeg``, + ``video/webm``. - The value of ``binary_types`` controls how API Gateway interprets requests - and responses as detailed below. - If an incoming request has a ``Content-Type`` header value that is present - in the ``binary_types`` list it will be assumed that its body is a sequence - of raw bytes. You can access these bytes by accessing the - ``app.current_request.raw_body`` property. + .. attribute:: binary_types - If an outgoing response from ``Chalice`` has a header ``Content-Type`` that - matches one of the ``binary_types`` its body must be a ``bytes`` type object. - It is important to note that originating request must have the ``Accept`` - header for the same type as the ``Content-Type`` on the response. Otherwise - a ``400`` error will be returned. + The value of ``binary_types`` controls how API Gateway interprets requests + and responses as detailed below. - Implementation note: API Gateway and Lambda communicate through a JSON event - which is encoded using ``UTF-8``. The raw bytes are temporarily encoded - using + If an incoming request has a ``Content-Type`` header value that is present + in the ``binary_types`` list it will be assumed that its body is a sequence + of raw bytes. You can access these bytes by accessing the + ``app.current_request.raw_body`` property. - base64 when being passed between API Gateway and Labmda. In the worst case - this encoding can cause the binary body to be inflated up to ``4/3`` its - original size. Lambda only accepts an event up to ``6mb``, which means even - if your binary data was not quite at that limit, with the base64 encoding it - may exceed that limit. This will manifest as a ``502`` Bad Gateway error. + If an outgoing response from ``Chalice`` has a header ``Content-Type`` that + matches one of the ``binary_types`` its body must be a ``bytes`` type object. + It is important to note that originating request must have the ``Accept`` + header for the same type as the ``Content-Type`` on the response. Otherwise + a ``400`` error will be returned. + + Implementation note: API Gateway and Lambda communicate through a JSON event + which is encoded using ``UTF-8``. The raw bytes are temporarily encoded + using + + base64 when being passed between API Gateway and Labmda. In the worst case + this encoding can cause the binary body to be inflated up to ``4/3`` its + original size. Lambda only accepts an event up to ``6mb``, which means even + if your binary data was not quite at that limit, with the base64 encoding it + may exceed that limit. This will manifest as a ``502`` Bad Gateway error. CORS From 8aa07e7a82ebd1f12b9ea8cf23c788b3c8f7c330 Mon Sep 17 00:00:00 2001 From: James Saryerwinnie Date: Wed, 21 Jun 2017 15:49:31 -0700 Subject: [PATCH 10/11] Add documentation for authorizers Includes api doc updates as well as a topic guide. --- docs/source/api.rst | 86 +++++++++++ docs/source/index.rst | 1 + docs/source/topics/authorizers.rst | 219 +++++++++++++++++++++++++++++ 3 files changed, 306 insertions(+) create mode 100644 docs/source/topics/authorizers.rst diff --git a/docs/source/api.rst b/docs/source/api.rst index e36a66190..ae7d707f8 100644 --- a/docs/source/api.rst +++ b/docs/source/api.rst @@ -92,6 +92,32 @@ Chalice you would like more control over how CORS is configured, you can provide an instance of :class:`CORSConfig`. + .. method:: authorizer(name, \*\*options) + + Register a built-in authorizer. + + .. code-block:: python + + from chalice import Chalice, AuthResponse + + app = Chalice(app_name="appname") + + @app.authorizer(ttl_seconds=30) + def my_auth(auth_request): + # Validate auth_request.token, and then: + return AuthResponse(routes=['/'], principal_id='username') + + @app.route('/', authorizer=my_auth) + def viewfunction(value): + pass + + :param ttl_seconds: The number of seconds to cache this response. + Subsequent requests that require this authorizer will use a + cached response if available. The default is 300 seconds. + + :param execution_role: An optional IAM role to specify when invoking + the Lambda function associated with the built-in authorizer. + Request ======= @@ -257,6 +283,66 @@ for an ``@app.route(authorizer=...)`` call: The header where the auth token will be specified. +Built-in Authorizers +-------------------- + +These classes are used when defining built-in authoriers in Chalice. + +.. class:: AuthRequest(auth_type, token, method_arn) + + An instance of this class is passed as the first argument + to an authorizer defined via ``@app.authorizer()``. You + generally do not instantiate this class directly. + + .. attribute:: auth_type + + The type of authentication + + .. attribute:: token + + The authorization token. This is usually the value of the + ``Authorization`` header. + + .. attribute:: method_arn + + The ARN of the API gateway being authorized. + +.. class:: AuthResponse(routes, principal_id, context=None) + + .. attribute:: routes + + A list of authorized routes. Each element in the list + can either by a string route such as `"/foo/bar"` or + an instance of ``AuthRoute``. If you specify the URL as + a string, then all supported HTTP methods will be authorized. + If you want to specify which HTTP methods are allowed, you + can use ``AuthRoute``. + + .. attribute:: principal_id + + The principal id of the user. + + .. attribute:: context + + An optional dictionary of key value pairs. This dictionary + will be accessible in the ``app.current_request.context`` + in all subsequent authorized requests for this user. + +.. class:: AuthRoute(path, methods) + + This class be used in the ``routes`` attribute of a + :class:`AuthResponse` instance to get fine grained control + over which HTTP methods are allowed for a given route. + + .. attribute:: path + + The allowed route specified as a string + + .. attribute:: methods + + A list of allowed HTTP methods. + + APIGateway ========== diff --git a/docs/source/index.rst b/docs/source/index.rst index 2e466f2e1..efee2d207 100644 --- a/docs/source/index.rst +++ b/docs/source/index.rst @@ -64,6 +64,7 @@ Topics topics/packaging topics/pyversion topics/cfn + topics/authorizers API Reference diff --git a/docs/source/topics/authorizers.rst b/docs/source/topics/authorizers.rst new file mode 100644 index 000000000..b5c0afc06 --- /dev/null +++ b/docs/source/topics/authorizers.rst @@ -0,0 +1,219 @@ +Authorization +============= + +Chalice supports multiple mechanisms for authorization. This topic +covers how you can integrate authorization into your Chalice applications. + +In Chalice, all the authorizers are configured per-route and specified +using the ``authorizer`` kwarg to an ``@app.route()`` call. You +control which type of authorizer to use based on what's passed as the +``authorizer`` kwarg. You can use the same authorizer instance for +multiple routes. + +The first set of authorizers chalice supports cover the scenario where +you have some existing authorization mechanism that you just want your +Chalice app to use. + +Chalice also supports built-in authorizers, which allows Chalice to +manage your custom authorizers as part of ``chalice deploy``. This is +covered in the Built-in Authorizers section. + + +AWS IAM Authorizer +------------------ + +The IAM Authorizer allows you to control access to API Gateway with +`IAM permissions`_ + +To associate an IAM authorizer with a route in chalice, you use the +:class:`IAMAUthorizer` class: + +.. code-block:: python + + authorizer = IAMAuthorizer() + + @app.route('/iam-auth', methods=['GET'], authorizer=authorizer) + def authenticated(): + return {"success": True} + + +See the `API Gateway documentation +`__ +for more information on controlling access to API Gateway with IAM permissions. + +Amazon Cognito User Pools +------------------------- + +In addition to using IAM roles and policies with the :class:`IAMAuthorizer` you +can also use a `Cognito user pools`_ to control who can access your Chalice +app. A cognito user pool serves as your own identity provider to maintain a +user directory. + +To integrate Cognito user pools with Chalice, you'll need to have an existing +cognito user pool configured. + + +.. code-block:: python + + authorizer = CognitoUserPoolAuthorizer( + 'MyPool', provider_arns=['arn:aws:cognito:...:userpool/name']) + + @app.route('/user-pools', methods=['GET'], authorizer=authorizer) + def authenticated(): + return {"sucecss": True} + + +Within a request, you can access the ``app.current_request.context`` object +for metadata about the authenticated request. + +For more information about using Cognito user pools with API Gateway, +see the `Use Amazon Cognito User Pools documentation +`__. + + +Custom Authorizers +------------------ + +API Gateway also lets you write custom authorizers using a Lambda function. +You can configure a Chalice route to use a pre-existing Lambda function as +a custom authorizer. If you also want to write and manage your Lambda +authorizer using Chalice, see the next section, Built-in Authorizers. + +To connect an existing Lambda function as a custom authorizer in chalice, +you use the ``CustomAuthorizer`` class: + +.. code-block:: python + + authorizer = CustomAuthorizer( + 'MyCustomAuth', header='Authorization', + authorizer_uri=('arn:aws:apigateway:region:lambda:path/2015-03-01' + '/functions/arn:aws:lambda:region:account-id:' + 'function:FunctionName/invocations')) + + @app.route('/custom-auth', methods=['GET'], authorizer=authorizer) + def authenticated(): + return {"success": True} + + +Built-in Authorizers +-------------------- + +The ``IAMAuthorizer``, ``CognitoUserPoolAuthorizer``, and the +``CustomAuthorizer`` classes are all for cases where you have existing +resources for managing authorization and you want to wire them together with +your Chalice app. A Built-in authorizer is used when you'd like to write your +custom authorizer in Chalice, and have the additional Lambda functions managed +when you run ``chalice deploy/delete``. This section will cover how to use the +built-in authorizers in chalice. + +Creating an authorizer in chalice requires you use the ``@app.authorizer`` +decorator to a function. The function must accept a single arg, which will be +an instance of :class:`AuthRequest`. The function must return a +:class:`AuthResponse`. As an example, we'll port the example from the `API +Gateway documentation`_. First, we'll show the code and then walk through it: + +.. code-block:: python + + from chalice import Chalice, AuthResponse + + app = Chalice(app_name='demoauth1') + + + @app.authorizer() + def demo_auth(auth_request): + token = auth_request.token + # This is just for demo purposes as shown in the API Gateway docs. + # Normally you'd call an oauth provider, validae the + # jwt token, etc. + # In this exampe, the token is treated as the status for demo + # purposes. + if token == 'allow': + return AuthResponse(routes=['/'], principal_id='user') + else: + return AuthResponse(routes=[], principal_id='user') + + + @app.route('/', authorizer=demo_auth) + def index(): + return {'context': app.current_request.context} + + +In the example above we define a built-in authorizer by decorating +the ``demo_auth`` function with the ``@app.authorizer()`` decorator. +Note you must use ``@app.authorizer()`` and not ``@app.authorizer``. +A built-in authorizer function has this type signature:: + + def auth_handler(auth_request: AuthRequest) -> AuthResponse: ... + +Within the auth handler you must determine if the request is +authorized or not. The ``AuthResponse`` contains the allowed +URLs as well as the principal id of the user. You can optionally +return a dictionary of key value pairs (as the ``context`` kwarg). +This dictionary will be passed through on subsequent requests. +In our example above we're not using the context dictionary. + +Now let's deploy our app. As usual, we just need to run +``chalice deploy`` and chalice will automatically deploy all the +necessary Lambda functions for us. + +Now when we try to make a request, we'll get an Unauthorized error:: + + $ http https://api.us-west-2.amazonaws.com/dev/ + HTTP/1.1 401 Unauthorized + + { + "message": "Unauthorized" + } + +If we add the appropriate authorization header, we'll see the call succeed:: + + $ http https://api.us-west-2.amazonaws.com/dev/ 'Authorization: allow' + HTTP/1.1 200 OK + + { + "context": { + "accountId": "12345", + "apiId": "api", + "authorizer": { + "principalId": "user" + }, + "httpMethod": "GET", + "identity": { + "accessKey": null, + "accountId": null, + "apiKey": "", + "caller": null, + "cognitoAuthenticationProvider": null, + "cognitoAuthenticationType": null, + "cognitoIdentityId": null, + "cognitoIdentityPoolId": null, + "sourceIp": "1.1.1.1", + "user": null, + "userAgent": "HTTPie/0.9.9", + "userArn": null + }, + "path": "/dev/", + "requestId": "d35d2063-56be-11e7-9ce1-dd61c24a3668", + "resourceId": "id", + "resourcePath": "/", + "stage": "dev" + } + } + +The low level API for API Gateway's custom authorizer feature requires +that an IAM policy must be returned. The :class:`AuthResponse` class we're +using is a wrapper over building the IAM policy ourself. If you want +low level control and would prefer to contruct the IAM policy yourself +you can return a dictionary of the IAM policy instead of an instance of +:class:`AuthResponse`. If you do that, the dictionary is returned +without modification back to API Gateway. + +For more information on custom authorizers, see the +`Use API Gateway Custom Authorizers +`__ +page in the API Gateway user guide. + + +.. _IAM permissions: http://docs.aws.amazon.com/IAM/latest/UserGuide/access_permissions.html +.. _Cognito User Pools: http://docs.aws.amazon.com/cognito/latest/developerguide/cognito-user-identity-pools.html +.. _API Gateway documentation: http://docs.aws.amazon.com/apigateway/latest/developerguide/use-custom-authorizer.html#api-gateway-custom-authorizer-lambda-function-create From c95b37ba89ac5f1f4ef704ff74484d334ecb6be9 Mon Sep 17 00:00:00 2001 From: James Saryerwinnie Date: Wed, 21 Jun 2017 16:37:44 -0700 Subject: [PATCH 11/11] Incorporate doc review feedback --- docs/source/topics/authorizers.rst | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/docs/source/topics/authorizers.rst b/docs/source/topics/authorizers.rst index b5c0afc06..78a2aa77c 100644 --- a/docs/source/topics/authorizers.rst +++ b/docs/source/topics/authorizers.rst @@ -63,9 +63,6 @@ cognito user pool configured. return {"sucecss": True} -Within a request, you can access the ``app.current_request.context`` object -for metadata about the authenticated request. - For more information about using Cognito user pools with API Gateway, see the `Use Amazon Cognito User Pools documentation `__. @@ -123,13 +120,17 @@ Gateway documentation`_. First, we'll show the code and then walk through it: def demo_auth(auth_request): token = auth_request.token # This is just for demo purposes as shown in the API Gateway docs. - # Normally you'd call an oauth provider, validae the + # Normally you'd call an oauth provider, validate the # jwt token, etc. # In this exampe, the token is treated as the status for demo # purposes. if token == 'allow': return AuthResponse(routes=['/'], principal_id='user') else: + # By specifying an empty list of routes, + # we're saying this user is not authorized + # for any URLs, which will result in an + # Unauthorized response. return AuthResponse(routes=[], principal_id='user')