Merge pull request #2810 from coreos-inc/joseph.schorr/QUAY-747/common-data-interface

Convert endpoints/common into using a data interface

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)

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

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}
 
 

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

endpoints/common.py

@@ -1,15 +1,8 @@
 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 import make_response, render_template, request, session
 from flask_login import login_user
 from flask_principal import identity_changed
 
@@ -21,83 +14,25 @@ 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 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
-from _init import STATIC_DIR, __version__
+from _init import __version__
 
 
 logger = logging.getLogger(__name__)
 
-route_data = None
 
+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
 
-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 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'}
-
-
-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)):
-    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 %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()
 
@@ -108,25 +43,21 @@ def common_login(db_user, permanent_session=True):
 
     # 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,
+      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
-  else:
-    logger.debug('User could not be logged in, inactive?.')
+
+  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
+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'
 
@@ -135,16 +66,15 @@ 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):
-  debugging = app.config.get('DEBUGGING', False)
-
+  """ Renders the page template with the given name as the response and returns its contents. """
   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:
@@ -184,7 +114,7 @@ def render_page_template(name, route_data=None, **kwargs):
   if not features.BILLING:
     version_number = 'Quay %s' % __version__
 
-  resp = make_response(render_template(name,
+  contents = render_template(name,
                              route_data=route_data,
                              external_styles=external_styles,
                              external_scripts=external_scripts,
@@ -216,7 +146,8 @@ def render_page_template(name, route_data=None, **kwargs):
                              license_insufficient=license_validator.insufficient,
                              license_expiring=license_validator.expiring_soon,
                              current_year=datetime.datetime.now().year,
-                                        **kwargs))
+                             **kwargs)
 
+  resp = make_response(contents)
   resp.headers['X-FRAME-OPTIONS'] = 'DENY'
   return resp

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.
+    """

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()

endpoints/decorators.py

@@ -1,14 +1,54 @@
 """ Various decorators for endpoint and API handlers. """
 
 from functools import wraps
-
-from flask import abort
+from flask import abort, request, make_response
 
 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 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 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):
@@ -25,6 +65,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,4 +78,19 @@ 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):
+      if not value:
+        abort(404)
+
+      return f(*args, **kwargs)
+    return decorated_function
+  return decorator

endpoints/githubtrigger.py

@@ -9,7 +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 route_show_if, parse_repository_name
+from endpoints.decorators import route_show_if, parse_repository_name
 from util.http import abort
 
 

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
 
 

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:

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__)

endpoints/test/test_common.py

@@ -0,0 +1,25 @@
+import pytest
+
+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.
+  ('devtable', True),
+  ('public', True),
+
+  # Org.
+  ('buynlarge', False),
+
+  # Robot.
+  ('devtable+dtrobot', False),
+
+  # Unverified user.
+  ('unverified', False),
+])
+def test_common_login(username, expect_success, app):
+  uuid = model.get_namespace_uuid(username)
+  with app.app_context():
+    assert common_login(uuid) == expect_success

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

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

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()

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)

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

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
 

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 route_show_if, parse_repository_name
-from endpoints.decorators import anon_protect
+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

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,
-                              parse_repository_name)
+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
+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
@@ -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.'