diff --git a/tests/unit/oidc/test_models.py b/tests/unit/oidc/test_models.py index 9ffd87bef676..bb12aebbd5ab 100644 --- a/tests/unit/oidc/test_models.py +++ b/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", ) diff --git a/warehouse/manage/views.py b/warehouse/manage/views.py index 423318ecb924..7861eed40ba3 100644 --- a/warehouse/manage/views.py +++ b/warehouse/manage/views.py @@ -1311,13 +1311,13 @@ 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() @@ -1325,8 +1325,8 @@ def add_github_oidc_provider(self): 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, ) diff --git a/warehouse/migrations/versions/bb986a64761a_rename_githubprovider_fields.py b/warehouse/migrations/versions/bb986a64761a_rename_githubprovider_fields.py new file mode 100644 index 000000000000..906809049d47 --- /dev/null +++ b/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"], + ) diff --git a/warehouse/oidc/models.py b/warehouse/oidc/models.py index f2f780671890..f3a228963354 100644 --- a/warehouse/oidc/models.py +++ b/warehouse/oidc/models.py @@ -132,7 +132,7 @@ class GitHubProvider(OIDCProvider): __table_args__ = ( UniqueConstraint( "repository_name", - "owner", + "repository_owner", "workflow_filename", name="_github_oidc_provider_uc", ), @@ -140,17 +140,20 @@ class GitHubProvider(OIDCProvider): 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):