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

Added pip style keyring password lookup as a fallback. #4086

Merged
merged 5 commits into from
Jul 30, 2021
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
15 changes: 15 additions & 0 deletions docs/repositories.md
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,21 @@ If a system keyring is available and supported, the password is stored to and re

Keyring support is enabled using the [keyring library](https://pypi.org/project/keyring/). For more information on supported backends refer to the [library documentation](https://keyring.readthedocs.io/en/latest/?badge=latest).

{{% note %}}

Poetry will fallback to Pip style use of keyring so that backends like
Microsoft's [artifacts-keyring](https://pypi.org/project/artifacts-keyring/) get a change to retrieve
valid credentials. It will need to be properly installed into Poetry's virtualenv,
preferrably by installing a plugin.

If you are letting Poetry manage your virtual environments you will want a virtualenv
seeder installed in Poetry's virtualenv that installs the desired keyring backend
during `poetry install`. To again use Azure DevOps as an example: [azure-devops-artifacts-helpers](https://pypi.org/project/azure-devops-artifacts-helpers/)
provides such a seeder. This would of course best achieved by installing a Poetry plugin
if it exists for you use case instead of doing it yourself.

{{% /note %}}

Alternatively, you can use environment variables to provide the credentials:

```bash
Expand Down
2 changes: 1 addition & 1 deletion poetry/installation/executor.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@
from poetry.utils.helpers import safe_rmtree
from poetry.utils.pip import pip_editable_install

from ..utils.authenticator import Authenticator
from ..utils.pip import pip_install
from .authenticator import Authenticator
from .chef import Chef
from .chooser import Chooser
from .operations.install import Install
Expand Down
13 changes: 6 additions & 7 deletions poetry/publishing/publisher.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,9 @@
from typing import Optional
from typing import Union

from poetry.utils.helpers import get_cert
from poetry.utils.helpers import get_client_cert
from poetry.utils.password_manager import PasswordManager

from ..utils.authenticator import Authenticator
from ..utils.helpers import get_cert
from ..utils.helpers import get_client_cert
from .uploader import Uploader


Expand All @@ -32,7 +31,7 @@ def __init__(self, poetry: "Poetry", io: Union["BufferedIO", "ConsoleIO"]) -> No
self._package = poetry.package
self._io = io
self._uploader = Uploader(poetry, io)
self._password_manager = PasswordManager(poetry.config)
self._authenticator = Authenticator(poetry.config, self._io)

@property
def files(self) -> List[Path]:
Expand All @@ -58,13 +57,13 @@ def publish(

if not (username and password):
# Check if we have a token first
token = self._password_manager.get_pypi_token(repository_name)
token = self._authenticator.get_pypi_token(repository_name)
if token:
logger.debug(f"Found an API token for {repository_name}.")
username = "__token__"
password = token
else:
auth = self._password_manager.get_http_auth(repository_name)
auth = self._authenticator.get_http_auth(repository_name)
if auth:
logger.debug(
"Found authentication information for {}.".format(
Expand Down
2 changes: 1 addition & 1 deletion poetry/repositories/legacy_repository.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@

from ..config.config import Config
from ..inspection.info import PackageInfo
from ..installation.authenticator import Authenticator
from ..utils.authenticator import Authenticator
from .exceptions import PackageNotFound
from .exceptions import RepositoryError
from .pypi_repository import PyPiRepository
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

from typing import TYPE_CHECKING
from typing import Any
from typing import Dict
from typing import Optional
from typing import Tuple

Expand Down Expand Up @@ -108,7 +109,7 @@ def get_credentials_for_url(self, url: str) -> Tuple[Optional[str], Optional[str

if credentials == (None, None):
if "@" not in netloc:
credentials = self._get_credentials_for_netloc_from_config(netloc)
credentials = self._get_credentials_for_netloc(netloc)
else:
# Split from the right because that's how urllib.parse.urlsplit()
# behaves if more than one @ is present (which can be checked using
Expand All @@ -133,28 +134,73 @@ def get_credentials_for_url(self, url: str) -> Tuple[Optional[str], Optional[str

return credentials[0], credentials[1]

def _get_credentials_for_netloc_from_config(
def get_pypi_token(self, name: str) -> str:
return self._password_manager.get_pypi_token(name)

def get_http_auth(self, name: str) -> Optional[Dict[str, str]]:
return self._get_http_auth(name, None)

def _get_http_auth(
self, name: str, netloc: Optional[str]
) -> Optional[Dict[str, str]]:
if name == "pypi":
url = "https://upload.pypi.org/legacy/"
else:
url = self._config.get(f"repositories.{name}.url")
if not url:
return

parsed_url = urllib.parse.urlsplit(url)

if netloc is None or netloc == parsed_url.netloc:
auth = self._password_manager.get_http_auth(name)

if auth is None or auth["password"] is None:
username = auth["username"] if auth else None
auth = self._get_credentials_for_netloc_from_keyring(
url, parsed_url.netloc, username
)

return auth

def _get_credentials_for_netloc(
self, netloc: str
) -> Tuple[Optional[str], Optional[str]]:
credentials = (None, None)

for repository_name in self._config.get("repositories", []):
repository_config = self._config.get(f"repositories.{repository_name}")
if not repository_config:
continue
auth = self._get_http_auth(repository_name, netloc)

url = repository_config.get("url")
if not url:
if auth is None:
continue

parsed_url = urllib.parse.urlsplit(url)

if netloc == parsed_url.netloc:
auth = self._password_manager.get_http_auth(repository_name)

if auth is None:
continue

return auth["username"], auth["password"]
return auth["username"], auth["password"]

return credentials

def _get_credentials_for_netloc_from_keyring(
self, url: str, netloc: str, username: Optional[str]
) -> Optional[Dict[str, str]]:
import keyring

cred = keyring.get_credential(url, username)
if cred is not None:
return {
"username": cred.username,
"password": cred.password,
}

cred = keyring.get_credential(netloc, username)
if cred is not None:
return {
"username": cred.username,
"password": cred.password,
}

if username:
return {
"username": username,
"password": None,
}

return None
56 changes: 56 additions & 0 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import pytest

from cleo.testers.command_tester import CommandTester
from keyring.backend import KeyringBackend

from poetry.config.config import Config as BaseConfig
from poetry.config.dict_config_source import DictConfigSource
Expand Down Expand Up @@ -53,6 +54,61 @@ def all(self) -> Dict[str, Any]:
return super(Config, self).all()


class DummyBackend(KeyringBackend):
def __init__(self):
self._passwords = {}

@classmethod
def priority(cls):
return 42

def set_password(self, service, username, password):
self._passwords[service] = {username: password}

def get_password(self, service, username):
return self._passwords.get(service, {}).get(username)

def get_credential(self, service, username):
return self._passwords.get(service, {}).get(username)

def delete_password(self, service, username):
if service in self._passwords and username in self._passwords[service]:
del self._passwords[service][username]


@pytest.fixture()
def dummy_keyring():
return DummyBackend()


@pytest.fixture()
def with_simple_keyring(dummy_keyring):
import keyring

keyring.set_keyring(dummy_keyring)


@pytest.fixture()
def with_fail_keyring():
import keyring

from keyring.backends.fail import Keyring

keyring.set_keyring(Keyring())


@pytest.fixture()
def with_chained_keyring(mocker):
from keyring.backends.fail import Keyring

mocker.patch("keyring.backend.get_all_keyring", [Keyring()])
import keyring

from keyring.backends.chainer import ChainerBackend

keyring.set_keyring(ChainerBackend())


@pytest.fixture
def config_cache_dir(tmp_dir):
path = Path(tmp_dir) / ".cache" / "pypoetry"
Expand Down
12 changes: 6 additions & 6 deletions tests/test_factory.py
Original file line number Diff line number Diff line change
Expand Up @@ -156,13 +156,13 @@ def test_create_poetry_with_multi_constraints_dependency():
assert len(package.requires) == 2


def test_poetry_with_default_source():
def test_poetry_with_default_source(with_simple_keyring):
poetry = Factory().create_poetry(fixtures_dir / "with_default_source")

assert 1 == len(poetry.pool.repositories)


def test_poetry_with_non_default_source():
def test_poetry_with_non_default_source(with_simple_keyring):
poetry = Factory().create_poetry(fixtures_dir / "with_non_default_source")

assert len(poetry.pool.repositories) == 2
Expand All @@ -176,7 +176,7 @@ def test_poetry_with_non_default_source():
assert isinstance(poetry.pool.repositories[1], PyPiRepository)


def test_poetry_with_non_default_secondary_source():
def test_poetry_with_non_default_secondary_source(with_simple_keyring):
poetry = Factory().create_poetry(fixtures_dir / "with_non_default_secondary_source")

assert len(poetry.pool.repositories) == 2
Expand All @@ -192,7 +192,7 @@ def test_poetry_with_non_default_secondary_source():
assert isinstance(repository, LegacyRepository)


def test_poetry_with_non_default_multiple_secondary_sources():
def test_poetry_with_non_default_multiple_secondary_sources(with_simple_keyring):
poetry = Factory().create_poetry(
fixtures_dir / "with_non_default_multiple_secondary_sources"
)
Expand All @@ -214,7 +214,7 @@ def test_poetry_with_non_default_multiple_secondary_sources():
assert isinstance(repository, LegacyRepository)


def test_poetry_with_non_default_multiple_sources():
def test_poetry_with_non_default_multiple_sources(with_simple_keyring):
poetry = Factory().create_poetry(fixtures_dir / "with_non_default_multiple_sources")

assert len(poetry.pool.repositories) == 3
Expand Down Expand Up @@ -245,7 +245,7 @@ def test_poetry_with_no_default_source():
assert isinstance(poetry.pool.repositories[0], PyPiRepository)


def test_poetry_with_two_default_sources():
def test_poetry_with_two_default_sources(with_simple_keyring):
with pytest.raises(ValueError) as e:
Factory().create_poetry(fixtures_dir / "with_two_default_sources")

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,13 @@

from cleo.io.null_io import NullIO

from poetry.installation.authenticator import Authenticator
from poetry.utils.authenticator import Authenticator


class SimpleCredential:
def __init__(self, username, password):
self.username = username
self.password = password


@pytest.fixture()
Expand Down Expand Up @@ -52,7 +58,9 @@ def test_authenticator_uses_credentials_from_config_if_not_provided(
assert "Basic YmFyOmJheg==" == request.headers["Authorization"]


def test_authenticator_uses_username_only_credentials(config, mock_remote, http):
def test_authenticator_uses_username_only_credentials(
config, mock_remote, http, with_simple_keyring
):
config.merge(
{
"repositories": {"foo": {"url": "https://foo.bar/simple/"}},
Expand Down Expand Up @@ -85,7 +93,7 @@ def test_authenticator_uses_password_only_credentials(config, mock_remote, http)


def test_authenticator_uses_empty_strings_as_default_password(
config, mock_remote, http
config, mock_remote, http, with_simple_keyring
):
config.merge(
{
Expand Down Expand Up @@ -120,6 +128,46 @@ def test_authenticator_uses_empty_strings_as_default_username(
assert "Basic OmJhcg==" == request.headers["Authorization"]


def test_authenticator_falls_back_to_keyring_url(
config, mock_remote, http, with_simple_keyring, dummy_keyring
):
config.merge(
{
"repositories": {"foo": {"url": "https://foo.bar/simple/"}},
}
)

dummy_keyring.set_password(
"https://foo.bar/simple/", None, SimpleCredential(None, "bar")
)

authenticator = Authenticator(config, NullIO())
authenticator.request("get", "https://foo.bar/files/foo-0.1.0.tar.gz")

request = http.last_request()

assert "Basic OmJhcg==" == request.headers["Authorization"]


def test_authenticator_falls_back_to_keyring_netloc(
config, mock_remote, http, with_simple_keyring, dummy_keyring
):
config.merge(
{
"repositories": {"foo": {"url": "https://foo.bar/simple/"}},
}
)

dummy_keyring.set_password("foo.bar", None, SimpleCredential(None, "bar"))

authenticator = Authenticator(config, NullIO())
authenticator.request("get", "https://foo.bar/files/foo-0.1.0.tar.gz")

request = http.last_request()

assert "Basic OmJhcg==" == request.headers["Authorization"]


def test_authenticator_request_retries_on_exception(mocker, config, http):
sleep = mocker.patch("time.sleep")
sdist_uri = "https://foo.bar/files/{}/foo-0.1.0.tar.gz".format(str(uuid.uuid4()))
Expand Down
Loading