Don't overuse the keyring

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).

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

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()]