Repository: desec-io/desec-stack
Issue: Deleting a user owning domains causes error 500

- We do not want to allow account deletion if the user still has domains.
- Currently, such cases raise error 500 because the deletion breaks due to foreign key constraint violation on the database level.
- We should catch the exception and return a reasonable status code, e.g. 409 Conflict

@nils-wisiol How should we do this? I suppose the exception needs to be caught/transformed into another one in `User.delete()`, and maybe caught again in the view. Also, do we want to check this condition already when the user requests a deletion code (at issue time, not at use time)?

Changed files:
api/desecapi/views.py, api/desecapi/tests/test_user_management.py

diff --git a/api/desecapi/tests/test_user_management.py b/api/desecapi/tests/test_user_management.py
index 4f0d433..fece0dc 100644
--- a/api/desecapi/tests/test_user_management.py
+++ b/api/desecapi/tests/test_user_management.py
@@ -323,6 +323,13 @@ class UserManagementTestCase(DesecTestCase, PublicSuffixMockMixin):
             status_code=status.HTTP_202_ACCEPTED
         )
 
+    def assertDeleteAccountFailureStillHasDomainsResponse(self, response):
+        return self.assertContains(
+            response=response,
+            text="To delete your user account, first delete all of your domains.",
+            status_code=status.HTTP_409_CONFLICT
+        )
+
     def assertDeleteAccountVerificationSuccessResponse(self, response):
         return self.assertContains(
             response=response,
@@ -748,6 +755,17 @@ class HasUserAccountTestCase(UserManagementTestCase):
         )
         self.assertNoEmailSent()
 
+    def test_delete_account_domains_present(self):
+        user = User.objects.get(email=self.email)
+        domain = self.create_domain(owner=user)
+        self.assertDeleteAccountFailureStillHasDomainsResponse(self.delete_account(self.email, self.password))
+        domain.delete()
+        confirmation_link = self._start_delete_account()
+        domain = self.create_domain(owner=user)
+        self.assertDeleteAccountFailureStillHasDomainsResponse(response=self.client.verify(confirmation_link))
+        domain.delete()
+        self._finish_delete_account(confirmation_link)
+
     def test_reset_password_password_strip(self):
         password = ' %s ' % self.random_password()
         self._test_reset_password(self.email, password)
diff --git a/api/desecapi/views.py b/api/desecapi/views.py
index f539d3b..2b3a561 100644
--- a/api/desecapi/views.py
+++ b/api/desecapi/views.py
@@ -437,8 +437,14 @@ class AccountView(generics.RetrieveAPIView):
 class AccountDeleteView(generics.GenericAPIView):
     authentication_classes = (auth.EmailPasswordPayloadAuthentication,)
     permission_classes = (IsAuthenticated,)
+    response_still_has_domains = Response(
+        data={'detail': 'To delete your user account, first delete all of your domains.'},
+        status=status.HTTP_409_CONFLICT,
+    )
 
     def post(self, request, *args, **kwargs):
+        if self.request.user.domains.exists():
+            return self.response_still_has_domains
         action = models.AuthenticatedDeleteUserAction(user=self.request.user)
         verification_code = serializers.AuthenticatedDeleteUserActionSerializer(action).data['code']
         request.user.send_email('delete-user', context={
@@ -630,6 +636,11 @@ class AuthenticatedDeleteUserActionView(AuthenticatedActionView):
     http_method_names = ['get']
     serializer_class = serializers.AuthenticatedDeleteUserActionSerializer
 
+    def take_action(self):
+        if self.request.user.domains.exists():
+            return AccountDeleteView.response_still_has_domains
+        return super().take_action()
+
     def finalize(self):
         return Response({'detail': 'All your data has been deleted. Bye bye, see you soon! <3'})