Make keyring integration opt-in (closes #8719, #6773)

news/8719.feature.rst

@@ -0,0 +1 @@
+Make support for the ``keyring`` library opt-in with the ``--enable-keyring`` flag

src/pip/_internal/cli/cmdoptions.py

@@ -1001,6 +1001,15 @@ def check_list_path_option(options: Values) -> None:
     help=("Enable deprecated functionality, that will be removed in the future."),
 )
 
+keyring: Callable[..., Option] = partial(
+    Option,
+    "--enable-keyring",
+    dest="keyring",
+    action="store_true",
+    default=False,
+    help="Enable keyring authentication",
+)
+
 
 ##########
 # groups #
@@ -1022,6 +1031,7 @@ def check_list_path_option(options: Values) -> None:
         proxy,
         retries,
         timeout,
+        keyring,
         exists_action,
         trusted_host,
         cert,

src/pip/_internal/cli/req_command.py

@@ -128,6 +128,7 @@ def _build_session(
             trusted_hosts=options.trusted_hosts,
             index_urls=self._get_index_urls(options),
             ssl_context=ssl_context,
+            allow_keyring=options.keyring,
         )
 
         # Handle custom ca-bundles from the user

src/pip/_internal/network/auth.py

@@ -190,11 +190,15 @@ def get_keyring_auth(url: Optional[str], username: Optional[str]) -> Optional[Au
 
 class MultiDomainBasicAuth(AuthBase):
     def __init__(
-        self, prompting: bool = True, index_urls: Optional[List[str]] = None
+        self,
+        prompting: bool = True,
+        index_urls: Optional[List[str]] = None,
+        allow_keyring: bool = False,
     ) -> None:
         self.prompting = prompting
         self.index_urls = index_urls
         self.passwords: Dict[str, AuthInfo] = {}
+        self.allow_keyring = allow_keyring
         # 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
@@ -228,7 +232,6 @@ def _get_new_credentials(
         self,
         original_url: str,
         allow_netrc: bool = True,
-        allow_keyring: bool = False,
     ) -> AuthInfo:
         """Find and return credentials for the specified URL."""
         # Split the credentials and netloc from the url.
@@ -266,7 +269,7 @@ def _get_new_credentials(
                 return netrc_auth
 
         # If we don't have a password and keyring is available, use it.
-        if allow_keyring:
+        if self.allow_keyring:
             # The index url is more specific than the netloc, so try it first
             # fmt: off
             kr_auth = (
@@ -379,7 +382,6 @@ def handle_401(self, resp: Response, **kwargs: Any) -> Response:
         username, password = self._get_new_credentials(
             resp.url,
             allow_netrc=False,
-            allow_keyring=True,
         )
 
         # Prompt the user for a new username and password

src/pip/_internal/network/session.py

@@ -327,6 +327,7 @@ def __init__(
         trusted_hosts: Sequence[str] = (),
         index_urls: Optional[List[str]] = None,
         ssl_context: Optional["SSLContext"] = None,
+        allow_keyring: bool = False,
         **kwargs: Any,
     ) -> None:
         """
@@ -343,7 +344,9 @@ def __init__(
         self.headers["User-Agent"] = user_agent()
 
         # Attach our Authentication handler to the session
-        self.auth = MultiDomainBasicAuth(index_urls=index_urls)
+        self.auth = MultiDomainBasicAuth(
+            index_urls=index_urls, allow_keyring=allow_keyring
+        )
 
         # Create our urllib3.Retry instance which will allow us to customize
         # how we handle retries.

tests/functional/test_install_config.py

@@ -413,7 +413,7 @@ def get_credential(url, username):
     keyring_path.write_text(keyring_content)
 
     with server_running(server):
-        result = script.pip(
+        args = [
             "install",
             "--index-url",
             url,
@@ -422,7 +422,10 @@ def get_credential(url, username):
             "--client-cert",
             cert_path,
             "simple",
-        )
+        ]
+        if auth_needed:
+            args.append("--enable-keyring")
+        result = script.pip(*args)
 
     if auth_needed:
         assert "get_credential was called" in result.stderr

tests/unit/test_network_auth.py

@@ -99,10 +99,10 @@ def test_get_credentials_uses_cached_credentials_only_username() -> None:
 
 
 def test_get_index_url_credentials() -> None:
-    auth = MultiDomainBasicAuth(index_urls=["http://foo:[email protected]/path"])
-    get = functools.partial(
-        auth._get_new_credentials, allow_netrc=False, allow_keyring=False
+    auth = MultiDomainBasicAuth(
+        index_urls=["http://foo:[email protected]/path"], allow_keyring=False
     )
+    get = functools.partial(auth._get_new_credentials, allow_netrc=False)
 
     # Check resolution of indexes
     assert get("http://example.com/path/path2") == ("foo", "bar")
@@ -147,9 +147,11 @@ def test_keyring_get_password(
 ) -> None:
     keyring = KeyringModuleV1()
     monkeypatch.setitem(sys.modules, "keyring", keyring)  # type: ignore[misc]
-    auth = MultiDomainBasicAuth(index_urls=["http://example.com/path2"])
+    auth = MultiDomainBasicAuth(
+        index_urls=["http://example.com/path2"], allow_keyring=True
+    )
 
-    actual = auth._get_new_credentials(url, allow_netrc=False, allow_keyring=True)
+    actual = auth._get_new_credentials(url, allow_netrc=False)
     assert actual == expect
 
 
@@ -193,10 +195,10 @@ def test_keyring_get_password_username_in_index(
 ) -> None:
     keyring = KeyringModuleV1()
     monkeypatch.setitem(sys.modules, "keyring", keyring)  # type: ignore[misc]
-    auth = MultiDomainBasicAuth(index_urls=["http://[email protected]/path2"])
-    get = functools.partial(
-        auth._get_new_credentials, allow_netrc=False, allow_keyring=True
+    auth = MultiDomainBasicAuth(
+        index_urls=["http://[email protected]/path2"], allow_keyring=True
     )
+    get = functools.partial(auth._get_new_credentials, allow_netrc=False)
 
     assert get("http://example.com/path2/path3") == ("user", "user!url")
     assert get("http://example.com/path4/path1") == (None, None)
@@ -302,12 +304,12 @@ def test_keyring_get_credential(
     monkeypatch: pytest.MonkeyPatch, url: str, expect: str
 ) -> None:
     monkeypatch.setitem(sys.modules, "keyring", KeyringModuleV2())  # type: ignore[misc]
-    auth = MultiDomainBasicAuth(index_urls=["http://example.com/path2"])
-
-    assert (
-        auth._get_new_credentials(url, allow_netrc=False, allow_keyring=True) == expect
+    auth = MultiDomainBasicAuth(
+        index_urls=["http://example.com/path2"], allow_keyring=True
     )
 
+    assert auth._get_new_credentials(url, allow_netrc=False) == expect
+
 
 class KeyringModuleBroken:
     """Represents the current supported API of keyring, but broken"""
@@ -324,14 +326,12 @@ def test_broken_keyring_disables_keyring(monkeypatch: pytest.MonkeyPatch) -> Non
     keyring_broken = KeyringModuleBroken()
     monkeypatch.setitem(sys.modules, "keyring", keyring_broken)  # type: ignore[misc]
 
-    auth = MultiDomainBasicAuth(index_urls=["http://example.com/"])
+    auth = MultiDomainBasicAuth(index_urls=["http://example.com/"], allow_keyring=True)
 
     assert keyring_broken._call_count == 0
     for i in range(5):
         url = "http://example.com/path" + str(i)
-        assert auth._get_new_credentials(
-            url, allow_netrc=False, allow_keyring=True
-        ) == (None, None)
+        assert auth._get_new_credentials(url, allow_netrc=False) == (None, None)
         assert keyring_broken._call_count == 1
 
 
@@ -399,9 +399,11 @@ def test_keyring_cli_get_password(
     monkeypatch.setattr(
         pip._internal.network.auth.subprocess, "run", KeyringSubprocessResult()
     )
-    auth = MultiDomainBasicAuth(index_urls=["http://example.com/path2"])
+    auth = MultiDomainBasicAuth(
+        index_urls=["http://example.com/path2"], allow_keyring=True
+    )
 
-    actual = auth._get_new_credentials(url, allow_netrc=False, allow_keyring=True)
+    actual = auth._get_new_credentials(url, allow_netrc=False)
     assert actual == expect