From 0fb9d5faa57dfd1417c341ae797bdeb44c2afe94 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Mon, 12 Aug 2024 13:01:42 +0200 Subject: [PATCH 1/4] Add a DependsOnUser dependency --- lib/galaxy/exceptions/__init__.py | 5 +++++ lib/galaxy/exceptions/error_codes.json | 5 +++++ lib/galaxy/webapps/galaxy/api/__init__.py | 15 ++++++++++++++- 3 files changed, 24 insertions(+), 1 deletion(-) diff --git a/lib/galaxy/exceptions/__init__.py b/lib/galaxy/exceptions/__init__.py index 8deab77bc17b..e52ec14db7fe 100644 --- a/lib/galaxy/exceptions/__init__.py +++ b/lib/galaxy/exceptions/__init__.py @@ -209,6 +209,11 @@ class UserCannotRunAsException(MessageException): err_code = error_codes_by_name["USER_CANNOT_RUN_AS"] +class UserRequiredException(MessageException): + status_code = 403 + err_code = error_codes_by_name["USER_REQUIRED"] + + class AdminRequiredException(MessageException): status_code = 403 err_code = error_codes_by_name["ADMIN_REQUIRED"] diff --git a/lib/galaxy/exceptions/error_codes.json b/lib/galaxy/exceptions/error_codes.json index 99464306acb6..5eea7fbbe9d3 100644 --- a/lib/galaxy/exceptions/error_codes.json +++ b/lib/galaxy/exceptions/error_codes.json @@ -144,6 +144,11 @@ "code": 403007, "message": "Action requires account activation." }, + { + "name": "USER_REQUIRED", + "code": 403008, + "message": "Action requires user authentication." + }, { "name": "USER_OBJECT_NOT_FOUND", "code": 404001, diff --git a/lib/galaxy/webapps/galaxy/api/__init__.py b/lib/galaxy/webapps/galaxy/api/__init__.py index 464d4bc90f41..3b96292f955d 100644 --- a/lib/galaxy/webapps/galaxy/api/__init__.py +++ b/lib/galaxy/webapps/galaxy/api/__init__.py @@ -68,6 +68,7 @@ from galaxy.exceptions import ( AdminRequiredException, UserCannotRunAsException, + UserRequiredException, ) from galaxy.managers.session import GalaxySessionManager from galaxy.managers.users import UserManager @@ -183,6 +184,17 @@ def get_user( return api_user +def get_required_user( + galaxy_session=cast(Optional[model.GalaxySession], Depends(get_session)), + api_user=cast(Optional[User], Depends(get_api_user)), +) -> User: + if galaxy_session and (user := galaxy_session.user): + return user + if api_user: + return api_user + raise UserRequiredException + + class UrlBuilder: def __init__(self, request: Request): self.request = request @@ -310,7 +322,7 @@ def set_cookie( ) -DependsOnUser = cast(Optional[User], Depends(get_user)) +DependsOnUser = cast(User, Depends(get_required_user)) def get_current_history_from_session(galaxy_session: Optional[model.GalaxySession]) -> Optional[model.History]: @@ -485,6 +497,7 @@ def cbv(self): class Router(FrameworkRouter): admin_user_dependency = AdminUserRequired + user_dependency = DependsOnUser class APIContentTypeRoute(APIRoute): From c2913e6a2f517531ccaab000302482e92deec3ee Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Mon, 12 Aug 2024 13:02:34 +0200 Subject: [PATCH 2/4] Require a user for all user-defined file source API routes --- lib/galaxy/webapps/galaxy/api/file_sources.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/galaxy/webapps/galaxy/api/file_sources.py b/lib/galaxy/webapps/galaxy/api/file_sources.py index ef777b0e3283..faa9f606e4f4 100644 --- a/lib/galaxy/webapps/galaxy/api/file_sources.py +++ b/lib/galaxy/webapps/galaxy/api/file_sources.py @@ -16,10 +16,12 @@ ModifyInstancePayload, UserFileSourceModel, ) +from galaxy.model import User from galaxy.util.config_templates import PluginStatus from . import ( depends, DependsOnTrans, + DependsOnUser, Router, ) @@ -36,6 +38,7 @@ @router.cbv class FastAPIFileSources: file_source_instances_manager: FileSourceInstancesManager = depends(FileSourceInstancesManager) + user: User = DependsOnUser @router.get( "/api/file_source_templates", From 2d149f6ee475bc053536dbffc604d14fb2d6cf38 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Mon, 12 Aug 2024 13:03:10 +0200 Subject: [PATCH 3/4] Require user for user based object store instances --- lib/galaxy/webapps/galaxy/api/object_store.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/lib/galaxy/webapps/galaxy/api/object_store.py b/lib/galaxy/webapps/galaxy/api/object_store.py index 886f526420fd..23b8d1575515 100644 --- a/lib/galaxy/webapps/galaxy/api/object_store.py +++ b/lib/galaxy/webapps/galaxy/api/object_store.py @@ -27,6 +27,7 @@ ObjectStoreInstancesManager, UserConcreteObjectStoreModel, ) +from galaxy.model import User from galaxy.objectstore import ( BaseObjectStore, ConcreteObjectStoreModel, @@ -36,6 +37,7 @@ from . import ( depends, DependsOnTrans, + DependsOnUser, Router, ) @@ -95,6 +97,7 @@ def index( def create( self, trans: ProvidesUserContext = DependsOnTrans, + user: User = DependsOnUser, payload: CreateInstancePayload = Body(...), ) -> UserConcreteObjectStoreModel: return self.object_store_instance_manager.create_instance(trans, payload) @@ -107,6 +110,7 @@ def create( def test_instance_configuration( self, trans: ProvidesUserContext = DependsOnTrans, + user: User = DependsOnUser, payload: CreateInstancePayload = Body(...), ) -> PluginStatus: return self.object_store_instance_manager.plugin_status(trans, payload) @@ -119,6 +123,7 @@ def test_instance_configuration( def instance_index( self, trans: ProvidesUserContext = DependsOnTrans, + user: User = DependsOnUser, ) -> List[UserConcreteObjectStoreModel]: return self.object_store_instance_manager.index(trans) @@ -130,6 +135,7 @@ def instance_index( def instances_show( self, trans: ProvidesUserContext = DependsOnTrans, + user: User = DependsOnUser, user_object_store_id: str = UserObjectStoreIdPathParam, ) -> UserConcreteObjectStoreModel: return self.object_store_instance_manager.show(trans, user_object_store_id) @@ -153,6 +159,7 @@ def show_info( def update_instance( self, trans: ProvidesUserContext = DependsOnTrans, + user: User = DependsOnUser, user_object_store_id: str = UserObjectStoreIdPathParam, payload: ModifyInstancePayload = Body(...), ) -> UserConcreteObjectStoreModel: @@ -167,6 +174,7 @@ def update_instance( def purge_instance( self, trans: ProvidesUserContext = DependsOnTrans, + user: User = DependsOnUser, user_object_store_id: str = UserObjectStoreIdPathParam, ): self.object_store_instance_manager.purge_instance(trans, user_object_store_id) @@ -181,6 +189,7 @@ def purge_instance( def index_templates( self, trans: ProvidesUserContext = DependsOnTrans, + user: User = DependsOnUser, ) -> ObjectStoreTemplateSummaries: return self.object_store_instance_manager.summaries From d23186b60593d6e250ffd4d60b54ece953d9d966 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Mon, 12 Aug 2024 13:16:21 +0200 Subject: [PATCH 4/4] Use pydantic UUID4 type to validate UUID strings This works fine for prasing strings: ``` In [9]: UserConcreteObjectStoreModel(uuid=uuid.uuid4().hex, private=False, quota={"enabled": False}, badges=[]) Out[9]: UserConcreteObjectStoreModel(object_store_id=None, private=False, name=None, description=None, quota=QuotaModel(source=None, enabled=False), badges=[], device=None, uuid=UUID('c7a6751b-4d78-4d7f-b3b1-b2baf0a50d90')) ``` Fixes https://github.com/galaxyproject/galaxy/issues/18684#event-13846279733 --- client/src/api/schema/schema.ts | 10 ++++++++-- lib/galaxy/managers/file_source_instances.py | 19 +++++++++--------- lib/galaxy/managers/object_store_instances.py | 20 ++++++++++--------- lib/galaxy/webapps/galaxy/api/file_sources.py | 9 +++++---- lib/galaxy/webapps/galaxy/api/object_store.py | 9 +++++---- 5 files changed, 39 insertions(+), 28 deletions(-) diff --git a/client/src/api/schema/schema.ts b/client/src/api/schema/schema.ts index ba79fc3a1ab3..fce5294b4509 100644 --- a/client/src/api/schema/schema.ts +++ b/client/src/api/schema/schema.ts @@ -12848,7 +12848,10 @@ export interface components { * @enum {string} */ type: "aws_s3" | "azure_blob" | "boto3" | "disk" | "generic_s3"; - /** Uuid */ + /** + * Uuid + * Format: uuid4 + */ uuid: string; /** Variables */ variables: { @@ -12922,7 +12925,10 @@ export interface components { type: "ftp" | "posix" | "s3fs" | "azure"; /** Uri Root */ uri_root: string; - /** Uuid */ + /** + * Uuid + * Format: uuid4 + */ uuid: string; /** Variables */ variables: { diff --git a/lib/galaxy/managers/file_source_instances.py b/lib/galaxy/managers/file_source_instances.py index 9b70926c8a42..f364110828cd 100644 --- a/lib/galaxy/managers/file_source_instances.py +++ b/lib/galaxy/managers/file_source_instances.py @@ -13,6 +13,7 @@ from pydantic import ( BaseModel, + UUID4, ValidationError, ) @@ -87,7 +88,7 @@ class UserFileSourceModel(BaseModel): - uuid: str + uuid: UUID4 uri_root: str name: str description: Optional[str] @@ -142,16 +143,16 @@ def index(self, trans: ProvidesUserContext) -> List[UserFileSourceModel]: stores = self._sa_session.query(UserFileSource).filter(UserFileSource.user_id == trans.user.id).all() return [self._to_model(trans, s) for s in stores] - def show(self, trans: ProvidesUserContext, uuid: str) -> UserFileSourceModel: + def show(self, trans: ProvidesUserContext, uuid: UUID4) -> UserFileSourceModel: user_file_source = self._get(trans, uuid) return self._to_model(trans, user_file_source) - def purge_instance(self, trans: ProvidesUserContext, uuid: str) -> None: + def purge_instance(self, trans: ProvidesUserContext, uuid: UUID4) -> None: persisted_file_source = self._get(trans, uuid) purge_template_instance(trans, persisted_file_source, self._app_config) def modify_instance( - self, trans: ProvidesUserContext, id: str, payload: ModifyInstancePayload + self, trans: ProvidesUserContext, id: UUID4, payload: ModifyInstancePayload ) -> UserFileSourceModel: if isinstance(payload, UpgradeInstancePayload): return self._upgrade_instance(trans, id, payload) @@ -162,7 +163,7 @@ def modify_instance( return self._update_instance(trans, id, payload) def _upgrade_instance( - self, trans: ProvidesUserContext, id: str, payload: UpgradeInstancePayload + self, trans: ProvidesUserContext, id: UUID4, payload: UpgradeInstancePayload ) -> UserFileSourceModel: persisted_file_source = self._get(trans, id) template = self._get_template(persisted_file_source, payload.template_version) @@ -181,7 +182,7 @@ def _upgrade_instance( return self._to_model(trans, persisted_file_source) def _update_instance( - self, trans: ProvidesUserContext, id: str, payload: UpdateInstancePayload + self, trans: ProvidesUserContext, id: UUID4, payload: UpdateInstancePayload ) -> UserFileSourceModel: persisted_file_source = self._get(trans, id) template = self._get_template(persisted_file_source) @@ -189,7 +190,7 @@ def _update_instance( return self._to_model(trans, persisted_file_source) def _update_instance_secret( - self, trans: ProvidesUserContext, id: str, payload: UpdateInstanceSecretPayload + self, trans: ProvidesUserContext, id: UUID4, payload: UpdateInstanceSecretPayload ) -> UserFileSourceModel: persisted_file_source = self._get(trans, id) template = self._get_template(persisted_file_source) @@ -300,10 +301,10 @@ def _connection_status( exception = e return file_source, connection_exception_to_status("file source", exception) - def _index_filter(self, uuid: str): + def _index_filter(self, uuid: UUID4): return UserFileSource.__table__.c.uuid == uuid - def _get(self, trans: ProvidesUserContext, uuid: str) -> UserFileSource: + def _get(self, trans: ProvidesUserContext, uuid: UUID4) -> UserFileSource: filter = self._index_filter(uuid) user_file_source = self._sa_session.query(UserFileSource).filter(filter).one_or_none() if user_file_source is None: diff --git a/lib/galaxy/managers/object_store_instances.py b/lib/galaxy/managers/object_store_instances.py index ca1df59912a2..b0163c7be249 100644 --- a/lib/galaxy/managers/object_store_instances.py +++ b/lib/galaxy/managers/object_store_instances.py @@ -16,6 +16,8 @@ ) from uuid import uuid4 +from pydantic import UUID4 + from galaxy.exceptions import ( ItemOwnershipException, RequestParameterInvalidException, @@ -73,7 +75,7 @@ class UserConcreteObjectStoreModel(ConcreteObjectStoreModel): - uuid: str + uuid: UUID4 type: ObjectStoreTemplateType template_id: str template_version: int @@ -106,7 +108,7 @@ def summaries(self) -> ObjectStoreTemplateSummaries: return self._catalog.summaries def modify_instance( - self, trans: ProvidesUserContext, id: str, payload: ModifyInstancePayload + self, trans: ProvidesUserContext, id: UUID4, payload: ModifyInstancePayload ) -> UserConcreteObjectStoreModel: if isinstance(payload, UpgradeInstancePayload): return self._upgrade_instance(trans, id, payload) @@ -116,12 +118,12 @@ def modify_instance( assert isinstance(payload, UpdateInstancePayload) return self._update_instance(trans, id, payload) - def purge_instance(self, trans: ProvidesUserContext, id: str) -> None: + def purge_instance(self, trans: ProvidesUserContext, id: UUID4) -> None: persisted_object_store = self._get(trans, id) purge_template_instance(trans, persisted_object_store, self._app_config) def _upgrade_instance( - self, trans: ProvidesUserContext, id: str, payload: UpgradeInstancePayload + self, trans: ProvidesUserContext, id: UUID4, payload: UpgradeInstancePayload ) -> UserConcreteObjectStoreModel: persisted_object_store = self._get(trans, id) template = self._get_template(persisted_object_store, payload.template_version) @@ -140,7 +142,7 @@ def _upgrade_instance( return self._to_model(trans, persisted_object_store) def _update_instance( - self, trans: ProvidesUserContext, id: str, payload: UpdateInstancePayload + self, trans: ProvidesUserContext, id: UUID4, payload: UpdateInstancePayload ) -> UserConcreteObjectStoreModel: persisted_object_store = self._get(trans, id) template = self._get_template(persisted_object_store) @@ -148,7 +150,7 @@ def _update_instance( return self._to_model(trans, persisted_object_store) def _update_instance_secret( - self, trans: ProvidesUserContext, id: str, payload: UpdateInstanceSecretPayload + self, trans: ProvidesUserContext, id: UUID4, payload: UpdateInstanceSecretPayload ) -> UserConcreteObjectStoreModel: persisted_object_store = self._get(trans, id) template = self._get_template(persisted_object_store) @@ -200,14 +202,14 @@ def index(self, trans: ProvidesUserContext) -> List[UserConcreteObjectStoreModel stores = self._sa_session.query(UserObjectStore).filter(UserObjectStore.user_id == trans.user.id).all() return [self._to_model(trans, s) for s in stores] - def show(self, trans: ProvidesUserContext, id: str) -> UserConcreteObjectStoreModel: + def show(self, trans: ProvidesUserContext, id: UUID4) -> UserConcreteObjectStoreModel: user_object_store = self._get(trans, id) return self._to_model(trans, user_object_store) def _save(self, persisted_object_store: UserObjectStore) -> None: save_template_instance(self._sa_session, persisted_object_store) - def _get(self, trans: ProvidesUserContext, id: str) -> UserObjectStore: + def _get(self, trans: ProvidesUserContext, id: UUID4) -> UserObjectStore: filter = self._index_filter(id) user_object_store = self._sa_session.query(UserObjectStore).filter(filter).one_or_none() if user_object_store is None: @@ -274,7 +276,7 @@ def _connection_status( exception = e return object_store, connection_exception_to_status("storage location", exception) - def _index_filter(self, uuid: str): + def _index_filter(self, uuid: UUID4): return UserObjectStore.__table__.c.uuid == uuid def _get_template( diff --git a/lib/galaxy/webapps/galaxy/api/file_sources.py b/lib/galaxy/webapps/galaxy/api/file_sources.py index faa9f606e4f4..957da9b54092 100644 --- a/lib/galaxy/webapps/galaxy/api/file_sources.py +++ b/lib/galaxy/webapps/galaxy/api/file_sources.py @@ -7,6 +7,7 @@ Response, status, ) +from pydantic import UUID4 from galaxy.files.templates import FileSourceTemplateSummaries from galaxy.managers.context import ProvidesUserContext @@ -30,7 +31,7 @@ router = Router(tags=["file_sources"]) -UserFileSourceIdPathParam: str = Path( +UserFileSourceIdPathParam: UUID4 = Path( ..., title="User File Source UUID", description="The UUID index for a persisted UserFileSourceStore object." ) @@ -95,7 +96,7 @@ def instance_index( def instances_show( self, trans: ProvidesUserContext = DependsOnTrans, - user_file_source_id: str = UserFileSourceIdPathParam, + user_file_source_id: UUID4 = UserFileSourceIdPathParam, ) -> UserFileSourceModel: return self.file_source_instances_manager.show(trans, user_file_source_id) @@ -107,7 +108,7 @@ def instances_show( def update_instance( self, trans: ProvidesUserContext = DependsOnTrans, - user_file_source_id: str = UserFileSourceIdPathParam, + user_file_source_id: UUID4 = UserFileSourceIdPathParam, payload: ModifyInstancePayload = Body(...), ) -> UserFileSourceModel: return self.file_source_instances_manager.modify_instance(trans, user_file_source_id, payload) @@ -121,7 +122,7 @@ def update_instance( def purge_instance( self, trans: ProvidesUserContext = DependsOnTrans, - user_file_source_id: str = UserFileSourceIdPathParam, + user_file_source_id: UUID4 = UserFileSourceIdPathParam, ): self.file_source_instances_manager.purge_instance(trans, user_file_source_id) return Response(status_code=status.HTTP_204_NO_CONTENT) diff --git a/lib/galaxy/webapps/galaxy/api/object_store.py b/lib/galaxy/webapps/galaxy/api/object_store.py index 23b8d1575515..3927ff17b1d5 100644 --- a/lib/galaxy/webapps/galaxy/api/object_store.py +++ b/lib/galaxy/webapps/galaxy/api/object_store.py @@ -15,6 +15,7 @@ Response, status, ) +from pydantic import UUID4 from galaxy.exceptions import ( ObjectNotFound, @@ -49,7 +50,7 @@ ..., title="Concrete Object Store ID", description="The concrete object store ID." ) -UserObjectStoreIdPathParam: str = Path( +UserObjectStoreIdPathParam: UUID4 = Path( ..., title="User Object Store UUID", description="The UUID used to identify a persisted UserObjectStore object.", @@ -136,7 +137,7 @@ def instances_show( self, trans: ProvidesUserContext = DependsOnTrans, user: User = DependsOnUser, - user_object_store_id: str = UserObjectStoreIdPathParam, + user_object_store_id: UUID4 = UserObjectStoreIdPathParam, ) -> UserConcreteObjectStoreModel: return self.object_store_instance_manager.show(trans, user_object_store_id) @@ -160,7 +161,7 @@ def update_instance( self, trans: ProvidesUserContext = DependsOnTrans, user: User = DependsOnUser, - user_object_store_id: str = UserObjectStoreIdPathParam, + user_object_store_id: UUID4 = UserObjectStoreIdPathParam, payload: ModifyInstancePayload = Body(...), ) -> UserConcreteObjectStoreModel: return self.object_store_instance_manager.modify_instance(trans, user_object_store_id, payload) @@ -175,7 +176,7 @@ def purge_instance( self, trans: ProvidesUserContext = DependsOnTrans, user: User = DependsOnUser, - user_object_store_id: str = UserObjectStoreIdPathParam, + user_object_store_id: UUID4 = UserObjectStoreIdPathParam, ): self.object_store_instance_manager.purge_instance(trans, user_object_store_id) return Response(status_code=status.HTTP_204_NO_CONTENT)