Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 33 additions & 19 deletions src/sentry/api/endpoints/user_details.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -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():
Expand Down Expand Up @@ -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)
Expand All @@ -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),
Expand All @@ -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,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This used to just assume you were deleting yourself, this updates things so you can delete someone else (with the proper authorization of course, which happens in UserEndpoint)

).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)
44 changes: 44 additions & 0 deletions tests/sentry/api/endpoints/test_user_details.py
Original file line number Diff line number Diff line change
Expand Up @@ -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="[email protected]")

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