diff --git a/osf/external/spam/tasks.py b/osf/external/spam/tasks.py index d67b21912dc..9fdaea63d98 100644 --- a/osf/external/spam/tasks.py +++ b/osf/external/spam/tasks.py @@ -17,7 +17,7 @@ DOMAIN_REGEX = re.compile(r'\W*(?P\w+://)?(?Pwww\.)?(?P([\w-]+\.)+[a-zA-Z]+)(?P[/\-\.\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): diff --git a/osf/models/user.py b/osf/models/user.py index 5739813cd48..556fb0cff31 100644 --- a/osf/models/user.py +++ b/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,11 +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) - - if has_domain and was_creating: - raise ValidationError('Invalid personal information.') + has_domain, status = has_domain_in_user_fields_for_names(self) + should_mark_ham = True if status == SpamStatus.HAM else False self.update_is_active() self.username = self.username.lower().strip() if self.username else None @@ -1068,8 +1067,16 @@ 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: - self.confirm_spam() + if was_creating: + if self.is_hammy and has_domain: + self.flag_spam() + if not self.is_hammy and has_domain: + self.confirm_spam() + if should_mark_ham: + self.confirm_ham() + + if has_domain and was_creating: + raise ValidationError('Invalid personal information.') if set(self.SPAM_USER_PROFILE_FIELDS.keys()).intersection(dirty_fields): request = get_current_request() @@ -1482,7 +1489,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: diff --git a/osf/models/validators.py b/osf/models/validators.py index c5acafd3ea7..de051ecccc2 100644 --- a/osf/models/validators.py +++ b/osf/models/validators.py @@ -1,6 +1,6 @@ import re 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 @@ -11,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): @@ -114,9 +115,32 @@ 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]) - return True if DOMAIN_REGEX.search(name_content) else False - + 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, SpamStatus.SPAM + return False, SpamStatus.UNKNOWN + +def looks_like_url(candidate): + 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 diff --git a/osf_tests/test_user.py b/osf_tests/test_user.py index 43ccf47ffcf..41dfe5f84b0 100644 --- a/osf_tests/test_user.py +++ b/osf_tests/test_user.py @@ -2081,11 +2081,34 @@ def test_validate_domains_field(self): self.user.save() self.user.refresh_from_db() - assert self.user.is_spammy is True + assert self.user.is_spammy is False self.user.unspam(save=True) setattr(self.user, field, 'not a url') self.user.save() + def test_validate_domain_fields_unregistered_contributor(self): + user = OSFUser.create_unregistered(fullname='google.com') + with pytest.raises(ValidationError): + user.save() + assert user.is_spammy is True + assert user.is_hammy is False + + def test_validate_domain_fields_notable_domain_in_field_unregistered_contributor(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_for_real(self): + user = OSFUser.create_unregistered(fullname='Sandhya N.Sathesh') + for field in user.DOMAIN_VALIDATION_FIELDS: + setattr(user, field, 'Sandhya N.Sathesh') + user.save() + user.refresh_from_db() + + assert user.is_spammy is False + assert user.is_hammy is False class TestUserGdprDelete: diff --git a/osf_tests/test_validators.py b/osf_tests/test_validators.py index dc561389356..ebe7456ee32 100644 --- a/osf_tests/test_validators.py +++ b/osf_tests/test_validators.py @@ -1,11 +1,21 @@ import pytest from osf.exceptions import ValidationValueError -from osf.models import validators +from types import SimpleNamespace +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 + # Ported from tests/framework/test_mongo.py +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 @@ -50,3 +60,183 @@ def test_validate_expand_subject_hierarchy(): subject_list = [fruit._id, '12345_bad_id'] with pytest.raises(ValidationValueError): validators.expand_subject_hierarchy(subject_list) + + +@pytest.mark.parametrize( + 'user_data', + [ + { + 'fullname': 'Judith Sarah Preuss, M.Sc.', + 'given_name': 'Judith', + 'family_name': 'Preuss', + 'middle_names': 'Sarah', + 'suffix': 'M.Sc.', + }, + { + 'fullname': 'J.H. van Hateren', + 'given_name': 'J.H.', + 'family_name': 'van Hateren', + 'middle_names': '', + }, + { + 'fullname': 'Sami-Egil Ahonen', + 'given_name': 'Sami-Egil', + 'family_name': 'Ahonen', + 'middle_names': '', + }, + { + 'fullname': 'Giovanni Luca Ciampaglia', + 'given_name': 'Giovanni Luca', + 'family_name': 'Ciampaglia', + 'middle_names': '', + }, + { + 'fullname': 'Joseph P.R.O. Orgel', + 'given_name': 'Joseph', + 'family_name': 'Orgel', + 'middle_names': 'P.R.O.', + }, + { + 'fullname': 'Andrew Daoust', + 'given_name': 'Andrew', + 'family_name': 'Daoust', + 'middle_names': '', + }, + { + 'fullname': 'Aidan G.C. Wright', + 'given_name': 'Aidan', + 'family_name': 'Wright', + 'middle_names': 'G.C.', + }, + { + 'fullname': 'Guillermo Perez Algorta', + 'given_name': 'Guillermo', + 'family_name': 'Perez Algorta', + 'middle_names': '', + }, + { + 'fullname': 'Sarah Wojkowski, MSc.PT, PhD.', + 'given_name': 'Sarah', + 'family_name': 'Wojkowski', + 'middle_names': 'MSc.PT', + }, + { + 'fullname': 'Brockmann, L.C. (Leon)', + 'given_name': 'Leon', + 'family_name': 'Brockmann', + 'middle_names': 'L.C.', + }, + { + 'fullname': 'Gragnolati, G.M. (Gaia Mariavittoria)', + 'given_name': 'Gaia', + 'family_name': 'Gragnolati', + 'middle_names': 'G.M.', + }, + { + 'fullname': 'F.H. Leeuwis', + 'given_name': 'F.H.', + 'family_name': 'Leeuwis', + 'middle_names': '', + }, + { + 'fullname': 'Grauss, S.E. (Sophie)', + 'given_name': 'Sophie', + 'family_name': 'Grauss', + 'middle_names': 'S.E.', + }, + { + 'fullname': 'Sandhya N.Sathesh', + 'given_name': 'Sandhya', + 'family_name': 'N.Sathesh', + 'middle_names': '', + }, + { + 'fullname': 'John Doe', + 'given_name': 'John', + 'family_name': 'Doe', + 'middle_names': '', + } + ] +) +def test_has_domain_in_user_fields(user_data): + user = mock_user(user_data) + has_domain, status = has_domain_in_user_fields_for_names(user) + assert has_domain is False + assert status == SpamStatus.UNKNOWN + + +@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.', + }, + { + '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) + has_domain, status = has_domain_in_user_fields_for_names(user) + assert has_domain is True + assert status == 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) + has_domain, status = has_domain_in_user_fields_for_names(user) + assert has_domain is False + assert status == 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) + has_domain, status = has_domain_in_user_fields_for_names(user) + assert has_domain is True + assert status == SpamStatus.SPAM diff --git a/poetry.lock b/poetry.lock index 67311478796..6a2bc3e2d44 100644 --- a/poetry.lock +++ b/poetry.lock @@ -1556,7 +1556,7 @@ version = "3.16.1" description = "A platform independent file lock." optional = false python-versions = ">=3.8" -groups = ["dev"] +groups = ["main", "dev"] files = [ {file = "filelock-3.16.1-py3-none-any.whl", hash = "sha256:2082e5703d51fbf98ea75855d9d5527e33d8ff23099bec374a134febee6946b0"}, {file = "filelock-3.16.1.tar.gz", hash = "sha256:c249fbfcd5db47e5e2d6d62198e565475ee65e4831e2561c8e313fa7eb961435"}, @@ -2972,7 +2972,7 @@ version = "4.3.6" description = "A small Python package for determining appropriate platform-specific dirs, e.g. a `user data dir`." optional = false python-versions = ">=3.8" -groups = ["dev"] +groups = ["main", "dev"] files = [ {file = "platformdirs-4.3.6-py3-none-any.whl", hash = "sha256:73e575e1408ab8103900836b97580d5307456908a03e92031bab39e4554cc3fb"}, {file = "platformdirs-4.3.6.tar.gz", hash = "sha256:357fb2acbc885b0419afd3ce3ed34564c13c9b95c89360cd9563f73aa5e2b907"}, @@ -4449,6 +4449,36 @@ files = [ {file = "uritemplate-4.1.1.tar.gz", hash = "sha256:4346edfc5c3b79f694bccd6d6099a322bbeb628dbf2cd86eea55a456ce5124f0"}, ] +[[package]] +name = "uritools" +version = "6.1.2" +description = "URI parsing, classification and composition" +optional = false +python-versions = ">=3.10" +groups = ["main"] +files = [ + {file = "uritools-6.1.2-py3-none-any.whl", hash = "sha256:5682f8c58e96e379224fd72aec227a45432740c1fa860a06b3024d47c2968601"}, + {file = "uritools-6.1.2.tar.gz", hash = "sha256:fa60028843a8be651699a1ee2b399066eeaef349224b32a177efa4aeba463f00"}, +] + +[[package]] +name = "urlextract" +version = "1.9.0" +description = "Collects and extracts URLs from given text." +optional = false +python-versions = "*" +groups = ["main"] +files = [ + {file = "urlextract-1.9.0-py3-none-any.whl", hash = "sha256:f88963532488b1c7c405e21bd162ae97871754ea04b60e18d33ee075b19b82fd"}, + {file = "urlextract-1.9.0.tar.gz", hash = "sha256:70508e02ba9df372e25cf0642db367cece273e8712cd0ce78178fc5dd7ea00db"}, +] + +[package.dependencies] +filelock = "*" +idna = "*" +platformdirs = "*" +uritools = "*" + [[package]] name = "urllib3" version = "1.26.18" @@ -4727,4 +4757,4 @@ testing = ["coverage (>=5.0.3)", "zope.event", "zope.testing"] [metadata] lock-version = "2.1" python-versions = "^3.12" -content-hash = "9b2f29b9b4958b47e4c428f4225d04bbd12f239c02f9afc186989a49547b83ed" +content-hash = "5599dfc677ced71d0e9097822fb1d1f6f0e22320eacca87299d1542b89e9f286" diff --git a/pyproject.toml b/pyproject.toml index b789987d9a9..2112ccf307a 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -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"