From cfe0aa748493a2392518817fb2dbc38622dbea02 Mon Sep 17 00:00:00 2001 From: jeansfelix Date: Thu, 12 Sep 2019 11:54:58 -0300 Subject: [PATCH 01/43] Fix the visibility of action secret parameters on rule --- st2common/st2common/models/db/rule.py | 55 ++++++++++++++++++++++++++- 1 file changed, 54 insertions(+), 1 deletion(-) diff --git a/st2common/st2common/models/db/rule.py b/st2common/st2common/models/db/rule.py index 7492b5abaa..3d9a988d79 100644 --- a/st2common/st2common/models/db/rule.py +++ b/st2common/st2common/models/db/rule.py @@ -14,11 +14,15 @@ from __future__ import absolute_import import mongoengine as me +from mongoengine.queryset import Q +import copy +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): enabled = me.BooleanField( @@ -101,6 +105,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) + + 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.get('action', {}).get('ref', None) + + 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 From 0f1bae2715af88a2ef14114f8dc7b6c4943ac5f8 Mon Sep 17 00:00:00 2001 From: jeansfelix Date: Thu, 12 Sep 2019 15:02:22 -0300 Subject: [PATCH 02/43] Fix PEP8 --- st2common/st2common/models/db/rule.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/st2common/st2common/models/db/rule.py b/st2common/st2common/models/db/rule.py index 3d9a988d79..809d3c2f5f 100644 --- a/st2common/st2common/models/db/rule.py +++ b/st2common/st2common/models/db/rule.py @@ -13,10 +13,10 @@ # limitations under the License. from __future__ import absolute_import +import copy import mongoengine as me -from mongoengine.queryset import Q -import copy +from mongoengine.queryset import Q from st2common.persistence.action import Action from st2common.models.db import MongoDBAccess from st2common.models.db import stormbase @@ -24,6 +24,7 @@ from st2common.util.secrets import get_secret_parameters from st2common.util.secrets import mask_secret_parameters + class RuleTypeDB(stormbase.StormBaseDB): enabled = me.BooleanField( default=True, @@ -119,8 +120,9 @@ def mask_secrets(self, value): 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) + result['action']['parameters'] = mask_secret_parameters( + parameters=result['action']['parameters'], + secret_parameters=secret_parameters) return result @@ -153,7 +155,6 @@ def _get_entity(self, model_persistence, ref, query_args): return None - def __init__(self, *args, **values): super(RuleDB, self).__init__(*args, **values) self.ref = self.get_reference().ref From 4f7f7a1d7ca8fb9456cacbd795aff9582f14bb3b Mon Sep 17 00:00:00 2001 From: Felipe Oliveira Date: Wed, 9 Oct 2019 14:51:42 -0300 Subject: [PATCH 03/43] Add tests when secrets masking works correctly when ?include_attributes and ?exclude_attributes --- st2api/tests/unit/controllers/v1/test_rule_views.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/st2api/tests/unit/controllers/v1/test_rule_views.py b/st2api/tests/unit/controllers/v1/test_rule_views.py index 7f98da158f..88c81f808a 100644 --- a/st2api/tests/unit/controllers/v1/test_rule_views.py +++ b/st2api/tests/unit/controllers/v1/test_rule_views.py @@ -86,6 +86,16 @@ def test_get_one_fail(self): resp = self.app.get('/v1/rules/1', expect_errors=True) self.assertEqual(resp.status_int, http_client.NOT_FOUND) + def test_get_one_parameters_mask_with_include_parameters(self): + resp = self.app.get('/api/v1/rules?include_attributes=action') + self.assertEqual(resp.status_int, http_client.OK) + pass + + def test_get_one_parameters_mask_with_exclude_parameters(self): + resp = self.app.get('/api/v1/rules?exclude_attributes=action') + self.assertEqual(resp.status_int, http_client.OK) + pass + def _insert_mock_models(self): rule_ids = [rule['id'] for rule in self.rules.values()] return rule_ids From 36980e52f62780216d2a5f5764757946451a33fa Mon Sep 17 00:00:00 2001 From: Felipe Oliveira Date: Wed, 9 Oct 2019 14:57:44 -0300 Subject: [PATCH 04/43] Change name method test_get_one_parameters_mask_with_include_parameters for test_get_all_parameters_mask_with_include_parameters and test_get_one_parameters_mask_with_exclude_parameters for test_get_all_parameters_mask_with_exclude_parameters --- st2api/tests/unit/controllers/v1/test_rule_views.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/st2api/tests/unit/controllers/v1/test_rule_views.py b/st2api/tests/unit/controllers/v1/test_rule_views.py index 88c81f808a..8236a7fa2d 100644 --- a/st2api/tests/unit/controllers/v1/test_rule_views.py +++ b/st2api/tests/unit/controllers/v1/test_rule_views.py @@ -86,12 +86,12 @@ def test_get_one_fail(self): resp = self.app.get('/v1/rules/1', expect_errors=True) self.assertEqual(resp.status_int, http_client.NOT_FOUND) - def test_get_one_parameters_mask_with_include_parameters(self): + def test_get_all_parameters_mask_with_include_parameters(self): resp = self.app.get('/api/v1/rules?include_attributes=action') self.assertEqual(resp.status_int, http_client.OK) pass - def test_get_one_parameters_mask_with_exclude_parameters(self): + def test_get_all_parameters_mask_with_exclude_parameters(self): resp = self.app.get('/api/v1/rules?exclude_attributes=action') self.assertEqual(resp.status_int, http_client.OK) pass From 6b9eb4eb921628246f3e24e3e3961dda95170b12 Mon Sep 17 00:00:00 2001 From: Felipe Oliveira Date: Wed, 9 Oct 2019 15:11:32 -0300 Subject: [PATCH 05/43] add assertequal for length of json --- st2api/tests/unit/controllers/v1/test_rule_views.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/st2api/tests/unit/controllers/v1/test_rule_views.py b/st2api/tests/unit/controllers/v1/test_rule_views.py index 8236a7fa2d..caa0c97352 100644 --- a/st2api/tests/unit/controllers/v1/test_rule_views.py +++ b/st2api/tests/unit/controllers/v1/test_rule_views.py @@ -89,11 +89,13 @@ def test_get_one_fail(self): def test_get_all_parameters_mask_with_include_parameters(self): resp = self.app.get('/api/v1/rules?include_attributes=action') self.assertEqual(resp.status_int, http_client.OK) + self.assertEqual(len(resp.json), 3) pass def test_get_all_parameters_mask_with_exclude_parameters(self): resp = self.app.get('/api/v1/rules?exclude_attributes=action') self.assertEqual(resp.status_int, http_client.OK) + self.assertEqual(len(resp.json), 3) pass def _insert_mock_models(self): From 42eb3937a86f876892f166836d093da640532349 Mon Sep 17 00:00:00 2001 From: Felipe Oliveira Date: Wed, 9 Oct 2019 15:36:17 -0300 Subject: [PATCH 06/43] Add test model test_rule_with_secret_parameter_masked --- .../tests/unit/controllers/v1/test_rule_views.py | 2 -- st2common/tests/unit/test_db.py | 14 +++++++++++++- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/st2api/tests/unit/controllers/v1/test_rule_views.py b/st2api/tests/unit/controllers/v1/test_rule_views.py index caa0c97352..82f23061f4 100644 --- a/st2api/tests/unit/controllers/v1/test_rule_views.py +++ b/st2api/tests/unit/controllers/v1/test_rule_views.py @@ -90,13 +90,11 @@ def test_get_all_parameters_mask_with_include_parameters(self): resp = self.app.get('/api/v1/rules?include_attributes=action') self.assertEqual(resp.status_int, http_client.OK) self.assertEqual(len(resp.json), 3) - pass def test_get_all_parameters_mask_with_exclude_parameters(self): resp = self.app.get('/api/v1/rules?exclude_attributes=action') self.assertEqual(resp.status_int, http_client.OK) self.assertEqual(len(resp.json), 3) - pass def _insert_mock_models(self): rule_ids = [rule['id'] for rule in self.rules.values()] diff --git a/st2common/tests/unit/test_db.py b/st2common/tests/unit/test_db.py index 8db54e7cf4..2ddea6f009 100644 --- a/st2common/tests/unit/test_db.py +++ b/st2common/tests/unit/test_db.py @@ -441,6 +441,17 @@ def test_trigger_lookup(self): 'Incorrect trigger returned.') ReactorModelTestCase._delete([saved, triggertype]) + def test_rule_with_secret_parameter_masked(self): + triggertype = ReactorModelTestCase._create_save_triggertype() + trigger = ReactorModelTestCase._create_save_trigger(triggertype) + runnertype = ActionModelTestCase._create_save_runnertype() + action = ActionModelTestCase._create_save_action(runnertype) + saved = ReactorModelTestCase._create_save_rule(trigger, action, False) + masked = RuleDB._mask_secrets(saved.action.) + + for value in masked['parameters']['p4'].values(): + self.assertEqual(value, MASKED_ATTRIBUTE_VALUE) + @staticmethod def _create_save_triggertype(): created = TriggerTypeDB(pack='dummy_pack_1', name='triggertype-1', description='', @@ -666,7 +677,8 @@ def _create_save_action(runnertype, metadata=False): created.parameters = { 'p1': {'type': 'string', 'required': True}, 'p2': {'type': 'number', 'default': 2868}, - 'p3': {'type': 'boolean', 'default': False} + 'p3': {'type': 'boolean', 'default': False}, + 'p4': {'type': 'secret', 'default': "*****"} } return Action.add_or_update(created) From 76cd651a737c681b5fa4c036cdfcc0a518ee7fcd Mon Sep 17 00:00:00 2001 From: Felipe Oliveira Date: Thu, 10 Oct 2019 16:00:06 -0300 Subject: [PATCH 07/43] Fix syntax error --- st2common/tests/unit/test_db.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/st2common/tests/unit/test_db.py b/st2common/tests/unit/test_db.py index 2ddea6f009..68f1bbc93a 100644 --- a/st2common/tests/unit/test_db.py +++ b/st2common/tests/unit/test_db.py @@ -447,7 +447,7 @@ def test_rule_with_secret_parameter_masked(self): runnertype = ActionModelTestCase._create_save_runnertype() action = ActionModelTestCase._create_save_action(runnertype) saved = ReactorModelTestCase._create_save_rule(trigger, action, False) - masked = RuleDB._mask_secrets(saved.action.) + masked = RuleDB._mask_secrets(saved.action) for value in masked['parameters']['p4'].values(): self.assertEqual(value, MASKED_ATTRIBUTE_VALUE) From 5defcdab528b76110169a46a4a99ce455aea5a4e Mon Sep 17 00:00:00 2001 From: Felipe Oliveira Date: Thu, 10 Oct 2019 16:14:23 -0300 Subject: [PATCH 08/43] Fix api path --- st2api/tests/unit/controllers/v1/test_rule_views.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/st2api/tests/unit/controllers/v1/test_rule_views.py b/st2api/tests/unit/controllers/v1/test_rule_views.py index 82f23061f4..ab4b54529f 100644 --- a/st2api/tests/unit/controllers/v1/test_rule_views.py +++ b/st2api/tests/unit/controllers/v1/test_rule_views.py @@ -87,12 +87,12 @@ def test_get_one_fail(self): self.assertEqual(resp.status_int, http_client.NOT_FOUND) def test_get_all_parameters_mask_with_include_parameters(self): - resp = self.app.get('/api/v1/rules?include_attributes=action') + resp = self.app.get('/v1/rules?include_attributes=action') self.assertEqual(resp.status_int, http_client.OK) self.assertEqual(len(resp.json), 3) def test_get_all_parameters_mask_with_exclude_parameters(self): - resp = self.app.get('/api/v1/rules?exclude_attributes=action') + resp = self.app.get('/v1/rules?exclude_attributes=action') self.assertEqual(resp.status_int, http_client.OK) self.assertEqual(len(resp.json), 3) From 421321dac9a31bb216a35a659f4517a12af938c7 Mon Sep 17 00:00:00 2001 From: Felipe Oliveira Date: Thu, 10 Oct 2019 17:29:49 -0300 Subject: [PATCH 09/43] Add default value p4 parameter --- st2common/tests/unit/test_db.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/st2common/tests/unit/test_db.py b/st2common/tests/unit/test_db.py index 68f1bbc93a..b25347ed99 100644 --- a/st2common/tests/unit/test_db.py +++ b/st2common/tests/unit/test_db.py @@ -672,7 +672,7 @@ 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}, From 0cc4cb52e6d359044e22a16f33220d5537e0fce2 Mon Sep 17 00:00:00 2001 From: Felipe Oliveira Date: Thu, 10 Oct 2019 17:44:39 -0300 Subject: [PATCH 10/43] Include import constant --- st2common/tests/unit/test_db.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/st2common/tests/unit/test_db.py b/st2common/tests/unit/test_db.py index b25347ed99..09010f90e0 100644 --- a/st2common/tests/unit/test_db.py +++ b/st2common/tests/unit/test_db.py @@ -21,7 +21,7 @@ import mongoengine.connection from oslo_config import cfg from pymongo.errors import ConnectionFailure - +from st2common.constants.secrets import MASKED_ATTRIBUTE_VALUE from st2common.constants.triggers import TRIGGER_INSTANCE_PROCESSED from st2common.models.system.common import ResourceReference from st2common.transport.publishers import PoolPublisher From c07522aebd5dc066d48d01bc04651c49bd18c4e3 Mon Sep 17 00:00:00 2001 From: Felipe Oliveira Date: Thu, 10 Oct 2019 17:56:23 -0300 Subject: [PATCH 11/43] Fix whitespace --- st2common/tests/unit/test_db.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/st2common/tests/unit/test_db.py b/st2common/tests/unit/test_db.py index 09010f90e0..616bdb77ce 100644 --- a/st2common/tests/unit/test_db.py +++ b/st2common/tests/unit/test_db.py @@ -672,7 +672,7 @@ def _create_save_action(runnertype, metadata=False): runner_type={'name': runnertype.name}) if not metadata: - created.parameters = {'p1': None, 'p2': None, 'p3': None, 'p4' : None} + created.parameters = {'p1': None, 'p2': None, 'p3': None, 'p4': None} else: created.parameters = { 'p1': {'type': 'string', 'required': True}, From 8b019496306b5c70442993c5ff43f3b6944c0e5d Mon Sep 17 00:00:00 2001 From: Felipe Oliveira Date: Thu, 10 Oct 2019 18:34:17 -0300 Subject: [PATCH 12/43] Fix delete before finish test method --- st2common/tests/unit/test_db.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/st2common/tests/unit/test_db.py b/st2common/tests/unit/test_db.py index 616bdb77ce..af0550efef 100644 --- a/st2common/tests/unit/test_db.py +++ b/st2common/tests/unit/test_db.py @@ -447,10 +447,12 @@ def test_rule_with_secret_parameter_masked(self): runnertype = ActionModelTestCase._create_save_runnertype() action = ActionModelTestCase._create_save_action(runnertype) saved = ReactorModelTestCase._create_save_rule(trigger, action, False) - masked = RuleDB._mask_secrets(saved.action) + masked = RuleDB._mask_secrets(retrieved.action) for value in masked['parameters']['p4'].values(): self.assertEqual(value, MASKED_ATTRIBUTE_VALUE) + ReactorModelTestCase._delete([saved, trigger, action, runnertype, triggertype]) + @staticmethod def _create_save_triggertype(): From 9f3feaf1bf1937fa884f6a3ba55c580920d060b3 Mon Sep 17 00:00:00 2001 From: Felipe Oliveira Date: Thu, 10 Oct 2019 18:50:40 -0300 Subject: [PATCH 13/43] Fix parameters --- st2common/tests/unit/test_db.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/st2common/tests/unit/test_db.py b/st2common/tests/unit/test_db.py index af0550efef..aa70784f24 100644 --- a/st2common/tests/unit/test_db.py +++ b/st2common/tests/unit/test_db.py @@ -680,7 +680,7 @@ def _create_save_action(runnertype, metadata=False): 'p1': {'type': 'string', 'required': True}, 'p2': {'type': 'number', 'default': 2868}, 'p3': {'type': 'boolean', 'default': False}, - 'p4': {'type': 'secret', 'default': "*****"} + 'p4': {'type': 'string', 'default': '******', 'secret': True} } return Action.add_or_update(created) From 7ff661ea41bd82a45c17f111c381a7652c45bd76 Mon Sep 17 00:00:00 2001 From: Felipe Oliveira Date: Thu, 10 Oct 2019 19:01:42 -0300 Subject: [PATCH 14/43] Fix parameters --- st2common/tests/unit/test_db.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/st2common/tests/unit/test_db.py b/st2common/tests/unit/test_db.py index aa70784f24..baa091b07c 100644 --- a/st2common/tests/unit/test_db.py +++ b/st2common/tests/unit/test_db.py @@ -447,7 +447,7 @@ def test_rule_with_secret_parameter_masked(self): runnertype = ActionModelTestCase._create_save_runnertype() action = ActionModelTestCase._create_save_action(runnertype) saved = ReactorModelTestCase._create_save_rule(trigger, action, False) - masked = RuleDB._mask_secrets(retrieved.action) + masked = RuleDB._mask_secrets(saved.action) for value in masked['parameters']['p4'].values(): self.assertEqual(value, MASKED_ATTRIBUTE_VALUE) From 4182f4bb5b622178e5d78219fdca13ec95d4823d Mon Sep 17 00:00:00 2001 From: Felipe Oliveira Date: Thu, 10 Oct 2019 19:11:46 -0300 Subject: [PATCH 15/43] Remove blank lines --- st2common/tests/unit/test_db.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/st2common/tests/unit/test_db.py b/st2common/tests/unit/test_db.py index baa091b07c..2d3768a469 100644 --- a/st2common/tests/unit/test_db.py +++ b/st2common/tests/unit/test_db.py @@ -448,12 +448,10 @@ def test_rule_with_secret_parameter_masked(self): action = ActionModelTestCase._create_save_action(runnertype) saved = ReactorModelTestCase._create_save_rule(trigger, action, False) masked = RuleDB._mask_secrets(saved.action) - for value in masked['parameters']['p4'].values(): self.assertEqual(value, MASKED_ATTRIBUTE_VALUE) ReactorModelTestCase._delete([saved, trigger, action, runnertype, triggertype]) - @staticmethod def _create_save_triggertype(): created = TriggerTypeDB(pack='dummy_pack_1', name='triggertype-1', description='', From 46dd3fa05e0ad059ef55f4652dcf2af9f6c74365 Mon Sep 17 00:00:00 2001 From: Felipe Oliveira Date: Thu, 10 Oct 2019 19:28:28 -0300 Subject: [PATCH 16/43] Simple string in secret true parameter --- st2common/tests/unit/test_db.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/st2common/tests/unit/test_db.py b/st2common/tests/unit/test_db.py index 2d3768a469..b2f9291f18 100644 --- a/st2common/tests/unit/test_db.py +++ b/st2common/tests/unit/test_db.py @@ -678,7 +678,7 @@ def _create_save_action(runnertype, metadata=False): 'p1': {'type': 'string', 'required': True}, 'p2': {'type': 'number', 'default': 2868}, 'p3': {'type': 'boolean', 'default': False}, - 'p4': {'type': 'string', 'default': '******', 'secret': True} + 'p4': {'type': 'string', 'default': 'secret', 'secret': True} } return Action.add_or_update(created) From 7ad78a9b2a576ca7a3c4cf79917ecbe0a6354db2 Mon Sep 17 00:00:00 2001 From: Felipe Oliveira Date: Fri, 11 Oct 2019 10:08:34 -0300 Subject: [PATCH 17/43] Add import RuleTypeDB --- st2common/tests/unit/test_db.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/st2common/tests/unit/test_db.py b/st2common/tests/unit/test_db.py index b2f9291f18..4ea51c0eeb 100644 --- a/st2common/tests/unit/test_db.py +++ b/st2common/tests/unit/test_db.py @@ -34,7 +34,7 @@ from st2common.models.db.trigger import TriggerTypeDB, TriggerDB, TriggerInstanceDB from st2common.models.db.rule import RuleDB, ActionExecutionSpecDB from st2common.persistence.cleanup import db_cleanup -from st2common.persistence.rule import Rule +from st2common.persistence.rule import Rule, RuleTypeDB from st2common.persistence.trigger import TriggerType, Trigger, TriggerInstance from st2tests import DbTestCase @@ -447,7 +447,7 @@ def test_rule_with_secret_parameter_masked(self): runnertype = ActionModelTestCase._create_save_runnertype() action = ActionModelTestCase._create_save_action(runnertype) saved = ReactorModelTestCase._create_save_rule(trigger, action, False) - masked = RuleDB._mask_secrets(saved.action) + masked = RuleTypeDB._mask_secrets(saved.action) for value in masked['parameters']['p4'].values(): self.assertEqual(value, MASKED_ATTRIBUTE_VALUE) ReactorModelTestCase._delete([saved, trigger, action, runnertype, triggertype]) From 4c0fd03786da65cfde1eba4d1cdf70d29c8d1900 Mon Sep 17 00:00:00 2001 From: Felipe Oliveira Date: Fri, 11 Oct 2019 10:33:01 -0300 Subject: [PATCH 18/43] Fix test_rule_with_secret_parameter_masked --- st2common/tests/unit/test_db.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/st2common/tests/unit/test_db.py b/st2common/tests/unit/test_db.py index 4ea51c0eeb..c80f9cb44d 100644 --- a/st2common/tests/unit/test_db.py +++ b/st2common/tests/unit/test_db.py @@ -34,7 +34,7 @@ from st2common.models.db.trigger import TriggerTypeDB, TriggerDB, TriggerInstanceDB from st2common.models.db.rule import RuleDB, ActionExecutionSpecDB from st2common.persistence.cleanup import db_cleanup -from st2common.persistence.rule import Rule, RuleTypeDB +from st2common.persistence.rule import Rule from st2common.persistence.trigger import TriggerType, Trigger, TriggerInstance from st2tests import DbTestCase @@ -447,10 +447,10 @@ def test_rule_with_secret_parameter_masked(self): runnertype = ActionModelTestCase._create_save_runnertype() action = ActionModelTestCase._create_save_action(runnertype) saved = ReactorModelTestCase._create_save_rule(trigger, action, False) - masked = RuleTypeDB._mask_secrets(saved.action) + retrieved = Rule.get_by_id(saved.id) for value in masked['parameters']['p4'].values(): self.assertEqual(value, MASKED_ATTRIBUTE_VALUE) - ReactorModelTestCase._delete([saved, trigger, action, runnertype, triggertype]) + ReactorModelTestCase._delete([retrieved, trigger, action, runnertype, triggertype]) @staticmethod def _create_save_triggertype(): From 3271499cdbd030941b535c254f223895a8bd547b Mon Sep 17 00:00:00 2001 From: Felipe Oliveira Date: Fri, 11 Oct 2019 10:43:17 -0300 Subject: [PATCH 19/43] Fix test_rule_with_secret_parameter_masked --- st2common/tests/unit/test_db.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/st2common/tests/unit/test_db.py b/st2common/tests/unit/test_db.py index c80f9cb44d..e927a2368a 100644 --- a/st2common/tests/unit/test_db.py +++ b/st2common/tests/unit/test_db.py @@ -448,7 +448,7 @@ def test_rule_with_secret_parameter_masked(self): action = ActionModelTestCase._create_save_action(runnertype) saved = ReactorModelTestCase._create_save_rule(trigger, action, False) retrieved = Rule.get_by_id(saved.id) - for value in masked['parameters']['p4'].values(): + for value in retrieved['parameters']['p4'].values(): self.assertEqual(value, MASKED_ATTRIBUTE_VALUE) ReactorModelTestCase._delete([retrieved, trigger, action, runnertype, triggertype]) From 4ffaa6f9785ed904a411694db365abb1614a09d7 Mon Sep 17 00:00:00 2001 From: Felipe Oliveira Date: Fri, 11 Oct 2019 11:14:40 -0300 Subject: [PATCH 20/43] Remove test test_rule_with_secret_parameter_masked --- st2common/tests/unit/test_db.py | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/st2common/tests/unit/test_db.py b/st2common/tests/unit/test_db.py index e927a2368a..d97e492855 100644 --- a/st2common/tests/unit/test_db.py +++ b/st2common/tests/unit/test_db.py @@ -441,17 +441,6 @@ def test_trigger_lookup(self): 'Incorrect trigger returned.') ReactorModelTestCase._delete([saved, triggertype]) - def test_rule_with_secret_parameter_masked(self): - triggertype = ReactorModelTestCase._create_save_triggertype() - trigger = ReactorModelTestCase._create_save_trigger(triggertype) - runnertype = ActionModelTestCase._create_save_runnertype() - action = ActionModelTestCase._create_save_action(runnertype) - saved = ReactorModelTestCase._create_save_rule(trigger, action, False) - retrieved = Rule.get_by_id(saved.id) - for value in retrieved['parameters']['p4'].values(): - self.assertEqual(value, MASKED_ATTRIBUTE_VALUE) - ReactorModelTestCase._delete([retrieved, trigger, action, runnertype, triggertype]) - @staticmethod def _create_save_triggertype(): created = TriggerTypeDB(pack='dummy_pack_1', name='triggertype-1', description='', From 4d7e552846dfd7c22793df6bc8041426c5c274ba Mon Sep 17 00:00:00 2001 From: Felipe Oliveira Date: Fri, 11 Oct 2019 11:27:56 -0300 Subject: [PATCH 21/43] Remove parameters p4 --- st2common/tests/unit/test_db.py | 1 - 1 file changed, 1 deletion(-) diff --git a/st2common/tests/unit/test_db.py b/st2common/tests/unit/test_db.py index d97e492855..ef0cbc9626 100644 --- a/st2common/tests/unit/test_db.py +++ b/st2common/tests/unit/test_db.py @@ -667,7 +667,6 @@ def _create_save_action(runnertype, metadata=False): 'p1': {'type': 'string', 'required': True}, 'p2': {'type': 'number', 'default': 2868}, 'p3': {'type': 'boolean', 'default': False}, - 'p4': {'type': 'string', 'default': 'secret', 'secret': True} } return Action.add_or_update(created) From ee220c19cefea22fefed7d9a3d93eb5596797ff8 Mon Sep 17 00:00:00 2001 From: Felipe Oliveira Date: Fri, 11 Oct 2019 11:44:15 -0300 Subject: [PATCH 22/43] Fix syntax error --- st2common/tests/unit/test_db.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/st2common/tests/unit/test_db.py b/st2common/tests/unit/test_db.py index ef0cbc9626..8b281c305b 100644 --- a/st2common/tests/unit/test_db.py +++ b/st2common/tests/unit/test_db.py @@ -661,12 +661,12 @@ def _create_save_action(runnertype, metadata=False): runner_type={'name': runnertype.name}) if not metadata: - created.parameters = {'p1': None, 'p2': None, 'p3': None, 'p4': None} + created.parameters = {'p1': None, 'p2': None, 'p3': None} else: created.parameters = { 'p1': {'type': 'string', 'required': True}, 'p2': {'type': 'number', 'default': 2868}, - 'p3': {'type': 'boolean', 'default': False}, + 'p3': {'type': 'boolean', 'default': False} } return Action.add_or_update(created) From b73e1c34737d2cec6f4c53cfce1284aac8912178 Mon Sep 17 00:00:00 2001 From: Felipe Oliveira Date: Fri, 11 Oct 2019 11:55:58 -0300 Subject: [PATCH 23/43] Remove unnecessary constant --- st2common/tests/unit/test_db.py | 1 - 1 file changed, 1 deletion(-) diff --git a/st2common/tests/unit/test_db.py b/st2common/tests/unit/test_db.py index 8b281c305b..d7c72ae964 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.secrets import MASKED_ATTRIBUTE_VALUE from st2common.constants.triggers import TRIGGER_INSTANCE_PROCESSED from st2common.models.system.common import ResourceReference from st2common.transport.publishers import PoolPublisher From a202c4bac50e90c52746dec3c92db92ee660d55e Mon Sep 17 00:00:00 2001 From: Felipe Oliveira Date: Fri, 11 Oct 2019 14:16:15 -0300 Subject: [PATCH 24/43] Remove All tests test_get_all_parameters_mask_with_include_parameters and test_get_all_parameters_mask_with_exclude_parameters --- st2api/tests/unit/controllers/v1/test_rule_views.py | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/st2api/tests/unit/controllers/v1/test_rule_views.py b/st2api/tests/unit/controllers/v1/test_rule_views.py index ab4b54529f..7f98da158f 100644 --- a/st2api/tests/unit/controllers/v1/test_rule_views.py +++ b/st2api/tests/unit/controllers/v1/test_rule_views.py @@ -86,16 +86,6 @@ def test_get_one_fail(self): resp = self.app.get('/v1/rules/1', expect_errors=True) self.assertEqual(resp.status_int, http_client.NOT_FOUND) - def test_get_all_parameters_mask_with_include_parameters(self): - resp = self.app.get('/v1/rules?include_attributes=action') - self.assertEqual(resp.status_int, http_client.OK) - self.assertEqual(len(resp.json), 3) - - def test_get_all_parameters_mask_with_exclude_parameters(self): - resp = self.app.get('/v1/rules?exclude_attributes=action') - self.assertEqual(resp.status_int, http_client.OK) - self.assertEqual(len(resp.json), 3) - def _insert_mock_models(self): rule_ids = [rule['id'] for rule in self.rules.values()] return rule_ids From 1ca28547bea85e43097690dd4f50d1ada2191732 Mon Sep 17 00:00:00 2001 From: Felipe Oliveira Date: Fri, 11 Oct 2019 14:55:17 -0300 Subject: [PATCH 25/43] Add tests with include_attribute and exclude_attributes --- st2api/tests/unit/controllers/v1/test_rule_views.py | 2 +- st2api/tests/unit/controllers/v1/test_rules.py | 10 +++++++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/st2api/tests/unit/controllers/v1/test_rule_views.py b/st2api/tests/unit/controllers/v1/test_rule_views.py index 7f98da158f..ae2305565d 100644 --- a/st2api/tests/unit/controllers/v1/test_rule_views.py +++ b/st2api/tests/unit/controllers/v1/test_rule_views.py @@ -63,7 +63,7 @@ def test_get_all(self): resp = self.app.get('/v1/rules/views') self.assertEqual(resp.status_int, http_client.OK) self.assertEqual(len(resp.json), 3) - + def test_get_one_by_id(self): rule_id = str(RuleViewControllerTestCase.RULE_1.id) get_resp = self.__do_get_one(rule_id) diff --git a/st2api/tests/unit/controllers/v1/test_rules.py b/st2api/tests/unit/controllers/v1/test_rules.py index ae8063095f..8c62996174 100644 --- a/st2api/tests/unit/controllers/v1/test_rules.py +++ b/st2api/tests/unit/controllers/v1/test_rules.py @@ -185,6 +185,14 @@ 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): + resp = self.app.get('/v1/rules?include_attributes=action') + self.assertEqual(resp.status_int, http_client.OK) + + def test_get_all_parameters_mask_with_exclude_parameters(self): + resp = self.app.get('/v1/rules?exclude_attributes=action') + self.assertEqual(resp.status_int, http_client.OK) + def test_get_one_by_id(self): post_resp = self.__do_post(RulesControllerTestCase.RULE_1) rule_id = self.__get_rule_id(post_resp) @@ -466,7 +474,7 @@ def setUpClass(cls): cls.RULE_1 = cls.fixtures_loader.load_fixtures( fixtures_pack=FIXTURES_PACK, fixtures_dict={'rules': [file_name]})['rules'][file_name] - + def test_ref_count_trigger_increment(self): post_resp = self.__do_post(self.RULE_1) rule_1_id = self.__get_rule_id(post_resp) From a6440140a9744da64003a8079e744a0299b09bbe Mon Sep 17 00:00:00 2001 From: Felipe Oliveira Date: Wed, 16 Oct 2019 19:23:05 -0300 Subject: [PATCH 26/43] Model tests done, WIP API Tests --- st2api/st2api/controllers/v1/rule_views.py | 3 ++- st2api/st2api/controllers/v1/rules.py | 4 ++-- st2api/tests/unit/controllers/v1/test_rules.py | 18 ++++++++++++++---- st2common/st2common/models/db/rule.py | 5 ++--- st2common/tests/unit/test_db.py | 9 +++++++-- 5 files changed, 27 insertions(+), 12 deletions(-) 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 8c62996174..d52753f043 100644 --- a/st2api/tests/unit/controllers/v1/test_rules.py +++ b/st2api/tests/unit/controllers/v1/test_rules.py @@ -128,6 +128,11 @@ def setUpClass(cls): fixtures_pack=FIXTURES_PACK, fixtures_dict={'rules': [file_name]})['rules'][file_name] + file_name = 'rule1.yaml' + RulesControllerTestCase.RULE_WITH_SECRET_PARAMETERS = RulesControllerTestCase.fixtures_loader.load_fixtures( + fixtures_pack=FIXTURES_PACK, + fixtures_dict={'rules': [file_name]})['rules'][file_name] + @classmethod def tearDownClass(cls): # Replace original configured value for trigger parameter validation @@ -186,13 +191,18 @@ def test_get_all_enabled(self): self.__do_delete(self.__get_rule_id(post_resp_rule_3)) def test_get_all_parameters_mask_with_include_parameters(self): - resp = self.app.get('/v1/rules?include_attributes=action') - self.assertEqual(resp.status_int, http_client.OK) + post_resp_rule_1 = self.__do_post(RulesControllerTestCase.RULE_1) + resp = self.app.get('/v1/rules?include-attributes=name') + self.assertEqual('name' in resp.json[0], True) + print(resp.json[0]) + 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(resp.status_int, http_client.OK) - + 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 809d3c2f5f..83e2365622 100644 --- a/st2common/st2common/models/db/rule.py +++ b/st2common/st2common/models/db/rule.py @@ -135,9 +135,8 @@ def _get_referenced_models(self, rule): :rtype: ``ActionDB`` """ - - action_ref = rule.get('action', {}).get('ref', None) - + + action_ref = rule['action']['ref'] def ref_query_args(ref): return {'ref': ref} diff --git a/st2common/tests/unit/test_db.py b/st2common/tests/unit/test_db.py index d7c72ae964..462ab8fa3e 100644 --- a/st2common/tests/unit/test_db.py +++ b/st2common/tests/unit/test_db.py @@ -520,6 +520,10 @@ def _delete(model_objects): "p3": { "type": "boolean", "default": False + }, + "p4": { + "type": "string", + "secret": True } }, "additionalProperties": False @@ -660,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) From 8faa9aba1caf694b3723d55775370ab2f8be8b42 Mon Sep 17 00:00:00 2001 From: Felipe Oliveira Date: Thu, 17 Oct 2019 10:27:47 -0300 Subject: [PATCH 27/43] Fix get api rule include attribute --- st2api/tests/unit/controllers/v1/test_rules.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/st2api/tests/unit/controllers/v1/test_rules.py b/st2api/tests/unit/controllers/v1/test_rules.py index d52753f043..f7b3d1a31b 100644 --- a/st2api/tests/unit/controllers/v1/test_rules.py +++ b/st2api/tests/unit/controllers/v1/test_rules.py @@ -192,9 +192,8 @@ def test_get_all_enabled(self): 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=name') + resp = self.app.get('/v1/rules?include_attributes=name') self.assertEqual('name' in resp.json[0], True) - print(resp.json[0]) self.__do_delete(self.__get_rule_id(post_resp_rule_1)) def test_get_all_parameters_mask_with_exclude_parameters(self): From a1b97c600e290807ea67b241a98bfb65718e8668 Mon Sep 17 00:00:00 2001 From: Felipe Oliveira Date: Thu, 17 Oct 2019 11:04:35 -0300 Subject: [PATCH 28/43] Include test key action in map rule --- st2common/st2common/models/db/rule.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/st2common/st2common/models/db/rule.py b/st2common/st2common/models/db/rule.py index 83e2365622..d2a9dcae59 100644 --- a/st2common/st2common/models/db/rule.py +++ b/st2common/st2common/models/db/rule.py @@ -116,7 +116,8 @@ def mask_secrets(self, value): :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']) From 07d4eb706a8b53ab21320e759365e251d5a254e0 Mon Sep 17 00:00:00 2001 From: Felipe Oliveira Date: Thu, 17 Oct 2019 11:27:44 -0300 Subject: [PATCH 29/43] Clean Lint errors --- st2api/tests/unit/controllers/v1/test_rule_views.py | 2 +- st2api/tests/unit/controllers/v1/test_rules.py | 9 ++------- st2common/st2common/models/db/rule.py | 1 - 3 files changed, 3 insertions(+), 9 deletions(-) diff --git a/st2api/tests/unit/controllers/v1/test_rule_views.py b/st2api/tests/unit/controllers/v1/test_rule_views.py index ae2305565d..7f98da158f 100644 --- a/st2api/tests/unit/controllers/v1/test_rule_views.py +++ b/st2api/tests/unit/controllers/v1/test_rule_views.py @@ -63,7 +63,7 @@ def test_get_all(self): resp = self.app.get('/v1/rules/views') self.assertEqual(resp.status_int, http_client.OK) self.assertEqual(len(resp.json), 3) - + def test_get_one_by_id(self): rule_id = str(RuleViewControllerTestCase.RULE_1.id) get_resp = self.__do_get_one(rule_id) diff --git a/st2api/tests/unit/controllers/v1/test_rules.py b/st2api/tests/unit/controllers/v1/test_rules.py index f7b3d1a31b..6d5a84cfec 100644 --- a/st2api/tests/unit/controllers/v1/test_rules.py +++ b/st2api/tests/unit/controllers/v1/test_rules.py @@ -128,11 +128,6 @@ def setUpClass(cls): fixtures_pack=FIXTURES_PACK, fixtures_dict={'rules': [file_name]})['rules'][file_name] - file_name = 'rule1.yaml' - RulesControllerTestCase.RULE_WITH_SECRET_PARAMETERS = RulesControllerTestCase.fixtures_loader.load_fixtures( - fixtures_pack=FIXTURES_PACK, - fixtures_dict={'rules': [file_name]})['rules'][file_name] - @classmethod def tearDownClass(cls): # Replace original configured value for trigger parameter validation @@ -201,7 +196,7 @@ def test_get_all_parameters_mask_with_exclude_parameters(self): 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) @@ -483,7 +478,7 @@ def setUpClass(cls): cls.RULE_1 = cls.fixtures_loader.load_fixtures( fixtures_pack=FIXTURES_PACK, fixtures_dict={'rules': [file_name]})['rules'][file_name] - + def test_ref_count_trigger_increment(self): post_resp = self.__do_post(self.RULE_1) rule_1_id = self.__get_rule_id(post_resp) diff --git a/st2common/st2common/models/db/rule.py b/st2common/st2common/models/db/rule.py index d2a9dcae59..983dd38746 100644 --- a/st2common/st2common/models/db/rule.py +++ b/st2common/st2common/models/db/rule.py @@ -136,7 +136,6 @@ def _get_referenced_models(self, rule): :rtype: ``ActionDB`` """ - action_ref = rule['action']['ref'] def ref_query_args(ref): return {'ref': ref} From b3a753131ed91f8bf94a8ff2872a05007ef8f883 Mon Sep 17 00:00:00 2001 From: Felipe Oliveira Date: Thu, 17 Oct 2019 11:41:55 -0300 Subject: [PATCH 30/43] Fix lin rule.py --- st2common/st2common/models/db/rule.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/st2common/st2common/models/db/rule.py b/st2common/st2common/models/db/rule.py index 983dd38746..e78ba58044 100644 --- a/st2common/st2common/models/db/rule.py +++ b/st2common/st2common/models/db/rule.py @@ -137,7 +137,7 @@ def _get_referenced_models(self, rule): :rtype: ``ActionDB`` """ action_ref = rule['action']['ref'] - def ref_query_args(ref): + def ref_query_args(ref): return {'ref': ref} action_db = self._get_entity(model_persistence=Action, From 8277c78fb8070bd59119a9e37ccff4e17f6b37bb Mon Sep 17 00:00:00 2001 From: Felipe Oliveira Date: Thu, 17 Oct 2019 14:20:28 -0300 Subject: [PATCH 31/43] Fix test rule --- st2api/tests/unit/controllers/v1/test_rules.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/st2api/tests/unit/controllers/v1/test_rules.py b/st2api/tests/unit/controllers/v1/test_rules.py index 6d5a84cfec..c6831393ab 100644 --- a/st2api/tests/unit/controllers/v1/test_rules.py +++ b/st2api/tests/unit/controllers/v1/test_rules.py @@ -187,8 +187,8 @@ def test_get_all_enabled(self): 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=name') - self.assertEqual('name' in resp.json[0], True) + 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): From 61b8b58e583b47711fc810cb62016769c0f987c2 Mon Sep 17 00:00:00 2001 From: Felipe Oliveira Date: Thu, 17 Oct 2019 14:40:50 -0300 Subject: [PATCH 32/43] Fix Db rule --- st2common/st2common/models/db/rule.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/st2common/st2common/models/db/rule.py b/st2common/st2common/models/db/rule.py index e78ba58044..983dd38746 100644 --- a/st2common/st2common/models/db/rule.py +++ b/st2common/st2common/models/db/rule.py @@ -137,7 +137,7 @@ def _get_referenced_models(self, rule): :rtype: ``ActionDB`` """ action_ref = rule['action']['ref'] - def ref_query_args(ref): + def ref_query_args(ref): return {'ref': ref} action_db = self._get_entity(model_persistence=Action, From f1c4de4e495e5bc1a3f5cdfe4741a88a763646e0 Mon Sep 17 00:00:00 2001 From: Felipe Oliveira Date: Thu, 17 Oct 2019 15:05:31 -0300 Subject: [PATCH 33/43] Fix method ref_query_args --- st2common/st2common/models/db/rule.py | 1 - 1 file changed, 1 deletion(-) diff --git a/st2common/st2common/models/db/rule.py b/st2common/st2common/models/db/rule.py index 983dd38746..ae8a1fe47e 100644 --- a/st2common/st2common/models/db/rule.py +++ b/st2common/st2common/models/db/rule.py @@ -139,7 +139,6 @@ def _get_referenced_models(self, rule): 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) From 8b85474f04e397715eadd001caaeb4734323ce57 Mon Sep 17 00:00:00 2001 From: Felipe Oliveira Date: Thu, 17 Oct 2019 15:18:29 -0300 Subject: [PATCH 34/43] Fix Lint --- st2common/st2common/models/db/rule.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/st2common/st2common/models/db/rule.py b/st2common/st2common/models/db/rule.py index ae8a1fe47e..f7b3c99a55 100644 --- a/st2common/st2common/models/db/rule.py +++ b/st2common/st2common/models/db/rule.py @@ -137,8 +137,10 @@ def _get_referenced_models(self, 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) From 4614d442ad418368fb2cfedb92cb328deb676115 Mon Sep 17 00:00:00 2001 From: Felipe Oliveira Date: Fri, 18 Oct 2019 17:48:39 -0300 Subject: [PATCH 35/43] Add changelog Added mask of rule action secret parameters #4788 --- CHANGELOG.rst | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 1aa027c819..6845822aa9 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -47,6 +47,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 --------------------- From dcfe02f1c009bd41b123e20419f512ff45f8fb70 Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Tue, 29 Oct 2019 17:04:44 +0100 Subject: [PATCH 36/43] Fix changelog entry. --- CHANGELOG.rst | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index f3608e2f7c..cb33bc1dcf 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -51,10 +51,10 @@ Fixed * 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 + When we had an action on a rule with secret type parameters, the parameters were visible. This + behavior has been resolved in #4788 + Contributed by @Nicodemos305 and @jeansfelix #4788 3.1.0 - June 27, 2019 --------------------- From bf421a304cc7a410693d5ae90203cf8f97c27510 Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Tue, 29 Oct 2019 17:58:16 +0100 Subject: [PATCH 37/43] Don't break the DB abstraction and use persistance class to query for the model in question. Also only retrieve fields we need for the masking operation. --- st2common/st2common/models/db/rule.py | 30 ++++++++++++++++----------- 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/st2common/st2common/models/db/rule.py b/st2common/st2common/models/db/rule.py index f7b3c99a55..e7b84a5413 100644 --- a/st2common/st2common/models/db/rule.py +++ b/st2common/st2common/models/db/rule.py @@ -116,19 +116,27 @@ def mask_secrets(self, value): :rtype: ``dict`` """ result = copy.deepcopy(value) - if('action' not in result): + + action_ref = result.get('action', {}).get('ref', None) + + if not action_ref: return result - action_db = self._get_referenced_models(rule=result) - secret_parameters = get_secret_parameters(parameters=action_db['parameters']) + action_db = self._get_referenced_action_model(action_ref=action_ref) + + if not action_db: + return 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): + def _get_referenced_action_model(self, action_ref): """ + Return Action object for the action referenced in a rule. Return the action model referenced from rule. :type rule: ``dict`` @@ -136,16 +144,14 @@ def _get_referenced_models(self, rule): :rtype: ``ActionDB`` """ - action_ref = rule['action']['ref'] - - def ref_query_args(ref): - return {'ref': ref} + # NOTE: We need to retrieve pack and name since that's needed for the PK + action_dbs = Action.query(only_fields=['pack', 'ref', 'name', 'parameters'], + ref=action_ref, limit=1) - action_db = self._get_entity(model_persistence=Action, - ref=action_ref, - query_args=ref_query_args) + if action_dbs: + return action_dbs[0] - return action_db + return None def _get_entity(self, model_persistence, ref, query_args): q = Q(**query_args(ref)) From 70f960bbfe3758d756e1ac1c861c96b49989bf3d Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Tue, 29 Oct 2019 19:23:38 +0100 Subject: [PATCH 38/43] get one and get all rules api endpoint show also support ?show_secrets=True for admins. --- st2api/st2api/controllers/v1/rules.py | 17 ++++++++++++----- st2common/st2common/openapi.yaml.j2 | 8 ++++++++ 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/st2api/st2api/controllers/v1/rules.py b/st2api/st2api/controllers/v1/rules.py index 04215f2527..3d764d5847 100644 --- a/st2api/st2api/controllers/v1/rules.py +++ b/st2api/st2api/controllers/v1/rules.py @@ -20,6 +20,7 @@ from st2common import log as logging from st2common.exceptions.apivalidation import ValueValidationException from st2common.exceptions.triggers import TriggerDoesNotExistException +from st2api.controllers.base import BaseRestControllerMixin from st2api.controllers.resource import BaseResourceIsolationControllerMixin from st2api.controllers.resource import ContentPackResourceController from st2api.controllers.controller_transforms import transform_to_bool @@ -39,7 +40,7 @@ LOG = logging.getLogger(__name__) -class RuleController(BaseResourceIsolationControllerMixin, ContentPackResourceController): +class RuleController(BaseRestControllerMixin, BaseResourceIsolationControllerMixin, ContentPackResourceController): """ Implements the RESTful web endpoint that handles the lifecycle of Rules in the system. @@ -68,8 +69,11 @@ class RuleController(BaseResourceIsolationControllerMixin, ContentPackResourceCo mandatory_include_fields_retrieve = ['pack', 'name', 'trigger'] 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, 'mask_secrets': True} + limit=None, show_secrets=False, requester_user=None, **raw_filters): + from_model_kwargs = { + 'ignore_missing_trigger': True, + 'mask_secrets': self._get_mask_secrets(requester_user, show_secrets=show_secrets) + } return super(RuleController, self)._get_all(exclude_fields=exclude_attributes, include_fields=include_attributes, from_model_kwargs=from_model_kwargs, @@ -79,8 +83,11 @@ def get_all(self, exclude_attributes=None, include_attributes=None, sort=None, o raw_filters=raw_filters, requester_user=requester_user) - def get_one(self, ref_or_id, requester_user): - from_model_kwargs = {'ignore_missing_trigger': True, 'mask_secrets': True} + def get_one(self, ref_or_id, requester_user, show_secrets=False): + from_model_kwargs = { + 'ignore_missing_trigger': True, + 'mask_secrets': self._get_mask_secrets(requester_user, show_secrets=show_secrets) + } 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/st2common/st2common/openapi.yaml.j2 b/st2common/st2common/openapi.yaml.j2 index 253f45cfde..3387564e8a 100644 --- a/st2common/st2common/openapi.yaml.j2 +++ b/st2common/st2common/openapi.yaml.j2 @@ -2645,6 +2645,10 @@ paths: in: query description: Enabled filter type: string + - name: show_secrets + in: query + description: Show secrets in plain text + type: boolean x-parameters: - name: user in: context @@ -2706,6 +2710,10 @@ paths: description: Entity reference or id type: string required: true + - name: show_secrets + in: query + description: Show secrets in plain text + type: boolean x-parameters: - name: user in: context From 3400a76aaef258b94e046e4d4be3689e9e064fd6 Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Tue, 29 Oct 2019 19:23:57 +0100 Subject: [PATCH 39/43] Add some test cases which verify secrets masking works correctly for rules API endpoint. --- .../tests/unit/controllers/v1/test_rules.py | 45 ++++++++++++++++++- .../fixtures/generic/actions/action1.yaml | 3 ++ .../fixtures/generic/rules/rule1.yaml | 1 + 3 files changed, 47 insertions(+), 2 deletions(-) diff --git a/st2api/tests/unit/controllers/v1/test_rules.py b/st2api/tests/unit/controllers/v1/test_rules.py index c6831393ab..3651bef6ac 100644 --- a/st2api/tests/unit/controllers/v1/test_rules.py +++ b/st2api/tests/unit/controllers/v1/test_rules.py @@ -20,6 +20,7 @@ from st2common.constants.rules import RULE_TYPE_STANDARD, RULE_TYPE_BACKSTOP from st2common.constants.pack import DEFAULT_PACK_NAME +from st2common.constants.secrets import MASKED_ATTRIBUTE_VALUE from st2common.persistence.trigger import Trigger from st2common.models.system.common import ResourceReference from st2common.transport.publishers import PoolPublisher @@ -185,10 +186,20 @@ 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): + def test_get_all_action_parameters_secrets_masking(self): post_resp_rule_1 = self.__do_post(RulesControllerTestCase.RULE_1) - resp = self.app.get('/v1/rules?include_attributes=action') + + # Verify parameter is masked by default + resp = self.app.get('/v1/rules') + self.assertEqual('action' in resp.json[0], True) + self.assertEqual(resp.json[0]['action']['parameters']['action_secret'], + MASKED_ATTRIBUTE_VALUE) + + # Verify ?show_secrets=true works + resp = self.app.get('/v1/rules?include_attributes=action&show_secrets=true') self.assertEqual('action' in resp.json[0], True) + self.assertEqual(resp.json[0]['action']['parameters']['action_secret'], 'secret') + self.__do_delete(self.__get_rule_id(post_resp_rule_1)) def test_get_all_parameters_mask_with_exclude_parameters(self): @@ -197,6 +208,36 @@ def test_get_all_parameters_mask_with_exclude_parameters(self): self.assertEqual('action' in resp.json[0], False) self.__do_delete(self.__get_rule_id(post_resp_rule_1)) + def test_get_all_parameters_mask_with_include_parameters(self): + post_resp_rule_1 = self.__do_post(RulesControllerTestCase.RULE_1) + + # Verify parameter is masked by default + resp = self.app.get('/v1/rules?include_attributes=action') + self.assertEqual('action' in resp.json[0], True) + self.assertEqual(resp.json[0]['action']['parameters']['action_secret'], + MASKED_ATTRIBUTE_VALUE) + + # Verify ?show_secrets=true works + resp = self.app.get('/v1/rules?include_attributes=action&show_secrets=true') + self.assertEqual('action' in resp.json[0], True) + self.assertEqual(resp.json[0]['action']['parameters']['action_secret'], 'secret') + + self.__do_delete(self.__get_rule_id(post_resp_rule_1)) + + def test_get_one_action_parameters_secrets_masking(self): + post_resp_rule_1 = self.__do_post(RulesControllerTestCase.RULE_1) + + # Verify parameter is masked by default + resp = self.app.get('/v1/rules/%s' % (post_resp_rule_1.json['id'])) + self.assertEqual(resp.json['action']['parameters']['action_secret'], + MASKED_ATTRIBUTE_VALUE) + + # Verify ?show_secrets=true works + resp = self.app.get('/v1/rules/%s?show_secrets=true' % (post_resp_rule_1.json['id'])) + self.assertEqual(resp.json['action']['parameters']['action_secret'], 'secret') + + 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/st2tests/st2tests/fixtures/generic/actions/action1.yaml b/st2tests/st2tests/fixtures/generic/actions/action1.yaml index 26522c26e3..15422c71b7 100644 --- a/st2tests/st2tests/fixtures/generic/actions/action1.yaml +++ b/st2tests/st2tests/fixtures/generic/actions/action1.yaml @@ -18,6 +18,9 @@ parameters: async_test: default: false type: boolean + action_secret: + type: string + secret: true runnerdummy: default: actiondummy immutable: true diff --git a/st2tests/st2tests/fixtures/generic/rules/rule1.yaml b/st2tests/st2tests/fixtures/generic/rules/rule1.yaml index a8c0809138..0e4302e7ca 100644 --- a/st2tests/st2tests/fixtures/generic/rules/rule1.yaml +++ b/st2tests/st2tests/fixtures/generic/rules/rule1.yaml @@ -3,6 +3,7 @@ action: parameters: ip1: '{{trigger.t1_p}}' ip2: '{{trigger}}' + action_secret: 'secret' ref: wolfpack.action-1 criteria: trigger.t1_p: From 0343096689a78af373fa51b5cba01c4f78b21324 Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Tue, 29 Oct 2019 19:25:11 +0100 Subject: [PATCH 40/43] Fix lint, add missing change. --- st2api/st2api/controllers/v1/rules.py | 3 ++- st2common/st2common/openapi.yaml | 8 ++++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/st2api/st2api/controllers/v1/rules.py b/st2api/st2api/controllers/v1/rules.py index 3d764d5847..4921b7b279 100644 --- a/st2api/st2api/controllers/v1/rules.py +++ b/st2api/st2api/controllers/v1/rules.py @@ -40,7 +40,8 @@ LOG = logging.getLogger(__name__) -class RuleController(BaseRestControllerMixin, BaseResourceIsolationControllerMixin, ContentPackResourceController): +class RuleController(BaseRestControllerMixin, BaseResourceIsolationControllerMixin, + ContentPackResourceController): """ Implements the RESTful web endpoint that handles the lifecycle of Rules in the system. diff --git a/st2common/st2common/openapi.yaml b/st2common/st2common/openapi.yaml index 91e299ff05..0896532f74 100644 --- a/st2common/st2common/openapi.yaml +++ b/st2common/st2common/openapi.yaml @@ -2649,6 +2649,10 @@ paths: in: query description: Enabled filter type: string + - name: show_secrets + in: query + description: Show secrets in plain text + type: boolean x-parameters: - name: user in: context @@ -2710,6 +2714,10 @@ paths: description: Entity reference or id type: string required: true + - name: show_secrets + in: query + description: Show secrets in plain text + type: boolean x-parameters: - name: user in: context From bfafd43fa55969c2b0d0705af609836463bbe0ec Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Tue, 29 Oct 2019 22:43:08 +0100 Subject: [PATCH 41/43] Remove unused method, fix docstring. --- st2common/st2common/models/db/rule.py | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/st2common/st2common/models/db/rule.py b/st2common/st2common/models/db/rule.py index e7b84a5413..0650363a5a 100644 --- a/st2common/st2common/models/db/rule.py +++ b/st2common/st2common/models/db/rule.py @@ -16,7 +16,6 @@ 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 @@ -137,10 +136,9 @@ def mask_secrets(self, value): def _get_referenced_action_model(self, action_ref): """ Return Action object for the action referenced in a rule. - Return the action model referenced from rule. - :type rule: ``dict`` - :param rule: rule + :param action_ref: Action reference. + :type action_ref: ``str`` :rtype: ``ActionDB`` """ @@ -153,14 +151,6 @@ def _get_referenced_action_model(self, action_ref): return None - 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 From ec6b7fdfdb421f7f242555f534f8e67c4ef59c95 Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Tue, 29 Oct 2019 22:43:57 +0100 Subject: [PATCH 42/43] Add additional clarification. --- st2common/st2common/models/db/rule.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/st2common/st2common/models/db/rule.py b/st2common/st2common/models/db/rule.py index 0650363a5a..e56d66cea9 100644 --- a/st2common/st2common/models/db/rule.py +++ b/st2common/st2common/models/db/rule.py @@ -109,6 +109,9 @@ def mask_secrets(self, value): """ Process the model dictionary and mask secret values. + NOTE: This method results in one addition "get one" query where we retrieve corresponding + action model so we can correctly mask secret parameters. + :type value: ``dict`` :param value: Document dictionary. From 06628a7999f5bf0ec9db71163651a66ee8901c02 Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Wed, 30 Oct 2019 08:08:42 +0100 Subject: [PATCH 43/43] Update changelog. --- CHANGELOG.rst | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 689df29023..2a6f4937ea 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -15,7 +15,7 @@ Added * Add support for `immutable_parameters` on Action Aliases. This feature allows default parameters to be supplied to the action on every execution of the alias. #4786 * Add ``get_entrypoint()`` method to ``ActionResourceManager`` attribute of st2client. - #4791 + #4791 Changed ~~~~~~~ @@ -61,11 +61,10 @@ 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: +* Fix secrets masking in action parameters section defined inside the rule when using + ``GET /v1/rules`` and ``GET /v1/rules/`` API endpoint. (bug fix) #4788 #4807 - When we had an action on a rule with secret type parameters, the parameters were visible. This - behavior has been resolved in #4788 - Contributed by @Nicodemos305 and @jeansfelix #4788 + Contributed by @Nicodemos305 and @jeansfelix 3.1.0 - June 27, 2019 ---------------------