Review cleanup

tests/unit/oidc/test_views.py

@@ -383,11 +383,11 @@ def test_mint_token_from_oidc_pending_publisher_ok(
     pending_publisher = PendingGitHubPublisherFactory.create(
         project_name="does-not-exist",
         added_by=user,
-        workflow_filename="example.yml",
-        environment="fake",
         repository_name="bar",
         repository_owner="foo",
         repository_owner_id="123",
+        workflow_filename="example.yml",
+        environment="",
     )
 
     db_request.flags.disallow_oidc = lambda f=None: False
@@ -419,13 +419,13 @@ def test_mint_token_from_pending_trusted_publisher_invalidates_others(
 
     user = UserFactory.create()
     pending_publisher = PendingGitHubPublisherFactory.create(
-        project_name="DOES-NOT-EXIST",
+        project_name="does-not-exist",
         added_by=user,
-        workflow_filename="example.yml",
-        environment="fake",
         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,

warehouse/oidc/models/_core.py

@@ -140,6 +140,7 @@ def lookup_by_claims(cls, session, signed_claims: SignedClaims):
                 # We might not build a query if we know the claim set can't
                 # satisfy it. If that's the case, then we skip.
                 continue
+
             if publisher := query.with_session(session).one_or_none():
                 return publisher
         raise InvalidPublisherError("All lookup strategies exhausted")

warehouse/oidc/models/activestate.py

@@ -88,8 +88,6 @@ class ActiveStatePublisherMixin:
     }
 
     __optional_verifiable_claims__: dict[str, CheckClaimCallable[Any]] = {
-        # Should we check that the pypi projectname matches this ingredient
-        # name?
         "ingredient": oidccore.check_claim_binary(str.__eq__),
     }
 
@@ -130,12 +128,10 @@ def publisher_name(self) -> str:
     def project(self) -> str:
         return self.activestate_project_name
 
-    @property
-    def activestate_project_path(self) -> str:
-        return f"{self.organization}/{self.activestate_project_name}"
-
     def publisher_url(self, claims: SignedClaims | None = None) -> str:
-        return urllib.parse.urljoin(ACTIVESTATE_URL, self.activestate_project_path)
+        return urllib.parse.urljoin(
+            ACTIVESTATE_URL, f"{self.organization}/{self.activestate_project_name}"
+        )
 
     def __str__(self) -> str:
         return self.publisher_url()
@@ -148,7 +144,9 @@ class ActiveStatePublisher(ActiveStatePublisherMixin, OIDCPublisher):
         UniqueConstraint(
             "organization",
             "activestate_project_name",
-            # This field is NOT populated from the form but from
+            # This field is NOT populated from the form but from an API call to
+            # ActiveState. We make the API call to confirm that the `actor`
+            # provided actually exists.
             "actor_id",
             name="_activestate_oidc_publisher_uc",
         ),

warehouse/oidc/models/github.py

@@ -269,7 +269,6 @@ def reify(self, session):
         Returns a `GitHubPublisher` for this `PendingGitHubPublisher`,
         deleting the `PendingGitHubPublisher` in the process.
         """
-        print("Github.reify")
         maybe_publisher = (
             session.query(GitHubPublisher)
             .filter(
@@ -280,7 +279,6 @@ def reify(self, session):
             )
             .one_or_none()
         )
-        print(f"Github.reify: {maybe_publisher}")
 
         publisher = maybe_publisher or GitHubPublisher(
             repository_name=self.repository_name,
@@ -289,7 +287,6 @@ def reify(self, session):
             workflow_filename=self.workflow_filename,
             environment=self.environment,
         )
-        print(f"Github.reify: {publisher}")
 
         session.delete(self)
         return publisher

warehouse/oidc/models/google.py

@@ -9,6 +9,7 @@
 # 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.
+
 from typing import Any
 
 from sqlalchemy import ForeignKey, String, UniqueConstraint

warehouse/oidc/utils.py

@@ -37,7 +37,7 @@
 OIDC_ISSUER_SERVICE_NAMES = {
     GITHUB_OIDC_ISSUER_URL: "github",
     GOOGLE_OIDC_ISSUER_URL: "google",
-    ACTIVESTATE_OIDC_ISSUER_URL: "google",
+    ACTIVESTATE_OIDC_ISSUER_URL: "activestate",
 }
 
 OIDC_ISSUER_ADMIN_FLAGS = {
@@ -80,6 +80,7 @@ def find_publisher_by_issuer(session, issuer_url, signed_claims, *, pending=Fals
         # This indicates a logic error, since we shouldn't have verified
         # claims for an issuer that we don't recognize and support.
         raise InvalidPublisherError(f"Issuer {issuer_url!r} is unsupported")
+
     return publisher_cls.lookup_by_claims(session, signed_claims)
 
 

warehouse/oidc/views.py

@@ -241,6 +241,7 @@ def mint_token(
             ],
             request=request,
         )
+
     # At this point, we've verified that the given JWT is valid for the given
     # project. All we need to do is mint a new token.
     # NOTE: For OIDC-minted API tokens, the Macaroon's description string