From 4f2d262f2ff5d5a1f6d2207134136c85a99b4582 Mon Sep 17 00:00:00 2001 From: Thomas Desveaux Date: Thu, 25 Apr 2024 21:42:39 +0200 Subject: [PATCH 1/3] git: add detection of credential store support to GitMixin --- master/buildbot/test/unit/util/test_git.py | 4 ++++ master/buildbot/util/git.py | 3 +++ 2 files changed, 7 insertions(+) diff --git a/master/buildbot/test/unit/util/test_git.py b/master/buildbot/test/unit/util/test_git.py index 5a6ebc6d3a05..bc4b0d80bf89 100644 --- a/master/buildbot/test/unit/util/test_git.py +++ b/master/buildbot/test/unit/util/test_git.py @@ -113,6 +113,7 @@ def test_no_output(self): self.assertFalse(self.supportsSshPrivateKeyAsEnvOption) self.assertFalse(self.supportsSshPrivateKeyAsConfigOption) self.assertFalse(self.supports_lsremote_symref) + self.assertFalse(self.supports_credential_store) def test_git_noversion(self): self.parseGitFeatures('git') @@ -123,6 +124,7 @@ def test_git_noversion(self): self.assertFalse(self.supportsSshPrivateKeyAsEnvOption) self.assertFalse(self.supportsSshPrivateKeyAsConfigOption) self.assertFalse(self.supports_lsremote_symref) + self.assertFalse(self.supports_credential_store) def test_git_zero_version(self): self.parseGitFeatures('git version 0.0.0') @@ -133,6 +135,7 @@ def test_git_zero_version(self): self.assertFalse(self.supportsSshPrivateKeyAsEnvOption) self.assertFalse(self.supportsSshPrivateKeyAsConfigOption) self.assertFalse(self.supports_lsremote_symref) + self.assertFalse(self.supports_credential_store) def test_git_2_10_0(self): self.parseGitFeatures('git version 2.10.0') @@ -143,6 +146,7 @@ def test_git_2_10_0(self): self.assertTrue(self.supportsSshPrivateKeyAsEnvOption) self.assertTrue(self.supportsSshPrivateKeyAsConfigOption) self.assertTrue(self.supports_lsremote_symref) + self.assertTrue(self.supports_credential_store) class TestAdjustCommandParamsForSshPrivateKey(GitMixin, unittest.TestCase): diff --git a/master/buildbot/util/git.py b/master/buildbot/util/git.py index 73258cebecfd..bc27363f7436 100644 --- a/master/buildbot/util/git.py +++ b/master/buildbot/util/git.py @@ -89,6 +89,7 @@ def setupGit(self): self.supportsSshPrivateKeyAsConfigOption = False self.supportsFilters = False self.supports_lsremote_symref = False + self.supports_credential_store = False def parseGitFeatures(self, version_stdout): match = re.match(r"^git version (\d+(\.\d+)*)", version_stdout) @@ -106,6 +107,8 @@ def parseGitFeatures(self, version_stdout): self.supportsSubmoduleForce = True if version >= parse_version("1.7.8"): self.supportsSubmoduleCheckout = True + if version >= parse_version("1.7.9"): + self.supports_credential_store = True if version >= parse_version("2.3.0"): self.supportsSshPrivateKeyAsEnvOption = True if version >= parse_version("2.8.0"): From e16de24ec378b0822ebc40676091f10d530eab6e Mon Sep 17 00:00:00 2001 From: Thomas Desveaux Date: Fri, 26 Apr 2024 00:51:34 +0200 Subject: [PATCH 2/3] git: implement git auth with user/password for Git and GitPush steps --- master/buildbot/steps/source/git.py | 42 +- .../test/unit/steps/test_source_git.py | 402 ++++++++++++++++++ .../test/unit/util/test_git_credential.py | 42 ++ master/buildbot/util/git.py | 177 ++++++-- master/buildbot/util/git_credential.py | 91 ++++ .../misc/git_credential_options.rst | 39 ++ .../docs/manual/configuration/misc/index.rst | 2 + .../manual/configuration/steps/gitpush.rst | 10 + .../manual/configuration/steps/source_git.rst | 10 + master/setup.py | 4 + .../git-step-credential-auth.feature | 1 + 11 files changed, 790 insertions(+), 30 deletions(-) create mode 100644 master/buildbot/test/unit/util/test_git_credential.py create mode 100644 master/buildbot/util/git_credential.py create mode 100644 master/docs/manual/configuration/misc/git_credential_options.rst create mode 100644 newsfragments/git-step-credential-auth.feature diff --git a/master/buildbot/steps/source/git.py b/master/buildbot/steps/source/git.py index a1b5a226fb92..f00e822d071d 100644 --- a/master/buildbot/steps/source/git.py +++ b/master/buildbot/steps/source/git.py @@ -13,6 +13,10 @@ # # Copyright Buildbot Team Members +from __future__ import annotations + +from typing import TYPE_CHECKING + from twisted.internet import defer from twisted.internet import reactor from twisted.python import log @@ -25,6 +29,12 @@ from buildbot.steps.worker import CompositeStepMixin from buildbot.util.git import RC_SUCCESS from buildbot.util.git import GitStepMixin +from buildbot.util.git_credential import GitCredentialOptions +from buildbot.util.git_credential import add_user_password_to_credentials + +if TYPE_CHECKING: + from buildbot.interfaces import IRenderable + GIT_HASH_LENGTH = 40 @@ -89,6 +99,8 @@ def __init__( sshPrivateKey=None, sshHostKey=None, sshKnownHosts=None, + auth_credentials: tuple[IRenderable | str, IRenderable | str] | None = None, + git_credentials: GitCredentialOptions | None = None, **kwargs, ): if not getDescription and not isinstance(getDescription, dict): @@ -115,7 +127,19 @@ def __init__( super().__init__(**kwargs) self.setupGitStep() - self.setup_git_auth(sshPrivateKey, sshHostKey, sshKnownHosts) + if auth_credentials is not None: + git_credentials = add_user_password_to_credentials( + auth_credentials, + repourl, + git_credentials, + ) + + self.setup_git_auth( + sshPrivateKey, + sshHostKey, + sshKnownHosts, + git_credentials, + ) if isinstance(self.mode, str): if not self._hasAttrGroupMember('mode', self.mode): @@ -596,6 +620,8 @@ def __init__( sshPrivateKey=None, sshHostKey=None, sshKnownHosts=None, + auth_credentials: tuple[IRenderable | str, IRenderable | str] | None = None, + git_credentials: GitCredentialOptions | None = None, config=None, **kwargs, ): @@ -611,7 +637,19 @@ def __init__( super().__init__(**kwargs) self.setupGitStep() - self.setup_git_auth(sshPrivateKey, sshHostKey, sshKnownHosts) + if auth_credentials is not None: + git_credentials = add_user_password_to_credentials( + auth_credentials, + repourl, + git_credentials, + ) + + self.setup_git_auth( + sshPrivateKey, + sshHostKey, + sshKnownHosts, + git_credentials, + ) if not self.branch: bbconfig.error('GitPush: must provide branch') diff --git a/master/buildbot/test/unit/steps/test_source_git.py b/master/buildbot/test/unit/steps/test_source_git.py index d1c870040f9e..ab7e01619b91 100644 --- a/master/buildbot/test/unit/steps/test_source_git.py +++ b/master/buildbot/test/unit/steps/test_source_git.py @@ -39,6 +39,7 @@ from buildbot.test.util import config from buildbot.test.util import sourcesteps from buildbot.util import unicode2bytes +from buildbot.util.git_credential import GitCredentialOptions class TestGit( @@ -3891,6 +3892,268 @@ def test_mode_full_copy_recursive_fetch_fail_clobberOnFailure(self): self.expect_outcome(result=SUCCESS) return self.run_step() + @parameterized.expand([ + ('', None), + ('use_http_path', True), + ('dont_use_http_path', False), + ]) + def test_mode_full_clean_auth_credential(self, name, use_http_path): + self.setup_step( + self.stepClass( + repourl='https://example.com/test/test.git', + mode='full', + method='clean', + auth_credentials=('username', 'token'), + git_credentials=GitCredentialOptions( + credentials=[], + use_http_path=use_http_path, + ), + ) + ) + + ssh_workdir = '/wrk/.bldr.wkdir.buildbot' + git_credential_path = '/wrk/.bldr.wkdir.buildbot/.git-credentials' + + use_http_path_arg = [] + if use_http_path is not None: + use_http_path_arg.append('-c') + if use_http_path: + use_http_path_arg.append('credential.useHttpPath=true') + else: + use_http_path_arg.append('credential.useHttpPath=false') + + self.expect_commands( + ExpectShell(workdir='wkdir', command=['git', '--version']) + .stdout('git version 2.10.0') + .exit(0), + ExpectStat(file='wkdir/.buildbot-patched', log_environ=True).exit(1), + ExpectMkdir(dir=ssh_workdir, log_environ=True).exit(0), + ExpectShell( + workdir='wkdir', + command=[ + 'git', + '-c', + 'credential.helper=', + '-c', + f'credential.helper=store "--file={git_credential_path}"', + ] + + use_http_path_arg + + [ + 'credential', + 'approve', + ], + initial_stdin=( + "url=https://example.com/test/test.git\n" + "username=username\n" + "password=token\n" + ), + ).exit(0), + ExpectListdir(dir='wkdir').files(['.git']).exit(0), + ExpectShell(workdir='wkdir', command=['git', 'clean', '-f', '-f', '-d']).exit(0), + ExpectShell( + workdir='wkdir', + command=[ + 'git', + '-c', + 'credential.helper=', + '-c', + f'credential.helper=store "--file={git_credential_path}"', + ] + + use_http_path_arg + + [ + 'fetch', + '-f', + '-t', + 'https://example.com/test/test.git', + 'HEAD', + '--progress', + ], + ).exit(0), + ExpectShell(workdir='wkdir', command=['git', 'checkout', '-f', 'FETCH_HEAD']).exit(0), + ExpectShell(workdir='wkdir', command=['git', 'rev-parse', 'HEAD']) + .stdout('f6ad368298bd941e934a41f3babc827b2aa95a1d') + .exit(0), + ExpectRmdir(dir=ssh_workdir, log_environ=True).exit(0), + ) + self.expect_outcome(result=SUCCESS) + self.expect_property( + 'got_revision', 'f6ad368298bd941e934a41f3babc827b2aa95a1d', self.sourceName + ) + return self.run_step() + + def test_mode_full_clean_git_credential(self): + self.setup_step( + self.stepClass( + repourl='https://example.com/test/test.git', + mode='full', + method='clean', + git_credentials=GitCredentialOptions( + credentials=[ + ( + "url=https://example.com/test/test.git\n" + "username=username\n" + "password=token\n" + ), + ], + ), + ) + ) + + ssh_workdir = '/wrk/.bldr.wkdir.buildbot' + git_credential_path = '/wrk/.bldr.wkdir.buildbot/.git-credentials' + + self.expect_commands( + ExpectShell(workdir='wkdir', command=['git', '--version']) + .stdout('git version 2.10.0') + .exit(0), + ExpectStat(file='wkdir/.buildbot-patched', log_environ=True).exit(1), + ExpectMkdir(dir=ssh_workdir, log_environ=True).exit(0), + ExpectShell( + workdir='wkdir', + command=[ + 'git', + '-c', + 'credential.helper=', + '-c', + f'credential.helper=store "--file={git_credential_path}"', + 'credential', + 'approve', + ], + initial_stdin=( + "url=https://example.com/test/test.git\n" + "username=username\n" + "password=token\n" + ), + ).exit(0), + ExpectListdir(dir='wkdir').files(['.git']).exit(0), + ExpectShell(workdir='wkdir', command=['git', 'clean', '-f', '-f', '-d']).exit(0), + ExpectShell( + workdir='wkdir', + command=[ + 'git', + '-c', + 'credential.helper=', + '-c', + f'credential.helper=store "--file={git_credential_path}"', + 'fetch', + '-f', + '-t', + 'https://example.com/test/test.git', + 'HEAD', + '--progress', + ], + ).exit(0), + ExpectShell(workdir='wkdir', command=['git', 'checkout', '-f', 'FETCH_HEAD']).exit(0), + ExpectShell(workdir='wkdir', command=['git', 'rev-parse', 'HEAD']) + .stdout('f6ad368298bd941e934a41f3babc827b2aa95a1d') + .exit(0), + ExpectRmdir(dir=ssh_workdir, log_environ=True).exit(0), + ) + self.expect_outcome(result=SUCCESS) + self.expect_property( + 'got_revision', 'f6ad368298bd941e934a41f3babc827b2aa95a1d', self.sourceName + ) + return self.run_step() + + def test_mode_full_clean_auth_and_git_credential(self): + self.setup_step( + self.stepClass( + repourl='https://example.com/test/test.git', + mode='full', + method='clean', + auth_credentials=('auth_username', 'auth_token'), + git_credentials=GitCredentialOptions( + credentials=[ + ( + "url=https://example.com/test/submodule_test.git\n" + "username=username\n" + "password=token\n" + ), + ], + use_http_path=True, + ), + ) + ) + + ssh_workdir = '/wrk/.bldr.wkdir.buildbot' + git_credential_path = '/wrk/.bldr.wkdir.buildbot/.git-credentials' + + self.expect_commands( + ExpectShell(workdir='wkdir', command=['git', '--version']) + .stdout('git version 2.10.0') + .exit(0), + ExpectStat(file='wkdir/.buildbot-patched', log_environ=True).exit(1), + ExpectMkdir(dir=ssh_workdir, log_environ=True).exit(0), + ExpectShell( + workdir='wkdir', + command=[ + 'git', + '-c', + 'credential.helper=', + '-c', + f'credential.helper=store "--file={git_credential_path}"', + '-c', + 'credential.useHttpPath=true', + 'credential', + 'approve', + ], + initial_stdin=( + "url=https://example.com/test/test.git\n" + "username=auth_username\n" + "password=auth_token\n" + ), + ).exit(0), + ExpectShell( + workdir='wkdir', + command=[ + 'git', + '-c', + 'credential.helper=', + '-c', + f'credential.helper=store "--file={git_credential_path}"', + '-c', + 'credential.useHttpPath=true', + 'credential', + 'approve', + ], + initial_stdin=( + "url=https://example.com/test/submodule_test.git\n" + "username=username\n" + "password=token\n" + ), + ).exit(0), + ExpectListdir(dir='wkdir').files(['.git']).exit(0), + ExpectShell(workdir='wkdir', command=['git', 'clean', '-f', '-f', '-d']).exit(0), + ExpectShell( + workdir='wkdir', + command=[ + 'git', + '-c', + 'credential.helper=', + '-c', + f'credential.helper=store "--file={git_credential_path}"', + '-c', + 'credential.useHttpPath=true', + 'fetch', + '-f', + '-t', + 'https://example.com/test/test.git', + 'HEAD', + '--progress', + ], + ).exit(0), + ExpectShell(workdir='wkdir', command=['git', 'checkout', '-f', 'FETCH_HEAD']).exit(0), + ExpectShell(workdir='wkdir', command=['git', 'rev-parse', 'HEAD']) + .stdout('f6ad368298bd941e934a41f3babc827b2aa95a1d') + .exit(0), + ExpectRmdir(dir=ssh_workdir, log_environ=True).exit(0), + ) + self.expect_outcome(result=SUCCESS) + self.expect_property( + 'got_revision', 'f6ad368298bd941e934a41f3babc827b2aa95a1d', self.sourceName + ) + return self.run_step() + class TestGitPush( TestBuildStepMixin, config.ConfigErrorsMixin, TestReactorMixin, unittest.TestCase @@ -4241,6 +4504,145 @@ def test_config_fail_no_branch(self): with self.assertRaisesConfigError("GitPush: must provide branch"): self.stepClass(workdir='wkdir', repourl="url") + @parameterized.expand([ + ('', None), + ('use_http_path', True), + ('dont_use_http_path', False), + ]) + def test_push_auth_credential(self, name, use_http_path): + url = 'https://example.com/test/test.git' + self.setup_step( + self.stepClass( + workdir='wkdir', + repourl=url, + branch='testbranch', + auth_credentials=('username', 'token'), + git_credentials=GitCredentialOptions( + credentials=[], + use_http_path=use_http_path, + ), + ) + ) + + ssh_workdir = '/wrk/.bldr.wkdir.buildbot' + git_credential_path = '/wrk/.bldr.wkdir.buildbot/.git-credentials' + + use_http_path_arg = [] + if use_http_path is not None: + use_http_path_arg.append('-c') + if use_http_path: + use_http_path_arg.append('credential.useHttpPath=true') + else: + use_http_path_arg.append('credential.useHttpPath=false') + + self.expect_commands( + ExpectShell(workdir='wkdir', command=['git', '--version']) + .stdout('git version 1.7.9') + .exit(0), + ExpectMkdir(dir=ssh_workdir, log_environ=True).exit(0), + ExpectShell( + workdir='wkdir', + command=[ + 'git', + '-c', + 'credential.helper=', + '-c', + f'credential.helper=store "--file={git_credential_path}"', + ] + + use_http_path_arg + + [ + 'credential', + 'approve', + ], + initial_stdin=( + "url=https://example.com/test/test.git\n" + "username=username\n" + "password=token\n" + ), + ).exit(0), + ExpectShell( + workdir='wkdir', + command=[ + 'git', + '-c', + 'credential.helper=', + '-c', + f'credential.helper=store "--file={git_credential_path}"', + ] + + use_http_path_arg + + [ + 'push', + url, + 'testbranch', + ], + ).exit(0), + ExpectRmdir(dir=ssh_workdir, log_environ=True).exit(0), + ) + self.expect_outcome(result=SUCCESS) + return self.run_step() + + def test_push_git_credential(self): + url = 'https://example.com/test/test.git' + self.setup_step( + self.stepClass( + workdir='wkdir', + repourl=url, + branch='testbranch', + git_credentials=GitCredentialOptions( + credentials=[ + ( + "url=https://example.com/test/test.git\n" + "username=username\n" + "password=token\n" + ), + ] + ), + ) + ) + + ssh_workdir = '/wrk/.bldr.wkdir.buildbot' + git_credential_path = '/wrk/.bldr.wkdir.buildbot/.git-credentials' + + self.expect_commands( + ExpectShell(workdir='wkdir', command=['git', '--version']) + .stdout('git version 1.7.9') + .exit(0), + ExpectMkdir(dir=ssh_workdir, log_environ=True).exit(0), + ExpectShell( + workdir='wkdir', + command=[ + 'git', + '-c', + 'credential.helper=', + '-c', + f'credential.helper=store "--file={git_credential_path}"', + 'credential', + 'approve', + ], + initial_stdin=( + "url=https://example.com/test/test.git\n" + "username=username\n" + "password=token\n" + ), + ).exit(0), + ExpectShell( + workdir='wkdir', + command=[ + 'git', + '-c', + 'credential.helper=', + '-c', + f'credential.helper=store "--file={git_credential_path}"', + 'push', + url, + 'testbranch', + ], + ).exit(0), + ExpectRmdir(dir=ssh_workdir, log_environ=True).exit(0), + ) + self.expect_outcome(result=SUCCESS) + return self.run_step() + class TestGitTag(TestBuildStepMixin, config.ConfigErrorsMixin, TestReactorMixin, unittest.TestCase): stepClass = git.GitTag diff --git a/master/buildbot/test/unit/util/test_git_credential.py b/master/buildbot/test/unit/util/test_git_credential.py new file mode 100644 index 000000000000..28966f724653 --- /dev/null +++ b/master/buildbot/test/unit/util/test_git_credential.py @@ -0,0 +1,42 @@ +# This file is part of Buildbot. Buildbot is free software: you can +# redistribute it and/or modify it under the terms of the GNU General Public +# License as published by the Free Software Foundation, version 2. +# +# This program is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS +# FOR A PARTICULAR PURPOSE. See the GNU General Public License for more +# details. +# +# You should have received a copy of the GNU General Public License along with +# this program; if not, write to the Free Software Foundation, Inc., 51 +# Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. +# +# Copyright Buildbot Team Members + +from twisted.internet import defer +from twisted.trial import unittest + +from buildbot.process.properties import Properties +from buildbot.process.properties import Property +from buildbot.test.fake.fakebuild import FakeBuild +from buildbot.util.git_credential import GitCredentialInputRenderer + + +class TestGitCredentialInputRenderer(unittest.TestCase): + def setUp(self): + self.props = Properties() + self.build = FakeBuild(props=self.props) + + @defer.inlineCallbacks + def test_render(self): + self.props.setProperty("password", "property_password", "test") + renderer = GitCredentialInputRenderer( + username="user", + password=Property("password"), + url="https://example.com/repo.git", + ) + rendered = yield self.build.render(renderer) + self.assertEqual( + rendered, + "url=https://example.com/repo.git\nusername=user\npassword=property_password\n", + ) diff --git a/master/buildbot/util/git.py b/master/buildbot/util/git.py index bc27363f7436..10e50852cc01 100644 --- a/master/buildbot/util/git.py +++ b/master/buildbot/util/git.py @@ -32,6 +32,7 @@ from buildbot.steps.worker import CompositeStepMixin from buildbot.util import ComparableMixin from buildbot.util import bytes2unicode +from buildbot.util.git_credential import GitCredentialOptions from buildbot.util.misc import writeLocalFile if TYPE_CHECKING: @@ -177,12 +178,14 @@ def setup_git_auth( ssh_private_key: IRenderable | None, ssh_host_key: IRenderable | None, ssh_known_hosts: IRenderable | None, + git_credential_options: GitCredentialOptions | None = None, ) -> None: self._git_auth = GitStepAuth( self, ssh_private_key, ssh_host_key, ssh_known_hosts, + git_credential_options=git_credential_options, ) def _get_auth_data_workdir(self) -> str: @@ -268,6 +271,7 @@ class AbstractGitAuth(ComparableMixin): "ssh_private_key", "ssh_host_key", "ssh_known_hosts", + "git_credential_options", ) def __init__( @@ -275,6 +279,7 @@ def __init__( ssh_private_key: IRenderable | None = None, ssh_host_key: IRenderable | None = None, ssh_known_hosts: IRenderable | None = None, + git_credential_options: GitCredentialOptions | None = None, ) -> None: self.did_download_auth_files = False @@ -282,14 +287,20 @@ def __init__( self.ssh_host_key = ssh_host_key self.ssh_known_hosts = ssh_known_hosts + self.git_credential_options = git_credential_options + check_ssh_config('Git', self.ssh_private_key, self.ssh_host_key, self.ssh_known_hosts) def is_auth_needed_for_git_command(self, git_command: str) -> bool: - if not git_command or self.ssh_private_key is None: + if not git_command: + return False + + if self.ssh_private_key is None and self.git_credential_options is None: return False git_commands_that_need_auth = [ 'clone', + 'credential', 'fetch', 'ls-remote', 'push', @@ -316,6 +327,9 @@ def _get_ssh_host_key_path(self, ssh_data_path: str) -> str: def _get_ssh_wrapper_script_path(self, ssh_data_path: str) -> str: return self._path_module.join(ssh_data_path, 'ssh-wrapper.sh') + def _get_credential_store_file_path(self, ssh_data_path): + return self._path_module.join(ssh_data_path, '.git-credentials') + def _adjust_command_params_for_ssh_private_key( self, full_command: list[str], @@ -323,6 +337,9 @@ def _adjust_command_params_for_ssh_private_key( workdir: str, git_mixin: GitMixin, ) -> None: + if self.ssh_private_key is None: + return + key_path = self._get_ssh_private_key_path(workdir) host_key_path = None if self.ssh_host_key is not None or self.ssh_known_hosts is not None: @@ -338,6 +355,43 @@ def _adjust_command_params_for_ssh_private_key( host_key_path, ) + def _adjust_command_params_for_credential_store( + self, + full_command: list[str], + workdir: str, + git_mixin: GitMixin, + ): + if self.git_credential_options is None: + return + + if not git_mixin.supports_credential_store: + raise RuntimeError('git credential-store is not supported') + + credentials_path = self._get_credential_store_file_path(workdir) + + # This will unset the `credential.helper` config for this command + # so that system/global credential store is not used + # NOTE: This could be optional allowing credential retrieval from system sources + # However, it would need the store process (`credential approve`) to pass it + # as `credential approve` will store the credential in ALL credential helpers + full_command.extend([ + '-c', + 'credential.helper=', + ]) + + full_command.extend([ + '-c', + f'credential.helper=store "--file={credentials_path}"', + ]) + + if self.git_credential_options.use_http_path is not None: + # Whether or not to only use domain for credential lookup + value = 'true' if self.git_credential_options.use_http_path else 'false' + full_command.extend([ + '-c', + f'credential.useHttpPath={value}', + ]) + def adjust_git_command_params_for_auth( self, full_command: list[str], @@ -351,6 +405,19 @@ def adjust_git_command_params_for_auth( workdir=workdir, git_mixin=git_mixin, ) + self._adjust_command_params_for_credential_store( + full_command, + workdir=workdir, + git_mixin=git_mixin, + ) + + def _dovccmd( + self, + command: list[str], + initial_stdin: str | None = None, + workdir: str | None = None, + ) -> defer.Deferred[None]: + raise NotImplementedError() def _download_file( self, @@ -362,20 +429,14 @@ def _download_file( raise NotImplementedError() @defer.inlineCallbacks - def download_auth_files_if_needed( + def _download_ssh_files( self, + private_key: str, + host_key: str | None, + known_hosts: str | None, workdir: str, download_wrapper_script: bool = False, ): - if self.ssh_private_key is None: - return RC_SUCCESS - - p = Properties() - p.master = self._master - private_key = yield p.render(self.ssh_private_key) - host_key = yield p.render(self.ssh_host_key) - known_hosts = yield p.render(self.ssh_known_hosts) - private_key_path = self._get_ssh_private_key_path(workdir) private_key = ensureSshKeyNewline(private_key) yield self._download_file( @@ -385,15 +446,14 @@ def download_auth_files_if_needed( workdir=workdir, ) - known_hosts_path = None - if self.ssh_host_key is not None or self.ssh_known_hosts is not None: - known_hosts_path = self._get_ssh_host_key_path(workdir) - - if known_hosts is not None: - known_hosts_contents = known_hosts - else: - known_hosts_contents = getSshKnownHostsContents(host_key) + known_hosts_path = self._get_ssh_host_key_path(workdir) + known_hosts_contents = None + if known_hosts is not None: + known_hosts_contents = known_hosts + elif host_key is not None: + known_hosts_contents = getSshKnownHostsContents(host_key) + if known_hosts_contents is not None: yield self._download_file( known_hosts_path, known_hosts_contents, @@ -402,13 +462,11 @@ def download_auth_files_if_needed( ) if download_wrapper_script: - private_key_path = self._get_ssh_private_key_path(workdir) - known_hosts_path = None - if self.ssh_host_key is not None or self.ssh_known_hosts is not None: - known_hosts_path = self._get_ssh_host_key_path(workdir) - script_path = self._get_ssh_wrapper_script_path(workdir) - script_contents = getSshWrapperScriptContents(private_key_path, known_hosts_path) + script_contents = getSshWrapperScriptContents( + private_key_path, + (known_hosts_path if known_hosts_contents is not None else None), + ) yield self._download_file( script_path, @@ -417,7 +475,56 @@ def download_auth_files_if_needed( workdir=workdir, ) - self.did_download_auth_files = True + @defer.inlineCallbacks + def _download_credentials( + self, + credentials: list[str], + workdir: str, + ): + for creds in credentials: + # Using credential approve here instead of directly writing to the file + # as recommended by Git doc (https://git-scm.com/docs/git-credential-store#_storage_format) + # "Do not view or edit the file with editors." + yield self._dovccmd( + ['credential', 'approve'], + initial_stdin=creds, + workdir=workdir, + ) + + @defer.inlineCallbacks + def download_auth_files_if_needed( + self, + workdir: str, + download_wrapper_script: bool = False, + ): + p = Properties() + p.master = self._master + + private_key: str | None = yield p.render(self.ssh_private_key) + host_key: str | None = yield p.render(self.ssh_host_key) + known_hosts: str | None = yield p.render(self.ssh_known_hosts) + + if private_key is not None: + yield self._download_ssh_files( + private_key, + host_key, + known_hosts, + workdir, + download_wrapper_script, + ) + self.did_download_auth_files = True + + if self.git_credential_options is not None: + credentials: list[str] = [] + for creds in self.git_credential_options.credentials: + rendered: str | None = yield p.render(creds) + if rendered: + credentials.append(rendered) + + if credentials: + yield self._download_credentials(credentials, workdir) + self.did_download_auth_files = True + return RC_SUCCESS def remove_auth_files_if_needed(self, workdir: str) -> defer.Deferred[int]: @@ -432,10 +539,11 @@ def __init__( ssh_private_key: IRenderable | None = None, ssh_host_key: IRenderable | None = None, ssh_known_hosts: IRenderable | None = None, + git_credential_options: GitCredentialOptions | None = None, ) -> None: self.step = step - super().__init__(ssh_private_key, ssh_host_key, ssh_known_hosts) + super().__init__(ssh_private_key, ssh_host_key, ssh_known_hosts, git_credential_options) def _get_auth_data_path(self, data_workdir: str) -> str: # we can't use the workdir for temporary ssh-related files, because @@ -500,13 +608,26 @@ def _download_file(self, path: str, content: str, mode: int, workdir: str | None workdir=workdir, ) + @defer.inlineCallbacks + def _dovccmd( + self, + command: list[str], + initial_stdin: str | None = None, + workdir: str | None = None, + ): + assert isinstance(self.step, GitStepMixin) + yield self.step._dovccmd( + command=command, + initialStdin=initial_stdin, + ) + @defer.inlineCallbacks def download_auth_files_if_needed( self, workdir: str, download_wrapper_script: bool = False, ): - if self.ssh_private_key is None: + if self.ssh_private_key is None and self.git_credential_options is None: return RC_SUCCESS assert isinstance(self.step, CompositeStepMixin) and isinstance(self.step, GitMixin) diff --git a/master/buildbot/util/git_credential.py b/master/buildbot/util/git_credential.py new file mode 100644 index 000000000000..ccbe48b9bf38 --- /dev/null +++ b/master/buildbot/util/git_credential.py @@ -0,0 +1,91 @@ +# This file is part of Buildbot. Buildbot is free software: you can +# redistribute it and/or modify it under the terms of the GNU General Public +# License as published by the Free Software Foundation, version 2. +# +# This program is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS +# FOR A PARTICULAR PURPOSE. See the GNU General Public License for more +# details. +# +# You should have received a copy of the GNU General Public License along with +# this program; if not, write to the Free Software Foundation, Inc., 51 +# Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. +# +# Copyright Buildbot Team Members + +from __future__ import annotations + +from typing import NamedTuple + +from twisted.internet import defer +from zope.interface import implementer + +from buildbot.interfaces import IRenderable +from buildbot.util import ComparableMixin + + +@implementer(IRenderable) +class GitCredentialInputRenderer(ComparableMixin): + compare_attrs = ('_credential_attributes',) + + def __init__(self, **credential_attributes) -> None: + self._credential_attributes: dict[str, IRenderable | str] = credential_attributes + + @defer.inlineCallbacks + def getRenderingFor(self, build): + props = build.getProperties() + + rendered_attributes = [] + + attributes = list(self._credential_attributes.items()) + + # git-credential-approve parsing of the `url` attribute + # will reset all other fields + # So make sure it's the first attribute in the form + if 'url' in self._credential_attributes: + attributes.sort(key=lambda e: e[0] != "url") + + for key, value in attributes: + rendered_value = yield props.render(value) + if rendered_value is not None: + rendered_attributes.append(f"{key}={rendered_value}\n") + + return "".join(rendered_attributes) + + +class GitCredentialOptions(NamedTuple): + # Each element of `credentials` should be a `str` which is a input format for git-credential + # ref: https://git-scm.com/docs/git-credential#IOFMT + credentials: list[IRenderable | str] + # value to set the git config `credential.useHttpPath` to. + # ref: https://git-scm.com/docs/gitcredentials#Documentation/gitcredentials.txt-useHttpPath + use_http_path: bool | None = None + + +def add_user_password_to_credentials( + auth_credentials: tuple[IRenderable | str, IRenderable | str], + url: IRenderable | str | None, + credential_options: GitCredentialOptions | None, +) -> GitCredentialOptions: + if credential_options is None: + credential_options = GitCredentialOptions(credentials=[]) + else: + # create a new instance to avoid side-effects + credential_options = GitCredentialOptions( + credentials=credential_options.credentials[:], + use_http_path=credential_options.use_http_path, + ) + + username, password = auth_credentials + credential_options.credentials.insert( + 0, + IRenderable( # placate typing + GitCredentialInputRenderer( + url=url, + username=username, + password=password, + ) + ), + ) + + return credential_options diff --git a/master/docs/manual/configuration/misc/git_credential_options.rst b/master/docs/manual/configuration/misc/git_credential_options.rst new file mode 100644 index 000000000000..e308aa694cd2 --- /dev/null +++ b/master/docs/manual/configuration/misc/git_credential_options.rst @@ -0,0 +1,39 @@ +.. _GitCredentialOptions: + +GitCredentialOptions +++++++++++++++++++++ + +.. py:class:: buildbot.util.GitCredentialOptions + +The following parameters are supported by the :py:class:`GitCredentialOptions`: + + +``credentials`` + (optional, a list of strings) + Each element of the list must be in the `git-credential input format `_ + and will be passed as input to ``git credential approve``. + +``use_http_path`` + (optional, a boolean) + If provided, will set the `credential.useHttpPath `_ + configuration to it's value for commands that require credentials. + +Examples +~~~~~~~~ + +.. code-block:: python + + from buildbot.plugins import util + + factory.addStep(steps.Git( + repourl='https://example.com/hello-world.git', mode='incremental', + git_credentials=util.GitCredentialOptions( + credentials=[ + ( + "url=https://example.com/hello-world.git\n" + "username=username\n" + "password=token\n" + ), + ], + ), + )) diff --git a/master/docs/manual/configuration/misc/index.rst b/master/docs/manual/configuration/misc/index.rst index d496c5dd3226..8aa3258deb8d 100644 --- a/master/docs/manual/configuration/misc/index.rst +++ b/master/docs/manual/configuration/misc/index.rst @@ -9,9 +9,11 @@ Miscellaneous Configuration source_stamp_filter change_filter + git_credential_options This section outlines miscellaneous functionality that is useful for configuration but does not fit any other section. * :ref:`SourceStampFilter` * :ref:`ChangeFilter` +* :ref:`GitCredentialOptions` diff --git a/master/docs/manual/configuration/steps/gitpush.rst b/master/docs/manual/configuration/steps/gitpush.rst index c07ec00f945e..49513439dfd0 100644 --- a/master/docs/manual/configuration/steps/gitpush.rst +++ b/master/docs/manual/configuration/steps/gitpush.rst @@ -60,3 +60,13 @@ The GitPush step takes the following arguments: This may be either a :ref:`Secret` or just a string. `sshPrivateKey` must be specified in order to use this option. `sshHostKey` must not be specified in order to use this option. + +``auth_credentials`` + + (optional) An username/password tuple to use when running git for push operations. + The worker's git version needs to be at least 1.7.9. + +``git_credentials`` + + (optional) See :ref:`GitCredentialOptions`. + The worker's git version needs to be at least 1.7.9. diff --git a/master/docs/manual/configuration/steps/source_git.rst b/master/docs/manual/configuration/steps/source_git.rst index 4bc83865e61c..e4716740f88f 100644 --- a/master/docs/manual/configuration/steps/source_git.rst +++ b/master/docs/manual/configuration/steps/source_git.rst @@ -152,3 +152,13 @@ The Git step takes the following arguments: This may be either a :ref:`Secret` or just a string. `sshPrivateKey` must be specified in order to use this option. `sshHostKey` must not be specified in order to use this option. + +``auth_credentials`` + + (optional) An username/password tuple to use when running git for fetch operations. + The worker's git version needs to be at least 1.7.9. + +``git_credentials`` + + (optional) See :ref:`GitCredentialOptions`. + The worker's git version needs to be at least 1.7.9. diff --git a/master/setup.py b/master/setup.py index dea381734abc..907cbe540116 100755 --- a/master/setup.py +++ b/master/setup.py @@ -564,6 +564,10 @@ def define_plugin_entries(groups): ], ), ('buildbot.steps.shellsequence', ['ShellArg']), + ( + 'buildbot.util.git_credential', + ['GitCredentialInputRenderer', 'GitCredentialOptions'], + ), ( 'buildbot.util.kubeclientservice', [ diff --git a/newsfragments/git-step-credential-auth.feature b/newsfragments/git-step-credential-auth.feature new file mode 100644 index 000000000000..a49de77e66d9 --- /dev/null +++ b/newsfragments/git-step-credential-auth.feature @@ -0,0 +1 @@ +:bb:step:`Git` and :bb:step:`GitPush` now supports authentication with username/password. Credentials can be provided through the `auth_credentials` and/or `git_credentials` parameters. From f7982af009e0c136cf26e79f6fc2d67b1829a266 Mon Sep 17 00:00:00 2001 From: Thomas Desveaux Date: Tue, 30 Apr 2024 10:42:23 +0200 Subject: [PATCH 3/3] git: port auth helpers to async --- master/buildbot/util/git.py | 100 ++++++++++++++----------- master/buildbot/util/git_credential.py | 8 +- 2 files changed, 59 insertions(+), 49 deletions(-) diff --git a/master/buildbot/util/git.py b/master/buildbot/util/git.py index 10e50852cc01..6e1a02f893fe 100644 --- a/master/buildbot/util/git.py +++ b/master/buildbot/util/git.py @@ -34,6 +34,7 @@ from buildbot.util import bytes2unicode from buildbot.util.git_credential import GitCredentialOptions from buildbot.util.misc import writeLocalFile +from buildbot.util.twisted import async_to_deferred if TYPE_CHECKING: from buildbot.interfaces import IRenderable @@ -411,35 +412,36 @@ def adjust_git_command_params_for_auth( git_mixin=git_mixin, ) - def _dovccmd( + @async_to_deferred + async def _dovccmd( self, command: list[str], initial_stdin: str | None = None, workdir: str | None = None, - ) -> defer.Deferred[None]: + ) -> None: raise NotImplementedError() - def _download_file( + async def _download_file( self, path: str, content: str, mode: int, workdir: str | None = None, - ) -> defer.Deferred[None]: + ) -> None: raise NotImplementedError() - @defer.inlineCallbacks - def _download_ssh_files( + @async_to_deferred + async def _download_ssh_files( self, private_key: str, host_key: str | None, known_hosts: str | None, workdir: str, download_wrapper_script: bool = False, - ): + ) -> None: private_key_path = self._get_ssh_private_key_path(workdir) private_key = ensureSshKeyNewline(private_key) - yield self._download_file( + await self._download_file( private_key_path, private_key, mode=stat.S_IRUSR, @@ -454,7 +456,7 @@ def _download_ssh_files( known_hosts_contents = getSshKnownHostsContents(host_key) if known_hosts_contents is not None: - yield self._download_file( + await self._download_file( known_hosts_path, known_hosts_contents, mode=stat.S_IRUSR, @@ -468,44 +470,44 @@ def _download_ssh_files( (known_hosts_path if known_hosts_contents is not None else None), ) - yield self._download_file( + await self._download_file( script_path, script_contents, mode=stat.S_IRWXU, workdir=workdir, ) - @defer.inlineCallbacks - def _download_credentials( + @async_to_deferred + async def _download_credentials( self, credentials: list[str], workdir: str, - ): + ) -> None: for creds in credentials: # Using credential approve here instead of directly writing to the file # as recommended by Git doc (https://git-scm.com/docs/git-credential-store#_storage_format) # "Do not view or edit the file with editors." - yield self._dovccmd( + await self._dovccmd( ['credential', 'approve'], initial_stdin=creds, workdir=workdir, ) - @defer.inlineCallbacks - def download_auth_files_if_needed( + @async_to_deferred + async def download_auth_files_if_needed( self, workdir: str, download_wrapper_script: bool = False, - ): + ) -> int: p = Properties() p.master = self._master - private_key: str | None = yield p.render(self.ssh_private_key) - host_key: str | None = yield p.render(self.ssh_host_key) - known_hosts: str | None = yield p.render(self.ssh_known_hosts) + private_key: str | None = await p.render(self.ssh_private_key) + host_key: str | None = await p.render(self.ssh_host_key) + known_hosts: str | None = await p.render(self.ssh_known_hosts) if private_key is not None: - yield self._download_ssh_files( + await self._download_ssh_files( private_key, host_key, known_hosts, @@ -517,17 +519,18 @@ def download_auth_files_if_needed( if self.git_credential_options is not None: credentials: list[str] = [] for creds in self.git_credential_options.credentials: - rendered: str | None = yield p.render(creds) + rendered: str | None = await p.render(creds) if rendered: credentials.append(rendered) if credentials: - yield self._download_credentials(credentials, workdir) + await self._download_credentials(credentials, workdir) self.did_download_auth_files = True return RC_SUCCESS - def remove_auth_files_if_needed(self, workdir: str) -> defer.Deferred[int]: + @async_to_deferred + async def remove_auth_files_if_needed(self, workdir: str) -> int: raise NotImplementedError() @@ -598,44 +601,50 @@ def _master(self): assert isinstance(self.step, buildstep.BuildStep) and self.step.master is not None return self.step.master - @defer.inlineCallbacks - def _download_file(self, path: str, content: str, mode: int, workdir: str | None = None): + @async_to_deferred + async def _download_file( + self, + path: str, + content: str, + mode: int, + workdir: str | None = None, + ) -> None: assert isinstance(self.step, CompositeStepMixin) - yield self.step.downloadFileContentToWorker( + await self.step.downloadFileContentToWorker( path, content, mode=mode, workdir=workdir, ) - @defer.inlineCallbacks - def _dovccmd( + @async_to_deferred + async def _dovccmd( self, command: list[str], initial_stdin: str | None = None, workdir: str | None = None, - ): + ) -> None: assert isinstance(self.step, GitStepMixin) - yield self.step._dovccmd( + await self.step._dovccmd( command=command, initialStdin=initial_stdin, ) - @defer.inlineCallbacks - def download_auth_files_if_needed( + @async_to_deferred + async def download_auth_files_if_needed( self, workdir: str, download_wrapper_script: bool = False, - ): + ) -> int: if self.ssh_private_key is None and self.git_credential_options is None: return RC_SUCCESS assert isinstance(self.step, CompositeStepMixin) and isinstance(self.step, GitMixin) workdir = self._get_auth_data_path(workdir) - yield self.step.runMkdir(workdir) + await self.step.runMkdir(workdir) - return_code = yield super().download_auth_files_if_needed( + return_code = await super().download_auth_files_if_needed( workdir=workdir, download_wrapper_script=( download_wrapper_script or not self.step.supportsSshPrivateKeyAsEnvOption @@ -643,13 +652,13 @@ def download_auth_files_if_needed( ) return return_code - @defer.inlineCallbacks - def remove_auth_files_if_needed(self, workdir: str): + @async_to_deferred + async def remove_auth_files_if_needed(self, workdir: str) -> int: if not self.did_download_auth_files: return RC_SUCCESS assert isinstance(self.step, CompositeStepMixin) - yield self.step.runRmdir(self._get_auth_data_path(workdir)) + await self.step.runRmdir(self._get_auth_data_path(workdir)) return RC_SUCCESS @@ -674,21 +683,22 @@ def _master(self): assert self._service.master is not None return self._service.master - def _download_file( + @async_to_deferred + async def _download_file( self, path: str, content: str, mode: int, workdir: str | None = None, - ) -> defer.Deferred[None]: + ) -> None: writeLocalFile(path, content, mode=mode) - return defer.succeed(None) - def remove_auth_files_if_needed(self, workdir: str) -> defer.Deferred[int]: + @async_to_deferred + async def remove_auth_files_if_needed(self, workdir: str) -> int: if not self.did_download_auth_files: - return defer.succeed(RC_SUCCESS) + return RC_SUCCESS Path(self._get_ssh_private_key_path(workdir)).unlink(missing_ok=True) Path(self._get_ssh_host_key_path(workdir)).unlink(missing_ok=True) - return defer.succeed(RC_SUCCESS) + return RC_SUCCESS diff --git a/master/buildbot/util/git_credential.py b/master/buildbot/util/git_credential.py index ccbe48b9bf38..7561fbf7d320 100644 --- a/master/buildbot/util/git_credential.py +++ b/master/buildbot/util/git_credential.py @@ -17,11 +17,11 @@ from typing import NamedTuple -from twisted.internet import defer from zope.interface import implementer from buildbot.interfaces import IRenderable from buildbot.util import ComparableMixin +from buildbot.util.twisted import async_to_deferred @implementer(IRenderable) @@ -31,8 +31,8 @@ class GitCredentialInputRenderer(ComparableMixin): def __init__(self, **credential_attributes) -> None: self._credential_attributes: dict[str, IRenderable | str] = credential_attributes - @defer.inlineCallbacks - def getRenderingFor(self, build): + @async_to_deferred + async def getRenderingFor(self, build): props = build.getProperties() rendered_attributes = [] @@ -46,7 +46,7 @@ def getRenderingFor(self, build): attributes.sort(key=lambda e: e[0] != "url") for key, value in attributes: - rendered_value = yield props.render(value) + rendered_value = await props.render(value) if rendered_value is not None: rendered_attributes.append(f"{key}={rendered_value}\n")