From 857cc66022bd3516e8d83584d211cd41b9b10fab Mon Sep 17 00:00:00 2001 From: Jan-Benedikt Jagusch Date: Mon, 10 Apr 2023 10:58:32 +0200 Subject: [PATCH 1/7] Fix pyright errors. --- .../quetz_conda_suggest/main.py | 2 ++ .../quetz_content_trust/quetz_content_trust/api.py | 3 +++ plugins/quetz_runexports/quetz_runexports/main.py | 2 ++ quetz/dao.py | 14 ++++++++++---- quetz/main.py | 12 ++++++++++++ quetz/tasks/mirror.py | 4 ++++ quetz/tasks/workers.py | 2 ++ 7 files changed, 35 insertions(+), 4 deletions(-) diff --git a/plugins/quetz_conda_suggest/quetz_conda_suggest/main.py b/plugins/quetz_conda_suggest/quetz_conda_suggest/main.py index f2daa487..d9b2a63b 100644 --- a/plugins/quetz_conda_suggest/quetz_conda_suggest/main.py +++ b/plugins/quetz_conda_suggest/quetz_conda_suggest/main.py @@ -53,6 +53,8 @@ def post_add_package_version(version, condainfo): db.add(metadata) else: metadata = db.get(db_models.CondaSuggestMetadata, version.id) + if not metadata: + raise KeyError(f"Metadata with version '{version.id}' not found") metadata.data = json.dumps(suggest_map) db.commit() generate_channel_suggest_map(db, version.channel_name, subdir) diff --git a/plugins/quetz_content_trust/quetz_content_trust/api.py b/plugins/quetz_content_trust/quetz_content_trust/api.py index 3ae0d754..5f55dbef 100644 --- a/plugins/quetz_content_trust/quetz_content_trust/api.py +++ b/plugins/quetz_content_trust/quetz_content_trust/api.py @@ -171,6 +171,9 @@ def get_self_delegation(nullable: bool = False): # mamba API when loading the root role from file self_delegation = [r for r in db_role.delegations if r.type == "root"][0] + if not self_delegation: + raise RuntimeError("self_delegation must not be None") + # db_role.delegation = self_delegation self_delegation.consumers.append(db_role) diff --git a/plugins/quetz_runexports/quetz_runexports/main.py b/plugins/quetz_runexports/quetz_runexports/main.py index 2f2d8d66..4a580b81 100644 --- a/plugins/quetz_runexports/quetz_runexports/main.py +++ b/plugins/quetz_runexports/quetz_runexports/main.py @@ -24,5 +24,7 @@ def post_add_package_version(version, condainfo): db.add(metadata) else: metadata = db.get(db_models.PackageVersionMetadata, version.id) + if not metadata: + raise KeyError(f"No metadata found for version '{version.id}'.") metadata.data = run_exports db.commit() diff --git a/quetz/dao.py b/quetz/dao.py index 3ceb78f8..e83774f0 100644 --- a/quetz/dao.py +++ b/quetz/dao.py @@ -11,9 +11,9 @@ from sqlalchemy import and_, func, insert, or_ from sqlalchemy.dialects.postgresql import insert as pg_insert -from sqlalchemy.exc import IntegrityError +from sqlalchemy.exc import IntegrityError, NoResultFound from sqlalchemy.ext.compiler import compiles -from sqlalchemy.orm import Query, Session, aliased, exc, joinedload +from sqlalchemy.orm import Query, Session, aliased, joinedload from sqlalchemy.sql.expression import FunctionElement, Insert from sqlalchemy.types import DateTime @@ -193,13 +193,13 @@ def rollback(self): def get_profile(self, user_id): try: return self.db.query(Profile).filter(Profile.user_id == user_id).one() - except exc.NoResultFound: + except NoResultFound: logger.error("User not found") def get_user(self, user_id): try: return self.db.query(User).filter(User.id == user_id).one() - except exc.NoResultFound: + except NoResultFound: logger.error("User not found") def get_users(self, skip: int, limit: int, q: str, order_by: str = 'username:asc'): @@ -621,6 +621,8 @@ def create_package( def update_package_channeldata(self, channel_name, package_name, channeldata): package = self.get_package(channel_name, package_name) + if package is None: + raise KeyError(f"Package '{package_name}' not found.") if package.channeldata: old_data = json.loads(package.channeldata) else: @@ -651,6 +653,8 @@ def get_channel_member(self, channel_name, username): def create_channel_member(self, channel_name, new_member): user = self.get_user_by_username(new_member.username) + if user is None: + raise KeyError(f"User '{new_member.username}' not found.") member = ChannelMember( channel_name=channel_name, user_id=user.id, role=new_member.role @@ -681,6 +685,8 @@ def get_package_member(self, channel_name, package_name, username): def create_package_member(self, channel_name, package_name, new_member): user = self.get_user_by_username(new_member.username) + if user is None: + raise KeyError(f"User '{new_member.username}' not found.") member = PackageMember( channel_name=channel_name, diff --git a/quetz/main.py b/quetz/main.py index c4401e15..8be22e5d 100644 --- a/quetz/main.py +++ b/quetz/main.py @@ -250,6 +250,8 @@ def dummy_login( username: str, dao: Dao = Depends(get_dao), session=Depends(get_session) ): user = dao.get_user_by_username(username) + if user is None: + raise HTTPException(status_code=404, detail=f"User '{username}' not found.") logout(session) session["user_id"] = str(uuid.UUID(bytes=user.id)) @@ -439,6 +441,11 @@ def delete_user( ): user = dao.get_user_by_username(username) + if not user or not user.profile: + raise HTTPException( + status_code=status.HTTP_404_NOT_FOUND, detail=f"User {username} not found" + ) + auth.assert_delete_user(user.id) dao.delete_user(user.id) @@ -1106,6 +1113,11 @@ def delete_package_version( channel_name, package_name, filename, platform ) + if not version or not version.package: + raise HTTPException( + status_code=status.HTTP_404_NOT_FOUND, detail="Package version not found" + ) + auth.assert_package_delete(version.package) if not version: diff --git a/quetz/tasks/mirror.py b/quetz/tasks/mirror.py index 67cb8dbc..879d0717 100644 --- a/quetz/tasks/mirror.py +++ b/quetz/tasks/mirror.py @@ -459,6 +459,10 @@ def create_packages_from_channeldata( except DBError: # package already exists so skip it so we retrieve and update it package = dao.get_package(channel_name, package_name) + if not package: + raise KeyError( + f"Package '{package_name}' not found in channel {channel_name}" + ) package.description = description package.summary = summary package.url = metadata.get("home", "") diff --git a/quetz/tasks/workers.py b/quetz/tasks/workers.py index 05ac8974..14f15ce2 100644 --- a/quetz/tasks/workers.py +++ b/quetz/tasks/workers.py @@ -154,6 +154,8 @@ def job_wrapper( user_id: Optional[str] if task_id: task = db.query(Task).filter(Task.id == task_id).one_or_none() + if not task: + raise KeyError(f"Task '{task_id}' not found") # take extra arguments from job definition if task.job.extra_args: job_extra_args = json.loads(task.job.extra_args) From 310da71927e0b7c033899b4df31e0965a4f85556 Mon Sep 17 00:00:00 2001 From: Jan-Benedikt Jagusch Date: Mon, 10 Apr 2023 11:13:16 +0200 Subject: [PATCH 2/7] Silence mypy error. --- quetz/dao.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/quetz/dao.py b/quetz/dao.py index e83774f0..115680da 100644 --- a/quetz/dao.py +++ b/quetz/dao.py @@ -11,7 +11,7 @@ from sqlalchemy import and_, func, insert, or_ from sqlalchemy.dialects.postgresql import insert as pg_insert -from sqlalchemy.exc import IntegrityError, NoResultFound +from sqlalchemy.exc import IntegrityError, NoResultFound # type: ignore from sqlalchemy.ext.compiler import compiles from sqlalchemy.orm import Query, Session, aliased, joinedload from sqlalchemy.sql.expression import FunctionElement, Insert From 5f08de548150f5fdcb75d698a52fee992191eddf Mon Sep 17 00:00:00 2001 From: Jan-Benedikt Jagusch Date: Mon, 10 Apr 2023 11:17:57 +0200 Subject: [PATCH 3/7] Return 404s. --- quetz/main.py | 34 ++++++++++++++++++++++++++++------ 1 file changed, 28 insertions(+), 6 deletions(-) diff --git a/quetz/main.py b/quetz/main.py index 8be22e5d..5532de06 100644 --- a/quetz/main.py +++ b/quetz/main.py @@ -944,7 +944,12 @@ def post_channel_member( channel_member.role = new_member.role db.commit() else: - dao.create_channel_member(channel.name, new_member) + try: + dao.create_channel_member(channel.name, new_member) + except KeyError as error: + raise HTTPException( + status_code=status.HTTP_404_NOT_FOUND, detail=error.args[0] + ) from error @api_router.delete("/channels/{channel_name}/members", tags=["channels"]) @@ -1015,7 +1020,12 @@ def post_package_member( ), ) - dao.create_package_member(package.channel.name, package.name, new_member) + try: + dao.create_package_member(package.channel.name, package.name, new_member) + except KeyError as error: + raise HTTPException( + status_code=status.HTTP_404_NOT_FOUND, detail=error.args[0] + ) from error @api_router.get( @@ -1380,7 +1390,14 @@ async def post_upload( ) # Update channeldata info - dao.update_package_channeldata(channel_name, package_name, condainfo.channeldata) + try: + dao.update_package_channeldata( + channel_name, package_name, condainfo.channeldata + ) + except KeyError as error: + raise HTTPException( + status_code=status.HTTP_404_NOT_FOUND, detail=error.args[0] + ) from error try: version = dao.create_version( @@ -1628,9 +1645,14 @@ def _delete_file(condainfo, filename): ) # Update channeldata info - dao.update_package_channeldata( - channel.name, package_name, condainfo.channeldata - ) + try: + dao.update_package_channeldata( + channel.name, package_name, condainfo.channeldata + ) + except KeyError as error: + raise HTTPException( + status_code=status.HTTP_404_NOT_FOUND, detail=error.args[0] + ) from error try: version = dao.create_version( From ff862f0b7823f690829093e59134a6af746d2598 Mon Sep 17 00:00:00 2001 From: Jan-Benedikt Jagusch Date: Mon, 10 Apr 2023 11:19:13 +0200 Subject: [PATCH 4/7] Use status constant. --- quetz/main.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/quetz/main.py b/quetz/main.py index 5532de06..5bfd2d89 100644 --- a/quetz/main.py +++ b/quetz/main.py @@ -251,7 +251,10 @@ def dummy_login( ): user = dao.get_user_by_username(username) if user is None: - raise HTTPException(status_code=404, detail=f"User '{username}' not found.") + raise HTTPException( + status_code=status.HTTP_404_NOT_FOUND, + detail=f"User '{username}' not found.", + ) logout(session) session["user_id"] = str(uuid.UUID(bytes=user.id)) From 10f4331bdfd092281157167491bfca64ac768516 Mon Sep 17 00:00:00 2001 From: Jan-Benedikt Jagusch Date: Mon, 10 Apr 2023 11:20:10 +0200 Subject: [PATCH 5/7] Remove dot. --- quetz/main.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/quetz/main.py b/quetz/main.py index 5bfd2d89..8e1a4d18 100644 --- a/quetz/main.py +++ b/quetz/main.py @@ -253,7 +253,7 @@ def dummy_login( if user is None: raise HTTPException( status_code=status.HTTP_404_NOT_FOUND, - detail=f"User '{username}' not found.", + detail=f"User '{username}' not found", ) logout(session) From 5fa3ae90305b62292f2c6a8a19a06b55dcad46a9 Mon Sep 17 00:00:00 2001 From: Jan-Benedikt Jagusch Date: Mon, 10 Apr 2023 11:23:41 +0200 Subject: [PATCH 6/7] Add some return type hints. --- quetz/dao.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/quetz/dao.py b/quetz/dao.py index 115680da..3e61aa0b 100644 --- a/quetz/dao.py +++ b/quetz/dao.py @@ -222,7 +222,7 @@ def get_users(self, skip: int, limit: int, q: str, order_by: str = 'username:asc return get_paginated_result(query, skip, limit) - def get_user_by_username(self, username: str): + def get_user_by_username(self, username: str) -> Optional[User]: return ( self.db.query(User) .filter(User.username == username) @@ -577,7 +577,7 @@ def search_channels( def get_channel(self, channel_name: str) -> Optional[Channel]: return self.db.query(Channel).filter(Channel.name == channel_name).one_or_none() - def get_package(self, channel_name: str, package_name: str): + def get_package(self, channel_name: str, package_name: str) -> Optional[Package]: return ( self.db.query(Package) .join(Channel) @@ -963,7 +963,7 @@ def get_package_versions( def get_package_version_by_filename( self, channel_name: str, package_name: str, filename: str, platform: str - ): + ) -> Optional[PackageVersion]: query = ( self.db.query(PackageVersion) .filter(PackageVersion.channel_name == channel_name) From 7c362e59e4f33ab2f135ef5b1b9d8b3bddbe3766 Mon Sep 17 00:00:00 2001 From: Jan-Benedikt Jagusch Date: Mon, 10 Apr 2023 11:26:37 +0200 Subject: [PATCH 7/7] Revert "Return 404s." This reverts commit 5f08de548150f5fdcb75d698a52fee992191eddf. --- quetz/main.py | 34 ++++++---------------------------- 1 file changed, 6 insertions(+), 28 deletions(-) diff --git a/quetz/main.py b/quetz/main.py index 8e1a4d18..d3dfb7d5 100644 --- a/quetz/main.py +++ b/quetz/main.py @@ -947,12 +947,7 @@ def post_channel_member( channel_member.role = new_member.role db.commit() else: - try: - dao.create_channel_member(channel.name, new_member) - except KeyError as error: - raise HTTPException( - status_code=status.HTTP_404_NOT_FOUND, detail=error.args[0] - ) from error + dao.create_channel_member(channel.name, new_member) @api_router.delete("/channels/{channel_name}/members", tags=["channels"]) @@ -1023,12 +1018,7 @@ def post_package_member( ), ) - try: - dao.create_package_member(package.channel.name, package.name, new_member) - except KeyError as error: - raise HTTPException( - status_code=status.HTTP_404_NOT_FOUND, detail=error.args[0] - ) from error + dao.create_package_member(package.channel.name, package.name, new_member) @api_router.get( @@ -1393,14 +1383,7 @@ async def post_upload( ) # Update channeldata info - try: - dao.update_package_channeldata( - channel_name, package_name, condainfo.channeldata - ) - except KeyError as error: - raise HTTPException( - status_code=status.HTTP_404_NOT_FOUND, detail=error.args[0] - ) from error + dao.update_package_channeldata(channel_name, package_name, condainfo.channeldata) try: version = dao.create_version( @@ -1648,14 +1631,9 @@ def _delete_file(condainfo, filename): ) # Update channeldata info - try: - dao.update_package_channeldata( - channel.name, package_name, condainfo.channeldata - ) - except KeyError as error: - raise HTTPException( - status_code=status.HTTP_404_NOT_FOUND, detail=error.args[0] - ) from error + dao.update_package_channeldata( + channel.name, package_name, condainfo.channeldata + ) try: version = dao.create_version(