diff --git a/lib/galaxy/managers/users.py b/lib/galaxy/managers/users.py index 1c2cb729a90..e891d2ec948 100644 --- a/lib/galaxy/managers/users.py +++ b/lib/galaxy/managers/users.py @@ -4,6 +4,7 @@ Manager and Serializer for Users. import logging import random import socket +import time from datetime import datetime from markupsafe import escape @@ -20,6 +21,7 @@ from galaxy.managers import ( deletable ) from galaxy.security.validate_user_input import validate_email, validate_password, validate_publicname +from galaxy.util.hash_util import new_secure_hash from galaxy.web import url_for log = logging.getLogger(__name__) @@ -108,10 +110,92 @@ class UserManager(base.ModelManager, deletable.PurgableManagerMixin): self.app.security_agent.user_set_default_permissions(user, default_access_private=permissions) return user - def delete(self, user): - user.deleted = True - self.session().add(user) - self.session().flush() + def delete(self, user, flush=True): + """Mark the given user deleted.""" + if not self.app.config.allow_user_deletion: + raise exceptions.ConfigDoesNotAllowException('The configuration of this Galaxy instance does not allow admins to delete users.') + super(UserManager, self).delete(user, flush=flush) + + def undelete(self, user, flush=True): + """Remove the deleted flag for the given user.""" + if not self.app.config.allow_user_deletion: + raise exceptions.ConfigDoesNotAllowException('The configuration of this Galaxy instance does not allow admins to undelete users.') + if user.purged: + raise exceptions.ItemDeletionException('Purged user cannot be undeleted.') + super(UserManager, self).undelete(user, flush=flush) + + def purge(self, user, flush=True): + """Purge the given user. They must have the deleted flag already.""" + if not self.app.config.allow_user_deletion: + raise exceptions.ConfigDoesNotAllowException('The configuration of this Galaxy instance does not allow admins to delete or purge users.') + if not user.deleted: + raise exceptions.MessageException('User \'%s\' has not been deleted, so they cannot be purged.' % user.email) + private_role = self.app.security_agent.get_private_user_role(user) + # Delete History + for active_history in user.active_histories: + self.session().refresh(active_history) + for hda in active_history.active_datasets: + # Delete HistoryDatasetAssociation + hda.deleted = True + self.session().add(hda) + active_history.deleted = True + self.session().add(active_history) + # Delete UserGroupAssociations + for uga in user.groups: + self.session().delete(uga) + # Delete UserRoleAssociations EXCEPT FOR THE PRIVATE ROLE + for ura in user.roles: + if ura.role_id != private_role.id: + self.session().delete(ura) + # Delete UserAddresses + for address in user.addresses: + self.session().delete(address) + compliance_log = logging.getLogger('COMPLIANCE') + compliance_log.info('delete-user-event: %s' % user.username) + # Maybe there is some case in the future where an admin needs + # to prove that a user was using a server for some reason (e.g. + # a court case.) So we make this painfully hard to recover (and + # not immediately reversable) in line with GDPR, but still + # leave open the possibility to prove someone was part of the + # server just in case. By knowing the exact email + approximate + # time of deletion, one could run through hashes for every + # second of the surrounding days/weeks. + pseudorandom_value = str(int(time.time())) + # Replace email + username with a (theoretically) unreversable + # hash. If provided with the username we can probably re-hash + # to identify if it is needed for some reason. + # + # Deleting multiple times will re-hash the username/email + email_hash = new_secure_hash(user.email + pseudorandom_value) + uname_hash = new_secure_hash(user.username + pseudorandom_value) + # We must also redact username + for role in user.all_roles(): + if self.app.config.redact_username_during_deletion: + role.name = role.name.replace(user.username, uname_hash) + role.description = role.description.replace(user.username, uname_hash) + + 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) + user.email = email_hash + user.username = uname_hash + # Redact user addresses as well + if self.app.config.redact_user_address_during_deletion: + user_addresses = self.session().query(self.app.model.UserAddress) \ + .filter(self.app.model.UserAddress.user_id == user.id).all() + for addr in user_addresses: + addr.desc = new_secure_hash(addr.desc + pseudorandom_value) + addr.name = new_secure_hash(addr.name + pseudorandom_value) + addr.institution = new_secure_hash(addr.institution + pseudorandom_value) + addr.address = new_secure_hash(addr.address + pseudorandom_value) + addr.city = new_secure_hash(addr.city + pseudorandom_value) + addr.state = new_secure_hash(addr.state + pseudorandom_value) + addr.postal_code = new_secure_hash(addr.postal_code + pseudorandom_value) + addr.country = new_secure_hash(addr.country + pseudorandom_value) + addr.phone = new_secure_hash(addr.phone + pseudorandom_value) + self.session().add(addr) + # Purge the user + super(UserManager, self).purge(user, flush=flush) def _error_on_duplicate_email(self, email): """ diff --git a/lib/galaxy/webapps/galaxy/api/users.py b/lib/galaxy/webapps/galaxy/api/users.py index c394af5824f..01546f7ef03 100644 --- a/lib/galaxy/webapps/galaxy/api/users.py +++ b/lib/galaxy/webapps/galaxy/api/users.py @@ -168,6 +168,8 @@ class UserAPIController(BaseAPIController, UsesTagsMixin, CreatesApiKeysMixin, B if not trans.user_is_admin: assert trans.user == user assert not user.deleted + except exceptions.ItemDeletionException: + raise except Exception: raise exceptions.RequestParameterInvalidException('Invalid user id specified', id=id) return self.user_serializer.serialize_to_view(user, view='detailed') @@ -240,19 +242,28 @@ class UserAPIController(BaseAPIController, UsesTagsMixin, CreatesApiKeysMixin, B :param purge: (optional) if True, purge the user :type purge: bool """ - if not trans.app.config.allow_user_deletion: - raise exceptions.ConfigDoesNotAllowException('The configuration of this Galaxy instance does not allow admins to delete users.') + user = self.get_user(trans, id) purge = util.string_as_bool(kwd.get('purge', False)) if purge: - raise exceptions.NotImplemented('Purge option has not been implemented yet') - user = self.get_user(trans, id) - self.user_manager.delete(user) + log.debug("Purging user %s" % user) + self.user_manager.purge(user) + else: + self.user_manager.delete(user) return self.user_serializer.serialize_to_view(user, view='detailed') @expose_api @web.require_admin - def undelete(self, trans, **kwd): - raise exceptions.NotImplemented() + def undelete(self, trans, id, **kwd): + """ + POST /api/users/deleted/{id}/undelete + Undelete the user with the given ``id`` + + :param id: the encoded id of the user to be undeleted + :type id: str + """ + user = self.get_user(trans, id) + self.user_manager.undelete(user) + return self.user_serializer.serialize_to_view(user, view='detailed') # TODO: move to more basal, common resource than this def anon_user_api_value(self, trans): diff --git a/lib/galaxy/webapps/galaxy/controllers/admin.py b/lib/galaxy/webapps/galaxy/controllers/admin.py index 0dc8a1930ab..8dd5ad54242 100644 --- a/lib/galaxy/webapps/galaxy/controllers/admin.py +++ b/lib/galaxy/webapps/galaxy/controllers/admin.py @@ -1,7 +1,6 @@ import imp import logging import os -import time from collections import OrderedDict from datetime import datetime, timedelta from string import punctuation as PUNCTUATION @@ -22,7 +21,6 @@ from galaxy.util import ( sanitize_text, url_get ) -from galaxy.util.hash_util import new_secure_hash from galaxy.web import url_for from galaxy.web.framework.helpers import grids, time_ago from galaxy.web.params import QuotaParamParser @@ -37,7 +35,6 @@ from tool_shed.util.web_util import escape log = logging.getLogger(__name__) -compliance_log = logging.getLogger('COMPLIANCE') class UserListGrid(grids.Grid): @@ -160,8 +157,8 @@ class UserListGrid(grids.Grid): grids.GridOperation("Generate New API Key", allow_multiple=False, async_compatible=True) - ] + standard_filters = [ grids.GridColumnFilter("Active", args=dict(deleted=False)), grids.GridColumnFilter("Deleted", args=dict(deleted=True, purged=False)), @@ -460,6 +457,7 @@ class QuotaListGrid(grids.Grid): standard_filters = [ grids.GridColumnFilter("Active", args=dict(deleted=False)), grids.GridColumnFilter("Deleted", args=dict(deleted=True)), + grids.GridColumnFilter("Purged", args=dict(purged=True)), grids.GridColumnFilter("All", args=dict(deleted='All')) ] num_rows_per_page = 50 @@ -527,12 +525,13 @@ class AdminGalaxy(controller.JSAppLauncher, AdminActions, UsesQuotaMixin, QuotaP group_list_grid = GroupListGrid() quota_list_grid = QuotaListGrid() tool_version_list_grid = ToolVersionListGrid() - delete_operation = grids.GridOperation("Delete", condition=(lambda item: not item.deleted), allow_multiple=True) + delete_operation = grids.GridOperation("Delete", condition=(lambda item: not item.deleted and not item.purged), allow_multiple=True) undelete_operation = grids.GridOperation("Undelete", condition=(lambda item: item.deleted and not item.purged), allow_multiple=True) purge_operation = grids.GridOperation("Purge", condition=(lambda item: item.deleted and not item.purged), allow_multiple=True) impersonate_operation = grids.GridOperation( "Impersonate", url_args=dict(controller="admin", action="impersonate"), + condition=(lambda item: not item.deleted and not item.purged), allow_multiple=False ) @@ -1411,61 +1410,9 @@ class AdminGalaxy(controller.JSAppLauncher, AdminActions, UsesQuotaMixin, QuotaP message = 'Deleted %d users: ' % len(ids) for user_id in ids: user = get_user(trans, user_id) - user.deleted = True - - compliance_log.info('delete-user-event: %s' % user_id) - - # Maybe there is some case in the future where an admin needs - # to prove that a user was using a server for some reason (e.g. - # a court case.) So we make this painfully hard to recover (and - # not immediately reversable) in line with GDPR, but still - # leave open the possibility to prove someone was part of the - # server just in case. By knowing the exact email + approximate - # time of deletion, one could run through hashes for every - # second of the surrounding days/weeks. - pseudorandom_value = str(int(time.time())) - # Replace email + username with a (theoretically) unreversable - # hash. If provided with the username we can probably re-hash - # to identify if it is needed for some reason. - # - # Deleting multiple times will re-hash the username/email - email_hash = new_secure_hash(user.email + pseudorandom_value) - uname_hash = new_secure_hash(user.username + pseudorandom_value) - - # We must also redact username - for role in user.all_roles(): - if self.app.config.redact_username_during_deletion: - role.name = role.name.replace(user.username, uname_hash) - role.description = role.description.replace(user.username, uname_hash) - - 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) - - if self.app.config.redact_email_during_deletion: - user.email = email_hash - if self.app.config.redact_username_during_deletion: - user.username = uname_hash - - # Redact user addresses as well - if self.app.config.redact_user_address_during_deletion: - user_addresses = trans.sa_session.query(trans.app.model.UserAddress) \ - .filter(trans.app.model.UserAddress.user_id == user.id).all() - - for addr in user_addresses: - addr.desc = new_secure_hash(addr.desc + pseudorandom_value) - addr.name = new_secure_hash(addr.name + pseudorandom_value) - addr.institution = new_secure_hash(addr.institution + pseudorandom_value) - addr.address = new_secure_hash(addr.address + pseudorandom_value) - addr.city = new_secure_hash(addr.city + pseudorandom_value) - addr.state = new_secure_hash(addr.state + pseudorandom_value) - addr.postal_code = new_secure_hash(addr.postal_code + pseudorandom_value) - addr.country = new_secure_hash(addr.country + pseudorandom_value) - addr.phone = new_secure_hash(addr.phone + pseudorandom_value) - trans.sa_session.add(addr) - - trans.sa_session.add(user) - trans.sa_session.flush() + # Actually do the delete + self.user_manager.delete(user) + # Accumulate messages for the return message message += ' %s ' % user.email return (message, 'done') @@ -1474,12 +1421,9 @@ class AdminGalaxy(controller.JSAppLauncher, AdminActions, UsesQuotaMixin, QuotaP undeleted_users = "" for user_id in ids: user = get_user(trans, user_id) - if not user.deleted: - message = 'User \'%s\' has not been deleted, so it cannot be undeleted.' % user.email - return (message, 'error') - user.deleted = False - trans.sa_session.add(user) - trans.sa_session.flush() + # Actually do the undelete + self.user_manager.undelete(user) + # Count and accumulate messages to return to the admin panel count += 1 undeleted_users += ' %s' % user.email message = 'Undeleted %d users: %s' % (count, undeleted_users) @@ -1500,33 +1444,8 @@ class AdminGalaxy(controller.JSAppLauncher, AdminActions, UsesQuotaMixin, QuotaP message = 'Purged %d users: ' % len(ids) for user_id in ids: user = get_user(trans, user_id) - if not user.deleted: - return ('User \'%s\' has not been deleted, so it cannot be purged.' % user.email, 'error') - private_role = trans.app.security_agent.get_private_user_role(user) - # Delete History - for h in user.active_histories: - trans.sa_session.refresh(h) - for hda in h.active_datasets: - # Delete HistoryDatasetAssociation - hda.deleted = True - trans.sa_session.add(hda) - h.deleted = True - trans.sa_session.add(h) - # Delete UserGroupAssociations - for uga in user.groups: - trans.sa_session.delete(uga) - # Delete UserRoleAssociations EXCEPT FOR THE PRIVATE ROLE - for ura in user.roles: - if ura.role_id != private_role.id: - trans.sa_session.delete(ura) - # Delete UserAddresses - for address in user.addresses: - trans.sa_session.delete(address) - # Purge the user - user.purged = True - trans.sa_session.add(user) - trans.sa_session.flush() - message += '%s ' % user.email + self.user_manager.purge(user) + message += '\t%s\n ' % user.email return (message, 'done') def _recalculate_user(self, trans, user_id): diff --git a/test/api/test_users.py b/test/api/test_users.py index f1a98a121e5..2d05e0d4219 100644 --- a/test/api/test_users.py +++ b/test/api/test_users.py @@ -10,6 +10,9 @@ from base import api # noqa: I100,I202 from base.populators import skip_without_tool TEST_USER_EMAIL = "user_for_users_index_test@bx.psu.edu" +TEST_USER_EMAIL_DELETE = "user_for_delete_test@bx.psu.edu" +TEST_USER_EMAIL_PURGE = "user_for_purge_test@bx.psu.edu" +TEST_USER_EMAIL_UNDELETE = "user_for_undelete_test@bx.psu.edu" class UsersApiTestCase(api.ApiTestCase): @@ -74,6 +77,36 @@ class UsersApiTestCase(api.ApiTestCase): update_json = update_response.json() self.assertEqual(update_json['username'], new_name) + def test_delete_user(self): + user = self._setup_user(TEST_USER_EMAIL_DELETE) + self._delete("users/%s" % user["id"], admin=True) + updated_user = self._get("users/deleted/%s" % user['id'], admin=True).json() + assert updated_user['deleted'] is True, updated_user + + def test_purge_user(self): + """Delete user and then purge them.""" + user = self._setup_user(TEST_USER_EMAIL_PURGE) + response = self._delete("users/%s" % user["id"], admin=True) + self._assert_status_code_is_ok(response) + data = dict(purge="True") + response = self._delete("users/%s" % user["id"], data=data, admin=True) + self._assert_status_code_is_ok(response) + payload = {'deleted': "True"} + purged_user = self._get("users/%s" % user['id'], payload, admin=True).json() + assert purged_user['deleted'] is True, purged_user + assert purged_user['purged'] is True, purged_user + + def test_undelete_user(self): + """Delete user and then undelete them.""" + user = self._setup_user(TEST_USER_EMAIL_UNDELETE) + self._delete("users/%s" % user["id"], admin=True) + payload = {'deleted': "True"} + deleted_user = self._get("users/%s" % user['id'], payload, admin=True).json() + assert deleted_user['deleted'] is True, deleted_user + self._post("users/deleted/%s/undelete" % user["id"], admin=True) + undeleted_user = self._get("users/%s" % user['id'], admin=True).json() + assert undeleted_user['deleted'] is False, undeleted_user + def test_information(self): user = self._setup_user(TEST_USER_EMAIL) url = self.__url("information/inputs", user)