From 477a3fdcdcfc422d1426b61cca94511142f4e613 Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Tue, 2 Jun 2015 15:56:44 -0400 Subject: [PATCH] Add a test to verify that all important blueprints have all their methods decorated This ensures that we don't accidentally add a blueprint method without either explicitly blacklisting or whitelisting anonymous access --- endpoints/api/__init__.py | 6 +++--- endpoints/decorators.py | 6 ++++++ endpoints/index.py | 11 ++++++++++- endpoints/registry.py | 3 +++ endpoints/tags.py | 2 ++ test/test_anon_checked.py | 33 +++++++++++++++++++++++++++++++++ 6 files changed, 57 insertions(+), 4 deletions(-) create mode 100644 test/test_anon_checked.py diff --git a/endpoints/api/__init__.py b/endpoints/api/__init__.py index d55e99632..9a235d6a0 100644 --- a/endpoints/api/__init__.py +++ b/endpoints/api/__init__.py @@ -19,7 +19,7 @@ from auth import scopes from auth.auth_context import get_authenticated_user, get_validated_oauth_token from auth.auth import process_oauth from endpoints.csrf import csrf_protect -from endpoints.decorators import anon_protect +from endpoints.decorators import check_anon_protection logger = logging.getLogger(__name__) @@ -229,14 +229,14 @@ def parse_repository_name(func): class ApiResource(Resource): - method_decorators = [anon_protect] + method_decorators = [check_anon_protection] def options(self): return None, 200 class RepositoryParamResource(ApiResource): - method_decorators = [anon_protect, parse_repository_name] + method_decorators = [check_anon_protection, parse_repository_name] def require_repo_permission(permission_class, scope, allow_public=False): diff --git a/endpoints/decorators.py b/endpoints/decorators.py index 05440fbfc..421edd041 100644 --- a/endpoints/decorators.py +++ b/endpoints/decorators.py @@ -15,6 +15,12 @@ def anon_allowed(func): def anon_protect(func): """ Marks a method as requiring some form of valid user auth before it can be executed. """ + func.__anon_protected = True + return check_anon_protection(func) + + +def check_anon_protection(func): + """ Validates a method as requiring some form of valid user auth before it can be executed. """ @wraps(func) def wrapper(*args, **kwargs): # Skip if anonymous access is allowed. diff --git a/endpoints/index.py b/endpoints/index.py index e84a93719..c89414724 100644 --- a/endpoints/index.py +++ b/endpoints/index.py @@ -20,7 +20,7 @@ from auth.permissions import (ModifyRepositoryPermission, UserAdminPermission, from util.http import abort from endpoints.trackhelper import track_and_log from endpoints.notificationhelper import spawn_notification -from endpoints.decorators import anon_protect +from endpoints.decorators import anon_protect, anon_allowed import features @@ -74,6 +74,7 @@ def generate_headers(scope=GrantType.READ_REPOSITORY): @index.route('/users', methods=['POST']) @index.route('/users/', methods=['POST']) +@anon_allowed def create_user(): user_data = request.get_json() if not user_data or not 'username' in user_data: @@ -146,6 +147,7 @@ def create_user(): @index.route('/users', methods=['GET']) @index.route('/users/', methods=['GET']) @process_auth +@anon_allowed def get_user(): if get_validated_oauth_token(): return jsonify({ @@ -167,6 +169,7 @@ def get_user(): @index.route('/users//', methods=['PUT']) @process_auth +@anon_allowed def update_user(username): permission = UserAdminPermission(username) @@ -194,6 +197,7 @@ def update_user(username): @process_auth @parse_repository_name @generate_headers(scope=GrantType.WRITE_REPOSITORY) +@anon_allowed def create_repository(namespace, repository): logger.debug('Parsing image descriptions') image_descriptions = json.loads(request.data.decode('utf8')) @@ -246,6 +250,7 @@ def create_repository(namespace, repository): @process_auth @parse_repository_name @generate_headers(scope=GrantType.WRITE_REPOSITORY) +@anon_allowed def update_images(namespace, repository): permission = ModifyRepositoryPermission(namespace, repository) @@ -304,12 +309,14 @@ def get_repository_images(namespace, repository): @process_auth @parse_repository_name @generate_headers(scope=GrantType.WRITE_REPOSITORY) +@anon_allowed def delete_repository_images(namespace, repository): abort(501, 'Not Implemented', issue='not-implemented') @index.route('/repositories//auth', methods=['PUT']) @parse_repository_name +@anon_allowed def put_repository_auth(namespace, repository): abort(501, 'Not Implemented', issue='not-implemented') @@ -353,11 +360,13 @@ def get_search(): # Note: This is *not* part of the Docker index spec. This is here for our own health check, # since we have nginx handle the _ping below. @index.route('/_internal_ping') +@anon_allowed def internal_ping(): return make_response('true', 200) @index.route('/_ping') @index.route('/_ping') +@anon_allowed def ping(): # NOTE: any changes made here must also be reflected in the nginx config response = make_response('true', 200) diff --git a/endpoints/registry.py b/endpoints/registry.py index 89ba9bd89..2cb6174e4 100644 --- a/endpoints/registry.py +++ b/endpoints/registry.py @@ -174,6 +174,7 @@ def get_image_layer(namespace, repository, image_id, headers): @registry.route('/images//layer', methods=['PUT']) @process_auth @extract_namespace_repo_from_session +@anon_protect def put_image_layer(namespace, repository, image_id): logger.debug('Checking repo permissions') permission = ModifyRepositoryPermission(namespace, repository) @@ -279,6 +280,7 @@ def put_image_layer(namespace, repository, image_id): @registry.route('/images//checksum', methods=['PUT']) @process_auth @extract_namespace_repo_from_session +@anon_protect def put_image_checksum(namespace, repository, image_id): logger.debug('Checking repo permissions') permission = ModifyRepositoryPermission(namespace, repository) @@ -439,6 +441,7 @@ def store_checksum(image_storage, checksum): @registry.route('/images//json', methods=['PUT']) @process_auth @extract_namespace_repo_from_session +@anon_protect def put_image_json(namespace, repository, image_id): logger.debug('Checking repo permissions') permission = ModifyRepositoryPermission(namespace, repository) diff --git a/endpoints/tags.py b/endpoints/tags.py index 8911a9b50..4b2ce90b6 100644 --- a/endpoints/tags.py +++ b/endpoints/tags.py @@ -54,6 +54,7 @@ def get_tag(namespace, repository, tag): @tags.route('/repositories//tags/', methods=['PUT']) @process_auth +@anon_protect @parse_repository_name def put_tag(namespace, repository, tag): permission = ModifyRepositoryPermission(namespace, repository) @@ -76,6 +77,7 @@ def put_tag(namespace, repository, tag): @tags.route('/repositories//tags/', methods=['DELETE']) @process_auth +@anon_protect @parse_repository_name def delete_tag(namespace, repository, tag): permission = ModifyRepositoryPermission(namespace, repository) diff --git a/test/test_anon_checked.py b/test/test_anon_checked.py new file mode 100644 index 000000000..f9eed9ee8 --- /dev/null +++ b/test/test_anon_checked.py @@ -0,0 +1,33 @@ +import unittest + +from endpoints.tags import tags +from endpoints.registry import registry +from endpoints.index import index +from endpoints.verbs import verbs + + +class TestAnonymousAccessChecked(unittest.TestCase): + def verifyBlueprint(self, blueprint): + class Checker(object): + def __init__(self, test_case): + self.test_case = test_case + + def add_url_rule(self, rule, endpoint, view_function, methods=None): + if (not '__anon_protected' in dir(view_function) and + not '__anon_allowed' in dir(view_function)): + error_message = ('Missing anonymous access protection decorator on function ' + + '%s under blueprint %s' % (endpoint, blueprint.name)) + self.test_case.fail(error_message) + + for deferred_function in blueprint.deferred_functions: + deferred_function(Checker(self)) + + def test_anonymous_access_checked(self): + self.verifyBlueprint(tags) + self.verifyBlueprint(registry) + self.verifyBlueprint(index) + self.verifyBlueprint(verbs) + +if __name__ == '__main__': + unittest.main() +