From a9496fdb9c52896e89993b5b52b858b26e3860b9 Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Sat, 13 Feb 2021 12:54:46 +0100 Subject: [PATCH 1/7] Update CLI and HTTP client so it supports authenticating against API endpoints (api, auth, stream) using additional basic auth credentials in addition to the standard auth token / api key auth. This should come handy in situation where SSO is not used, but MFA is desired or similar --- st2client/st2client/base.py | 6 +- st2client/st2client/client.py | 99 +++++++++++++++++-------- st2client/st2client/config_parser.py | 5 ++ st2client/st2client/models/core.py | 47 +++++++----- st2client/st2client/shell.py | 8 ++ st2client/st2client/utils/httpclient.py | 23 +++++- 6 files changed, 136 insertions(+), 52 deletions(-) diff --git a/st2client/st2client/base.py b/st2client/st2client/base.py index e435540726..ff0f701bb8 100644 --- a/st2client/st2client/base.py +++ b/st2client/st2client/base.py @@ -58,7 +58,8 @@ 'api_version': ['general', 'api_version'], 'api_key': ['credentials', 'api_key'], 'cacert': ['general', 'cacert'], - 'debug': ['cli', 'debug'] + 'debug': ['cli', 'debug'], + 'basic_auth': ['credentials', 'basic_auth'], } @@ -82,7 +83,8 @@ def get_client(self, args, debug=False): # Note: Options provided as the CLI argument have the highest precedence # Precedence order: cli arguments > environment variables > rc file variables - cli_options = ['base_url', 'auth_url', 'api_url', 'stream_url', 'api_version', 'cacert'] + cli_options = ['base_url', 'auth_url', 'api_url', 'stream_url', 'api_version', 'cacert', + 'basic_auth'] cli_options = {opt: getattr(args, opt, None) for opt in cli_options} if cli_options.get("cacert", None) is not None: if cli_options["cacert"].lower() in ['true', '1', 't', 'y', 'yes']: diff --git a/st2client/st2client/client.py b/st2client/st2client/client.py index 6bda37942b..f4686b27c1 100644 --- a/st2client/st2client/client.py +++ b/st2client/st2client/client.py @@ -53,7 +53,8 @@ class Client(object): def __init__(self, base_url=None, auth_url=None, api_url=None, stream_url=None, - api_version=None, cacert=None, debug=False, token=None, api_key=None): + api_version=None, cacert=None, debug=False, token=None, api_key=None, + basic_auth=None): # Get CLI options. If not given, then try to get it from the environment. self.endpoints = dict() @@ -114,77 +115,113 @@ def __init__(self, base_url=None, auth_url=None, api_url=None, stream_url=None, self.api_key = api_key + if basic_auth: + if len(basic_auth.split(":")) != 2: + raise ValueError("basic_auth config options needs to be in the " + "username:password notation") + + self.basic_auth = tuple(basic_auth.split(":")) + else: + self.basic_auth = None + # Instantiate resource managers and assign appropriate API endpoint. self.managers = dict() self.managers['Token'] = ResourceManager( - models.Token, self.endpoints['auth'], cacert=self.cacert, debug=self.debug) + models.Token, self.endpoints['auth'], cacert=self.cacert, debug=self.debug, + basic_auth=self.basic_auth) self.managers['RunnerType'] = ResourceManager( - models.RunnerType, self.endpoints['api'], cacert=self.cacert, debug=self.debug) + models.RunnerType, self.endpoints['api'], cacert=self.cacert, debug=self.debug, + basic_auth=self.basic_auth) self.managers['Action'] = ActionResourceManager( - models.Action, self.endpoints['api'], cacert=self.cacert, debug=self.debug) + models.Action, self.endpoints['api'], cacert=self.cacert, debug=self.debug, + basic_auth=self.basic_auth) self.managers['ActionAlias'] = ActionAliasResourceManager( - models.ActionAlias, self.endpoints['api'], cacert=self.cacert, debug=self.debug) + models.ActionAlias, self.endpoints['api'], cacert=self.cacert, debug=self.debug, + basic_auth=self.basic_auth) self.managers['ActionAliasExecution'] = ActionAliasExecutionManager( models.ActionAliasExecution, self.endpoints['api'], - cacert=self.cacert, debug=self.debug) + cacert=self.cacert, debug=self.debug, + basic_auth=self.basic_auth) self.managers['ApiKey'] = ResourceManager( - models.ApiKey, self.endpoints['api'], cacert=self.cacert, debug=self.debug) + models.ApiKey, self.endpoints['api'], cacert=self.cacert, debug=self.debug, + basic_auth=self.basic_auth) self.managers['Config'] = ConfigManager( - models.Config, self.endpoints['api'], cacert=self.cacert, debug=self.debug) + models.Config, self.endpoints['api'], cacert=self.cacert, debug=self.debug, + basic_auth=self.basic_auth) self.managers['ConfigSchema'] = ResourceManager( - models.ConfigSchema, self.endpoints['api'], cacert=self.cacert, debug=self.debug) + models.ConfigSchema, self.endpoints['api'], cacert=self.cacert, debug=self.debug, + basic_auth=self.basic_auth) self.managers['Execution'] = ExecutionResourceManager( - models.Execution, self.endpoints['api'], cacert=self.cacert, debug=self.debug) + models.Execution, self.endpoints['api'], cacert=self.cacert, debug=self.debug, + basic_auth=self.basic_auth) # NOTE: LiveAction has been deprecated in favor of Execution. It will be left here for # backward compatibility reasons until v3.2.0 self.managers['LiveAction'] = self.managers['Execution'] self.managers['Inquiry'] = InquiryResourceManager( - models.Inquiry, self.endpoints['api'], cacert=self.cacert, debug=self.debug) + models.Inquiry, self.endpoints['api'], cacert=self.cacert, debug=self.debug, + basic_auth=self.basic_auth) self.managers['Pack'] = PackResourceManager( - models.Pack, self.endpoints['api'], cacert=self.cacert, debug=self.debug) + models.Pack, self.endpoints['api'], cacert=self.cacert, debug=self.debug, + basic_auth=self.basic_auth) self.managers['Policy'] = ResourceManager( - models.Policy, self.endpoints['api'], cacert=self.cacert, debug=self.debug) + models.Policy, self.endpoints['api'], cacert=self.cacert, debug=self.debug, + basic_auth=self.basic_auth) self.managers['PolicyType'] = ResourceManager( - models.PolicyType, self.endpoints['api'], cacert=self.cacert, debug=self.debug) + models.PolicyType, self.endpoints['api'], cacert=self.cacert, debug=self.debug, + basic_auth=self.basic_auth) self.managers['Rule'] = ResourceManager( - models.Rule, self.endpoints['api'], cacert=self.cacert, debug=self.debug) + models.Rule, self.endpoints['api'], cacert=self.cacert, debug=self.debug, + basic_auth=self.basic_auth) self.managers['Sensor'] = ResourceManager( - models.Sensor, self.endpoints['api'], cacert=self.cacert, debug=self.debug) + models.Sensor, self.endpoints['api'], cacert=self.cacert, debug=self.debug, + basic_auth=self.basic_auth) self.managers['TriggerType'] = ResourceManager( - models.TriggerType, self.endpoints['api'], cacert=self.cacert, debug=self.debug) + models.TriggerType, self.endpoints['api'], cacert=self.cacert, debug=self.debug, + basic_auth=self.basic_auth) self.managers['Trigger'] = ResourceManager( - models.Trigger, self.endpoints['api'], cacert=self.cacert, debug=self.debug) + models.Trigger, self.endpoints['api'], cacert=self.cacert, debug=self.debug, + basic_auth=self.basic_auth) self.managers['TriggerInstance'] = TriggerInstanceResourceManager( - models.TriggerInstance, self.endpoints['api'], cacert=self.cacert, debug=self.debug) + models.TriggerInstance, self.endpoints['api'], cacert=self.cacert, debug=self.debug, + basic_auth=self.basic_auth) self.managers['KeyValuePair'] = ResourceManager( - models.KeyValuePair, self.endpoints['api'], cacert=self.cacert, debug=self.debug) + models.KeyValuePair, self.endpoints['api'], cacert=self.cacert, debug=self.debug, + basic_auth=self.basic_auth) self.managers['Webhook'] = WebhookManager( - models.Webhook, self.endpoints['api'], cacert=self.cacert, debug=self.debug) + models.Webhook, self.endpoints['api'], cacert=self.cacert, debug=self.debug, + basic_auth=self.basic_auth) self.managers['Timer'] = ResourceManager( - models.Timer, self.endpoints['api'], cacert=self.cacert, debug=self.debug) + models.Timer, self.endpoints['api'], cacert=self.cacert, debug=self.debug, + basic_auth=self.basic_auth) self.managers['Trace'] = ResourceManager( - models.Trace, self.endpoints['api'], cacert=self.cacert, debug=self.debug) + models.Trace, self.endpoints['api'], cacert=self.cacert, debug=self.debug, + basic_auth=self.basic_auth) self.managers['RuleEnforcement'] = ResourceManager( - models.RuleEnforcement, self.endpoints['api'], cacert=self.cacert, debug=self.debug) + models.RuleEnforcement, self.endpoints['api'], cacert=self.cacert, debug=self.debug, + basic_auth=self.basic_auth) self.managers['Stream'] = StreamManager( - self.endpoints['stream'], cacert=self.cacert, debug=self.debug) + self.endpoints['stream'], cacert=self.cacert, debug=self.debug, + basic_auth=self.basic_auth) self.managers['Workflow'] = WorkflowManager( - self.endpoints['api'], cacert=self.cacert, debug=self.debug) + self.endpoints['api'], cacert=self.cacert, debug=self.debug, + basic_auth=self.basic_auth) # Service Registry self.managers['ServiceRegistryGroups'] = ServiceRegistryGroupsManager( models.ServiceRegistryGroup, self.endpoints['api'], cacert=self.cacert, - debug=self.debug) + debug=self.debug, basic_auth=self.basic_auth) self.managers['ServiceRegistryMembers'] = ServiceRegistryMembersManager( models.ServiceRegistryMember, self.endpoints['api'], cacert=self.cacert, - debug=self.debug) + debug=self.debug, basic_auth=self.basic_auth) # RBAC self.managers['Role'] = ResourceManager( - models.Role, self.endpoints['api'], cacert=self.cacert, debug=self.debug) + models.Role, self.endpoints['api'], cacert=self.cacert, debug=self.debug, + basic_auth=self.basic_auth) self.managers['UserRoleAssignment'] = ResourceManager( - models.UserRoleAssignment, self.endpoints['api'], cacert=self.cacert, debug=self.debug) + models.UserRoleAssignment, self.endpoints['api'], cacert=self.cacert, debug=self.debug, + basic_auth=self.basic_auth) @add_auth_token_to_kwargs_from_env def get_user_info(self, **kwargs): @@ -195,7 +232,7 @@ def get_user_info(self, **kwargs): """ url = '/user' client = httpclient.HTTPClient(root=self.endpoints['api'], cacert=self.cacert, - debug=self.debug) + debug=self.debug, basic_auth=self.basic_auth) response = client.get(url=url, **kwargs) if response.status_code != 200: diff --git a/st2client/st2client/config_parser.py b/st2client/st2client/config_parser.py index e5095df3e2..36e1442310 100644 --- a/st2client/st2client/config_parser.py +++ b/st2client/st2client/config_parser.py @@ -93,6 +93,11 @@ 'api_key': { 'type': 'string', 'default': None + }, + 'basic_auth': { + # Basic auth credentials in username:password notation + 'type': 'string', + 'default': None, } }, 'api': { diff --git a/st2client/st2client/models/core.py b/st2client/st2client/models/core.py index d102d3158f..c1822a4cd6 100644 --- a/st2client/st2client/models/core.py +++ b/st2client/st2client/models/core.py @@ -15,6 +15,9 @@ from __future__ import absolute_import +from typing import Optional +from typing import Tuple + import os import json import logging @@ -150,10 +153,23 @@ def __repr__(self): class ResourceManager(object): - def __init__(self, resource, endpoint, cacert=None, debug=False): + def __init__(self, resource: str, endpoint: str, cacert: Optional[str] = None, + debug: bool = False, basic_auth: Optional[Tuple[str, str]] = None): + """ + :param resource: Name of the resource to operate on. + :param endpoint: API endpoint URL. + :param cacert: Optional path to CA cert to use to validate the server side cert. + :param debug: True to enable debug mode where additional debug information will be logged. + :param basic_auth: Optional additional basic auth credentials in tuple(username, password) + notation. + """ self.resource = resource + self.endpoint = endpoint + self.cacert = cacert self.debug = debug - self.client = httpclient.HTTPClient(endpoint, cacert=cacert, debug=debug) + self.basic_auth = basic_auth + self.client = httpclient.HTTPClient(endpoint, cacert=cacert, debug=debug, + basic_auth=basic_auth) @staticmethod def handle_error(response): @@ -350,11 +366,6 @@ def delete_by_id(self, instance_id, **kwargs): class ActionAliasResourceManager(ResourceManager): - def __init__(self, resource, endpoint, cacert=None, debug=False): - self.resource = resource - self.debug = debug - self.client = httpclient.HTTPClient(root=endpoint, cacert=cacert, debug=debug) - @add_auth_token_to_kwargs_from_env def match(self, instance, **kwargs): url = '/%s/match' % self.resource.get_url_path_name() @@ -577,11 +588,6 @@ def update(self, instance, **kwargs): class WebhookManager(ResourceManager): - def __init__(self, resource, endpoint, cacert=None, debug=False): - self.resource = resource - self.debug = debug - self.client = httpclient.HTTPClient(root=endpoint, cacert=cacert, debug=debug) - @add_auth_token_to_kwargs_from_env def post_generic_webhook(self, trigger, payload=None, trace_tag=None, **kwargs): url = '/webhooks/st2' @@ -612,11 +618,12 @@ def match(self, instance, **kwargs): return (self.resource.deserialize(match['actionalias']), match['representation']) -class StreamManager(object): - def __init__(self, endpoint, cacert=None, debug=False): +class StreamManager(ResourceManager): + def __init__(self, endpoint, cacert=None, debug=False, basic_auth=None): + super(StreamManager, self).__init__(resource=None, + endpoint=endpoint, cacert=cacert, + debug=debug, basic_auth=basic_auth) self._url = httpclient.get_url_without_trailing_slash(endpoint) + '/stream' - self.debug = debug - self.cacert = cacert @add_auth_token_to_kwargs_from_env def listen(self, events=None, **kwargs): @@ -649,6 +656,9 @@ def listen(self, events=None, **kwargs): if self.cacert is not None: request_params['verify'] = self.cacert + if self.basic_auth: + request_params["auth"] = self.basic_auth + query_string = '?' + urllib.parse.urlencode(query_params) url = url + query_string @@ -664,11 +674,12 @@ def listen(self, events=None, **kwargs): class WorkflowManager(object): - def __init__(self, endpoint, cacert, debug): + def __init__(self, endpoint, cacert, debug, basic_auth=None): self.debug = debug self.cacert = cacert self.endpoint = endpoint + '/workflows' - self.client = httpclient.HTTPClient(root=self.endpoint, cacert=cacert, debug=debug) + self.client = httpclient.HTTPClient(root=self.endpoint, cacert=cacert, debug=debug, + basic_auth=basic_auth) @staticmethod def handle_error(response): diff --git a/st2client/st2client/shell.py b/st2client/st2client/shell.py index ac6108d796..6dda8d82ab 100755 --- a/st2client/st2client/shell.py +++ b/st2client/st2client/shell.py @@ -207,6 +207,14 @@ def __init__(self): 'If this is not provided, then SSL cert will not be verified.' ) + self.parser.add_argument( + '--basic-auth', + action='store', + dest='basic_auth', + default=None, + help='Optional additional basic auth credentials used to authenticate' + ) + self.parser.add_argument( '--config-file', action='store', diff --git a/st2client/st2client/utils/httpclient.py b/st2client/st2client/utils/httpclient.py index 089f6b88d6..320a9c6b3e 100644 --- a/st2client/st2client/utils/httpclient.py +++ b/st2client/st2client/utils/httpclient.py @@ -34,6 +34,20 @@ def decorate(*args, **kwargs): return decorate +def add_basic_auth_creds_to_kwargs(func): + """ + Add "auth" tuple parameter to the kwargs object which is passed to requests method in case it's + present on the HTTPClient object instance. + """ + def decorate(*args, **kwargs): + if isinstance(args[0], HTTPClient) and getattr(args[0], 'basic_auth', None): + kwargs['auth'] = args[0].basic_auth + + print(kwargs) + return func(*args, **kwargs) + return decorate + + def add_auth_token_to_headers(func): def decorate(*args, **kwargs): headers = kwargs.get('headers', dict()) @@ -77,13 +91,15 @@ def get_url_without_trailing_slash(value): class HTTPClient(object): - def __init__(self, root, cacert=None, debug=False): + def __init__(self, root, cacert=None, debug=False, basic_auth=None): self.root = get_url_without_trailing_slash(root) self.cacert = cacert self.debug = debug + self.basic_auth = basic_auth @add_ssl_verify_to_kwargs @add_auth_token_to_headers + @add_basic_auth_creds_to_kwargs def get(self, url, **kwargs): response = requests.get(self.root + url, **kwargs) response = self._response_hook(response=response) @@ -92,6 +108,7 @@ def get(self, url, **kwargs): @add_ssl_verify_to_kwargs @add_auth_token_to_headers @add_json_content_type_to_headers + @add_basic_auth_creds_to_kwargs def post(self, url, data, **kwargs): response = requests.post(self.root + url, json.dumps(data), **kwargs) response = self._response_hook(response=response) @@ -99,6 +116,7 @@ def post(self, url, data, **kwargs): @add_ssl_verify_to_kwargs @add_auth_token_to_headers + @add_basic_auth_creds_to_kwargs def post_raw(self, url, data, **kwargs): response = requests.post(self.root + url, data, **kwargs) response = self._response_hook(response=response) @@ -107,6 +125,7 @@ def post_raw(self, url, data, **kwargs): @add_ssl_verify_to_kwargs @add_auth_token_to_headers @add_json_content_type_to_headers + @add_basic_auth_creds_to_kwargs def put(self, url, data, **kwargs): response = requests.put(self.root + url, json.dumps(data), **kwargs) response = self._response_hook(response=response) @@ -115,6 +134,7 @@ def put(self, url, data, **kwargs): @add_ssl_verify_to_kwargs @add_auth_token_to_headers @add_json_content_type_to_headers + @add_basic_auth_creds_to_kwargs def patch(self, url, data, **kwargs): response = requests.patch(self.root + url, data, **kwargs) response = self._response_hook(response=response) @@ -122,6 +142,7 @@ def patch(self, url, data, **kwargs): @add_ssl_verify_to_kwargs @add_auth_token_to_headers + @add_basic_auth_creds_to_kwargs def delete(self, url, **kwargs): response = requests.delete(self.root + url, **kwargs) response = self._response_hook(response=response) From 2f39b060d30ece8e8ace95839da06800f8b550ac Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Sat, 13 Feb 2021 14:01:41 +0100 Subject: [PATCH 2/7] Don't pass additional basic auth credentials to /auth/tokens API endpoint since this one also utilizes basic auth based authentication mechanism. This approach is not ideal, but it's a comprompose - in case additional proxy based basic auth is enabled, user will still need to provide those additional basic auth credentials header for rest of the API operations (just not the /auth/tokens one). --- st2client/st2client/utils/httpclient.py | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/st2client/st2client/utils/httpclient.py b/st2client/st2client/utils/httpclient.py index 320a9c6b3e..5272047d3f 100644 --- a/st2client/st2client/utils/httpclient.py +++ b/st2client/st2client/utils/httpclient.py @@ -40,10 +40,19 @@ def add_basic_auth_creds_to_kwargs(func): present on the HTTPClient object instance. """ def decorate(*args, **kwargs): - if isinstance(args[0], HTTPClient) and getattr(args[0], 'basic_auth', None): + # NOTE: When logging in using /v1/auth/tokens API endpoint, "auth" argument will already be + # present since basic authentication is used to authenticate against auth service to obtain + # a token. + # + # In such scenarios, we don't pass additional basic auth headers to the server. + # + # This is not ideal, because it means if additional proxy based http auth is enabled, user + # may be able to authenticate against StackStorm auth service and obtain a valid auth token + # without using additional basic auth credentials, but all the request of the API operations + # on StackStorm API won't work without additional basic auth credentials. + if "auth" not in kwargs and isinstance(args[0], HTTPClient) and \ + getattr(args[0], 'basic_auth', None): kwargs['auth'] = args[0].basic_auth - - print(kwargs) return func(*args, **kwargs) return decorate From a4fec5cf98fdf9c891c3ab46c1162c8c436dea6e Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Sun, 14 Feb 2021 12:10:03 +0100 Subject: [PATCH 3/7] Update existing test, add tests for the new functionality. --- st2client/tests/fixtures/st2rc.full.ini | 2 ++ st2client/tests/unit/test_client.py | 29 +++++++++++++++++++++- st2client/tests/unit/test_config_parser.py | 3 ++- 3 files changed, 32 insertions(+), 2 deletions(-) diff --git a/st2client/tests/fixtures/st2rc.full.ini b/st2client/tests/fixtures/st2rc.full.ini index a4ac10f746..b55bd619d0 100644 --- a/st2client/tests/fixtures/st2rc.full.ini +++ b/st2client/tests/fixtures/st2rc.full.ini @@ -10,6 +10,8 @@ cache_token = False [credentials] username = test1 password = test1 +api_key = api_key +basic_auth = user1:pass1 [api] url = http://127.0.0.1:9101/v1 diff --git a/st2client/tests/unit/test_client.py b/st2client/tests/unit/test_client.py index 2e9fd95095..43a32d5ec1 100644 --- a/st2client/tests/unit/test_client.py +++ b/st2client/tests/unit/test_client.py @@ -14,14 +14,21 @@ # limitations under the License. from __future__ import absolute_import + import os -import six +import json import logging import unittest2 +import six +import mock +import requests + from st2client import models from st2client.client import Client +from tests import base + LOG = logging.getLogger(__name__) @@ -65,6 +72,26 @@ def test_default(self): self.assertEqual(endpoints['api'], api_url) self.assertEqual(endpoints['stream'], stream_url) + @mock.patch.object( + requests, + 'get', + mock.MagicMock(return_value=base.FakeResponse(json.dumps({}), 200, 'OK'))) + def test_basic_auth_option_success(self): + client = Client(basic_auth='username:password') + self.assertEqual(client.basic_auth, ('username', 'password')) + + self.assertEqual(requests.get.call_count, 0) + client.actions.get_all() + self.assertEqual(requests.get.call_count, 1) + + requests.get.assert_called_with( + 'http://127.0.0.1:9101/v1/actions', auth=('username', 'password'), params={} + ) + + def test_basic_auth_option_invalid_notation(self): + self.assertRaisesRegex(ValueError, "needs to be in the username:password notation", + Client, basic_auth='username_password') + def test_env(self): base_url = 'http://www.stackstorm.com' api_url = 'http://www.st2.com:9101/v1' diff --git a/st2client/tests/unit/test_config_parser.py b/st2client/tests/unit/test_config_parser.py index 35a125ebeb..cdd48c3460 100644 --- a/st2client/tests/unit/test_config_parser.py +++ b/st2client/tests/unit/test_config_parser.py @@ -63,7 +63,8 @@ def test_parse(self): 'credentials': { 'username': 'test1', 'password': 'test1', - 'api_key': None + 'api_key': "api_key", + 'basic_auth': "user1:pass1", }, 'api': { 'url': 'http://127.0.0.1:9101/v1' From a768611461ca6db8d82f3ea8e3dccde6542d2b26 Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Sun, 14 Feb 2021 12:13:28 +0100 Subject: [PATCH 4/7] Update sample config. --- conf/st2rc.sample.ini | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/conf/st2rc.sample.ini b/conf/st2rc.sample.ini index 15186bba10..ebc1244a74 100644 --- a/conf/st2rc.sample.ini +++ b/conf/st2rc.sample.ini @@ -19,6 +19,12 @@ timezone = Europe/Ljubljana # token username = test1 password = testpassword +# api key auth +# api_key = api_key +# Optional additional http basic auth credentials which are sent with each HTTP +# request except the auth request to /v1/auth/tokens endpoint. +# Available in StackStorm >= v3.4.0 +# basic_auth = username:password [api] url = http://127.0.0.1:9101/v1 From 566f1ba24e9f87183bd4182f99bb5e36d605018e Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Sun, 2 May 2021 13:05:45 +0200 Subject: [PATCH 5/7] Use maxsplit=1, update sample config, add test. --- conf/st2rc.sample.ini | 1 + st2client/st2client/client.py | 5 +++-- st2client/tests/unit/test_client.py | 19 +++++++++++++++++++ 3 files changed, 23 insertions(+), 2 deletions(-) diff --git a/conf/st2rc.sample.ini b/conf/st2rc.sample.ini index ebc1244a74..34cd080fdd 100644 --- a/conf/st2rc.sample.ini +++ b/conf/st2rc.sample.ini @@ -24,6 +24,7 @@ password = testpassword # Optional additional http basic auth credentials which are sent with each HTTP # request except the auth request to /v1/auth/tokens endpoint. # Available in StackStorm >= v3.4.0 +# NOTE: Username can't contain colon (:) character. # basic_auth = username:password [api] diff --git a/st2client/st2client/client.py b/st2client/st2client/client.py index 62d74f0cc0..ec2878758d 100644 --- a/st2client/st2client/client.py +++ b/st2client/st2client/client.py @@ -131,13 +131,14 @@ def __init__( self.api_key = api_key if basic_auth: - if len(basic_auth.split(":")) != 2: + # NOTE: We assume username can't contain colons + if len(basic_auth.split(":", 1)) != 2: raise ValueError( "basic_auth config options needs to be in the " "username:password notation" ) - self.basic_auth = tuple(basic_auth.split(":")) + self.basic_auth = tuple(basic_auth.split(":", 1)) else: self.basic_auth = None diff --git a/st2client/tests/unit/test_client.py b/st2client/tests/unit/test_client.py index 34395effc1..01d5bb4800 100644 --- a/st2client/tests/unit/test_client.py +++ b/st2client/tests/unit/test_client.py @@ -89,6 +89,25 @@ def test_basic_auth_option_success(self): "http://127.0.0.1:9101/v1/actions", auth=("username", "password"), params={} ) + @mock.patch.object( + requests, + "get", + mock.MagicMock(return_value=base.FakeResponse(json.dumps({}), 200, "OK")), + ) + def test_basic_auth_option_success_password_with_colon(self): + client = Client(basic_auth="username:password:with:colon") + self.assertEqual(client.basic_auth, ("username", "password:with:colon")) + + self.assertEqual(requests.get.call_count, 0) + client.actions.get_all() + self.assertEqual(requests.get.call_count, 1) + + requests.get.assert_called_with( + "http://127.0.0.1:9101/v1/actions", + auth=("username", "password:with:colon"), + params={}, + ) + def test_basic_auth_option_invalid_notation(self): self.assertRaisesRegex( ValueError, From a313e367efbf64fc1ada7c15210891caa2258fa8 Mon Sep 17 00:00:00 2001 From: Jacob Floyd Date: Sat, 2 Oct 2021 01:45:02 -0500 Subject: [PATCH 6/7] Drop unnecessary assigns now that super does it --- st2client/st2client/models/core.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/st2client/st2client/models/core.py b/st2client/st2client/models/core.py index 3460ad49c2..6f41dc3b23 100644 --- a/st2client/st2client/models/core.py +++ b/st2client/st2client/models/core.py @@ -702,8 +702,6 @@ def __init__(self, endpoint, cacert=None, debug=False, basic_auth=None): basic_auth=basic_auth, ) self._url = httpclient.get_url_without_trailing_slash(endpoint) + "/stream" - self.debug = debug - self.cacert = cacert @add_auth_token_to_kwargs_from_env def listen(self, events=None, **kwargs): From 63068684a7f874acb625fe69dca2e5c0b723033b Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Sun, 27 Mar 2022 13:12:32 +0200 Subject: [PATCH 7/7] Add changelog entry. --- CHANGELOG.rst | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 3caf14d7bc..aa4439df03 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -139,6 +139,19 @@ Added Contributed by @Kami. +* Add new ``credentials.basic_auth = username:password`` CLI configuration option. + + This argument allows client to use additional set of basic auth credentials when talking to the + StackStorm API endpoints (api, auth, stream) - that is, in addition to the token / api key + native StackStorm auth. + + This allows for simple basic auth based multi factor authentication implementation for + installations which don't utilize SSO. + + #5152 + + Contributed by @Kami. + Fixed ~~~~~