From 0f8e16d21a326d1efb023e0f86d42ccaf5af1d78 Mon Sep 17 00:00:00 2001 From: Evan Purkhiser Date: Wed, 17 Jul 2019 19:37:02 -0700 Subject: [PATCH] ref(api): Update DELETE users/ to support hard deleting This is a replacement for the web.frontend.admin endpoint that is used in the admin manage interface (primarily for open source sentry) to hard delete accounts. --- src/sentry/api/endpoints/user_details.py | 52 ++++++++++++------- .../sentry/api/endpoints/test_user_details.py | 44 ++++++++++++++++ 2 files changed, 77 insertions(+), 19 deletions(-) diff --git a/src/sentry/api/endpoints/user_details.py b/src/sentry/api/endpoints/user_details.py index 57ac1a9265b1a2..45285db07701d0 100644 --- a/src/sentry/api/endpoints/user_details.py +++ b/src/sentry/api/endpoints/user_details.py @@ -97,8 +97,9 @@ class Meta: # write_only_fields = ('password',) -class OrganizationsSerializer(serializers.Serializer): +class DeleteUserSerializer(serializers.Serializer): organizations = ListField(child=serializers.CharField(required=False), required=True) + hardDelete = serializers.BooleanField(required=False) class UserDetailsEndpoint(UserEndpoint): @@ -135,7 +136,8 @@ def put(self, request, user): serializer_cls = UserSerializer serializer = serializer_cls(user, data=request.data, partial=True) - serializer_options = UserOptionsSerializer(data=request.data.get('options', {}), partial=True) + serializer_options = UserOptionsSerializer( + data=request.data.get('options', {}), partial=True) # This serializer should NOT include privileged fields e.g. password if not serializer.is_valid() or not serializer_options.is_valid(): @@ -170,11 +172,12 @@ def delete(self, request, user): Also removes organizations if they are an owner :pparam string user_id: user id + :param boolean hard_delete: Completely remove the user from the database (requires super user) :param list organizations: List of organization ids to remove :auth required: """ - serializer = OrganizationsSerializer(data=request.data) + serializer = DeleteUserSerializer(data=request.data) if not serializer.is_valid(): return Response(status=status.HTTP_400_BAD_REQUEST) @@ -194,20 +197,13 @@ def delete(self, request, user): }) avail_org_slugs = set([o['organization'].slug for o in org_results]) - orgs_to_remove = set(serializer.validated_data.get('organizations')).intersection(avail_org_slugs) + orgs_to_remove = set(serializer.validated_data.get( + 'organizations')).intersection(avail_org_slugs) for result in org_results: if result['single_owner']: orgs_to_remove.add(result['organization'].slug) - delete_logger.info( - 'user.deactivate', - extra={ - 'actor_id': request.user.id, - 'ip_address': request.META['REMOTE_ADDR'], - } - ) - for org_slug in orgs_to_remove: client.delete( path=u'/organizations/{}/'.format(org_slug), @@ -221,15 +217,33 @@ def delete(self, request, user): if remaining_org_ids: OrganizationMember.objects.filter( organization__in=remaining_org_ids, - user=request.user, + user=user, ).delete() - User.objects.filter( - id=request.user.id, - ).update( - is_active=False, - ) + logging_data = { + 'actor_id': request.user.id, + 'ip_address': request.META['REMOTE_ADDR'], + } + + hard_delete = serializer.validated_data.get('hardDelete', False) + + # Only active superusers can hard delete accounts + if hard_delete and not is_active_superuser(request): + return Response( + {'detail': 'Only superusers may hard delete a user account'}, + status=status.HTTP_403_FORBIDDEN) + + is_current_user = request.user.id == user.id + + if hard_delete: + user.delete() + delete_logger.info('user.removed', extra=logging_data) + else: + User.objects.filter(id=user.id).update(is_active=False) + delete_logger.info('user.deactivate', extra=logging_data) - logout(request) + # if the user deleted their own account log them out + if is_current_user: + logout(request) return Response(status=status.HTTP_204_NO_CONTENT) diff --git a/tests/sentry/api/endpoints/test_user_details.py b/tests/sentry/api/endpoints/test_user_details.py index a556fb798b8680..ffa9316568fd5f 100644 --- a/tests/sentry/api/endpoints/test_user_details.py +++ b/tests/sentry/api/endpoints/test_user_details.py @@ -273,3 +273,47 @@ def test_close_account_no_orgs(self): id=org_single_owner.id).status == OrganizationStatus.PENDING_DELETION # should NOT delete `not_owned_org` assert Organization.objects.get(id=not_owned_org.id).status == OrganizationStatus.ACTIVE + + def test_cannot_hard_delete_account(self): + self.login_as(user=self.user) + + url = reverse( + 'sentry-api-0-user-details', kwargs={ + 'user_id': self.user.id, + } + ) + + # Cannot hard delete your own account + response = self.client.delete(url, data={ + 'hardDelete': True, + 'organizations': [] + }) + assert response.status_code == 403 + + def test_hard_delete_account(self): + self.login_as(user=self.user) + user2 = self.create_user(email="user2@example.com") + + url = reverse( + 'sentry-api-0-user-details', kwargs={ + 'user_id': user2.id, + } + ) + + # failed authorization, user does not have permissios to delete another + # user + response = self.client.delete(url, data={ + 'hardDelete': True, + 'organizations': [] + }) + assert response.status_code == 403 + + # Reauthenticate as super user to hard delete an account + self.user.update(is_superuser=True) + self.login_as(user=self.user, superuser=True) + + response = self.client.delete(url, data={ + 'hardDelete': True, + 'organizations': [] + }) + assert response.status_code == 204