Fix login with robot to quay-appr

data/interfaces/appr.py

@@ -8,10 +8,10 @@ from cnr.exception import raise_package_not_found, raise_channel_not_found
 from six import add_metaclass
 
 
-from app import storage
+from app import storage, authentication
 from data import model, oci_model
 from data.database import Tag, Manifest, MediaType, Blob, Repository, Channel
-
+from util.names import parse_robot_username
 
 class BlobDescriptor(namedtuple('Blob', ['mediaType', 'size', 'digest', 'urls'])):
   """ BlobDescriptor describes a blob with its mediatype, size and digest.
@@ -427,6 +427,16 @@ class OCIAppModel(AppRegistryDataInterface):
     return  ChannelView(current=channel.linked_tag.name,
                         name=channel.name)
 
+  def get_user(self, username, password):
+    err_msg = None
+    if parse_robot_username(username) is not None:
+      try:
+        user = model.user.verify_robot(username, password)
+      except model.InvalidRobotException as exc:
+        return (None, exc.message)
+    else:
+      user, err_msg = authentication.verify_and_link_user(username, password)
+    return (user, err_msg)
 
 def _strip_sha256_header(digest):
   if digest.startswith('sha256:'):

endpoints/appr/cnr_backend.py

@@ -8,7 +8,7 @@ from cnr.models.package_base import PackageBase, manifest_media_type
 
 from app import storage
 from data.interfaces.appr import oci_app_model
-from data.oci_model import blob # TODO these calls should be through oci_app_model
+from data.oci_model import blob  # TODO these calls should be through oci_app_model
 
 
 class Blob(BlobBase):
@@ -83,6 +83,15 @@ class Channel(ChannelBase):
     oci_app_model.delete_channel(self.package, self.name)
 
 
+class User(object):
+  """ User in CNR models """
+
+  @classmethod
+  def get_user(cls, username, password):
+    """ Returns True if user creds is valid """
+    return oci_app_model.get_user(username, password)
+
+
 class Package(PackageBase):
   """ CNR Package model implemented against the Quay data model. """
 

endpoints/appr/registry.py

@@ -1,23 +1,21 @@
 import logging
-
 from base64 import b64encode
 
 import cnr
-
 from cnr.api.impl import registry as cnr_registry
-from cnr.api.registry import repo_name, _pull
-from cnr.exception import (CnrException, InvalidUsage, InvalidParams, InvalidRelease,
-                           UnableToLockResource, UnauthorizedAccess, Unsupported, ChannelNotFound, Forbidden,
-                           PackageAlreadyExists, PackageNotFound, PackageReleaseNotFound)
-from flask import request, jsonify
+from cnr.api.registry import _pull, repo_name
+from cnr.exception import (
+  ChannelNotFound, CnrException, Forbidden, InvalidParams, InvalidRelease, InvalidUsage,
+  PackageAlreadyExists, PackageNotFound, PackageReleaseNotFound, UnableToLockResource,
+  UnauthorizedAccess, Unsupported)
+from flask import jsonify, request
 
-from app import authentication
 from auth.auth_context import get_authenticated_user
 from auth.decorators import process_auth
-from auth.permissions import CreateRepositoryPermission, ModifyRepositoryPermission
-from endpoints.appr import appr_bp, require_app_repo_read, require_app_repo_write
+from auth.permissions import (CreateRepositoryPermission, ModifyRepositoryPermission)
+from endpoints.appr import (appr_bp, require_app_repo_read, require_app_repo_write)
+from endpoints.appr.cnr_backend import Blob, Channel, Package, User
 from endpoints.appr.decorators import disallow_for_image_repository
-from endpoints.appr.cnr_backend import Package, Channel, Blob
 from endpoints.decorators import anon_allowed, anon_protect
 from util.names import REPOSITORY_NAME_REGEX, TAG_REGEX
 
@@ -58,7 +56,7 @@ def login():
   if not username or not password:
     raise InvalidUsage('Missing username or password')
 
-  user, err = authentication.verify_credentials(username, password)
+  user, err = User.get_user(username, password)
   if err is not None:
     raise UnauthorizedAccess(err)
 
@@ -185,7 +183,7 @@ def push(namespace, package_name):
 
   if not REPOSITORY_NAME_REGEX.match(package_name):
     logger.debug('Found invalid repository name CNR push: %s', reponame)
-    raise InvalidUsage()
+    raise InvalidUsage('invalid repository name: %s' % reponame)
 
   values = request.get_json(force=True, silent=True) or {}
   private = values.get('visibility', 'private')
@@ -258,19 +256,24 @@ def show_channel(namespace, package_name, channel_name):
 @require_app_repo_write
 @anon_protect
 def add_channel_release(namespace, package_name, channel_name, release):
-  if not TAG_REGEX.match(channel_name):
-    logger.debug('Found invalid channel name CNR add channel release: %s', channel_name)
-    raise InvalidUsage()
-
-  if not TAG_REGEX.match(release):
-    logger.debug('Found invalid release name CNR add channel release: %s', release)
-    raise InvalidUsage()
-
+  _check_channel_name(channel_name, release)
   reponame = repo_name(namespace, package_name)
   result = cnr_registry.add_channel_release(reponame, channel_name, release, channel_class=Channel,
                                             package_class=Package)
   return jsonify(result)
 
+def _check_channel_name(channel_name, release=None):
+  if not TAG_REGEX.match(channel_name):
+    logger.debug('Found invalid channel name CNR add channel release: %s', channel_name)
+    raise InvalidUsage("Found invalid channelname %s" % release,
+                       {'name': channel_name,
+                        "release": release})
+
+  if release is not None and not TAG_REGEX.match(release):
+    logger.debug('Found invalid release name CNR add channel release: %s', release)
+    raise InvalidUsage("Found invalid channel release name %s" % release,
+                       {'name': channel_name,
+                        "release": release})
 
 @appr_bp.route(
   "/api/v1/packages/<string:namespace>/<string:package_name>/channels/<string:channel_name>/<string:release>",
@@ -281,14 +284,7 @@ def add_channel_release(namespace, package_name, channel_name, release):
 @require_app_repo_write
 @anon_protect
 def delete_channel_release(namespace, package_name, channel_name, release):
-  if not TAG_REGEX.match(channel_name):
-    logger.debug('Found invalid channel name CNR delete channel release: %s', channel_name)
-    raise InvalidUsage()
-
-  if not TAG_REGEX.match(release):
-    logger.debug('Found invalid release name CNR delete channel release: %s', release)
-    raise InvalidUsage()
-
+  _check_channel_name(channel_name, release)
   reponame = repo_name(namespace, package_name)
   result = cnr_registry.delete_channel_release(reponame, channel_name, release,
                                                channel_class=Channel, package_class=Package)
@@ -304,10 +300,7 @@ def delete_channel_release(namespace, package_name, channel_name, release):
 @require_app_repo_write
 @anon_protect
 def delete_channel(namespace, package_name, channel_name):
-  if not TAG_REGEX.match(channel_name):
-    logger.debug('Found invalid channel name CNR delete channel: %s', channel_name)
-    raise InvalidUsage()
-
+  _check_channel_name(channel_name)
   reponame = repo_name(namespace, package_name)
   result = cnr_registry.delete_channel(reponame, channel_name, channel_class=Channel)
   return jsonify(result)

endpoints/appr/test/test_api.py

@@ -215,7 +215,7 @@ class TestQuayModels(CnrTestModels):
     b2db = oci_blob.get_blob(p2.digest)
     assert b2db.id == bdb.id
 
-  def test_push_same_blob(self, db_with_data1):
+  def test_force_push_different_blob(self, db_with_data1):
     p = db_with_data1.Package.get("titi/rocketchat", "2.0.1", 'kpm')
     assert p.package == "titi/rocketchat"
     assert p.release == "2.0.1"

endpoints/appr/test/test_registry.py

@@ -1,29 +1,28 @@
 import json
 
 from flask import url_for
+import pytest
 
 from data import model
 from endpoints.appr.registry import appr_bp
 from test.fixtures import app, appconfig, database_uri, init_db_path, sqlitedb_file
 
-def test_invalid_login(app, client):
-  app.register_blueprint(appr_bp, url_prefix='/cnr')
 
+@pytest.mark.parametrize('login_data, expected_code', [
+    ({"username": "devtable", "password": "password"}, 200),
+    ({"username": "devtable", "password": "badpass"}, 401),
+    ({"username": "devtable+dtrobot", "password": "badpass"}, 401),
+    ({"username": "devtable+dtrobot2", "password": None}, 200),
+  ])
+def test_login(login_data, expected_code, app, client):
+  if "+" in login_data['username'] and login_data['password'] is None:
+    username, robotname = login_data['username'].split("+")
+    _, login_data['password'] = model.user.create_robot(robotname, model.user.get_user(username))
+  app.register_blueprint(appr_bp, url_prefix='/cnr')
   url = url_for('appr.login')
   headers = {'Content-Type': 'application/json'}
-  data = {'user': {'username': 'foo', 'password': 'bar'}}
+  data = {'user': login_data}
 
   rv = client.open(url, method='POST', data=json.dumps(data), headers=headers)
-  assert rv.status_code == 401
-
-
-def test_valid_login(app, client):
-  app.register_blueprint(appr_bp, url_prefix='/cnr')
-
-  url = url_for('appr.login')
-  headers = {'Content-Type': 'application/json'}
-  data = {'user': {'username': 'devtable', 'password': 'password'}}
-
-  rv = client.open(url, method='POST', data=json.dumps(data), headers=headers)
-  assert rv.status_code == 200
+  assert rv.status_code == expected_code