From 17f3de811eac1c8a6ff834aeff8176207b9cda36 Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Thu, 20 Jul 2017 11:07:31 -0400 Subject: [PATCH 1/9] Move route_show_if into decorators Also removes unused route_hide_if --- endpoints/bitbuckettrigger.py | 2 +- endpoints/common.py | 23 ----------------------- endpoints/decorators.py | 14 ++++++++++++++ endpoints/githubtrigger.py | 3 ++- endpoints/gitlabtrigger.py | 2 +- endpoints/secscan.py | 2 +- endpoints/v2/__init__.py | 16 +--------------- endpoints/verbs/__init__.py | 4 ++-- endpoints/web.py | 4 ++-- 9 files changed, 24 insertions(+), 46 deletions(-) diff --git a/endpoints/bitbuckettrigger.py b/endpoints/bitbuckettrigger.py index e58f04db2..7e521c10d 100644 --- a/endpoints/bitbuckettrigger.py +++ b/endpoints/bitbuckettrigger.py @@ -8,7 +8,7 @@ from auth.decorators import require_session_login from buildtrigger.basehandler import BuildTriggerHandler from buildtrigger.bitbuckethandler import BitbucketBuildTrigger from data import model -from endpoints.common import route_show_if +from endpoints.decorators import route_show_if from util.http import abort import features diff --git a/endpoints/common.py b/endpoints/common.py index a43ecbd74..50eccc0f4 100644 --- a/endpoints/common.py +++ b/endpoints/common.py @@ -54,29 +54,6 @@ def parse_repository_name(include_tag=False, return inner -def route_show_if(value): - def decorator(f): - @wraps(f) - def decorated_function(*args, **kwargs): - if not value: - abort(404) - - return f(*args, **kwargs) - return decorated_function - return decorator - - -def route_hide_if(value): - def decorator(f): - @wraps(f) - def decorated_function(*args, **kwargs): - if value: - abort(404) - - return f(*args, **kwargs) - return decorated_function - return decorator - def truthy_param(param): return param not in {False, 'false', 'False', '0', 'FALSE', '', 'null'} diff --git a/endpoints/decorators.py b/endpoints/decorators.py index 3cc374db3..89093806a 100644 --- a/endpoints/decorators.py +++ b/endpoints/decorators.py @@ -38,3 +38,17 @@ def check_anon_protection(func): abort(401) return wrapper + + +def route_show_if(value): + """ Adds/shows the decorated route if the given value is True. """ + def decorator(f): + @wraps(f) + def decorated_function(*args, **kwargs): + if not value: + abort(404) + + return f(*args, **kwargs) + return decorated_function + return decorator + diff --git a/endpoints/githubtrigger.py b/endpoints/githubtrigger.py index 416df8842..3b373bdf0 100644 --- a/endpoints/githubtrigger.py +++ b/endpoints/githubtrigger.py @@ -9,7 +9,8 @@ from app import app, github_trigger from auth.decorators import require_session_login from auth.permissions import AdministerRepositoryPermission from data import model -from endpoints.common import route_show_if, parse_repository_name +from endpoints.common import parse_repository_name +from endpoints.decorators import route_show_if from util.http import abort diff --git a/endpoints/gitlabtrigger.py b/endpoints/gitlabtrigger.py index e1bb31001..4d97caffe 100644 --- a/endpoints/gitlabtrigger.py +++ b/endpoints/gitlabtrigger.py @@ -9,7 +9,7 @@ from app import app, gitlab_trigger from auth.decorators import require_session_login from auth.permissions import AdministerRepositoryPermission from data import model -from endpoints.common import route_show_if +from endpoints.decorators import route_show_if from util.http import abort diff --git a/endpoints/secscan.py b/endpoints/secscan.py index 6ce803e1e..ca061a050 100644 --- a/endpoints/secscan.py +++ b/endpoints/secscan.py @@ -5,7 +5,7 @@ import features from app import secscan_notification_queue from flask import request, make_response, Blueprint, abort -from endpoints.common import route_show_if +from endpoints.decorators import route_show_if logger = logging.getLogger(__name__) secscan = Blueprint('secscan', __name__) diff --git a/endpoints/v2/__init__.py b/endpoints/v2/__init__.py index db490de02..198b56cbe 100644 --- a/endpoints/v2/__init__.py +++ b/endpoints/v2/__init__.py @@ -15,7 +15,7 @@ from auth.auth_context import get_grant_context from auth.permissions import ( ReadRepositoryPermission, ModifyRepositoryPermission, AdministerRepositoryPermission) from auth.registry_jwt_auth import process_registry_jwt_auth, get_auth_headers -from endpoints.decorators import anon_protect, anon_allowed +from endpoints.decorators import anon_protect, anon_allowed, route_show_if from endpoints.v2.errors import V2RegistryException, Unauthorized, Unsupported, NameUnknown from endpoints.v2.models_pre_oci import data_model as model from util.http import abort @@ -126,20 +126,6 @@ def get_input_stream(flask_request): return flask_request.stream -def route_show_if(value): - def decorator(f): - @wraps(f) - def decorated_function(*args, **kwargs): - if not value: - abort(404) - - return f(*args, **kwargs) - - return decorated_function - - return decorator - - @v2_bp.route('/') @route_show_if(features.ADVERTISE_V2) @process_registry_jwt_auth() diff --git a/endpoints/verbs/__init__.py b/endpoints/verbs/__init__.py index 93f863989..0522c5a38 100644 --- a/endpoints/verbs/__init__.py +++ b/endpoints/verbs/__init__.py @@ -10,8 +10,8 @@ from auth.auth_context import get_authenticated_user from auth.decorators import process_auth from auth.permissions import ReadRepositoryPermission from data import database -from endpoints.common import route_show_if, parse_repository_name -from endpoints.decorators import anon_protect +from endpoints.common import parse_repository_name +from endpoints.decorators import anon_protect, route_show_if from endpoints.verbs.models_pre_oci import pre_oci_model as model from endpoints.v2.blob import BLOB_DIGEST_ROUTE from image.appc import AppCImageFormatter diff --git a/endpoints/web.py b/endpoints/web.py index feefc8465..db4a9d696 100644 --- a/endpoints/web.py +++ b/endpoints/web.py @@ -27,10 +27,10 @@ from buildtrigger.triggerutil import TriggerProviderException from data import model from data.database import db from endpoints.api.discovery import swagger_route_data -from endpoints.common import (common_login, render_page_template, route_show_if, param_required, +from endpoints.common import (common_login, render_page_template, param_required, parse_repository_name) from endpoints.csrf import csrf_protect, generate_csrf_token, verify_csrf -from endpoints.decorators import anon_protect, anon_allowed +from endpoints.decorators import anon_protect, anon_allowed, route_show_if from health.healthcheck import get_healthchecker from util.cache import no_cache from util.headers import parse_basic_auth From 0531f6c749965ffb76e6fea8a1495f055e3faf10 Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Thu, 20 Jul 2017 11:08:24 -0400 Subject: [PATCH 2/9] Small cleanups to the decorators file --- endpoints/decorators.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/endpoints/decorators.py b/endpoints/decorators.py index 89093806a..6534bc4e9 100644 --- a/endpoints/decorators.py +++ b/endpoints/decorators.py @@ -1,14 +1,11 @@ """ Various decorators for endpoint and API handlers. """ from functools import wraps - from flask import abort import features - -from auth.auth_context import (get_validated_oauth_token, get_authenticated_user, - get_validated_token, get_grant_context) -from data import model # TODO: stop using model directly +from auth.auth_context import ( + get_validated_oauth_token, get_authenticated_user, get_validated_token, get_grant_context) def anon_allowed(func): @@ -25,6 +22,7 @@ def anon_protect(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. @@ -37,11 +35,13 @@ def check_anon_protection(func): return func(*args, **kwargs) abort(401) + return wrapper def route_show_if(value): """ Adds/shows the decorated route if the given value is True. """ + def decorator(f): @wraps(f) def decorated_function(*args, **kwargs): @@ -51,4 +51,3 @@ def route_show_if(value): return f(*args, **kwargs) return decorated_function return decorator - From a64f268344df1d32cafbf703a9894c22376e6a4c Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Thu, 20 Jul 2017 11:12:17 -0400 Subject: [PATCH 3/9] Remove unused random_string method --- endpoints/common.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/endpoints/common.py b/endpoints/common.py index 50eccc0f4..6a18d1f0b 100644 --- a/endpoints/common.py +++ b/endpoints/common.py @@ -98,9 +98,6 @@ def common_login(db_user, permanent_session=True): logger.debug('User could not be logged in, inactive?.') return False -def random_string(): - random = SystemRandom() - return ''.join([random.choice(string.ascii_uppercase + string.digits) for _ in range(8)]) def list_files(path, extension): import os From 98e2ccf74d58474a9291cec4e423131f5032f67e Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Thu, 20 Jul 2017 11:31:22 -0400 Subject: [PATCH 4/9] Move parse_repository_name into decorators --- endpoints/common.py | 36 ++++-------------------------------- endpoints/decorators.py | 27 +++++++++++++++++++++++++++ endpoints/githubtrigger.py | 3 +-- endpoints/v1/index.py | 3 +-- endpoints/v1/tag.py | 3 +-- endpoints/v2/blob.py | 3 +-- endpoints/v2/manifest.py | 3 +-- endpoints/v2/tag.py | 3 +-- endpoints/verbs/__init__.py | 3 +-- endpoints/web.py | 5 ++--- 10 files changed, 40 insertions(+), 49 deletions(-) diff --git a/endpoints/common.py b/endpoints/common.py index 6a18d1f0b..887e34394 100644 --- a/endpoints/common.py +++ b/endpoints/common.py @@ -1,14 +1,9 @@ import logging -import json -import string import datetime import os -import re -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_login import login_user from flask_principal import identity_changed @@ -21,11 +16,10 @@ from auth import scopes from auth.permissions import QuayDeferredPermissionUser from config import frontend_visible_config from external_libraries import get_external_javascript, get_external_css -from util.names import parse_namespace_repository from util.secscan import PRIORITY_LEVELS from util.saas.useranalytics import build_error_callback from util.timedeltastring import convert_to_timedelta -from _init import STATIC_DIR, __version__ +from _init import __version__ logger = logging.getLogger(__name__) @@ -33,28 +27,6 @@ logger = logging.getLogger(__name__) route_data = None -def parse_repository_name(include_tag=False, - ns_kwarg_name='namespace_name', - repo_kwarg_name='repo_name', - tag_kwarg_name='tag_name', - incoming_repo_kwarg='repository'): - def inner(func): - @wraps(func) - def wrapper(*args, **kwargs): - 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] = repo_name_components[0] - kwargs[repo_kwarg_name] = repo_name_components[1] - if include_tag: - kwargs[tag_kwarg_name] = repo_name_components[2] - return func(*args, **kwargs) - return wrapper - return inner - - - def truthy_param(param): return param not in {False, 'false', 'False', '0', 'FALSE', '', 'null'} @@ -99,8 +71,8 @@ def common_login(db_user, permanent_session=True): return False -def list_files(path, extension): - import os +def _list_files(path, extension): + """ Returns a list of all the files with the given extension found under the given path. """ def matches(f): return os.path.splitext(f)[1] == '.' + extension and f.split(os.path.extsep)[1] != 'spec' @@ -118,7 +90,7 @@ def render_page_template(name, route_data=None, **kwargs): library_styles = [] main_styles = [] library_scripts = [] - main_scripts = list_files('build', 'js') + main_scripts = _list_files('build', 'js') use_cdn = app.config.get('USE_CDN', True) if request.args.get('use_cdn') is not None: diff --git a/endpoints/decorators.py b/endpoints/decorators.py index 6534bc4e9..6d8a9993c 100644 --- a/endpoints/decorators.py +++ b/endpoints/decorators.py @@ -4,8 +4,35 @@ from functools import wraps from flask import abort import features + +from app import app from auth.auth_context import ( get_validated_oauth_token, get_authenticated_user, get_validated_token, get_grant_context) +from util.names import parse_namespace_repository + + +def parse_repository_name(include_tag=False, + ns_kwarg_name='namespace_name', + repo_kwarg_name='repo_name', + tag_kwarg_name='tag_name', + incoming_repo_kwarg='repository'): + """ Decorator which parses the repository name found in the incoming_repo_kwarg argument, + and applies its pieces to the decorated function. + """ + def inner(func): + @wraps(func) + def wrapper(*args, **kwargs): + 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] = repo_name_components[0] + kwargs[repo_kwarg_name] = repo_name_components[1] + if include_tag: + kwargs[tag_kwarg_name] = repo_name_components[2] + return func(*args, **kwargs) + return wrapper + return inner def anon_allowed(func): diff --git a/endpoints/githubtrigger.py b/endpoints/githubtrigger.py index 3b373bdf0..3b4b21f0a 100644 --- a/endpoints/githubtrigger.py +++ b/endpoints/githubtrigger.py @@ -9,8 +9,7 @@ from app import app, github_trigger from auth.decorators import require_session_login from auth.permissions import AdministerRepositoryPermission from data import model -from endpoints.common import parse_repository_name -from endpoints.decorators import route_show_if +from endpoints.decorators import route_show_if, parse_repository_name from util.http import abort diff --git a/endpoints/v1/index.py b/endpoints/v1/index.py index b12c23dd5..877fb90c5 100644 --- a/endpoints/v1/index.py +++ b/endpoints/v1/index.py @@ -13,8 +13,7 @@ from auth.permissions import ( ModifyRepositoryPermission, UserAdminPermission, ReadRepositoryPermission, CreateRepositoryPermission, repository_read_grant, repository_write_grant) from auth.signedgrant import generate_signed_token -from endpoints.common import parse_repository_name -from endpoints.decorators import anon_protect, anon_allowed +from endpoints.decorators import anon_protect, anon_allowed, parse_repository_name from endpoints.notificationhelper import spawn_notification from endpoints.v1 import v1_bp from endpoints.v1.models_pre_oci import pre_oci_model as model diff --git a/endpoints/v1/tag.py b/endpoints/v1/tag.py index 4a741ff6b..acad5e303 100644 --- a/endpoints/v1/tag.py +++ b/endpoints/v1/tag.py @@ -6,8 +6,7 @@ from flask import abort, request, jsonify, make_response, session from auth.decorators import process_auth from auth.permissions import (ReadRepositoryPermission, ModifyRepositoryPermission) from data import model -from endpoints.common import parse_repository_name -from endpoints.decorators import anon_protect +from endpoints.decorators import anon_protect, parse_repository_name from endpoints.v1 import v1_bp from endpoints.v1.models_pre_oci import pre_oci_model as model from util.audit import track_and_log diff --git a/endpoints/v2/blob.py b/endpoints/v2/blob.py index f736eaccc..c7bf52c34 100644 --- a/endpoints/v2/blob.py +++ b/endpoints/v2/blob.py @@ -11,8 +11,7 @@ from app import storage, app, get_app_url, metric_queue from auth.registry_jwt_auth import process_registry_jwt_auth from data import database from digest import digest_tools -from endpoints.common import parse_repository_name -from endpoints.decorators import anon_protect +from endpoints.decorators import anon_protect, parse_repository_name from endpoints.v2 import v2_bp, require_repo_read, require_repo_write, get_input_stream from endpoints.v2.errors import ( BlobUnknown, BlobUploadInvalid, BlobUploadUnknown, Unsupported, NameUnknown, LayerTooLarge) diff --git a/endpoints/v2/manifest.py b/endpoints/v2/manifest.py index b79111f39..b4970e2f6 100644 --- a/endpoints/v2/manifest.py +++ b/endpoints/v2/manifest.py @@ -9,8 +9,7 @@ import features from app import docker_v2_signing_key, app, metric_queue from auth.registry_jwt_auth import process_registry_jwt_auth from digest import digest_tools -from endpoints.common import parse_repository_name -from endpoints.decorators import anon_protect +from endpoints.decorators import anon_protect, parse_repository_name from endpoints.notificationhelper import spawn_notification from endpoints.v2 import v2_bp, require_repo_read, require_repo_write from endpoints.v2.models_interface import Label diff --git a/endpoints/v2/tag.py b/endpoints/v2/tag.py index 776663520..558d33959 100644 --- a/endpoints/v2/tag.py +++ b/endpoints/v2/tag.py @@ -1,8 +1,7 @@ from flask import jsonify from auth.registry_jwt_auth import process_registry_jwt_auth -from endpoints.common import parse_repository_name -from endpoints.decorators import anon_protect +from endpoints.decorators import anon_protect, parse_repository_name from endpoints.v2 import v2_bp, require_repo_read, paginate from endpoints.v2.models_pre_oci import data_model as model diff --git a/endpoints/verbs/__init__.py b/endpoints/verbs/__init__.py index 0522c5a38..6ca94eb81 100644 --- a/endpoints/verbs/__init__.py +++ b/endpoints/verbs/__init__.py @@ -10,8 +10,7 @@ from auth.auth_context import get_authenticated_user from auth.decorators import process_auth from auth.permissions import ReadRepositoryPermission from data import database -from endpoints.common import parse_repository_name -from endpoints.decorators import anon_protect, route_show_if +from endpoints.decorators import anon_protect, route_show_if, parse_repository_name from endpoints.verbs.models_pre_oci import pre_oci_model as model from endpoints.v2.blob import BLOB_DIGEST_ROUTE from image.appc import AppCImageFormatter diff --git a/endpoints/web.py b/endpoints/web.py index db4a9d696..7f3bb79ca 100644 --- a/endpoints/web.py +++ b/endpoints/web.py @@ -27,10 +27,9 @@ from buildtrigger.triggerutil import TriggerProviderException from data import model from data.database import db from endpoints.api.discovery import swagger_route_data -from endpoints.common import (common_login, render_page_template, param_required, - parse_repository_name) +from endpoints.common import (common_login, render_page_template, param_required) from endpoints.csrf import csrf_protect, generate_csrf_token, verify_csrf -from endpoints.decorators import anon_protect, anon_allowed, route_show_if +from endpoints.decorators import anon_protect, anon_allowed, route_show_if, parse_repository_name from health.healthcheck import get_healthchecker from util.cache import no_cache from util.headers import parse_basic_auth From 5d69fc2aa31ac9d26a466e2b9b0c77a14c287510 Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Thu, 20 Jul 2017 11:41:19 -0400 Subject: [PATCH 5/9] Move param_required into the decorators module --- endpoints/common.py | 20 -------------------- endpoints/decorators.py | 18 +++++++++++++++++- endpoints/web.py | 5 +++-- 3 files changed, 20 insertions(+), 23 deletions(-) diff --git a/endpoints/common.py b/endpoints/common.py index 887e34394..96fe48069 100644 --- a/endpoints/common.py +++ b/endpoints/common.py @@ -24,24 +24,6 @@ from _init import __version__ logger = logging.getLogger(__name__) -route_data = None - - -def truthy_param(param): - return param not in {False, 'false', 'False', '0', 'FALSE', '', 'null'} - - -def param_required(param_name, allow_body=False): - def wrapper(wrapped): - @wraps(wrapped) - def decorated(*args, **kwargs): - if param_name not in request.args: - if not allow_body or param_name not in request.values: - abort(make_response('Required param: %s' % param_name, 400)) - return wrapped(*args, **kwargs) - return decorated - return wrapper - def common_login(db_user, permanent_session=True): if login_user(LoginWrappedDBUser(db_user.uuid, db_user)): @@ -85,8 +67,6 @@ def _list_files(path, extension): def render_page_template(name, route_data=None, **kwargs): - debugging = app.config.get('DEBUGGING', False) - library_styles = [] main_styles = [] library_scripts = [] diff --git a/endpoints/decorators.py b/endpoints/decorators.py index 6d8a9993c..8337da812 100644 --- a/endpoints/decorators.py +++ b/endpoints/decorators.py @@ -1,7 +1,7 @@ """ Various decorators for endpoint and API handlers. """ from functools import wraps -from flask import abort +from flask import abort, request, make_response import features @@ -35,6 +35,22 @@ def parse_repository_name(include_tag=False, return inner +def param_required(param_name, allow_body=False): + """ Marks a route as requiring a parameter with the given name to exist in the request's arguments + or (if allow_body=True) in its body values. If the parameter is not present, the request will + fail with a 400. + """ + def wrapper(wrapped): + @wraps(wrapped) + def decorated(*args, **kwargs): + if param_name not in request.args: + if not allow_body or param_name not in request.values: + abort(make_response('Required param: %s' % param_name, 400)) + return wrapped(*args, **kwargs) + return decorated + return wrapper + + def anon_allowed(func): """ Marks a method to allow anonymous access where it would otherwise be disallowed. """ func.__anon_allowed = True diff --git a/endpoints/web.py b/endpoints/web.py index 7f3bb79ca..da33b0fdb 100644 --- a/endpoints/web.py +++ b/endpoints/web.py @@ -27,9 +27,10 @@ from buildtrigger.triggerutil import TriggerProviderException from data import model from data.database import db from endpoints.api.discovery import swagger_route_data -from endpoints.common import (common_login, render_page_template, param_required) +from endpoints.common import common_login, render_page_template from endpoints.csrf import csrf_protect, generate_csrf_token, verify_csrf -from endpoints.decorators import anon_protect, anon_allowed, route_show_if, parse_repository_name +from endpoints.decorators import (anon_protect, anon_allowed, route_show_if, parse_repository_name, + param_required) from health.healthcheck import get_healthchecker from util.cache import no_cache from util.headers import parse_basic_auth From 41ac9019c6574d7ba0828fc6b222efd557e691f9 Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Thu, 20 Jul 2017 13:46:20 -0400 Subject: [PATCH 6/9] Small formatting improvements to common --- endpoints/common.py | 75 +++++++++++++++++++++++---------------------- 1 file changed, 38 insertions(+), 37 deletions(-) diff --git a/endpoints/common.py b/endpoints/common.py index 96fe48069..79a06fa2f 100644 --- a/endpoints/common.py +++ b/endpoints/common.py @@ -2,9 +2,7 @@ import logging import datetime import os -from functools import wraps - -from flask import make_response, render_template, request, abort, session +from flask import make_response, render_template, request, session from flask_login import login_user from flask_principal import identity_changed @@ -26,6 +24,7 @@ logger = logging.getLogger(__name__) def common_login(db_user, permanent_session=True): + """ Performs login of the given user, with optional non-permanence on the session. """ if login_user(LoginWrappedDBUser(db_user.uuid, db_user)): logger.debug('Successfully signed in as: %s (%s)' % (db_user.username, db_user.uuid)) new_identity = QuayDeferredPermissionUser.for_user(db_user) @@ -63,10 +62,11 @@ def _list_files(path, extension): return os.path.join(dp, f)[len('static/'):] filepath = os.path.join('static/', path) - return [join_path(dp, f) for dp, dn, files in os.walk(filepath) for f in files if matches(f)] + return [join_path(dp, f) for dp, _, files in os.walk(filepath) for f in files if matches(f)] def render_page_template(name, route_data=None, **kwargs): + """ Renders the page template with the given name as the response and returns its contents. """ library_styles = [] main_styles = [] library_scripts = [] @@ -110,39 +110,40 @@ def render_page_template(name, route_data=None, **kwargs): if not features.BILLING: version_number = 'Quay %s' % __version__ - resp = make_response(render_template(name, - route_data=route_data, - external_styles=external_styles, - external_scripts=external_scripts, - main_styles=main_styles, - library_styles=library_styles, - main_scripts=main_scripts, - library_scripts=library_scripts, - feature_set=features.get_features(), - config_set=frontend_visible_config(app.config), - oauth_set=get_oauth_config(), - external_login_set=get_external_login_config(), - scope_set=scopes.app_scopes(app.config), - vuln_priority_set=PRIORITY_LEVELS, - enterprise_logo=app.config.get('ENTERPRISE_LOGO_URL', ''), - mixpanel_key=app.config.get('MIXPANEL_KEY', ''), - munchkin_key=app.config.get('MARKETO_MUNCHKIN_ID', ''), - recaptcha_key=app.config.get('RECAPTCHA_SITE_KEY', ''), - google_tagmanager_key=app.config.get('GOOGLE_TAGMANAGER_KEY', ''), - google_anaytics_key=app.config.get('GOOGLE_ANALYTICS_KEY', ''), - sentry_public_dsn=app.config.get('SENTRY_PUBLIC_DSN', ''), - is_debug=str(app.config.get('DEBUGGING', False)).lower(), - show_chat=features.SUPPORT_CHAT, - aci_conversion=features.ACI_CONVERSION, - has_billing=features.BILLING, - contact_href=contact_href, - hostname=app.config['SERVER_HOSTNAME'], - preferred_scheme=app.config['PREFERRED_URL_SCHEME'], - version_number=version_number, - license_insufficient=license_validator.insufficient, - license_expiring=license_validator.expiring_soon, - current_year=datetime.datetime.now().year, - **kwargs)) + contents = render_template(name, + route_data=route_data, + external_styles=external_styles, + external_scripts=external_scripts, + main_styles=main_styles, + library_styles=library_styles, + main_scripts=main_scripts, + library_scripts=library_scripts, + feature_set=features.get_features(), + config_set=frontend_visible_config(app.config), + oauth_set=get_oauth_config(), + external_login_set=get_external_login_config(), + scope_set=scopes.app_scopes(app.config), + vuln_priority_set=PRIORITY_LEVELS, + enterprise_logo=app.config.get('ENTERPRISE_LOGO_URL', ''), + mixpanel_key=app.config.get('MIXPANEL_KEY', ''), + munchkin_key=app.config.get('MARKETO_MUNCHKIN_ID', ''), + recaptcha_key=app.config.get('RECAPTCHA_SITE_KEY', ''), + google_tagmanager_key=app.config.get('GOOGLE_TAGMANAGER_KEY', ''), + google_anaytics_key=app.config.get('GOOGLE_ANALYTICS_KEY', ''), + sentry_public_dsn=app.config.get('SENTRY_PUBLIC_DSN', ''), + is_debug=str(app.config.get('DEBUGGING', False)).lower(), + show_chat=features.SUPPORT_CHAT, + aci_conversion=features.ACI_CONVERSION, + has_billing=features.BILLING, + contact_href=contact_href, + hostname=app.config['SERVER_HOSTNAME'], + preferred_scheme=app.config['PREFERRED_URL_SCHEME'], + version_number=version_number, + license_insufficient=license_validator.insufficient, + license_expiring=license_validator.expiring_soon, + current_year=datetime.datetime.now().year, + **kwargs) + resp = make_response(contents) resp.headers['X-FRAME-OPTIONS'] = 'DENY' return resp From 7736de24fe659ea7eaeeff18ed84eccd200b2472 Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Thu, 20 Jul 2017 14:36:15 -0400 Subject: [PATCH 7/9] Add common_login test --- endpoints/common.py | 2 +- endpoints/test/test_common.py | 25 +++++++++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) create mode 100644 endpoints/test/test_common.py diff --git a/endpoints/common.py b/endpoints/common.py index 79a06fa2f..457faaa52 100644 --- a/endpoints/common.py +++ b/endpoints/common.py @@ -48,7 +48,7 @@ def common_login(db_user, permanent_session=True): return True else: - logger.debug('User could not be logged in, inactive?.') + logger.debug('User could not be logged in, inactive?') return False diff --git a/endpoints/test/test_common.py b/endpoints/test/test_common.py new file mode 100644 index 000000000..7ede7c62c --- /dev/null +++ b/endpoints/test/test_common.py @@ -0,0 +1,25 @@ +import pytest + +from data import model +from endpoints.common import common_login + +from test.fixtures import * + +@pytest.mark.parametrize('username, expect_success', [ + # Valid users. + ('devtable', True), + ('public', True), + + # Org. + ('buynlarge', False), + + # Robot. + ('devtable+dtrobot', False), + + # Unverified user. + ('unverified', False), +]) +def test_common_login(username, expect_success, app): + db_user = model.user.get_namespace_user(username) + with app.app_context(): + assert common_login(db_user) == expect_success From aecec02b6ce2a3532625c97b39576d3638f8843b Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Thu, 20 Jul 2017 15:40:14 -0400 Subject: [PATCH 8/9] Change common_login to take in a user uuid, instead of a user DB object --- data/model/user.py | 2 +- endpoints/api/suconfig.py | 2 +- endpoints/api/user.py | 8 ++++---- endpoints/common.py | 31 +++++++++++++++---------------- endpoints/oauth/login.py | 2 +- endpoints/test/test_common.py | 2 +- endpoints/web.py | 4 ++-- 7 files changed, 25 insertions(+), 26 deletions(-) diff --git a/data/model/user.py b/data/model/user.py index a1fb0d81e..b2c475b78 100644 --- a/data/model/user.py +++ b/data/model/user.py @@ -882,7 +882,7 @@ class LoginWrappedDBUser(UserMixin): @property def is_active(self): - return self.db_user().verified + return self.db_user() and self.db_user().verified def get_id(self): return unicode(self._uuid) diff --git a/endpoints/api/suconfig.py b/endpoints/api/suconfig.py index db5050489..4b7bee5d3 100644 --- a/endpoints/api/suconfig.py +++ b/endpoints/api/suconfig.py @@ -401,7 +401,7 @@ class SuperUserCreateInitialSuperUser(ApiResource): superusers.register_superuser(username) # Conduct login with that user. - common_login(superuser) + common_login(superuser.uuid) return { 'status': True diff --git a/endpoints/api/user.py b/endpoints/api/user.py index f359c16d5..14a180b50 100644 --- a/endpoints/api/user.py +++ b/endpoints/api/user.py @@ -317,7 +317,7 @@ class User(ApiResource): model.user.change_password(user, user_data['password']) # Login again to reset their session cookie. - common_login(user) + common_login(user.uuid) if features.MAILING: send_password_changed(user.username, user.email) @@ -436,7 +436,7 @@ class User(ApiResource): 'awaiting_verification': True } else: - common_login(new_user) + common_login(new_user.uuid) return user_view(new_user) except model.user.DataModelException as ex: raise request_error(exception=ex) @@ -528,7 +528,7 @@ def conduct_signin(username_or_email, password, invite_code=None): if invite_code: handle_invite_code(invite_code, found_user) - if common_login(found_user): + if common_login(found_user.uuid): return {'success': True} else: needs_email_verification = True @@ -688,7 +688,7 @@ class VerifyUser(ApiResource): 'invalidCredentials': True, }, 403 - common_login(result) + common_login(result.uuid) return {'success': True} diff --git a/endpoints/common.py b/endpoints/common.py index 457faaa52..dbe7fabc4 100644 --- a/endpoints/common.py +++ b/endpoints/common.py @@ -23,11 +23,11 @@ from _init import __version__ logger = logging.getLogger(__name__) -def common_login(db_user, permanent_session=True): +def common_login(user_uuid, permanent_session=True): """ Performs login of the given user, with optional non-permanence on the session. """ - if login_user(LoginWrappedDBUser(db_user.uuid, db_user)): - logger.debug('Successfully signed in as: %s (%s)' % (db_user.username, db_user.uuid)) - new_identity = QuayDeferredPermissionUser.for_user(db_user) + if login_user(LoginWrappedDBUser(user_uuid)): + logger.debug('Successfully signed in as user with uuid %s', user_uuid) + new_identity = QuayDeferredPermissionUser.for_id(user_uuid) identity_changed.send(app, identity=new_identity) session['login_time'] = datetime.datetime.now() @@ -37,19 +37,18 @@ def common_login(db_user, permanent_session=True): session.permanent_session_lifetime = convert_to_timedelta(session_timeout_str) # Inform our user analytics that we have a new "lead" - create_lead_future = user_analytics.create_lead( - db_user.email, - db_user.username, - db_user.given_name, - db_user.family_name, - db_user.company, - ) - create_lead_future.add_done_callback(build_error_callback('Create lead failed')) - + #create_lead_future = user_analytics.create_lead( + # db_user.email, + # db_user.username, + # db_user.given_name, + # db_user.family_name, + # db_user.company, + #) + #create_lead_future.add_done_callback(build_error_callback('Create lead failed')) return True - else: - logger.debug('User could not be logged in, inactive?') - return False + + logger.debug('User could not be logged in, inactive?') + return False def _list_files(path, extension): diff --git a/endpoints/oauth/login.py b/endpoints/oauth/login.py index 333f4e99d..3c21fde29 100644 --- a/endpoints/oauth/login.py +++ b/endpoints/oauth/login.py @@ -139,7 +139,7 @@ def _render_ologin_error(service_name, error_message=None, register_redirect=Fal def _perform_login(user_obj, service_name): """ Attempts to login the given user, returning the Flask result of whether the login succeeded. """ - if common_login(user_obj): + if common_login(user_obj.uuid): if model.user.has_user_prompts(user_obj): return redirect(url_for('web.updateuser')) else: diff --git a/endpoints/test/test_common.py b/endpoints/test/test_common.py index 7ede7c62c..39cc0c569 100644 --- a/endpoints/test/test_common.py +++ b/endpoints/test/test_common.py @@ -22,4 +22,4 @@ from test.fixtures import * def test_common_login(username, expect_success, app): db_user = model.user.get_namespace_user(username) with app.app_context(): - assert common_login(db_user) == expect_success + assert common_login(db_user.uuid) == expect_success diff --git a/endpoints/web.py b/endpoints/web.py index da33b0fdb..561c3efd4 100644 --- a/endpoints/web.py +++ b/endpoints/web.py @@ -464,7 +464,7 @@ def confirm_email(): change_email_future = user_analytics.change_email(old_email, new_email) change_email_future.add_done_callback(build_error_callback('Change email failed')) - common_login(user) + common_login(user.uuid) if model.user.has_user_prompts(user): return redirect(url_for('web.updateuser')) elif new_email: @@ -481,7 +481,7 @@ def confirm_recovery(): user = model.user.validate_reset_code(code) if user is not None: - common_login(user) + common_login(user.uuid) return redirect(url_for('web.user_view', path=user.username, tab='settings', action='password')) else: message = 'Invalid recovery code: This code is invalid or may have already been used.' From f976ffbdc7336accdce06f2da4a08b80c64efd20 Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Thu, 20 Jul 2017 15:57:42 -0400 Subject: [PATCH 9/9] Change endpoints/common to use a data interface --- endpoints/common.py | 23 +++++++++++++--------- endpoints/common_models_interface.py | 29 ++++++++++++++++++++++++++++ endpoints/common_models_pre_oci.py | 21 ++++++++++++++++++++ endpoints/test/test_common.py | 6 +++--- 4 files changed, 67 insertions(+), 12 deletions(-) create mode 100644 endpoints/common_models_interface.py create mode 100644 endpoints/common_models_pre_oci.py diff --git a/endpoints/common.py b/endpoints/common.py index dbe7fabc4..26b45848f 100644 --- a/endpoints/common.py +++ b/endpoints/common.py @@ -14,6 +14,7 @@ from auth import scopes from auth.permissions import QuayDeferredPermissionUser from config import frontend_visible_config from external_libraries import get_external_javascript, get_external_css +from endpoints.common_models_pre_oci import pre_oci_model as model from util.secscan import PRIORITY_LEVELS from util.saas.useranalytics import build_error_callback from util.timedeltastring import convert_to_timedelta @@ -25,8 +26,12 @@ logger = logging.getLogger(__name__) def common_login(user_uuid, permanent_session=True): """ Performs login of the given user, with optional non-permanence on the session. """ + user = model.get_user(user_uuid) + if user is None: + return False + if login_user(LoginWrappedDBUser(user_uuid)): - logger.debug('Successfully signed in as user with uuid %s', user_uuid) + logger.debug('Successfully signed in as user %s with uuid %s', user.username, user_uuid) new_identity = QuayDeferredPermissionUser.for_id(user_uuid) identity_changed.send(app, identity=new_identity) session['login_time'] = datetime.datetime.now() @@ -37,14 +42,14 @@ def common_login(user_uuid, permanent_session=True): session.permanent_session_lifetime = convert_to_timedelta(session_timeout_str) # Inform our user analytics that we have a new "lead" - #create_lead_future = user_analytics.create_lead( - # db_user.email, - # db_user.username, - # db_user.given_name, - # db_user.family_name, - # db_user.company, - #) - #create_lead_future.add_done_callback(build_error_callback('Create lead failed')) + create_lead_future = user_analytics.create_lead( + user.email, + user.username, + user.given_name, + user.family_name, + user.company, + ) + create_lead_future.add_done_callback(build_error_callback('Create lead failed')) return True logger.debug('User could not be logged in, inactive?') diff --git a/endpoints/common_models_interface.py b/endpoints/common_models_interface.py new file mode 100644 index 000000000..4a9978eab --- /dev/null +++ b/endpoints/common_models_interface.py @@ -0,0 +1,29 @@ +from abc import ABCMeta, abstractmethod +from collections import namedtuple + +from six import add_metaclass + + +class User(namedtuple('User', ['uuid', 'username', 'email', 'given_name', 'family_name', 'company'])): + """ + User represents a user. + """ + + +@add_metaclass(ABCMeta) +class EndpointsCommonDataInterface(object): + """ + Interface that represents all data store interactions required by the common endpoints lib. + """ + + @abstractmethod + def get_user(self, user_uuid): + """ + Returns the User matching the given uuid, if any or None if none. + """ + + @abstractmethod + def get_namespace_uuid(self, namespace_name): + """ + Returns the uuid of the Namespace with the given name, if any. + """ diff --git a/endpoints/common_models_pre_oci.py b/endpoints/common_models_pre_oci.py new file mode 100644 index 000000000..833579d90 --- /dev/null +++ b/endpoints/common_models_pre_oci.py @@ -0,0 +1,21 @@ +from data import model +from endpoints.common_models_interface import User, EndpointsCommonDataInterface + + +class EndpointsCommonDataPreOCIModel(EndpointsCommonDataInterface): + def get_user(self, user_uuid): + user = model.user.get_user_by_uuid(user_uuid) + if user is None: + return None + + return User(uuid=user.uuid, username=user.username, email=user.email, + given_name=user.given_name, family_name=user.family_name, company=user.company) + + def get_namespace_uuid(self, namespace_name): + user = model.user.get_namespace_user(namespace_name) + if user is None: + return None + + return user.uuid + +pre_oci_model = EndpointsCommonDataPreOCIModel() diff --git a/endpoints/test/test_common.py b/endpoints/test/test_common.py index 39cc0c569..f13ec6e02 100644 --- a/endpoints/test/test_common.py +++ b/endpoints/test/test_common.py @@ -1,9 +1,9 @@ import pytest -from data import model from endpoints.common import common_login from test.fixtures import * +from endpoints.common_models_pre_oci import pre_oci_model as model @pytest.mark.parametrize('username, expect_success', [ # Valid users. @@ -20,6 +20,6 @@ from test.fixtures import * ('unverified', False), ]) def test_common_login(username, expect_success, app): - db_user = model.user.get_namespace_user(username) + uuid = model.get_namespace_uuid(username) with app.app_context(): - assert common_login(db_user.uuid) == expect_success + assert common_login(uuid) == expect_success