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..2a831e96ed5 100644 --- a/src/pip/_internal/cli/base_command.py +++ b/src/pip/_internal/cli/base_command.py @@ -81,6 +81,20 @@ 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.""" + index_urls = [] + 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) + # 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 +104,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..c87cf2b5187 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 @@ -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,6 +59,7 @@ except ImportError: ssl = None + HAS_TLS = (ssl is not None) or IS_PYOPENSSL __all__ = ['get_file_content', @@ -70,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 @@ -177,46 +188,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 +370,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 +390,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 +403,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 +538,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 +546,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..168a95c1da9 100644 --- a/tests/unit/test_utils.py +++ b/tests/unit/test_utils.py @@ -29,7 +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, 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 @@ -1009,6 +1009,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'),