Make it possible to request a keyring provider: disabled, import or subprocess

Author: Dos Moonen

docs/html/topics/authentication.md

@@ -66,17 +66,79 @@ man pages][netrc-docs].
 ## Keyring Support
 
 pip supports loading credentials stored in your keyring using the
-{pypi}`keyring` library.
+{pypi}`keyring` library which can be enabled py passing `--keyring-provider`
+with a value of `import`, `subprocess`. Or `disabled` if you configured either
+of the first two to be used via `pip config` and want to turn it off again.
+
+### Configuring Pip
+Passing this as a command line argument will work, but is not how the majority
+of this feature's users will use it. They instead will want to overwrite the
+default of `disabled` in the global, user of site configuration file:
+```bash
+$ pip config set --global global.keyring-provider subprocess
+
+# A different user on the same system which has PYTHONPATH configured and and
+# wanting to use keyring installed that way could then run
+$ pip config set --user global.keyring-provider import
+
+# For a specific virtual environment you might want to use disable it again
+# because you will only be using PyPI and the private repo (and mirror)
+# requires 2FA with a keycard and a pincode
+$ pip config set --site global.index https://pypi.org/simple
+$ pip config set --site global.keyring-provider disabled
+
+# configuring it via environment variable is also possible
+$ export PIP_KEYRING_PROVIDER=disabled
+```
+
+### Installing and using the keyring python module
+
+Setting it to `import` tries to communicate with `keyring` by importing it
+and using its Python api.
 
 ```bash
-$ pip install keyring  # install keyring from PyPI
+# install keyring from PyPI
+$ pip install keyring --index-url https://pypi.org/simple
 $ echo "your-password" | keyring set pypi.company.com your-username
-$ pip install your-package --index-url https://pypi.company.com/
+$ pip install your-package --keyring-provider import --index-url https://pypi.company.com/
+```
+
+### Installing and using the keyring cli application
+
+Setting it to `subprocess` will look for a `keyring` executable on the PATH
+if one can be found that is different from the `keyring` installation `import`
+would be using.
+
+The cli requires a username, therefor you MUST put a username in the url.
+See the example below or the basic HTTP authentication section at the top of
+this page.
+
+```bash
+# install keyring from PyPI using pipx, which we assume if installed properly
+# you can also create a venv somewhere and add it to the PATH yourself instead
+$ pipx install keyring --index-url https://pypi.org/simple
+
+# install the keyring backend for Azure DevOps for example
+$ VssSessionToken is the username you MUST use for this backend
+$ pipx inject keyring artifacts-keyring --index-url https://pypi.org/simple
+
+# or the one for Google Artifact Registry
+$ pipx inject keyring keyrings.google-artifactregistry-auth --index-url https://pypi.org/simple
+$ gcloud auth login
+
+$ pip install your-package --keyring-provider subprocess --index-url https://[email protected]/
 ```
 
+### Here be dragons
+
 Pip is conservative and does not query keyring at all when `--no-input` is used
 because the keyring might require user interaction such as prompting the user
-on the console. You can force keyring usage by passing `--force-keyring` or one
+on the console. Third party tools frequently call Pip for you and do indeed pass
+`--no-input` as they are well-behaved and don't have much information to work
+with. (Keyring does have an api to request a backend that does not require user
+input.) You have more information about your system, however!
+
+You can force keyring usage by passing `--force-keyring` or one
 of the following:
 
 ```bash

news/8719.feature.rst

@@ -0,0 +1,4 @@
+Add ``--keyring-provider`` flag. ``keyring`` support is now disabled by default
+due to the default value for this flag being ``default``. To enable previous
+importing behaviour use ``import``. For the new CLI subprocess behaviour use
+``subprocess``. See the Authentication page in the documentation for more info.

src/pip/_internal/cli/cmdoptions.py

@@ -244,6 +244,19 @@ class PipOption(Option):
     help="Disable prompting for input.",
 )
 
+keyring_provider: Callable[..., Option] = partial(
+    Option,
+    "--keyring-provider",
+    dest="keyring_provider",
+    choices=["disabled", "import", "subprocess"],
+    default="disabled",
+    help=(
+        "Enable the credential lookup via the keyring library if user input is allowed."
+        " Specify which mechanism to use [disabled, import, subprocess]."
+        " (default: disabled)"
+    ),
+)
+
 force_keyring: Callable[..., Option] = partial(
     Option,
     "--force-keyring",
@@ -253,7 +266,8 @@ class PipOption(Option):
     help=(
         "Always query the keyring, regardless of pip's --no-input option. Note"
         " that this may cause problems if the keyring expects to be able to"
-        " prompt the user interactively and no interactive user is available."
+        " prompt the user interactively and no interactive user is available"
+        " or is unable to observe the communication channel."
     ),
 )
 
@@ -1032,6 +1046,7 @@ def check_list_path_option(options: Values) -> None:
         quiet,
         log,
         no_input,
+        keyring_provider,
         force_keyring,
         proxy,
         retries,

src/pip/_internal/cli/req_command.py

@@ -151,6 +151,7 @@ def _build_session(
 
         # Determine if we can prompt the user for authentication or not
         session.auth.prompting = not options.no_input
+        session.auth.keyring_provider = options.keyring_provider
         # We won't use keyring when --no-input is passed unless
         # --force-keyring is passed as well because it might require
         # user interaction

src/pip/_internal/network/auth.py

@@ -3,12 +3,17 @@
 Contains interface (MultiDomainBasicAuth) and associated glue code for
 providing credentials in the context of network requests.
 """
-
+import functools
 import os
 import shutil
 import subprocess
+import sys
+import sysconfig
+import typing
 import urllib.parse
 from abc import ABC, abstractmethod
+from functools import lru_cache
+from pathlib import Path
 from typing import Any, Dict, List, NamedTuple, Optional, Tuple
 
 from pip._vendor.requests.auth import AuthBase, HTTPBasicAuth
@@ -123,7 +128,8 @@ def _get_password(self, service_name: str, username: str) -> Optional[str]:
         res = subprocess.run(
             cmd,
             stdin=subprocess.DEVNULL,
-            capture_output=True,
+            stdout=subprocess.PIPE,
+            stderr=sys.stderr,
             env=env,
         )
         if res.returncode:
@@ -144,12 +150,19 @@ def _set_password(self, service_name: str, username: str, password: str) -> None
         return None
 
 
-def get_keyring_provider() -> KeyRingBaseProvider:
+@lru_cache(maxsize=None)
+def get_keyring_provider(provider: str) -> KeyRingBaseProvider:
+    logger.verbose("Keyring provider requested: %s", provider)
+
     # keyring has previously failed and been disabled
-    if not KEYRING_DISABLED:
-        # Default to trying to use Python provider
+    if KEYRING_DISABLED or provider == "disabled":
+        pass
+    elif provider == "import":
         try:
-            return KeyRingPythonProvider()
+            try:
+                return KeyRingPythonProvider()
+            finally:
+                logger.verbose("Keyring provider set: import")
         except ImportError:
             pass
         except Exception as exc:
@@ -160,22 +173,54 @@ def get_keyring_provider() -> KeyRingBaseProvider:
                 "trying to find a keyring executable as a fallback",
                 str(exc),
             )
-
-        # Fallback to Cli Provider if `keyring` isn't installed
+    elif provider == "subprocess":
         cli = shutil.which("keyring")
+        if cli and cli.startswith(sysconfig.get_path("scripts")):
+            # all code within this function is stolen from shutil.which implementation
+            @typing.no_type_check
+            def PATH_as_shutil_which_determines_it() -> str:
+                path = os.environ.get("PATH", None)
+                if path is None:
+                    try:
+                        path = os.confstr("CS_PATH")
+                    except (AttributeError, ValueError):
+                        # os.confstr() or CS_PATH is not available
+                        path = os.defpath
+                # bpo-35755: Don't use os.defpath if the PATH environment variable is
+                # set to an empty string
+
+                return path
+
+            scripts = Path(sysconfig.get_path("scripts")).resolve()
+
+            paths = []
+            for path in PATH_as_shutil_which_determines_it().split(os.pathsep):
+                p = Path(path)
+                if p.exists() and not p.resolve().samefile(scripts):
+                    paths.append(path)
+
+            path = os.pathsep.join(paths)
+
+            cli = shutil.which("keyring", path=path)
+
         if cli:
+            logger.verbose("Keyring provider set: subprocess with executable %s", cli)
             return KeyRingCliProvider(cli)
+    else:
+        logger.verbose("Unknown keyring provider requested: %s", provider)
 
+    logger.verbose("Keyring provider set: disabled")
     return KeyRingNullProvider()
 
 
-def get_keyring_auth(url: Optional[str], username: Optional[str]) -> Optional[AuthInfo]:
+def get_keyring_auth(
+    keyring: KeyRingBaseProvider, url: Optional[str], username: Optional[str]
+) -> Optional[AuthInfo]:
     """Return the tuple auth for a given url from keyring."""
     # Do nothing if no url was provided
     if not url:
         return None
 
-    keyring = get_keyring_provider()
     try:
         return keyring.get_auth_info(url, username)
     except Exception as exc:
@@ -185,6 +230,7 @@ def get_keyring_auth(url: Optional[str], username: Optional[str]) -> Optional[Au
         )
         global KEYRING_DISABLED
         KEYRING_DISABLED = True
+        get_keyring_provider.cache_clear()
         return None
 
 
@@ -193,10 +239,12 @@ def __init__(
         self,
         prompting: bool = True,
         index_urls: Optional[List[str]] = None,
+        keyring_provider: str = "disabled",
         use_keyring: bool = True,
     ) -> None:
         self.prompting = prompting
         self.index_urls = index_urls
+        self.keyring_provider = keyring_provider  # type: ignore[assignment]
         self.use_keyring = use_keyring
         self.passwords: Dict[str, AuthInfo] = {}
         # When the user is prompted to enter credentials and keyring is
@@ -206,6 +254,18 @@ def __init__(
         # ``save_credentials`` to save these.
         self._credentials_to_save: Optional[Credentials] = None
 
+    @property
+    def keyring_provider(self) -> KeyRingBaseProvider:
+        return self._keyring_provider()
+
+    @keyring_provider.setter
+    def keyring_provider(self, provider: str) -> None:
+        # The free function get_keyring_provider has been decorated with
+        # functools.cache. If an exception occurs in get_keyring_auth that
+        # cache will be cleared and keyring disabled, take that into account
+        # if you want to remove this indirection.
+        self._keyring_provider = functools.partial(get_keyring_provider, provider)
+
     def _get_index_url(self, url: str) -> Optional[str]:
         """Return the original index URL matching the requested URL.
 
@@ -275,8 +335,8 @@ def _get_new_credentials(
             # The index url is more specific than the netloc, so try it first
             # fmt: off
             kr_auth = (
-                get_keyring_auth(index_url, username) or
-                get_keyring_auth(netloc, username)
+                get_keyring_auth(self.keyring_provider, index_url, username) or
+                get_keyring_auth(self.keyring_provider, netloc, username)
             )
             # fmt: on
             if kr_auth:
@@ -356,15 +416,15 @@ def _prompt_for_password(
         username = ask_input(f"User for {netloc}: ") if self.prompting else None
         if not username:
             return None, None, False
-        auth = get_keyring_auth(netloc, username)
+        auth = get_keyring_auth(self.keyring_provider, netloc, username)
         if auth and auth[0] is not None and auth[1] is not None:
             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) -> bool:
-        if not self.prompting or get_keyring_provider() is None:
+        if not self.prompting or isinstance(self.keyring_provider, KeyRingNullProvider):
             return False
         return ask("Save credentials to keyring [y/N]: ", ["y", "n"]) == "y"
 
@@ -439,16 +499,17 @@ def warn_on_401(self, resp: Response, **kwargs: Any) -> None:
 
     def save_credentials(self, resp: Response, **kwargs: Any) -> None:
         """Response callback to save credentials on success."""
-        keyring = get_keyring_provider()
         assert not isinstance(
-            keyring, KeyRingNullProvider
+            self.keyring_provider, KeyRingNullProvider
         ), "should never reach here without keyring"
 
         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.save_auth_info(creds.url, creds.username, creds.password)
+                self.keyring_provider.save_auth_info(
+                    creds.url, creds.username, creds.password
+                )
             except Exception:
                 logger.exception("Failed to save credentials")

tests/conftest.py

@@ -10,6 +10,7 @@
 from pathlib import Path
 from typing import (
     TYPE_CHECKING,
+    AnyStr,
     Callable,
     Dict,
     Iterable,
@@ -507,7 +508,10 @@ def with_wheel(virtualenv: VirtualEnvironment, wheel_install: Path) -> None:
 
 class ScriptFactory(Protocol):
     def __call__(
-        self, tmpdir: Path, virtualenv: Optional[VirtualEnvironment] = None
+        self,
+        tmpdir: Path,
+        virtualenv: Optional[VirtualEnvironment] = None,
+        environ: Optional[Dict[AnyStr, AnyStr]] = None,
     ) -> PipTestEnvironment:
         ...
 
@@ -521,7 +525,11 @@ def script_factory(
     def factory(
         tmpdir: Path,
         virtualenv: Optional[VirtualEnvironment] = None,
+        environ: Optional[Dict[AnyStr, AnyStr]] = None,
     ) -> PipTestEnvironment:
+        kwargs = {}
+        if environ:
+            kwargs["environ"] = environ
         if virtualenv is None:
             virtualenv = virtualenv_factory(tmpdir.joinpath("venv"))
         return PipTestEnvironment(
@@ -541,6 +549,7 @@ def factory(
             pip_expect_warning=deprecated_python,
             # Tell the Test Environment if we want to run pip via a zipapp
             zipapp=zipapp,
+            **kwargs,
         )
 
     return factory