From db0eab04613da803a7a6e86f3cd777291210748a Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Wed, 10 Feb 2016 00:25:33 +0200 Subject: [PATCH] Fix V2 catalog and tag pagination --- endpoints/api/__init__.py | 24 ++++------------------ endpoints/api/logs.py | 2 +- endpoints/v2/catalog.py | 11 ++++++---- endpoints/v2/v2util.py | 43 ++++++++++++++++++++++++++++++--------- test/registry_tests.py | 35 +++++++++++++++++++++++++++++-- util/pagination.py | 26 +++++++++++++++++++++++ util/security/crypto.py | 2 +- 7 files changed, 105 insertions(+), 38 deletions(-) create mode 100644 util/pagination.py diff --git a/endpoints/api/__init__.py b/endpoints/api/__init__.py index 7a3e4ea99..fe8d73e6d 100644 --- a/endpoints/api/__init__.py +++ b/endpoints/api/__init__.py @@ -22,10 +22,7 @@ from auth.auth import process_oauth from endpoints.csrf import csrf_protect from endpoints.decorators import check_anon_protection from util.saas.metricqueue import time_decorator -from util.security.crypto import encrypt_string, decrypt_string - -# TTL (in seconds) for page tokens. -_PAGE_TOKEN_TTL = datetime.timedelta(days=2).total_seconds() +from util.pagination import encrypt_page_token, decrypt_page_token logger = logging.getLogger(__name__) api_bp = Blueprint('api', __name__) @@ -221,28 +218,15 @@ def page_support(page_token_kwarg='page_token', parsed_args_kwarg='parsed_args') @wraps(func) @query_param('next_page', 'The page token for the next page', type=str) def wrapper(self, *args, **kwargs): - page_token = None - - if kwargs[parsed_args_kwarg]['next_page']: - # Decrypt the page token. - unencrypted = decrypt_string(kwargs[parsed_args_kwarg]['next_page'], - app.config['PAGE_TOKEN_KEY'], - ttl=_PAGE_TOKEN_TTL) - if unencrypted is not None: - try: - page_token = json.loads(unencrypted) - except ValueError: - pass - # Note: if page_token is None, we'll receive the first page of results back. + page_token = decrypt_page_token(kwargs[parsed_args_kwarg]['next_page']) kwargs[page_token_kwarg] = page_token + (result, next_page_token) = func(self, *args, **kwargs) if next_page_token is not None: - result['next_page'] = encrypt_string(json.dumps(next_page_token), - app.config['PAGE_TOKEN_KEY']) + result['next_page'] = encrypt_page_token(next_page_token) return result - return wrapper return inner diff --git a/endpoints/api/logs.py b/endpoints/api/logs.py index e47939c60..3f805c69c 100644 --- a/endpoints/api/logs.py +++ b/endpoints/api/logs.py @@ -43,7 +43,7 @@ def aggregated_log_view(log, kinds, start_time): if synthetic_date.day < start_time.day: synthetic_date = synthetic_date + relativedelta(months=1) - view = { + view = { 'kind': kinds[log.kind_id], 'count': log.count, 'datetime': format_date(synthetic_date), diff --git a/endpoints/v2/catalog.py b/endpoints/v2/catalog.py index cb59d6cc5..1074fca49 100644 --- a/endpoints/v2/catalog.py +++ b/endpoints/v2/catalog.py @@ -1,19 +1,22 @@ from flask import jsonify, url_for from endpoints.v2 import v2_bp -from auth.auth import process_auth +from auth.registry_jwt_auth import process_registry_jwt_auth, get_granted_entity from endpoints.decorators import anon_protect from data import model from endpoints.v2.v2util import add_pagination -from auth.auth_context import get_authenticated_user @v2_bp.route('/_catalog', methods=['GET']) -@process_auth +@process_registry_jwt_auth @anon_protect def catalog_search(): url = url_for('v2.catalog_search') - username = get_authenticated_user().username if get_authenticated_user() else None + username = None + entity = get_granted_entity() + if entity: + username = entity.user.username + query = model.repository.get_visible_repositories(username, include_public=(username is None)) link, query = add_pagination(query, url) diff --git a/endpoints/v2/v2util.py b/endpoints/v2/v2util.py index 78ffdd756..df4a70fb9 100644 --- a/endpoints/v2/v2util.py +++ b/endpoints/v2/v2util.py @@ -1,19 +1,42 @@ from flask import request from app import get_app_url +from util.pagination import encrypt_page_token, decrypt_page_token +import urllib +import logging -_MAX_RESULTS_PER_PAGE = 100 +_MAX_RESULTS_PER_PAGE = 50 def add_pagination(query, url): """ Adds optional pagination to the given query by looking for the Docker V2 pagination request - args. """ - limit = request.args.get('n', None) - page = request.args.get('page', 1) + args. + """ + try: + requested_limit = int(request.args.get('n', _MAX_RESULTS_PER_PAGE)) + except ValueError: + requested_limit = 0 - if limit is None: - return None, query + limit = max(min(requested_limit, _MAX_RESULTS_PER_PAGE), 1) + next_page_token = request.args.get('next_page', None) - limit = max(limit, _MAX_RESULTS_PER_PAGE) + # Decrypt the next page token, if any. + offset = 0 + page_info = decrypt_page_token(next_page_token) + if page_info is not None: + # Note: we use offset here instead of ID >= n because one of the V2 queries is a UNION. + offset = page_info.get('offset', 0) + query = query.offset(offset) + + query = query.limit(limit + 1) url = get_app_url() + url - query = query.paginate(page, limit) - link = url + '?n=%s&last=%s; rel="next"' % (limit, page + 1) - return link, query + + results = list(query) + if len(results) <= limit: + return None, results + + # Add a link to the next page of results. + page_info = dict(offset=limit + offset) + next_page_token = encrypt_page_token(page_info) + + link = url + '?' + urllib.urlencode(dict(n=limit, next_page=next_page_token)) + link = link + '; rel="next"' + return link, results[0:-1] diff --git a/test/registry_tests.py b/test/registry_tests.py index a5770bc8a..6a7b977ec 100644 --- a/test/registry_tests.py +++ b/test/registry_tests.py @@ -458,10 +458,12 @@ class V2RegistryMixin(BaseRegistryMixin): params = { 'account': username, - 'scope': 'repository:%s:%s' % (repo_name, ','.join(scopes)), 'service': app.config['SERVER_HOSTNAME'], } + if scopes: + params['scope'] = 'repository:%s:%s' % (repo_name, ','.join(scopes)) + response = self.conduct('GET', '/v2/auth', params=params, auth=auth, expected_code=expected_code) @@ -1200,7 +1202,6 @@ class V2RegistryTests(V2RegistryPullMixin, V2RegistryPushMixin, RegistryTestsMix data = json.loads(response.text) self.assertEquals(data['name'], "devtable/newrepo") self.assertEquals(len(data['tags']), 1) - self.assertTrue(response.headers['Link'].find('n=1&last=2') > 0) # Try to get tags before a repo exists. self.conduct('GET', '/v2/devtable/doesnotexist/tags/list', auth='jwt', expected_code=401) @@ -1208,6 +1209,36 @@ class V2RegistryTests(V2RegistryPullMixin, V2RegistryPushMixin, RegistryTestsMix def test_one_five_blacklist(self): self.conduct('GET', '/v2/', expected_code=404, user_agent='Go 1.1 package http') + def test_catalog(self): + # Look for public repositories and ensure all are public. + response = self.conduct('GET', '/v2/_catalog') + data = response.json() + self.assertTrue(len(data['repositories']) > 0) + + for reponame in data['repositories']: + self.assertTrue(reponame.find('public/') == 0) + + # Perform auth and lookup the catalog again. + self.do_auth('devtable', 'password', 'devtable', 'simple') + + response = self.conduct('GET', '/v2/_catalog', params=dict(n=2), auth='jwt') + data = response.json() + self.assertEquals(len(data['repositories']), 2) + + # Ensure we have a next link. + self.assertIsNotNone(response.headers.get('Link')) + + # Request with the next link. + link_url = response.headers.get('Link').split(';')[0] + v2_index = link_url.find('/v2/') + relative_url = link_url[v2_index:] + + next_response = self.conduct('GET', relative_url, auth='jwt') + next_data = next_response.json() + + self.assertEquals(len(next_data['repositories']), 2) + self.assertNotEquals(next_data['repositories'], data['repositories']) + class V1PushV2PullRegistryTests(V2RegistryPullMixin, V1RegistryPushMixin, RegistryTestsMixin, RegistryTestCaseMixin, LiveServerTestCase): diff --git a/util/pagination.py b/util/pagination.py new file mode 100644 index 000000000..579541496 --- /dev/null +++ b/util/pagination.py @@ -0,0 +1,26 @@ +import datetime +import json + +from app import app +from util.security.crypto import encrypt_string, decrypt_string + +# TTL (in seconds) for page tokens. +_PAGE_TOKEN_TTL = datetime.timedelta(days=2).total_seconds() + +def decrypt_page_token(token_string): + if token_string is None: + return None + + unencrypted = decrypt_string(token_string, app.config['PAGE_TOKEN_KEY'], ttl=_PAGE_TOKEN_TTL) + if unencrypted is None: + return None + + try: + return json.loads(unencrypted) + except ValueError: + return None + + +def encrypt_page_token(page_token): + return encrypt_string(json.dumps(page_token), app.config['PAGE_TOKEN_KEY']) + diff --git a/util/security/crypto.py b/util/security/crypto.py index 1bf46228b..0792c9d66 100644 --- a/util/security/crypto.py +++ b/util/security/crypto.py @@ -11,7 +11,7 @@ def decrypt_string(string, key, ttl=None): """ Decrypts an encrypted string with the specified key. The key must be 32 raw bytes. """ f = Fernet(base64.urlsafe_b64encode(key)) try: - return f.decrypt(string, ttl=ttl) + return f.decrypt(str(string), ttl=ttl) except InvalidToken: return None except TypeError: