From 475e2fac54b225c4bf5dd95ff7230cd168ad9fc7 Mon Sep 17 00:00:00 2001 From: Facundo Tuesca Date: Wed, 2 Jul 2025 17:36:36 +0200 Subject: [PATCH] oidc: fix bug when matching GitLab environment claims Signed-off-by: Facundo Tuesca --- tests/unit/oidc/models/test_gitlab.py | 53 +++++++++++++++++++++++++++ warehouse/oidc/models/gitlab.py | 2 +- 2 files changed, 54 insertions(+), 1 deletion(-) diff --git a/tests/unit/oidc/models/test_gitlab.py b/tests/unit/oidc/models/test_gitlab.py index d3fe8c64e20b..4954bfdd524d 100644 --- a/tests/unit/oidc/models/test_gitlab.py +++ b/tests/unit/oidc/models/test_gitlab.py @@ -71,6 +71,59 @@ def test_lookup_fails_invalid_ci_config_ref_uri(self, environment): ): gitlab.GitLabPublisher.lookup_by_claims(pretend.stub(), signed_claims) + @pytest.mark.parametrize("environment", ["SomeEnvironment", "SOME_ENVIRONMENT"]) + def test_lookup_succeeds_with_non_lowercase_environment( + self, db_request, environment + ): + # Test that we find a matching publisher when the environment claims match + # If we incorrectly normalized the incoming capitalized claim, we wouldn't + # find the matching publisher. + stored_publisher = GitLabPublisherFactory( + id="aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa", + namespace="foo", + project="bar", + workflow_filepath=".gitlab-ci.yml", + environment=environment, + ) + + signed_claims = { + "project_path": "foo/bar", + "ci_config_ref_uri": ("gitlab.com/foo/bar//.gitlab-ci.yml@refs/heads/main"), + "environment": environment, + } + + publisher = gitlab.GitLabPublisher.lookup_by_claims( + db_request.db, signed_claims + ) + + assert publisher.id == stored_publisher.id + assert publisher.environment == environment + + @pytest.mark.parametrize("environment", ["SomeEnvironment", "SOME_ENVIRONMENT"]) + def test_lookup_is_case_sensitive_for_environment(self, db_request, environment): + # Test that we don't find a matching publisher when the environment claims don't + # exactly match. + # If we incorrectly normalized the incoming capitalized claim, we would match + # a publisher that has a different environment. + GitLabPublisherFactory( + id="aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa", + namespace="foo", + project="bar", + workflow_filepath=".gitlab-ci.yml", + # stored environment is all lowercase, doesn't match incoming claims + environment=environment.lower(), + ) + + signed_claims = { + "project_path": "foo/bar", + "ci_config_ref_uri": ("gitlab.com/foo/bar//.gitlab-ci.yml@refs/heads/main"), + "environment": environment, + } + + with pytest.raises(errors.InvalidPublisherError) as e: + gitlab.GitLabPublisher.lookup_by_claims(db_request.db, signed_claims) + assert str(e.value) == "Publisher with matching claims was not found" + @pytest.mark.parametrize("environment", ["", "some_environment"]) @pytest.mark.parametrize( ("workflow_filepath_a", "workflow_filepath_b"), diff --git a/warehouse/oidc/models/gitlab.py b/warehouse/oidc/models/gitlab.py index 7e5e74e47db2..48c1473ce1c9 100644 --- a/warehouse/oidc/models/gitlab.py +++ b/warehouse/oidc/models/gitlab.py @@ -188,7 +188,7 @@ def _get_publisher_for_environment( ) -> Self | None: if environment: if specific_publisher := first_true( - publishers, pred=lambda p: p.environment == environment.lower() + publishers, pred=lambda p: p.environment == environment ): return specific_publisher