From ea2e17cc1164222f0ec4b58e1bec52c4ebda64c3 Mon Sep 17 00:00:00 2001 From: Jimmy Zelinskie Date: Wed, 9 Mar 2016 18:09:20 -0500 Subject: [PATCH] v2: send proper scopes for authorization failures Fixes #1278. --- auth/registry_jwt_auth.py | 56 +++++++++++++++++++------------ endpoints/common.py | 24 ++++++------- endpoints/v2/__init__.py | 37 +++++++++++--------- endpoints/v2/blob.py | 16 ++++----- endpoints/v2/catalog.py | 2 +- endpoints/v2/errors.py | 11 ++++-- endpoints/v2/manifest.py | 10 +++--- endpoints/v2/tag.py | 2 +- test/test_v2_endpoint_security.py | 4 ++- 9 files changed, 91 insertions(+), 71 deletions(-) diff --git a/auth/registry_jwt_auth.py b/auth/registry_jwt_auth.py index 38c10f7ad..97616e761 100644 --- a/auth/registry_jwt_auth.py +++ b/auth/registry_jwt_auth.py @@ -157,13 +157,18 @@ def build_context_and_subject(user, token, oauthtoken): return (context, ANONYMOUS_SUB) -def get_auth_headers(): +def get_auth_headers(repository=None, scopes=None): """ Returns a dictionary of headers for auth responses. """ headers = {} realm_auth_path = url_for('v2.generate_registry_jwt') authenticate = 'Bearer realm="{0}{1}",service="{2}"'.format(get_app_url(), realm_auth_path, app.config['SERVER_HOSTNAME']) + if repository: + authenticate += ',scope=repository:{0}'.format(repository) + if scopes: + authenticate += ':' + ','.join(scopes) + headers['WWW-Authenticate'] = authenticate headers['Docker-Distribution-API-Version'] = 'registry/2.0' return headers @@ -237,27 +242,34 @@ def load_public_key(certificate_file_path): return cert_obj.public_key() -def process_registry_jwt_auth(func): - @wraps(func) - def wrapper(*args, **kwargs): - logger.debug('Called with params: %s, %s', args, kwargs) - auth = request.headers.get('authorization', '').strip() - if auth: - max_signature_seconds = app.config.get('JWT_AUTH_MAX_FRESH_S', 3660) - certificate_file_path = app.config['JWT_AUTH_CERTIFICATE_PATH'] - public_key = load_public_key(certificate_file_path) +def process_registry_jwt_auth(scopes=None): + def inner(func): + @wraps(func) + def wrapper(*args, **kwargs): + logger.debug('Called with params: %s, %s', args, kwargs) + auth = request.headers.get('authorization', '').strip() + if auth: + max_signature_seconds = app.config.get('JWT_AUTH_MAX_FRESH_S', 3660) + certificate_file_path = app.config['JWT_AUTH_CERTIFICATE_PATH'] + public_key = load_public_key(certificate_file_path) - try: - extracted_identity, context = identity_from_bearer_token(auth, max_signature_seconds, - public_key) + try: + extracted_identity, context = identity_from_bearer_token(auth, max_signature_seconds, + public_key) - identity_changed.send(app, identity=extracted_identity) - set_grant_context(context) - logger.debug('Identity changed to %s', extracted_identity.id) - except InvalidJWTException as ije: - abort(401, message=ije.message, headers=get_auth_headers()) - else: - logger.debug('No auth header.') + identity_changed.send(app, identity=extracted_identity) + set_grant_context(context) + logger.debug('Identity changed to %s', extracted_identity.id) + except InvalidJWTException as ije: + repository = None + if 'namespace_name' in kwargs and 'repo_name' in kwargs: + repository = kwargs['namespace_name'] + '/' + kwargs['repo_name'] - return func(*args, **kwargs) - return wrapper + abort(401, message=ije.message, headers=get_auth_headers(repository=repository, + scopes=scopes)) + else: + logger.debug('No auth header.') + + return func(*args, **kwargs) + return wrapper + return inner diff --git a/endpoints/common.py b/endpoints/common.py index c683fab55..2852e1485 100644 --- a/endpoints/common.py +++ b/endpoints/common.py @@ -5,25 +5,23 @@ import datetime import os import re -# Register the various exceptions via decorators. -import endpoints.decorated +from random import SystemRandom +from functools import wraps +from cachetools import lru_cache from flask import make_response, render_template, request, abort, session from flask.ext.login import login_user from flask.ext.principal import identity_changed -from random import SystemRandom -from cachetools import lru_cache from app import app, oauth_apps, LoginWrappedDBUser - from auth.permissions import QuayDeferredPermissionUser from auth import scopes -from functools import wraps from config import frontend_visible_config from external_libraries import get_external_javascript, get_external_css from util.secscan import PRIORITY_LEVELS from util.names import parse_namespace_repository +import endpoints.decorated # Register the various exceptions via decorators. import features logger = logging.getLogger(__name__) @@ -55,21 +53,19 @@ def parse_repository_name(include_tag=False, def inner(func): @wraps(func) def wrapper(*args, **kwargs): - parsed_stuff = parse_namespace_repository(kwargs[incoming_repo_kwarg], - app.config['LIBRARY_NAMESPACE'], - include_tag=include_tag) + repo_name_components = parse_namespace_repository(kwargs[incoming_repo_kwarg], + app.config['LIBRARY_NAMESPACE'], + include_tag=include_tag) del kwargs[incoming_repo_kwarg] - kwargs[ns_kwarg_name] = parsed_stuff[0] - kwargs[repo_kwarg_name] = parsed_stuff[1] + kwargs[ns_kwarg_name] = repo_name_components[0] + kwargs[repo_kwarg_name] = repo_name_components[1] if include_tag: - kwargs[tag_kwarg_name] = parsed_stuff[2] + kwargs[tag_kwarg_name] = repo_name_components[2] return func(*args, **kwargs) return wrapper return inner -# TODO get rid of all calls to this parse_repository_name_and_tag - def route_show_if(value): def decorator(f): @wraps(f) diff --git a/endpoints/v2/__init__.py b/endpoints/v2/__init__.py index 10a69be9d..e57dcb623 100644 --- a/endpoints/v2/__init__.py +++ b/endpoints/v2/__init__.py @@ -1,24 +1,24 @@ import logging -from flask import Blueprint, make_response, url_for, request, jsonify from functools import wraps from urlparse import urlparse + +from flask import Blueprint, make_response, url_for, request, jsonify from semantic_version import Spec import features -from app import metric_queue -from endpoints.decorators import anon_protect, anon_allowed -from endpoints.v2.errors import V2RegistryException, Unauthorized +from app import app, metric_queue from auth.auth_context import get_grant_context from auth.permissions import (ReadRepositoryPermission, ModifyRepositoryPermission, AdministerRepositoryPermission) -from data import model -from app import app -from util.http import abort -from util.saas.metricqueue import time_blueprint -from util.registry.dockerver import docker_version from auth.registry_jwt_auth import process_registry_jwt_auth, get_auth_headers +from data import model +from endpoints.decorators import anon_protect, anon_allowed +from endpoints.v2.errors import V2RegistryException, Unauthorized +from util.http import abort +from util.registry.dockerver import docker_version +from util.saas.metricqueue import time_blueprint logger = logging.getLogger(__name__) v2_bp = Blueprint('v2', __name__) @@ -33,12 +33,12 @@ def handle_registry_v2_exception(error): response.status_code = error.http_status_code if response.status_code == 401: - response.headers.extend(get_auth_headers()) + response.headers.extend(get_auth_headers(repository=error.repository, scopes=error.scopes)) logger.debug('sending response: %s', response.get_data()) return response -def _require_repo_permission(permission_class, allow_public=False): +def _require_repo_permission(permission_class, scopes=None, allow_public=False): def wrapper(func): @wraps(func) def wrapped(namespace_name, repo_name, *args, **kwargs): @@ -49,14 +49,19 @@ def _require_repo_permission(permission_class, allow_public=False): (allow_public and model.repository.repository_is_public(namespace_name, repo_name))): return func(namespace_name, repo_name, *args, **kwargs) - raise Unauthorized() + repository = namespace_name + '/' + repo_name + raise Unauthorized(repository=repository, scopes=scopes) return wrapped return wrapper -require_repo_read = _require_repo_permission(ReadRepositoryPermission, True) -require_repo_write = _require_repo_permission(ModifyRepositoryPermission) -require_repo_admin = _require_repo_permission(AdministerRepositoryPermission) +require_repo_read = _require_repo_permission(ReadRepositoryPermission, + scopes=['pull'], + allow_public=True) +require_repo_write = _require_repo_permission(ModifyRepositoryPermission, + scopes=['pull', 'push']) +require_repo_admin = _require_repo_permission(AdministerRepositoryPermission, + scopes=['pull', 'push']) def get_input_stream(flask_request): @@ -79,7 +84,7 @@ def route_show_if(value): @v2_bp.route('/') @route_show_if(features.ADVERTISE_V2) -@process_registry_jwt_auth +@process_registry_jwt_auth() @anon_allowed def v2_support_enabled(): docker_ver = docker_version(request.user_agent.string) diff --git a/endpoints/v2/blob.py b/endpoints/v2/blob.py index 07182a1d4..aacf0d905 100644 --- a/endpoints/v2/blob.py +++ b/endpoints/v2/blob.py @@ -57,8 +57,8 @@ def _base_blob_fetch(namespace_name, repo_name, digest): @v2_bp.route(BLOB_DIGEST_ROUTE, methods=['HEAD']) -@process_registry_jwt_auth @parse_repository_name() +@process_registry_jwt_auth(scopes=['pull']) @require_repo_read @anon_protect @cache_control(max_age=31436000) @@ -73,8 +73,8 @@ def check_blob_exists(namespace_name, repo_name, digest): @v2_bp.route(BLOB_DIGEST_ROUTE, methods=['GET']) -@process_registry_jwt_auth @parse_repository_name() +@process_registry_jwt_auth(scopes=['pull']) @require_repo_read @anon_protect @cache_control(max_age=31536000) @@ -107,8 +107,8 @@ def _render_range(num_uploaded_bytes, with_bytes_prefix=True): @v2_bp.route('//blobs/uploads/', methods=['POST']) -@process_registry_jwt_auth @parse_repository_name() +@process_registry_jwt_auth(scopes=['pull', 'push']) @require_repo_write @anon_protect def start_blob_upload(namespace_name, repo_name): @@ -143,8 +143,8 @@ def start_blob_upload(namespace_name, repo_name): @v2_bp.route('//blobs/uploads/', methods=['GET']) -@process_registry_jwt_auth @parse_repository_name() +@process_registry_jwt_auth(scopes=['pull']) @require_repo_write @anon_protect def fetch_existing_upload(namespace_name, repo_name, upload_uuid): @@ -325,8 +325,8 @@ def _finish_upload(namespace_name, repo_name, upload_obj, expected_digest): @v2_bp.route('//blobs/uploads/', methods=['PATCH']) -@process_registry_jwt_auth @parse_repository_name() +@process_registry_jwt_auth(scopes=['pull', 'push']) @require_repo_write @anon_protect def upload_chunk(namespace_name, repo_name, upload_uuid): @@ -344,8 +344,8 @@ def upload_chunk(namespace_name, repo_name, upload_uuid): @v2_bp.route('//blobs/uploads/', methods=['PUT']) -@process_registry_jwt_auth @parse_repository_name() +@process_registry_jwt_auth(scopes=['pull', 'push']) @require_repo_write @anon_protect def monolithic_upload_or_last_chunk(namespace_name, repo_name, upload_uuid): @@ -364,7 +364,7 @@ def monolithic_upload_or_last_chunk(namespace_name, repo_name, upload_uuid): @v2_bp.route('//blobs/uploads/', methods=['DELETE']) @parse_repository_name() -@process_registry_jwt_auth +@process_registry_jwt_auth(scopes=['pull', 'push']) @require_repo_write @anon_protect def cancel_upload(namespace_name, repo_name, upload_uuid): @@ -383,8 +383,8 @@ def cancel_upload(namespace_name, repo_name, upload_uuid): @v2_bp.route('//blobs/', methods=['DELETE']) -@process_registry_jwt_auth @parse_repository_name() +@process_registry_jwt_auth(scopes=['pull', 'push']) @require_repo_write @anon_protect def delete_digest(namespace_name, repo_name, upload_uuid): diff --git a/endpoints/v2/catalog.py b/endpoints/v2/catalog.py index 1074fca49..c49b4091a 100644 --- a/endpoints/v2/catalog.py +++ b/endpoints/v2/catalog.py @@ -7,7 +7,7 @@ from data import model from endpoints.v2.v2util import add_pagination @v2_bp.route('/_catalog', methods=['GET']) -@process_registry_jwt_auth +@process_registry_jwt_auth() @anon_protect def catalog_search(): url = url_for('v2.catalog_search') diff --git a/endpoints/v2/errors.py b/endpoints/v2/errors.py index 0f33b92e3..ed0965b2e 100644 --- a/endpoints/v2/errors.py +++ b/endpoints/v2/errors.py @@ -1,7 +1,10 @@ class V2RegistryException(Exception): - def __init__(self, error_code_str, message, detail, http_status_code=400): + def __init__(self, error_code_str, message, detail, http_status_code=400, + repository=None, scopes=None): super(V2RegistryException, self).__init__(message) self.http_status_code = http_status_code + self.repository = repository + self.scopes = scopes self._error_code_str = error_code_str self._detail = detail @@ -104,11 +107,13 @@ class TagInvalid(V2RegistryException): class Unauthorized(V2RegistryException): - def __init__(self, detail=None): + def __init__(self, detail=None, repository=None, scopes=None): super(Unauthorized, self).__init__('UNAUTHORIZED', 'access to the requested resource is not authorized', detail, - 401) + 401, + repository=repository, + scopes=scopes) class Unsupported(V2RegistryException): diff --git a/endpoints/v2/manifest.py b/endpoints/v2/manifest.py index 5fbc248f0..9f6eb38e0 100644 --- a/endpoints/v2/manifest.py +++ b/endpoints/v2/manifest.py @@ -241,8 +241,8 @@ class SignedManifestBuilder(object): @v2_bp.route(MANIFEST_TAGNAME_ROUTE, methods=['GET']) -@process_registry_jwt_auth @parse_repository_name() +@process_registry_jwt_auth(scopes=['pull']) @require_repo_read @anon_protect def fetch_manifest_by_tagname(namespace_name, repo_name, manifest_ref): @@ -272,8 +272,8 @@ def fetch_manifest_by_tagname(namespace_name, repo_name, manifest_ref): @v2_bp.route(MANIFEST_DIGEST_ROUTE, methods=['GET']) -@process_registry_jwt_auth @parse_repository_name() +@process_registry_jwt_auth(scopes=['pull']) @require_repo_read @anon_protect def fetch_manifest_by_digest(namespace_name, repo_name, manifest_ref): @@ -304,8 +304,8 @@ def _reject_manifest2_schema2(func): @v2_bp.route(MANIFEST_TAGNAME_ROUTE, methods=['PUT']) -@process_registry_jwt_auth @parse_repository_name() +@process_registry_jwt_auth(scopes=['pull', 'push']) @require_repo_write @anon_protect @_reject_manifest2_schema2 @@ -322,8 +322,8 @@ def write_manifest_by_tagname(namespace_name, repo_name, manifest_ref): @v2_bp.route(MANIFEST_DIGEST_ROUTE, methods=['PUT']) -@process_registry_jwt_auth @parse_repository_name() +@process_registry_jwt_auth(scopes=['pull', 'push']) @require_repo_write @anon_protect @_reject_manifest2_schema2 @@ -471,8 +471,8 @@ def _write_manifest(namespace_name, repo_name, manifest): @v2_bp.route(MANIFEST_DIGEST_ROUTE, methods=['DELETE']) -@process_registry_jwt_auth @parse_repository_name() +@process_registry_jwt_auth(scopes=['pull', 'push']) @require_repo_write @anon_protect def delete_manifest_by_digest(namespace_name, repo_name, manifest_ref): diff --git a/endpoints/v2/tag.py b/endpoints/v2/tag.py index 19b046468..44e87c7c0 100644 --- a/endpoints/v2/tag.py +++ b/endpoints/v2/tag.py @@ -9,8 +9,8 @@ from endpoints.decorators import anon_protect from data import model @v2_bp.route('//tags/list', methods=['GET']) -@process_registry_jwt_auth @parse_repository_name() +@process_registry_jwt_auth(scopes=['pull']) @require_repo_read @anon_protect def list_all_tags(namespace_name, repo_name): diff --git a/test/test_v2_endpoint_security.py b/test/test_v2_endpoint_security.py index 215c257b1..63128579a 100644 --- a/test/test_v2_endpoint_security.py +++ b/test/test_v2_endpoint_security.py @@ -1,10 +1,12 @@ import unittest import json +import endpoints.decorated # Register the various exceptions via decorators. + from app import app +from endpoints.v2 import v2_bp from initdb import setup_database_for_testing, finished_database_for_testing from test.specs import build_v2_index_specs -from endpoints.v2 import v2_bp app.register_blueprint(v2_bp, url_prefix='/v2')