diff --git a/news/6795.bugfix b/news/6795.bugfix new file mode 100644 index 00000000000..f80bd9b4b2f --- /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 1578521c765..d5c57ee0395 100644 --- a/src/pip/_internal/download.py +++ b/src/pip/_internal/download.py @@ -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. @@ -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): diff --git a/tests/unit/test_download.py b/tests/unit/test_download.py index d63d7420178..42918c20e75 100644 --- a/tests/unit/test_download.py +++ b/tests/unit/test_download.py @@ -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():