From 25825e3e309a9a9ddf38b28cbc4300df72225cb8 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Wed, 8 May 2024 08:38:12 +0200 Subject: [PATCH 1/4] Allow purge query param, deprecate purge body param Fixes https://github.com/galaxyproject/galaxy/issues/18104 --- client/src/api/schema/schema.ts | 10 ++++++++-- lib/galaxy/schema/schema.py | 7 ++++++- lib/galaxy/webapps/galaxy/api/users.py | 9 +++------ 3 files changed, 17 insertions(+), 9 deletions(-) diff --git a/client/src/api/schema/schema.ts b/client/src/api/schema/schema.ts index 278532a3248..18b6dd6142a 100644 --- a/client/src/api/schema/schema.ts +++ b/client/src/api/schema/schema.ts @@ -9576,9 +9576,11 @@ export interface components { UserDeletionPayload: { /** * Purge user - * @description Purge the user + * @deprecated + * @description Purge the user. Deprecated, please use the `purge` query parameter instead. + * @default false */ - purge: boolean; + purge?: boolean; }; /** UserEmail */ UserEmail: { @@ -18633,6 +18635,10 @@ export interface operations { delete_user_api_users__user_id__delete: { /** Delete a user. Only admins can delete others or purge users. */ parameters: { + /** @description Purge user. Only deleted users can be purged. */ + query?: { + purge?: boolean; + }; /** @description The user ID that will be used to effectively make this API call. Only admins and designated users can make API calls on behalf of other users. */ header?: { "run-as"?: string; diff --git a/lib/galaxy/schema/schema.py b/lib/galaxy/schema/schema.py index 6920fe5dbb9..95e8bcb7b93 100644 --- a/lib/galaxy/schema/schema.py +++ b/lib/galaxy/schema/schema.py @@ -351,7 +351,12 @@ class RemoteUserCreationPayload(Model): class UserDeletionPayload(Model): - purge: bool = Field(default=Required, title="Purge user", description="Purge the user") + purge: bool = Field( + default=False, + title="Purge user", + description="Purge the user. Deprecated, please use the `purge` query parameter instead.", + deprecated=True, + ) class FavoriteObject(Model): diff --git a/lib/galaxy/webapps/galaxy/api/users.py b/lib/galaxy/webapps/galaxy/api/users.py index 917e4955bda..c10d9b8653f 100644 --- a/lib/galaxy/webapps/galaxy/api/users.py +++ b/lib/galaxy/webapps/galaxy/api/users.py @@ -135,7 +135,6 @@ RecalculateDiskUsageResponseDescriptions = { }, } -UserDeletionBody = Body(default=None, title="Purge user", description="Purge the user.") UserUpdateBody = Body(default=Required, title="Update user", description="The user values to update.") FavoriteObjectBody = Body( default=Required, title="Set favorite", description="The id of an object the user wants to favorite." @@ -656,13 +655,11 @@ class FastAPIUsers: self, trans: ProvidesUserContext = DependsOnTrans, user_id: DecodedDatabaseIdField = UserIdPathParamQueryParam, - payload: Optional[UserDeletionPayload] = UserDeletionBody, + purge: bool = Query(False, title="Purge user", description="Purge user. Only deleted users can be purged."), + payload: Optional[UserDeletionPayload] = None, ) -> DetailedUserModel: user_to_update = self.service.user_manager.by_id(user_id) - if payload: - purge = payload.purge - else: - purge = False + purge = payload and payload.purge or purge if trans.user_is_admin: if purge: log.debug("Purging user %s", user_to_update) From 788c870d132020ce4ae7cf220991aecf3ea8cae4 Mon Sep 17 00:00:00 2001 From: Nicola Soranzo Date: Thu, 9 May 2024 11:13:43 +0100 Subject: [PATCH 2/4] Redact private role only once --- lib/galaxy/managers/users.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/galaxy/managers/users.py b/lib/galaxy/managers/users.py index ee9b05c3ac7..0600feb3627 100644 --- a/lib/galaxy/managers/users.py +++ b/lib/galaxy/managers/users.py @@ -227,9 +227,10 @@ class UserManager(base.ModelManager, deletable.PurgableManagerMixin): if self.app.config.redact_email_during_deletion: role.name = role.name.replace(user.email, email_hash) role.description = role.description.replace(user.email, email_hash) - private_role.name = email_hash - private_role.description = f"Private Role for {email_hash}" - self.session().add(private_role) + self.session().add(role) + private_role.name = email_hash + private_role.description = f"Private Role for {email_hash}" + self.session().add(private_role) # Redact user's email and username user.email = email_hash user.username = uname_hash From 127e4612ccb971ad69db362278d04a948a3a7085 Mon Sep 17 00:00:00 2001 From: Nicola Soranzo Date: Thu, 9 May 2024 11:16:42 +0100 Subject: [PATCH 3/4] Use ``Annotated`` and actual default value as recommended in https://fastapi.tiangolo.com/tutorial/query-params-str-validations/#alternative-old-query-as-the-default-value --- client/src/api/schema/schema.ts | 2 +- lib/galaxy/webapps/galaxy/api/users.py | 9 ++++++++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/client/src/api/schema/schema.ts b/client/src/api/schema/schema.ts index 18b6dd6142a..0fa8cf0bc68 100644 --- a/client/src/api/schema/schema.ts +++ b/client/src/api/schema/schema.ts @@ -18635,7 +18635,7 @@ export interface operations { delete_user_api_users__user_id__delete: { /** Delete a user. Only admins can delete others or purge users. */ parameters: { - /** @description Purge user. Only deleted users can be purged. */ + /** @description Whether to definitely remove this user. Only deleted users can be purged. */ query?: { purge?: boolean; }; diff --git a/lib/galaxy/webapps/galaxy/api/users.py b/lib/galaxy/webapps/galaxy/api/users.py index c10d9b8653f..ee9608cf088 100644 --- a/lib/galaxy/webapps/galaxy/api/users.py +++ b/lib/galaxy/webapps/galaxy/api/users.py @@ -22,6 +22,7 @@ from fastapi import ( ) from markupsafe import escape from pydantic import Required +from typing_extensions import Annotated from galaxy import ( exceptions, @@ -655,7 +656,13 @@ class FastAPIUsers: self, trans: ProvidesUserContext = DependsOnTrans, user_id: DecodedDatabaseIdField = UserIdPathParamQueryParam, - purge: bool = Query(False, title="Purge user", description="Purge user. Only deleted users can be purged."), + purge: Annotated[ + bool, + Query( + title="Purge user", + description="Whether to definitely remove this user. Only deleted users can be purged.", + ), + ] = False, payload: Optional[UserDeletionPayload] = None, ) -> DetailedUserModel: user_to_update = self.service.user_manager.by_id(user_id) From 40b31516268862bed20c61a9e6a21597fcfd8afe Mon Sep 17 00:00:00 2001 From: Nicola Soranzo Date: Thu, 9 May 2024 11:21:04 +0100 Subject: [PATCH 4/4] Update ``test_purge_user`` test to use request parameter --- lib/galaxy/tool_util/verify/interactor.py | 4 ++-- lib/galaxy_test/api/test_users.py | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/galaxy/tool_util/verify/interactor.py b/lib/galaxy/tool_util/verify/interactor.py index 94cdb25a8ef..5aef947c72b 100644 --- a/lib/galaxy/tool_util/verify/interactor.py +++ b/lib/galaxy/tool_util/verify/interactor.py @@ -938,10 +938,10 @@ class GalaxyInteractorApi: kwd["timeout"] = kwd.pop("timeout", util.DEFAULT_SOCKET_TIMEOUT) return requests.post(url, **kwd) - def _delete(self, path, data=None, key=None, headers=None, admin=False, anon=False, json=False): + def _delete(self, path, data=None, key=None, headers=None, admin=False, anon=False, json=False, params=None): headers = self.api_key_header(key=key, admin=admin, anon=anon, headers=headers) url = self.get_api_url(path) - kwd = self._prepare_request_params(data=data, as_json=json, headers=headers) + kwd = self._prepare_request_params(data=data, as_json=json, params=params, headers=headers) kwd["timeout"] = kwd.pop("timeout", util.DEFAULT_SOCKET_TIMEOUT) return requests.delete(url, **kwd) diff --git a/lib/galaxy_test/api/test_users.py b/lib/galaxy_test/api/test_users.py index a92e13769a8..d1c3a4e0590 100644 --- a/lib/galaxy_test/api/test_users.py +++ b/lib/galaxy_test/api/test_users.py @@ -108,11 +108,11 @@ class TestUsersApi(ApiTestCase): user = self._setup_user(TEST_USER_EMAIL_PURGE) response = self._delete(f"users/{user['id']}", admin=True) self._assert_status_code_is_ok(response) - data = dict(purge="True") - response = self._delete(f"users/{user['id']}", data=data, admin=True, json=True) + params = dict(purge="True") + response = self._delete(f"users/{user['id']}", params=params, admin=True, json=True) self._assert_status_code_is_ok(response) - payload = {"deleted": "True"} - purged_user = self._get(f"users/{user['id']}", payload, admin=True).json() + params = {"deleted": "True"} + purged_user = self._get(f"users/{user['id']}", params, admin=True).json() assert purged_user["deleted"] is True, purged_user assert purged_user["purged"] is True, purged_user