From 8ecf55926f8e345960560e5c5dd6716199cfb0ec Mon Sep 17 00:00:00 2001 From: "Kay-Uwe (Kiwi) Lorenz" Date: Sat, 6 Mar 2021 12:09:20 +0100 Subject: [PATCH 01/12] feat: option to add a helper to lookup token --- docs/cli-usage.rst | 2 +- gitlab/config.py | 11 +++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/docs/cli-usage.rst b/docs/cli-usage.rst index 1c24824c8..7b1425775 100644 --- a/docs/cli-usage.rst +++ b/docs/cli-usage.rst @@ -48,7 +48,7 @@ example: [elsewhere] url = http://else.whe.re:8080 - private_token = CkqsjqcQSFH5FQKDccu4 + private_token = lookup: pass show path/to/password timeout = 1 The ``default`` option of the ``[global]`` section defines the GitLab server to diff --git a/gitlab/config.py b/gitlab/config.py index 710a35459..a6f25ace2 100644 --- a/gitlab/config.py +++ b/gitlab/config.py @@ -17,6 +17,7 @@ import os import configparser +import subprocess from typing import List, Optional, Union from gitlab.const import USER_AGENT @@ -150,6 +151,16 @@ def __init__( except Exception: pass + for attr in ("job_token", "http_password", "private_token", "oauth_token"): + value = getattr(self, attr) + prefix = "lookup:" + if isinstance(value, str) and value.lower().strip().startswith(prefix): + helper = value[len(prefix) :].strip() + value = ( + subprocess.check_output(helper, shell=True).decode("utf-8").strip() + ) + setattr(self, attr, value) + self.api_version = "4" try: self.api_version = self._config.get("global", "api_version") From 9ef83118efde3d0f35d73812ce8398be2c18ebff Mon Sep 17 00:00:00 2001 From: "Kay-Uwe (Kiwi) Lorenz" Date: Sat, 6 Mar 2021 12:12:59 +0100 Subject: [PATCH 02/12] fix: better real life token lookup example --- docs/cli-usage.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/cli-usage.rst b/docs/cli-usage.rst index 7b1425775..71c8577b5 100644 --- a/docs/cli-usage.rst +++ b/docs/cli-usage.rst @@ -48,7 +48,7 @@ example: [elsewhere] url = http://else.whe.re:8080 - private_token = lookup: pass show path/to/password + private_token = lookup: pass show path/to/password | head -n1 timeout = 1 The ``default`` option of the ``[global]`` section defines the GitLab server to From f8cf1e110401dcc6b9b176beb8675513fc1c7d17 Mon Sep 17 00:00:00 2001 From: "Kay-Uwe (Kiwi) Lorenz" Date: Sat, 6 Mar 2021 12:22:33 +0100 Subject: [PATCH 03/12] chore: add test --- gitlab/tests/test_config.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/gitlab/tests/test_config.py b/gitlab/tests/test_config.py index 7a9e23954..acef00773 100644 --- a/gitlab/tests/test_config.py +++ b/gitlab/tests/test_config.py @@ -51,6 +51,10 @@ [four] url = https://four.url oauth_token = STUV + +[five] +url = https://five.url +oauth_token = lookup: echo "foobar" """ custom_user_agent_config = """[global] @@ -192,6 +196,13 @@ def test_valid_data(m_open, path_exists): assert 2 == cp.timeout assert True == cp.ssl_verify + cp = config.GitlabConfigParser(gitlab_id="five") + assert "five" == cp.gitlab_id + assert "https://five.url" == cp.url + assert None == cp.private_token + assert "foobar" == cp.oauth_token + assert 2 == cp.timeout + assert True == cp.ssl_verify @mock.patch("os.path.exists") @mock.patch("builtins.open") From b04dd2c08b69619bb58832f40a4c4391e350a735 Mon Sep 17 00:00:00 2001 From: "Kay-Uwe (Kiwi) Lorenz" Date: Sat, 6 Mar 2021 12:25:39 +0100 Subject: [PATCH 04/12] fix: linting issues and test --- gitlab/tests/test_config.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/gitlab/tests/test_config.py b/gitlab/tests/test_config.py index acef00773..644b0c1c0 100644 --- a/gitlab/tests/test_config.py +++ b/gitlab/tests/test_config.py @@ -196,6 +196,9 @@ def test_valid_data(m_open, path_exists): assert 2 == cp.timeout assert True == cp.ssl_verify + fd = io.StringIO(valid_config) + fd.close = mock.Mock(return_value=None) + m_open.return_value = fd cp = config.GitlabConfigParser(gitlab_id="five") assert "five" == cp.gitlab_id assert "https://five.url" == cp.url @@ -204,6 +207,7 @@ def test_valid_data(m_open, path_exists): assert 2 == cp.timeout assert True == cp.ssl_verify + @mock.patch("os.path.exists") @mock.patch("builtins.open") @pytest.mark.parametrize( From fc2798fc31a08997c049f609c19dd4ab8d75964e Mon Sep 17 00:00:00 2001 From: "Kay-Uwe (Kiwi) Lorenz" Date: Sun, 7 Mar 2021 15:13:52 +0100 Subject: [PATCH 05/12] fix: make secret helper more user friendly --- docs/cli-usage.rst | 23 ++++++++++++++++++++++- gitlab/config.py | 25 ++++++++++++++++--------- gitlab/tests/test_config.py | 36 +++++++++++++++++++++++++----------- 3 files changed, 63 insertions(+), 21 deletions(-) diff --git a/docs/cli-usage.rst b/docs/cli-usage.rst index 71c8577b5..c27e6c57a 100644 --- a/docs/cli-usage.rst +++ b/docs/cli-usage.rst @@ -48,7 +48,7 @@ example: [elsewhere] url = http://else.whe.re:8080 - private_token = lookup: pass show path/to/password | head -n1 + private_token = helper: path/to/helper.sh timeout = 1 The ``default`` option of the ``[global]`` section defines the GitLab server to @@ -119,6 +119,27 @@ server, with very limited permissions. * - ``http_password`` - Password for optional HTTP authentication +For all settings, which contain secrets (``http_password``, +``personal_token``, ``oauth_token``, ``job_token``), you can specify +a helper program to retrieve the secret indicated by ``helper:`` +prefix. You can only specify a path to a program without any +parameters. It is expected, that the program prints the secret to +standard output. + +Example for a `keyring `_ helper: + +.. code-block:: bash + + #!/bin/bash + keyring get Service Username + +Example for a `pass `_ helper: + +.. code-block:: bash + + #!/bin/bash + pass show path/to/password | head -n 1 + CLI === diff --git a/gitlab/config.py b/gitlab/config.py index a6f25ace2..67f508229 100644 --- a/gitlab/config.py +++ b/gitlab/config.py @@ -34,6 +34,11 @@ def _env_config() -> List[str]: os.path.expanduser("~/.python-gitlab.cfg"), ] +HELPER_PREFIX = "helper:" + +HELPER_ATTRIBUTES = [ + "job_token", "http_password", "private_token", "oauth_token" +] class ConfigError(Exception): pass @@ -151,15 +156,7 @@ def __init__( except Exception: pass - for attr in ("job_token", "http_password", "private_token", "oauth_token"): - value = getattr(self, attr) - prefix = "lookup:" - if isinstance(value, str) and value.lower().strip().startswith(prefix): - helper = value[len(prefix) :].strip() - value = ( - subprocess.check_output(helper, shell=True).decode("utf-8").strip() - ) - setattr(self, attr, value) + self._get_values_from_helper() self.api_version = "4" try: @@ -203,3 +200,13 @@ def __init__( self.user_agent = self._config.get(self.gitlab_id, "user_agent") except Exception: pass + + def _get_values_from_helper(self): + """Update attributes, which may get values from an external helper program + """ + for attr in HELPER_ATTRIBUTES: + value = getattr(self, attr) + if isinstance(value, str) and value.lower().strip().startswith(HELPER_PREFIX): + helper = value[len(HELPER_PREFIX) :].strip() + value = subprocess.check_output([helper]).decode("utf-8").strip() + setattr(self, attr, value) \ No newline at end of file diff --git a/gitlab/tests/test_config.py b/gitlab/tests/test_config.py index 644b0c1c0..60c88539d 100644 --- a/gitlab/tests/test_config.py +++ b/gitlab/tests/test_config.py @@ -17,6 +17,7 @@ import os import unittest +from textwrap import dedent import mock import io @@ -51,10 +52,6 @@ [four] url = https://four.url oauth_token = STUV - -[five] -url = https://five.url -oauth_token = lookup: echo "foobar" """ custom_user_agent_config = """[global] @@ -196,16 +193,33 @@ def test_valid_data(m_open, path_exists): assert 2 == cp.timeout assert True == cp.ssl_verify - fd = io.StringIO(valid_config) + +@mock.patch("os.path.exists") +@mock.patch("builtins.open") +def test_data_from_helper(m_open, path_exists, tmp_path): + helper = (tmp_path / "helper.sh") + helper.write_text(dedent("""\ + #!/bin/sh + echo "secret" + """)) + helper.chmod(0o755) + + fd = io.StringIO(dedent("""\ + [global] + default = helper + + [helper] + url = https://helper.url + oauth_token = helper: %s + """) % helper) + fd.close = mock.Mock(return_value=None) m_open.return_value = fd - cp = config.GitlabConfigParser(gitlab_id="five") - assert "five" == cp.gitlab_id - assert "https://five.url" == cp.url + cp = config.GitlabConfigParser(gitlab_id="helper") + assert "helper" == cp.gitlab_id + assert "https://helper.url" == cp.url assert None == cp.private_token - assert "foobar" == cp.oauth_token - assert 2 == cp.timeout - assert True == cp.ssl_verify + assert "secret" == cp.oauth_token @mock.patch("os.path.exists") From 732e49c6547c181de8cc56e93b30dc399e87091d Mon Sep 17 00:00:00 2001 From: "Kay-Uwe (Kiwi) Lorenz" Date: Sun, 7 Mar 2021 15:28:52 +0100 Subject: [PATCH 06/12] chore: make lint happy --- gitlab/tests/test_config.py | 35 ++++++++++++++++++++++------------- 1 file changed, 22 insertions(+), 13 deletions(-) diff --git a/gitlab/tests/test_config.py b/gitlab/tests/test_config.py index 60c88539d..58ccbb0cc 100644 --- a/gitlab/tests/test_config.py +++ b/gitlab/tests/test_config.py @@ -197,21 +197,30 @@ def test_valid_data(m_open, path_exists): @mock.patch("os.path.exists") @mock.patch("builtins.open") def test_data_from_helper(m_open, path_exists, tmp_path): - helper = (tmp_path / "helper.sh") - helper.write_text(dedent("""\ - #!/bin/sh - echo "secret" - """)) + helper = tmp_path / "helper.sh" + helper.write_text( + dedent( + """\ + #!/bin/sh + echo "secret" + """ + ) + ) helper.chmod(0o755) - fd = io.StringIO(dedent("""\ - [global] - default = helper - - [helper] - url = https://helper.url - oauth_token = helper: %s - """) % helper) + fd = io.StringIO( + dedent( + """\ + [global] + default = helper + + [helper] + url = https://helper.url + oauth_token = helper: %s + """ + ) + % helper + ) fd.close = mock.Mock(return_value=None) m_open.return_value = fd From b5f43c83b25271f7aff917a9ce8826d39ff94034 Mon Sep 17 00:00:00 2001 From: "Kay-Uwe (Kiwi) Lorenz" Date: Sun, 7 Mar 2021 15:39:19 +0100 Subject: [PATCH 07/12] chore: make lint happy --- gitlab/config.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/gitlab/config.py b/gitlab/config.py index 67f508229..09732bb40 100644 --- a/gitlab/config.py +++ b/gitlab/config.py @@ -36,9 +36,7 @@ def _env_config() -> List[str]: HELPER_PREFIX = "helper:" -HELPER_ATTRIBUTES = [ - "job_token", "http_password", "private_token", "oauth_token" -] +HELPER_ATTRIBUTES = ["job_token", "http_password", "private_token", "oauth_token"] class ConfigError(Exception): pass @@ -202,11 +200,11 @@ def __init__( pass def _get_values_from_helper(self): - """Update attributes, which may get values from an external helper program - """ + """Update attributes, which may get values from an external helper program""" for attr in HELPER_ATTRIBUTES: value = getattr(self, attr) - if isinstance(value, str) and value.lower().strip().startswith(HELPER_PREFIX): + _value_lower = value.lower().strip() + if isinstance(value, str) and _value_lower.startswith(HELPER_PREFIX): helper = value[len(HELPER_PREFIX) :].strip() value = subprocess.check_output([helper]).decode("utf-8").strip() setattr(self, attr, value) \ No newline at end of file From fc7387a0a6039bc58b2a741ac9b73d7068375be7 Mon Sep 17 00:00:00 2001 From: "Kay-Uwe (Kiwi) Lorenz" Date: Sun, 7 Mar 2021 15:51:46 +0100 Subject: [PATCH 08/12] fix: let the homedir be expanded in path of helper --- gitlab/config.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/gitlab/config.py b/gitlab/config.py index 09732bb40..d2cdbfd4d 100644 --- a/gitlab/config.py +++ b/gitlab/config.py @@ -19,6 +19,7 @@ import configparser import subprocess from typing import List, Optional, Union +from os.path import expanduser from gitlab.const import USER_AGENT @@ -205,6 +206,6 @@ def _get_values_from_helper(self): value = getattr(self, attr) _value_lower = value.lower().strip() if isinstance(value, str) and _value_lower.startswith(HELPER_PREFIX): - helper = value[len(HELPER_PREFIX) :].strip() + helper = expanduser(value[len(HELPER_PREFIX) :].strip()) value = subprocess.check_output([helper]).decode("utf-8").strip() setattr(self, attr, value) \ No newline at end of file From 3ac6fa12b37dd33610ef2206ef4ddc3b20d9fd3f Mon Sep 17 00:00:00 2001 From: "Kay-Uwe (Kiwi) Lorenz" Date: Sun, 7 Mar 2021 15:53:44 +0100 Subject: [PATCH 09/12] fix: update doc for token helper --- docs/cli-usage.rst | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/docs/cli-usage.rst b/docs/cli-usage.rst index c27e6c57a..666819772 100644 --- a/docs/cli-usage.rst +++ b/docs/cli-usage.rst @@ -123,8 +123,9 @@ For all settings, which contain secrets (``http_password``, ``personal_token``, ``oauth_token``, ``job_token``), you can specify a helper program to retrieve the secret indicated by ``helper:`` prefix. You can only specify a path to a program without any -parameters. It is expected, that the program prints the secret to -standard output. +parameters. You may use ``~`` for expanding your homedir in helper +program's path. It is expected, that the program prints the secret +to standard output. Example for a `keyring `_ helper: From 9dfb4cd97e6eb5bbfc29935cbb190b70b739cf9f Mon Sep 17 00:00:00 2001 From: "Kay-Uwe (Kiwi) Lorenz" Date: Sun, 7 Mar 2021 15:56:36 +0100 Subject: [PATCH 10/12] fix: handling config value in _get_values_from_helper --- gitlab/config.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/gitlab/config.py b/gitlab/config.py index d2cdbfd4d..f3fea4f22 100644 --- a/gitlab/config.py +++ b/gitlab/config.py @@ -204,8 +204,10 @@ def _get_values_from_helper(self): """Update attributes, which may get values from an external helper program""" for attr in HELPER_ATTRIBUTES: value = getattr(self, attr) - _value_lower = value.lower().strip() - if isinstance(value, str) and _value_lower.startswith(HELPER_PREFIX): + if not isinstance(value, str): + continue + + if value.lower().strip().startswith(HELPER_PREFIX): helper = expanduser(value[len(HELPER_PREFIX) :].strip()) value = subprocess.check_output([helper]).decode("utf-8").strip() setattr(self, attr, value) \ No newline at end of file From 7a7c9fd932def75a2f2c517482784e445d83881a Mon Sep 17 00:00:00 2001 From: "Kay-Uwe (Kiwi) Lorenz" Date: Sun, 7 Mar 2021 15:58:19 +0100 Subject: [PATCH 11/12] chore: make lint happy --- gitlab/config.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/gitlab/config.py b/gitlab/config.py index f3fea4f22..d9da5b389 100644 --- a/gitlab/config.py +++ b/gitlab/config.py @@ -39,6 +39,7 @@ def _env_config() -> List[str]: HELPER_ATTRIBUTES = ["job_token", "http_password", "private_token", "oauth_token"] + class ConfigError(Exception): pass @@ -210,4 +211,4 @@ def _get_values_from_helper(self): if value.lower().strip().startswith(HELPER_PREFIX): helper = expanduser(value[len(HELPER_PREFIX) :].strip()) value = subprocess.check_output([helper]).decode("utf-8").strip() - setattr(self, attr, value) \ No newline at end of file + setattr(self, attr, value) From 91ffb8e97e213d2f14340b952630875995ecedb2 Mon Sep 17 00:00:00 2001 From: "Kay-Uwe (Kiwi) Lorenz" Date: Sun, 18 Apr 2021 11:21:19 +0200 Subject: [PATCH 12/12] chore(config): allow simple commands without external script --- docs/cli-usage.rst | 50 ++++++++++++++++++++++++++++++++++++---------- gitlab/config.py | 33 ++++++++++++++++++++++++------ 2 files changed, 67 insertions(+), 16 deletions(-) diff --git a/docs/cli-usage.rst b/docs/cli-usage.rst index 666819772..c1b59bfef 100644 --- a/docs/cli-usage.rst +++ b/docs/cli-usage.rst @@ -93,6 +93,8 @@ Only one of ``private_token``, ``oauth_token`` or ``job_token`` should be defined. If neither are defined an anonymous request will be sent to the Gitlab server, with very limited permissions. +We recommend that you use `Credential helpers`_ to securely store your tokens. + .. list-table:: GitLab server options :header-rows: 1 @@ -119,22 +121,50 @@ server, with very limited permissions. * - ``http_password`` - Password for optional HTTP authentication -For all settings, which contain secrets (``http_password``, + +Credential helpers +------------------ + +For all configuration options that contain secrets (``http_password``, ``personal_token``, ``oauth_token``, ``job_token``), you can specify -a helper program to retrieve the secret indicated by ``helper:`` -prefix. You can only specify a path to a program without any -parameters. You may use ``~`` for expanding your homedir in helper -program's path. It is expected, that the program prints the secret -to standard output. +a helper program to retrieve the secret indicated by a ``helper:`` +prefix. This allows you to fetch values from a local keyring store +or cloud-hosted vaults such as Bitwarden. Environment variables are +expanded if they exist and ``~`` expands to your home directory. + +It is expected that the helper program prints the secret to standard output. +To use shell features such as piping to retrieve the value, you will need +to use a wrapper script; see below. Example for a `keyring `_ helper: -.. code-block:: bash +.. code-block:: ini - #!/bin/bash - keyring get Service Username + [global] + default = somewhere + ssl_verify = true + timeout = 5 + + [somewhere] + url = http://somewhe.re + private_token = helper: keyring get Service Username + timeout = 1 + +Example for a `pass `_ helper with a wrapper script: + +.. code-block:: ini + + [global] + default = somewhere + ssl_verify = true + timeout = 5 + + [somewhere] + url = http://somewhe.re + private_token = helper: /path/to/helper.sh + timeout = 1 -Example for a `pass `_ helper: +In `/path/to/helper.sh`: .. code-block:: bash diff --git a/gitlab/config.py b/gitlab/config.py index d9da5b389..c663bf841 100644 --- a/gitlab/config.py +++ b/gitlab/config.py @@ -17,9 +17,10 @@ import os import configparser +import shlex import subprocess from typing import List, Optional, Union -from os.path import expanduser +from os.path import expanduser, expandvars from gitlab.const import USER_AGENT @@ -56,6 +57,10 @@ class GitlabConfigMissingError(ConfigError): pass +class GitlabConfigHelperError(ConfigError): + pass + + class GitlabConfigParser(object): def __init__( self, gitlab_id: Optional[str] = None, config_files: Optional[List[str]] = None @@ -202,13 +207,29 @@ def __init__( pass def _get_values_from_helper(self): - """Update attributes, which may get values from an external helper program""" + """Update attributes that may get values from an external helper program""" for attr in HELPER_ATTRIBUTES: value = getattr(self, attr) if not isinstance(value, str): continue - if value.lower().strip().startswith(HELPER_PREFIX): - helper = expanduser(value[len(HELPER_PREFIX) :].strip()) - value = subprocess.check_output([helper]).decode("utf-8").strip() - setattr(self, attr, value) + if not value.lower().strip().startswith(HELPER_PREFIX): + continue + + helper = value[len(HELPER_PREFIX) :].strip() + commmand = [expanduser(expandvars(token)) for token in shlex.split(helper)] + + try: + value = ( + subprocess.check_output(commmand, stderr=subprocess.PIPE) + .decode("utf-8") + .strip() + ) + except subprocess.CalledProcessError as e: + stderr = e.stderr.decode().strip() + raise GitlabConfigHelperError( + f"Failed to read {attr} value from helper " + f"for {self.gitlab_id}:\n{stderr}" + ) from e + + setattr(self, attr, value)