diff --git a/tests/unit/forklift/test_legacy.py b/tests/unit/forklift/test_legacy.py index 5903de734636..a080588d3f03 100644 --- a/tests/unit/forklift/test_legacy.py +++ b/tests/unit/forklift/test_legacy.py @@ -3255,6 +3255,9 @@ def test_upload_succeeds_creates_release( identity.username if test_with_user else "OpenID created token" ), "canonical_version": release.canonical_version, + "base_publisher_url": ( + f"{identity.publisher.publisher_url()}" if not test_with_user else None + ), "publisher_url": ( f"{identity.publisher.publisher_url()}/commit/somesha" if not test_with_user @@ -3269,6 +3272,9 @@ def test_upload_succeeds_creates_release( identity.username if test_with_user else "OpenID created token" ), "canonical_version": release.canonical_version, + "base_publisher_url": ( + f"{identity.publisher.publisher_url()}" if not test_with_user else None + ), "publisher_url": ( f"{identity.publisher.publisher_url()}/commit/somesha" if not test_with_user diff --git a/tests/unit/oidc/models/test_activestate.py b/tests/unit/oidc/models/test_activestate.py index f03e23fdf20f..b44b12b66662 100644 --- a/tests/unit/oidc/models/test_activestate.py +++ b/tests/unit/oidc/models/test_activestate.py @@ -84,6 +84,18 @@ def test_publisher_name(self): assert publisher.publisher_name == "ActiveState" + def test_base_publisher_url(self): + org_name = "fakeorg" + project_name = "fakeproject" + publisher = ActiveStatePublisher( + organization=org_name, activestate_project_name=project_name + ) + + assert ( + publisher.base_publisher_url() + == f"https://platform.activestate.com/{org_name}/{project_name}" + ) + def test_publisher_url(self): org_name = "fakeorg" project_name = "fakeproject" diff --git a/tests/unit/oidc/models/test_github.py b/tests/unit/oidc/models/test_github.py index a0bf7826dfec..a3fd038a9978 100644 --- a/tests/unit/oidc/models/test_github.py +++ b/tests/unit/oidc/models/test_github.py @@ -96,6 +96,7 @@ def test_github_publisher_computed_properties(self): assert getattr(publisher, claim_name) is not None assert str(publisher) == "fakeworkflow.yml" + assert publisher.base_publisher_url() == "https://github.com/fakeowner/fakerepo" assert publisher.publisher_url() == "https://github.com/fakeowner/fakerepo" assert ( publisher.publisher_url({"sha": "somesha"}) diff --git a/tests/unit/oidc/models/test_gitlab.py b/tests/unit/oidc/models/test_gitlab.py index fa0ac48b3fc8..ca59410e71a4 100644 --- a/tests/unit/oidc/models/test_gitlab.py +++ b/tests/unit/oidc/models/test_gitlab.py @@ -94,6 +94,7 @@ def test_gitlab_publisher_computed_properties(self): assert getattr(publisher, claim_name) is not None assert str(publisher) == "subfolder/fakeworkflow.yml" + assert publisher.base_publisher_url() == "https://gitlab.com/fakeowner/fakerepo" assert publisher.publisher_url() == "https://gitlab.com/fakeowner/fakerepo" assert ( publisher.publisher_url({"sha": "somesha"}) diff --git a/tests/unit/oidc/models/test_google.py b/tests/unit/oidc/models/test_google.py index d0ce98e689d3..aa49b187b28b 100644 --- a/tests/unit/oidc/models/test_google.py +++ b/tests/unit/oidc/models/test_google.py @@ -33,6 +33,11 @@ def test_publisher_name(self): assert publisher.publisher_name == "Google" + def test_base_publisher_url(self): + publisher = google.GooglePublisher(email="fake@example.com") + + assert publisher.base_publisher_url() is None + def test_publisher_url(self): publisher = google.GooglePublisher(email="fake@example.com") diff --git a/tests/unit/packaging/test_models.py b/tests/unit/packaging/test_models.py index 5eaf18a33a50..147fc0c67287 100644 --- a/tests/unit/packaging/test_models.py +++ b/tests/unit/packaging/test_models.py @@ -759,17 +759,18 @@ def test_trusted_published_all(self, db_session): assert release.trusted_published @pytest.mark.parametrize( - "url, publisher_url, expected", + "url, base_publisher_url, expected", [ ("xpto.com", "https://pub/url/", False), # Totally different ("https://pub/", "https://pub/url/", False), # Missing parts - ("https://pub/url/", "https://pub/url/", True), # Exactly the same + ("https://pub/url/", "https://pub/url/", True), # Exactly the same with / + ("https://pub/url", "https://pub/url", True), # The same without / ("https://pub/url/blah.md", "https://pub/url/", True), # Additonal parts ("https://pub/url", "https://pub/url/", True), # Missing trailing slash ("https://pub/url/", "https://pub/url", True), # Extratrailing slash ], ) - def test_is_url_verified(self, db_session, url, publisher_url, expected): + def test_is_url_verified(self, db_session, url, base_publisher_url, expected): project = DBProjectFactory.create() release = DBReleaseFactory.create(project=project) release_file = DBFileFactory.create( @@ -780,12 +781,12 @@ def test_is_url_verified(self, db_session, url, publisher_url, expected): DBFileEventFactory.create( source=release_file, tag="fake:event", - additional={"publisher_url": publisher_url}, + additional={"base_publisher_url": base_publisher_url}, ) DBFileEventFactory.create( source=release_file, tag="fake:event", - additional={"publisher_url": publisher_url}, + additional={"base_publisher_url": base_publisher_url}, ) assert project.is_verified_url(url) is expected diff --git a/warehouse/forklift/legacy.py b/warehouse/forklift/legacy.py index 8485e0795154..e7dddfd1aba3 100644 --- a/warehouse/forklift/legacy.py +++ b/warehouse/forklift/legacy.py @@ -800,6 +800,11 @@ def file_upload(request): request.user.username if request.user else "OpenID created token" ), "canonical_version": release.canonical_version, + "base_publisher_url": ( + request.oidc_publisher.base_publisher_url(request.oidc_claims) + if request.oidc_publisher + else None + ), "publisher_url": ( request.oidc_publisher.publisher_url(request.oidc_claims) if request.oidc_publisher @@ -1120,6 +1125,11 @@ def file_upload(request): request.user.username if request.user else "OpenID created token" ), "canonical_version": release.canonical_version, + "base_publisher_url": ( + request.oidc_publisher.base_publisher_url(request.oidc_claims) + if request.oidc_publisher + else None + ), "publisher_url": ( request.oidc_publisher.publisher_url(request.oidc_claims) if request.oidc_publisher diff --git a/warehouse/oidc/models/_core.py b/warehouse/oidc/models/_core.py index 336dddf130b9..d6d06a79a604 100644 --- a/warehouse/oidc/models/_core.py +++ b/warehouse/oidc/models/_core.py @@ -235,6 +235,16 @@ def publisher_name(self) -> str: # pragma: no cover # Only concrete subclasses are constructed. raise NotImplementedError + def base_publisher_url( + self, claims: SignedClaims | None = None + ) -> str | None: # pragma: no cover + """ + NOTE: This is **NOT** a `@property` because we pass `claims` to it. + When calling, make sure to use `publisher_url()` + """ + # Only concrete subclasses are constructed. + raise NotImplementedError + def publisher_url( self, claims: SignedClaims | None = None ) -> str | None: # pragma: no cover diff --git a/warehouse/oidc/models/activestate.py b/warehouse/oidc/models/activestate.py index b5939f3b4a82..45b50ec95bd6 100644 --- a/warehouse/oidc/models/activestate.py +++ b/warehouse/oidc/models/activestate.py @@ -118,11 +118,14 @@ def publisher_name(self) -> str: def project(self) -> str: return self.activestate_project_name - def publisher_url(self, claims: SignedClaims | None = None) -> str: + def base_publisher_url(self, claims: SignedClaims | None = None) -> str: return urllib.parse.urljoin( _ACTIVESTATE_URL, f"{self.organization}/{self.activestate_project_name}" ) + def publisher_url(self, claims: SignedClaims | None = None) -> str: + return self.base_publisher_url(claims) + def stored_claims(self, claims=None): return {} diff --git a/warehouse/oidc/models/github.py b/warehouse/oidc/models/github.py index 840039355a8e..cd93f1c260f5 100644 --- a/warehouse/oidc/models/github.py +++ b/warehouse/oidc/models/github.py @@ -227,8 +227,11 @@ def job_workflow_ref(self): def sub(self): return f"repo:{self.repository}" + def base_publisher_url(self, claims=None): + return f"https://github.com/{self.repository}" + def publisher_url(self, claims=None): - base = f"https://github.com/{self.repository}" + base = self.base_publisher_url(claims) sha = claims.get("sha") if claims else None if sha: diff --git a/warehouse/oidc/models/gitlab.py b/warehouse/oidc/models/gitlab.py index 33d2fc947260..bebd15e53abb 100644 --- a/warehouse/oidc/models/gitlab.py +++ b/warehouse/oidc/models/gitlab.py @@ -221,8 +221,11 @@ def ci_config_ref_uri(self): def publisher_name(self): return "GitLab" + def base_publisher_url(self, claims=None): + return f"https://gitlab.com/{self.project_path}" + def publisher_url(self, claims=None): - base = f"https://gitlab.com/{self.project_path}" + base = self.base_publisher_url(claims) return f"{base}/commit/{claims['sha']}" if claims else base def stored_claims(self, claims=None): diff --git a/warehouse/oidc/models/google.py b/warehouse/oidc/models/google.py index acd660f7d355..c084b07643d0 100644 --- a/warehouse/oidc/models/google.py +++ b/warehouse/oidc/models/google.py @@ -81,6 +81,9 @@ def __lookup_no_sub__(klass, signed_claims: SignedClaims) -> Query | None: def publisher_name(self): return "Google" + def base_publisher_url(self, claims=None): + return None + def publisher_url(self, claims=None): return None diff --git a/warehouse/packaging/models.py b/warehouse/packaging/models.py index 69846f5b8064..570d9f19f0ef 100644 --- a/warehouse/packaging/models.py +++ b/warehouse/packaging/models.py @@ -411,7 +411,7 @@ def is_verified_url(self, url): .filter(Project.id == self.id) .filter( literal(url).ilike( - File.Event.additional.op("->>")("publisher_url") + "%" + File.Event.additional.op("->>")("base_publisher_url") + "%" ) ) .first()