apply new logic, add check for notable domains, add tests

Author: bodintsov

osf/external/spam/tasks.py

@@ -17,7 +17,7 @@
 
 DOMAIN_REGEX = re.compile(r'\W*(?P<protocol>\w+://)?(?P<www>www\.)?(?P<domain>([\w-]+\.)+[a-zA-Z]+)(?P<path>[/\-\.\w]*)?\W*')
 REDIRECT_CODES = {301, 302, 303, 307, 308}
-
+NOTABLE_POST_NOMINALS = ['m.sc.', 'msc.', 'phd.', 'ph.d.', 'msc.pt', 'pt.', 'prof.', 'dr.', 'md.', 'jd.', 'esq.']
 
 @celery_app.task()
 def reclassify_domain_references(notable_domain_id, current_note, previous_note):

osf/models/user.py

@@ -54,7 +54,7 @@
 from .institution import Institution
 from .institution_affiliation import InstitutionAffiliation
 from .mixins import AddonModelMixin, ShareIndexMixin
-from .spam import SpamMixin
+from .spam import SpamMixin, SpamStatus
 from .session import UserSessionMap
 from .tag import Tag
 from .validators import validate_email, validate_social, validate_history_item, has_domain_in_user_fields_for_names
@@ -1056,8 +1056,10 @@ def save(self, *args, **kwargs):
 
         was_creating = self._state.adding
         has_domain = False
+        should_mark_ham = False
         if not self.is_spammy:
-            has_domain = has_domain_in_user_fields_for_names(self)
+            has_domain, status = has_domain_in_user_fields_for_names(self)
+            should_mark_ham = True if status == SpamStatus.HAM else False
 
         if has_domain and was_creating:
             raise ValidationError('Invalid personal information.')
@@ -1068,11 +1070,14 @@ def save(self, *args, **kwargs):
         dirty_fields = self.get_dirty_fields(check_relationship=True)
         ret = super().save(*args, **kwargs)  # must save BEFORE spam check, as user needs guid.
 
-        if has_domain and self.is_hammy:
-            self.flag_spam()
+        if has_domain:
+            if self.is_hammy:
+                self.flag_spam()
+            if not self.is_hammy:
+                self.confirm_spam()
 
-        if has_domain and not was_creating and not self.is_hammy:
-            self.confirm_spam()
+        if was_creating and should_mark_ham:
+            self.confirm_ham()
 
         if set(self.SPAM_USER_PROFILE_FIELDS.keys()).intersection(dirty_fields):
             request = get_current_request()

osf/models/validators.py

@@ -1,7 +1,6 @@
 import re
-from urllib.parse import urlsplit
 import waffle
-
+from django.db.models import Q
 from jsonschema import ValidationError as JsonSchemaValidationError, SchemaError, Draft7Validator, validate, validators
 from django.conf import settings
 from django.core.validators import URLValidator, validate_email as django_validate_email
@@ -12,9 +11,10 @@
 from osf.utils.registrations import FILE_VIEW_URL_REGEX
 from osf.utils.sanitize import strip_html
 from osf.exceptions import ValidationError, ValidationValueError, reraise_django_validation_errors, BlockedEmailError
-from osf.external.spam.tasks import DOMAIN_REGEX
+from osf.external.spam.tasks import DOMAIN_REGEX, NOTABLE_POST_NOMINALS
 
 from website.language import SWITCH_VALIDATOR_ERROR
+from urlextract import URLExtract
 
 
 def validate_history_item(items):
@@ -115,18 +115,32 @@ def validate_email(value):
 
 
 def has_domain_in_user_fields_for_names(user):
+    from osf.models import NotableDomain
+    from .spam import SpamStatus
+    notable_domain_list = set(
+        NotableDomain.objects.filter(
+            Q(note=NotableDomain.Note.ASSUME_HAM_UNTIL_REPORTED) |
+            Q(note=NotableDomain.Note.IGNORED)
+        )
+        .values_list('domain', flat=True)
+        .distinct()
+    )
     name_content = ' '.join(getattr(user, field) or '' for field in user.DOMAIN_VALIDATION_FIELDS).strip()
     for match in DOMAIN_REGEX.finditer(name_content):
         candidate = match.group(0).strip()
+        if candidate in notable_domain_list:
+            return False, SpamStatus.HAM
         if looks_like_url(candidate):
-            return True
-    return False
+            return True, SpamStatus.SPAM
+    return False, SpamStatus.UNKNOWN
 
 def looks_like_url(candidate):
-    if candidate.startswith('www.'):
-        candidate = f'https://{candidate}'
-    parsed = urlsplit(candidate)
-    return bool(parsed.scheme and parsed.netloc)
+    candidate = candidate.strip()
+    words = re.findall(r'[A-Za-z.]+', candidate.lower())
+    if any(word in NOTABLE_POST_NOMINALS for word in words):
+        return False
+    extractor = URLExtract()
+    return bool(extractor.find_urls(candidate))
 
 def validate_subject_hierarchy_length(parent):
     from osf.models import Subject

osf_tests/test_user.py

@@ -2086,6 +2086,57 @@ def test_validate_domains_field(self):
             setattr(self.user, field, 'not a url')
             self.user.save()
 
+    def test_validate_domain_fields_notable_domain_in_field(self):
+        NotableDomain.objects.get_or_create(domain='google.com', note=NotableDomain.Note.IGNORED)
+        user = OSFUser.create_unregistered(
+            fullname='google.com',
+        )
+        user.save()
+        assert user.is_spammy is False
+        assert user.is_hammy is True
+
+    def test_validate_domain_fields_notable_domain_in_field_for_existing_ham(self):
+        NotableDomain.objects.get_or_create(domain='google.com', note=NotableDomain.Note.IGNORED)
+        self.user.fullname = 'google.com'
+        self.user.confirm_ham(save=True)
+        self.user.save()
+        assert self.user.is_spammy is False
+        assert self.user.is_hammy is True
+
+        self.user.unspam(save=True)
+        self.user.fullname = 'not a url'
+        self.user.save()
+
+    def test_validate_domain_in_field_for_existing_not_ham(self):
+        self.user.fullname = 'google.com'
+        self.user.save()
+        assert self.user.is_spammy is True
+        assert self.user.is_hammy is False
+
+        self.user.unspam(save=True)
+        self.user.fullname = 'not a url'
+        self.user.save()
+
+    def test_validate_domain_fields_notable_domain_in_field_for_existing_not_ham(self):
+        NotableDomain.objects.get_or_create(domain='google.com', note=NotableDomain.Note.IGNORED)
+        self.user.fullname = 'google.com'
+        self.user.save()
+        assert self.user.is_spammy is False
+        assert self.user.is_hammy is False
+
+        self.user.unspam(save=True)
+        self.user.fullname = 'not a url'
+        self.user.save()
+
+    def test_validate_domain_fields_for_real(self):
+        self.user.fullname = 'Sandhya N.Sathesh'
+        self.user.save()
+        assert self.user.is_spammy is False
+        assert self.user.is_hammy is False
+
+        self.user.unspam(save=True)
+        self.user.fullname = 'not a url'
+        self.user.save()
 
 class TestUserGdprDelete:
 

osf_tests/test_validators.py

@@ -2,7 +2,7 @@
 from osf.exceptions import ValidationValueError
 
 from types import SimpleNamespace
-from osf.models import validators, OSFUser
+from osf.models import validators, OSFUser, NotableDomain, SpamStatus
 from osf.models.validators import has_domain_in_user_fields_for_names
 from osf_tests.factories import SubjectFactory
 
@@ -160,7 +160,9 @@ def test_validate_expand_subject_hierarchy():
 )
 def test_has_domain_in_user_fields(user_data):
     user = mock_user(user_data)
-    assert has_domain_in_user_fields_for_names(user) is False
+    result, message = has_domain_in_user_fields_for_names(user)
+    assert result is False
+    assert message == SpamStatus.UNKNOWN
 
 
 @pytest.mark.parametrize(
@@ -186,8 +188,55 @@ def test_has_domain_in_user_fields(user_data):
             'middle_names': 'www.google.com',
             'suffix': 'M.Sc.',
         },
+        {
+            'fullname': 'Judith Hateren',
+            'given_name': 'Judith',
+            'family_name': 'Hateren',
+            'middle_names': 'google.com',
+            'suffix': 'M.Sc.',
+        },
     ]
 )
 def test_has_domain_in_user_fields_fail(user_data):
     user = mock_user(user_data)
-    assert has_domain_in_user_fields_for_names(user) is True
+    result, message = has_domain_in_user_fields_for_names(user)
+    assert result is True
+    assert message == SpamStatus.SPAM
+
+@pytest.mark.parametrize(
+    'user_data',
+    [
+        {
+            'fullname': 'Judith Sarah',
+            'given_name': 'Judith',
+            'family_name': 'Preuss',
+            'middle_names': 'osf.io',
+            'suffix' : 'M.Sc.',
+        },
+    ]
+)
+def test_has_notable_domain_in_user_fields(user_data):
+    NotableDomain.objects.get_or_create(domain='osf.io', note=NotableDomain.Note.IGNORED)
+    user = mock_user(user_data)
+    result, message = has_domain_in_user_fields_for_names(user)
+    assert result is False
+    assert message == SpamStatus.HAM
+
+@pytest.mark.parametrize(
+    'user_data',
+    [
+        {
+            'fullname': 'Judith Sarah',
+            'given_name': 'Judith',
+            'family_name': 'Preuss',
+            'middle_names': 'osf.io',
+            'suffix' : 'M.Sc.',
+        },
+    ]
+)
+def test_has_no_notable_domain_in_user_fields(user_data):
+    NotableDomain.objects.get_or_create(domain='google.com', note=NotableDomain.Note.IGNORED)
+    user = mock_user(user_data)
+    result, message = has_domain_in_user_fields_for_names(user)
+    assert result is True
+    assert message == SpamStatus.SPAM

poetry.lock

@@ -102,6 +102,7 @@ djangorestframework-csv = "3.0.2"
 gevent = "24.2.1"
 packaging = "^24.0"
 flask-cors = "6.0.1"
+urlextract = "^1.9.0"
 
 [tool.poetry.group.dev.dependencies]
 pytest = "7.4.4"