From bd0a0982824c3d78864d8340b9f55b922c2b71bc Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Tue, 22 Dec 2015 09:05:17 -0500 Subject: [PATCH 1/3] Add ID-based pagination to logs using new decorators and an encrypted token Fixes #599 --- config.py | 5 ++++ data/model/__init__.py | 2 +- data/model/log.py | 9 ++---- data/model/modelutil.py | 29 ++++++++++++++++++++ endpoints/api/__init__.py | 37 +++++++++++++++++++++++++ endpoints/api/logs.py | 41 +++++++++++++++------------- endpoints/api/superuser.py | 8 ++++-- static/js/directives/ui/logs-view.js | 15 ++++++---- 8 files changed, 110 insertions(+), 36 deletions(-) create mode 100644 data/model/modelutil.py diff --git a/config.py b/config.py index 42908672c..7dd862d80 100644 --- a/config.py +++ b/config.py @@ -289,3 +289,8 @@ class DefaultConfig(object): BITTORRENT_PIECE_SIZE = 512 * 1024 BITTORRENT_ANNOUNCE_URL = 'https://localhost:6881/announce' BITTORRENT_FILENAME_PEPPER = '3ae93fef-c30a-427e-9ba0-eea0fd710419' + + # "Secret" key for generating encrypted paging tokens. Only needed to be secret to + # hide the ID range for production (in which this value is overridden). Should *not* + # be relied upon for secure encryption otherwise. + PAGE_TOKEN_KEY = 'GdE4<~8&9LHDPM++' diff --git a/data/model/__init__.py b/data/model/__init__.py index cfc68a0c3..b6f4cd975 100644 --- a/data/model/__init__.py +++ b/data/model/__init__.py @@ -95,4 +95,4 @@ config = Config() # moving the minimal number of things to _basequery # TODO document the methods and modules for each one of the submodules below. from data.model import (blob, build, image, log, notification, oauth, organization, permission, - repository, storage, tag, team, token, user, release) + repository, storage, tag, team, token, user, release, modelutil) diff --git a/data/model/log.py b/data/model/log.py index 9bf11216d..a2037e6f7 100644 --- a/data/model/log.py +++ b/data/model/log.py @@ -41,9 +41,7 @@ def get_aggregated_logs(start_time, end_time, performer=None, repository=None, n return query.group_by(date, LogEntry.kind) -def list_logs(start_time, end_time, performer=None, repository=None, namespace=None, page=None, - count=None): - +def get_logs_query(start_time, end_time, performer=None, repository=None, namespace=None): Performer = User.alias() selections = [LogEntry, Performer] @@ -52,10 +50,7 @@ def list_logs(start_time, end_time, performer=None, repository=None, namespace=N .join(Performer, JOIN_LEFT_OUTER, on=(LogEntry.performer == Performer.id).alias('performer'))) - if page and count: - query = query.paginate(page, count) - - return list(query.order_by(LogEntry.datetime.desc())) + return query def log_action(kind_name, user_or_organization_name, performer=None, repository=None, diff --git a/data/model/modelutil.py b/data/model/modelutil.py new file mode 100644 index 000000000..197baa83e --- /dev/null +++ b/data/model/modelutil.py @@ -0,0 +1,29 @@ +def paginate(query, model, descending=False, page_token=None, limit=50): + """ Paginates the given query using an ID range, starting at the optional page_token. + Returns a *list* of matching results along with an unencrypted page_token for the + next page, if any. If descending is set to True, orders by the ID descending rather + than ascending. + """ + query = query.limit(limit + 1) + + if descending: + query = query.order_by(model.id.desc()) + else: + query = query.order_by(model.id) + + if page_token is not None: + start_id = page_token.get('start_id') + if start_id is not None: + if descending: + query = query.where(model.id <= start_id) + else: + query = query.where(model.id >= start_id) + + results = list(query) + page_token = None + if len(results) > limit: + page_token = { + 'start_id': results[limit].id + } + + return results[0:limit], page_token diff --git a/endpoints/api/__init__.py b/endpoints/api/__init__.py index 018b5277d..575e49a0c 100644 --- a/endpoints/api/__init__.py +++ b/endpoints/api/__init__.py @@ -1,5 +1,6 @@ import logging import datetime +import json from app import app, metric_queue from flask import Blueprint, request, make_response, jsonify, session @@ -21,6 +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.aes import AESCipher logger = logging.getLogger(__name__) @@ -209,6 +211,41 @@ def query_param(name, help_str, type=reqparse.text_type, default=None, return add_param +def page_support(func): + """ Adds pagination support to an API endpoint. The decorated API will have an + added query parameter named 'next_page'. Works in tandem with the + modelutil paginate method. + """ + @wraps(func) + @query_param('next_page', 'The page token for the next page', type=str) + def wrapper(self, query_args, *args, **kwargs): + page_token = None + unecrypted = None + + if query_args['next_page']: + # Decrypt the page token. + cipher = AESCipher(app.config['PAGE_TOKEN_KEY']) + try: + unecrypted = cipher.decrypt(query_args['next_page']) + except TypeError: + pass + + if unecrypted is not None: + try: + page_token = json.loads(unecrypted) + except ValueError: + pass + + (result, next_page_token) = func(self, query_args, page_token, *args, **kwargs) + if next_page_token is not None: + cipher = AESCipher(app.config['PAGE_TOKEN_KEY']) + result['next_page'] = cipher.encrypt(json.dumps(next_page_token)) + + return result + + return wrapper + + def parse_args(func): @wraps(func) def wrapper(self, *args, **kwargs): diff --git a/endpoints/api/logs.py b/endpoints/api/logs.py index b3e910401..62c397c4d 100644 --- a/endpoints/api/logs.py +++ b/endpoints/api/logs.py @@ -8,15 +8,14 @@ from dateutil.relativedelta import relativedelta from endpoints.api import (resource, nickname, ApiResource, query_param, parse_args, RepositoryParamResource, require_repo_admin, related_user_resource, format_date, Unauthorized, NotFound, require_user_admin, - internal_only, path_param, require_scope) + internal_only, path_param, require_scope, page_support) from auth.permissions import AdministerOrganizationPermission, AdministerOrganizationPermission from auth.auth_context import get_authenticated_user -from data import model +from data import model, database from auth import scopes from app import avatar -LOGS_PER_PAGE = 50 -MAX_PAGES = 20 +LOGS_PER_PAGE = 20 def log_view(log, kinds): view = { @@ -79,20 +78,22 @@ def _validate_logs_arguments(start_time, end_time, performer_name): return (start_time, end_time, performer) -def get_logs(start_time, end_time, performer_name=None, repository=None, namespace=None, page=None): +def get_logs(start_time, end_time, performer_name=None, repository=None, namespace=None, + page_token=None): (start_time, end_time, performer) = _validate_logs_arguments(start_time, end_time, performer_name) - page = min(MAX_PAGES, page if page else 1) kinds = model.log.get_log_entry_kinds() - logs = model.log.list_logs(start_time, end_time, performer=performer, repository=repository, - namespace=namespace, page=page, count=LOGS_PER_PAGE + 1) + logs_query = model.log.get_logs_query(start_time, end_time, performer=performer, + repository=repository, namespace=namespace) + + logs, next_page_token = model.modelutil.paginate(logs_query, database.LogEntry, descending=True, + page_token=page_token, limit=LOGS_PER_PAGE) return { 'start_time': format_date(start_time), 'end_time': format_date(end_time), - 'logs': [log_view(log, kinds) for log in logs[0:LOGS_PER_PAGE]], - 'page': page, - 'has_additional': len(logs) > LOGS_PER_PAGE, - } + 'logs': [log_view(log, kinds) for log in logs], + }, next_page_token + def get_aggregate_logs(start_time, end_time, performer_name=None, repository=None, namespace=None): (start_time, end_time, performer) = _validate_logs_arguments(start_time, end_time, performer_name) @@ -116,7 +117,8 @@ class RepositoryLogs(RepositoryParamResource): @query_param('starttime', 'Earliest time from which to get logs (%m/%d/%Y %Z)', type=str) @query_param('endtime', 'Latest time to which to get logs (%m/%d/%Y %Z)', type=str) @query_param('page', 'The page number for the logs', type=int, default=1) - def get(self, args, namespace, repository): + @page_support + def get(self, args, page_token, namespace, repository): """ List the logs for the specified repository. """ repo = model.repository.get_repository(namespace, repository) if not repo: @@ -124,7 +126,7 @@ class RepositoryLogs(RepositoryParamResource): start_time = args['starttime'] end_time = args['endtime'] - return get_logs(start_time, end_time, repository=repo, page=args['page']) + return get_logs(start_time, end_time, repository=repo, page_token=page_token) @resource('/v1/user/logs') @@ -137,8 +139,8 @@ class UserLogs(ApiResource): @query_param('starttime', 'Earliest time from which to get logs. (%m/%d/%Y %Z)', type=str) @query_param('endtime', 'Latest time to which to get logs. (%m/%d/%Y %Z)', type=str) @query_param('performer', 'Username for which to filter logs.', type=str) - @query_param('page', 'The page number for the logs', type=int, default=1) - def get(self, args): + @page_support + def get(self, args, page_token): """ List the logs for the current user. """ performer_name = args['performer'] start_time = args['starttime'] @@ -146,7 +148,7 @@ class UserLogs(ApiResource): user = get_authenticated_user() return get_logs(start_time, end_time, performer_name=performer_name, namespace=user.username, - page=args['page']) + page_token=page_token) @resource('/v1/organization//logs') @@ -160,8 +162,9 @@ class OrgLogs(ApiResource): @query_param('endtime', 'Latest time to which to get logs. (%m/%d/%Y %Z)', type=str) @query_param('performer', 'Username for which to filter logs.', type=str) @query_param('page', 'The page number for the logs', type=int, default=1) + @page_support @require_scope(scopes.ORG_ADMIN) - def get(self, args, orgname): + def get(self, args, page_token, orgname): """ List the logs for the specified organization. """ permission = AdministerOrganizationPermission(orgname) if permission.can(): @@ -170,7 +173,7 @@ class OrgLogs(ApiResource): end_time = args['endtime'] return get_logs(start_time, end_time, namespace=orgname, performer_name=performer_name, - page=args['page']) + page_token=page_token) raise Unauthorized() diff --git a/endpoints/api/superuser.py b/endpoints/api/superuser.py index 62218075f..71b154910 100644 --- a/endpoints/api/superuser.py +++ b/endpoints/api/superuser.py @@ -12,7 +12,8 @@ import features from app import app, avatar, superusers, authentication, config_provider from endpoints.api import (ApiResource, nickname, resource, validate_json_request, internal_only, require_scope, show_if, parse_args, - query_param, abort, require_fresh_login, path_param, verify_not_prod) + query_param, abort, require_fresh_login, path_param, verify_not_prod, + page_support) from endpoints.api.logs import get_logs, get_aggregate_logs from data import model from auth.permissions import SuperUserPermission @@ -116,14 +117,15 @@ class SuperUserLogs(ApiResource): @query_param('starttime', 'Earliest time from which to get logs (%m/%d/%Y %Z)', type=str) @query_param('endtime', 'Latest time to which to get logs (%m/%d/%Y %Z)', type=str) @query_param('page', 'The page number for the logs', type=int, default=1) + @page_support @require_scope(scopes.SUPERUSER) - def get(self, args): + def get(self, args, page_token): """ List the usage logs for the current system. """ if SuperUserPermission().can(): start_time = args['starttime'] end_time = args['endtime'] - return get_logs(start_time, end_time, page=args['page']) + return get_logs(start_time, end_time, page_token=page_token) abort(403) diff --git a/static/js/directives/ui/logs-view.js b/static/js/directives/ui/logs-view.js index 44b021a41..c62296187 100644 --- a/static/js/directives/ui/logs-view.js +++ b/static/js/directives/ui/logs-view.js @@ -23,7 +23,6 @@ angular.module('quay').directive('logsView', function () { $scope.kindsAllowed = null; $scope.chartVisible = true; $scope.chartLoading = true; - $scope.currentPage = 1; $scope.options = {}; @@ -309,7 +308,7 @@ angular.module('quay').directive('logsView', function () { $scope.chartLoading = false; }); - $scope.currentPage = 0; + $scope.nextPageToken = null; $scope.hasAdditional = true; $scope.loading = false; $scope.logs = []; @@ -317,11 +316,15 @@ angular.module('quay').directive('logsView', function () { }; $scope.nextPage = function() { - if ($scope.loading) { return; } + if ($scope.loading || !$scope.hasAdditional) { return; } $scope.loading = true; - var logsUrl = getUrl('logs') + '&page=' + ($scope.currentPage + 1); + var logsUrl = getUrl('logs'); + if ($scope.nextPageToken) { + logsUrl = logsUrl + '&next_page=' + encodeURIComponent($scope.nextPageToken); + } + var loadLogs = Restangular.one(logsUrl); loadLogs.customGET().then(function(resp) { resp.logs.forEach(function(log) { @@ -329,8 +332,8 @@ angular.module('quay').directive('logsView', function () { }); $scope.loading = false; - $scope.currentPage = resp.page; - $scope.hasAdditional = resp.has_additional; + $scope.nextPageToken = resp.next_page; + $scope.hasAdditional = !!resp.next_page; }); }; From b4bddacedbaf9bc215990f7a2847abd85e02c19c Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Tue, 12 Jan 2016 14:08:41 -0500 Subject: [PATCH 2/3] Switch to Fernet crypto as per gtank's recommendation --- config.py | 2 +- endpoints/api/__init__.py | 19 +++++++------------ util/security/crypto.py | 18 ++++++++++++++++++ 3 files changed, 26 insertions(+), 13 deletions(-) create mode 100644 util/security/crypto.py diff --git a/config.py b/config.py index 7dd862d80..d0766680e 100644 --- a/config.py +++ b/config.py @@ -293,4 +293,4 @@ class DefaultConfig(object): # "Secret" key for generating encrypted paging tokens. Only needed to be secret to # hide the ID range for production (in which this value is overridden). Should *not* # be relied upon for secure encryption otherwise. - PAGE_TOKEN_KEY = 'GdE4<~8&9LHDPM++' + PAGE_TOKEN_KEY = 'um=/?Kqgp)2yQaS/A6C{NL=dXE&>C:}(' diff --git a/endpoints/api/__init__.py b/endpoints/api/__init__.py index 575e49a0c..2dc28490c 100644 --- a/endpoints/api/__init__.py +++ b/endpoints/api/__init__.py @@ -22,7 +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.aes import AESCipher +from util.security.crypto import encrypt_string, decrypt_string logger = logging.getLogger(__name__) @@ -220,26 +220,21 @@ def page_support(func): @query_param('next_page', 'The page token for the next page', type=str) def wrapper(self, query_args, *args, **kwargs): page_token = None - unecrypted = None if query_args['next_page']: # Decrypt the page token. - cipher = AESCipher(app.config['PAGE_TOKEN_KEY']) - try: - unecrypted = cipher.decrypt(query_args['next_page']) - except TypeError: - pass - - if unecrypted is not None: + unencrypted = decrypt_string(query_args['next_page'], app.config['PAGE_TOKEN_KEY']) + if unencrypted is not None: try: - page_token = json.loads(unecrypted) + page_token = json.loads(unencrypted) except ValueError: pass + # Note: if page_token is None, we'll receive the first page of results back. (result, next_page_token) = func(self, query_args, page_token, *args, **kwargs) if next_page_token is not None: - cipher = AESCipher(app.config['PAGE_TOKEN_KEY']) - result['next_page'] = cipher.encrypt(json.dumps(next_page_token)) + result['next_page'] = encrypt_string(json.dumps(next_page_token), + app.config['PAGE_TOKEN_KEY']) return result diff --git a/util/security/crypto.py b/util/security/crypto.py new file mode 100644 index 000000000..fe1de2953 --- /dev/null +++ b/util/security/crypto.py @@ -0,0 +1,18 @@ +import base64 + +from cryptography.fernet import Fernet, InvalidToken + +def encrypt_string(string, key): + """ Encrypts a string with the specified key. The key must be 32 raw bytes. """ + f = Fernet(base64.urlsafe_b64encode(key)) + return f.encrypt(string) + +def decrypt_string(string, key): + """ 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) + except InvalidToken: + return None + except TypeError: + return None From 335c8eb3a93d1c2e656fc4ac28321c07cc6735f1 Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Wed, 20 Jan 2016 13:53:51 -0500 Subject: [PATCH 3/3] Add 2 day TTL to page tokens --- endpoints/api/__init__.py | 7 +++++-- endpoints/api/logs.py | 4 ++-- util/security/crypto.py | 4 ++-- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/endpoints/api/__init__.py b/endpoints/api/__init__.py index 2dc28490c..78100f4a7 100644 --- a/endpoints/api/__init__.py +++ b/endpoints/api/__init__.py @@ -24,6 +24,8 @@ 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() logger = logging.getLogger(__name__) api_bp = Blueprint('api', __name__) @@ -223,7 +225,8 @@ def page_support(func): if query_args['next_page']: # Decrypt the page token. - unencrypted = decrypt_string(query_args['next_page'], app.config['PAGE_TOKEN_KEY']) + unencrypted = decrypt_string(query_args['next_page'], app.config['PAGE_TOKEN_KEY'], + ttl=_PAGE_TOKEN_TTL) if unencrypted is not None: try: page_token = json.loads(unencrypted) @@ -231,7 +234,7 @@ def page_support(func): pass # Note: if page_token is None, we'll receive the first page of results back. - (result, next_page_token) = func(self, query_args, page_token, *args, **kwargs) + (result, next_page_token) = func(self, query_args, page_token=page_token, *args, **kwargs) if next_page_token is not None: result['next_page'] = encrypt_string(json.dumps(next_page_token), app.config['PAGE_TOKEN_KEY']) diff --git a/endpoints/api/logs.py b/endpoints/api/logs.py index 62c397c4d..d2c144c1f 100644 --- a/endpoints/api/logs.py +++ b/endpoints/api/logs.py @@ -118,7 +118,7 @@ class RepositoryLogs(RepositoryParamResource): @query_param('endtime', 'Latest time to which to get logs (%m/%d/%Y %Z)', type=str) @query_param('page', 'The page number for the logs', type=int, default=1) @page_support - def get(self, args, page_token, namespace, repository): + def get(self, args, namespace, repository, page_token): """ List the logs for the specified repository. """ repo = model.repository.get_repository(namespace, repository) if not repo: @@ -164,7 +164,7 @@ class OrgLogs(ApiResource): @query_param('page', 'The page number for the logs', type=int, default=1) @page_support @require_scope(scopes.ORG_ADMIN) - def get(self, args, page_token, orgname): + def get(self, args, orgname, page_token): """ List the logs for the specified organization. """ permission = AdministerOrganizationPermission(orgname) if permission.can(): diff --git a/util/security/crypto.py b/util/security/crypto.py index fe1de2953..1bf46228b 100644 --- a/util/security/crypto.py +++ b/util/security/crypto.py @@ -7,11 +7,11 @@ def encrypt_string(string, key): f = Fernet(base64.urlsafe_b64encode(key)) return f.encrypt(string) -def decrypt_string(string, key): +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) + return f.decrypt(string, ttl=ttl) except InvalidToken: return None except TypeError: