Fix crash when uploading a package through scoped API key (mamba-org#647

)

* add get_owner and use for package upload

* fix error when handling owners

quetz/authorization.py

@@ -39,19 +39,59 @@ 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.get_valid_api_key()
+            if api_key:
+                owner_id = api_key.owner_id
+        else:
+            user_id = self.session.get("user_id")
+            if user_id:
+                owner_id = uuid.UUID(user_id).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,7 +912,11 @@ def post_package(
     auth: authorization.Rules = Depends(get_rules),
     dao: Dao = Depends(get_dao),
 ):
-    user_id = 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)
     pm.hook.validate_new_package(
         channel_name=channel.name,
@@ -1382,7 +1386,11 @@ async def post_upload(
             status_code=status.HTTP_406_NOT_ACCEPTABLE, detail="Wrong SHA256 checksum"
         )
 
-    user_id = 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)
     dest = os.path.join(condainfo.info["subdir"], filename)
@@ -1504,7 +1512,10 @@ def handle_package_files(
     package=None,
     is_mirror_op=False,
 ):
-    user_id = 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()
 
     # quick fail if not allowed to upload
     # note: we're checking later that `parts[0] == conda_info.package_name`

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,140 @@ 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