diff --git a/admin_tests/nodes/test_views.py b/admin_tests/nodes/test_views.py index b63618e8426..884027821f8 100644 --- a/admin_tests/nodes/test_views.py +++ b/admin_tests/nodes/test_views.py @@ -52,6 +52,7 @@ from api_tests.share._utils import mock_update_share from osf.models.files import Folder from tests.utils import capture_notifications +from osf.models.notification_type import NotificationTypeEnum from website import settings from framework.auth.core import Auth @@ -180,8 +181,11 @@ def test_node_spam_ham_workflow_if_node_is_public(self): guid = node._id request = RequestFactory().post('/fake_path') request.user = superuser - node = handle_post_view_request(request, NodeConfirmSpamView(), node, guid) + with capture_notifications() as notifications: + node = handle_post_view_request(request, NodeConfirmSpamView(), node, guid) assert not node.is_public + assert len(notifications['emits']) == 1 + assert notifications['emits'][0]['type'] == NotificationTypeEnum.NODE_CONFIRMED_SPAM node = handle_post_view_request(request, NodeConfirmHamView(), node, guid) assert node.is_public diff --git a/admin_tests/preprints/test_views.py b/admin_tests/preprints/test_views.py index d70eff1ef35..937e47415ff 100644 --- a/admin_tests/preprints/test_views.py +++ b/admin_tests/preprints/test_views.py @@ -156,7 +156,8 @@ def test_confirm_spam(self, flagged_preprint, superuser, mock_akismet): view = views.PreprintConfirmSpamView() view = setup_view(view, request, guid=flagged_preprint._id) - view.post(request) + with assert_notification(type=NotificationTypeEnum.PREPRINT_CONFIRMED_SPAM): + view.post(request) assert flagged_preprint.is_public flagged_preprint.refresh_from_db() diff --git a/admin_tests/users/test_views.py b/admin_tests/users/test_views.py index a6ea9f07638..f77fd9b3a2f 100644 --- a/admin_tests/users/test_views.py +++ b/admin_tests/users/test_views.py @@ -247,7 +247,8 @@ def setUp(self): def test_disable_spam_user(self): settings.ENABLE_EMAIL_SUBSCRIPTIONS = False - self.view().post(self.request) + with assert_notification(type=NotificationTypeEnum.USER_SPAM_BANNED, user=self.user): + self.view().post(self.request) self.user.reload() self.public_node.reload() assert self.user.is_disabled diff --git a/notifications.yaml b/notifications.yaml index 6161abd9fc6..6bbb88178d3 100644 --- a/notifications.yaml +++ b/notifications.yaml @@ -569,6 +569,13 @@ notification_types: template: 'website/templates/new_public_project.html.mako' tests: ['osf_tests/test_node.py'] + - name: node_confirmed_spam + subject: 'Your OSF project has been flagged as spam' + __docs__: ... + object_content_type_model_name: abstractnode + template: 'website/templates/spam_resource_confirmed.html.mako' + tests: ['tests/test_spam_mixin.py'] + #### PREPRINT - name: preprint_contributor_added_preprint_node_from_osf @@ -599,6 +606,13 @@ notification_types: template: 'website/templates/contributor_added_preprints.html.mako' tests: ['api_tests/preprints/views/test_preprint_contributors_list.py'] + - name: preprint_confirmed_spam + subject: 'Your OSF preprint has been flagged as spam' + __docs__: ... + object_content_type_model_name: preprint + template: 'website/templates/spam_resource_confirmed.html.mako' + tests: ['tests/test_spam_mixin.py'] + #### SUPPORT #### Collection Submissions - name: collection_submission_removed_moderator diff --git a/osf/external/spam/tasks.py b/osf/external/spam/tasks.py index 56f7f16aa2d..6fdd31a408d 100644 --- a/osf/external/spam/tasks.py +++ b/osf/external/spam/tasks.py @@ -29,7 +29,9 @@ def reclassify_domain_references(notable_domain_id, current_note, previous_note) for item in references: item.is_triaged = current_note != NotableDomain.Note.UNKNOWN if current_note == NotableDomain.Note.EXCLUDE_FROM_ACCOUNT_CREATION_AND_CONTENT: - item.referrer.confirm_spam(save=False, domains=[domain.domain]) + # Retroactive/bulk reclassification of a domain already flagged in the past -- + # don't send a fresh "flagged as spam" email long after the fact. + item.referrer.confirm_spam(save=False, domains=[domain.domain], notify=False) elif previous_note == NotableDomain.Note.EXCLUDE_FROM_ACCOUNT_CREATION_AND_CONTENT: try: item.referrer.spam_data['domains'].remove(domain.domain) diff --git a/osf/models/mixins.py b/osf/models/mixins.py index 1fe75420fa2..7bc7bdfe28d 100644 --- a/osf/models/mixins.py +++ b/osf/models/mixins.py @@ -2285,12 +2285,14 @@ def unspam(self, save=False): super().unspam(save=save) self.undelete(save=save) - def confirm_spam(self, domains=None, save=True, train_spam_services=True): + def confirm_spam(self, domains=None, save=True, train_spam_services=True, notify=True): """ This should add behavior specific nodes/preprints confirmed to be spam. :param save: :return: """ + was_already_spam = self.spam_status == SpamStatus.SPAM + super().confirm_spam(save=save, domains=domains or [], train_spam_services=train_spam_services) self.deleted = timezone.now() was_public = self.was_public_at_spam @@ -2308,6 +2310,31 @@ def confirm_spam(self, domains=None, save=True, train_spam_services=True): if save: self.save() + if notify and not was_already_spam: + Preprint = apps.get_model('osf.Preprint') + Registration = apps.get_model('osf.Registration') + if isinstance(self, Preprint): + notification_type = NotificationTypeEnum.PREPRINT_CONFIRMED_SPAM + resource_type_label = 'preprint' + elif isinstance(self, Registration): + notification_type = NotificationTypeEnum.NODE_CONFIRMED_SPAM + resource_type_label = 'registration' + else: + notification_type = NotificationTypeEnum.NODE_CONFIRMED_SPAM + resource_type_label = 'project' + for contributor in self.contributors: + notification_type.instance.emit( + user=contributor, + subscribed_object=self, + event_context={ + 'user_fullname': contributor.fullname, + 'resource_title': self.title, + 'resource_absolute_url': self.absolute_url, + 'resource_type_label': resource_type_label, + 'osf_support_email': settings.OSF_SUPPORT_EMAIL, + } + ) + def confirm_ham(self, save=False, train_spam_services=True): """ This should add behavior specific nodes/preprints confirmed to be ham. @@ -2403,29 +2430,22 @@ def suspend_spam_user(self, user, domains=None, self_spam=False): self.flag_spam(skip_user_suspension=True) - # Suspend the flagged user for spam. - if not user.is_disabled: - user.deactivate_account() - NotificationTypeEnum.USER_SPAM_BANNED.instance.emit( - user, - event_context={ - 'user_fullname': user.fullname, - 'osf_support_email': settings.OSF_SUPPORT_EMAIL, - } - ) - + # Suspend the flagged user for spam. Deactivation and the account-ban + # notification are both handled inside user.confirm_spam(). user.confirm_spam(domains=domains or [], save=False, skip_resources_spam=True) user.save() - # Make public nodes private from this contributor + # Make public nodes private from this contributor. These are a side effect + # of the account-level ban, so they don't get their own notification -- + # the single account-ban email already covers it. for node in user.all_nodes: if (self._id != node._id or self_spam) and len(node.contributors) == 1 and node.is_public: - node.confirm_spam(save=True, domains=domains, train_spam_services=False) + node.confirm_spam(save=True, domains=domains, train_spam_services=False, notify=False) # Make preprints private from this contributor for preprint in user.preprints.all(): if (self._id != preprint._id or self_spam) and len(preprint.contributors) == 1 and preprint.is_public: - preprint.confirm_spam(save=True, domains=domains, train_spam_services=False) + preprint.confirm_spam(save=True, domains=domains, train_spam_services=False, notify=False) def flag_spam(self, skip_user_suspension=False): """ Overrides SpamMixin#flag_spam. diff --git a/osf/models/notification_type.py b/osf/models/notification_type.py index 9a6629e22e2..749843068ff 100644 --- a/osf/models/notification_type.py +++ b/osf/models/notification_type.py @@ -107,6 +107,7 @@ class NotificationTypeEnum(str, Enum): NODE_WITHDRAWAl_REQUEST_APPROVED = 'node_withdrawal_request_approved' NODE_WITHDRAWAl_REQUEST_REJECTED = 'node_withdrawal_request_rejected' NODE_NEW_PUBLIC_PROJECT = 'node_new_public_project' + NODE_CONFIRMED_SPAM = 'node_confirmed_spam' FILE_UPDATED = 'file_updated' FILE_ADDED = 'file_added' @@ -131,6 +132,7 @@ class NotificationTypeEnum(str, Enum): PREPRINT_REQUEST_WITHDRAWAL_DECLINED = 'preprint_request_withdrawal_declined' PREPRINT_CONTRIBUTOR_ADDED_PREPRINT_NODE_FROM_OSF = 'preprint_contributor_added_preprint_node_from_osf' PREPRINT_CONTRIBUTOR_ADDED_DEFAULT = 'preprint_contributor_added_default' + PREPRINT_CONFIRMED_SPAM = 'preprint_confirmed_spam' # Collections Submission notifications COLLECTION_SUBMISSION_REMOVED_ADMIN = 'collection_submission_removed_admin' diff --git a/osf/models/spam.py b/osf/models/spam.py index 43e862d97db..75327b09403 100644 --- a/osf/models/spam.py +++ b/osf/models/spam.py @@ -165,7 +165,7 @@ def confirm_ham(self, save=False, train_spam_services=True): ) ) - def confirm_spam(self, domains=None, save=True, train_spam_services=True): + def confirm_spam(self, domains=None, save=True, train_spam_services=True, notify=True): if domains: if 'domains' in self.spam_data: self.spam_data['domains'].extend(domains) diff --git a/osf/models/user.py b/osf/models/user.py index 98461a3cf0a..edc85a3a2e0 100644 --- a/osf/models/user.py +++ b/osf/models/user.py @@ -1475,18 +1475,30 @@ def confirm_email(self, token, merge=False): return True - def confirm_spam(self, domains=None, save=True, train_spam_services=False, skip_resources_spam=False): + def confirm_spam(self, domains=None, save=True, train_spam_services=False, skip_resources_spam=False, notify=True): + was_disabled = self.is_disabled self.deactivate_account() super().confirm_spam(domains=domains, save=save, train_spam_services=train_spam_services) + if notify and not was_disabled: + NotificationTypeEnum.USER_SPAM_BANNED.instance.emit( + user=self, + event_context={ + 'user_fullname': self.fullname, + 'osf_support_email': website_settings.OSF_SUPPORT_EMAIL, + } + ) + if skip_resources_spam: return - # Don't train on resources merely associated with spam user + # Don't train on resources merely associated with spam user, and don't + # send a separate per-item notification for content caught up in the + # account-level ban cascade -- the single account-ban email covers it. for node in self.nodes.filter(is_public=True, is_deleted=False): - node.confirm_spam(domains=domains, train_spam_services=train_spam_services) + node.confirm_spam(domains=domains, train_spam_services=train_spam_services, notify=False) for preprint in self.preprints.filter(is_public=True, deleted__isnull=True): - preprint.confirm_spam(domains=domains, train_spam_services=train_spam_services) + preprint.confirm_spam(domains=domains, train_spam_services=train_spam_services, notify=False) def confirm_ham(self, save=False, train_spam_services=False): self.reactivate_account() diff --git a/osf_tests/test_notable_domains.py b/osf_tests/test_notable_domains.py index 317ae2dc7fc..a5fd4663eef 100644 --- a/osf_tests/test_notable_domains.py +++ b/osf_tests/test_notable_domains.py @@ -11,7 +11,8 @@ from osf.models import ( NotableDomain, DomainReference, - SpamStatus + SpamStatus, + NotificationTypeEnum, ) from osf.utils.workflows import DefaultStates from osf_tests.factories import ( @@ -21,6 +22,7 @@ RegistrationFactory, UserFactory ) +from tests.utils import capture_notifications class TestDomainExtraction: @@ -249,6 +251,23 @@ def test_check_resource_for_duplicate_spam_domains(self, factory, spam_domain, m domain__domain=spam_domain.netloc ).count() == 1 + def test_check_resource_for_spam_postcommit_bans_user_flagged_via_domain(self, spam_domain, marked_as_spam_domain): + user = UserFactory() + with mock.patch.object(spam_tasks.requests, 'head'), capture_notifications() as notifications: + spam_tasks.check_resource_for_spam_postcommit( + guid=user._id, + content=spam_domain.geturl(), + author=user.fullname, + author_email=user.username, + request_headers={}, + ) + + user.reload() + assert user.is_disabled + assert user.spam_status == SpamStatus.SPAM + assert len(notifications['emits']) == 1 + assert notifications['emits'][0]['type'] == NotificationTypeEnum.USER_SPAM_BANNED + @pytest.mark.enable_enqueue_task def test_extract_domains_from_wiki__public_project_extracts_domains_on_wiki_save(self, request_context): assert DomainReference.objects.count() == 0 diff --git a/tests/test_spam_mixin.py b/tests/test_spam_mixin.py index ca3e25a9cda..9004a53df4b 100644 --- a/tests/test_spam_mixin.py +++ b/tests/test_spam_mixin.py @@ -10,7 +10,7 @@ from tests.base import DbTestCase from osf_tests.factories import UserFactory, CommentFactory, ProjectFactory, PreprintFactory, RegistrationFactory, AuthUserFactory -from osf.models import NotableDomain, SpamStatus, NotificationTypeEnum +from osf.models import NotableDomain, SpamStatus, NotificationTypeEnum, OSFUser, AbstractNode, Preprint from tests.utils import capture_notifications from website import settings @@ -28,12 +28,36 @@ def test_throttled_autoban(): projects.append(proj) assert len(notifications['emits']) == 1 assert notifications['emits'][0]['type'] == NotificationTypeEnum.USER_SPAM_BANNED + assert not any(e['type'] == NotificationTypeEnum.NODE_CONFIRMED_SPAM for e in notifications['emits']) user.reload() assert user.is_disabled for project in projects: assert not project.is_public +@pytest.mark.django_db +def test_user_confirm_spam_cascade_suppresses_content_notifications(): + user = AuthUserFactory() + project = ProjectFactory(creator=user, is_public=True) + preprint = PreprintFactory(creator=user, is_public=True) + + with capture_notifications() as notifications: + user.confirm_spam(save=True) + + assert len(notifications['emits']) == 1 + assert notifications['emits'][0]['type'] == NotificationTypeEnum.USER_SPAM_BANNED + assert not any( + e['type'] in (NotificationTypeEnum.NODE_CONFIRMED_SPAM, NotificationTypeEnum.PREPRINT_CONFIRMED_SPAM) + for e in notifications['emits'] + ) + user.reload() + assert user.is_disabled + project.reload() + assert not project.is_public + preprint.reload() + assert not preprint.is_public + + @pytest.mark.enable_implicit_clean class TestReportAbuse(DbTestCase): @@ -173,7 +197,35 @@ def test_confirm_ham(self, spammable_thing): assert spammable_thing.is_ham def test_confirm_spam(self, spammable_thing): - spammable_thing.confirm_spam(save=True) + if isinstance(spammable_thing, OSFUser): + expected_type = NotificationTypeEnum.USER_SPAM_BANNED + elif isinstance(spammable_thing, Preprint): + expected_type = NotificationTypeEnum.PREPRINT_CONFIRMED_SPAM + elif isinstance(spammable_thing, AbstractNode): + expected_type = NotificationTypeEnum.NODE_CONFIRMED_SPAM + else: + expected_type = None + + with capture_notifications(allow_none=expected_type is None) as notifications: + spammable_thing.confirm_spam(save=True) + + assert spammable_thing.is_spam + if expected_type is None: + assert notifications['emits'] == [] + else: + assert len(notifications['emits']) == 1 + assert notifications['emits'][0]['type'] == expected_type + + def test_confirm_spam_is_idempotent_for_notifications(self, spammable_thing): + with capture_notifications(allow_none=True) as notifications: + spammable_thing.confirm_spam(save=True) + spammable_thing.confirm_spam(save=True) + + assert len(notifications['emits']) <= 1 + + def test_confirm_spam_notify_false_suppresses_emit(self, spammable_thing): + with capture_notifications(expect_none=True): + spammable_thing.confirm_spam(save=True, notify=False) assert spammable_thing.is_spam @pytest.mark.parametrize('assume_ham', (True, False)) diff --git a/website/templates/spam_resource_confirmed.html.mako b/website/templates/spam_resource_confirmed.html.mako new file mode 100644 index 00000000000..8d838eb0e6e --- /dev/null +++ b/website/templates/spam_resource_confirmed.html.mako @@ -0,0 +1,16 @@ +<%inherit file="notify_base.mako" /> + +<%def name="content()"> +