fix error when handling owners

quetz/authorization.py

@@ -39,19 +39,26 @@ def __init__(self, API_key: Optional[str], session: dict, db: Session):
         self.session = session
         self.db = db
 
+    def get_valid_api_key(self) -> Optional[ApiKey]:
+        if not self.API_key:
+            return None
+        return (self.db.query(ApiKey)
+                .filter(ApiKey.key == self.API_key, ~ApiKey.deleted)
+                .filter(
+            ApiKey.key == self.API_key,
+            or_(ApiKey.expire_at >= date.today(), ApiKey.expire_at.is_(None)),
+        ).one_or_none())
+
     def get_owner(self) -> Optional[bytes]:
+        """
+        gets the id of the owner of the authenticated session, if any. Either:
+          a) the owner of the API key that is used for authentication
+          b) the user_id of the encrypted session cookie
+        """
         owner_id = None
 
         if self.API_key:
-            api_key = (
-                self.db.query(ApiKey)
-                .filter(ApiKey.key == self.API_key, ~ApiKey.deleted)
-                .filter(
-                    ApiKey.key == self.API_key,
-                    or_(ApiKey.expire_at >= date.today(), ApiKey.expire_at.is_(None)),
-                )
-                .one_or_none()
-            )
+            api_key = self.get_valid_api_key()
             if api_key:
                 owner_id = api_key.owner_id
         else:
@@ -61,19 +68,27 @@ def get_owner(self) -> Optional[bytes]:
 
         return owner_id
 
+    def assert_owner(self) -> bytes:
+        owner_id = self.get_owner()
+
+        if not owner_id or not self.db.query(User).filter(User.id == owner_id).count():
+            raise HTTPException(
+                status_code=status.HTTP_401_UNAUTHORIZED,
+                detail="Not logged in",
+            )
+
+        return owner_id
+
     def get_user(self) -> Optional[bytes]:
+        """
+        gets the id of the user of the authenticated session, if any. Either:
+          a) the user_id of the API key (not its owner!) that is used for authentication
+          b) the user_id of the encrypted session cookie
+        """
         user_id = None
 
         if self.API_key:
-            api_key = (
-                self.db.query(ApiKey)
-                .filter(ApiKey.key == self.API_key, ~ApiKey.deleted)
-                .filter(
-                    ApiKey.key == self.API_key,
-                    or_(ApiKey.expire_at >= date.today(), ApiKey.expire_at.is_(None)),
-                )
-                .one_or_none()
-            )
+            api_key = self.get_valid_api_key()
             if api_key:
                 user_id = api_key.user_id
         else:

quetz/main.py

@@ -912,8 +912,10 @@ def post_package(
     auth: authorization.Rules = Depends(get_rules),
     dao: Dao = Depends(get_dao),
 ):
-    user_id = auth.assert_user()
-    owner_id = auth.get_owner()
+    # here we use the owner_id as user_id. In case the authentication
+    # was done using an API Key, we want to attribute the uploaded package
+    # to the owner of that API Key and not the anonymous API Key itself.
+    user_id = auth.assert_owner()
 
     auth.assert_create_package(channel.name)
     pm.hook.validate_new_package(
@@ -929,7 +931,7 @@ def post_package(
             detail=f"Package {channel.name}/{new_package.name} exists",
         )
 
-    dao.create_package(channel.name, new_package, owner_id, authorization.OWNER)
+    dao.create_package(channel.name, new_package, user_id, authorization.OWNER)
 
 
 @api_router.get(
@@ -1384,7 +1386,10 @@ async def post_upload(
             status_code=status.HTTP_406_NOT_ACCEPTABLE, detail="Wrong SHA256 checksum"
         )
 
-    _ = auth.assert_user()
+    # here we use the owner_id as user_id. In case the authentication
+    # was done using an API Key, we want to attribute the uploaded package
+    # to the owner of that API Key and not the anonymous API Key itself.
+    user_id = auth.assert_owner()
 
     auth.assert_create_package(channel_name)
     condainfo = CondaInfo((body), filename)
@@ -1393,9 +1398,6 @@ async def post_upload(
     body.seek(0)
     await pkgstore.add_package_async(body, channel_name, dest)
 
-    # get the id of the owner, in case auth was done through an API key
-    owner_id = auth.get_owner()
-
     package_name = str(condainfo.info.get("name"))
     package_data = rest_models.Package(
         name=package_name,
@@ -1406,7 +1408,7 @@ async def post_upload(
         dao.create_package(
             channel_name,
             package_data,
-            owner_id,
+            user_id,
             authorization.OWNER,
         )
 
@@ -1425,7 +1427,7 @@ async def post_upload(
             size=condainfo.info["size"],
             filename=filename,
             info=json.dumps(condainfo.info),
-            uploader_id=owner_id,
+            uploader_id=user_id,
             upsert=force,
         )
     except IntegrityError:
@@ -1510,8 +1512,10 @@ def handle_package_files(
     package=None,
     is_mirror_op=False,
 ):
-    _ = auth.assert_user()
-    owner_id = auth.get_owner()
+    # here we use the owner_id as user_id. In case the authentication
+    # was done using an API Key, we want to attribute the uploaded package
+    # to the owner of that API Key and not the anonymous API Key itself.
+    user_id = auth.assert_owner()
 
     # quick fail if not allowed to upload
     # note: we're checking later that `parts[0] == conda_info.package_name`
@@ -1655,7 +1659,7 @@ def _delete_file(condainfo, filename):
             dao.create_package(
                 channel.name,
                 package_data,
-                owner_id,
+                user_id,
                 authorization.OWNER,
             )
 
@@ -1676,7 +1680,7 @@ def _delete_file(condainfo, filename):
                 size=condainfo.info["size"],
                 filename=file.filename,
                 info=json.dumps(condainfo.info),
-                uploader_id=owner_id,
+                uploader_id=user_id,
                 upsert=force,
             )
         except IntegrityError:

quetz/tests/api/test_main_packages.py

@@ -7,11 +7,13 @@
 import pytest
 
 from quetz import hookimpl
-from quetz.authorization import MAINTAINER, MEMBER, OWNER
+from quetz.authorization import MAINTAINER, MEMBER, OWNER, SERVER_OWNER
 from quetz.condainfo import CondaInfo
 from quetz.config import Config
+from quetz.dao import Dao
 from quetz.db_models import ChannelMember, Package, PackageMember, PackageVersion
 from quetz.errors import ValidationError
+from quetz.rest_models import BaseApiKey, Channel
 from quetz.tasks.indexing import update_indexes
 
 
@@ -732,3 +734,121 @@ def test_package_channel_data_attributes(
     content = response.json()
     assert content['platforms'] == ['linux-64']
     assert content['url'].startswith("https://")
+
+
+@pytest.fixture
+def owner(db, dao: Dao):
+    # create an owner with OWNER role
+    owner = dao.create_user_with_role("owner", role=SERVER_OWNER)
+
+    yield owner
+
+    db.delete(owner)
+    db.commit()
+
+@pytest.fixture
+def private_channel(db, dao:Dao, owner):
+    # create a channel
+    channel_data = Channel(name='private-channel', private=True)
+    channel = dao.create_channel(channel_data, owner.id, "owner")
+
+    yield channel
+
+    db.delete(channel)
+    db.commit()
+
+
+@pytest.fixture
+def private_package(db, owner, private_channel, dao):
+    package_data = Package(name='test-package')
+
+    package = dao.create_package(
+        private_channel.name, package_data, owner.id, "owner"
+    )
+
+    yield package
+
+    db.delete(package)
+    db.commit()
+
+@pytest.fixture
+def api_key(db, dao: Dao, owner, private_channel):
+    # create an api key with restriction
+    key = dao.create_api_key(
+            owner.id,
+            BaseApiKey.parse_obj(
+                dict(description="test api key", expire_at="2099-12-31", roles=[{'role': 'maintainer', 'package': None, 'channel': private_channel.name}])
+            ),
+            "API key with role restruction",
+        )
+
+    yield key
+
+    # delete API Key
+    key.deleted = True
+    db.commit()
+
+
+def test_upload_package_with_api_key(client, dao: Dao, owner, private_channel, api_key):
+    # set api key of the anonymous user 'owner_key' to headers
+    client.headers['X-API-Key'] = api_key.key
+
+    # post new package
+    response = client.post(
+        f"/api/channels/{private_channel.name}/packages",
+        json={"name": "test-package", "summary": "none", "description": "none"},
+    )
+    assert response.status_code == 201
+
+    # we used the anonymous user of the API key for upload, but expect the owner to be the package owner
+    member = dao.get_package_member(private_channel.name, 'test-package', username=owner.username)
+    assert member
+
+
+def test_upload_file_to_package_with_api_key(dao: Dao, client, owner, private_channel, private_package, api_key):
+    # set api key of the anonymous user 'owner_key' to headers
+    client.headers['X-API-Key'] = api_key.key
+
+    package_filename = "test-package-0.1-0.tar.bz2"
+    with open(package_filename, "rb") as fid:
+        files = {"files": (package_filename, fid)}
+        response = client.post(
+            f"/api/channels/{private_channel.name}/packages/"
+            f"{private_package.name}/files/",
+            files=files,
+        )
+        assert response.status_code == 201
+
+    # we used the anonymous user of the API key for upload, but expect the owner to be the package owner
+    version = dao.get_package_version_by_filename(
+        channel_name=private_channel.name,
+        package_name=private_package.name,
+        filename=package_filename,
+        platform='linux-64'
+    )
+    assert version
+    assert version.uploader == owner
+
+
+def test_upload_file_to_channel_with_api_key(dao: Dao, client, owner, private_channel, api_key):
+    # set api key of the anonymous user 'owner_key' to headers
+    client.headers['X-API-Key'] = api_key.key
+
+    package_filename = "test-package-0.1-0.tar.bz2"
+    with open(package_filename, "rb") as fid:
+        files = {"files": (package_filename, fid)}
+        response = client.post(
+            f"/api/channels/{private_channel.name}/files/",
+            files=files,
+        )
+        assert response.status_code == 201
+
+    # we used the anonymous user of the API key for upload, but expect the owner to be the package owner
+    version = dao.get_package_version_by_filename(
+        channel_name=private_channel.name,
+        package_name='test-package',
+        filename=package_filename,
+        platform='linux-64'
+    )
+    assert version
+    assert version.uploader == owner