From 21dbeccef179b2b51b71b3a988ee14a4afde271f Mon Sep 17 00:00:00 2001 From: Patrick Uiterwijk Date: Jun 02 2018 13:42:29 +0000 Subject: [PATCH 1/5] Create a "gitcred" command that functions as an OIDC git-credential helper Signed-off-by: Patrick Uiterwijk --- diff --git a/pyrpkg/__init__.py b/pyrpkg/__init__.py index 178e6df..d95ae43 100644 --- a/pyrpkg/__init__.py +++ b/pyrpkg/__init__.py @@ -42,7 +42,7 @@ from pyrpkg.errors import HashtypeMixingError, rpkgError, rpkgAuthError, \ from .gitignore import GitIgnore from pyrpkg.lookaside import CGILookasideCache from pyrpkg.sources import SourcesFile -from pyrpkg.utils import cached_property, log_result +from pyrpkg.utils import cached_property, log_result, find_me class NullHandler(logging.Handler): @@ -1266,11 +1266,10 @@ class Commands(object): self._run_command(cmd, cwd=path) - if self.clone_config: - base_module = self.get_base_module(module) - git_dir = target if target else bare_dir if bare_dir else base_module - conf_git = git.Git(os.path.join(path, git_dir)) - self._clone_config(conf_git, module) + base_module = self.get_base_module(module) + git_dir = target if target else bare_dir if bare_dir else base_module + conf_git = git.Git(os.path.join(path, git_dir)) + self._clone_config(conf_git, module) return @@ -1348,10 +1347,15 @@ class Commands(object): shutil.rmtree(repo_path, ignore_errors=True) def _clone_config(self, conf_git, module): - clone_config = self.clone_config.strip() % {'module': module} - for confline in clone_config.splitlines(): - if confline: - conf_git.config(*confline.split()) + # Inject ourselves as the git credential helper for https pushing + conf_git.config('credential.helper', ' '.join(find_me() + ['gitcred'])) + conf_git.config('credential.useHttpPath', 'true') + # Set any other clone_config + if self.clone_config: + clone_config = self.clone_config.strip() % {'module': module} + for confline in clone_config.splitlines(): + if confline: + conf_git.config(*confline.split()) def commit(self, message=None, file=None, files=[], signoff=False): """Commit changes to a module (optionally found at path) diff --git a/pyrpkg/cli.py b/pyrpkg/cli.py index 9dd61de..1d8b480 100644 --- a/pyrpkg/cli.py +++ b/pyrpkg/cli.py @@ -18,6 +18,8 @@ import getpass import logging import os import random +import requests +from requests.auth import HTTPBasicAuth import string import sys import time @@ -137,6 +139,7 @@ class cliClient(object): self.config = config self._name = name + self._oidc_client = None # Define default name in child class # self.DEFAULT_CLI_NAME = None # Property holders, set to none @@ -378,6 +381,7 @@ class cliClient(object): self.register_diff() self.register_gimmespec() self.register_gitbuildhash() + self.register_gitcred() self.register_giturl() self.register_import_srpm() self.register_install() @@ -667,6 +671,120 @@ defined, packages will be built sequentially.""" % {'name': self.name}) 'build', help='name-version-release of the build to query.') gitbuildhash_parser.set_defaults(command=self.gitbuildhash) + def register_gitcred(self): + """Register the (hidden) gitcred target + + These commands implement the git-credential helper API, so that we are + able to provide OpenID Connect/OAuth2 tokens if requested for https + based pushing. + """ + + gitcred_parser = self.subparsers.add_parser( + 'gitcred') + cred_subs = gitcred_parser.add_subparsers() + get_parser = cred_subs.add_parser('get') + get_parser.set_defaults(command=self.gitcred_get) + store_parser = cred_subs.add_parser('store') + store_parser.set_defaults(command=self.gitcred_store) + erase_parser = cred_subs.add_parser('erase') + erase_parser.set_defaults(command=self.gitcred_erase) + + def gitcred_store(self): + """Nothing to do here""" + pass + + def _gitcred_check_input(self): + inp = {} + for line in sys.stdin: + vals = line.split('=', 2) + if len(vals) != 2: + print('Invalid input: %s' % line, file=sys.stderr) + return False + key, val = vals + inp[key] = val.strip() + + if inp['protocol'] != 'https': + # Nothing to do for us here + return False + return inp + + def _gitcred_return(self, args): + for arg in args: + print('%s=%s' % (arg, args[arg])) + print('') + + @property + def oidc_client(self): + if self._oidc_client: + return self._oidc_client + + import openidc_client + for opt in ['oidc_id_provider', 'oidc_client_id', 'oidc_client_secret', + 'oidc_scopes']: + if not self.config.has_option(self.name, opt): + print('OpenID Connect param %s not configured' % opt, + file=sys.stderr) + return None + + self._oidc_client = openidc_client.OpenIDCClient( + self.name, + self.config.get(self.name, 'oidc_id_provider'), + {'Token': 'Token', 'Authorization': 'Authorization'}, + self.config.get(self.name, 'oidc_client_id'), + self.config.get(self.name, 'oidc_client_secret'), + printfd=sys.stderr) + return self._oidc_client + + @property + def _oidc_scopes(self): + return self.config.get(self.name, 'oidc_scopes').split(',') + + def _oidc_token(self, **kwargs): + client = self.oidc_client + if not client: + return None + return client.get_token(self._oidc_scopes, **kwargs) + + def _gitcred_test(self, args): + # 'path' is provided if git config credential.useHttpPath is true + if 'path' in args: + # Without "path", we can't really test... + url = '%(protocol)s://%(host)s/%(path)s/info/refs?service=git-receive-pack' % args + resp = requests.head(url, auth=HTTPBasicAuth(args['username'], + args['password'])) + if resp.status_code == 401: + return self.oidc_client.report_token_issue() + + def gitcred_get(self): + args = self._gitcred_check_input() + if not args: + return + token = self._oidc_token() + args['quit'] = '1' + if not token: + self._gitcred_return(args) + print('No token received. OpenIDC configured?', file=sys.stderr) + return + args['username'] = '-openidc-' + args['password'] = token + newtoken = self._gitcred_test(args) + if newtoken is not None: + args['password'] = newtoken + self._gitcred_return(args) + + def gitcred_erase(self): + args = self._gitcred_check_input() + if not args: + return + token = self._oidc_token(new_token=False) + if token and token == args.get('password'): + newtoken = self.oidc_client.report_token_issue() + if newtoken is None: + print('Issue with your token renewal', file=sys.stderr) + else: + print('Token was renewed. Please rerun command', + file=sys.stderr) + def register_giturl(self): """Register the giturl target""" diff --git a/pyrpkg/utils.py b/pyrpkg/utils.py index 2313ef0..ab78cf7 100644 --- a/pyrpkg/utils.py +++ b/pyrpkg/utils.py @@ -13,6 +13,7 @@ This module contains a bunch of utilities used elsewhere in pyrpkg. """ +import argparse import os import six import sys @@ -89,3 +90,19 @@ def log_result(log_func, result, level=0, indent=2): log_result(log_func, value, level+1) else: _log_value(log_func, result, level, indent) + + +def find_me(): + """Find the way to call the same binary/config as is being called now""" + parser = argparse.ArgumentParser(add_help=False) + parser.add_argument( + '-C', '--config', help='Specify a config file to use') + (args, other) = parser.parse_known_args() + + binary = os.path.abspath(sys.argv[0]) + + cmd = [binary] + if args.config: + cmd += ['--config', args.config] + + return cmd diff --git a/tests/commands/__init__.py b/tests/commands/__init__.py index 388d2e0..46cbdef 100644 --- a/tests/commands/__init__.py +++ b/tests/commands/__init__.py @@ -3,7 +3,11 @@ import shutil import subprocess import sys import tempfile -import unittest +# For running tests with Python 2.6 +try: + import unittest2 as unittest +except ImportError: + import unittest class CommandTestCase(unittest.TestCase): diff --git a/tests/commands/test_clone.py b/tests/commands/test_clone.py index 2109e37..f4daff0 100644 --- a/tests/commands/test_clone.py +++ b/tests/commands/test_clone.py @@ -29,6 +29,7 @@ class CommandCloneTestCase(CommandTestCase): moduledir = os.path.join(self.path, self.module) self.assertTrue(os.path.isdir(os.path.join(moduledir, '.git'))) confgit = git.Git(moduledir) + self.assertIn('gitcred', confgit.config('credential.helper')) self.assertEqual(confgit.config('bz.default-component'), self.module) self.assertEqual(confgit.config('sendemail.to'), "%s-owner@fedoraproject.org" % self.module) From a28701257a282e6a57888710b1fe24765c3fdeb8 Mon Sep 17 00:00:00 2001 From: Patrick Uiterwijk Date: Jun 02 2018 13:42:29 +0000 Subject: [PATCH 2/5] Also inject the credential helper with rpkg push Signed-off-by: Patrick Uiterwijk --- diff --git a/pyrpkg/__init__.py b/pyrpkg/__init__.py index d95ae43..2640ce2 100644 --- a/pyrpkg/__init__.py +++ b/pyrpkg/__init__.py @@ -1716,7 +1716,7 @@ class Commands(object): return patches_not_tracked - def push(self, force=False): + def push(self, force=False, extra_config=None): """Push changes to the remote repository""" self.check_repo(is_dirty=False, all_pushed=False) @@ -1733,7 +1733,11 @@ class Commands(object): ', '.join(untracked_patches), 'is' if len(untracked_patches) == 1 else 'are') - cmd = ['git', 'push'] + cmd = ['git'] + if extra_config: + for opt in extra_config: + cmd += ['-c', '%s=%s' % (opt, extra_config[opt])] + cmd.append('push') if self.quiet: cmd.append('-q') self._run_command(cmd, cwd=self.path) diff --git a/pyrpkg/cli.py b/pyrpkg/cli.py index 1d8b480..4432138 100644 --- a/pyrpkg/cli.py +++ b/pyrpkg/cli.py @@ -1925,7 +1925,10 @@ see API KEY section of copr-cli(1) man page. norebase=self.args.no_rebase) def push(self): - self.cmd.push(getattr(self.args, 'force', False)) + extra_config = { + 'credential.helper': ' '.join(utils.find_me() + ['gitcred']), + 'credential.useHttpPath': 'true'} + self.cmd.push(getattr(self.args, 'force', False), extra_config) def scratch_build(self): # A scratch build is just a build with --scratch From 676aa33cc8654620e97f00d8f59034350b916753 Mon Sep 17 00:00:00 2001 From: Patrick Uiterwijk Date: Jun 05 2018 04:49:09 +0000 Subject: [PATCH 3/5] Add docblocks to gitcred methods and don't quit if OpenIDC is unconfigured Signed-off-by: Patrick Uiterwijk --- diff --git a/pyrpkg/cli.py b/pyrpkg/cli.py index 4432138..5a10389 100644 --- a/pyrpkg/cli.py +++ b/pyrpkg/cli.py @@ -690,10 +690,16 @@ defined, packages will be built sequentially.""" % {'name': self.name}) erase_parser.set_defaults(command=self.gitcred_erase) def gitcred_store(self): - """Nothing to do here""" + """Nothing to do here. + + If we returned a token, that's already stored. If the user manually + entered a password, we do not want to store it. + Thus we present to you, a no-op. + """ pass def _gitcred_check_input(self): + """Parses the git-credential-helper IO format input.""" inp = {} for line in sys.stdin: vals = line.split('=', 2) @@ -709,16 +715,21 @@ defined, packages will be built sequentially.""" % {'name': self.name}) return inp def _gitcred_return(self, args): + """Returns git-credential-helper IO format output.""" for arg in args: print('%s=%s' % (arg, args[arg])) print('') @property def oidc_client(self): + """Returns a OpenID Connect client reference. + + Returns: (openidc_client.OpenIDCClient or None): client if configured, + None if unconfigured. + """ if self._oidc_client: return self._oidc_client - import openidc_client for opt in ['oidc_id_provider', 'oidc_client_id', 'oidc_client_secret', 'oidc_scopes']: if not self.config.has_option(self.name, opt): @@ -726,6 +737,7 @@ defined, packages will be built sequentially.""" % {'name': self.name}) file=sys.stderr) return None + import openidc_client self._oidc_client = openidc_client.OpenIDCClient( self.name, self.config.get(self.name, 'oidc_id_provider'), @@ -737,15 +749,27 @@ defined, packages will be built sequentially.""" % {'name': self.name}) @property def _oidc_scopes(self): + """Returns the configured OIDC scopes to request.""" return self.config.get(self.name, 'oidc_scopes').split(',') def _oidc_token(self, **kwargs): + """Returns an OpenID Connect token via the global client. + + Returns: (string or bool or None): Returns a string token, None if the + client did not return a token, or False if the client was not configured. + """ client = self.oidc_client if not client: - return None + return False return client.get_token(self._oidc_scopes, **kwargs) def _gitcred_test(self, args): + """Test the token we are about to return for freshness. + + Note that this is a best-effort, and it could be that we return scucess + but the actual push fails, in which case git will call the erase method, + and we tell the user to retry. + """ # 'path' is provided if git config credential.useHttpPath is true if 'path' in args: # Without "path", we can't really test... @@ -756,14 +780,24 @@ defined, packages will be built sequentially.""" % {'name': self.name}) return self.oidc_client.report_token_issue() def gitcred_get(self): + """Performs the git-credential-helper get operation.""" args = self._gitcred_check_input() if not args: return token = self._oidc_token() + if token is False: + # This happens if OpenID Connect was unconfigured. Don't tell git + # to quit, so that users get a fighting chance to enter a password + # by hand. + self._gitcred_return(args) + return + # If, however, we are configured to use OpenID Connect, and we just + # didn't get a (valid) token, tell git to not ask the user, since they + # won't be able to manually provide a valid token. args['quit'] = '1' if not token: self._gitcred_return(args) - print('No token received. OpenIDC configured?', file=sys.stderr) + print('No token received.', file=sys.stderr) return args['username'] = '-openidc-' args['password'] = token @@ -773,6 +807,7 @@ defined, packages will be built sequentially.""" % {'name': self.name}) self._gitcred_return(args) def gitcred_erase(self): + """Performs the git-credential-helper erase operation.""" args = self._gitcred_check_input() if not args: return From f8dccb91b7febf08a51a5c78b5f3c246c0be10e6 Mon Sep 17 00:00:00 2001 From: Patrick Uiterwijk Date: Jun 06 2018 11:44:24 +0000 Subject: [PATCH 4/5] Don't inject the credential helper to push if OIDC is unconfigured Signed-off-by: Patrick Uiterwijk --- diff --git a/pyrpkg/cli.py b/pyrpkg/cli.py index 5a10389..8916248 100644 --- a/pyrpkg/cli.py +++ b/pyrpkg/cli.py @@ -721,6 +721,16 @@ defined, packages will be built sequentially.""" % {'name': self.name}) print('') @property + def oidc_configured(self): + """Returns a boolean indicating whether OIDC is configured.""" + for opt in ['oidc_id_provider', 'oidc_client_id', 'oidc_client_secret', + 'oidc_scopes']: + if not self.config.has_option(self.name, opt): + return False + + return True + + @property def oidc_client(self): """Returns a OpenID Connect client reference. @@ -730,12 +740,9 @@ defined, packages will be built sequentially.""" % {'name': self.name}) if self._oidc_client: return self._oidc_client - for opt in ['oidc_id_provider', 'oidc_client_id', 'oidc_client_secret', - 'oidc_scopes']: - if not self.config.has_option(self.name, opt): - print('OpenID Connect param %s not configured' % opt, - file=sys.stderr) - return None + if not self.oidc_configured: + print('OpenID Connect not configured', file=sys.stderr) + return None import openidc_client self._oidc_client = openidc_client.OpenIDCClient( @@ -1960,9 +1967,12 @@ see API KEY section of copr-cli(1) man page. norebase=self.args.no_rebase) def push(self): - extra_config = { - 'credential.helper': ' '.join(utils.find_me() + ['gitcred']), - 'credential.useHttpPath': 'true'} + if not self.oidc_configured: + extra_config = {} + else: + extra_config = { + 'credential.helper': ' '.join(utils.find_me() + ['gitcred']), + 'credential.useHttpPath': 'true'} self.cmd.push(getattr(self.args, 'force', False), extra_config) def scratch_build(self): From 907705c7050937901141e5c6704e816623c8890a Mon Sep 17 00:00:00 2001 From: Patrick Uiterwijk Date: Jun 07 2018 05:05:26 +0000 Subject: [PATCH 5/5] Make sure gitcred doesn't land in man Signed-off-by: Patrick Uiterwijk --- diff --git a/pyrpkg/cli.py b/pyrpkg/cli.py index 8916248..ac0d8fc 100644 --- a/pyrpkg/cli.py +++ b/pyrpkg/cli.py @@ -680,7 +680,8 @@ defined, packages will be built sequentially.""" % {'name': self.name}) """ gitcred_parser = self.subparsers.add_parser( - 'gitcred') + 'gitcred', + add_help=False) cred_subs = gitcred_parser.add_subparsers() get_parser = cred_subs.add_parser('get') get_parser.set_defaults(command=self.gitcred_get)