Skip to content

Commit d6d91d5

Browse files
twmdi
andauthored
Trusted publishing: surface pending publisher name collisions (#16260)
* IProjectService.check_project_name Extract the distribution name validation logic into a separate function for reuse elsewhere. * Wire pending publisher forms to IProjectService * Update tests * Invalidate any conflicting publisher with a similar name * Add pending_project_name_ultranormalized index * Drop redundant normalization clause * Update translations * Remove unnecessary or_ Co-authored-by: Dustin Ingram <[email protected]> * Rebase migration --------- Co-authored-by: Dustin Ingram <[email protected]>
1 parent 304671b commit d6d91d5

File tree

23 files changed

+387
-163
lines changed

23 files changed

+387
-163
lines changed

tests/unit/accounts/test_views.py

Lines changed: 54 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@
6565
OrganizationRole,
6666
OrganizationRoleType,
6767
)
68+
from warehouse.packaging.interfaces import IProjectService
6869
from warehouse.packaging.models import Role, RoleInvitation
6970
from warehouse.rate_limiting.interfaces import IRateLimiter
7071

@@ -3306,8 +3307,17 @@ def test_reauth_no_user(self, monkeypatch, pyramid_request):
33063307

33073308
class TestManageAccountPublishingViews:
33083309
def test_initializes(self, metrics):
3310+
project_service = pretend.stub(check_project_name=lambda name: None)
3311+
3312+
def find_service(iface, name=None, context=None):
3313+
if iface is IMetricsService:
3314+
return metrics
3315+
if iface is IProjectService:
3316+
return project_service
3317+
return pretend.stub()
3318+
33093319
request = pretend.stub(
3310-
find_service=pretend.call_recorder(lambda *a, **kw: metrics),
3320+
find_service=pretend.call_recorder(find_service),
33113321
route_url=pretend.stub(),
33123322
POST=MultiDict(),
33133323
registry=pretend.stub(
@@ -3320,9 +3330,11 @@ def test_initializes(self, metrics):
33203330

33213331
assert view.request is request
33223332
assert view.metrics is metrics
3333+
assert view.project_service is project_service
33233334

33243335
assert view.request.find_service.calls == [
3325-
pretend.call(IMetricsService, context=None)
3336+
pretend.call(IMetricsService, context=None),
3337+
pretend.call(IProjectService, context=None),
33263338
]
33273339

33283340
@pytest.mark.parametrize(
@@ -3348,6 +3360,8 @@ def test_ratelimiting(self, metrics, ip_exceeded, user_exceeded):
33483360
def find_service(iface, name=None, context=None):
33493361
if iface is IMetricsService:
33503362
return metrics
3363+
if iface is IProjectService:
3364+
return pretend.stub(check_project_name=lambda name: None)
33513365

33523366
if name == "user_oidc.publisher.register":
33533367
return user_rate_limiter
@@ -3375,6 +3389,7 @@ def find_service(iface, name=None, context=None):
33753389
}
33763390
assert request.find_service.calls == [
33773391
pretend.call(IMetricsService, context=None),
3392+
pretend.call(IProjectService, context=None),
33783393
pretend.call(IRateLimiter, name="user_oidc.publisher.register"),
33793394
pretend.call(IRateLimiter, name="ip_oidc.publisher.register"),
33803395
]
@@ -3402,16 +3417,17 @@ def test_manage_publishing(self, metrics, monkeypatch):
34023417
"github.token": "fake-api-token",
34033418
}
34043419
),
3405-
find_service=pretend.call_recorder(lambda *a, **kw: metrics),
3420+
find_service=lambda svc, **kw: {
3421+
IMetricsService: metrics,
3422+
IProjectService: project_service,
3423+
}[svc],
34063424
flags=pretend.stub(
34073425
disallow_oidc=pretend.call_recorder(lambda f=None: False)
34083426
),
34093427
POST=pretend.stub(),
34103428
)
34113429

3412-
project_factory = pretend.stub()
3413-
project_factory_cls = pretend.call_recorder(lambda r: project_factory)
3414-
monkeypatch.setattr(views, "ProjectFactory", project_factory_cls)
3430+
project_service = pretend.stub(check_project_name=lambda name: None)
34153431

34163432
pending_github_publisher_form_obj = pretend.stub()
34173433
pending_github_publisher_form_cls = pretend.call_recorder(
@@ -3466,24 +3482,26 @@ def test_manage_publishing(self, metrics, monkeypatch):
34663482
pretend.call(AdminFlagValue.DISALLOW_GOOGLE_OIDC),
34673483
pretend.call(AdminFlagValue.DISALLOW_ACTIVESTATE_OIDC),
34683484
]
3469-
assert project_factory_cls.calls == [pretend.call(request)]
34703485
assert pending_github_publisher_form_cls.calls == [
34713486
pretend.call(
34723487
request.POST,
34733488
api_token="fake-api-token",
34743489
route_url=route_url,
3475-
project_factory=project_factory,
3490+
check_project_name=project_service.check_project_name,
34763491
)
34773492
]
34783493
assert pending_gitlab_publisher_form_cls.calls == [
34793494
pretend.call(
34803495
request.POST,
34813496
route_url=route_url,
3482-
project_factory=project_factory,
3497+
check_project_name=project_service.check_project_name,
34833498
)
34843499
]
34853500

34863501
def test_manage_publishing_admin_disabled(self, monkeypatch, pyramid_request):
3502+
project_service = pretend.stub(check_project_name=lambda name: None)
3503+
pyramid_request.find_service = lambda _, **kw: project_service
3504+
34873505
pyramid_request.user = pretend.stub()
34883506
pyramid_request.registry = pretend.stub(
34893507
settings={
@@ -3497,10 +3515,6 @@ def test_manage_publishing_admin_disabled(self, monkeypatch, pyramid_request):
34973515
flash=pretend.call_recorder(lambda *a, **kw: None)
34983516
)
34993517

3500-
project_factory = pretend.stub()
3501-
project_factory_cls = pretend.call_recorder(lambda r: project_factory)
3502-
monkeypatch.setattr(views, "ProjectFactory", project_factory_cls)
3503-
35043518
pending_github_publisher_form_obj = pretend.stub()
35053519
pending_github_publisher_form_cls = pretend.call_recorder(
35063520
lambda *a, **kw: pending_github_publisher_form_obj
@@ -3568,14 +3582,14 @@ def test_manage_publishing_admin_disabled(self, monkeypatch, pyramid_request):
35683582
pyramid_request.POST,
35693583
api_token="fake-api-token",
35703584
route_url=pyramid_request.route_url,
3571-
project_factory=project_factory,
3585+
check_project_name=project_service.check_project_name,
35723586
)
35733587
]
35743588
assert pending_gitlab_publisher_form_cls.calls == [
35753589
pretend.call(
35763590
pyramid_request.POST,
35773591
route_url=pyramid_request.route_url,
3578-
project_factory=project_factory,
3592+
check_project_name=project_service.check_project_name,
35793593
)
35803594
]
35813595

@@ -3607,6 +3621,12 @@ def test_manage_publishing_admin_disabled(self, monkeypatch, pyramid_request):
36073621
def test_add_pending_oidc_publisher_admin_disabled(
36083622
self, monkeypatch, pyramid_request, view_name, flag, publisher_name
36093623
):
3624+
project_service = pretend.stub(check_project_name=lambda name: None)
3625+
pyramid_request.find_service = lambda interface, **kwargs: {
3626+
IProjectService: project_service,
3627+
IMetricsService: pretend.stub(),
3628+
}[interface]
3629+
36103630
pyramid_request.user = pretend.stub()
36113631
pyramid_request.registry = pretend.stub(
36123632
settings={
@@ -3620,10 +3640,6 @@ def test_add_pending_oidc_publisher_admin_disabled(
36203640
flash=pretend.call_recorder(lambda *a, **kw: None)
36213641
)
36223642

3623-
project_factory = pretend.stub()
3624-
project_factory_cls = pretend.call_recorder(lambda r: project_factory)
3625-
monkeypatch.setattr(views, "ProjectFactory", project_factory_cls)
3626-
36273643
pending_github_publisher_form_obj = pretend.stub()
36283644
pending_github_publisher_form_cls = pretend.call_recorder(
36293645
lambda *a, **kw: pending_github_publisher_form_obj
@@ -3698,14 +3714,14 @@ def test_add_pending_oidc_publisher_admin_disabled(
36983714
pyramid_request.POST,
36993715
api_token="fake-api-token",
37003716
route_url=pyramid_request.route_url,
3701-
project_factory=project_factory,
3717+
check_project_name=project_service.check_project_name,
37023718
)
37033719
]
37043720
assert pending_gitlab_publisher_form_cls.calls == [
37053721
pretend.call(
37063722
pyramid_request.POST,
37073723
route_url=pyramid_request.route_url,
3708-
project_factory=project_factory,
3724+
check_project_name=project_service.check_project_name,
37093725
)
37103726
]
37113727

@@ -3741,7 +3757,14 @@ def test_add_pending_oidc_publisher_user_cannot_register(
37413757
view_name,
37423758
flag,
37433759
publisher_name,
3760+
metrics,
37443761
):
3762+
project_service = pretend.stub(check_project_name=lambda name: None)
3763+
pyramid_request.find_service = lambda interface, **kwargs: {
3764+
IProjectService: project_service,
3765+
IMetricsService: metrics,
3766+
}[interface]
3767+
37453768
pyramid_request.registry = pretend.stub(
37463769
settings={
37473770
"github.token": "fake-api-token",
@@ -3757,10 +3780,6 @@ def test_add_pending_oidc_publisher_user_cannot_register(
37573780
flash=pretend.call_recorder(lambda *a, **kw: None)
37583781
)
37593782

3760-
project_factory = pretend.stub()
3761-
project_factory_cls = pretend.call_recorder(lambda r: project_factory)
3762-
monkeypatch.setattr(views, "ProjectFactory", project_factory_cls)
3763-
37643783
pending_github_publisher_form_obj = pretend.stub()
37653784
pending_github_publisher_form_cls = pretend.call_recorder(
37663785
lambda *a, **kw: pending_github_publisher_form_obj
@@ -3839,14 +3858,14 @@ def test_add_pending_oidc_publisher_user_cannot_register(
38393858
pyramid_request.POST,
38403859
api_token="fake-api-token",
38413860
route_url=pyramid_request.route_url,
3842-
project_factory=project_factory,
3861+
check_project_name=project_service.check_project_name,
38433862
)
38443863
]
38453864
assert pending_gitlab_publisher_form_cls.calls == [
38463865
pretend.call(
38473866
pyramid_request.POST,
38483867
route_url=pyramid_request.route_url,
3849-
project_factory=project_factory,
3868+
check_project_name=project_service.check_project_name,
38503869
)
38513870
]
38523871

@@ -4474,6 +4493,12 @@ def test_add_pending_oidc_publisher(
44744493
def test_delete_pending_oidc_publisher_admin_disabled(
44754494
self, monkeypatch, pyramid_request
44764495
):
4496+
project_service = pretend.stub(check_project_name=lambda name: None)
4497+
pyramid_request.find_service = lambda interface, **kwargs: {
4498+
IProjectService: project_service,
4499+
IMetricsService: pretend.stub(),
4500+
}[interface]
4501+
44774502
pyramid_request.user = pretend.stub()
44784503
pyramid_request.registry = pretend.stub(
44794504
settings={
@@ -4487,10 +4512,6 @@ def test_delete_pending_oidc_publisher_admin_disabled(
44874512
flash=pretend.call_recorder(lambda *a, **kw: None)
44884513
)
44894514

4490-
project_factory = pretend.stub()
4491-
project_factory_cls = pretend.call_recorder(lambda r: project_factory)
4492-
monkeypatch.setattr(views, "ProjectFactory", project_factory_cls)
4493-
44944515
pending_github_publisher_form_obj = pretend.stub()
44954516
pending_github_publisher_form_cls = pretend.call_recorder(
44964517
lambda *a, **kw: pending_github_publisher_form_obj
@@ -4558,14 +4579,14 @@ def test_delete_pending_oidc_publisher_admin_disabled(
45584579
pyramid_request.POST,
45594580
api_token="fake-api-token",
45604581
route_url=pyramid_request.route_url,
4561-
project_factory=project_factory,
4582+
check_project_name=project_service.check_project_name,
45624583
)
45634584
]
45644585
assert pending_gitlab_publisher_form_cls.calls == [
45654586
pretend.call(
45664587
pyramid_request.POST,
45674588
route_url=pyramid_request.route_url,
4568-
project_factory=project_factory,
4589+
check_project_name=project_service.check_project_name,
45694590
)
45704591
]
45714592

tests/unit/oidc/forms/test_activestate.py

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
from webob.multidict import MultiDict
2020

2121
from warehouse.oidc.forms import activestate
22+
from warehouse.packaging.interfaces import ProjectNameUnavailableReason
2223

2324
fake_username = "some-username"
2425
fake_org_name = "some-org"
@@ -30,14 +31,13 @@
3031
_requests = requests
3132

3233

33-
def _raise(exception):
34-
raise exception
35-
36-
3734
class TestPendingActiveStatePublisherForm:
3835
def test_validate(self, monkeypatch):
39-
project_factory = []
4036
route_url = pretend.stub()
37+
38+
def check_project_name(name):
39+
return None
40+
4141
data = MultiDict(
4242
{
4343
"organization": "some-org",
@@ -47,24 +47,29 @@ def test_validate(self, monkeypatch):
4747
}
4848
)
4949
form = activestate.PendingActiveStatePublisherForm(
50-
MultiDict(data), route_url=route_url, project_factory=project_factory
50+
MultiDict(data),
51+
route_url=route_url,
52+
check_project_name=check_project_name,
5153
)
5254

5355
# Test built-in validations
5456
monkeypatch.setattr(form, "_lookup_actor", lambda *o: {"user_id": "some-id"})
5557

5658
monkeypatch.setattr(form, "_lookup_organization", lambda *o: None)
5759

58-
assert form._project_factory == project_factory
60+
assert form._check_project_name == check_project_name
5961
assert form._route_url == route_url
6062
assert form.validate()
6163

6264
def test_validate_project_name_already_in_use(self, pyramid_config):
63-
project_factory = ["some-project"]
6465
route_url = pretend.call_recorder(lambda *args, **kwargs: "")
6566

67+
def check_project_name(name):
68+
return ProjectNameUnavailableReason.AlreadyExists
69+
6670
form = activestate.PendingActiveStatePublisherForm(
67-
route_url=route_url, project_factory=project_factory
71+
route_url=route_url,
72+
check_project_name=check_project_name,
6873
)
6974

7075
field = pretend.stub(data="some-project")
@@ -208,7 +213,7 @@ def test_lookup_actor_non_json(self, monkeypatch):
208213
response = pretend.stub(
209214
status_code=200,
210215
raise_for_status=pretend.call_recorder(lambda: None),
211-
json=lambda: _raise(_requests.exceptions.JSONDecodeError("", "", 0)),
216+
json=pretend.raiser(_requests.exceptions.JSONDecodeError("", "", 0)),
212217
content=b"",
213218
)
214219

@@ -437,7 +442,7 @@ def test_lookup_organization_non_json(self, monkeypatch):
437442
response = pretend.stub(
438443
status_code=200,
439444
raise_for_status=pretend.call_recorder(lambda: None),
440-
json=lambda: _raise(_requests.exceptions.JSONDecodeError("", "", 0)),
445+
json=pretend.raiser(_requests.exceptions.JSONDecodeError("", "", 0)),
441446
content=b"",
442447
)
443448

0 commit comments

Comments
 (0)