From 069e0ed751403899bf5b292577534162bf4eb531 Mon Sep 17 00:00:00 2001 From: Matthias Gabriel Date: Thu, 22 Jun 2023 15:39:53 +0200 Subject: [PATCH] fix error when handling owners --- quetz/authorization.py | 51 +++++++++++++++++++++++++++--------------- quetz/main.py | 30 ++++++++++++++----------- 2 files changed, 50 insertions(+), 31 deletions(-) diff --git a/quetz/authorization.py b/quetz/authorization.py index 5c792371..52976ad5 100644 --- a/quetz/authorization.py +++ b/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: diff --git a/quetz/main.py b/quetz/main.py index 9ca4ffb6..3afb6df0 100644 --- a/quetz/main.py +++ b/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: