Only query the keyring for URLs that actually trigger error 401

Fixes #8090

news/8090.bugfix

@@ -1,3 +1,6 @@
 Only attempt to use the keyring once and if it fails, don't try again.
 This prevents spamming users with several keyring unlock prompts when they
 cannot unlock or don't want to do so.
+Only query the keyring for URLs that actually trigger error 401.
+This prevents an unnecessary keyring unlock prompt on every pip install
+invocation (even with default index URL which is not password protected).

src/pip/_internal/network/auth.py

@@ -112,7 +112,7 @@ def _get_index_url(self, url):
         return None
 
     def _get_new_credentials(self, original_url, allow_netrc=True,
-                             allow_keyring=True):
+                             allow_keyring=False):
         # type: (str, bool, bool) -> AuthInfo
         """Find and return credentials for the specified URL."""
         # Split the credentials and netloc from the url.
@@ -252,8 +252,15 @@ def handle_401(self, resp, **kwargs):
 
         parsed = urllib_parse.urlparse(resp.url)
 
+        # Query the keyring for credentials:
+        username, password = self._get_new_credentials(resp.url,
+                                                       allow_netrc=False,
+                                                       allow_keyring=True)
+
         # Prompt the user for a new username and password
-        username, password, save = self._prompt_for_password(parsed.netloc)
+        save = False
+        if not username or not password:
+            username, password, save = self._prompt_for_password(parsed.netloc)
 
         # Store the new username and password to use for future requests
         self._credentials_to_save = None

tests/functional/test_install_config.py

@@ -274,3 +274,52 @@ def test_do_not_prompt_for_authentication(script, data, cert_factory):
                             '--no-input', 'simple', expect_error=True)
 
     assert "ERROR: HTTP error 401" in result.stderr
+
+
+@pytest.mark.parametrize("auth_needed", (True, False))
+def test_prompt_for_keyring_if_needed(script, data, cert_factory, auth_needed):
+    """Test behaviour while installing from a index url
+    requiring authentication and keyring is possible.
+    """
+    cert_path = cert_factory()
+    ctx = ssl.SSLContext(ssl.PROTOCOL_SSLv23)
+    ctx.load_cert_chain(cert_path, cert_path)
+    ctx.load_verify_locations(cafile=cert_path)
+    ctx.verify_mode = ssl.CERT_REQUIRED
+
+    response = authorization_response if auth_needed else file_response
+
+    server = make_mock_server(ssl_context=ctx)
+    server.mock.side_effect = [
+        package_page({
+            "simple-3.0.tar.gz": "/files/simple-3.0.tar.gz",
+        }),
+        response(str(data.packages / "simple-3.0.tar.gz")),
+        response(str(data.packages / "simple-3.0.tar.gz")),
+    ]
+
+    url = "https://{}:{}/simple".format(server.host, server.port)
+
+    keyring_content = textwrap.dedent("""\
+        import os
+        import sys
+        from collections import namedtuple
+
+        Cred = namedtuple("Cred", ["username", "password"])
+
+        def get_credential(url, username):
+            sys.stderr.write("get_credential was called" + os.linesep)
+            return Cred("USERNAME", "PASSWORD")
+    """)
+    keyring_path = script.site_packages_path / 'keyring.py'
+    keyring_path.write_text(keyring_content)
+
+    with server_running(server):
+        result = script.pip('install', "--index-url", url,
+                            "--cert", cert_path, "--client-cert", cert_path,
+                            'simple')
+
+    if auth_needed:
+        assert "get_credential was called" in result.stderr
+    else:
+        assert "get_credential was called" not in result.stderr

tests/lib/server.py

@@ -2,6 +2,7 @@
 import signal
 import ssl
 import threading
+from base64 import b64encode
 from contextlib import contextmanager
 from textwrap import dedent
 
@@ -218,14 +219,26 @@ def responder(environ, start_response):
 
 
 def authorization_response(path):
+    # type: (str) -> Responder
+    correct_auth = "Basic " + b64encode(b"USERNAME:PASSWORD").decode("ascii")
+
     def responder(environ, start_response):
         # type: (Environ, StartResponse) -> Body
 
-        start_response(
-            "401 Unauthorized", [
-                ("WWW-Authenticate", "Basic"),
-            ],
-        )
+        if environ.get('HTTP_AUTHORIZATION') == correct_auth:
+            size = os.stat(path).st_size
+            start_response(
+                "200 OK", [
+                    ("Content-Type", "application/octet-stream"),
+                    ("Content-Length", str(size)),
+                ],
+            )
+        else:
+            start_response(
+                "401 Unauthorized", [
+                    ("WWW-Authenticate", "Basic"),
+                ],
+            )
 
         with open(path, 'rb') as f:
             return [f.read()]