Only query the keyring for URLs that actually trigger error 401

Fixes pypa#8090

news/8090.bugfix

@@ -0,0 +1,3 @@
+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
 
@@ -219,14 +220,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()]

tests/unit/test_network_auth.py

@@ -267,4 +267,4 @@ def test_broken_keyring_disables_keyring(monkeypatch):
         assert auth._get_new_credentials(
             url, allow_netrc=False, allow_keyring=True
         ) == (None, None)
-        assert keyring_broken._call_count == 1
+    assert keyring_broken._call_count == 1