Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add cli flag --force-keyring #11029

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ repos:
additional_dependencies: [
'keyring==23.0.1',
'nox==2021.6.12',
'pytest==7.1.1',
'pytest',
'types-docutils==0.18.3',
'types-setuptools==57.4.14',
'types-freezegun==1.1.9',
Expand Down
19 changes: 19 additions & 0 deletions docs/html/topics/authentication.md
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,25 @@ $ echo "your-password" | keyring set pypi.company.com your-username
$ pip install your-package --index-url https://pypi.company.com/
```

Pip is conservative and does not query keyring at all when `--no-input` is used
because the keyring might require user interaction such as prompting the user
on the console. You can force keyring usage by passing `--force-keyring` or one
of the following:

```bash
# possibly with --user, --global or --site
$ pip config set global.force-keyring true
# or
$ export PIP_FORCE_KEYRING=1
```

```{warning}
Be careful when doing this since it could cause tools such as Pipx and Pipenv
to appear to hang. They show their own progress indicator while hiding output
from the subprocess in which they run Pip. You won't know whether the keyring
backend is waiting the user input or not in such situations.
uranusjr marked this conversation as resolved.
Show resolved Hide resolved
```

Note that `keyring` (the Python package) needs to be installed separately from
pip. This can create a bootstrapping issue if you need the credentials stored in
the keyring to download and install keyring.
Expand Down
3 changes: 3 additions & 0 deletions news/11020.feature.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Add ``--force-keyring`` flag which allows ``keyring`` lookups in combination
with ``--no-input``. See the Authentication page in the documentation for
more info.
14 changes: 14 additions & 0 deletions src/pip/_internal/cli/cmdoptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,19 @@ class PipOption(Option):
help="Disable prompting for input.",
)

force_keyring: Callable[..., Option] = partial(
Option,
"--force-keyring",
dest="force_keyring",
action="store_true",
default=False,
help=(
"Always query the keyring, regardless of pip's --no-input option. Note"
" that this may cause problems if the keyring expects to be able to"
" prompt the user interactively and no interactive user is available."
),
)

proxy: Callable[..., Option] = partial(
Option,
"--proxy",
Expand Down Expand Up @@ -1019,6 +1032,7 @@ def check_list_path_option(options: Values) -> None:
quiet,
log,
no_input,
force_keyring,
proxy,
retries,
timeout,
Expand Down
4 changes: 4 additions & 0 deletions src/pip/_internal/cli/req_command.py
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,10 @@ def _build_session(

# Determine if we can prompt the user for authentication or not
session.auth.prompting = not options.no_input
# We won't use keyring when --no-input is passed unless
# --force-keyring is passed as well because it might require
# user interaction
session.auth.use_keyring = session.auth.prompting or options.force_keyring

return session

Expand Down
36 changes: 22 additions & 14 deletions src/pip/_internal/network/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -128,15 +128,15 @@ def _get_password(self, service_name: str, username: str) -> Optional[str]:
)
if res.returncode:
return None
return res.stdout.decode("utf-8").strip("\n")
return res.stdout.decode("utf-8").strip(os.linesep)

def _set_password(self, service_name: str, username: str, password: str) -> None:
"""Mirror the implementation of keyring.set_password using cli"""
if self.keyring is None:
return None

cmd = [self.keyring, "set", service_name, username]
input_ = password.encode("utf-8") + b"\n"
input_ = (password + os.linesep).encode("utf-8")
env = os.environ.copy()
env["PYTHONIOENCODING"] = "utf-8"
res = subprocess.run(cmd, input=input_, env=env)
Expand Down Expand Up @@ -190,10 +190,14 @@ def get_keyring_auth(url: Optional[str], username: Optional[str]) -> Optional[Au

class MultiDomainBasicAuth(AuthBase):
def __init__(
self, prompting: bool = True, index_urls: Optional[List[str]] = None
self,
prompting: bool = True,
index_urls: Optional[List[str]] = None,
use_keyring: bool = True,
) -> None:
self.prompting = prompting
self.index_urls = index_urls
self.use_keyring = use_keyring
self.passwords: Dict[str, AuthInfo] = {}
# When the user is prompted to enter credentials and keyring is
# available, we will offer to save them. If the user accepts,
Expand Down Expand Up @@ -227,7 +231,8 @@ def _get_index_url(self, url: str) -> Optional[str]:
def _get_new_credentials(
self,
original_url: str,
allow_netrc: bool = True,
*,
allow_netrc: bool = False,
allow_keyring: bool = False,
) -> AuthInfo:
"""Find and return credentials for the specified URL."""
Expand Down Expand Up @@ -348,7 +353,7 @@ def __call__(self, req: Request) -> Request:
def _prompt_for_password(
self, netloc: str
) -> Tuple[Optional[str], Optional[str], bool]:
username = ask_input(f"User for {netloc}: ")
username = ask_input(f"User for {netloc}: ") if self.prompting else None
if not username:
return None, None, False
auth = get_keyring_auth(netloc, username)
Expand All @@ -359,7 +364,7 @@ def _prompt_for_password(

# Factored out to allow for easy patching in tests
def _should_save_password_to_keyring(self) -> bool:
if get_keyring_provider() is None:
if not self.prompting or get_keyring_provider() is None:
return False
return ask("Save credentials to keyring [y/N]: ", ["y", "n"]) == "y"

Expand All @@ -369,19 +374,22 @@ def handle_401(self, resp: Response, **kwargs: Any) -> Response:
if resp.status_code != 401:
return resp

username, password = None, None

# Query the keyring for credentials:
if self.use_keyring:
username, password = self._get_new_credentials(
resp.url,
allow_netrc=False,
allow_keyring=True,
)

# We are not able to prompt the user so simply return the response
if not self.prompting:
if not self.prompting and not username and not password:
return resp

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
save = False
if not username and not password:
Expand Down
42 changes: 41 additions & 1 deletion tests/functional/test_install_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import sys
import tempfile
import textwrap
import typing

import pytest

Expand Down Expand Up @@ -361,16 +362,54 @@ def test_do_not_prompt_for_authentication(
assert "ERROR: HTTP error 401" in result.stderr


@pytest.fixture(params=(True, False), ids=("auth_needed", "auth_not_needed"))
def auth_needed(request: pytest.FixtureRequest) -> bool:
return request.param


@pytest.fixture(
params=(
(False, False, False),
(False, True, False),
(True, True, False),
(True, False, True),
),
ids=(
"default-default",
"default-force_keyring",
"no_input-force_keyring",
"no_input-default",
),
)
def flags(request: pytest.FixtureRequest, auth_needed: bool) -> typing.List[str]:
(
no_input,
force_keyring,
xfail,
) = request.param

flags = []
if no_input:
flags.append("--no-input")
if force_keyring:
flags.append(
"--force-keyring",
)
if auth_needed and xfail:
request.applymarker(pytest.mark.xfail())
return flags


@pytest.mark.skipif(
sys.platform == "linux" and sys.version_info < (3, 8),
reason="Custom SSL certification not running well in CI",
)
@pytest.mark.parametrize("auth_needed", (True, False))
def test_prompt_for_keyring_if_needed(
script: PipTestEnvironment,
data: TestData,
cert_factory: CertFactory,
auth_needed: bool,
flags: typing.List[str],
) -> None:
"""Test behaviour while installing from a index url
requiring authentication and keyring is possible.
Expand Down Expand Up @@ -421,6 +460,7 @@ def get_credential(url, username):
cert_path,
"--client-cert",
cert_path,
*flags,
"simple",
)

Expand Down
5 changes: 3 additions & 2 deletions tests/unit/test_network_auth.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import functools
import os
import sys
from typing import Any, Dict, Iterable, List, Optional, Tuple

Expand Down Expand Up @@ -360,7 +361,7 @@ def __call__(
self.returncode = 1
else:
# Passwords are returned encoded with a newline appended
self.stdout = password.encode("utf-8") + b"\n"
self.stdout = (password + os.linesep).encode("utf-8")

if cmd[1] == "set":
assert stdin is None
Expand All @@ -369,7 +370,7 @@ def __call__(
assert input is not None

# Input from stdin is encoded
self.set_password(cmd[2], cmd[3], input.decode("utf-8").strip("\n"))
self.set_password(cmd[2], cmd[3], input.decode("utf-8").strip(os.linesep))

return self

Expand Down