[ENG-9002] Unable to GDPR delete users with Registrations or Preprints (CenterForOpenScience#11568)

* update gdpr delete logic

* fix test; better error handling

Author: antkryt

admin/users/views.py

@@ -216,17 +216,25 @@ def post(self, request, *args, **kwargs):
             user = self.get_object()
             user.gdpr_delete()
             user.save()
-            messages.success(request, f'User {user._id} was successfully GDPR deleted')
-            update_admin_log(
-                user_id=self.request.user.id,
-                object_id=user.pk,
-                object_repr='User',
-                message=f'User {user._id} was successfully GDPR deleted',
-                action_flag=USER_GDPR_DELETED
-            )
         except UserStateError as e:
             messages.warning(request, str(e))
 
+        messages.success(request, f'User {user._id} was successfully GDPR deleted')
+
+        # Update SHARE for all public resources
+        for node in user.nodes.filter(is_public=True, is_deleted=False):
+            node.update_search()
+        for preprint in user.preprints.filter(is_public=True, deleted__isnull=True):
+            preprint.update_search()
+
+        update_admin_log(
+            user_id=self.request.user.id,
+            object_id=user.pk,
+            object_repr='User',
+            message=f'User {user._id} was successfully GDPR deleted',
+            action_flag=USER_GDPR_DELETED
+        )
+
         return redirect(self.get_success_url())
 
 

osf/models/user.py

@@ -18,6 +18,7 @@
 from django.contrib.auth.base_user import AbstractBaseUser, BaseUserManager
 from django.contrib.auth.hashers import check_password
 from django.contrib.auth.models import PermissionsMixin
+from django.core.exceptions import FieldDoesNotExist
 from django.dispatch import receiver
 from django.db import models
 from django.db.models import Count, Exists, OuterRef
@@ -2020,14 +2021,12 @@ def gdpr_delete(self):
         """
         Complies with GDPR guidelines by disabling the account and removing identifying information.
         """
-
-        # Check if user has something intentionally public, like preprints or registrations
-        self._validate_no_public_entities()
-
-        # Check if user has any non-registration AbstractNodes or DraftRegistrations that they might still share with
-        # other contributors
         self._validate_and_remove_resource_for_gdpr_delete(
-            self.nodes.exclude(type='osf.registration'),  # Includes DraftNodes and other typed nodes
+            self.nodes.all(),
+            hard_delete=False
+        )
+        self._validate_and_remove_resource_for_gdpr_delete(
+            self.preprints.all(),
             hard_delete=False
         )
         self._validate_and_remove_resource_for_gdpr_delete(
@@ -2038,39 +2037,6 @@ def gdpr_delete(self):
         # Finally delete the user's info.
         self._clear_identifying_information()
 
-    def _validate_no_public_entities(self):
-        """
-        Ensure that the user doesn't have any public facing resources like Registrations or Preprints
-        that would be left with other contributors after this deletion.
-
-        Allow GDPR deletion if the user is the sole contributor on a public Registration or Preprint.
-        """
-        from osf.models import Preprint, AbstractNode
-
-        registrations_with_others = AbstractNode.objects.annotate(
-            contrib_count=Count('_contributors', distinct=True),
-        ).filter(
-            _contributors=self,
-            deleted__isnull=True,
-            type='osf.registration',
-            contrib_count__gt=1
-        ).exists()
-
-        if registrations_with_others:
-            raise UserStateError('You cannot delete this user because they have one or more registrations.')
-
-        preprints_with_others = Preprint.objects.annotate(
-            contrib_count=Count('_contributors', distinct=True),
-        ).filter(
-            _contributors=self,
-            ever_public=True,
-            deleted__isnull=True,
-            contrib_count__gt=1
-        ).exists()
-
-        if preprints_with_others:
-            raise UserStateError('You cannot delete this user because they have one or more preprints.')
-
     def _validate_and_remove_resource_for_gdpr_delete(self, resources, hard_delete):
         """
         This method ensures a user's resources are properly deleted of using during GDPR delete request.
@@ -2095,26 +2061,35 @@ def _validate_and_remove_resource_for_gdpr_delete(self, resources, hard_delete):
         )
 
         shared_resources = resources.exclude(id__in=personal_resources.values_list('id'))
-        for node in shared_resources:
-            self._validate_admin_status_for_gdpr_delete(node)
-            self._validate_addons_for_gdpr_delete(node)
+        for resource in shared_resources:
+            self._validate_admin_status_for_gdpr_delete(resource)
+            self._validate_addons_for_gdpr_delete(resource)
 
         for resource in shared_resources.all():
             logger.info(f'Removing {self._id} as a contributor to {resource.__class__.__name__} (pk:{resource.pk})...')
             resource.remove_contributor(self, auth=Auth(self), log=False)
+            if getattr(resource, 'is_public', False) and hasattr(resource, 'update_search'):
+                resource.update_search()
 
-        # Delete all personal entities (excluding public registrations)
+        # Delete all personal non-public entities
         personal_to_delete = personal_resources
-        if hasattr(model, 'is_public') and hasattr(model, 'type'):
-            personal_to_delete = personal_to_delete.exclude(is_public=True, type='osf.registration')
+        try:
+            if model._meta.get_field('is_public'):
+                personal_to_delete = personal_to_delete.exclude(is_public=True)
+        except FieldDoesNotExist:
+            pass
 
         for entity in personal_to_delete.all():
             if hard_delete:
                 logger.info(f'Hard-deleting {entity.__class__.__name__} (pk: {entity.pk})...')
                 entity.delete()
             else:
                 logger.info(f'Soft-deleting {entity.__class__.__name__} (pk: {entity.pk})...')
-                entity.remove_node(auth=Auth(self))
+                if hasattr(entity, 'remove_node'):
+                    entity.remove_node(auth=Auth(self))
+                else:
+                    entity.is_deleted = True
+                    entity.save()
 
     def _clear_identifying_information(self):
         '''

osf_tests/test_user.py

@@ -2143,6 +2143,37 @@ def project_user_is_only_admin(self, user):
         project.save()
         return project
 
+    @mock.patch('osf.models.node.AbstractNode.update_search')
+    def test_gdpr_delete_triggers_share_update_for_public_shared_nodes(
+            self, mock_update_search, user, project_with_two_admins):
+        project_with_two_admins.is_public = True
+        project_with_two_admins.save()
+
+        user.gdpr_delete()
+
+        assert mock_update_search.called
+
+    @mock.patch('osf.models.node.AbstractNode.update_search')
+    def test_gdpr_delete_does_not_trigger_share_update_for_non_public_shared_nodes(
+            self, mock_update_search, user, project_with_two_admins):
+        assert project_with_two_admins.is_public is False
+
+        user.gdpr_delete()
+
+        assert not mock_update_search.called
+
+    @mock.patch('osf.models.preprint.Preprint.update_search')
+    def test_gdpr_delete_triggers_share_update_for_public_shared_preprints(
+            self, mock_update_search, user, preprint):
+        other_user = AuthUserFactory()
+        preprint.add_contributor(other_user, auth=Auth(user), permissions='admin')
+        preprint.save()
+        assert preprint.is_public is True
+
+        user.gdpr_delete()
+
+        assert mock_update_search.called
+
     def test_can_gdpr_delete(self, user):
         user.social = ['fake social']
         user.schools = ['fake schools']
@@ -2184,28 +2215,36 @@ def test_can_gdpr_delete_shared_draft_registration_with_multiple_admins(self, us
         assert draft_registrations.contributors.get() == other_admin
         assert user.nodes.filter(deleted__isnull=True).count() == 0
 
-    def test_cant_gdpr_delete_multiple_contributors_registrations(self, user, registration):
+    def test_gdpr_delete_removes_user_from_shared_registrations(self, user, registration):
         registration.is_public = True
         other_user = AuthUserFactory()
         registration.add_contributor(other_user, auth=Auth(user), permissions='admin')
         registration.save()
 
         assert registration.contributors.count() == 2
 
-        with pytest.raises(UserStateError) as exc_info:
-            user.gdpr_delete()
+        user.gdpr_delete()
+        registration.reload()
 
-        assert exc_info.value.args[0] == 'You cannot delete this user because they have one or more registrations.'
+        assert registration.contributors.count() == 1
+        assert registration.contributors.first() == other_user
+        assert not registration.is_deleted
+        assert user.deleted is not None
 
-    def test_cant_gdpr_delete_multiple_contributors_preprints(self, user, preprint):
+    def test_gdpr_delete_removes_user_from_shared_preprints(self, user, preprint):
         other_user = AuthUserFactory()
         preprint.add_contributor(other_user, auth=Auth(user), permissions='admin')
         preprint.save()
 
-        with pytest.raises(UserStateError) as exc_info:
-            user.gdpr_delete()
+        assert preprint.contributors.count() == 2
+
+        user.gdpr_delete()
+        preprint.reload()
 
-        assert exc_info.value.args[0] == 'You cannot delete this user because they have one or more preprints.'
+        assert preprint.contributors.count() == 1
+        assert preprint.contributors.first() == other_user
+        assert not preprint.is_deleted
+        assert user.deleted is not None
 
     def test_can_gdpr_delete_sole_contributor_registration(self, user):
         registration = RegistrationFactory(creator=user)