diff --git a/tests/unit/oidc/models/test_core.py b/tests/unit/oidc/models/test_core.py new file mode 100644 index 000000000000..2f9459584e4a --- /dev/null +++ b/tests/unit/oidc/models/test_core.py @@ -0,0 +1,29 @@ +# 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. + +import pretend + +from warehouse.oidc.models import _core + + +def test_check_claim_binary(): + wrapped = _core._check_claim_binary(str.__eq__) + + assert wrapped("foo", "bar", pretend.stub()) is False + assert wrapped("foo", "foo", pretend.stub()) is True + + +class TestOIDCPublisher: + def test_oidc_publisher_not_default_verifiable(self): + publisher = _core.OIDCPublisher(projects=[]) + + assert not publisher.verify_claims(signed_claims={}) diff --git a/tests/unit/oidc/test_models.py b/tests/unit/oidc/models/test_github.py similarity index 88% rename from tests/unit/oidc/test_models.py rename to tests/unit/oidc/models/test_github.py index bc635ce0d1ed..ea7b1e6ce4f3 100644 --- a/tests/unit/oidc/test_models.py +++ b/tests/unit/oidc/models/test_github.py @@ -14,31 +14,17 @@ import pytest from tests.common.db.oidc import GitHubPublisherFactory, PendingGitHubPublisherFactory -from warehouse.oidc import models - - -def test_check_claim_binary(): - wrapped = models._check_claim_binary(str.__eq__) - - assert wrapped("foo", "bar", pretend.stub()) is False - assert wrapped("foo", "foo", pretend.stub()) is True +from warehouse.oidc.models import _core, github @pytest.mark.parametrize("claim", ["", "repo", "repo:"]) def test_check_sub(claim): - assert models._check_sub(pretend.stub(), claim, pretend.stub()) is False - - -class TestOIDCPublisher: - def test_oidc_publisher_not_default_verifiable(self): - publisher = models.OIDCPublisher(projects=[]) - - assert not publisher.verify_claims(signed_claims={}) + assert github._check_sub(pretend.stub(), claim, pretend.stub()) is False class TestGitHubPublisher: def test_github_publisher_all_known_claims(self): - assert models.GitHubPublisher.all_known_claims() == { + assert github.GitHubPublisher.all_known_claims() == { # verifiable claims "sub", "repository", @@ -78,7 +64,7 @@ def test_github_publisher_all_known_claims(self): } def test_github_publisher_computed_properties(self): - publisher = models.GitHubPublisher( + publisher = github.GitHubPublisher( repository_name="fakerepo", repository_owner="fakeowner", repository_owner_id="fakeid", @@ -93,7 +79,7 @@ def test_github_publisher_computed_properties(self): assert publisher.publisher_url == "https://github.com/fakeowner/fakerepo" def test_github_publisher_unaccounted_claims(self, monkeypatch): - publisher = models.GitHubPublisher( + publisher = github.GitHubPublisher( repository_name="fakerepo", repository_owner="fakeowner", repository_owner_id="fakeid", @@ -109,12 +95,12 @@ def test_github_publisher_unaccounted_claims(self, monkeypatch): ) ), ) - monkeypatch.setattr(models, "sentry_sdk", sentry_sdk) + monkeypatch.setattr(_core, "sentry_sdk", sentry_sdk) # We don't care if these actually verify, only that they're present. signed_claims = { claim_name: "fake" - for claim_name in models.GitHubPublisher.all_known_claims() + for claim_name in github.GitHubPublisher.all_known_claims() } signed_claims["fake-claim"] = "fake" signed_claims["another-fake-claim"] = "also-fake" @@ -128,7 +114,7 @@ def test_github_publisher_unaccounted_claims(self, monkeypatch): assert scope.fingerprint == ["another-fake-claim", "fake-claim"] def test_github_publisher_missing_claims(self, monkeypatch): - publisher = models.GitHubPublisher( + publisher = github.GitHubPublisher( repository_name="fakerepo", repository_owner="fakeowner", repository_owner_id="fakeid", @@ -144,11 +130,11 @@ def test_github_publisher_missing_claims(self, monkeypatch): ) ), ) - monkeypatch.setattr(models, "sentry_sdk", sentry_sdk) + monkeypatch.setattr(_core, "sentry_sdk", sentry_sdk) signed_claims = { claim_name: "fake" - for claim_name in models.GitHubPublisher.all_known_claims() + for claim_name in github.GitHubPublisher.all_known_claims() } # Pop the first signed claim, so that it's the first one to fail. signed_claims.pop("sub") @@ -161,7 +147,7 @@ def test_github_publisher_missing_claims(self, monkeypatch): assert scope.fingerprint == ["sub"] def test_github_publisher_missing_optional_claims(self, monkeypatch): - publisher = models.GitHubPublisher( + publisher = github.GitHubPublisher( repository_name="fakerepo", repository_owner="fakeowner", repository_owner_id="fakeid", @@ -170,11 +156,11 @@ def test_github_publisher_missing_optional_claims(self, monkeypatch): ) sentry_sdk = pretend.stub(capture_message=pretend.call_recorder(lambda s: None)) - monkeypatch.setattr(models, "sentry_sdk", sentry_sdk) + monkeypatch.setattr(_core, "sentry_sdk", sentry_sdk) signed_claims = { claim_name: getattr(publisher, claim_name) - for claim_name in models.GitHubPublisher.__required_verifiable_claims__ + for claim_name in github.GitHubPublisher.__required_verifiable_claims__ } signed_claims["ref"] = "ref" signed_claims["job_workflow_ref"] = publisher.job_workflow_ref + "@ref" @@ -185,10 +171,10 @@ def test_github_publisher_missing_optional_claims(self, monkeypatch): @pytest.mark.parametrize("environment", [None, "some-environment"]) @pytest.mark.parametrize( "missing_claims", - [set(), models.GitHubPublisher.__optional_verifiable_claims__.keys()], + [set(), github.GitHubPublisher.__optional_verifiable_claims__.keys()], ) def test_github_publisher_verifies(self, monkeypatch, environment, missing_claims): - publisher = models.GitHubPublisher( + publisher = github.GitHubPublisher( repository_name="fakerepo", repository_owner="fakeowner", repository_owner_id="fakeid", @@ -214,7 +200,7 @@ def test_github_publisher_verifies(self, monkeypatch, environment, missing_claim signed_claims = { claim_name: "fake" - for claim_name in models.GitHubPublisher.all_known_claims() + for claim_name in github.GitHubPublisher.all_known_claims() if claim_name not in missing_claims } assert publisher.verify_claims(signed_claims=signed_claims) @@ -271,14 +257,14 @@ def test_github_publisher_verifies(self, monkeypatch, environment, missing_claim ], ) def test_github_publisher_job_workflow_ref(self, claim, ref, valid): - publisher = models.GitHubPublisher( + publisher = github.GitHubPublisher( repository_name="bar", repository_owner="foo", repository_owner_id=pretend.stub(), workflow_filename="baz.yml", ) - check = models.GitHubPublisher.__required_verifiable_claims__[ + check = github.GitHubPublisher.__required_verifiable_claims__[ "job_workflow_ref" ] assert check(publisher.job_workflow_ref, claim, {"ref": ref}) is valid @@ -294,7 +280,7 @@ def test_github_publisher_job_workflow_ref(self, claim, ref, valid): ], ) def test_github_publisher_sub_claim(self, truth, claim, valid): - check = models.GitHubPublisher.__required_verifiable_claims__["sub"] + check = github.GitHubPublisher.__required_verifiable_claims__["sub"] assert check(truth, claim, pretend.stub()) is valid @pytest.mark.parametrize( @@ -309,7 +295,7 @@ def test_github_publisher_sub_claim(self, truth, claim, valid): ], ) def test_github_publisher_environment_claim(self, truth, claim, valid): - check = models.GitHubPublisher.__optional_verifiable_claims__["environment"] + check = github.GitHubPublisher.__optional_verifiable_claims__["environment"] assert check(truth, claim, pretend.stub()) is valid @@ -317,7 +303,7 @@ class TestPendingGitHubPublisher: def test_reify_does_not_exist_yet(self, db_request): pending_publisher = PendingGitHubPublisherFactory.create() assert ( - db_request.db.query(models.GitHubPublisher) + db_request.db.query(github.GitHubPublisher) .filter_by( repository_name=pending_publisher.repository_name, repository_owner=pending_publisher.repository_owner, @@ -332,7 +318,7 @@ def test_reify_does_not_exist_yet(self, db_request): # If an OIDC publisher for this pending publisher does not already exist, # a new one is created and the pending publisher is marked for deletion. - assert isinstance(publisher, models.GitHubPublisher) + assert isinstance(publisher, github.GitHubPublisher) assert pending_publisher in db_request.db.deleted assert publisher.repository_name == pending_publisher.repository_name assert publisher.repository_owner == pending_publisher.repository_owner diff --git a/warehouse/oidc/models/__init__.py b/warehouse/oidc/models/__init__.py new file mode 100644 index 000000000000..3434d9d88ecf --- /dev/null +++ b/warehouse/oidc/models/__init__.py @@ -0,0 +1,21 @@ +# 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. + +from warehouse.oidc.models._core import OIDCPublisher, PendingOIDCPublisher +from warehouse.oidc.models.github import GitHubPublisher, PendingGitHubPublisher + +__all__ = [ + "OIDCPublisher", + "PendingOIDCPublisher", + "PendingGitHubPublisher", + "GitHubPublisher", +] diff --git a/warehouse/oidc/models.py b/warehouse/oidc/models/_core.py similarity index 55% rename from warehouse/oidc/models.py rename to warehouse/oidc/models/_core.py index a821a6909242..b57d8f413eb9 100644 --- a/warehouse/oidc/models.py +++ b/warehouse/oidc/models/_core.py @@ -10,13 +10,12 @@ # See the License for the specific language governing permissions and # limitations under the License. - from collections.abc import Callable from typing import Any import sentry_sdk -from sqlalchemy import Column, ForeignKey, String, UniqueConstraint, orm +from sqlalchemy import Column, ForeignKey, String, orm from sqlalchemy.dialects.postgresql import UUID from warehouse import db @@ -40,64 +39,6 @@ def wrapper(ground_truth, signed_claim, all_signed_claims): return wrapper -def _check_job_workflow_ref(ground_truth, signed_claim, all_signed_claims): - # We expect a string formatted as follows: - # OWNER/REPO/.github/workflows/WORKFLOW.yml@REF - # where REF is the value of the `ref` claim. - - # Defensive: GitHub should never give us an empty job_workflow_ref, - # but we check for one anyways just in case. - if not signed_claim: - return False - - ref = all_signed_claims.get("ref") - if not ref: - return False - - return f"{ground_truth}@{ref}" == signed_claim - - -def _check_environment(ground_truth, signed_claim, all_signed_claims): - # When there is an environment, we expect a case-insensitive string. - # https://docs.github.com/en/actions/deployment/targeting-different-environments/using-environments-for-deployment - # For tokens that are generated outside of an environment, the claim will - # be missing. - - # If we haven't set an environment name for the publisher, we don't need to - # check this claim - if ground_truth is None: - return True - - # Defensive: GitHub might give us an empty environment if this token wasn't - # generated from within an environment, in which case the check should - # fail. - if not signed_claim: - return False - - # We store the normalized environment name, but we normalize both here to - # ensure we can't accidentally become case-sensitive. - return ground_truth.lower() == signed_claim.lower() - - -def _check_sub(ground_truth, signed_claim, _all_signed_claims): - # We expect a string formatted as follows: - # repo:ORG/REPO[:OPTIONAL-STUFF] - # where :OPTIONAL-STUFF is a concatenation of other job context - # metadata. We currently lack the ground context to verify that - # additional metadata, so we limit our verification to just the ORG/REPO - # component. - - # Defensive: GitHub should never give us an empty subject. - if not signed_claim: - return False - - components = signed_claim.split(":") - if len(components) < 2: - return False - - return f"{components[0]}:{components[1]}" == ground_truth - - class OIDCPublisherProjectAssociation(db.Model): __tablename__ = "oidc_publisher_project_association" @@ -273,140 +214,3 @@ def reify(self, session): # pragma: no cover # Only concrete subclasses are constructed. raise NotImplementedError - - -class GitHubPublisherMixin: - """ - Common functionality for both pending and concrete GitHub OIDC publishers. - """ - - repository_name = Column(String, nullable=False) - repository_owner = Column(String, nullable=False) - repository_owner_id = Column(String, nullable=False) - workflow_filename = Column(String, nullable=False) - environment = Column(String, nullable=True) - - __required_verifiable_claims__ = { - "sub": _check_sub, - "repository": _check_claim_binary(str.__eq__), - "repository_owner": _check_claim_binary(str.__eq__), - "repository_owner_id": _check_claim_binary(str.__eq__), - "job_workflow_ref": _check_job_workflow_ref, - } - - __optional_verifiable_claims__ = { - "environment": _check_environment, - } - - __unchecked_claims__ = { - "actor", - "actor_id", - "jti", - "ref", - "sha", - "run_id", - "run_number", - "run_attempt", - "head_ref", - "base_ref", - "event_name", - "ref_type", - "repository_id", - "workflow", - "repository_visibility", - "workflow_sha", - "job_workflow_sha", - "workflow_ref", - "runner_environment", - "environment_node_id", - "enterprise", - } - - @property - def _workflow_slug(self): - return f".github/workflows/{self.workflow_filename}" - - @property - def publisher_name(self): - return "GitHub" - - @property - def repository(self): - return f"{self.repository_owner}/{self.repository_name}" - - @property - def publisher_url(self): - return f"https://github.com/{self.repository}" - - @property - def job_workflow_ref(self): - return f"{self.repository}/{self._workflow_slug}" - - @property - def sub(self): - return f"repo:{self.repository}" - - def __str__(self): - return self.workflow_filename - - -class GitHubPublisher(GitHubPublisherMixin, OIDCPublisher): - __tablename__ = "github_oidc_publishers" - __mapper_args__ = {"polymorphic_identity": "github_oidc_publishers"} - __table_args__ = ( - UniqueConstraint( - "repository_name", - "repository_owner", - "workflow_filename", - "environment", - name="_github_oidc_publisher_uc", - ), - ) - - id = Column(UUID(as_uuid=True), ForeignKey(OIDCPublisher.id), primary_key=True) - - -class PendingGitHubPublisher(GitHubPublisherMixin, PendingOIDCPublisher): - __tablename__ = "pending_github_oidc_publishers" - __mapper_args__ = {"polymorphic_identity": "pending_github_oidc_publishers"} - __table_args__ = ( - UniqueConstraint( - "repository_name", - "repository_owner", - "workflow_filename", - "environment", - name="_pending_github_oidc_publisher_uc", - ), - ) - - id = Column( - UUID(as_uuid=True), ForeignKey(PendingOIDCPublisher.id), primary_key=True - ) - - def reify(self, session): - """ - Returns a `GitHubPublisher` for this `PendingGitHubPublisher`, - deleting the `PendingGitHubPublisher` in the process. - """ - - maybe_publisher = ( - session.query(GitHubPublisher) - .filter( - GitHubPublisher.repository_name == self.repository_name, - GitHubPublisher.repository_owner == self.repository_owner, - GitHubPublisher.workflow_filename == self.workflow_filename, - GitHubPublisher.environment == self.environment, - ) - .one_or_none() - ) - - publisher = maybe_publisher or GitHubPublisher( - repository_name=self.repository_name, - repository_owner=self.repository_owner, - repository_owner_id=self.repository_owner_id, - workflow_filename=self.workflow_filename, - environment=self.environment, - ) - - session.delete(self) - return publisher diff --git a/warehouse/oidc/models/github.py b/warehouse/oidc/models/github.py new file mode 100644 index 000000000000..4fc8f70b82b3 --- /dev/null +++ b/warehouse/oidc/models/github.py @@ -0,0 +1,215 @@ +# 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. + +from sqlalchemy import Column, ForeignKey, String, UniqueConstraint +from sqlalchemy.dialects.postgresql import UUID + +from warehouse.oidc.models._core import ( + OIDCPublisher, + PendingOIDCPublisher, + _check_claim_binary, +) + + +def _check_job_workflow_ref(ground_truth, signed_claim, all_signed_claims): + # We expect a string formatted as follows: + # OWNER/REPO/.github/workflows/WORKFLOW.yml@REF + # where REF is the value of the `ref` claim. + + # Defensive: GitHub should never give us an empty job_workflow_ref, + # but we check for one anyways just in case. + if not signed_claim: + return False + + ref = all_signed_claims.get("ref") + if not ref: + return False + + return f"{ground_truth}@{ref}" == signed_claim + + +def _check_environment(ground_truth, signed_claim, all_signed_claims): + # When there is an environment, we expect a case-insensitive string. + # https://docs.github.com/en/actions/deployment/targeting-different-environments/using-environments-for-deployment + # For tokens that are generated outside of an environment, the claim will + # be missing. + + # If we haven't set an environment name for the publisher, we don't need to + # check this claim + if ground_truth is None: + return True + + # Defensive: GitHub might give us an empty environment if this token wasn't + # generated from within an environment, in which case the check should + # fail. + if not signed_claim: + return False + + # We store the normalized environment name, but we normalize both here to + # ensure we can't accidentally become case-sensitive. + return ground_truth.lower() == signed_claim.lower() + + +def _check_sub(ground_truth, signed_claim, _all_signed_claims): + # We expect a string formatted as follows: + # repo:ORG/REPO[:OPTIONAL-STUFF] + # where :OPTIONAL-STUFF is a concatenation of other job context + # metadata. We currently lack the ground context to verify that + # additional metadata, so we limit our verification to just the ORG/REPO + # component. + + # Defensive: GitHub should never give us an empty subject. + if not signed_claim: + return False + + components = signed_claim.split(":") + if len(components) < 2: + return False + + return f"{components[0]}:{components[1]}" == ground_truth + + +class GitHubPublisherMixin: + """ + Common functionality for both pending and concrete GitHub OIDC publishers. + """ + + repository_name = Column(String, nullable=False) + repository_owner = Column(String, nullable=False) + repository_owner_id = Column(String, nullable=False) + workflow_filename = Column(String, nullable=False) + environment = Column(String, nullable=True) + + __required_verifiable_claims__ = { + "sub": _check_sub, + "repository": _check_claim_binary(str.__eq__), + "repository_owner": _check_claim_binary(str.__eq__), + "repository_owner_id": _check_claim_binary(str.__eq__), + "job_workflow_ref": _check_job_workflow_ref, + } + + __optional_verifiable_claims__ = { + "environment": _check_environment, + } + + __unchecked_claims__ = { + "actor", + "actor_id", + "jti", + "ref", + "sha", + "run_id", + "run_number", + "run_attempt", + "head_ref", + "base_ref", + "event_name", + "ref_type", + "repository_id", + "workflow", + "repository_visibility", + "workflow_sha", + "job_workflow_sha", + "workflow_ref", + "runner_environment", + "environment_node_id", + "enterprise", + } + + @property + def _workflow_slug(self): + return f".github/workflows/{self.workflow_filename}" + + @property + def publisher_name(self): + return "GitHub" + + @property + def repository(self): + return f"{self.repository_owner}/{self.repository_name}" + + @property + def publisher_url(self): + return f"https://github.com/{self.repository}" + + @property + def job_workflow_ref(self): + return f"{self.repository}/{self._workflow_slug}" + + @property + def sub(self): + return f"repo:{self.repository}" + + def __str__(self): + return self.workflow_filename + + +class GitHubPublisher(GitHubPublisherMixin, OIDCPublisher): + __tablename__ = "github_oidc_publishers" + __mapper_args__ = {"polymorphic_identity": "github_oidc_publishers"} + __table_args__ = ( + UniqueConstraint( + "repository_name", + "repository_owner", + "workflow_filename", + "environment", + name="_github_oidc_publisher_uc", + ), + ) + + id = Column(UUID(as_uuid=True), ForeignKey(OIDCPublisher.id), primary_key=True) + + +class PendingGitHubPublisher(GitHubPublisherMixin, PendingOIDCPublisher): + __tablename__ = "pending_github_oidc_publishers" + __mapper_args__ = {"polymorphic_identity": "pending_github_oidc_publishers"} + __table_args__ = ( + UniqueConstraint( + "repository_name", + "repository_owner", + "workflow_filename", + "environment", + name="_pending_github_oidc_publisher_uc", + ), + ) + + id = Column( + UUID(as_uuid=True), ForeignKey(PendingOIDCPublisher.id), primary_key=True + ) + + def reify(self, session): + """ + Returns a `GitHubPublisher` for this `PendingGitHubPublisher`, + deleting the `PendingGitHubPublisher` in the process. + """ + + maybe_publisher = ( + session.query(GitHubPublisher) + .filter( + GitHubPublisher.repository_name == self.repository_name, + GitHubPublisher.repository_owner == self.repository_owner, + GitHubPublisher.workflow_filename == self.workflow_filename, + GitHubPublisher.environment == self.environment, + ) + .one_or_none() + ) + + publisher = maybe_publisher or GitHubPublisher( + repository_name=self.repository_name, + repository_owner=self.repository_owner, + repository_owner_id=self.repository_owner_id, + workflow_filename=self.workflow_filename, + environment=self.environment, + ) + + session.delete(self) + return publisher