fix: remove exception when keyring is locked (#8227)

Co-authored-by: Matthieu Bizien <oao2005@gmail.com>
Co-authored-by: real-yfprojects <real-yfprojects@users.noreply.github.com>
Co-authored-by: Randy Döring <30527984+radoering@users.noreply.github.com>

src/poetry/utils/password_manager.py

@@ -47,8 +47,18 @@ def get_credential(
 
         import keyring
 
+        from keyring.errors import KeyringError
+        from keyring.errors import KeyringLocked
+
         for name in names:
-            credential = keyring.get_credential(name, username)
+            credential = None
+            try:
+                credential = keyring.get_credential(name, username)
+            except KeyringLocked:
+                logger.debug("Keyring %s is locked", name)
+            except (KeyringError, RuntimeError):
+                logger.debug("Accessing keyring %s failed", name, exc_info=True)
+
             if credential:
                 return HTTPAuthCredential(
                     username=credential.username, password=credential.password

tests/conftest.py

@@ -12,9 +12,13 @@
 from typing import Any
 
 import httpretty
+import keyring
 import pytest
 
+from keyring import backends
 from keyring.backend import KeyringBackend
+from keyring.errors import KeyringError
+from keyring.errors import KeyringLocked
 
 from poetry.config.config import Config as BaseConfig
 from poetry.config.dict_config_source import DictConfigSource
@@ -113,31 +117,62 @@ def delete_password(self, service: str, username: str | None) -> None:
             del self._passwords[service][username]
 
 
+class LockedBackend(KeyringBackend):
+    @classmethod
+    def priority(cls) -> int:
+        return 42
+
+    def set_password(self, service: str, username: str | None, password: Any) -> None:
+        raise KeyringLocked()
+
+    def get_password(self, service: str, username: str | None) -> Any:
+        raise KeyringLocked()
+
+    def get_credential(self, service: str, username: str | None) -> Any:
+        raise KeyringLocked()
+
+    def delete_password(self, service: str, username: str | None) -> None:
+        raise KeyringLocked()
+
+
+class ErroneousBackend(backends.fail.Keyring):
+    @classmethod
+    def priority(cls) -> int:
+        return 42
+
+    def get_credential(self, service: str, username: str | None) -> Any:
+        raise KeyringError()
+
+
 @pytest.fixture()
 def dummy_keyring() -> DummyBackend:
     return DummyBackend()
 
 
 @pytest.fixture()
 def with_simple_keyring(dummy_keyring: DummyBackend) -> None:
-    import keyring
-
     keyring.set_keyring(dummy_keyring)
 
 
 @pytest.fixture()
 def with_fail_keyring() -> None:
-    import keyring
-
     from keyring.backends.fail import Keyring
 
     keyring.set_keyring(Keyring())  # type: ignore[no-untyped-call]
 
 
 @pytest.fixture()
-def with_null_keyring() -> None:
-    import keyring
+def with_locked_keyring() -> None:
+    keyring.set_keyring(LockedBackend())  # type: ignore[no-untyped-call]
+
 
+@pytest.fixture()
+def with_erroneous_keyring() -> None:
+    keyring.set_keyring(ErroneousBackend())  # type: ignore[no-untyped-call]
+
+
+@pytest.fixture()
+def with_null_keyring() -> None:
     from keyring.backends.null import Keyring
 
     keyring.set_keyring(Keyring())  # type: ignore[no-untyped-call]
@@ -151,8 +186,6 @@ def with_chained_fail_keyring(mocker: MockerFixture) -> None:
         "keyring.backend.get_all_keyring",
         lambda: [Keyring()],  # type: ignore[no-untyped-call]
     )
-    import keyring
-
     from keyring.backends.chainer import ChainerBackend
 
     keyring.set_keyring(ChainerBackend())  # type: ignore[no-untyped-call]
@@ -166,8 +199,6 @@ def with_chained_null_keyring(mocker: MockerFixture) -> None:
         "keyring.backend.get_all_keyring",
         lambda: [Keyring()],  # type: ignore[no-untyped-call]
     )
-    import keyring
-
     from keyring.backends.chainer import ChainerBackend
 
     keyring.set_keyring(ChainerBackend())  # type: ignore[no-untyped-call]
@@ -206,8 +237,6 @@ def config(
     auth_config_source: DictConfigSource,
     mocker: MockerFixture,
 ) -> Config:
-    import keyring
-
     from keyring.backends.fail import Keyring
 
     keyring.set_keyring(Keyring())  # type: ignore[no-untyped-call]

tests/utils/test_authenticator.py

@@ -1,6 +1,7 @@
 from __future__ import annotations
 
 import base64
+import logging
 import re
 import uuid
 
@@ -20,6 +21,7 @@
 
 
 if TYPE_CHECKING:
+    from _pytest.logging import LogCaptureFixture
     from _pytest.monkeypatch import MonkeyPatch
     from pytest_mock import MockerFixture
 
@@ -65,8 +67,8 @@ def test_authenticator_uses_url_provided_credentials(
     authenticator.request("get", "https://foo001:bar002@foo.bar/files/foo-0.1.0.tar.gz")
 
     request = http.last_request()
-
-    assert request.headers["Authorization"] == "Basic Zm9vMDAxOmJhcjAwMg=="
+    basic_auth = base64.b64encode(b"foo001:bar002").decode()
+    assert request.headers["Authorization"] == f"Basic {basic_auth}"
 
 
 def test_authenticator_uses_credentials_from_config_if_not_provided(
@@ -76,8 +78,8 @@ def test_authenticator_uses_credentials_from_config_if_not_provided(
     authenticator.request("get", "https://foo.bar/files/foo-0.1.0.tar.gz")
 
     request = http.last_request()
-
-    assert request.headers["Authorization"] == "Basic YmFyOmJheg=="
+    basic_auth = base64.b64encode(b"bar:baz").decode()
+    assert request.headers["Authorization"] == f"Basic {basic_auth}"
 
 
 def test_authenticator_uses_username_only_credentials(
@@ -90,8 +92,38 @@ def test_authenticator_uses_username_only_credentials(
     authenticator.request("get", "https://foo001@foo.bar/files/foo-0.1.0.tar.gz")
 
     request = http.last_request()
+    basic_auth = base64.b64encode(b"foo001:").decode()
+    assert request.headers["Authorization"] == f"Basic {basic_auth}"
+
+
+def test_authenticator_ignores_locked_keyring(
+    mock_remote: None,
+    http: type[httpretty.httpretty],
+    with_locked_keyring: None,
+    caplog: LogCaptureFixture,
+) -> None:
+    caplog.set_level(logging.DEBUG, logger="poetry.utils.password_manager")
+    authenticator = Authenticator()
+    authenticator.request("get", "https://foo.bar/files/foo-0.1.0.tar.gz")
+
+    request = http.last_request()
+    assert request.headers["Authorization"] is None
+    assert "Keyring foo.bar is locked" in caplog.messages
 
-    assert request.headers["Authorization"] == "Basic Zm9vMDAxOg=="
+
+def test_authenticator_ignores_failing_keyring(
+    mock_remote: None,
+    http: type[httpretty.httpretty],
+    with_erroneous_keyring: None,
+    caplog: LogCaptureFixture,
+) -> None:
+    caplog.set_level(logging.DEBUG, logger="poetry.utils.password_manager")
+    authenticator = Authenticator()
+    authenticator.request("get", "https://foo.bar/files/foo-0.1.0.tar.gz")
+
+    request = http.last_request()
+    assert request.headers["Authorization"] is None
+    assert "Accessing keyring foo.bar failed" in caplog.messages
 
 
 def test_authenticator_uses_password_only_credentials(
@@ -101,8 +133,8 @@ def test_authenticator_uses_password_only_credentials(
     authenticator.request("get", "https://:bar002@foo.bar/files/foo-0.1.0.tar.gz")
 
     request = http.last_request()
-
-    assert request.headers["Authorization"] == "Basic OmJhcjAwMg=="
+    basic_auth = base64.b64encode(b":bar002").decode()
+    assert request.headers["Authorization"] == f"Basic {basic_auth}"
 
 
 def test_authenticator_uses_empty_strings_as_default_password(
@@ -123,8 +155,8 @@ def test_authenticator_uses_empty_strings_as_default_password(
     authenticator.request("get", "https://foo.bar/files/foo-0.1.0.tar.gz")
 
     request = http.last_request()
-
-    assert request.headers["Authorization"] == "Basic YmFyOg=="
+    basic_auth = base64.b64encode(b"bar:").decode()
+    assert request.headers["Authorization"] == f"Basic {basic_auth}"
 
 
 def test_authenticator_uses_empty_strings_as_default_username(
@@ -144,8 +176,8 @@ def test_authenticator_uses_empty_strings_as_default_username(
     authenticator.request("get", "https://foo.bar/files/foo-0.1.0.tar.gz")
 
     request = http.last_request()
-
-    assert request.headers["Authorization"] == "Basic OmJhcg=="
+    basic_auth = base64.b64encode(b":bar").decode()
+    assert request.headers["Authorization"] == f"Basic {basic_auth}"
 
 
 def test_authenticator_falls_back_to_keyring_url(
@@ -170,8 +202,8 @@ def test_authenticator_falls_back_to_keyring_url(
     authenticator.request("get", "https://foo.bar/files/foo-0.1.0.tar.gz")
 
     request = http.last_request()
-
-    assert request.headers["Authorization"] == "Basic Zm9vOmJhcg=="
+    basic_auth = base64.b64encode(b"foo:bar").decode()
+    assert request.headers["Authorization"] == f"Basic {basic_auth}"
 
 
 def test_authenticator_falls_back_to_keyring_netloc(
@@ -194,8 +226,8 @@ def test_authenticator_falls_back_to_keyring_netloc(
     authenticator.request("get", "https://foo.bar/files/foo-0.1.0.tar.gz")
 
     request = http.last_request()
-
-    assert request.headers["Authorization"] == "Basic Zm9vOmJhcg=="
+    basic_auth = base64.b64encode(b"foo:bar").decode()
+    assert request.headers["Authorization"] == f"Basic {basic_auth}"
 
 
 @pytest.mark.filterwarnings("ignore::pytest.PytestUnhandledThreadExceptionWarning")
@@ -330,7 +362,8 @@ def test_authenticator_uses_env_provided_credentials(
 
     request = http.last_request()
 
-    assert request.headers["Authorization"] == "Basic YmFyOmJheg=="
+    basic_auth = base64.b64encode(b"bar:baz").decode()
+    assert request.headers["Authorization"] == f"Basic {basic_auth}"
 
 
 @pytest.mark.parametrize(

tests/utils/test_password_manager.py

@@ -1,5 +1,6 @@
 from __future__ import annotations
 
+import logging
 import os
 
 from typing import TYPE_CHECKING
@@ -13,6 +14,7 @@
 
 
 if TYPE_CHECKING:
+    from _pytest.logging import LogCaptureFixture
     from pytest_mock import MockerFixture
 
     from tests.conftest import Config
@@ -190,6 +192,32 @@ def test_keyring_raises_errors_on_keyring_errors(
         key_ring.delete_password("foo", "bar")
 
 
+def test_keyring_returns_none_on_locked_keyring(
+    with_locked_keyring: None,
+    caplog: LogCaptureFixture,
+) -> None:
+    caplog.set_level(logging.DEBUG, logger="poetry.utils.password_manager")
+    key_ring = PoetryKeyring("poetry")
+
+    cred = key_ring.get_credential("foo")
+
+    assert cred.password is None
+    assert "Keyring foo is locked" in caplog.messages
+
+
+def test_keyring_returns_none_on_erroneous_keyring(
+    with_erroneous_keyring: None,
+    caplog: LogCaptureFixture,
+) -> None:
+    caplog.set_level(logging.DEBUG, logger="poetry.utils.password_manager")
+    key_ring = PoetryKeyring("poetry")
+
+    cred = key_ring.get_credential("foo")
+
+    assert cred.password is None
+    assert "Accessing keyring foo failed" in caplog.messages
+
+
 def test_keyring_with_chainer_backend_and_fail_keyring_should_be_unavailable(
     with_chained_fail_keyring: None,
 ) -> None:
@@ -222,6 +250,18 @@ def test_fail_keyring_should_be_unavailable(
     assert not key_ring.is_available()
 
 
+def test_locked_keyring_should_be_available(with_locked_keyring: None) -> None:
+    key_ring = PoetryKeyring("poetry")
+
+    assert key_ring.is_available()
+
+
+def test_erroneous_keyring_should_be_available(with_erroneous_keyring: None) -> None:
+    key_ring = PoetryKeyring("poetry")
+
+    assert key_ring.is_available()
+
+
 def test_get_http_auth_from_environment_variables(
     environ: None, config: Config
 ) -> None: