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

improve git http authentication via repository configuration #5581

Merged
merged 3 commits into from
May 10, 2022
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
36 changes: 36 additions & 0 deletions src/poetry/config/config.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
from __future__ import annotations

import logging
import os
import re

Expand All @@ -9,8 +10,12 @@
from typing import Any
from typing import Callable

from poetry.core.toml import TOMLFile

from poetry.config.dict_config_source import DictConfigSource
from poetry.config.file_config_source import FileConfigSource
from poetry.locations import CACHE_DIR
from poetry.locations import CONFIG_DIR


if TYPE_CHECKING:
Expand All @@ -29,6 +34,12 @@ def int_normalizer(val: str) -> int:
return int(val)


logger = logging.getLogger(__name__)


_default_config: Config | None = None


class Config:

default_config: dict[str, Any] = {
Expand Down Expand Up @@ -186,3 +197,28 @@ def _get_normalizer(name: str) -> Callable[[str], Any]:
return int_normalizer

return lambda val: val

@classmethod
def create(cls, reload: bool = False) -> Config:
global _default_config

if _default_config is None or reload:
_default_config = cls()

# Load global config
config_file = TOMLFile(CONFIG_DIR / "config.toml")
if config_file.exists():
logger.debug("Loading configuration file %s", config_file.path)
_default_config.merge(config_file.read())

_default_config.set_config_source(FileConfigSource(config_file))

# Load global auth config
auth_config_file = TOMLFile(CONFIG_DIR / "auth.toml")
if auth_config_file.exists():
logger.debug("Loading configuration file %s", auth_config_file.path)
_default_config.merge(auth_config_file.read())

_default_config.set_auth_config_source(FileConfigSource(auth_config_file))

return _default_config
4 changes: 2 additions & 2 deletions src/poetry/console/commands/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -117,11 +117,11 @@ def handle(self) -> int | None:
from poetry.core.pyproject.exceptions import PyProjectException
from poetry.core.toml.file import TOMLFile

from poetry.config.config import Config
from poetry.config.file_config_source import FileConfigSource
from poetry.factory import Factory
from poetry.locations import CONFIG_DIR

config = Factory.create_config(self.io)
config = Config.create()
config_file = TOMLFile(CONFIG_DIR / "config.toml")

try:
Expand Down
2 changes: 1 addition & 1 deletion src/poetry/console/commands/self/update.py
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ def _update(self, version: Version) -> None:
root,
NullLocker(self.data_dir.joinpath("poetry.lock"), {}),
self.pool,
Config(),
config=Config.create(),
installed=installed,
)
installer.update(True)
Expand Down
49 changes: 16 additions & 33 deletions src/poetry/factory.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import contextlib
import logging
import re
import warnings

from typing import TYPE_CHECKING
from typing import Any
Expand All @@ -14,14 +15,11 @@
from tomlkit.toml_document import TOMLDocument

from poetry.config.config import Config
from poetry.config.file_config_source import FileConfigSource
from poetry.locations import CONFIG_DIR
from poetry.packages.locker import Locker
from poetry.packages.project_package import ProjectPackage
from poetry.plugins.plugin import Plugin
from poetry.plugins.plugin_manager import PluginManager
from poetry.poetry import Poetry
from poetry.utils.dependency_specification import dependency_to_specification


try:
Expand Down Expand Up @@ -65,7 +63,12 @@ def create_poetry(
)

# Loading global configuration
config = self.create_config(io)
with warnings.catch_warnings():
# this is preserved to ensure export plugin tests pass in ci,
# once poetry-plugin-export version is updated to use one that do not
# use Factory.create_config(), this can be safely removed.
warnings.filterwarnings("ignore", category=DeprecationWarning)
config = self.create_config()
neersighted marked this conversation as resolved.
Show resolved Hide resolved

# Loading local configuration
local_config_file = TOMLFile(base_poetry.file.parent / "poetry.toml")
Expand Down Expand Up @@ -116,35 +119,13 @@ def get_package(cls, name: str, version: str) -> ProjectPackage:

@classmethod
def create_config(cls, io: IO | None = None) -> Config:
if io is None:
io = NullIO()

config = Config()
# Load global config
config_file = TOMLFile(CONFIG_DIR / "config.toml")
if config_file.exists():
if io.is_debug():
io.write_line(
f"<debug>Loading configuration file {config_file.path}</debug>"
)

config.merge(config_file.read())

config.set_config_source(FileConfigSource(config_file))

# Load global auth config
auth_config_file = TOMLFile(CONFIG_DIR / "auth.toml")
if auth_config_file.exists():
if io.is_debug():
io.write_line(
f"<debug>Loading configuration file {auth_config_file.path}</debug>"
)

config.merge(auth_config_file.read())

config.set_auth_config_source(FileConfigSource(auth_config_file))

return config
if io is not None:
logger.debug("Ignoring provided io when creating config.")
warnings.warn(
"Use of Factory.create_config() is deprecated, use Config.create() instead",
DeprecationWarning,
)
return Config.create()

@classmethod
def configure_sources(
Expand Down Expand Up @@ -223,6 +204,8 @@ def create_pyproject_from_package(
) -> TOMLDocument:
import tomlkit

from poetry.utils.dependency_specification import dependency_to_specification

pyproject: dict[str, Any] = tomlkit.document()

tool_table = tomlkit.table()
Expand Down
27 changes: 23 additions & 4 deletions src/poetry/utils/authenticator.py
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ def __init__(
cache_id: str | None = None,
disable_cache: bool = False,
) -> None:
self._config = config or Config(use_environment=True)
self._config = config or Config.create()
self._io = io
self._sessions_for_netloc: dict[str, requests.Session] = {}
self._credentials: dict[str, HTTPAuthCredential] = {}
Expand Down Expand Up @@ -245,8 +245,10 @@ def _get_credentials_for_repository(

return self._credentials[key]

def _get_credentials_for_url(self, url: str) -> HTTPAuthCredential:
repository = self.get_repository_config_for_url(url)
def _get_credentials_for_url(
self, url: str, exact_match: bool = False
) -> HTTPAuthCredential:
repository = self.get_repository_config_for_url(url, exact_match)

credential = (
self._get_credentials_for_repository(repository=repository)
Expand All @@ -267,6 +269,19 @@ def _get_credentials_for_url(self, url: str) -> HTTPAuthCredential:

return credential

def get_credentials_for_git_url(self, url: str) -> HTTPAuthCredential:
parsed_url = urllib.parse.urlsplit(url)

if parsed_url.scheme not in {"http", "https"}:
return HTTPAuthCredential()
neersighted marked this conversation as resolved.
Show resolved Hide resolved

key = f"git+{url}"

if key not in self._credentials:
self._credentials[key] = self._get_credentials_for_url(url, True)

return self._credentials[key]

def get_credentials_for_url(self, url: str) -> HTTPAuthCredential:
parsed_url = urllib.parse.urlsplit(url)
netloc = parsed_url.netloc
Expand Down Expand Up @@ -338,13 +353,17 @@ def get_certs_for_url(self, url: str) -> dict[str, Path | None]:

@functools.lru_cache(maxsize=None)
def get_repository_config_for_url(
self, url: str
self, url: str, exact_match: bool = False
) -> AuthenticatorRepositoryConfig | None:
parsed_url = urllib.parse.urlsplit(url)
candidates_netloc_only = []
candidates_path_match = []

for repository in self.configured_repositories.values():
if exact_match:
if parsed_url.path == repository.path:
return repository
continue

if repository.netloc == parsed_url.netloc:
if parsed_url.path.startswith(repository.path) or commonprefix(
Expand Down
22 changes: 14 additions & 8 deletions src/poetry/vcs/git/backend.py
Original file line number Diff line number Diff line change
Expand Up @@ -186,9 +186,17 @@ def _fetch_remote_refs(cls, url: str, local: Repo) -> FetchPackResult:
client: GitClient
path: str

credentials = get_default_authenticator().get_credentials_for_url(url=url)
kwargs: dict[str, str] = {}
credentials = get_default_authenticator().get_credentials_for_git_url(url=url)

if credentials.password and credentials.username:
# we do this conditionally as otherwise, dulwich might complain if these
# parameters are passed in for an ssh url
kwargs["username"] = credentials.username
kwargs["password"] = credentials.password

client, path = get_transport_and_path( # type: ignore[no-untyped-call]
url, username=credentials.username, password=credentials.password
url, **kwargs
)

with local:
Expand Down Expand Up @@ -349,20 +357,18 @@ def _clone_submodules(cls, repo: Repo) -> None:

@staticmethod
def is_using_legacy_client() -> bool:
from poetry.factory import Factory
from poetry.config.config import Config

legacy_client: bool = (
Factory.create_config()
.get("experimental", {})
.get("system-git-client", False)
Config.create().get("experimental", {}).get("system-git-client", False)
)
return legacy_client

@staticmethod
def get_default_source_root() -> Path:
from poetry.factory import Factory
from poetry.config.config import Config

return Path(Factory.create_config().get("cache-dir")) / "src"
return Path(Config.create().get("cache-dir")) / "src"

@classmethod
def clone(
Expand Down
4 changes: 2 additions & 2 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ def config(
c.set_config_source(config_source)
c.set_auth_config_source(auth_config_source)

mocker.patch("poetry.factory.Factory.create_config", return_value=c)
mocker.patch("poetry.config.config.Config.create", return_value=c)
mocker.patch("poetry.config.config.Config.set_config_source")

return c
Expand All @@ -219,7 +219,7 @@ def config_dir(tmp_dir: str) -> Path:
@pytest.fixture(autouse=True)
def mock_user_config_dir(mocker: MockerFixture, config_dir: Path) -> None:
mocker.patch("poetry.locations.CONFIG_DIR", new=config_dir)
mocker.patch("poetry.factory.CONFIG_DIR", new=config_dir)
mocker.patch("poetry.config.config.CONFIG_DIR", new=config_dir)


@pytest.fixture(autouse=True)
Expand Down
3 changes: 2 additions & 1 deletion tests/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
from poetry.core.toml.file import TOMLFile
from poetry.core.vcs.git import ParsedUrl

from poetry.config.config import Config
from poetry.console.application import Application
from poetry.factory import Factory
from poetry.installation.executor import Executor
Expand Down Expand Up @@ -107,7 +108,7 @@ def mock_clone(
folder = Path(__file__).parent / "fixtures" / "git" / parsed.resource / path

if not source_root:
source_root = Path(Factory.create_config().get("cache-dir")) / "src"
source_root = Path(Config.create().get("cache-dir")) / "src"

dest = source_root / path
dest.parent.mkdir(parents=True, exist_ok=True)
Expand Down
19 changes: 19 additions & 0 deletions tests/integration/test_utils_vcs_git.py
Original file line number Diff line number Diff line change
Expand Up @@ -298,6 +298,25 @@ def test_configured_repository_http_auth(
spy_get_transport_and_path.assert_called_once()


def test_username_password_parameter_is_not_passed_to_dulwich(
mocker: MockerFixture, source_url: str, config: Config
) -> None:
from poetry.vcs.git import backend

spy_clone = mocker.spy(Git, "_clone")
spy_get_transport_and_path = mocker.spy(backend, "get_transport_and_path")

with Git.clone(url=source_url, branch="0.1") as repo:
assert_version(repo, BRANCH_TO_REVISION_MAP["0.1"])

spy_clone.assert_called_once()

spy_get_transport_and_path.assert_called_with(
location=source_url,
)
spy_get_transport_and_path.assert_called_once()


def test_system_git_called_when_configured(
mocker: MockerFixture, source_url: str, use_system_git_client: None
) -> None:
Expand Down
2 changes: 1 addition & 1 deletion tests/publishing/test_publisher.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ def test_publish_can_publish_to_given_repository(
}
)

mocker.patch("poetry.factory.Factory.create_config", return_value=config)
mocker.patch("poetry.config.config.Config.create", return_value=config)
poetry = Factory().create_poetry(fixture_dir(fixture_name))

io = BufferedIO()
Expand Down
39 changes: 39 additions & 0 deletions tests/utils/test_authenticator.py
Original file line number Diff line number Diff line change
Expand Up @@ -560,3 +560,42 @@ def test_authenticator_add_repository(

basic_auth = base64.b64encode(b"foo:bar").decode()
assert request.headers["Authorization"] == f"Basic {basic_auth}"


def test_authenticator_git_repositories(
config: Config,
mock_remote: None,
http: type[httpretty.httpretty],
with_simple_keyring: None,
dummy_keyring: DummyBackend,
):
config.merge(
{
"repositories": {
"one": {"url": "https://foo.bar/org/one.git"},
"two": {"url": "https://foo.bar/org/two.git"},
},
"http-basic": {
"one": {"username": "foo", "password": "bar"},
"two": {"username": "baz", "password": "qux"},
},
}
)

authenticator = Authenticator(config, NullIO())

one = authenticator.get_credentials_for_git_url("https://foo.bar/org/one.git")
assert one.username == "foo"
assert one.password == "bar"

two = authenticator.get_credentials_for_git_url("https://foo.bar/org/two.git")
assert two.username == "baz"
assert two.password == "qux"

two_ssh = authenticator.get_credentials_for_git_url("ssh://git@foo.bar/org/two.git")
assert not two_ssh.username
assert not two_ssh.password

three = authenticator.get_credentials_for_git_url("https://foo.bar/org/three.git")
assert not three.username
assert not three.password