Skip to content

Commit 43ed1e1

Browse files
ref(api): Update DELETE users/ to support hard deleting (#14068)
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.
1 parent 13649ec commit 43ed1e1

File tree

2 files changed

+77
-19
lines changed

2 files changed

+77
-19
lines changed

src/sentry/api/endpoints/user_details.py

Lines changed: 33 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -97,8 +97,9 @@ class Meta:
9797
# write_only_fields = ('password',)
9898

9999

100-
class OrganizationsSerializer(serializers.Serializer):
100+
class DeleteUserSerializer(serializers.Serializer):
101101
organizations = ListField(child=serializers.CharField(required=False), required=True)
102+
hardDelete = serializers.BooleanField(required=False)
102103

103104

104105
class UserDetailsEndpoint(UserEndpoint):
@@ -135,7 +136,8 @@ def put(self, request, user):
135136
serializer_cls = UserSerializer
136137
serializer = serializer_cls(user, data=request.data, partial=True)
137138

138-
serializer_options = UserOptionsSerializer(data=request.data.get('options', {}), partial=True)
139+
serializer_options = UserOptionsSerializer(
140+
data=request.data.get('options', {}), partial=True)
139141

140142
# This serializer should NOT include privileged fields e.g. password
141143
if not serializer.is_valid() or not serializer_options.is_valid():
@@ -170,11 +172,12 @@ def delete(self, request, user):
170172
171173
Also removes organizations if they are an owner
172174
:pparam string user_id: user id
175+
:param boolean hard_delete: Completely remove the user from the database (requires super user)
173176
:param list organizations: List of organization ids to remove
174177
:auth required:
175178
"""
176179

177-
serializer = OrganizationsSerializer(data=request.data)
180+
serializer = DeleteUserSerializer(data=request.data)
178181

179182
if not serializer.is_valid():
180183
return Response(status=status.HTTP_400_BAD_REQUEST)
@@ -194,20 +197,13 @@ def delete(self, request, user):
194197
})
195198

196199
avail_org_slugs = set([o['organization'].slug for o in org_results])
197-
orgs_to_remove = set(serializer.validated_data.get('organizations')).intersection(avail_org_slugs)
200+
orgs_to_remove = set(serializer.validated_data.get(
201+
'organizations')).intersection(avail_org_slugs)
198202

199203
for result in org_results:
200204
if result['single_owner']:
201205
orgs_to_remove.add(result['organization'].slug)
202206

203-
delete_logger.info(
204-
'user.deactivate',
205-
extra={
206-
'actor_id': request.user.id,
207-
'ip_address': request.META['REMOTE_ADDR'],
208-
}
209-
)
210-
211207
for org_slug in orgs_to_remove:
212208
client.delete(
213209
path=u'/organizations/{}/'.format(org_slug),
@@ -221,15 +217,33 @@ def delete(self, request, user):
221217
if remaining_org_ids:
222218
OrganizationMember.objects.filter(
223219
organization__in=remaining_org_ids,
224-
user=request.user,
220+
user=user,
225221
).delete()
226222

227-
User.objects.filter(
228-
id=request.user.id,
229-
).update(
230-
is_active=False,
231-
)
223+
logging_data = {
224+
'actor_id': request.user.id,
225+
'ip_address': request.META['REMOTE_ADDR'],
226+
}
227+
228+
hard_delete = serializer.validated_data.get('hardDelete', False)
229+
230+
# Only active superusers can hard delete accounts
231+
if hard_delete and not is_active_superuser(request):
232+
return Response(
233+
{'detail': 'Only superusers may hard delete a user account'},
234+
status=status.HTTP_403_FORBIDDEN)
235+
236+
is_current_user = request.user.id == user.id
237+
238+
if hard_delete:
239+
user.delete()
240+
delete_logger.info('user.removed', extra=logging_data)
241+
else:
242+
User.objects.filter(id=user.id).update(is_active=False)
243+
delete_logger.info('user.deactivate', extra=logging_data)
232244

233-
logout(request)
245+
# if the user deleted their own account log them out
246+
if is_current_user:
247+
logout(request)
234248

235249
return Response(status=status.HTTP_204_NO_CONTENT)

tests/sentry/api/endpoints/test_user_details.py

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -273,3 +273,47 @@ def test_close_account_no_orgs(self):
273273
id=org_single_owner.id).status == OrganizationStatus.PENDING_DELETION
274274
# should NOT delete `not_owned_org`
275275
assert Organization.objects.get(id=not_owned_org.id).status == OrganizationStatus.ACTIVE
276+
277+
def test_cannot_hard_delete_account(self):
278+
self.login_as(user=self.user)
279+
280+
url = reverse(
281+
'sentry-api-0-user-details', kwargs={
282+
'user_id': self.user.id,
283+
}
284+
)
285+
286+
# Cannot hard delete your own account
287+
response = self.client.delete(url, data={
288+
'hardDelete': True,
289+
'organizations': []
290+
})
291+
assert response.status_code == 403
292+
293+
def test_hard_delete_account(self):
294+
self.login_as(user=self.user)
295+
user2 = self.create_user(email="[email protected]")
296+
297+
url = reverse(
298+
'sentry-api-0-user-details', kwargs={
299+
'user_id': user2.id,
300+
}
301+
)
302+
303+
# failed authorization, user does not have permissios to delete another
304+
# user
305+
response = self.client.delete(url, data={
306+
'hardDelete': True,
307+
'organizations': []
308+
})
309+
assert response.status_code == 403
310+
311+
# Reauthenticate as super user to hard delete an account
312+
self.user.update(is_superuser=True)
313+
self.login_as(user=self.user, superuser=True)
314+
315+
response = self.client.delete(url, data={
316+
'hardDelete': True,
317+
'organizations': []
318+
})
319+
assert response.status_code == 204

0 commit comments

Comments
 (0)