diff --git a/tests/conftest.py b/tests/conftest.py index b25d476147ef..b7510abb2c9a 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -52,6 +52,8 @@ from warehouse.email.interfaces import IEmailSender from warehouse.helpdesk import services as helpdesk_services from warehouse.helpdesk.interfaces import IAdminNotificationService, IHelpDeskService +from warehouse.legacy.api.xmlrpc.cache import services as xmlrpc_services +from warehouse.legacy.api.xmlrpc.cache.interfaces import IXMLRPCCache from warehouse.macaroons import services as macaroon_services from warehouse.macaroons.interfaces import IMacaroonService from warehouse.metrics import IMetricsService @@ -163,6 +165,7 @@ def pyramid_services( macaroon_service, helpdesk_service, notification_service, + xmlrpccache_service, ): services = _Services() @@ -186,6 +189,7 @@ def pyramid_services( services.register_service(macaroon_service, IMacaroonService, None, name="") services.register_service(helpdesk_service, IHelpDeskService, None) services.register_service(notification_service, IAdminNotificationService) + services.register_service(xmlrpccache_service, IXMLRPCCache) return services @@ -331,7 +335,7 @@ def get_app_config(database, nondefaults=None): "sessions.secret": "123456", "sessions.url": "redis://localhost:0/", "statuspage.url": "https://2p66nmmycsj3.statuspage.io", - "warehouse.xmlrpc.cache.url": "redis://localhost:0/", + "warehouse.xmlrpc.cache.url": "null://", "terms.revision": "initial", } @@ -524,6 +528,14 @@ def notification_service(): return helpdesk_services.ConsoleAdminNotificationService() +@pytest.fixture +def xmlrpccache_service(): + def purger(tags): + return None + + return xmlrpc_services.NullXMLRPCCache("null://", purger) + + class QueryRecorder: def __init__(self): self.queries = [] diff --git a/tests/unit/packaging/test_services.py b/tests/unit/packaging/test_services.py index 71f53993317c..8b21ef3fc07b 100644 --- a/tests/unit/packaging/test_services.py +++ b/tests/unit/packaging/test_services.py @@ -1057,7 +1057,14 @@ def test_check_project_name_typosquatting_prohibited(self, db_session): ProhibitedProjectFactory.create(name="numpy") with pytest.raises(ProjectNameUnavailableTypoSquattingError): - service.check_project_name("numpi") + service.check_project_name_after_commit("numpi") + + def test_check_project_name_typosquatting_no_typo(self, db_session): + # TODO: Update this test once we have a dynamic TopN approach + service = ProjectService(session=db_session) + ProhibitedProjectFactory.create(name="numpy") + + assert service.check_project_name_after_commit("foobar") is None def test_check_project_name_ok(self, db_session): service = ProjectService(session=db_session) diff --git a/tests/unit/packaging/test_typosnyper.py b/tests/unit/packaging/test_typosnyper.py index 9ad021d15734..68d80869ee89 100644 --- a/tests/unit/packaging/test_typosnyper.py +++ b/tests/unit/packaging/test_typosnyper.py @@ -12,12 +12,15 @@ import pytest +from warehouse.packaging import typosnyper +from warehouse.packaging.models import Project from warehouse.packaging.typosnyper import typo_check_name @pytest.mark.parametrize( ("name", "expected"), [ + ("x", None), # Pass, shorter than 4 characters ("numpy", None), # Pass, no typos, exists ("NuMpy", None), # Pass, same as `numpy` after canonicalization ("nuumpy", ("repeated_characters", "numpy")), @@ -43,3 +46,11 @@ def test_typo_check_name(name, expected): } assert typo_check_name(name, corpus=test_names_corpus) == expected + + +def test_check_typo_after_commit(db_request): + project = Project(name="foobar") + db_request.db.add(project) + db_request.db.commit() + + assert typosnyper.check_typo_after_commit.calls == [] diff --git a/warehouse/packaging/services.py b/warehouse/packaging/services.py index d3b14eb1fb8f..fb9fe7c3d6bb 100644 --- a/warehouse/packaging/services.py +++ b/warehouse/packaging/services.py @@ -36,7 +36,6 @@ from warehouse.admin.flags import AdminFlagValue from warehouse.email import send_pending_trusted_publisher_invalidated_email from warehouse.events.tags import EventTag -from warehouse.helpdesk.interfaces import IAdminNotificationService from warehouse.metrics import IMetricsService from warehouse.oidc.models import PendingOIDCPublisher from warehouse.packaging.interfaces import ( @@ -485,6 +484,9 @@ def check_project_name(self, name: str) -> None: ).first(): raise ProjectNameUnavailableSimilarError(similar_project_name) + return None + + def check_project_name_after_commit(self, name: str) -> None: # Check for typo-squatting. if typo_check_match := typo_check_name(canonicalize_name(name)): raise ProjectNameUnavailableTypoSquattingError( @@ -558,85 +560,6 @@ def create_project( projecthelp=request.help_url(_anchor="project-name"), ), ) from None - except ProjectNameUnavailableTypoSquattingError as exc: - # Don't yet raise an error here, as we want to allow the - # user to proceed with the project creation. We'll log a warning - # instead. - request.log.warning( - "ProjectNameUnavailableTypoSquattingError", - check_name=exc.check_name, - existing_project_name=exc.existing_project_name, - ) - # Send notification to Admins for review - notification_service = request.find_service(IAdminNotificationService) - - warehouse_domain = request.registry.settings.get("warehouse.domain") - new_project_page = request.route_url( - "packaging.project", - name=name, - _host=warehouse_domain, - ) - new_project_text = ( - f"During `file_upload`, Project Create for " - f"*<{new_project_page}|{name}>* was detected as a potential " - f"typo by the `{exc.check_name!r}` check." - ) - existing_project_page = request.route_url( - "packaging.project", - name=exc.existing_project_name, - _host=warehouse_domain, - ) - existing_project_text = ( - f"<{existing_project_page}|Existing project: " - f"{exc.existing_project_name}>" - ) - - webhook_payload = { - "blocks": [ - { - "type": "header", - "text": { - "type": "plain_text", - "text": "TypoSnyper :warning:", - "emoji": True, - }, - }, - { - "type": "section", - "text": { - "type": "mrkdwn", - "text": new_project_text, - }, - }, - { - "type": "section", - "text": { - "type": "mrkdwn", - "text": existing_project_text, - }, - }, - {"type": "divider"}, - { - "type": "context", - "elements": [ - { - "type": "plain_text", - "text": "Once reviewed/confirmed, " - "react to this message with :white_check_mark:", - "emoji": True, - } - ], - }, - ] - } - notification_service.send_notification(payload=webhook_payload) - - request.metrics.increment( - "warehouse.packaging.services.create_project.typo_squatting", - tags=[f"check_name:{exc.check_name!r}"], - ) - # and continue with the project creation - pass # The project name is valid: create it and add it project = Project(name=name) diff --git a/warehouse/packaging/typosnyper.py b/warehouse/packaging/typosnyper.py index 91ffd9988b07..d94ccf9eefb8 100644 --- a/warehouse/packaging/typosnyper.py +++ b/warehouse/packaging/typosnyper.py @@ -25,6 +25,16 @@ from itertools import permutations +from pyramid.threadlocal import get_current_request + +from warehouse import db +from warehouse.helpdesk.interfaces import IAdminNotificationService +from warehouse.packaging.interfaces import ( + IProjectService, + ProjectNameUnavailableTypoSquattingError, +) +from warehouse.packaging.models import Project + # Ensure all checks return a similar type, # where the first string is the check name, # followed by the matched project name, @@ -431,3 +441,92 @@ def typo_check_name(project_name: str, corpus=None) -> TypoCheckMatch: if result := check(project_name, corpus=corpus): return result return None + + +@db.listens_for(db.Session, "after_commit") +def check_typo_after_commit(config, session): + # Go through each new object and find any Project instances + for obj in session.new: + if obj.__class__ == Project: + request = get_current_request() + if not request: + # Can't do anything if there isn't a request + return + project_service = request.find_service(IProjectService) + name = obj.name + try: + project_service.check_project_name_after_commit(name) + except ProjectNameUnavailableTypoSquattingError as exc: + request.log.warning( + "ProjectNameUnavailableTypoSquattingError", + check_name=exc.check_name, + existing_project_name=exc.existing_project_name, + ) + # Send notification to Admins for review + notification_service = request.find_service(IAdminNotificationService) + + warehouse_domain = request.registry.settings.get("warehouse.domain") + new_project_page = request.route_url( + "packaging.project", + name=name, + _host=warehouse_domain, + ) + new_project_text = ( + f"During `file_upload`, Project Create for " + f"*<{new_project_page}|{name}>* was detected as a potential " + f"typo by the `{exc.check_name!r}` check." + ) + existing_project_page = request.route_url( + "packaging.project", + name=exc.existing_project_name, + _host=warehouse_domain, + ) + existing_project_text = ( + f"<{existing_project_page}|Existing project: " + f"{exc.existing_project_name}>" + ) + + webhook_payload = { + "blocks": [ + { + "type": "header", + "text": { + "type": "plain_text", + "text": "TypoSnyper :warning:", + "emoji": True, + }, + }, + { + "type": "section", + "text": { + "type": "mrkdwn", + "text": new_project_text, + }, + }, + { + "type": "section", + "text": { + "type": "mrkdwn", + "text": existing_project_text, + }, + }, + {"type": "divider"}, + { + "type": "context", + "elements": [ + { + "type": "plain_text", + "text": "Once reviewed/confirmed, " + "react to this message with :white_check_mark:", + "emoji": True, + } + ], + }, + ] + } + notification_service.send_notification(payload=webhook_payload) + + request.metrics.increment( + "warehouse.packaging.services.create_project.typo_squatting", + tags=[f"check_name:{exc.check_name!r}"], + )