Skip to content

Commit

Permalink
Only query the keyring for URLs that actually trigger error 401
Browse files Browse the repository at this point in the history
Fixes pypa#8090
  • Loading branch information
hroncok committed Dec 29, 2020
1 parent 7369ac2 commit 1ca4cbd
Show file tree
Hide file tree
Showing 4 changed files with 79 additions and 7 deletions.
3 changes: 3 additions & 0 deletions news/8090.bugfix
Original file line number Diff line number Diff line change
@@ -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).
11 changes: 9 additions & 2 deletions src/pip/_internal/network/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down
49 changes: 49 additions & 0 deletions tests/functional/test_install_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
23 changes: 18 additions & 5 deletions tests/lib/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
import signal
import ssl
import threading
from base64 import b64encode
from contextlib import contextmanager
from textwrap import dedent

Expand Down Expand Up @@ -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()]
Expand Down

0 comments on commit 1ca4cbd

Please sign in to comment.