refactor PoetryKeyring

src/poetry/utils/authenticator.py

@@ -90,17 +90,15 @@ def get_http_credentials(
             **(password_manager.get_http_auth(self.name) or {})
         )
 
-        if credential.password is None:
+        if credential.password is not None:
+            return credential
+
+        if password_manager.use_keyring:
             # fallback to url and netloc based keyring entries
-            credential = password_manager.keyring.get_credential(
+            credential = password_manager.get_credential(
                 self.url, self.netloc, username=credential.username
             )
 
-            if credential.password is not None:
-                return HTTPAuthCredential(
-                    username=credential.username, password=credential.password
-                )
-
         return credential
 
 
@@ -305,7 +303,7 @@ def _get_credentials_for_url(
         if credential.password is None:
             parsed_url = urllib.parse.urlsplit(url)
             netloc = parsed_url.netloc
-            credential = self._password_manager.keyring.get_credential(
+            credential = self._password_manager.get_credential(
                 url, netloc, username=credential.username
             )
 

src/poetry/utils/password_manager.py

@@ -1,13 +1,16 @@
 from __future__ import annotations
 
 import dataclasses
+import functools
 import logging
 
 from contextlib import suppress
 from typing import TYPE_CHECKING
 
 
 if TYPE_CHECKING:
+    from keyring.backend import KeyringBackend
+
     from poetry.config.config import Config
 
 logger = logging.getLogger(__name__)
@@ -30,21 +33,10 @@ class HTTPAuthCredential:
 class PoetryKeyring:
     def __init__(self, namespace: str) -> None:
         self._namespace = namespace
-        self._is_available = True
-
-        self._check()
-
-    def is_available(self) -> bool:
-        return self._is_available
 
     def get_credential(
         self, *names: str, username: str | None = None
     ) -> HTTPAuthCredential:
-        default = HTTPAuthCredential(username=username, password=None)
-
-        if not self.is_available():
-            return default
-
         import keyring
 
         from keyring.errors import KeyringError
@@ -64,12 +56,9 @@ def get_credential(
                     username=credential.username, password=credential.password
                 )
 
-        return default
+        return HTTPAuthCredential(username=username, password=None)
 
     def get_password(self, name: str, username: str) -> str | None:
-        if not self.is_available():
-            return None
-
         import keyring
         import keyring.errors
 
@@ -83,9 +72,6 @@ def get_password(self, name: str, username: str) -> str | None:
             )
 
     def set_password(self, name: str, username: str, password: str) -> None:
-        if not self.is_available():
-            return
-
         import keyring
         import keyring.errors
 
@@ -99,9 +85,6 @@ def set_password(self, name: str, username: str, password: str) -> None:
             )
 
     def delete_password(self, name: str, username: str) -> None:
-        if not self.is_available():
-            return
-
         import keyring.errors
 
         name = self.get_entry_name(name)
@@ -116,65 +99,69 @@ def delete_password(self, name: str, username: str) -> None:
     def get_entry_name(self, name: str) -> str:
         return f"{self._namespace}-{name}"
 
-    def _check(self) -> None:
+    @classmethod
+    def is_available(cls) -> bool:
+        logger.debug("Checking if keyring is available")
         try:
             import keyring
+            import keyring.backend
         except ImportError as e:
             logger.debug("An error occurred while importing keyring: %s", e)
-            self._is_available = False
+            return False
 
-            return
+        def backend_name(backend: KeyringBackend) -> str:
+            name: str = backend.name
+            return name.split(" ")[0]
 
-        backend = keyring.get_keyring()
-        name = backend.name.split(" ")[0]
-        if name in ("fail", "null"):
-            logger.debug("No suitable keyring backend found")
-            self._is_available = False
-        elif "plaintext" in backend.name.lower():
-            logger.debug("Only a plaintext keyring backend is available. Not using it.")
-            self._is_available = False
-        elif name == "chainer":
-            try:
-                import keyring.backend
+        def backend_is_valid(backend: KeyringBackend) -> bool:
+            name = backend_name(backend)
+            if name in ("chainer", "fail", "null"):
+                logger.debug(f"Backend {backend.name!r} is not suitable")
+                return False
+            elif "plaintext" in backend.name.lower():
+                logger.debug(f"Not using plaintext keyring backend {backend.name!r}")
+                return False
 
-                backends = keyring.backend.get_all_keyring()
+            return True
 
-                self._is_available = any(
-                    b.name.split(" ")[0] not in ["chainer", "fail", "null"]
-                    and "plaintext" not in b.name.lower()
-                    for b in backends
-                )
-            except ImportError:
-                self._is_available = False
+        backend = keyring.get_keyring()
+        if backend_name(backend) == "chainer":
+            backends = keyring.backend.get_all_keyring()
+            valid_backend = next((b for b in backends if backend_is_valid(b)), None)
+        else:
+            valid_backend = backend if backend_is_valid(backend) else None
 
-        if not self._is_available:
-            logger.debug("No suitable keyring backends were found")
+        if valid_backend is None:
+            logger.debug("No valid keyring backend was found")
+            return False
+        else:
+            logger.debug(f"Using keyring backend {backend.name!r}")
+            return True
 
 
 class PasswordManager:
     def __init__(self, config: Config) -> None:
         self._config = config
-        self._keyring: PoetryKeyring | None = None
 
-    @property
-    def keyring(self) -> PoetryKeyring:
-        if self._keyring is None:
-            self._keyring = PoetryKeyring("poetry-repository")
+    @functools.cached_property
+    def use_keyring(self) -> bool:
+        return PoetryKeyring.is_available()
 
-            if not self._keyring.is_available():
-                logger.debug(
-                    "<warning>Keyring is not available, credentials will be stored and "
-                    "retrieved from configuration files as plaintext.</>"
-                )
+    @functools.cached_property
+    def keyring(self) -> PoetryKeyring:
+        if not self.use_keyring:
+            raise PoetryKeyringError(
+                "Access to keyring was requested, but it is not available"
+            )
 
-        return self._keyring
+        return PoetryKeyring("poetry-repository")
 
     @staticmethod
     def warn_plaintext_credentials_stored() -> None:
         logger.warning("Using a plaintext file to store credentials")
 
     def set_pypi_token(self, name: str, token: str) -> None:
-        if not self.keyring.is_available():
+        if not self.use_keyring:
             self.warn_plaintext_credentials_stored()
             self._config.auth_config_source.add_property(f"pypi-token.{name}", token)
         else:
@@ -194,10 +181,13 @@ def get_pypi_token(self, repo_name: str) -> str | None:
         if token:
             return token
 
-        return self.keyring.get_password(repo_name, "__token__")
+        if self.use_keyring:
+            return self.keyring.get_password(repo_name, "__token__")
+        else:
+            return None
 
     def delete_pypi_token(self, name: str) -> None:
-        if not self.keyring.is_available():
+        if not self.use_keyring:
             return self._config.auth_config_source.remove_property(f"pypi-token.{name}")
 
         self.keyring.delete_password(name, "__token__")
@@ -219,7 +209,7 @@ def get_http_auth(self, name: str) -> dict[str, str | None] | None:
     def set_http_password(self, name: str, username: str, password: str) -> None:
         auth = {"username": username}
 
-        if not self.keyring.is_available():
+        if not self.use_keyring:
             self.warn_plaintext_credentials_stored()
             auth["password"] = password
         else:
@@ -240,3 +230,17 @@ def delete_http_password(self, name: str) -> None:
             self.keyring.delete_password(name, username)
 
         self._config.auth_config_source.remove_property(f"http-basic.{name}")
+
+    def get_credential(
+        self, *names: str, username: str | None = None
+    ) -> HTTPAuthCredential:
+        if self.use_keyring:
+            return self.keyring.get_credential(*names, username=username)
+        else:
+            return HTTPAuthCredential(username=username, password=None)
+
+    def get_password(self, name: str, username: str) -> str | None:
+        if self.use_keyring:
+            return self.keyring.get_password(name=name, username=username)
+        else:
+            return None

tests/utils/test_password_manager.py

@@ -26,7 +26,7 @@ def test_set_http_password(
 ) -> None:
     manager = PasswordManager(config)
 
-    assert manager.keyring.is_available()
+    assert PoetryKeyring.is_available()
     manager.set_http_password("foo", "bar", "baz")
 
     assert dummy_keyring.get_password("poetry-repository-foo", "bar") == "baz"
@@ -43,7 +43,7 @@ def test_get_http_auth(
     config.auth_config_source.add_property("http-basic.foo", {"username": "bar"})
     manager = PasswordManager(config)
 
-    assert manager.keyring.is_available()
+    assert PoetryKeyring.is_available()
     auth = manager.get_http_auth("foo")
     assert auth is not None
 
@@ -58,7 +58,7 @@ def test_delete_http_password(
     config.auth_config_source.add_property("http-basic.foo", {"username": "bar"})
     manager = PasswordManager(config)
 
-    assert manager.keyring.is_available()
+    assert PoetryKeyring.is_available()
     manager.delete_http_password("foo")
 
     assert dummy_keyring.get_password("poetry-repository-foo", "bar") is None
@@ -70,7 +70,7 @@ def test_set_pypi_token(
 ) -> None:
     manager = PasswordManager(config)
 
-    assert manager.keyring.is_available()
+    assert PoetryKeyring.is_available()
     manager.set_pypi_token("foo", "baz")
 
     assert config.get("pypi-token.foo") is None
@@ -84,7 +84,7 @@ def test_get_pypi_token(
     dummy_keyring.set_password("poetry-repository-foo", "__token__", "baz")
     manager = PasswordManager(config)
 
-    assert manager.keyring.is_available()
+    assert PoetryKeyring.is_available()
     assert manager.get_pypi_token("foo") == "baz"
 
 
@@ -94,7 +94,7 @@ def test_delete_pypi_token(
     dummy_keyring.set_password("poetry-repository-foo", "__token__", "baz")
     manager = PasswordManager(config)
 
-    assert manager.keyring.is_available()
+    assert PoetryKeyring.is_available()
     manager.delete_pypi_token("foo")
 
     assert dummy_keyring.get_password("poetry-repository-foo", "__token__") is None
@@ -105,7 +105,7 @@ def test_set_http_password_with_unavailable_backend(
 ) -> None:
     manager = PasswordManager(config)
 
-    assert not manager.keyring.is_available()
+    assert not PoetryKeyring.is_available()
     manager.set_http_password("foo", "bar", "baz")
 
     auth = config.get("http-basic.foo")
@@ -121,7 +121,7 @@ def test_get_http_auth_with_unavailable_backend(
     )
     manager = PasswordManager(config)
 
-    assert not manager.keyring.is_available()
+    assert not PoetryKeyring.is_available()
     auth = manager.get_http_auth("foo")
     assert auth is not None
 
@@ -137,7 +137,7 @@ def test_delete_http_password_with_unavailable_backend(
     )
     manager = PasswordManager(config)
 
-    assert not manager.keyring.is_available()
+    assert not PoetryKeyring.is_available()
     manager.delete_http_password("foo")
 
     assert config.get("http-basic.foo") is None
@@ -148,7 +148,7 @@ def test_set_pypi_token_with_unavailable_backend(
 ) -> None:
     manager = PasswordManager(config)
 
-    assert not manager.keyring.is_available()
+    assert not PoetryKeyring.is_available()
     manager.set_pypi_token("foo", "baz")
 
     assert config.get("pypi-token.foo") == "baz"
@@ -160,7 +160,7 @@ def test_get_pypi_token_with_unavailable_backend(
     config.auth_config_source.add_property("pypi-token.foo", "baz")
     manager = PasswordManager(config)
 
-    assert not manager.keyring.is_available()
+    assert not PoetryKeyring.is_available()
     assert manager.get_pypi_token("foo") == "baz"
 
 
@@ -170,7 +170,7 @@ def test_delete_pypi_token_with_unavailable_backend(
     config.auth_config_source.add_property("pypi-token.foo", "baz")
     manager = PasswordManager(config)
 
-    assert not manager.keyring.is_available()
+    assert not PoetryKeyring.is_available()
     manager.delete_pypi_token("foo")
 
     assert config.get("pypi-token.foo") is None
@@ -179,7 +179,7 @@ def test_delete_pypi_token_with_unavailable_backend(
 def test_keyring_raises_errors_on_keyring_errors(
     mocker: MockerFixture, with_fail_keyring: None
 ) -> None:
-    mocker.patch("poetry.utils.password_manager.PoetryKeyring._check")
+    mocker.patch("poetry.utils.password_manager.PoetryKeyring.is_available")
 
     key_ring = PoetryKeyring("poetry")
     with pytest.raises(PoetryKeyringError):
@@ -281,11 +281,11 @@ def test_get_http_auth_does_not_call_keyring_when_credentials_in_environment_var
     os.environ["POETRY_HTTP_BASIC_FOO_PASSWORD"] = "baz"
 
     manager = PasswordManager(config)
-    manager._keyring = MagicMock()
+    manager.keyring = MagicMock()
 
     auth = manager.get_http_auth("foo")
     assert auth == {"username": "bar", "password": "baz"}
-    manager._keyring.get_password.assert_not_called()
+    manager.keyring.get_password.assert_not_called()
 
 
 def test_get_http_auth_does_not_call_keyring_when_password_in_environment_variables(
@@ -297,11 +297,11 @@ def test_get_http_auth_does_not_call_keyring_when_password_in_environment_variab
     os.environ["POETRY_HTTP_BASIC_FOO_PASSWORD"] = "baz"
 
     manager = PasswordManager(config)
-    manager._keyring = MagicMock()
+    manager.keyring = MagicMock()
 
     auth = manager.get_http_auth("foo")
     assert auth == {"username": "bar", "password": "baz"}
-    manager._keyring.get_password.assert_not_called()
+    manager.keyring.get_password.assert_not_called()
 
 
 def test_get_pypi_token_with_env_var_positive(