OIDC: More claims for GitHub's provider (#11239)

* [WIP] initial renamings and migration

* migrations: bump revision

* Revert "migrations: bump revision"

This reverts commit 38035bd.

* tests, warehouse: update tests

* migrations: rename instead of drop/add

tests/unit/oidc/test_models.py

@@ -28,6 +28,8 @@ def test_github_provider_all_known_claims(self):
             # verifiable claims
             "repository",
             "workflow",
+            "repository_owner",
+            "repository_owner_id",
             # preverified claims
             "iss",
             "iat",
@@ -36,6 +38,7 @@ def test_github_provider_all_known_claims(self):
             "aud",
             # unchecked claims
             "actor",
+            "actor_id",
             "jti",
             "sub",
             "ref",
@@ -47,14 +50,15 @@ def test_github_provider_all_known_claims(self):
             "base_ref",
             "event_name",
             "ref_type",
+            "repository_id",
             "job_workflow_ref",
         }
 
     def test_github_provider_computed_properties(self):
         provider = models.GitHubProvider(
             repository_name="fakerepo",
-            owner="fakeowner",
-            owner_id="fakeid",
+            repository_owner="fakeowner",
+            repository_owner_id="fakeid",
             workflow_filename="fakeworkflow.yml",
         )
 
@@ -66,8 +70,8 @@ def test_github_provider_computed_properties(self):
     def test_github_provider_unaccounted_claims(self, monkeypatch):
         provider = models.GitHubProvider(
             repository_name="fakerepo",
-            owner="fakeowner",
-            owner_id="fakeid",
+            repository_owner="fakeowner",
+            repository_owner_id="fakeid",
             workflow_filename="fakeworkflow.yml",
         )
 
@@ -90,8 +94,8 @@ def test_github_provider_unaccounted_claims(self, monkeypatch):
     def test_github_provider_missing_claims(self, monkeypatch):
         provider = models.GitHubProvider(
             repository_name="fakerepo",
-            owner="fakeowner",
-            owner_id="fakeid",
+            repository_owner="fakeowner",
+            repository_owner_id="fakeid",
             workflow_filename="fakeworkflow.yml",
         )
 
@@ -111,8 +115,8 @@ def test_github_provider_missing_claims(self, monkeypatch):
     def test_github_provider_verifies(self, monkeypatch):
         provider = models.GitHubProvider(
             repository_name="fakerepo",
-            owner="fakeowner",
-            owner_id="fakeid",
+            repository_owner="fakeowner",
+            repository_owner_id="fakeid",
             workflow_filename="fakeworkflow.yml",
         )
 

warehouse/manage/views.py

@@ -1311,22 +1311,22 @@ def add_github_oidc_provider(self):
 
         if form.validate():
             # GitHub OIDC providers are unique on the tuple of
-            # (repository_name, owner, workflow_filename), so we check for
+            # (repository_name, repository_owner, workflow_filename), so we check for
             # an already registered one before creating.
             provider = (
                 self.request.db.query(GitHubProvider)
                 .filter(
                     GitHubProvider.repository_name == form.repository.data,
-                    GitHubProvider.owner == form.normalized_owner,
+                    GitHubProvider.repository_owner == form.normalized_owner,
                     GitHubProvider.workflow_filename == form.workflow_filename.data,
                 )
                 .one_or_none()
             )
             if provider is None:
                 provider = GitHubProvider(
                     repository_name=form.repository.data,
-                    owner=form.normalized_owner,
-                    owner_id=form.owner_id,
+                    repository_owner=form.normalized_owner,
+                    repository_owner_id=form.owner_id,
                     workflow_filename=form.workflow_filename.data,
                 )
 

warehouse/migrations/versions/bb986a64761a_rename_githubprovider_fields.py

@@ -0,0 +1,57 @@
+# 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.
+"""
+rename GitHubProvider fields
+
+Revision ID: bb986a64761a
+Revises: 9f0f99509d92
+Create Date: 2022-04-22 22:00:53.832695
+"""
+
+from alembic import op
+
+revision = "bb986a64761a"
+down_revision = "9f0f99509d92"
+
+
+def upgrade():
+    op.drop_constraint(
+        "_github_oidc_provider_uc", "github_oidc_providers", type_="unique"
+    )
+    op.alter_column(
+        "github_oidc_providers", "owner_id", new_column_name="repository_owner_id"
+    )
+    op.alter_column(
+        "github_oidc_providers", "owner", new_column_name="repository_owner"
+    )
+    op.create_unique_constraint(
+        "_github_oidc_provider_uc",
+        "github_oidc_providers",
+        ["repository_name", "repository_owner", "workflow_filename"],
+    )
+
+
+def downgrade():
+    op.drop_constraint(
+        "_github_oidc_provider_uc", "github_oidc_providers", type_="unique"
+    )
+    op.alter_column(
+        "github_oidc_providers", "repository_owner_id", new_column_name="owner_id"
+    )
+    op.alter_column(
+        "github_oidc_providers", "repository_owner", new_column_name="owner"
+    )
+    op.create_unique_constraint(
+        "_github_oidc_provider_uc",
+        "github_oidc_providers",
+        ["repository_name", "owner", "workflow_filename"],
+    )

warehouse/oidc/models.py

@@ -132,25 +132,28 @@ class GitHubProvider(OIDCProvider):
     __table_args__ = (
         UniqueConstraint(
             "repository_name",
-            "owner",
+            "repository_owner",
             "workflow_filename",
             name="_github_oidc_provider_uc",
         ),
     )
 
     id = Column(UUID(as_uuid=True), ForeignKey(OIDCProvider.id), primary_key=True)
     repository_name = Column(String)
-    owner = Column(String)
-    owner_id = Column(String)
+    repository_owner = Column(String)
+    repository_owner_id = Column(String)
     workflow_filename = Column(String)
 
     __verifiable_claims__ = {
         "repository": str.__eq__,
         "workflow": str.__eq__,
+        "repository_owner": str.__eq__,
+        "repository_owner_id": str.__eq__,
     }
 
     __unchecked_claims__ = {
         "actor",
+        "actor_id",
         "jti",
         "sub",
         "ref",
@@ -162,6 +165,7 @@ class GitHubProvider(OIDCProvider):
         "base_ref",
         "event_name",
         "ref_type",
+        "repository_id",
         # TODO(#11096): Support reusable workflows.
         "job_workflow_ref",
     }
@@ -172,7 +176,7 @@ def provider_name(self):
 
     @property
     def repository(self):
-        return f"{self.owner}/{self.repository_name}"
+        return f"{self.repository_owner}/{self.repository_name}"
 
     @property
     def workflow(self):