diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 6d79ce87f8..f3608e2f7c 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -50,6 +50,11 @@ Fixed doesn't depend on internal pip API and so it works with latest pip version. (bug fix) #4750 * Fix dependency conflicts in pack CI runs: downgrade requests dependency back to 0.21.0, update internal dependencies and test expectations (amqp, pyyaml, prance, six) (bugfix) #4774 +* Fix mask of rule action secret parameters: + When we had an action on a rule with secret type parameters, the parameters were visible. This behavior + has been resolved in PR4788. + Contributed by @Nicodemos305 and @jeansfelix + 3.1.0 - June 27, 2019 --------------------- diff --git a/st2api/st2api/controllers/v1/rule_views.py b/st2api/st2api/controllers/v1/rule_views.py index c607dac0cb..04c1114bad 100644 --- a/st2api/st2api/controllers/v1/rule_views.py +++ b/st2api/st2api/controllers/v1/rule_views.py @@ -99,8 +99,9 @@ def get_all(self, exclude_attributes=None, include_attributes=None, sort=None, o return rules def get_one(self, ref_or_id, requester_user): + from_model_kwargs = {'mask_secrets': True} rule = self._get_one(ref_or_id, permission_type=PermissionType.RULE_VIEW, - requester_user=requester_user) + requester_user=requester_user, from_model_kwargs=from_model_kwargs) result = self._append_view_properties([rule.json])[0] rule.json = result return rule diff --git a/st2api/st2api/controllers/v1/rules.py b/st2api/st2api/controllers/v1/rules.py index d64fb73732..04215f2527 100644 --- a/st2api/st2api/controllers/v1/rules.py +++ b/st2api/st2api/controllers/v1/rules.py @@ -69,7 +69,7 @@ class RuleController(BaseResourceIsolationControllerMixin, ContentPackResourceCo def get_all(self, exclude_attributes=None, include_attributes=None, sort=None, offset=0, limit=None, requester_user=None, **raw_filters): - from_model_kwargs = {'ignore_missing_trigger': True} + from_model_kwargs = {'ignore_missing_trigger': True, 'mask_secrets': True} return super(RuleController, self)._get_all(exclude_fields=exclude_attributes, include_fields=include_attributes, from_model_kwargs=from_model_kwargs, @@ -80,7 +80,7 @@ def get_all(self, exclude_attributes=None, include_attributes=None, sort=None, o requester_user=requester_user) def get_one(self, ref_or_id, requester_user): - from_model_kwargs = {'ignore_missing_trigger': True} + from_model_kwargs = {'ignore_missing_trigger': True, 'mask_secrets': True} return super(RuleController, self)._get_one(ref_or_id, from_model_kwargs=from_model_kwargs, requester_user=requester_user, permission_type=PermissionType.RULE_VIEW) diff --git a/st2api/tests/unit/controllers/v1/test_rules.py b/st2api/tests/unit/controllers/v1/test_rules.py index ae8063095f..c6831393ab 100644 --- a/st2api/tests/unit/controllers/v1/test_rules.py +++ b/st2api/tests/unit/controllers/v1/test_rules.py @@ -185,6 +185,18 @@ def test_get_all_enabled(self): self.__do_delete(self.__get_rule_id(post_resp_rule_1)) self.__do_delete(self.__get_rule_id(post_resp_rule_3)) + def test_get_all_parameters_mask_with_include_parameters(self): + post_resp_rule_1 = self.__do_post(RulesControllerTestCase.RULE_1) + resp = self.app.get('/v1/rules?include_attributes=action') + self.assertEqual('action' in resp.json[0], True) + self.__do_delete(self.__get_rule_id(post_resp_rule_1)) + + def test_get_all_parameters_mask_with_exclude_parameters(self): + post_resp_rule_1 = self.__do_post(RulesControllerTestCase.RULE_1) + resp = self.app.get('/v1/rules?exclude_attributes=action') + self.assertEqual('action' in resp.json[0], False) + self.__do_delete(self.__get_rule_id(post_resp_rule_1)) + def test_get_one_by_id(self): post_resp = self.__do_post(RulesControllerTestCase.RULE_1) rule_id = self.__get_rule_id(post_resp) diff --git a/st2common/st2common/models/db/rule.py b/st2common/st2common/models/db/rule.py index 7492b5abaa..f7b3c99a55 100644 --- a/st2common/st2common/models/db/rule.py +++ b/st2common/st2common/models/db/rule.py @@ -13,11 +13,16 @@ # limitations under the License. from __future__ import absolute_import +import copy import mongoengine as me +from mongoengine.queryset import Q +from st2common.persistence.action import Action from st2common.models.db import MongoDBAccess from st2common.models.db import stormbase from st2common.constants.types import ResourceType +from st2common.util.secrets import get_secret_parameters +from st2common.util.secrets import mask_secret_parameters class RuleTypeDB(stormbase.StormBaseDB): @@ -101,6 +106,55 @@ class RuleDB(stormbase.StormFoundationDB, stormbase.TagsMixin, stormbase.UIDFieldMixin.get_indexes()) } + def mask_secrets(self, value): + """ + Process the model dictionary and mask secret values. + + :type value: ``dict`` + :param value: Document dictionary. + + :rtype: ``dict`` + """ + result = copy.deepcopy(value) + if('action' not in result): + return result + action_db = self._get_referenced_models(rule=result) + + secret_parameters = get_secret_parameters(parameters=action_db['parameters']) + result['action']['parameters'] = mask_secret_parameters( + parameters=result['action']['parameters'], + secret_parameters=secret_parameters) + + return result + + def _get_referenced_models(self, rule): + """ + Return the action model referenced from rule. + + :type rule: ``dict`` + :param rule: rule + + :rtype: ``ActionDB`` + """ + action_ref = rule['action']['ref'] + + def ref_query_args(ref): + return {'ref': ref} + + action_db = self._get_entity(model_persistence=Action, + ref=action_ref, + query_args=ref_query_args) + + return action_db + + def _get_entity(self, model_persistence, ref, query_args): + q = Q(**query_args(ref)) + + if q: + return model_persistence._get_impl().model.objects(q).first() + + return None + def __init__(self, *args, **values): super(RuleDB, self).__init__(*args, **values) self.ref = self.get_reference().ref diff --git a/st2common/tests/unit/test_db.py b/st2common/tests/unit/test_db.py index 8db54e7cf4..462ab8fa3e 100644 --- a/st2common/tests/unit/test_db.py +++ b/st2common/tests/unit/test_db.py @@ -21,7 +21,6 @@ import mongoengine.connection from oslo_config import cfg from pymongo.errors import ConnectionFailure - from st2common.constants.triggers import TRIGGER_INSTANCE_PROCESSED from st2common.models.system.common import ResourceReference from st2common.transport.publishers import PoolPublisher @@ -521,6 +520,10 @@ def _delete(model_objects): "p3": { "type": "boolean", "default": False + }, + "p4": { + "type": "string", + "secret": True } }, "additionalProperties": False @@ -661,12 +664,13 @@ def _create_save_action(runnertype, metadata=False): runner_type={'name': runnertype.name}) if not metadata: - created.parameters = {'p1': None, 'p2': None, 'p3': None} + created.parameters = {'p1': None, 'p2': None, 'p3': None, 'p4': None} else: created.parameters = { 'p1': {'type': 'string', 'required': True}, 'p2': {'type': 'number', 'default': 2868}, - 'p3': {'type': 'boolean', 'default': False} + 'p3': {'type': 'boolean', 'default': False}, + 'p4': {'type': 'string', 'secret': True} } return Action.add_or_update(created)