Skip to content

Commit

Permalink
git: ignore http auth for ssh url
Browse files Browse the repository at this point in the history
This change ensures that http-basic auth credentials are only passed to
dulwich when the remote url uses http/https schemes.

In addition to the above, it is now ensured that username/password
parameters are not passed through to dulwich unless both username and
password are configured explicitly. This is to ensure that dulwich does
not bail out if it detects a username in the url
(eg: `ssh://git@github.com`).
  • Loading branch information
abn committed May 10, 2022
1 parent bb7b32e commit 78c597d
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 1 deletion.
5 changes: 5 additions & 0 deletions src/poetry/utils/authenticator.py
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,11 @@ def _get_credentials_for_url(
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()

key = f"git+{url}"

if key not in self._credentials:
Expand Down
10 changes: 9 additions & 1 deletion 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

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
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
4 changes: 4 additions & 0 deletions tests/utils/test_authenticator.py
Original file line number Diff line number Diff line change
Expand Up @@ -592,6 +592,10 @@ def test_authenticator_git_repositories(
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

0 comments on commit 78c597d

Please sign in to comment.