Skip to content

Commit

Permalink
Fix handling of tokens (single part credentials) in URLs (#6818)
Browse files Browse the repository at this point in the history
  • Loading branch information
pradyunsg authored Aug 4, 2019
1 parent b562531 commit 7d29841
Show file tree
Hide file tree
Showing 3 changed files with 62 additions and 12 deletions.
1 change: 1 addition & 0 deletions news/6795.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix handling of tokens (single part credentials) in URLs.
26 changes: 20 additions & 6 deletions src/pip/_internal/download.py
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,7 @@ def _get_new_credentials(self, original_url, allow_netrc=True,
logger.debug("Found credentials in keyring for %s", netloc)
return kr_auth

return None, None
return username, password

def _get_url_and_credentials(self, original_url):
"""Return the credentials to use for the provided URL.
Expand All @@ -324,15 +324,29 @@ def _get_url_and_credentials(self, original_url):
# Use any stored credentials that we have for this netloc
username, password = self.passwords.get(netloc, (None, None))

# If nothing cached, acquire new credentials without prompting
# the user (e.g. from netrc, keyring, or similar).
if username is None or password is None:
if username is None and password is None:
# No stored credentials. Acquire new credentials without prompting
# the user. (e.g. from netrc, keyring, or the URL itself)
username, password = self._get_new_credentials(original_url)

if username is not None and password is not None:
# Store the username and password
if username is not None or password is not None:
# Convert the username and password if they're None, so that
# this netloc will show up as "cached" in the conditional above.
# Further, HTTPBasicAuth doesn't accept None, so it makes sense to
# cache the value that is going to be used.
username = username or ""
password = password or ""

# Store any acquired credentials.
self.passwords[netloc] = (username, password)

assert (
# Credentials were found
(username is not None and password is not None) or
# Credentials were not found
(username is None and password is None)
), "Could not load credentials from url: {}".format(original_url)

return url, username, password

def __call__(self, req):
Expand Down
47 changes: 41 additions & 6 deletions tests/unit/test_download.py
Original file line number Diff line number Diff line change
Expand Up @@ -490,18 +490,53 @@ def test_insecure_host_cache_is_not_enabled(self, tmpdir):
assert not hasattr(session.adapters["https://example.com/"], "cache")


def test_get_credentials():
@pytest.mark.parametrize(["input_url", "url", "username", "password"], [
(
"http://user%40email.com:password@example.com/path",
"http://example.com/path",
"user@email.com",
"password",
),
(
"http://username:password@example.com/path",
"http://example.com/path",
"username",
"password",
),
(
"http://token@example.com/path",
"http://example.com/path",
"token",
"",
),
(
"http://example.com/path",
"http://example.com/path",
None,
None,
),
])
def test_get_credentials_parses_correctly(input_url, url, username, password):
auth = MultiDomainBasicAuth()
get = auth._get_url_and_credentials

# Check URL parsing
assert get("http://foo:bar@example.com/path") \
== ('http://example.com/path', 'foo', 'bar')
assert auth.passwords['example.com'] == ('foo', 'bar')
assert get(input_url) == (url, username, password)
assert (
# There are no credentials in the URL
(username is None and password is None) or
# Credentials were found and "cached" appropriately
auth.passwords['example.com'] == (username, password)
)


def test_get_credentials_uses_cached_credentials():
auth = MultiDomainBasicAuth()
auth.passwords['example.com'] = ('user', 'pass')
assert get("http://foo:bar@example.com/path") \
== ('http://example.com/path', 'user', 'pass')

got = auth._get_url_and_credentials("http://foo:bar@example.com/path")
expected = ('http://example.com/path', 'user', 'pass')
assert got == expected


def test_get_index_url_credentials():
Expand Down

0 comments on commit 7d29841

Please sign in to comment.