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
86 changes: 85 additions & 1 deletion src/sentry/api/endpoints/user_details.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,26 @@
from datetime import datetime

import pytz
import logging

from django.conf import settings
from django.utils.translation import ugettext_lazy as _
from django.contrib.auth import logout
from rest_framework import serializers, status
from rest_framework.response import Response

from sentry import roles
from sentry.api import client
from sentry.api.bases.user import UserEndpoint
from sentry.api.decorators import sudo_required
from sentry.api.serializers import serialize
from sentry.api.serializers.models.user import DetailedUserSerializer
from sentry.api.serializers.rest_framework import ListField
from sentry.auth.superuser import is_active_superuser
from sentry.constants import LANGUAGES
from sentry.models import User, UserOption
from sentry.models import Organization, OrganizationMember, OrganizationStatus, User, UserOption

delete_logger = logging.getLogger('sentry.deletions.ui')


def _get_timezone_choices():
Expand Down Expand Up @@ -90,6 +99,10 @@ class Meta:
# write_only_fields = ('password',)


class OrganizationsSerializer(serializers.Serializer):
organizations = ListField(child=serializers.CharField(), required=True)


class UserDetailsEndpoint(UserEndpoint):
def get(self, request, user):
"""
Expand Down Expand Up @@ -153,3 +166,74 @@ def put(self, request, user):
user = serializer.save()

return Response(serialize(user, request.user, DetailedUserSerializer()))

@sudo_required
def delete(self, request, user):
"""
Delete User Account

Also removes organizations if they are an owner
:pparam string user_id: user id
:param list organizations: List of organization ids to remove
Copy link
Contributor

Choose a reason for hiding this comment

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

i'm wondering if we should actually have people send a list or if we should just delete all of the ones that should be deleted automatically? do we allow people to delete their account without removing the single owner orgs?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah I was thinking the same thing - the existing behavior is that single owner orgs get listed and always get deleted.

If it is an org w/ multiple owners, you can choose to delete it (but maybe it's better to not be able to delete those at all?)

Copy link
Contributor

Choose a reason for hiding this comment

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

oh interesting. i didn't realize that you could also delete non-single owner orgs from here. hmmmm i mean i think in the interest of plowing through the setting stuff, it's probably a bad idea to change behavior, but seems like it'd be better if we forced people through the delete org flow when it's not single owner since it's so destructive. soo i'll defer to you/maybe you could check with chris to see if he's against removing the ability to delete those orgs from here?

Copy link
Member

Choose a reason for hiding this comment

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

I'd make it explicit (send the list of orgs to remove). Otherwise it should probably:

  • delete orgs explicitly selected
  • delete orgs with no owners
  • retain orgs with owners and not selected

Org deletion at least queues for 24 hours and emails all owners a way to undo it.

:auth required:
"""

serializer = OrganizationsSerializer(data=request.DATA)

if not serializer.is_valid():
return Response(status=status.HTTP_400_BAD_REQUEST)

# from `frontend/remove_account.py`
org_list = Organization.objects.filter(
member_set__role__in=[x.id for x in roles.with_scope('org:admin')],
member_set__user=user,
status=OrganizationStatus.VISIBLE,
)

org_results = []
for org in org_list:
org_results.append({
'organization': org,
'single_owner': org.has_single_owner(),
})

avail_org_slugs = set([o['organization'].slug for o in org_results])
orgs_to_remove = set(serializer.object.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='/organizations/{}/'.format(org_slug),
request=request,
is_sudo=True)

remaining_org_ids = [
o.id for o in org_list if o.slug in avail_org_slugs.difference(orgs_to_remove)
]

if remaining_org_ids:
OrganizationMember.objects.filter(
organization__in=remaining_org_ids,
user=request.user,
).delete()

User.objects.filter(
id=request.user.id,
).update(
is_active=False,
)

logout(request)

return Response(status=status.HTTP_204_NO_CONTENT)
56 changes: 55 additions & 1 deletion tests/sentry/api/endpoints/test_user_details.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

from django.core.urlresolvers import reverse

from sentry.models import User, UserOption
from sentry.models import Organization, OrganizationStatus, User, UserOption
from sentry.testutils import APITestCase


Expand Down Expand Up @@ -186,3 +186,57 @@ def test_change_username_when_same(self):

assert user.email == 'new@example.com'
assert user.username == 'new@example.com'

def test_close_account(self):
self.login_as(user=self.user)
org_single_owner = self.create_organization(name="A", owner=self.user)
user2 = self.create_user(email="user2@example.com")
org_with_other_owner = self.create_organization(name="B", owner=self.user)
org_as_other_owner = self.create_organization(name="C", owner=user2)
not_owned_org = self.create_organization(name="D", owner=user2)

self.create_member(
user=user2,
organization=org_with_other_owner,
role='owner',
)

self.create_member(
user=self.user,
organization=org_as_other_owner,
role='owner',
)

url = reverse(
'sentry-api-0-user-details', kwargs={
'user_id': self.user.id,
}
)

# test validations
response = self.client.delete(url, data={
})
assert response.status_code == 400
response = self.client.delete(url, data={
'organizations': None
})
assert response.status_code == 400

# test actual delete
response = self.client.delete(url, data={
'organizations': [org_with_other_owner.slug, org_as_other_owner.slug, not_owned_org.slug]
})

# deletes org_single_owner even though it wasn't specified in array
# because it has a single owner
assert Organization.objects.get(
id=org_single_owner.id).status == OrganizationStatus.PENDING_DELETION
# should delete org_with_other_owner, and org_as_other_owner
assert Organization.objects.get(
id=org_with_other_owner.id).status == OrganizationStatus.PENDING_DELETION
assert Organization.objects.get(
id=org_as_other_owner.id).status == OrganizationStatus.PENDING_DELETION
# should NOT delete `not_owned_org`
assert Organization.objects.get(id=not_owned_org.id).status == OrganizationStatus.ACTIVE

assert response.status_code == 204