Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ignore case when verifying GitLab/GitHub URLs #16899

Merged
merged 13 commits into from
Nov 14, 2024
16 changes: 16 additions & 0 deletions tests/unit/oidc/models/test_activestate.py
Original file line number Diff line number Diff line change
Expand Up @@ -350,6 +350,22 @@ def test_activestate_publisher_sub(
check(expected, actual, signed_claims)
assert str(e.value) == error_msg

@pytest.mark.parametrize(
("url", "expected"),
[
("https://platform.activestate.com/repository_name/project_name", True),
("https://platform.activestate.com/repository_name/PrOjECt_NaMe", False),
],
)
def test_activestate_publisher_verify_url(self, url, expected):
publisher = ActiveStatePublisher(
organization="repository_name",
activestate_project_name="project_name",
actor_id=ACTOR_ID,
actor=ACTOR,
)
assert publisher.verify_url(url) == expected


class TestPendingActiveStatePublisher:
def test_reify_does_not_exist_yet(self, db_request):
Expand Down
5 changes: 5 additions & 0 deletions tests/unit/oidc/models/test_core.py
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,11 @@ def test_supports_attestations(self):
"https://github.com",
False,
),
( # Default verification is case-sensitive
"https://publisher.com/owner/project",
"https://publisher.com/owner/PrOjeCt",
False,
),
],
)
def test_verify_url(self, monkeypatch, url, publisher_url, expected):
Expand Down
2 changes: 2 additions & 0 deletions tests/unit/oidc/models/test_github.py
Original file line number Diff line number Diff line change
Expand Up @@ -662,6 +662,8 @@ def test_github_publisher_duplicates_cant_be_created(self, db_request):
("https://repository_owner.github.io/repository_name/../malicious", False),
("https://repository_owner.github.io/", False),
("https://repository_owner.github.io/unrelated_name/", False),
("https://github.com/RePosItory_OwNeR/rePository_Name.git", True),
("https://repository_owner.github.io/RePoSiToRy_NaMe/subpage", True),
],
)
def test_github_publisher_verify_url(self, url, expected):
Expand Down
2 changes: 2 additions & 0 deletions tests/unit/oidc/models/test_gitlab.py
Original file line number Diff line number Diff line change
Expand Up @@ -608,6 +608,8 @@ def test_gitlab_publisher_duplicates_cant_be_created(self, db_request):
("https://gitlab.com/repository_owner/repository_name.git", True),
("https://gitlab.com/repository_owner/repository_name.git/", True),
("https://gitlab.com/repository_owner/repository_name.git/issues", False),
("https://gitlab.com/repository_OwNeR/RePoSiToRy_Name.git/", True),
("https://gitlab.com/another_owner/repository_name/issues", False),
],
)
def test_gitlab_publisher_verify_url(self, url, expected):
Expand Down
15 changes: 13 additions & 2 deletions warehouse/oidc/models/github.py
Original file line number Diff line number Diff line change
Expand Up @@ -361,13 +361,24 @@ def verify_url(self, url: str):
The suffix `.git` in repo URLs is ignored, since `github.com/org/repo.git`
always redirects to `github.com/org/repo`. This does not apply to subpaths,
like `github.com/org/repo.git/issues`, which do not redirect to the correct URL.

GitHub uses case-insensitive owner/repo slugs - so we perform a case-insensitive
comparison.
"""
url_for_generic_check = url.removesuffix("/").removesuffix(".git")
docs_url = (
f"https://{self.repository_owner}.github.io/{self.repository_name}".lower()
)

normalized_url_prefixes = (self.publisher_base_url.lower(), docs_url)
for prefix in normalized_url_prefixes:
if url.lower().startswith(prefix):
url = prefix + url[len(prefix) :]
break

url_for_generic_check = url.removesuffix("/").removesuffix(".git")
if super().verify_url(url_for_generic_check):
return True

docs_url = f"https://{self.repository_owner}.github.io/{self.repository_name}"
docs_uri = rfc3986.api.uri_reference(docs_url).normalize()
user_uri = rfc3986.api.uri_reference(url).normalize()

Expand Down
7 changes: 7 additions & 0 deletions warehouse/oidc/models/gitlab.py
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,14 @@ def verify_url(self, url: str):
in repo URLs, since `gitlab.com/org/repo.git` always redirects to
`gitlab.com/org/repo`. This does not apply to subpaths like
`gitlab.com/org/repo.git/issues`, which do not redirect to the correct URL.

GitLab uses case-insensitive owner/repo slugs - so we perform a case-insensitive
comparison.
"""
lowercase_base_url = self.publisher_base_url.lower()
if url.lower().startswith(lowercase_base_url):
url = lowercase_base_url + url[len(lowercase_base_url) :]

url_for_generic_check = url.removesuffix("/").removesuffix(".git")
return super().verify_url(url_for_generic_check)

Expand Down