Skip to content

Commit

Permalink
Merge pull request #11774 from frostming/fix/keyring
Browse files Browse the repository at this point in the history
fix: correct the way to decide if keyring is available
  • Loading branch information
pradyunsg authored Feb 3, 2023
2 parents 56e5fa3 + 2d0a5c9 commit 6a416d2
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 4 deletions.
1 change: 1 addition & 0 deletions news/11774.bugfix.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Correct the way to decide if keyring is available.
14 changes: 10 additions & 4 deletions src/pip/_internal/network/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ class Credentials(NamedTuple):
class KeyRingBaseProvider(ABC):
"""Keyring base provider interface"""

has_keyring: bool

@abstractmethod
def get_auth_info(self, url: str, username: Optional[str]) -> Optional[AuthInfo]:
...
Expand All @@ -51,6 +53,8 @@ def save_auth_info(self, url: str, username: str, password: str) -> None:
class KeyRingNullProvider(KeyRingBaseProvider):
"""Keyring null provider"""

has_keyring = False

def get_auth_info(self, url: str, username: Optional[str]) -> Optional[AuthInfo]:
return None

Expand All @@ -61,6 +65,8 @@ def save_auth_info(self, url: str, username: str, password: str) -> None:
class KeyRingPythonProvider(KeyRingBaseProvider):
"""Keyring interface which uses locally imported `keyring`"""

has_keyring = True

def __init__(self) -> None:
import keyring

Expand Down Expand Up @@ -97,6 +103,8 @@ class KeyRingCliProvider(KeyRingBaseProvider):
PATH.
"""

has_keyring = True

def __init__(self, cmd: str) -> None:
self.keyring = cmd

Expand Down Expand Up @@ -359,7 +367,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 get_keyring_provider().has_keyring:
return False
return ask("Save credentials to keyring [y/N]: ", ["y", "n"]) == "y"

Expand Down Expand Up @@ -432,9 +440,7 @@ def warn_on_401(self, resp: Response, **kwargs: Any) -> None:
def save_credentials(self, resp: Response, **kwargs: Any) -> None:
"""Response callback to save credentials on success."""
keyring = get_keyring_provider()
assert not isinstance(
keyring, KeyRingNullProvider
), "should never reach here without keyring"
assert keyring.has_keyring, "should never reach here without keyring"

creds = self._credentials_to_save
self._credentials_to_save = None
Expand Down

0 comments on commit 6a416d2

Please sign in to comment.