Skip to content

Commit 527e43e

Browse files
author
Dos Moonen
committed
Refactor _get_index_url() to get integration tests working again and add tests to make sure it stays that way.
Keyring support via the 'subprocess' provider can only retrieve a password, not a username-password combo. The username therefor MUST come from the URL. If the URL obtained from the index does not contain a username then the username from a matching index is used. `_get_index_url()` does that matching. The problem this refactoring solves is that the URL where a wheel or sdist can be downloaded from does not always start with the index url. Azure DevOps Artifacts Feeds are an example since it replaces the friendly name of the Feed with the GUID of the Feed. Causing `url.startswith(prefix)` to evaluate as `False`. The new behaviour is to return the index which matches the netloc and has the longest common prefix of the `path` property of the value returned by `urllib.parse.urlsplit()`. The behaviour for resolving ties is unspecified.
1 parent bd782bf commit 527e43e

File tree

2 files changed

+92
-16
lines changed

2 files changed

+92
-16
lines changed

src/pip/_internal/network/auth.py

Lines changed: 32 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
import urllib.parse
1414
from abc import ABC, abstractmethod
1515
from functools import lru_cache
16+
from os.path import commonprefix
1617
from pathlib import Path
1718
from typing import Any, Dict, List, NamedTuple, Optional, Tuple
1819

@@ -282,11 +283,37 @@ def _get_index_url(self, url: str) -> Optional[str]:
282283
if not url or not self.index_urls:
283284
return None
284285

285-
for u in self.index_urls:
286-
prefix = remove_auth_from_url(u).rstrip("/") + "/"
287-
if url.startswith(prefix):
288-
return u
289-
return None
286+
url = remove_auth_from_url(url).rstrip("/") + "/"
287+
parsed_url = urllib.parse.urlsplit(url)
288+
289+
candidates = []
290+
291+
for index in self.index_urls:
292+
index = index.rstrip("/") + "/"
293+
parsed_index = urllib.parse.urlsplit(remove_auth_from_url(index))
294+
if parsed_url == parsed_index:
295+
return index
296+
297+
if parsed_url.netloc != parsed_index.netloc:
298+
continue
299+
300+
candidate = urllib.parse.urlsplit(index)
301+
candidates.append(candidate)
302+
303+
if not candidates:
304+
return None
305+
306+
candidates.sort(
307+
reverse=True,
308+
key=lambda candidate: commonprefix(
309+
[
310+
parsed_url.path,
311+
candidate.path,
312+
]
313+
).rfind("/"),
314+
)
315+
316+
return urllib.parse.urlunsplit(candidates[0])
290317

291318
def _get_new_credentials(
292319
self,

tests/unit/test_network_auth.py

Lines changed: 60 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,12 @@ def test_get_credentials_uses_cached_credentials_only_username() -> None:
102102

103103

104104
def test_get_index_url_credentials() -> None:
105-
auth = MultiDomainBasicAuth(index_urls=["http://foo:[email protected]/path"])
105+
auth = MultiDomainBasicAuth(
106+
index_urls=[
107+
"http://example.com/",
108+
"http://foo:[email protected]/path",
109+
]
110+
)
106111
get = functools.partial(
107112
auth._get_new_credentials, allow_netrc=False, allow_keyring=False
108113
)
@@ -112,6 +117,45 @@ def test_get_index_url_credentials() -> None:
112117
assert get("http://example.com/path3/path2") == (None, None)
113118

114119

120+
def test_prioritize_longest_path_prefix_match_organization() -> None:
121+
auth = MultiDomainBasicAuth(
122+
index_urls=[
123+
"http://foo:[email protected]/org-name-alpha/repo-alias/simple",
124+
"http://bar:[email protected]/org-name-beta/repo-alias/simple",
125+
]
126+
)
127+
get = functools.partial(
128+
auth._get_new_credentials, allow_netrc=False, allow_keyring=False
129+
)
130+
131+
# Inspired by Azure DevOps URL structure, GitLab should look similar
132+
assert get("http://example.com/org-name-alpha/repo-guid/dowbload/") == (
133+
"foo",
134+
"bar",
135+
)
136+
assert get("http://example.com/org-name-beta/repo-guid/dowbload/") == ("bar", "foo")
137+
138+
139+
def test_prioritize_longest_path_prefix_match_project() -> None:
140+
auth = MultiDomainBasicAuth(
141+
index_urls=[
142+
"http://foo:[email protected]/org-alpha/project-name-alpha/repo-alias/simple",
143+
"http://bar:[email protected]/org-alpha/project-name-beta/repo-alias/simple",
144+
]
145+
)
146+
get = functools.partial(
147+
auth._get_new_credentials, allow_netrc=False, allow_keyring=False
148+
)
149+
150+
# Inspired by Azure DevOps URL structure, GitLab should look similar
151+
assert get(
152+
"http://example.com/org-alpha/project-name-alpha/repo-guid/dowbload/"
153+
) == ("foo", "bar")
154+
assert get(
155+
"http://example.com/org-alpha/project-name-beta/repo-guid/dowbload/"
156+
) == ("bar", "foo")
157+
158+
115159
class KeyringModuleV1:
116160
"""Represents the supported API of keyring before get_credential
117161
was added.
@@ -123,7 +167,7 @@ def __init__(self) -> None:
123167
def get_password(self, system: str, username: str) -> Optional[str]:
124168
if system == "example.com" and username:
125169
return username + "!netloc"
126-
if system == "http://example.com/path2" and username:
170+
if system == "http://example.com/path2/" and username:
127171
return username + "!url"
128172
return None
129173

@@ -136,8 +180,8 @@ def set_password(self, system: str, username: str, password: str) -> None:
136180
(
137181
("http://example.com/path1", (None, None)),
138182
# path1 URLs will be resolved by netloc
139-
("http://[email protected]/path1", ("user", "user!netloc")),
140-
("http://[email protected]/path1", ("user2", "user2!netloc")),
183+
("http://[email protected]/path3", ("user", "user!netloc")),
184+
("http://[email protected]/path3", ("user2", "user2!netloc")),
141185
# path2 URLs will be resolved by index URL
142186
("http://example.com/path2/path3", (None, None)),
143187
("http://[email protected]/path2/path3", ("foo", "foo!url")),
@@ -151,7 +195,8 @@ def test_keyring_get_password(
151195
keyring = KeyringModuleV1()
152196
monkeypatch.setitem(sys.modules, "keyring", keyring) # type: ignore[misc]
153197
auth = MultiDomainBasicAuth(
154-
index_urls=["http://example.com/path2"], keyring_provider="import"
198+
index_urls=["http://example.com/path2", "http://example.com/path3"],
199+
keyring_provider="import",
155200
)
156201

157202
actual = auth._get_new_credentials(url, allow_netrc=False, allow_keyring=True)
@@ -199,7 +244,8 @@ def test_keyring_get_password_username_in_index(
199244
keyring = KeyringModuleV1()
200245
monkeypatch.setitem(sys.modules, "keyring", keyring) # type: ignore[misc]
201246
auth = MultiDomainBasicAuth(
202-
index_urls=["http://[email protected]/path2"], keyring_provider="import"
247+
index_urls=["http://[email protected]/path2", "http://example.com/path4"],
248+
keyring_provider="import",
203249
)
204250
get = functools.partial(
205251
auth._get_new_credentials, allow_netrc=False, allow_keyring=True
@@ -290,7 +336,7 @@ def get_password(self, system: str, username: str) -> None:
290336
assert False, "get_password should not ever be called"
291337

292338
def get_credential(self, system: str, username: str) -> Optional[Credential]:
293-
if system == "http://example.com/path2":
339+
if system == "http://example.com/path2/":
294340
return self.Credential("username", "url")
295341
if system == "example.com":
296342
return self.Credential("username", "netloc")
@@ -310,7 +356,8 @@ def test_keyring_get_credential(
310356
) -> None:
311357
monkeypatch.setitem(sys.modules, "keyring", KeyringModuleV2()) # type: ignore[misc]
312358
auth = MultiDomainBasicAuth(
313-
index_urls=["http://example.com/path2"], keyring_provider="import"
359+
index_urls=["http://example.com/path1", "http://example.com/path2"],
360+
keyring_provider="import",
314361
)
315362

316363
assert (
@@ -375,6 +422,7 @@ def __call__(
375422
self.returncode = 1
376423
else:
377424
# Passwords are returned encoded with a newline appended
425+
self.returncode = 0
378426
self.stdout = (password + os.linesep).encode("utf-8")
379427

380428
if cmd[1] == "set":
@@ -400,8 +448,8 @@ def check_returncode(self) -> None:
400448
(
401449
("http://example.com/path1", (None, None)),
402450
# path1 URLs will be resolved by netloc
403-
("http://[email protected]/path1", ("user", "user!netloc")),
404-
("http://[email protected]/path1", ("user2", "user2!netloc")),
451+
("http://[email protected]/path3", ("user", "user!netloc")),
452+
("http://[email protected]/path3", ("user2", "user2!netloc")),
405453
# path2 URLs will be resolved by index URL
406454
("http://example.com/path2/path3", (None, None)),
407455
("http://[email protected]/path2/path3", ("foo", "foo!url")),
@@ -417,7 +465,8 @@ def test_keyring_cli_get_password(
417465
pip._internal.network.auth.subprocess, "run", KeyringSubprocessResult()
418466
)
419467
auth = MultiDomainBasicAuth(
420-
index_urls=["http://example.com/path2"], keyring_provider="subprocess"
468+
index_urls=["http://example.com/path2", "http://example.com/path3"],
469+
keyring_provider="subprocess",
421470
)
422471

423472
actual = auth._get_new_credentials(url, allow_netrc=False, allow_keyring=True)

0 commit comments

Comments
 (0)