revert regex change, add tests, add urlsplit

Author: bodintsov

osf/models/user.py

@@ -1068,7 +1068,10 @@ 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 not was_creating:
+        if has_domain and self.is_hammy:
+            self.flag_spam()
+
+        if has_domain and not was_creating and not self.is_hammy:
             self.confirm_spam()
 
         if set(self.SPAM_USER_PROFILE_FIELDS.keys()).intersection(dirty_fields):
@@ -1482,7 +1485,8 @@ 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):
-        self.deactivate_account()
+        if not self.is_hammy:
+            self.deactivate_account()
         super().confirm_spam(domains=domains, save=save, train_spam_services=train_spam_services)
 
         if skip_resources_spam:

osf/models/validators.py

@@ -1,4 +1,5 @@
 import re
+from urllib.parse import urlsplit
 import waffle
 
 from jsonschema import ValidationError as JsonSchemaValidationError, SchemaError, Draft7Validator, validate, validators
@@ -115,19 +116,17 @@ def validate_email(value):
 
 def has_domain_in_user_fields_for_names(user):
     name_content = ' '.join(getattr(user, field) or '' for field in user.DOMAIN_VALIDATION_FIELDS).strip()
-
-    if not name_content:
-        return False
-
-    text = name_content.lower()
-    if any(suffix in text for suffix in ['m.sc.', 'msc.', 'phd.', 'ph.d.', 'msc.pt', 'pt.', 'prof.', 'dr.', 'md.', 'jd.', 'esq.']):
-        return False
-
-    if re.search(r'\b[a-z]\.[a-z0-9-]+\b', text):
-        return False
-
-    return True if DOMAIN_REGEX.search(name_content) else False
-
+    for match in DOMAIN_REGEX.finditer(name_content):
+        candidate = match.group(0).strip()
+        if looks_like_url(candidate):
+            return True
+    return False
+
+def looks_like_url(candidate):
+    if candidate.startswith('www.'):
+        candidate = f'https://{candidate}'
+    parsed = urlsplit(candidate)
+    return bool(parsed.scheme and parsed.netloc)
 
 def validate_subject_hierarchy_length(parent):
     from osf.models import Subject

osf_tests/test_validators.py

@@ -1,17 +1,20 @@
 import pytest
 from osf.exceptions import ValidationValueError
 
-from osf.models import validators
+from types import SimpleNamespace
+from osf.models import validators, OSFUser
 from osf.models.validators import has_domain_in_user_fields_for_names
-from osf_tests.factories import SubjectFactory, AuthUserFactory
+from osf_tests.factories import SubjectFactory
 
 
 # Ported from tests/framework/test_mongo.py
 
-@pytest.fixture()
-def user(user_data):
-    return AuthUserFactory(**user_data)
-
+def mock_user(user_data):
+    user = SimpleNamespace()
+    user.DOMAIN_VALIDATION_FIELDS = OSFUser.DOMAIN_VALIDATION_FIELDS
+    for field in user.DOMAIN_VALIDATION_FIELDS:
+        setattr(user, field, user_data.get(field, ''))
+    return user
 
 def test_string_required_passes_with_string():
     assert validators.string_required('Hi!') is True
@@ -155,13 +158,36 @@ def test_validate_expand_subject_hierarchy():
         }
     ]
 )
-def test_has_domain_in_user_fields_for_names_returns_false(user, user_data):
-    user.DOMAIN_VALIDATION_FIELDS = [
-        'fullname',
-        'given_name',
-        'middle_names',
-        'family_name',
-        'suffix',
-    ]
-
+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
+
+
+@pytest.mark.parametrize(
+    'user_data',
+    [
+        {
+            'fullname': 'Judith Sarah',
+            'given_name': 'Judith',
+            'family_name': 'Preuss',
+            'middle_names': 'https://www.google.com',
+            'suffix': 'M.Sc.',
+        },
+        {
+            'fullname': 'J.H. van Hateren',
+            'given_name': 'J.H.',
+            'family_name': 'https://google.com',
+            'middle_names': '',
+        },
+        {
+            'fullname': 'Judith Sarah',
+            'given_name': 'Judith',
+            'family_name': 'Preuss',
+            'middle_names': 'www.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