From c63ee61027fe7dea421c724a31bcbbb37c8f318a Mon Sep 17 00:00:00 2001 From: Steve Dower Date: Tue, 12 Feb 2019 08:52:49 -0800 Subject: [PATCH 1/4] Issue #5948: Enable keyring support This requires keyring and any backends to be installed separately. Once discovered, it will be used to retrieve credentials by index URL and netloc before prompting. If the user is prompted and the credentials work, they will (optionally) be saved to keyring against the netloc of the requested URL. --- news/5948.feature | 1 + src/pip/_internal/cli/base_command.py | 16 ++ src/pip/_internal/download.py | 220 ++++++++++++++++++++++---- src/pip/_internal/utils/misc.py | 69 ++++++-- tests/unit/test_download.py | 215 ++++++++++++++++++++++++- tests/unit/test_utils.py | 35 +++- 6 files changed, 509 insertions(+), 47 deletions(-) create mode 100644 news/5948.feature diff --git a/news/5948.feature b/news/5948.feature new file mode 100644 index 00000000000..af2d8cdd63f --- /dev/null +++ b/news/5948.feature @@ -0,0 +1 @@ +Credentials will now be loaded using `keyring` when installed. diff --git a/src/pip/_internal/cli/base_command.py b/src/pip/_internal/cli/base_command.py index f6108c96eb5..4313e4968f8 100644 --- a/src/pip/_internal/cli/base_command.py +++ b/src/pip/_internal/cli/base_command.py @@ -81,6 +81,21 @@ def run(self, options, args): # type: (Values, List[Any]) -> Any raise NotImplementedError + @classmethod + def _get_index_urls(cls, options): + """Return a list of index urls from user-provided options.""" + if getattr(options, "no_index", False): + return None + index_urls = [] + url = getattr(options, "index_url", None) + if url: + index_urls.append(url) + urls = getattr(options, "extra_index_urls", None) + if urls: + index_urls.extend(urls) + # Return None rather than an empty list + return index_urls or None + def _build_session(self, options, retries=None, timeout=None): # type: (Values, Optional[int], Optional[int]) -> PipSession session = PipSession( @@ -90,6 +105,7 @@ def _build_session(self, options, retries=None, timeout=None): ), retries=retries if retries is not None else options.retries, insecure_hosts=options.trusted_hosts, + index_urls=self._get_index_urls(options), ) # Handle custom ca-bundles from the user diff --git a/src/pip/_internal/download.py b/src/pip/_internal/download.py index 2683cf08293..648f95af0e2 100644 --- a/src/pip/_internal/download.py +++ b/src/pip/_internal/download.py @@ -2,7 +2,6 @@ import cgi import email.utils -import getpass import json import logging import mimetypes @@ -12,7 +11,7 @@ import shutil import sys -from pip._vendor import requests, six, urllib3 +from pip._vendor import requests, urllib3 from pip._vendor.cachecontrol import CacheControlAdapter from pip._vendor.cachecontrol.caches import FileCache from pip._vendor.lockfile import LockError @@ -36,9 +35,10 @@ from pip._internal.utils.filesystem import check_path_owner from pip._internal.utils.glibc import libc_ver from pip._internal.utils.misc import ( - ARCHIVE_EXTENSIONS, ask_path_exists, backup_dir, consume, display_path, - format_size, get_installed_version, rmtree, split_auth_from_netloc, - splitext, unpack_file, + ARCHIVE_EXTENSIONS, ask, ask_input, ask_password, ask_path_exists, + backup_dir, consume, display_path, format_size, get_installed_version, + remove_auth_from_url, rmtree, split_auth_netloc_from_url, splitext, + unpack_file, ) from pip._internal.utils.temp_dir import TempDirectory from pip._internal.utils.typing import MYPY_CHECK_RUNNING @@ -58,6 +58,11 @@ except ImportError: ssl = None +try: + import keyring # noqa +except ImportError: + keyring = None + HAS_TLS = (ssl is not None) or IS_PYOPENSSL __all__ = ['get_file_content', @@ -177,46 +182,175 @@ def user_agent(): ) +def _get_keyring_auth(url, username): + """Return the tuple auth for a given url from keyring.""" + if not url or not keyring: + return None + + try: + try: + get_credential = keyring.get_credential + except AttributeError: + pass + else: + logger.debug("Getting credentials from keyring for %s", url) + cred = get_credential(url, username) + if cred is not None: + return cred.username, cred.password + return None + + if username: + logger.debug("Getting password from keyring for %s", url) + password = keyring.get_password(url, username) + if password: + return username, password + + except Exception as exc: + logger.warning("Keyring is skipped due to an exception: %s", + str(exc)) + + class MultiDomainBasicAuth(AuthBase): - def __init__(self, prompting=True): - # type: (bool) -> None + def __init__(self, prompting=True, index_urls=None): + # type: (bool, Optional[Values]) -> None self.prompting = prompting + self.index_urls = index_urls self.passwords = {} # type: Dict[str, AuthInfo] + # When the user is prompted to enter credentials and keyring is + # available, we will offer to save them. If the user accepts, + # this value is set to the credentials they entered. After the + # request authenticates, the caller should call + # ``save_credentials`` to save these. + self._credentials_to_save = None # type: Tuple[str, str, str] + + def _get_index_url(self, url): + """Return the original index URL matching the requested URL. + + Cached or dynamically generated credentials may work against + the original index URL rather than just the netloc. + + The provided url should have had its username and password + removed already. If the original index url had credentials then + they will be included in the return value. + + Returns None if no matching index was found, or if --no-index + was specified by the user. + """ + if not url or not self.index_urls: + return None + + for u in self.index_urls: + prefix = remove_auth_from_url(u).rstrip("/") + "/" + if url.startswith(prefix): + return u + + def _get_new_credentials(self, original_url, allow_netrc=True, + allow_keyring=True): + """Find and return credentials for the specified URL.""" + # Split the credentials and netloc from the url. + url, netloc, url_user_password = split_auth_netloc_from_url( + original_url) + + # Start with the credentials embedded in the url + username, password = url_user_password + if username is not None and password is not None: + logger.debug("Found credentials in url for %s", netloc) + return url_user_password + + # Find a matching index url for this request + index_url = self._get_index_url(url) + if index_url: + # Split the credentials from the url. + index_info = split_auth_netloc_from_url(index_url) + if index_info: + index_url, _, index_url_user_password = index_info + logger.debug("Found index url %s", index_url) + + # If an index URL was found, try its embedded credentials + if index_url and index_url_user_password[0] is not None: + username, password = index_url_user_password + if username is not None and password is not None: + logger.debug("Found credentials in index url for %s", netloc) + return index_url_user_password - def __call__(self, req): - parsed = urllib_parse.urlparse(req.url) - - # Split the credentials from the netloc. - netloc, url_user_password = split_auth_from_netloc(parsed.netloc) - - # Set the url of the request to the url without any credentials - req.url = urllib_parse.urlunparse(parsed[:1] + (netloc,) + parsed[2:]) + # Get creds from netrc if we still don't have them + if allow_netrc: + netrc_auth = get_netrc_auth(original_url) + if netrc_auth: + logger.debug("Found credentials in netrc for %s", netloc) + return netrc_auth + + # If we don't have a password and keyring is available, use it. + if allow_keyring: + # The index url is more specific than the netloc, so try it first + kr_auth = (_get_keyring_auth(index_url, username) or + _get_keyring_auth(netloc, username)) + if kr_auth: + logger.debug("Found credentials in keyring for %s", netloc) + return kr_auth + + return None, None + + def _get_url_and_credentials(self, original_url): + """Return the credentials to use for the provided URL. + + If allowed, netrc and keyring may be used to obtain the + correct credentials. + + Returns (url_without_credentials, username, password). Note + that even if the original URL contains credentials, this + function may return a different username and password. + """ + url, netloc, _ = split_auth_netloc_from_url(original_url) # Use any stored credentials that we have for this netloc username, password = self.passwords.get(netloc, (None, None)) - # Use the credentials embedded in the url if we have none stored - if username is None: - username, password = url_user_password - - # Get creds from netrc if we still don't have them - if username is None and password is None: - netrc_auth = get_netrc_auth(req.url) - username, password = netrc_auth if netrc_auth else (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: + username, password = self._get_new_credentials(original_url) - if username or password: + if username is not None and password is not None: # Store the username and password self.passwords[netloc] = (username, password) + return url, username, password + + def __call__(self, req): + # Get credentials for this request + url, username, password = self._get_url_and_credentials(req.url) + + # Set the url of the request to the url without any credentials + req.url = url + + if username is not None and password is not None: # Send the basic auth with this request - req = HTTPBasicAuth(username or "", password or "")(req) + req = HTTPBasicAuth(username, password)(req) # Attach a hook to handle 401 responses req.register_hook("response", self.handle_401) return req + # Factored out to allow for easy patching in tests + def _prompt_for_password(self, netloc): + username = ask_input("User for %s: " % netloc) + if not username: + return None, None + auth = _get_keyring_auth(netloc, username) + if auth: + return auth[0], auth[1], False + password = ask_password("Password: ") + return username, password, True + + # Factored out to allow for easy patching in tests + def _should_save_password_to_keyring(self): + if not keyring: + return False + return ask("Save credentials to keyring [y/N]: ", ["y", "n"]) == "y" + def handle_401(self, resp, **kwargs): # We only care about 401 responses, anything else we want to just # pass through the actual response @@ -230,13 +364,17 @@ def handle_401(self, resp, **kwargs): parsed = urllib_parse.urlparse(resp.url) # Prompt the user for a new username and password - username = six.moves.input("User for %s: " % parsed.netloc) - password = getpass.getpass("Password: ") + username, password, save = self._prompt_for_password(parsed.netloc) # Store the new username and password to use for future requests - if username or password: + self._credentials_to_save = None + if username is not None and password is not None: self.passwords[parsed.netloc] = (username, password) + # Prompt to save the password to keyring + if save and self._should_save_password_to_keyring(): + self._credentials_to_save = (parsed.netloc, username, password) + # Consume content and release the original connection to allow our new # request to reuse the same one. resp.content @@ -246,6 +384,12 @@ def handle_401(self, resp, **kwargs): req = HTTPBasicAuth(username or "", password or "")(resp.request) req.register_hook("response", self.warn_on_401) + # On successful request, save the credentials that were used to + # keyring. (Note that if the user responded "no" above, this member + # is not set and nothing will be saved.) + if self._credentials_to_save: + req.register_hook("response", self.save_credentials) + # Send our new request new_resp = resp.connection.send(req, **kwargs) new_resp.history.append(resp) @@ -253,11 +397,26 @@ def handle_401(self, resp, **kwargs): return new_resp def warn_on_401(self, resp, **kwargs): - # warn user that they provided incorrect credentials + """Response callback to warn about incorrect credentials.""" if resp.status_code == 401: logger.warning('401 Error, Credentials not correct for %s', resp.request.url) + def save_credentials(self, resp, **kwargs): + """Response callback to save credentials on success.""" + assert keyring is not None, "should never reach here without keyring" + if not keyring: + return + + creds = self._credentials_to_save + self._credentials_to_save = None + if creds and resp.status_code < 400: + try: + logger.info('Saving credentials to keyring') + keyring.set_password(*creds) + except Exception: + logger.exception('Failed to save credentials') + class LocalFSAdapter(BaseAdapter): @@ -373,6 +532,7 @@ def __init__(self, *args, **kwargs): retries = kwargs.pop("retries", 0) cache = kwargs.pop("cache", None) insecure_hosts = kwargs.pop("insecure_hosts", []) + index_urls = kwargs.pop("index_urls", None) super(PipSession, self).__init__(*args, **kwargs) @@ -380,7 +540,7 @@ def __init__(self, *args, **kwargs): self.headers["User-Agent"] = user_agent() # Attach our Authentication handler to the session - self.auth = MultiDomainBasicAuth() + self.auth = MultiDomainBasicAuth(index_urls=index_urls) # Create our urllib3.Retry instance which will allow us to customize # how we handle retries. diff --git a/src/pip/_internal/utils/misc.py b/src/pip/_internal/utils/misc.py index ca7a529387c..49f3bf22f59 100644 --- a/src/pip/_internal/utils/misc.py +++ b/src/pip/_internal/utils/misc.py @@ -2,6 +2,7 @@ import contextlib import errno +import getpass import io # we have a submodule named 'logging' which would shadow this if we used the # regular name: @@ -171,15 +172,21 @@ def ask_path_exists(message, options): return ask(message, options) +def _check_no_input(message): + # type: (str) -> None + """Raise an error if no input is allowed.""" + if os.environ.get('PIP_NO_INPUT'): + raise Exception( + 'No input was expected ($PIP_NO_INPUT set); question: %s' % + message + ) + + def ask(message, options): # type: (str, Iterable[str]) -> str """Ask the message interactively, with the given possible responses""" while 1: - if os.environ.get('PIP_NO_INPUT'): - raise Exception( - 'No input was expected ($PIP_NO_INPUT set); question: %s' % - message - ) + _check_no_input(message) response = input(message) response = response.strip().lower() if response not in options: @@ -191,6 +198,20 @@ def ask(message, options): return response +def ask_input(message): + # type: (str) -> str + """Ask for input interactively.""" + _check_no_input(message) + return input(message) + + +def ask_password(message): + # type: (str) -> str + """Ask for a password interactively.""" + _check_no_input(message) + return getpass.getpass(message) + + def format_size(bytes): # type: (float) -> str if bytes > 1000 * 1000: @@ -954,32 +975,56 @@ def redact_netloc(netloc): def _transform_url(url, transform_netloc): + """Transform and replace netloc in a url. + + transform_netloc is a function taking the netloc and returning a + tuple. The first element of this tuple is the new netloc. The + entire tuple is returned. + + Returns a tuple containing the transformed url as item 0 and the + original tuple returned by transform_netloc as item 1. + """ purl = urllib_parse.urlsplit(url) - netloc = transform_netloc(purl.netloc) + netloc_tuple = transform_netloc(purl.netloc) # stripped url url_pieces = ( - purl.scheme, netloc, purl.path, purl.query, purl.fragment + purl.scheme, netloc_tuple[0], purl.path, purl.query, purl.fragment ) surl = urllib_parse.urlunsplit(url_pieces) - return surl + return surl, netloc_tuple def _get_netloc(netloc): - return split_auth_from_netloc(netloc)[0] + return split_auth_from_netloc(netloc) + + +def _redact_netloc(netloc): + return (redact_netloc(netloc),) + + +def split_auth_netloc_from_url(url): + # type: (str) -> Tuple[str, str, Tuple[str, str]] + """ + Parse a url into separate netloc, auth, and url with no auth. + + Returns: (url_without_auth, netloc, (username, password)) + """ + url_without_auth, (netloc, auth) = _transform_url(url, _get_netloc) + return url_without_auth, netloc, auth def remove_auth_from_url(url): # type: (str) -> str - # Return a copy of url with 'username:password@' removed. + """Return a copy of url with 'username:password@' removed.""" # username/pass params are passed to subversion through flags # and are not recognized in the url. - return _transform_url(url, _get_netloc) + return _transform_url(url, _get_netloc)[0] def redact_password_from_url(url): # type: (str) -> str """Replace the password in a given url with ****.""" - return _transform_url(url, redact_netloc) + return _transform_url(url, _redact_netloc)[0] def protect_pip_from_modification_on_windows(modifying_pip): diff --git a/tests/unit/test_download.py b/tests/unit/test_download.py index 73a3d42b751..57f3cdeb5fa 100644 --- a/tests/unit/test_download.py +++ b/tests/unit/test_download.py @@ -1,3 +1,4 @@ +import functools import hashlib import os import sys @@ -11,8 +12,8 @@ import pip from pip._internal.download import ( - CI_ENVIRONMENT_VARIABLES, PipSession, SafeFileCache, path_to_url, - unpack_file_url, unpack_http_url, url_to_path, + CI_ENVIRONMENT_VARIABLES, MultiDomainBasicAuth, PipSession, SafeFileCache, + path_to_url, unpack_file_url, unpack_http_url, url_to_path, ) from pip._internal.exceptions import HashMismatch from pip._internal.models.link import Link @@ -111,16 +112,49 @@ def read(self, size, decode_content=None): def stream(self, size, decode_content=None): yield self._io.read(size) + def release_conn(self): + pass + class MockResponse(object): def __init__(self, contents): self.raw = FakeStream(contents) + self.content = contents + self.request = None + self.status_code = 200 + self.connection = None + self.url = None + self.headers = {} + self.history = [] def raise_for_status(self): pass +class MockConnection(object): + + def _send(self, req, **kwargs): + raise NotImplementedError("_send must be overridden for tests") + + def send(self, req, **kwargs): + resp = self._send(req, **kwargs) + for cb in req.hooks.get("response", []): + cb(resp) + return resp + + +class MockRequest(object): + + def __init__(self, url): + self.url = url + self.headers = {} + self.hooks = {} + + def register_hook(self, event_name, callback): + self.hooks.setdefault(event_name, []).append(callback) + + @patch('pip._internal.download.unpack_file') def test_unpack_http_url_bad_downloaded_checksum(mock_unpack_file): """ @@ -378,3 +412,180 @@ def test_insecure_host_cache_is_not_enabled(self, tmpdir): ) assert not hasattr(session.adapters["https://example.com/"], "cache") + + +def test_get_credentials(): + 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') + + auth.passwords['example.com'] = ('user', 'pass') + assert get("http://foo:bar@example.com/path") \ + == ('http://example.com/path', 'user', 'pass') + + +def test_get_index_url_credentials(): + auth = MultiDomainBasicAuth(index_urls=[ + "http://foo:bar@example.com/path" + ]) + get = functools.partial( + auth._get_new_credentials, + allow_netrc=False, + allow_keyring=False + ) + + # Check resolution of indexes + assert get("http://example.com/path/path2") == ('foo', 'bar') + assert get("http://example.com/path3/path2") == (None, None) + + +class KeyringModuleV1(object): + """Represents the supported API of keyring before get_credential + was added. + """ + + def __init__(self): + self.saved_passwords = [] + + def get_password(self, system, username): + if system == "example.com" and username: + return username + "!netloc" + if system == "http://example.com/path2" and username: + return username + "!url" + return None + + def set_password(self, system, username, password): + self.saved_passwords.append((system, username, password)) + + +@pytest.mark.parametrize('url, expect', ( + ("http://example.com/path1", (None, None)), + # path1 URLs will be resolved by netloc + ("http://user@example.com/path1", ("user", "user!netloc")), + ("http://user2@example.com/path1", ("user2", "user2!netloc")), + # path2 URLs will be resolved by index URL + ("http://example.com/path2/path3", (None, None)), + ("http://foo@example.com/path2/path3", ("foo", "foo!url")), +)) +def test_keyring_get_password(monkeypatch, url, expect): + monkeypatch.setattr('pip._internal.download.keyring', KeyringModuleV1()) + auth = MultiDomainBasicAuth(index_urls=["http://example.com/path2"]) + + actual = auth._get_new_credentials(url, allow_netrc=False, + allow_keyring=True) + assert actual == expect + + +def test_keyring_get_password_after_prompt(monkeypatch): + monkeypatch.setattr('pip._internal.download.keyring', KeyringModuleV1()) + auth = MultiDomainBasicAuth() + + def ask_input(prompt): + assert prompt == "User for example.com: " + return "user" + + monkeypatch.setattr('pip._internal.download.ask_input', ask_input) + actual = auth._prompt_for_password("example.com") + assert actual == ("user", "user!netloc", False) + + +def test_keyring_get_password_username_in_index(monkeypatch): + monkeypatch.setattr('pip._internal.download.keyring', KeyringModuleV1()) + auth = MultiDomainBasicAuth(index_urls=["http://user@example.com/path2"]) + get = functools.partial( + auth._get_new_credentials, + allow_netrc=False, + allow_keyring=True + ) + + assert get("http://example.com/path2/path3") == ("user", "user!url") + assert get("http://example.com/path4/path1") == (None, None) + + +@pytest.mark.parametrize("response_status, creds, expect_save", ( + (403, ("user", "pass", True), False), + (200, ("user", "pass", True), True,), + (200, ("user", "pass", False), False,), +)) +def test_keyring_set_password(monkeypatch, response_status, creds, + expect_save): + keyring = KeyringModuleV1() + monkeypatch.setattr('pip._internal.download.keyring', keyring) + auth = MultiDomainBasicAuth(prompting=True) + monkeypatch.setattr(auth, '_get_url_and_credentials', + lambda u: (u, None, None)) + monkeypatch.setattr(auth, '_prompt_for_password', lambda *a: creds) + if creds[2]: + # when _prompt_for_password indicates to save, we should save + def should_save_password_to_keyring(*a): + return True + else: + # when _prompt_for_password indicates not to save, we should + # never call this function + def should_save_password_to_keyring(*a): + assert False, ("_should_save_password_to_keyring should not be " + + "called") + monkeypatch.setattr(auth, '_should_save_password_to_keyring', + should_save_password_to_keyring) + + req = MockRequest("https://example.com") + resp = MockResponse(b"") + resp.url = req.url + connection = MockConnection() + + def _send(sent_req, **kwargs): + assert sent_req is req + assert "Authorization" in sent_req.headers + r = MockResponse(b"") + r.status_code = response_status + return r + + connection._send = _send + + resp.request = req + resp.status_code = 401 + resp.connection = connection + + auth.handle_401(resp) + + if expect_save: + assert keyring.saved_passwords == [("example.com", creds[0], creds[1])] + else: + assert keyring.saved_passwords == [] + + +class KeyringModuleV2(object): + """Represents the current supported API of keyring""" + + class Credential(object): + def __init__(self, username, password): + self.username = username + self.password = password + + def get_password(self, system, username): + assert False, "get_password should not ever be called" + + def get_credential(self, system, username): + if system == "http://example.com/path2": + return self.Credential("username", "url") + if system == "example.com": + return self.Credential("username", "netloc") + return None + + +@pytest.mark.parametrize('url, expect', ( + ("http://example.com/path1", ("username", "netloc")), + ("http://example.com/path2/path3", ("username", "url")), + ("http://user2@example.com/path2/path3", ("username", "url")), +)) +def test_keyring_get_credential(monkeypatch, url, expect): + monkeypatch.setattr(pip._internal.download, 'keyring', KeyringModuleV2()) + auth = MultiDomainBasicAuth(index_urls=["http://example.com/path2"]) + + assert auth._get_new_credentials(url, allow_netrc=False, + allow_keyring=True) \ + == expect diff --git a/tests/unit/test_utils.py b/tests/unit/test_utils.py index dc508b614a0..6ca6fd684ab 100644 --- a/tests/unit/test_utils.py +++ b/tests/unit/test_utils.py @@ -27,9 +27,10 @@ from pip._internal.utils.hashes import Hashes, MissingHashes from pip._internal.utils.misc import ( call_subprocess, egg_link_path, ensure_dir, format_command_args, - get_installed_distributions, get_prog, normalize_path, redact_netloc, - redact_password_from_url, remove_auth_from_url, rmtree, - split_auth_from_netloc, untar_file, unzip_file, + get_installed_distributions, get_prog, make_vcs_requirement_url, + normalize_path, redact_netloc, redact_password_from_url, + remove_auth_from_url, rmtree, split_auth_from_netloc, + split_auth_netloc_from_url, untar_file, unzip_file, ) from pip._internal.utils.packaging import check_dist_requires_python from pip._internal.utils.temp_dir import AdjacentTempDirectory, TempDirectory @@ -1009,6 +1010,34 @@ def test_split_auth_from_netloc(netloc, expected): assert actual == expected +@pytest.mark.parametrize('url, expected', [ + # Test a basic case. + ('http://example.com/path#anchor', + ('http://example.com/path#anchor', 'example.com', (None, None))), + # Test with username and no password. + ('http://user@example.com/path#anchor', + ('http://example.com/path#anchor', 'example.com', ('user', None))), + # Test with username and password. + ('http://user:pass@example.com/path#anchor', + ('http://example.com/path#anchor', 'example.com', ('user', 'pass'))), + # Test with username and empty password. + ('http://user:@example.com/path#anchor', + ('http://example.com/path#anchor', 'example.com', ('user', ''))), + # Test the password containing an @ symbol. + ('http://user:pass@word@example.com/path#anchor', + ('http://example.com/path#anchor', 'example.com', ('user', 'pass@word'))), + # Test the password containing a : symbol. + ('http://user:pass:word@example.com/path#anchor', + ('http://example.com/path#anchor', 'example.com', ('user', 'pass:word'))), + # Test URL-encoded reserved characters. + ('http://user%3Aname:%23%40%5E@example.com/path#anchor', + ('http://example.com/path#anchor', 'example.com', ('user:name', '#@^'))), +]) +def test_split_auth_netloc_from_url(url, expected): + actual = split_auth_netloc_from_url(url) + assert actual == expected + + @pytest.mark.parametrize('netloc, expected', [ # Test a basic case. ('example.com', 'example.com'), From e04cb497c87c7f9b221fcb6c815659005c5d81d5 Mon Sep 17 00:00:00 2001 From: Steve Dower Date: Tue, 7 May 2019 09:59:43 -0400 Subject: [PATCH 2/4] Fix mismerged import --- tests/unit/test_utils.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/unit/test_utils.py b/tests/unit/test_utils.py index 6ca6fd684ab..23f3c53d234 100644 --- a/tests/unit/test_utils.py +++ b/tests/unit/test_utils.py @@ -27,10 +27,10 @@ from pip._internal.utils.hashes import Hashes, MissingHashes from pip._internal.utils.misc import ( call_subprocess, egg_link_path, ensure_dir, format_command_args, - get_installed_distributions, get_prog, make_vcs_requirement_url, - normalize_path, redact_netloc, redact_password_from_url, - remove_auth_from_url, rmtree, split_auth_from_netloc, - split_auth_netloc_from_url, untar_file, unzip_file, + get_installed_distributions, get_prog, normalize_path, redact_netloc, + redact_password_from_url, remove_auth_from_url, rmtree, + split_auth_from_netloc, split_auth_netloc_from_url, untar_file, + unzip_file, ) from pip._internal.utils.packaging import check_dist_requires_python from pip._internal.utils.temp_dir import AdjacentTempDirectory, TempDirectory From 49b9298a44f1bfa7ab5be00e9c71021094c55a28 Mon Sep 17 00:00:00 2001 From: Steve Dower Date: Tue, 7 May 2019 17:01:41 -0400 Subject: [PATCH 3/4] Improve import error handling Fix --no-index usage Fix missing type annotation type --- src/pip/_internal/cli/base_command.py | 9 ++++----- src/pip/_internal/download.py | 14 ++++++++++---- 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/src/pip/_internal/cli/base_command.py b/src/pip/_internal/cli/base_command.py index 4313e4968f8..2a831e96ed5 100644 --- a/src/pip/_internal/cli/base_command.py +++ b/src/pip/_internal/cli/base_command.py @@ -84,12 +84,11 @@ def run(self, options, args): @classmethod def _get_index_urls(cls, options): """Return a list of index urls from user-provided options.""" - if getattr(options, "no_index", False): - return None index_urls = [] - url = getattr(options, "index_url", None) - if url: - index_urls.append(url) + if not getattr(options, "no_index", False): + url = getattr(options, "index_url", None) + if url: + index_urls.append(url) urls = getattr(options, "extra_index_urls", None) if urls: index_urls.extend(urls) diff --git a/src/pip/_internal/download.py b/src/pip/_internal/download.py index 648f95af0e2..c87cf2b5187 100644 --- a/src/pip/_internal/download.py +++ b/src/pip/_internal/download.py @@ -49,6 +49,7 @@ from typing import ( Optional, Tuple, Dict, IO, Text, Union ) + from optparse import Values from pip._internal.models.link import Link from pip._internal.utils.hashes import Hashes from pip._internal.vcs import AuthInfo @@ -58,10 +59,6 @@ except ImportError: ssl = None -try: - import keyring # noqa -except ImportError: - keyring = None HAS_TLS = (ssl is not None) or IS_PYOPENSSL @@ -75,6 +72,15 @@ logger = logging.getLogger(__name__) +try: + import keyring # noqa +except ImportError: + keyring = None +except Exception as exc: + logger.warning("Keyring is skipped due to an exception: %s", + str(exc)) + keyring = None + # These are environment variables present when running under various # CI systems. For each variable, some CI systems that use the variable # are indicated. The collection was chosen so that for each of a number From 3816a747e1c3eaf20458be3f59d170df4bd1b51a Mon Sep 17 00:00:00 2001 From: Steve Dower Date: Wed, 8 May 2019 10:58:25 -0400 Subject: [PATCH 4/4] Upwrap import --- tests/unit/test_utils.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/unit/test_utils.py b/tests/unit/test_utils.py index 23f3c53d234..168a95c1da9 100644 --- a/tests/unit/test_utils.py +++ b/tests/unit/test_utils.py @@ -29,8 +29,7 @@ call_subprocess, egg_link_path, ensure_dir, format_command_args, get_installed_distributions, get_prog, normalize_path, redact_netloc, redact_password_from_url, remove_auth_from_url, rmtree, - split_auth_from_netloc, split_auth_netloc_from_url, untar_file, - unzip_file, + split_auth_from_netloc, split_auth_netloc_from_url, untar_file, unzip_file, ) from pip._internal.utils.packaging import check_dist_requires_python from pip._internal.utils.temp_dir import AdjacentTempDirectory, TempDirectory