From 886275de2d7f92823c812fd0b89a402560c357df Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Miro=20Hron=C4=8Dok?= <miro@hroncok.cz>
Date: Wed, 22 Jul 2020 18:30:59 +0200
Subject: [PATCH] Only query the keyring for URLs that actually trigger error
 401

Fixes https://github.com/pypa/pip/issues/8090
---
 news/8090.bugfix.rst                    |  3 ++
 src/pip/_internal/network/auth.py       | 11 +++++-
 tests/functional/test_install_config.py | 49 +++++++++++++++++++++++++
 tests/lib/server.py                     | 23 +++++++++---
 4 files changed, 79 insertions(+), 7 deletions(-)
 create mode 100644 news/8090.bugfix.rst

diff --git a/news/8090.bugfix.rst b/news/8090.bugfix.rst
new file mode 100644
index 00000000000..ff8b80f3c8e
--- /dev/null
+++ b/news/8090.bugfix.rst
@@ -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).
diff --git a/src/pip/_internal/network/auth.py b/src/pip/_internal/network/auth.py
index 174eb834875..99fa2bd6762 100644
--- a/src/pip/_internal/network/auth.py
+++ b/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 and 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
diff --git a/tests/functional/test_install_config.py b/tests/functional/test_install_config.py
index 41be6fbbbb6..27e4f0b0cc4 100644
--- a/tests/functional/test_install_config.py
+++ b/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
diff --git a/tests/lib/server.py b/tests/lib/server.py
index cd3c522bfec..dbeff934d6f 100644
--- a/tests/lib/server.py
+++ b/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()]