diff --git a/tests/unit/accounts/test_views.py b/tests/unit/accounts/test_views.py
index 2864a7dfdbba..adbc36064306 100644
--- a/tests/unit/accounts/test_views.py
+++ b/tests/unit/accounts/test_views.py
@@ -65,6 +65,7 @@
OrganizationRole,
OrganizationRoleType,
)
+from warehouse.packaging.interfaces import IProjectService
from warehouse.packaging.models import Role, RoleInvitation
from warehouse.rate_limiting.interfaces import IRateLimiter
@@ -3306,8 +3307,17 @@ def test_reauth_no_user(self, monkeypatch, pyramid_request):
class TestManageAccountPublishingViews:
def test_initializes(self, metrics):
+ project_service = pretend.stub(check_project_name=lambda name: None)
+
+ def find_service(iface, name=None, context=None):
+ if iface is IMetricsService:
+ return metrics
+ if iface is IProjectService:
+ return project_service
+ return pretend.stub()
+
request = pretend.stub(
- find_service=pretend.call_recorder(lambda *a, **kw: metrics),
+ find_service=pretend.call_recorder(find_service),
route_url=pretend.stub(),
POST=MultiDict(),
registry=pretend.stub(
@@ -3320,9 +3330,11 @@ def test_initializes(self, metrics):
assert view.request is request
assert view.metrics is metrics
+ assert view.project_service is project_service
assert view.request.find_service.calls == [
- pretend.call(IMetricsService, context=None)
+ pretend.call(IMetricsService, context=None),
+ pretend.call(IProjectService, context=None),
]
@pytest.mark.parametrize(
@@ -3348,6 +3360,8 @@ def test_ratelimiting(self, metrics, ip_exceeded, user_exceeded):
def find_service(iface, name=None, context=None):
if iface is IMetricsService:
return metrics
+ if iface is IProjectService:
+ return pretend.stub(check_project_name=lambda name: None)
if name == "user_oidc.publisher.register":
return user_rate_limiter
@@ -3375,6 +3389,7 @@ def find_service(iface, name=None, context=None):
}
assert request.find_service.calls == [
pretend.call(IMetricsService, context=None),
+ pretend.call(IProjectService, context=None),
pretend.call(IRateLimiter, name="user_oidc.publisher.register"),
pretend.call(IRateLimiter, name="ip_oidc.publisher.register"),
]
@@ -3402,16 +3417,17 @@ def test_manage_publishing(self, metrics, monkeypatch):
"github.token": "fake-api-token",
}
),
- find_service=pretend.call_recorder(lambda *a, **kw: metrics),
+ find_service=lambda svc, **kw: {
+ IMetricsService: metrics,
+ IProjectService: project_service,
+ }[svc],
flags=pretend.stub(
disallow_oidc=pretend.call_recorder(lambda f=None: False)
),
POST=pretend.stub(),
)
- project_factory = pretend.stub()
- project_factory_cls = pretend.call_recorder(lambda r: project_factory)
- monkeypatch.setattr(views, "ProjectFactory", project_factory_cls)
+ project_service = pretend.stub(check_project_name=lambda name: None)
pending_github_publisher_form_obj = pretend.stub()
pending_github_publisher_form_cls = pretend.call_recorder(
@@ -3466,24 +3482,26 @@ def test_manage_publishing(self, metrics, monkeypatch):
pretend.call(AdminFlagValue.DISALLOW_GOOGLE_OIDC),
pretend.call(AdminFlagValue.DISALLOW_ACTIVESTATE_OIDC),
]
- assert project_factory_cls.calls == [pretend.call(request)]
assert pending_github_publisher_form_cls.calls == [
pretend.call(
request.POST,
api_token="fake-api-token",
route_url=route_url,
- project_factory=project_factory,
+ check_project_name=project_service.check_project_name,
)
]
assert pending_gitlab_publisher_form_cls.calls == [
pretend.call(
request.POST,
route_url=route_url,
- project_factory=project_factory,
+ check_project_name=project_service.check_project_name,
)
]
def test_manage_publishing_admin_disabled(self, monkeypatch, pyramid_request):
+ project_service = pretend.stub(check_project_name=lambda name: None)
+ pyramid_request.find_service = lambda _, **kw: project_service
+
pyramid_request.user = pretend.stub()
pyramid_request.registry = pretend.stub(
settings={
@@ -3497,10 +3515,6 @@ def test_manage_publishing_admin_disabled(self, monkeypatch, pyramid_request):
flash=pretend.call_recorder(lambda *a, **kw: None)
)
- project_factory = pretend.stub()
- project_factory_cls = pretend.call_recorder(lambda r: project_factory)
- monkeypatch.setattr(views, "ProjectFactory", project_factory_cls)
-
pending_github_publisher_form_obj = pretend.stub()
pending_github_publisher_form_cls = pretend.call_recorder(
lambda *a, **kw: pending_github_publisher_form_obj
@@ -3568,14 +3582,14 @@ def test_manage_publishing_admin_disabled(self, monkeypatch, pyramid_request):
pyramid_request.POST,
api_token="fake-api-token",
route_url=pyramid_request.route_url,
- project_factory=project_factory,
+ check_project_name=project_service.check_project_name,
)
]
assert pending_gitlab_publisher_form_cls.calls == [
pretend.call(
pyramid_request.POST,
route_url=pyramid_request.route_url,
- project_factory=project_factory,
+ check_project_name=project_service.check_project_name,
)
]
@@ -3607,6 +3621,12 @@ def test_manage_publishing_admin_disabled(self, monkeypatch, pyramid_request):
def test_add_pending_oidc_publisher_admin_disabled(
self, monkeypatch, pyramid_request, view_name, flag, publisher_name
):
+ project_service = pretend.stub(check_project_name=lambda name: None)
+ pyramid_request.find_service = lambda interface, **kwargs: {
+ IProjectService: project_service,
+ IMetricsService: pretend.stub(),
+ }[interface]
+
pyramid_request.user = pretend.stub()
pyramid_request.registry = pretend.stub(
settings={
@@ -3620,10 +3640,6 @@ def test_add_pending_oidc_publisher_admin_disabled(
flash=pretend.call_recorder(lambda *a, **kw: None)
)
- project_factory = pretend.stub()
- project_factory_cls = pretend.call_recorder(lambda r: project_factory)
- monkeypatch.setattr(views, "ProjectFactory", project_factory_cls)
-
pending_github_publisher_form_obj = pretend.stub()
pending_github_publisher_form_cls = pretend.call_recorder(
lambda *a, **kw: pending_github_publisher_form_obj
@@ -3698,14 +3714,14 @@ def test_add_pending_oidc_publisher_admin_disabled(
pyramid_request.POST,
api_token="fake-api-token",
route_url=pyramid_request.route_url,
- project_factory=project_factory,
+ check_project_name=project_service.check_project_name,
)
]
assert pending_gitlab_publisher_form_cls.calls == [
pretend.call(
pyramid_request.POST,
route_url=pyramid_request.route_url,
- project_factory=project_factory,
+ check_project_name=project_service.check_project_name,
)
]
@@ -3741,7 +3757,14 @@ def test_add_pending_oidc_publisher_user_cannot_register(
view_name,
flag,
publisher_name,
+ metrics,
):
+ project_service = pretend.stub(check_project_name=lambda name: None)
+ pyramid_request.find_service = lambda interface, **kwargs: {
+ IProjectService: project_service,
+ IMetricsService: metrics,
+ }[interface]
+
pyramid_request.registry = pretend.stub(
settings={
"github.token": "fake-api-token",
@@ -3757,10 +3780,6 @@ def test_add_pending_oidc_publisher_user_cannot_register(
flash=pretend.call_recorder(lambda *a, **kw: None)
)
- project_factory = pretend.stub()
- project_factory_cls = pretend.call_recorder(lambda r: project_factory)
- monkeypatch.setattr(views, "ProjectFactory", project_factory_cls)
-
pending_github_publisher_form_obj = pretend.stub()
pending_github_publisher_form_cls = pretend.call_recorder(
lambda *a, **kw: pending_github_publisher_form_obj
@@ -3839,14 +3858,14 @@ def test_add_pending_oidc_publisher_user_cannot_register(
pyramid_request.POST,
api_token="fake-api-token",
route_url=pyramid_request.route_url,
- project_factory=project_factory,
+ check_project_name=project_service.check_project_name,
)
]
assert pending_gitlab_publisher_form_cls.calls == [
pretend.call(
pyramid_request.POST,
route_url=pyramid_request.route_url,
- project_factory=project_factory,
+ check_project_name=project_service.check_project_name,
)
]
@@ -4474,6 +4493,12 @@ def test_add_pending_oidc_publisher(
def test_delete_pending_oidc_publisher_admin_disabled(
self, monkeypatch, pyramid_request
):
+ project_service = pretend.stub(check_project_name=lambda name: None)
+ pyramid_request.find_service = lambda interface, **kwargs: {
+ IProjectService: project_service,
+ IMetricsService: pretend.stub(),
+ }[interface]
+
pyramid_request.user = pretend.stub()
pyramid_request.registry = pretend.stub(
settings={
@@ -4487,10 +4512,6 @@ def test_delete_pending_oidc_publisher_admin_disabled(
flash=pretend.call_recorder(lambda *a, **kw: None)
)
- project_factory = pretend.stub()
- project_factory_cls = pretend.call_recorder(lambda r: project_factory)
- monkeypatch.setattr(views, "ProjectFactory", project_factory_cls)
-
pending_github_publisher_form_obj = pretend.stub()
pending_github_publisher_form_cls = pretend.call_recorder(
lambda *a, **kw: pending_github_publisher_form_obj
@@ -4558,14 +4579,14 @@ def test_delete_pending_oidc_publisher_admin_disabled(
pyramid_request.POST,
api_token="fake-api-token",
route_url=pyramid_request.route_url,
- project_factory=project_factory,
+ check_project_name=project_service.check_project_name,
)
]
assert pending_gitlab_publisher_form_cls.calls == [
pretend.call(
pyramid_request.POST,
route_url=pyramid_request.route_url,
- project_factory=project_factory,
+ check_project_name=project_service.check_project_name,
)
]
diff --git a/tests/unit/oidc/forms/test_activestate.py b/tests/unit/oidc/forms/test_activestate.py
index 15e288861889..17e5181f478d 100644
--- a/tests/unit/oidc/forms/test_activestate.py
+++ b/tests/unit/oidc/forms/test_activestate.py
@@ -19,6 +19,7 @@
from webob.multidict import MultiDict
from warehouse.oidc.forms import activestate
+from warehouse.packaging.interfaces import ProjectNameUnavailableReason
fake_username = "some-username"
fake_org_name = "some-org"
@@ -30,14 +31,13 @@
_requests = requests
-def _raise(exception):
- raise exception
-
-
class TestPendingActiveStatePublisherForm:
def test_validate(self, monkeypatch):
- project_factory = []
route_url = pretend.stub()
+
+ def check_project_name(name):
+ return None
+
data = MultiDict(
{
"organization": "some-org",
@@ -47,7 +47,9 @@ def test_validate(self, monkeypatch):
}
)
form = activestate.PendingActiveStatePublisherForm(
- MultiDict(data), route_url=route_url, project_factory=project_factory
+ MultiDict(data),
+ route_url=route_url,
+ check_project_name=check_project_name,
)
# Test built-in validations
@@ -55,16 +57,19 @@ def test_validate(self, monkeypatch):
monkeypatch.setattr(form, "_lookup_organization", lambda *o: None)
- assert form._project_factory == project_factory
+ assert form._check_project_name == check_project_name
assert form._route_url == route_url
assert form.validate()
def test_validate_project_name_already_in_use(self, pyramid_config):
- project_factory = ["some-project"]
route_url = pretend.call_recorder(lambda *args, **kwargs: "")
+ def check_project_name(name):
+ return ProjectNameUnavailableReason.AlreadyExists
+
form = activestate.PendingActiveStatePublisherForm(
- route_url=route_url, project_factory=project_factory
+ route_url=route_url,
+ check_project_name=check_project_name,
)
field = pretend.stub(data="some-project")
@@ -208,7 +213,7 @@ def test_lookup_actor_non_json(self, monkeypatch):
response = pretend.stub(
status_code=200,
raise_for_status=pretend.call_recorder(lambda: None),
- json=lambda: _raise(_requests.exceptions.JSONDecodeError("", "", 0)),
+ json=pretend.raiser(_requests.exceptions.JSONDecodeError("", "", 0)),
content=b"",
)
@@ -437,7 +442,7 @@ def test_lookup_organization_non_json(self, monkeypatch):
response = pretend.stub(
status_code=200,
raise_for_status=pretend.call_recorder(lambda: None),
- json=lambda: _raise(_requests.exceptions.JSONDecodeError("", "", 0)),
+ json=pretend.raiser(_requests.exceptions.JSONDecodeError("", "", 0)),
content=b"",
)
diff --git a/tests/unit/oidc/forms/test_github.py b/tests/unit/oidc/forms/test_github.py
index 19fb9b01f54a..ad88ba67a1c7 100644
--- a/tests/unit/oidc/forms/test_github.py
+++ b/tests/unit/oidc/forms/test_github.py
@@ -18,12 +18,16 @@
from webob.multidict import MultiDict
from warehouse.oidc.forms import github
+from warehouse.packaging.interfaces import ProjectNameUnavailableReason
class TestPendingGitHubPublisherForm:
def test_validate(self, monkeypatch):
- project_factory = []
route_url = pretend.stub()
+
+ def check_project_name(name):
+ return None # Name is available.
+
data = MultiDict(
{
"owner": "some-owner",
@@ -36,23 +40,24 @@ def test_validate(self, monkeypatch):
MultiDict(data),
api_token=pretend.stub(),
route_url=route_url,
- project_factory=project_factory,
+ check_project_name=check_project_name,
)
# We're testing only the basic validation here.
owner_info = {"login": "fake-username", "id": "1234"}
monkeypatch.setattr(form, "_lookup_owner", lambda o: owner_info)
- assert form._project_factory == project_factory
+ assert form._check_project_name == check_project_name
assert form._route_url == route_url
assert form.validate()
def test_validate_project_name_already_in_use(self, pyramid_config):
- project_factory = ["some-project"]
route_url = pretend.call_recorder(lambda *args, **kwargs: "")
form = github.PendingGitHubPublisherForm(
- api_token="fake-token", route_url=route_url, project_factory=project_factory
+ api_token="fake-token",
+ route_url=route_url,
+ check_project_name=lambda name: ProjectNameUnavailableReason.AlreadyExists,
)
field = pretend.stub(data="some-project")
@@ -66,6 +71,18 @@ def test_validate_project_name_already_in_use(self, pyramid_config):
)
]
+ @pytest.mark.parametrize("reason", list(ProjectNameUnavailableReason))
+ def test_validate_project_name_unavailable(self, reason, pyramid_config):
+ form = github.PendingGitHubPublisherForm(
+ api_token="fake-token",
+ route_url=pretend.call_recorder(lambda *args, **kwargs: ""),
+ check_project_name=lambda name: reason,
+ )
+
+ field = pretend.stub(data="some-project")
+ with pytest.raises(wtforms.validators.ValidationError):
+ form.validate_project_name(field)
+
class TestGitHubPublisherForm:
@pytest.mark.parametrize(
diff --git a/tests/unit/oidc/forms/test_gitlab.py b/tests/unit/oidc/forms/test_gitlab.py
index 4afa300e2b5f..08b6473f2adb 100644
--- a/tests/unit/oidc/forms/test_gitlab.py
+++ b/tests/unit/oidc/forms/test_gitlab.py
@@ -17,12 +17,16 @@
from webob.multidict import MultiDict
from warehouse.oidc.forms import gitlab
+from warehouse.packaging.interfaces import ProjectNameUnavailableReason
class TestPendingGitLabPublisherForm:
def test_validate(self, monkeypatch):
- project_factory = []
route_url = pretend.stub()
+
+ def check_project_name(name):
+ return None # Name is available.
+
data = MultiDict(
{
"namespace": "some-owner",
@@ -32,20 +36,20 @@ def test_validate(self, monkeypatch):
}
)
form = gitlab.PendingGitLabPublisherForm(
- MultiDict(data), route_url=route_url, project_factory=project_factory
+ MultiDict(data), route_url=route_url, check_project_name=check_project_name
)
assert form._route_url == route_url
- assert form._project_factory == project_factory
+ assert form._check_project_name == check_project_name
# We're testing only the basic validation here.
assert form.validate()
def test_validate_project_name_already_in_use(self, pyramid_config):
- project_factory = ["some-project"]
route_url = pretend.call_recorder(lambda *args, **kwargs: "my_url")
form = gitlab.PendingGitLabPublisherForm(
- route_url=route_url, project_factory=project_factory
+ route_url=route_url,
+ check_project_name=lambda name: ProjectNameUnavailableReason.AlreadyExists,
)
field = pretend.stub(data="some-project")
diff --git a/tests/unit/oidc/forms/test_google.py b/tests/unit/oidc/forms/test_google.py
index 1596d1dfbdc7..0da2163507f6 100644
--- a/tests/unit/oidc/forms/test_google.py
+++ b/tests/unit/oidc/forms/test_google.py
@@ -17,12 +17,16 @@
from webob.multidict import MultiDict
from warehouse.oidc.forms import google
+from warehouse.packaging.interfaces import ProjectNameUnavailableReason
class TestPendingGooglePublisherForm:
def test_validate(self, monkeypatch):
- project_factory = []
route_url = pretend.stub()
+
+ def check_project_name(name):
+ return None # Name is available.
+
data = MultiDict(
{
"sub": "some-subject",
@@ -31,18 +35,18 @@ def test_validate(self, monkeypatch):
}
)
form = google.PendingGooglePublisherForm(
- MultiDict(data), route_url=route_url, project_factory=project_factory
+ MultiDict(data), route_url=route_url, check_project_name=check_project_name
)
- assert form._project_factory == project_factory
+ assert form._check_project_name == check_project_name
assert form._route_url == route_url
assert form.validate()
def test_validate_project_name_already_in_use(self, pyramid_config):
- project_factory = ["some-project"]
route_url = pretend.call_recorder(lambda *args, **kwargs: "my_url")
form = google.PendingGooglePublisherForm(
- route_url=route_url, project_factory=project_factory
+ route_url=route_url,
+ check_project_name=lambda name: ProjectNameUnavailableReason.AlreadyExists,
)
field = pretend.stub(data="some-project")
diff --git a/tests/unit/packaging/test_services.py b/tests/unit/packaging/test_services.py
index 87de9980b057..9769fdae4838 100644
--- a/tests/unit/packaging/test_services.py
+++ b/tests/unit/packaging/test_services.py
@@ -24,7 +24,13 @@
import warehouse.packaging.services
-from warehouse.packaging.interfaces import IDocsStorage, IFileStorage, ISimpleStorage
+from warehouse.packaging.interfaces import (
+ IDocsStorage,
+ IFileStorage,
+ IProjectService,
+ ISimpleStorage,
+ ProjectNameUnavailableReason,
+)
from warehouse.packaging.services import (
B2FileStorage,
GCSFileStorage,
@@ -34,12 +40,15 @@
LocalDocsStorage,
LocalFileStorage,
LocalSimpleStorage,
+ ProjectService,
S3ArchiveFileStorage,
S3DocsStorage,
S3FileStorage,
project_service_factory,
)
+from ...common.db.packaging import ProhibitedProjectFactory, ProjectFactory
+
class TestLocalFileStorage:
def test_verify_service(self):
@@ -979,6 +988,60 @@ def test_notimplementederror(self):
GenericLocalBlobStorage.create_service(pretend.stub(), pretend.stub())
+class TestProjectService:
+ def test_verify_service(self):
+ assert verifyClass(IProjectService, ProjectService)
+
+ @pytest.mark.parametrize("name", ["", ".,;", "_z"])
+ def test_check_project_name_invalid(self, name):
+ service = ProjectService(session=pretend.stub())
+
+ assert service.check_project_name(name) is ProjectNameUnavailableReason.Invalid
+
+ @pytest.mark.parametrize("name", ["uu", "cgi", "nis", "mailcap"])
+ def test_check_project_name_stdlib(self, name):
+ service = ProjectService(session=pretend.stub())
+
+ assert service.check_project_name(name) is ProjectNameUnavailableReason.Stdlib
+
+ def test_check_project_name_already_exists(self, db_session):
+ service = ProjectService(session=db_session)
+ ProjectFactory.create(name="foo")
+
+ assert (
+ service.check_project_name("foo")
+ is ProjectNameUnavailableReason.AlreadyExists
+ )
+ assert (
+ service.check_project_name("Foo")
+ is ProjectNameUnavailableReason.AlreadyExists
+ )
+
+ def test_check_project_name_prohibited(self, db_session):
+ service = ProjectService(session=db_session)
+ ProhibitedProjectFactory.create(name="foo")
+
+ assert (
+ service.check_project_name("foo") is ProjectNameUnavailableReason.Prohibited
+ )
+ assert (
+ service.check_project_name("Foo") is ProjectNameUnavailableReason.Prohibited
+ )
+
+ def test_check_project_name_too_similar(self, db_session):
+ service = ProjectService(session=db_session)
+ ProjectFactory.create(name="f00")
+
+ assert (
+ service.check_project_name("foo") is ProjectNameUnavailableReason.TooSimilar
+ )
+
+ def test_check_project_name_ok(self, db_session):
+ service = ProjectService(session=db_session)
+
+ assert service.check_project_name("foo") is None
+
+
def test_project_service_factory():
db = pretend.stub()
request = pretend.stub(
diff --git a/warehouse/accounts/views.py b/warehouse/accounts/views.py
index 03e85dc712e1..b7cab36cf2da 100644
--- a/warehouse/accounts/views.py
+++ b/warehouse/accounts/views.py
@@ -94,10 +94,10 @@
)
from warehouse.organizations.interfaces import IOrganizationService
from warehouse.organizations.models import OrganizationRole, OrganizationRoleType
+from warehouse.packaging.interfaces import IProjectService
from warehouse.packaging.models import (
JournalEntry,
Project,
- ProjectFactory,
Release,
Role,
RoleInvitation,
@@ -1464,28 +1464,28 @@ def reauthenticate(request, _form_class=ReAuthenticateForm):
class ManageAccountPublishingViews:
def __init__(self, request):
self.request = request
- self.project_factory = ProjectFactory(request)
self.metrics = self.request.find_service(IMetricsService, context=None)
+ self.project_service = self.request.find_service(IProjectService, context=None)
self.pending_github_publisher_form = PendingGitHubPublisherForm(
self.request.POST,
api_token=self.request.registry.settings.get("github.token"),
route_url=self.request.route_url,
- project_factory=self.project_factory,
+ check_project_name=self.project_service.check_project_name,
)
self.pending_gitlab_publisher_form = PendingGitLabPublisherForm(
self.request.POST,
route_url=self.request.route_url,
- project_factory=self.project_factory,
+ check_project_name=self.project_service.check_project_name,
)
self.pending_google_publisher_form = PendingGooglePublisherForm(
self.request.POST,
route_url=self.request.route_url,
- project_factory=self.project_factory,
+ check_project_name=self.project_service.check_project_name,
)
self.pending_activestate_publisher_form = PendingActiveStatePublisherForm(
self.request.POST,
route_url=self.request.route_url,
- project_factory=self.project_factory,
+ check_project_name=self.project_service.check_project_name,
)
@property
diff --git a/warehouse/locale/messages.pot b/warehouse/locale/messages.pot
index 4e2cd43a1f3b..606c857c2339 100644
--- a/warehouse/locale/messages.pot
+++ b/warehouse/locale/messages.pot
@@ -373,7 +373,7 @@ msgstr ""
msgid "Select project"
msgstr ""
-#: warehouse/manage/forms.py:495 warehouse/oidc/forms/_core.py:22
+#: warehouse/manage/forms.py:495 warehouse/oidc/forms/_core.py:23
#: warehouse/oidc/forms/gitlab.py:43
msgid "Specify project name"
msgstr ""
@@ -624,21 +624,36 @@ msgstr ""
msgid "Expired invitation for '${username}' deleted."
msgstr ""
-#: warehouse/oidc/forms/_core.py:24 warehouse/oidc/forms/gitlab.py:45
+#: warehouse/oidc/forms/_core.py:25 warehouse/oidc/forms/_core.py:35
+#: warehouse/oidc/forms/gitlab.py:45
msgid "Invalid project name"
msgstr ""
-#: warehouse/oidc/forms/_core.py:45
+#: warehouse/oidc/forms/_core.py:49
msgid ""
-"This project already exists, use the project's publishing settings here to create a Trusted Publisher for it."
msgstr ""
-#: warehouse/oidc/forms/_core.py:65
+#: warehouse/oidc/forms/_core.py:59
+msgid "This project name isn't allowed"
+msgstr ""
+
+#: warehouse/oidc/forms/_core.py:63
+msgid "This project name is too similar to an existing project"
+msgstr ""
+
+#: warehouse/oidc/forms/_core.py:67
+msgid ""
+"This project name isn't allowed (conflict with the Python standard "
+"library module name)"
+msgstr ""
+
+#: warehouse/oidc/forms/_core.py:84
msgid "Specify a publisher ID"
msgstr ""
-#: warehouse/oidc/forms/_core.py:66
+#: warehouse/oidc/forms/_core.py:85
msgid "Publisher must be specified by ID"
msgstr ""
diff --git a/warehouse/migrations/versions/f7720656a33c_index_ultranormalized_pending_project_.py b/warehouse/migrations/versions/f7720656a33c_index_ultranormalized_pending_project_.py
new file mode 100644
index 000000000000..cce2d8eacf7f
--- /dev/null
+++ b/warehouse/migrations/versions/f7720656a33c_index_ultranormalized_pending_project_.py
@@ -0,0 +1,58 @@
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+"""
+Index ultranormalized pending project_name
+
+Revision ID: f7720656a33c
+Revises: 26455e3712a2
+Create Date: 2024-08-20 06:07:46.546659
+"""
+
+from alembic import op
+
+revision = "f7720656a33c"
+down_revision = "a8050411bc65"
+
+# Note: It is VERY important to ensure that a migration does not lock for a
+# long period of time and to ensure that each individual migration does
+# not break compatibility with the *previous* version of the code base.
+# This is because the migrations will be ran automatically as part of the
+# deployment process, but while the previous version of the code is still
+# up and running. Thus backwards incompatible changes must be broken up
+# over multiple migrations inside of multiple pull requests in order to
+# phase them in over multiple deploys.
+#
+# By default, migrations cannot wait more than 4s on acquiring a lock
+# and each individual statement cannot take more than 5s. This helps
+# prevent situations where a slow migration takes the entire site down.
+#
+# If you need to increase this timeout for a migration, you can do so
+# by adding:
+#
+# op.execute("SET statement_timeout = 5000")
+# op.execute("SET lock_timeout = 4000")
+#
+# To whatever values are reasonable for this migration as part of your
+# migration.
+
+
+def upgrade():
+ op.execute(
+ """
+ CREATE INDEX pending_project_name_ultranormalized
+ ON pending_oidc_publishers (ultranormalize_name(project_name));
+ """
+ )
+
+
+def downgrade():
+ op.execute("DROP INDEX pending_project_name_ultranormalized")
diff --git a/warehouse/oidc/forms/_core.py b/warehouse/oidc/forms/_core.py
index d3c9f29992a2..fb2af35d0001 100644
--- a/warehouse/oidc/forms/_core.py
+++ b/warehouse/oidc/forms/_core.py
@@ -13,6 +13,7 @@
import wtforms
from warehouse.i18n import localize as _
+from warehouse.packaging.interfaces import ProjectNameUnavailableReason
from warehouse.utils.project import PROJECT_NAME_RE
@@ -29,27 +30,45 @@ class PendingPublisherMixin:
def validate_project_name(self, field):
project_name = field.data
- if project_name in self._project_factory:
- url_params = {name: value for name, value in self.data.items() if value}
- url_params["provider"] = {self.provider}
- url = self._route_url(
- "manage.project.settings.publishing",
- project_name=project_name,
- _query=url_params,
- )
+ match self._check_project_name(project_name):
+ case ProjectNameUnavailableReason.Invalid:
+ raise wtforms.validators.ValidationError(_("Invalid project name"))
+ case ProjectNameUnavailableReason.AlreadyExists:
+ url_params = {name: value for name, value in self.data.items() if value}
+ url_params["provider"] = {self.provider}
+ url = self._route_url(
+ "manage.project.settings.publishing",
+ project_name=project_name,
+ _query=url_params,
+ )
- # We mark the error message as safe, so that the HTML hyperlink is
- # not escaped by Jinja
- raise wtforms.validators.ValidationError(
- markupsafe.Markup(
+ # We mark the error message as safe, so that the HTML hyperlink is
+ # not escaped by Jinja
+ raise wtforms.validators.ValidationError(
+ markupsafe.Markup(
+ _(
+ "This project already exists: use the project's publishing"
+ " settings here to create a Trusted"
+ " Publisher for it.",
+ mapping={"url": url},
+ )
+ )
+ )
+ case ProjectNameUnavailableReason.Prohibited:
+ raise wtforms.validators.ValidationError(
+ _("This project name isn't allowed")
+ )
+ case ProjectNameUnavailableReason.TooSimilar:
+ raise wtforms.validators.ValidationError(
+ _("This project name is too similar to an existing project")
+ )
+ case ProjectNameUnavailableReason.Stdlib:
+ raise wtforms.validators.ValidationError(
_(
- "This project already exists, use the project's publishing"
- " settings here to create a Trusted"
- " Publisher for it.",
- mapping={"url": url},
+ "This project name isn't allowed (conflict with the Python"
+ " standard library module name)"
)
)
- )
@property
def provider(self) -> str: # pragma: no cover
diff --git a/warehouse/oidc/forms/activestate.py b/warehouse/oidc/forms/activestate.py
index 3d3b387d526a..46b5336f7705 100644
--- a/warehouse/oidc/forms/activestate.py
+++ b/warehouse/oidc/forms/activestate.py
@@ -191,10 +191,10 @@ def validate_actor(self, field):
class PendingActiveStatePublisherForm(ActiveStatePublisherBase, PendingPublisherMixin):
__params__ = ActiveStatePublisherBase.__params__ + ["project_name"]
- def __init__(self, *args, route_url, project_factory, **kwargs):
+ def __init__(self, *args, route_url, check_project_name, **kwargs):
super().__init__(*args, **kwargs)
self._route_url = route_url
- self._project_factory = project_factory
+ self._check_project_name = check_project_name
@property
def provider(self) -> str:
diff --git a/warehouse/oidc/forms/github.py b/warehouse/oidc/forms/github.py
index 0b545f5c7afa..1ee524b5ea42 100644
--- a/warehouse/oidc/forms/github.py
+++ b/warehouse/oidc/forms/github.py
@@ -168,10 +168,10 @@ def normalized_environment(self):
class PendingGitHubPublisherForm(GitHubPublisherBase, PendingPublisherMixin):
__params__ = GitHubPublisherBase.__params__ + ["project_name"]
- def __init__(self, *args, route_url, project_factory, **kwargs):
+ def __init__(self, *args, route_url, check_project_name, **kwargs):
super().__init__(*args, **kwargs)
self._route_url = route_url
- self._project_factory = project_factory
+ self._check_project_name = check_project_name
@property
def provider(self) -> str:
diff --git a/warehouse/oidc/forms/gitlab.py b/warehouse/oidc/forms/gitlab.py
index b709f641e724..3bc5b3095d65 100644
--- a/warehouse/oidc/forms/gitlab.py
+++ b/warehouse/oidc/forms/gitlab.py
@@ -96,10 +96,10 @@ def normalized_environment(self):
class PendingGitLabPublisherForm(GitLabPublisherBase, PendingPublisherMixin):
__params__ = GitLabPublisherBase.__params__ + ["project_name"]
- def __init__(self, *args, route_url, project_factory, **kwargs):
+ def __init__(self, *args, route_url, check_project_name, **kwargs):
super().__init__(*args, **kwargs)
self._route_url = route_url
- self._project_factory = project_factory
+ self._check_project_name = check_project_name
@property
def provider(self) -> str:
diff --git a/warehouse/oidc/forms/google.py b/warehouse/oidc/forms/google.py
index f2e59add912d..13b8aa30b6fc 100644
--- a/warehouse/oidc/forms/google.py
+++ b/warehouse/oidc/forms/google.py
@@ -34,10 +34,10 @@ def __init__(self, *args, **kwargs):
class PendingGooglePublisherForm(GooglePublisherBase, PendingPublisherMixin):
__params__ = GooglePublisherBase.__params__ + ["project_name"]
- def __init__(self, *args, route_url, project_factory, **kwargs):
+ def __init__(self, *args, route_url, check_project_name, **kwargs):
super().__init__(*args, **kwargs)
self._route_url = route_url
- self._project_factory = project_factory
+ self._check_project_name = check_project_name
@property
def provider(self) -> str:
diff --git a/warehouse/oidc/models/_core.py b/warehouse/oidc/models/_core.py
index 11e872faa8d4..a7dd4d1dcb77 100644
--- a/warehouse/oidc/models/_core.py
+++ b/warehouse/oidc/models/_core.py
@@ -18,7 +18,7 @@
import rfc3986
import sentry_sdk
-from sqlalchemy import ForeignKey, String, orm
+from sqlalchemy import ForeignKey, Index, String, func, orm
from sqlalchemy.dialects.postgresql import UUID
from sqlalchemy.orm import Mapped, mapped_column
@@ -397,6 +397,12 @@ class PendingOIDCPublisher(OIDCPublisherMixin, db.Model):
)
added_by: Mapped[User] = orm.relationship(back_populates="pending_oidc_publishers")
+ __table_args__ = (
+ Index(
+ "pending_project_name_ultranormalized",
+ func.ultranormalize_name(project_name),
+ ),
+ )
__mapper_args__ = {
"polymorphic_identity": "pending_oidc_publishers",
"polymorphic_on": OIDCPublisherMixin.discriminator,
diff --git a/warehouse/oidc/models/activestate.py b/warehouse/oidc/models/activestate.py
index 8c32689485d0..514ee55caf8b 100644
--- a/warehouse/oidc/models/activestate.py
+++ b/warehouse/oidc/models/activestate.py
@@ -159,7 +159,7 @@ class ActiveStatePublisher(ActiveStatePublisherMixin, OIDCPublisher):
class PendingActiveStatePublisher(ActiveStatePublisherMixin, PendingOIDCPublisher):
__tablename__ = "pending_activestate_oidc_publishers"
__mapper_args__ = {"polymorphic_identity": "pending_activestate_oidc_publishers"}
- __table_args__ = (
+ __table_args__ = ( # type: ignore[assignment]
UniqueConstraint(
"organization",
"activestate_project_name",
diff --git a/warehouse/oidc/models/github.py b/warehouse/oidc/models/github.py
index 5822ef07adc7..05792a2c1eff 100644
--- a/warehouse/oidc/models/github.py
+++ b/warehouse/oidc/models/github.py
@@ -349,7 +349,7 @@ def verify_url(self, url: str):
class PendingGitHubPublisher(GitHubPublisherMixin, PendingOIDCPublisher):
__tablename__ = "pending_github_oidc_publishers"
__mapper_args__ = {"polymorphic_identity": "pending_github_oidc_publishers"}
- __table_args__ = (
+ __table_args__ = ( # type: ignore[assignment]
UniqueConstraint(
"repository_name",
"repository_owner",
diff --git a/warehouse/oidc/models/gitlab.py b/warehouse/oidc/models/gitlab.py
index 6b63c3619201..acab69595804 100644
--- a/warehouse/oidc/models/gitlab.py
+++ b/warehouse/oidc/models/gitlab.py
@@ -353,7 +353,7 @@ def verify_url(self, url: str):
class PendingGitLabPublisher(GitLabPublisherMixin, PendingOIDCPublisher):
__tablename__ = "pending_gitlab_oidc_publishers"
__mapper_args__ = {"polymorphic_identity": "pending_gitlab_oidc_publishers"}
- __table_args__ = (
+ __table_args__ = ( # type: ignore[assignment]
UniqueConstraint(
"namespace",
"project",
diff --git a/warehouse/oidc/models/google.py b/warehouse/oidc/models/google.py
index 63a5a580de1b..f911dfc8d445 100644
--- a/warehouse/oidc/models/google.py
+++ b/warehouse/oidc/models/google.py
@@ -125,7 +125,7 @@ class GooglePublisher(GooglePublisherMixin, OIDCPublisher):
class PendingGooglePublisher(GooglePublisherMixin, PendingOIDCPublisher):
__tablename__ = "pending_google_oidc_publishers"
__mapper_args__ = {"polymorphic_identity": "pending_google_oidc_publishers"}
- __table_args__ = (
+ __table_args__ = ( # type: ignore[assignment]
UniqueConstraint(
"email",
"sub",
diff --git a/warehouse/packaging/interfaces.py b/warehouse/packaging/interfaces.py
index d5c6c6b7fc23..ecafdf6c217a 100644
--- a/warehouse/packaging/interfaces.py
+++ b/warehouse/packaging/interfaces.py
@@ -10,6 +10,8 @@
# See the License for the specific language governing permissions and
# limitations under the License.
+import enum
+
from zope.interface import Interface
from warehouse.rate_limiting.interfaces import RateLimiterException
@@ -73,7 +75,20 @@ def remove_by_prefix(prefix):
"""
+class ProjectNameUnavailableReason(enum.Enum):
+ Invalid = "invalid"
+ Stdlib = "stdlib"
+ AlreadyExists = "already-exists"
+ Prohibited = "prohibited"
+ TooSimilar = "too-similar"
+
+
class IProjectService(Interface):
+ def check_project_name(name):
+ """
+ Check if a project name is valid and available for use.
+ """
+
def create_project(name, creator, request, *, creator_is_owner=True):
"""
Creates a new project, recording a user as its creator.
diff --git a/warehouse/packaging/services.py b/warehouse/packaging/services.py
index 1271d45e355e..b766cb3b0474 100644
--- a/warehouse/packaging/services.py
+++ b/warehouse/packaging/services.py
@@ -31,7 +31,6 @@
from packaging.utils import canonicalize_name
from pyramid.httpexceptions import HTTPBadRequest, HTTPConflict, HTTPForbidden
from sqlalchemy import exists, func
-from sqlalchemy.exc import NoResultFound
from zope.interface import implementer
from warehouse.admin.flags import AdminFlagValue
@@ -44,6 +43,7 @@
IFileStorage,
IProjectService,
ISimpleStorage,
+ ProjectNameUnavailableReason,
TooManyProjectsCreated,
)
from warehouse.packaging.models import (
@@ -443,63 +443,68 @@ def _hit_ratelimits(self, request, creator):
self.ratelimiters["project.create.user"].hit(creator.id)
self.ratelimiters["project.create.ip"].hit(request.remote_addr)
+ def check_project_name(self, name: str) -> ProjectNameUnavailableReason | None:
+ if not PROJECT_NAME_RE.match(name):
+ return ProjectNameUnavailableReason.Invalid
+
+ # Also check for collisions with Python Standard Library modules.
+ if canonicalize_name(name) in STDLIB_PROHIBITED:
+ return ProjectNameUnavailableReason.Stdlib
+
+ if self.db.query(
+ exists().where(Project.normalized_name == func.normalize_pep426_name(name))
+ ).scalar():
+ return ProjectNameUnavailableReason.AlreadyExists
+
+ if self.db.query(
+ exists().where(
+ ProhibitedProjectName.name == func.normalize_pep426_name(name)
+ )
+ ).scalar():
+ return ProjectNameUnavailableReason.Prohibited
+
+ if self.db.query(
+ exists().where(
+ func.ultranormalize_name(Project.name) == func.ultranormalize_name(name)
+ )
+ ).scalar():
+ return ProjectNameUnavailableReason.TooSimilar
+
+ return None
+
def create_project(
self, name, creator, request, *, creator_is_owner=True, ratelimited=True
):
if ratelimited:
self._check_ratelimits(request, creator)
- # Sanity check that the project name is valid. This may have already
- # happened via form validation prior to calling this function, but
- # isn't guaranteed.
- if not PROJECT_NAME_RE.match(name):
- raise HTTPBadRequest(f"The name {name!r} is invalid.")
-
- # Look up the project first before doing anything else, and fail if it
- # already exists. If it does not exist, proceed with additional checks
- # to ensure that the project has a valid name before creating it.
- try:
- # Find existing project or raise NoResultFound.
- (
- request.db.query(Project.id)
- .filter(Project.normalized_name == func.normalize_pep426_name(name))
- .one()
- )
-
- # Found existing project with conflicting name.
- raise HTTPConflict(
+ # Check for AdminFlag set by a PyPI Administrator disabling new project
+ # registration, reasons for this include Spammers, security
+ # vulnerabilities, or just wanting to be lazy and not worry ;)
+ if request.flags.enabled(AdminFlagValue.DISALLOW_NEW_PROJECT_REGISTRATION):
+ raise HTTPForbidden(
(
- "The name {name!r} conflicts with an existing project. "
+ "New project registration temporarily disabled. "
"See {projecthelp} for more information."
- ).format(
- name=name,
- projecthelp=request.help_url(_anchor="project-name"),
- ),
+ ).format(projecthelp=request.help_url(_anchor="admin-intervention")),
) from None
- except NoResultFound:
- # Check for AdminFlag set by a PyPI Administrator disabling new project
- # registration, reasons for this include Spammers, security
- # vulnerabilities, or just wanting to be lazy and not worry ;)
- if request.flags.enabled(AdminFlagValue.DISALLOW_NEW_PROJECT_REGISTRATION):
- raise HTTPForbidden(
+
+ # Verify that the project name is both valid and currently available.
+ match self.check_project_name(name):
+ case ProjectNameUnavailableReason.Invalid:
+ raise HTTPBadRequest(f"The name {name!r} is invalid.")
+ case ProjectNameUnavailableReason.AlreadyExists:
+ # Found existing project with conflicting name.
+ raise HTTPConflict(
(
- "New project registration temporarily disabled. "
+ "The name {name!r} conflicts with an existing project. "
"See {projecthelp} for more information."
).format(
- projecthelp=request.help_url(_anchor="admin-intervention")
+ name=name,
+ projecthelp=request.help_url(_anchor="project-name"),
),
) from None
-
- # Before we create the project, we're going to check our prohibited
- # names to see if this project name prohibited, or if the project name
- # is a close approximation of an existing project name. If it is,
- # then we're going to deny the request to create this project.
- _prohibited_name = request.db.query(
- exists().where(
- ProhibitedProjectName.name == func.normalize_pep426_name(name)
- )
- ).scalar()
- if _prohibited_name:
+ case ProjectNameUnavailableReason.Prohibited:
raise HTTPBadRequest(
(
"The name {name!r} isn't allowed. "
@@ -509,14 +514,7 @@ def create_project(
projecthelp=request.help_url(_anchor="project-name"),
),
) from None
-
- _ultranormalize_collision = request.db.query(
- exists().where(
- func.ultranormalize_name(Project.name)
- == func.ultranormalize_name(name)
- )
- ).scalar()
- if _ultranormalize_collision:
+ case ProjectNameUnavailableReason.TooSimilar:
raise HTTPBadRequest(
(
"The name {name!r} is too similar to an existing project. "
@@ -526,9 +524,7 @@ def create_project(
projecthelp=request.help_url(_anchor="project-name"),
),
) from None
-
- # Also check for collisions with Python Standard Library modules.
- if canonicalize_name(name) in STDLIB_PROHIBITED:
+ case ProjectNameUnavailableReason.Stdlib:
raise HTTPBadRequest(
(
"The name {name!r} isn't allowed (conflict with Python "
@@ -584,15 +580,15 @@ def create_project(
)
# Remove all pending publishers not owned by the creator.
- # There might be other pending publishers for the same project name,
- # which we've now invalidated by creating the project. These would
+ # There might be other pending publishers for the same or similar project
+ # name, which we've now invalidated by creating the project. These would
# be disposed of on use, but we explicitly dispose of them here while
# also sending emails to their owners.
stale_pending_publishers = (
request.db.query(PendingOIDCPublisher)
.filter(
- func.normalize_pep426_name(PendingOIDCPublisher.project_name)
- == func.normalize_pep426_name(project.name),
+ func.ultranormalize_name(PendingOIDCPublisher.project_name)
+ == func.ultranormalize_name(project.name),
PendingOIDCPublisher.added_by != creator,
)
.all()
diff --git a/warehouse/templates/email/pending-trusted-publisher-invalidated/body.html b/warehouse/templates/email/pending-trusted-publisher-invalidated/body.html
index 156bc9be4985..24e998234cf3 100644
--- a/warehouse/templates/email/pending-trusted-publisher-invalidated/body.html
+++ b/warehouse/templates/email/pending-trusted-publisher-invalidated/body.html
@@ -17,6 +17,7 @@
You registered a pending trusted publisher for a project
({{ project_name }}), but someone else has invalidated your
- pending publisher by creating the project before you did.
+ pending publisher by creating the project, or one with a conflicting name,
+ before you did.
{% endblock %}
diff --git a/warehouse/templates/email/pending-trusted-publisher-invalidated/body.txt b/warehouse/templates/email/pending-trusted-publisher-invalidated/body.txt
index 6994dd7cefe7..b2d1151685a2 100644
--- a/warehouse/templates/email/pending-trusted-publisher-invalidated/body.txt
+++ b/warehouse/templates/email/pending-trusted-publisher-invalidated/body.txt
@@ -16,6 +16,6 @@
{% block content %}
You registered a pending trusted publisher for a project
({{ project_name }}), but someone else has invalidated your pending publisher
-by creating the project before you did.
+by creating the project, or one with a conflicting name, before you did.
{% endblock %}