From f41f747f8b70322155489d07b0f23beebf64f93f Mon Sep 17 00:00:00 2001 From: Pradyun Gedam Date: Sun, 4 Aug 2019 19:56:41 +0530 Subject: [PATCH] Fix handling of tokens (single part credentials) in URLs (#6818) --- news/6795.bugfix | 1 + src/pip/_internal/download.py | 26 ++++++++++++++----- tests/unit/test_download.py | 47 ++++++++++++++++++++++++++++++----- 3 files changed, 62 insertions(+), 12 deletions(-) create mode 100644 news/6795.bugfix diff --git a/news/6795.bugfix b/news/6795.bugfix new file mode 100644 index 000000000..f80bd9b4b --- /dev/null +++ b/news/6795.bugfix @@ -0,0 +1 @@ + Fix handling of tokens (single part credentials) in URLs. diff --git a/src/pip/_internal/download.py b/src/pip/_internal/download.py index 8715eb5b1..fc1f4dddd 100644 --- a/src/pip/_internal/download.py +++ b/src/pip/_internal/download.py @@ -295,7 +295,7 @@ class MultiDomainBasicAuth(AuthBase): 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. @@ -312,15 +312,29 @@ class MultiDomainBasicAuth(AuthBase): # 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): diff --git a/tests/unit/test_download.py b/tests/unit/test_download.py index 7b421a7d7..7be682dcd 100644 --- a/tests/unit/test_download.py +++ b/tests/unit/test_download.py @@ -483,18 +483,53 @@ class TestPipSession: 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():