From 57d9495870a5751c52bf40d8e045bf973ac503af Mon Sep 17 00:00:00 2001 From: dberenbaum Date: Thu, 30 Mar 2023 14:40:02 -0400 Subject: [PATCH 1/3] disallow cred helper to return empty output --- src/scmrepo/git/credentials.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/scmrepo/git/credentials.py b/src/scmrepo/git/credentials.py index 963c1c07..bce3c3a7 100644 --- a/src/scmrepo/git/credentials.py +++ b/src/scmrepo/git/credentials.py @@ -156,6 +156,8 @@ def get(self, **kwargs) -> "Credential": raise CredentialNotFoundError("Helper not found") from exc if res.stderr: logger.debug(res.stderr) + if not res.stdout: + raise CredentialNotFoundError("No credentials found") credentials = {} for line in res.stdout.strip().splitlines(): From 13c53db08c373cc032c68b3790e03028ad015e30 Mon Sep 17 00:00:00 2001 From: Peter Rowlands Date: Fri, 31 Mar 2023 13:05:15 +0900 Subject: [PATCH 2/3] check for empty credentials instead of empty output --- src/scmrepo/git/credentials.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/scmrepo/git/credentials.py b/src/scmrepo/git/credentials.py index bce3c3a7..ff69be3d 100644 --- a/src/scmrepo/git/credentials.py +++ b/src/scmrepo/git/credentials.py @@ -156,8 +156,6 @@ def get(self, **kwargs) -> "Credential": raise CredentialNotFoundError("Helper not found") from exc if res.stderr: logger.debug(res.stderr) - if not res.stdout: - raise CredentialNotFoundError("No credentials found") credentials = {} for line in res.stdout.strip().splitlines(): @@ -166,6 +164,8 @@ def get(self, **kwargs) -> "Credential": credentials[key] = value except ValueError: continue + if not credentials: + raise CredentialNotFoundError("No credentials found") return Credential(**credentials) def store(self, **kwargs): From 17c8a9b6c4127b6842218e3ffef9debc60358311 Mon Sep 17 00:00:00 2001 From: Peter Rowlands Date: Fri, 31 Mar 2023 13:05:58 +0900 Subject: [PATCH 3/3] add tests for credentialhelper --- src/scmrepo/git/credentials.py | 5 +++ tests/test_credentials.py | 64 ++++++++++++++++++++++++++++++++++ 2 files changed, 69 insertions(+) create mode 100644 tests/test_credentials.py diff --git a/src/scmrepo/git/credentials.py b/src/scmrepo/git/credentials.py index ff69be3d..04a87f9f 100644 --- a/src/scmrepo/git/credentials.py +++ b/src/scmrepo/git/credentials.py @@ -369,6 +369,11 @@ def __init__( self.username = self.username or parsed.username self.password = self.password or parsed.password + def __eq__(self, other: object) -> bool: + if isinstance(other, Credential): + return self._helper_kwargs == other._helper_kwargs + return False + @property def url(self) -> str: if self.username or self.password: diff --git a/tests/test_credentials.py b/tests/test_credentials.py new file mode 100644 index 00000000..4b8df28a --- /dev/null +++ b/tests/test_credentials.py @@ -0,0 +1,64 @@ +import os + +import pytest + +from scmrepo.git.credentials import ( + Credential, + CredentialNotFoundError, + GitCredentialHelper, +) + + +@pytest.fixture(name="helper") +def helper_fixture(mocker) -> "GitCredentialHelper": + mocker.patch("shutil.which", return_value="/usr/bin/git-credential-foo") + return GitCredentialHelper("foo") + + +def test_subprocess_get(helper, mocker): + run = mocker.patch( + "subprocess.run", + return_value=mocker.Mock( + stdout=os.linesep.join( + ["protocol=https", "host=foo.com", "username=foo", "password=bar", ""] + ) + ), + ) + creds = helper.get(protocol="https", host="foo.com") + assert run.call_args.args[0] == ["git-credential-foo", "get"] + assert run.call_args.kwargs.get("input") == os.linesep.join( + ["protocol=https", "host=foo.com", ""] + ) + assert creds == Credential(url="https://foo:bar@foo.com") + + +def test_subprocess_get_failed(helper, mocker): + from subprocess import CalledProcessError + + mocker.patch("subprocess.run", side_effect=CalledProcessError(1, "/usr/bin/foo")) + with pytest.raises(CredentialNotFoundError): + helper.get(protocol="https", host="foo.com") + + +def test_subprocess_get_no_output(helper, mocker): + mocker.patch("subprocess.run", return_value=mocker.Mock(stdout=os.linesep)) + with pytest.raises(CredentialNotFoundError): + helper.get(protocol="https", host="foo.com") + + +def test_subprocess_store(helper, mocker): + run = mocker.patch("subprocess.run") + helper.store(protocol="https", host="foo.com", username="foo", password="bar") + assert run.call_args.args[0] == ["git-credential-foo", "store"] + assert run.call_args.kwargs.get("input") == os.linesep.join( + ["protocol=https", "host=foo.com", "username=foo", "password=bar", ""] + ) + + +def test_subprocess_erase(helper, mocker): + run = mocker.patch("subprocess.run") + helper.erase(protocol="https", host="foo.com") + assert run.call_args.args[0] == ["git-credential-foo", "erase"] + assert run.call_args.kwargs.get("input") == os.linesep.join( + ["protocol=https", "host=foo.com", ""] + )