Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: remove exception when keyring is locked #8227

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion src/poetry/utils/password_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
radoering marked this conversation as resolved.
Show resolved Hide resolved
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
Expand Down
53 changes: 41 additions & 12 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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]
Expand All @@ -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]
Expand All @@ -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]
Expand Down Expand Up @@ -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]
Expand Down
65 changes: 49 additions & 16 deletions tests/utils/test_authenticator.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from __future__ import annotations

import base64
import logging
import re
import uuid

Expand All @@ -20,6 +21,7 @@


if TYPE_CHECKING:
from _pytest.logging import LogCaptureFixture
from _pytest.monkeypatch import MonkeyPatch
from pytest_mock import MockerFixture

Expand Down Expand Up @@ -65,8 +67,8 @@ def test_authenticator_uses_url_provided_credentials(
authenticator.request("get", "https://foo001:[email protected]/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(
Expand All @@ -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(
Expand All @@ -90,8 +92,38 @@ def test_authenticator_uses_username_only_credentials(
authenticator.request("get", "https://[email protected]/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(
Expand All @@ -101,8 +133,8 @@ def test_authenticator_uses_password_only_credentials(
authenticator.request("get", "https://:[email protected]/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(
Expand All @@ -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(
Expand All @@ -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(
Expand All @@ -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(
Expand All @@ -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")
Expand Down Expand Up @@ -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(
Expand Down
40 changes: 40 additions & 0 deletions tests/utils/test_password_manager.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
from __future__ import annotations

import logging
import os

from typing import TYPE_CHECKING
Expand All @@ -13,6 +14,7 @@


if TYPE_CHECKING:
from _pytest.logging import LogCaptureFixture
from pytest_mock import MockerFixture

from tests.conftest import Config
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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:
Expand Down