diff --git a/news/8718.bugfix.rst b/news/8718.bugfix.rst new file mode 100644 index 00000000000..aec325dcfac --- /dev/null +++ b/news/8718.bugfix.rst @@ -0,0 +1 @@ +Make keyring support opt-in. diff --git a/src/pip/_internal/cli/cmdoptions.py b/src/pip/_internal/cli/cmdoptions.py index 663143950ac..86713583ca5 100644 --- a/src/pip/_internal/cli/cmdoptions.py +++ b/src/pip/_internal/cli/cmdoptions.py @@ -766,6 +766,15 @@ def _handle_no_use_pep517(option, opt, value, parser): help=SUPPRESS_HELP ) # type: Any +use_keyring = partial( + Option, + '--use-keyring', + dest='use_keyring', + action='store_true', + default=False, + help="Attempt to get credentials from the user's keyring." +) # type: Any + install_options = partial( Option, '--install-option', diff --git a/src/pip/_internal/cli/req_command.py b/src/pip/_internal/cli/req_command.py index 468b3cceab5..372e853ddcf 100644 --- a/src/pip/_internal/cli/req_command.py +++ b/src/pip/_internal/cli/req_command.py @@ -90,6 +90,7 @@ def _build_session(self, options, retries=None, timeout=None): ), retries=retries if retries is not None else options.retries, trusted_hosts=options.trusted_hosts, + use_keyring=options.use_keyring, index_urls=self._get_index_urls(options), ) diff --git a/src/pip/_internal/network/auth.py b/src/pip/_internal/network/auth.py index 174eb834875..64a6b960e6e 100644 --- a/src/pip/_internal/network/auth.py +++ b/src/pip/_internal/network/auth.py @@ -27,59 +27,71 @@ from pip._internal.vcs.versioncontrol import AuthInfo Credentials = Tuple[str, str, str] + Keyring = Any # TODO: Use a more-specific type for the keyring module + logger = logging.getLogger(__name__) -try: - import keyring -except ImportError: - keyring = None -except Exception as exc: - logger.warning( - "Keyring is skipped due to an exception: %s", str(exc), - ) - keyring = None - - -def get_keyring_auth(url, username): - # type: (str, str) -> Optional[AuthInfo] - """Return the tuple auth for a given url from keyring.""" - global keyring - if not url or not keyring: - return None - try: - try: - get_credential = keyring.get_credential - except AttributeError: - pass - else: - logger.debug("Getting credentials from keyring for %s", url) - cred = get_credential(url, username) - if cred is not None: - return cred.username, cred.password - return None +class MultiDomainBasicAuth(AuthBase): + @classmethod + def get_keyring(cls): + # type: () -> Optional[Keyring] + # Cache so the import is attempted at most once + if hasattr(cls, '_keyring'): + return cls._keyring - if username: - logger.debug("Getting password from keyring for %s", url) - password = keyring.get_password(url, username) - if password: - return username, password + try: + import keyring + except ImportError: + keyring = None + except Exception as exc: + logger.warning( + "Keyring is skipped due to an exception: %s", str(exc), + ) + keyring = None - except Exception as exc: - logger.warning( - "Keyring is skipped due to an exception: %s", str(exc), - ) - keyring = None - return None + cls._keyring = keyring + return keyring + @classmethod + def get_keyring_auth(cls, url, username): + # type: (str, str) -> Optional[AuthInfo] + """Return the tuple auth for a given url from keyring.""" + keyring = cls.get_keyring() + if not url or not keyring: + return None -class MultiDomainBasicAuth(AuthBase): + try: + try: + get_credential = keyring.get_credential + except AttributeError: + pass + else: + logger.debug("Getting credentials from keyring for %s", url) + cred = get_credential(url, username) + if cred is not None: + return cred.username, cred.password + return None + + if username: + logger.debug("Getting password from keyring for %s", url) + password = keyring.get_password(url, username) + if password: + return username, password + + except Exception as exc: + logger.warning( + "Keyring is skipped due to an exception: %s", str(exc), + ) + cls._keyring = None + return None - def __init__(self, prompting=True, index_urls=None): - # type: (bool, Optional[List[str]]) -> None + def __init__(self, use_keyring=True, prompting=True, index_urls=None): + # type: (bool, bool, Optional[List[str]]) -> None self.prompting = prompting self.index_urls = index_urls + self.use_keyring = use_keyring self.passwords = {} # type: 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, @@ -150,11 +162,11 @@ def _get_new_credentials(self, original_url, allow_netrc=True, return netrc_auth # If we don't have a password and keyring is available, use it. - if allow_keyring: + if allow_keyring and self.use_keyring: # The index url is more specific than the netloc, so try it first kr_auth = ( - get_keyring_auth(index_url, username) or - get_keyring_auth(netloc, username) + self.get_keyring_auth(index_url, username) or + self.get_keyring_auth(netloc, username) ) if kr_auth: logger.debug("Found credentials in keyring for %s", netloc) @@ -226,7 +238,7 @@ def _prompt_for_password(self, netloc): username = ask_input(f"User for {netloc}: ") if not username: return None, None, False - auth = get_keyring_auth(netloc, username) + auth = self.get_keyring_auth(netloc, username) if auth and auth[0] is not None and auth[1] is not None: return auth[0], auth[1], False password = ask_password("Password: ") @@ -235,7 +247,7 @@ def _prompt_for_password(self, netloc): # Factored out to allow for easy patching in tests def _should_save_password_to_keyring(self): # type: () -> bool - if not keyring: + if not self.get_keyring(): return False return ask("Save credentials to keyring [y/N]: ", ["y", "n"]) == "y" @@ -296,6 +308,7 @@ def warn_on_401(self, resp, **kwargs): def save_credentials(self, resp, **kwargs): # type: (Response, **Any) -> None """Response callback to save credentials on success.""" + keyring = self.get_keyring() assert keyring is not None, "should never reach here without keyring" if not keyring: return diff --git a/src/pip/_internal/network/session.py b/src/pip/_internal/network/session.py index 5021b8eefaa..3ba074fbac8 100644 --- a/src/pip/_internal/network/session.py +++ b/src/pip/_internal/network/session.py @@ -234,6 +234,7 @@ def __init__(self, *args, **kwargs): cache = kwargs.pop("cache", None) trusted_hosts = kwargs.pop("trusted_hosts", []) # type: List[str] index_urls = kwargs.pop("index_urls", None) + use_keyring = kwargs.pop("use_keyring", False) super().__init__(*args, **kwargs) @@ -245,7 +246,10 @@ def __init__(self, *args, **kwargs): self.headers["User-Agent"] = user_agent() # Attach our Authentication handler to the session - self.auth = MultiDomainBasicAuth(index_urls=index_urls) + self.auth = MultiDomainBasicAuth( + use_keyring=use_keyring, + index_urls=index_urls, + ) # Create our urllib3.Retry instance which will allow us to customize # how we handle retries. diff --git a/tests/unit/test_network_auth.py b/tests/unit/test_network_auth.py index 44c739d864f..0196389142a 100644 --- a/tests/unit/test_network_auth.py +++ b/tests/unit/test_network_auth.py @@ -90,6 +90,10 @@ def set_password(self, system, username, password): self.saved_passwords.append((system, username, password)) +class MDBA_KeyringV1(MultiDomainBasicAuth): + _keyring = KeyringModuleV1() + + @pytest.mark.parametrize('url, expect', ( ("http://example.com/path1", (None, None)), # path1 URLs will be resolved by netloc @@ -99,10 +103,8 @@ def set_password(self, system, username, password): ("http://example.com/path2/path3", (None, None)), ("http://foo@example.com/path2/path3", ("foo", "foo!url")), )) -def test_keyring_get_password(monkeypatch, url, expect): - keyring = KeyringModuleV1() - monkeypatch.setattr('pip._internal.network.auth.keyring', keyring) - auth = MultiDomainBasicAuth(index_urls=["http://example.com/path2"]) +def test_keyring_get_password(url, expect): + auth = MDBA_KeyringV1(index_urls=["http://example.com/path2"]) actual = auth._get_new_credentials(url, allow_netrc=False, allow_keyring=True) @@ -110,9 +112,7 @@ def test_keyring_get_password(monkeypatch, url, expect): def test_keyring_get_password_after_prompt(monkeypatch): - keyring = KeyringModuleV1() - monkeypatch.setattr('pip._internal.network.auth.keyring', keyring) - auth = MultiDomainBasicAuth() + auth = MDBA_KeyringV1() def ask_input(prompt): assert prompt == "User for example.com: " @@ -124,9 +124,7 @@ def ask_input(prompt): def test_keyring_get_password_after_prompt_when_none(monkeypatch): - keyring = KeyringModuleV1() - monkeypatch.setattr('pip._internal.network.auth.keyring', keyring) - auth = MultiDomainBasicAuth() + auth = MDBA_KeyringV1() def ask_input(prompt): assert prompt == "User for unknown.com: " @@ -143,10 +141,8 @@ def ask_password(prompt): assert actual == ("user", "fake_password", True) -def test_keyring_get_password_username_in_index(monkeypatch): - keyring = KeyringModuleV1() - monkeypatch.setattr('pip._internal.network.auth.keyring', keyring) - auth = MultiDomainBasicAuth(index_urls=["http://user@example.com/path2"]) +def test_keyring_get_password_username_in_index(): + auth = MDBA_KeyringV1(index_urls=["http://user@example.com/path2"]) get = functools.partial( auth._get_new_credentials, allow_netrc=False, @@ -164,9 +160,7 @@ def test_keyring_get_password_username_in_index(monkeypatch): )) def test_keyring_set_password(monkeypatch, response_status, creds, expect_save): - keyring = KeyringModuleV1() - monkeypatch.setattr('pip._internal.network.auth.keyring', keyring) - auth = MultiDomainBasicAuth(prompting=True) + auth = MDBA_KeyringV1(prompting=True) monkeypatch.setattr(auth, '_get_url_and_credentials', lambda u: (u, None, None)) monkeypatch.setattr(auth, '_prompt_for_password', lambda *a: creds) @@ -203,6 +197,7 @@ def _send(sent_req, **kwargs): auth.handle_401(resp) + keyring = auth.get_keyring() if expect_save: assert keyring.saved_passwords == [("example.com", creds[0], creds[1])] else: @@ -228,16 +223,17 @@ def get_credential(self, system, username): return None +class MDBA_KeyringV2(MultiDomainBasicAuth): + _keyring = KeyringModuleV2() + + @pytest.mark.parametrize('url, expect', ( ("http://example.com/path1", ("username", "netloc")), ("http://example.com/path2/path3", ("username", "url")), ("http://user2@example.com/path2/path3", ("username", "url")), )) -def test_keyring_get_credential(monkeypatch, url, expect): - monkeypatch.setattr( - pip._internal.network.auth, 'keyring', KeyringModuleV2() - ) - auth = MultiDomainBasicAuth(index_urls=["http://example.com/path2"]) +def test_keyring_get_credential(url, expect): + auth = MDBA_KeyringV1(index_urls=["http://example.com/path2"]) assert auth._get_new_credentials( url, allow_netrc=False, allow_keyring=True @@ -255,11 +251,13 @@ def get_credential(self, system, username): raise Exception("This keyring is broken!") -def test_broken_keyring_disables_keyring(monkeypatch): - keyring_broken = KeyringModuleBroken() - monkeypatch.setattr(pip._internal.network.auth, 'keyring', keyring_broken) +class MDBA_KeyringBroken(MultiDomainBasicAuth): + _keyring = KeyringModuleBroken() + +def test_broken_keyring_disables_keyring(monkeypatch): auth = MultiDomainBasicAuth(index_urls=["http://example.com/"]) + keyring_broken = auth.get_keyring() assert keyring_broken._call_count == 0 for i in range(5):