From 9a1a0d66060dc056f0ece5bf02b2dbc3ceeb8df5 Mon Sep 17 00:00:00 2001 From: Pradyun Gedam Date: Sat, 27 Jul 2019 12:25:55 +0700 Subject: [PATCH 1/6] Fix handling of credentials in URL parsing --- src/pip/_internal/download.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/pip/_internal/download.py b/src/pip/_internal/download.py index 1578521c765..e99a1a39941 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. @@ -326,11 +326,11 @@ def _get_url_and_credentials(self, original_url): # 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: 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: + # Store any acquired credentials self.passwords[netloc] = (username, password) return url, username, password From 383bdc924d01848c076ff160ce5a12848030af05 Mon Sep 17 00:00:00 2001 From: Pradyun Gedam Date: Sun, 28 Jul 2019 00:15:45 +0700 Subject: [PATCH 2/6] Convert username/password from None to empty string --- src/pip/_internal/download.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/pip/_internal/download.py b/src/pip/_internal/download.py index e99a1a39941..58722a74648 100644 --- a/src/pip/_internal/download.py +++ b/src/pip/_internal/download.py @@ -330,7 +330,12 @@ def _get_url_and_credentials(self, original_url): username, password = self._get_new_credentials(original_url) if username is not None or password is not None: - # Store any acquired credentials + # Store any acquired credentials, converting None to empty strings + # The conversion ensures that this netloc will matched the "cached" + # 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 "" self.passwords[netloc] = (username, password) return url, username, password From 1c7ed4884943045484851d753883ead8f8b14e19 Mon Sep 17 00:00:00 2001 From: Pradyun Gedam Date: Tue, 30 Jul 2019 20:46:36 +0530 Subject: [PATCH 3/6] Add a sanity-check assert Ensure in _get_url_and_credentials, that the values extracted are sane. --- src/pip/_internal/download.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/pip/_internal/download.py b/src/pip/_internal/download.py index 58722a74648..98e132f0d1a 100644 --- a/src/pip/_internal/download.py +++ b/src/pip/_internal/download.py @@ -338,6 +338,13 @@ def _get_url_and_credentials(self, original_url): password = password or "" 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): From 5eb92958166874ed7f0c81573713a7f6eee9a7e0 Mon Sep 17 00:00:00 2001 From: Pradyun Gedam Date: Tue, 30 Jul 2019 21:06:06 +0530 Subject: [PATCH 4/6] Better comments --- src/pip/_internal/download.py | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/pip/_internal/download.py b/src/pip/_internal/download.py index 98e132f0d1a..d5c57ee0395 100644 --- a/src/pip/_internal/download.py +++ b/src/pip/_internal/download.py @@ -324,18 +324,20 @@ 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 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 or password is not None: - # Store any acquired credentials, converting None to empty strings - # The conversion ensures that this netloc will matched the "cached" - # conditional above. Further, HTTPBasicAuth doesn't accept None so - # it makes sense to cache the value that is going to be used. + # 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 ( From 9d7f3b8814527829e1e6b1833414e04530adad95 Mon Sep 17 00:00:00 2001 From: Pradyun Gedam Date: Sat, 27 Jul 2019 13:50:57 +0700 Subject: [PATCH 5/6] :newspaper: --- news/6795.bugfix | 1 + 1 file changed, 1 insertion(+) create mode 100644 news/6795.bugfix 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. From 0a6c1c75e87cff88fd544f5b21c15db8eb236571 Mon Sep 17 00:00:00 2001 From: Pradyun Gedam Date: Tue, 30 Jul 2019 20:19:24 +0530 Subject: [PATCH 6/6] Improve tests for URL credential parsing --- tests/unit/test_download.py | 47 ++++++++++++++++++++++++++++++++----- 1 file changed, 41 insertions(+), 6 deletions(-) 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():