From 828adc1f06835f1d1f95a7caedee8bc8741c7cfa Mon Sep 17 00:00:00 2001 From: Anton Krytskyi Date: Thu, 29 Jan 2026 03:41:45 +0200 Subject: [PATCH 1/2] update gdpr delete logic --- admin/users/views.py | 7 +++++ osf/models/user.py | 69 ++++++++++++++---------------------------- osf_tests/test_user.py | 54 ++++++++++++++++++++++++++++----- 3 files changed, 75 insertions(+), 55 deletions(-) diff --git a/admin/users/views.py b/admin/users/views.py index 1584c78158e..85da88cea62 100644 --- a/admin/users/views.py +++ b/admin/users/views.py @@ -216,6 +216,13 @@ def post(self, request, *args, **kwargs): user = self.get_object() user.gdpr_delete() user.save() + + # Bulk 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() + messages.success(request, f'User {user._id} was successfully GDPR deleted') update_admin_log( user_id=self.request.user.id, diff --git a/osf/models/user.py b/osf/models/user.py index 72ca2b8f3f8..99558175886 100644 --- a/osf/models/user.py +++ b/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 @@ -2015,14 +2016,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( @@ -2033,39 +2032,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. @@ -2090,18 +2056,23 @@ 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: @@ -2109,7 +2080,11 @@ def _validate_and_remove_resource_for_gdpr_delete(self, resources, hard_delete): 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): ''' diff --git a/osf_tests/test_user.py b/osf_tests/test_user.py index a2fc5b0e92a..4cec8940622 100644 --- a/osf_tests/test_user.py +++ b/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,7 +2215,7 @@ 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') @@ -2192,20 +2223,27 @@ def test_cant_gdpr_delete_multiple_contributors_registrations(self, user, regist 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 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) From fde6f1bb26dc7e295e9417f02affdbd014a23449 Mon Sep 17 00:00:00 2001 From: Anton Krytskyi Date: Thu, 29 Jan 2026 15:28:32 +0200 Subject: [PATCH 2/2] fix test; better error handling --- admin/users/views.py | 31 ++++++++++++++++--------------- osf_tests/test_user.py | 1 + 2 files changed, 17 insertions(+), 15 deletions(-) diff --git a/admin/users/views.py b/admin/users/views.py index 85da88cea62..814cc2327d5 100644 --- a/admin/users/views.py +++ b/admin/users/views.py @@ -216,24 +216,25 @@ def post(self, request, *args, **kwargs): user = self.get_object() user.gdpr_delete() user.save() - - # Bulk 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() - - 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()) diff --git a/osf_tests/test_user.py b/osf_tests/test_user.py index 4cec8940622..97d20db65d7 100644 --- a/osf_tests/test_user.py +++ b/osf_tests/test_user.py @@ -2228,6 +2228,7 @@ def test_gdpr_delete_removes_user_from_shared_registrations(self, user, registra assert registration.contributors.count() == 1 assert registration.contributors.first() == other_user + assert not registration.is_deleted assert user.deleted is not None def test_gdpr_delete_removes_user_from_shared_preprints(self, user, preprint):