publisher_name to method. Add more factories. Publisher agnostic token handler tests

Author: th3coop

tests/common/db/oidc.py

@@ -11,18 +11,37 @@
 # limitations under the License.
 
 import factory
+import pretend
 
 from warehouse.oidc.models import (
     GitHubPublisher,
     GooglePublisher,
+    OIDCPublisher,
     PendingGitHubPublisher,
     PendingGooglePublisher,
+    PendingOIDCPublisher,
 )
 
 from .accounts import UserFactory
 from .base import WarehouseFactory
 
 
+class PendingOIDCPublisherFactory(WarehouseFactory):
+    class Meta:
+        model = PendingOIDCPublisher
+
+    id = factory.Faker("uuid4", cast_to=None)
+    project_name = "fake-nonexistent-project"
+    added_by = factory.SubFactory(UserFactory)
+
+
+class OIDCPublisherFactory(WarehouseFactory):
+    class Meta:
+        model = OIDCPublisher
+
+    id = factory.Faker("uuid4", cast_to=None)
+
+
 class GitHubPublisherFactory(WarehouseFactory):
     class Meta:
         model = GitHubPublisher

tests/unit/oidc/test_views.py

@@ -18,7 +18,7 @@
 import pytest
 
 from tests.common.db.accounts import UserFactory
-from tests.common.db.oidc import PendingGitHubPublisherFactory
+from tests.common.db.oidc import OIDCPublisherFactory, PendingOIDCPublisherFactory
 from tests.common.db.packaging import ProjectFactory
 from warehouse.events.tags import EventTag
 from warehouse.macaroons import caveats
@@ -111,7 +111,7 @@ def test_mint_token_from_github_oidc_not_enabled():
         {"token": {}},
     ],
 )
-def test_mint_token_from_github_oidc_invalid_payload(body):
+def test_mint_token_oidc_invalid_payload(body):
     class Request:
         def __init__(self):
             self.response = pretend.stub(status=None)
@@ -202,7 +202,9 @@ def test_mint_token_from_trusted_publisher_lookup_fails():
 
 def test_mint_token_from_oidc_pending_publisher_project_already_exists(db_request):
     project = ProjectFactory.create()
-    pending_publisher = PendingGitHubPublisherFactory.create(project_name=project.name)
+    pending_publisher = PendingOIDCPublisherFactory.create(
+        project_name=project.name,
+    )
 
     db_request.flags.disallow_oidc = lambda f=None: False
     db_request.body = json.dumps({"token": "faketoken"})
@@ -237,14 +239,9 @@ def test_mint_token_from_oidc_pending_publisher_ok(
     db_request,
 ):
     user = UserFactory.create()
-    pending_publisher = PendingGitHubPublisherFactory.create(
+    pending_publisher = PendingOIDCPublisherFactory.create(
         project_name="does-not-exist",
         added_by=user,
-        repository_name="bar",
-        repository_owner="foo",
-        repository_owner_id="123",
-        workflow_filename="example.yml",
-        environment="",
     )
 
     db_request.flags.disallow_oidc = lambda f=None: False
@@ -293,14 +290,9 @@ def test_mint_token_from_pending_trusted_publisher_invalidates_others(
     monkeypatch.setattr(views, "time", time)
 
     user = UserFactory.create()
-    pending_publisher = PendingGitHubPublisherFactory.create(
+    pending_publisher = PendingPublisherFactory.create(
         project_name="does-not-exist",
         added_by=user,
-        repository_name="bar",
-        repository_owner="foo",
-        repository_owner_id="123",
-        workflow_filename="example.yml",
-        environment="",
     )
 
     # Create some other pending publishers for the same nonexistent project,
@@ -309,7 +301,7 @@ def test_mint_token_from_pending_trusted_publisher_invalidates_others(
     emailed_users = []
     for project_name in ["does_not_exist", "does-not-exist", "dOeS-NoT-ExISt"]:
         user = UserFactory.create()
-        PendingGitHubPublisherFactory.create(
+        PendingPublisherFactory.create(
             project_name=project_name,
             added_by=user,
         )
@@ -342,6 +334,7 @@ def test_mint_token_from_pending_trusted_publisher_invalidates_others(
     )
     db_request.body = json.dumps({"token": token})
     db_request.remote_addr = "0.0.0.0"
+    db_request.db = pretend.stub(delete=pretend.call_recorder(lambda _S: None))
 
     ratelimiter = pretend.stub(clear=pretend.call_recorder(lambda id: None))
     ratelimiters = {
@@ -369,6 +362,10 @@ def test_mint_token_from_pending_trusted_publisher_invalidates_others(
         pretend.call(db_request.remote_addr),
     ]
 
+    assert db_request.db.delete.calls == [
+        pretend.call(pending_publisher),
+    ]
+
 
 @pytest.mark.parametrize(
     ("claims_in_token", "claims_input"),
@@ -379,7 +376,7 @@ def test_mint_token_from_pending_trusted_publisher_invalidates_others(
     ],
 )
 def test_mint_token_from_oidc_no_pending_publisher_ok(
-    monkeypatch, claims_in_token, claims_input
+    monkeypatch, db_request, claims_in_token, claims_input
 ):
     time = pretend.stub(time=pretend.call_recorder(lambda: 0))
     monkeypatch.setattr(views, "time", time)
@@ -389,14 +386,10 @@ def test_mint_token_from_oidc_no_pending_publisher_ok(
         project, "record_event", pretend.call_recorder(lambda **kw: None)
     )
 
-    publisher = github.GitHubPublisher(
-        repository_name="fakerepo",
-        repository_owner="fakeowner",
-        repository_owner_id="fakeid",
-        workflow_filename="fakeworkflow.yml",
-        environment="fakeenv",
-    )
+    publisher = OIDCPublisherFactory.create()
     publisher.projects = [project]
+    publisher.publisher_name = pretend.call_recorder(lambda **kw: "FakePublisher")
+    publisher.publisher_url = pretend.call_recorder(lambda **kw: "FakePubURL")
     # NOTE: Can't set __str__ using pretend.stub()
     monkeypatch.setattr(publisher, "id", "fakepublisherid")
 
@@ -425,16 +418,14 @@ def find_service(iface, **kw):
             return macaroon_service
         assert False, iface
 
-    request = pretend.stub(
-        response=pretend.stub(status=None),
-        body=json.dumps({"token": "faketoken"}),
-        find_service=find_service,
-        domain="fakedomain",
-        remote_addr="0.0.0.0",
-        flags=pretend.stub(disallow_oidc=lambda *a: False),
-    )
+    db_request.response = pretend.stub(status=None)
+    db_request.body = json.dumps({"token": "faketoken"})
+    db_request.find_service = find_service
+    db_request.domain = "fakedomain"
+    db_request.remote_addr = "0.0.0.0"
+    db_request.flags = pretend.stub(disallow_oidc=lambda *a: False)
 
-    response = views.mint_token(oidc_service, request)
+    response = views.mint_token(oidc_service, db_request)
     assert response == {
         "success": True,
         "token": "raw-macaroon",
@@ -448,7 +439,7 @@ def find_service(iface, **kw):
     assert macaroon_service.create_macaroon.calls == [
         pretend.call(
             "fakedomain",
-            f"OpenID token: fakeworkflow.yml ({datetime.fromtimestamp(0).isoformat()})",
+            f"OpenID token: OIDCPublisher(discriminator='oidc_publishers', id='fakepublisherid') ({datetime.fromtimestamp(0).isoformat()})",
             [
                 caveats.OIDCPublisher(
                     oidc_publisher_id="fakepublisherid",
@@ -463,11 +454,11 @@ def find_service(iface, **kw):
     assert project.record_event.calls == [
         pretend.call(
             tag=EventTag.Project.ShortLivedAPITokenAdded,
-            request=request,
+            request=db_request,
             additional={
                 "expires": 900,
-                "publisher_name": "GitHub",
-                "publisher_url": f"https://github.com/{publisher.repository_owner}/{publisher.repository_name}",  # noqa
+                "publisher_name": "FakePublisher",
+                "publisher_url": "FakePubURL",
             },
         )
     ]

warehouse/oidc/models/_core.py

@@ -225,14 +225,11 @@ def verify_claims(self, signed_claims: SignedClaims):
 
         return True
 
-    @property
     def publisher_name(self) -> str:  # pragma: no cover
         # Only concrete subclasses are constructed.
         raise NotImplementedError
 
-    def publisher_url(
-        self, claims: SignedClaims | None = None
-    ) -> str:  # pragma: no cover
+    def publisher_url(self, claims: SignedClaims | None = None) -> str | None:
         """
         NOTE: This is **NOT** a `@property` because we pass `claims` to it.
         When calling, make sure to use `publisher_url()`

warehouse/oidc/views.py

@@ -250,7 +250,7 @@ def mint_token(oidc_service: OIDCPublisherService, request: Request) -> JsonResp
                 request=request,
                 additional={
                     "expires": expires_at,
-                    "publisher_name": publisher.publisher_name,
+                    "publisher_name": publisher.publisher_name(),
                     "publisher_url": publisher.publisher_url(),
                 },
             )