From e4c525ecbf30f3708987105c5d967f927f35eac5 Mon Sep 17 00:00:00 2001 From: Haibo Lin Date: Mon, 26 Jun 2023 13:13:08 +0800 Subject: [PATCH] Support OIDC Client Credentials authentication to CTS JIRA: RHELCMP-11324 Signed-off-by: Haibo Lin --- doc/configuration.rst | 11 +++++ pungi/checks.py | 2 + pungi/compose.py | 95 +++++++++++++++++++++++++++---------- pungi/scripts/pungi_koji.py | 2 +- tests/test_compose.py | 15 ++++-- 5 files changed, 93 insertions(+), 32 deletions(-) diff --git a/doc/configuration.rst b/doc/configuration.rst index 80c0c962..3912e475 100644 --- a/doc/configuration.rst +++ b/doc/configuration.rst @@ -194,6 +194,17 @@ Options Tracking Service Kerberos authentication. If not defined, the default Kerberos principal is used. +**cts_oidc_token_url** + (*str*) -- URL to the OIDC token endpoint. + For example ``https://oidc.example.com/openid-connect/token``. + This option can be overridden by the environment variable ``CTS_OIDC_TOKEN_URL``. + +**cts_oidc_client_id* + (*str*) -- OIDC client ID. + This option can be overridden by the environment variable ``CTS_OIDC_CLIENT_ID``. + Note that environment variable ``CTS_OIDC_CLIENT_SECRET`` must be configured with + corresponding client secret to authenticate to CTS via OIDC. + **compose_type** (*str*) -- Allows to set default compose type. Type set via a command-line option overwrites this. diff --git a/pungi/checks.py b/pungi/checks.py index 0c607cd2..6fd0f968 100644 --- a/pungi/checks.py +++ b/pungi/checks.py @@ -824,6 +824,8 @@ def make_schema(): "pdc_insecure": {"deprecated": "Koji is queried instead"}, "cts_url": {"type": "string"}, "cts_keytab": {"type": "string"}, + "cts_oidc_token_url": {"type": "url"}, + "cts_oidc_client_id": {"type": "string"}, "koji_profile": {"type": "string"}, "koji_event": {"type": "number"}, "pkgset_koji_tag": {"$ref": "#/definitions/strings"}, diff --git a/pungi/compose.py b/pungi/compose.py index ab1d4794..e980a9fd 100644 --- a/pungi/compose.py +++ b/pungi/compose.py @@ -70,39 +70,82 @@ def is_status_fatal(status_code): @retry(wait_on=RequestException) -def retry_request(method, url, data=None, auth=None): +def retry_request(method, url, data=None, json_data=None, auth=None): + """ + :param str method: Reqest method. + :param str url: Target URL. + :param dict data: form-urlencoded data to send in the body of the request. + :param dict json_data: json data to send in the body of the request. + """ request_method = getattr(requests, method) - rv = request_method(url, json=data, auth=auth) + rv = request_method(url, data=data, json=json_data, auth=auth) if is_status_fatal(rv.status_code): try: - error = rv.json()["message"] + error = rv.json() except ValueError: error = rv.text - raise RuntimeError("CTS responded with %d: %s" % (rv.status_code, error)) + raise RuntimeError("%s responded with %d: %s" % (url, rv.status_code, error)) rv.raise_for_status() return rv -@contextlib.contextmanager -def cts_auth(cts_keytab): - auth = None - if cts_keytab: - # requests-kerberos cannot accept custom keytab, we need to use - # environment variable for this. But we need to change environment - # only temporarily just for this single requests.post. - # So at first backup the current environment and revert to it - # after the requests call. - from requests_kerberos import HTTPKerberosAuth +class BearerAuth(requests.auth.AuthBase): + def __init__(self, token): + self.token = token - auth = HTTPKerberosAuth() - environ_copy = dict(os.environ) - if "$HOSTNAME" in cts_keytab: - cts_keytab = cts_keytab.replace("$HOSTNAME", socket.gethostname()) - os.environ["KRB5_CLIENT_KTNAME"] = cts_keytab - os.environ["KRB5CCNAME"] = "DIR:%s" % tempfile.mkdtemp() + def __call__(self, r): + r.headers["authorization"] = "Bearer " + self.token + return r + + +@contextlib.contextmanager +def cts_auth(pungi_conf): + """ + :param dict pungi_conf: dict obj of pungi.json config. + """ + auth = None + token = None + cts_keytab = pungi_conf.get("cts_keytab") + cts_oidc_token_url = os.environ.get("CTS_OIDC_TOKEN_URL", "") or pungi_conf.get( + "cts_oidc_token_url" + ) try: + if cts_keytab: + # requests-kerberos cannot accept custom keytab, we need to use + # environment variable for this. But we need to change environment + # only temporarily just for this single requests.post. + # So at first backup the current environment and revert to it + # after the requests call. + from requests_kerberos import HTTPKerberosAuth + + auth = HTTPKerberosAuth() + environ_copy = dict(os.environ) + if "$HOSTNAME" in cts_keytab: + cts_keytab = cts_keytab.replace("$HOSTNAME", socket.gethostname()) + os.environ["KRB5_CLIENT_KTNAME"] = cts_keytab + os.environ["KRB5CCNAME"] = "DIR:%s" % tempfile.mkdtemp() + elif cts_oidc_token_url: + cts_oidc_client_id = os.environ.get( + "CTS_OIDC_CLIENT_ID", "" + ) or pungi_conf.get("cts_oidc_client_id", "") + token = retry_request( + "post", + cts_oidc_token_url, + data={ + "grant_type": "client_credentials", + "client_id": cts_oidc_client_id, + "client_secret": os.environ.get("CTS_OIDC_CLIENT_SECRET", ""), + }, + ).json()["access_token"] + auth = BearerAuth(token) + del token + yield auth + except Exception as e: + # Avoid leaking client secret in trackback + e.show_locals = False + raise e finally: if cts_keytab: shutil.rmtree(os.environ["KRB5CCNAME"].split(":", 1)[1]) @@ -150,8 +193,8 @@ def get_compose_info( "parent_compose_ids": parent_compose_ids, "respin_of": respin_of, } - with cts_auth(conf.get("cts_keytab")) as authentication: - rv = retry_request("post", url, data=data, auth=authentication) + with cts_auth(conf) as authentication: + rv = retry_request("post", url, json_data=data, auth=authentication) # Update local ComposeInfo with received ComposeInfo. cts_ci = ComposeInfo() @@ -187,8 +230,8 @@ def update_compose_url(compose_id, compose_dir, conf): "action": "set_url", "compose_url": compose_url, } - with cts_auth(conf.get("cts_keytab")) as authentication: - return retry_request("patch", url, data=data, auth=authentication) + with cts_auth(conf) as authentication: + return retry_request("patch", url, json_data=data, auth=authentication) def get_compose_dir( @@ -661,7 +704,7 @@ class Compose(kobo.log.LoggingBase): separators=(",", ": "), ) - def traceback(self, detail=None): + def traceback(self, detail=None, show_locals=True): """Store an extended traceback. This method should only be called when handling an exception. @@ -673,7 +716,7 @@ class Compose(kobo.log.LoggingBase): tb_path = self.paths.log.log_file("global", basename) self.log_error("Extended traceback in: %s", tb_path) with open(tb_path, "wb") as f: - f.write(kobo.tback.Traceback().get_traceback()) + f.write(kobo.tback.Traceback(show_locals=show_locals).get_traceback()) def load_old_compose_config(self): """ diff --git a/pungi/scripts/pungi_koji.py b/pungi/scripts/pungi_koji.py index 5281852f..63deeaa5 100644 --- a/pungi/scripts/pungi_koji.py +++ b/pungi/scripts/pungi_koji.py @@ -684,7 +684,7 @@ def cli_main(): except (Exception, KeyboardInterrupt) as ex: if COMPOSE: COMPOSE.log_error("Compose run failed: %s" % ex) - COMPOSE.traceback() + COMPOSE.traceback(show_locals=getattr(ex, "show_locals", True)) COMPOSE.log_critical("Compose failed: %s" % COMPOSE.topdir) COMPOSE.write_status("DOOMED") else: diff --git a/tests/test_compose.py b/tests/test_compose.py index 493c1127..246acc65 100644 --- a/tests/test_compose.py +++ b/tests/test_compose.py @@ -656,6 +656,7 @@ class ComposeTestCase(unittest.TestCase): mocked_requests.post.assert_called_once_with( "https://cts.localhost.tld/api/1/composes/", auth=mock.ANY, + data=None, json=expected_json, ) @@ -794,12 +795,16 @@ class TracebackTest(unittest.TestCase): shutil.rmtree(self.tmp_dir) self.patcher.stop() - def assertTraceback(self, filename): + def assertTraceback(self, filename, show_locals=True): self.assertTrue( os.path.isfile("%s/logs/global/%s.global.log" % (self.tmp_dir, filename)) ) self.assertEqual( - self.Traceback.mock_calls, [mock.call(), mock.call().get_traceback()] + self.Traceback.mock_calls, + [ + mock.call(show_locals=show_locals), + mock.call(show_locals=show_locals).get_traceback(), + ], ) def test_traceback_default(self): @@ -824,8 +829,8 @@ class RetryRequestTest(unittest.TestCase): self.assertEqual( mocked_requests.mock_calls, [ - mock.call.post(url, json=None, auth=None), - mock.call.post(url, json=None, auth=None), + mock.call.post(url, data=None, json=None, auth=None), + mock.call.post(url, data=None, json=None, auth=None), ], ) self.assertEqual(rv.status_code, 200) @@ -841,5 +846,5 @@ class RetryRequestTest(unittest.TestCase): self.assertEqual( mocked_requests.mock_calls, - [mock.call.post(url, json=None, auth=None)], + [mock.call.post(url, data=None, json=None, auth=None)], )