Make keyring usage opt-in

news/8718.bugfix.rst

@@ -0,0 +1 @@
+Make keyring support opt-in.

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',

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

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

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.

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,20 +103,16 @@ def set_password(self, system, username, password):
     ("http://example.com/path2/path3", (None, None)),
     ("http://[email protected]/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)
     assert actual == 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://[email protected]/path2"])
+def test_keyring_get_password_username_in_index():
+    auth = MDBA_KeyringV1(index_urls=["http://[email protected]/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://[email protected]/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):