diff --git a/CHANGELOG.rst b/CHANGELOG.rst index ff3f22e160..54ff8b3382 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -33,6 +33,15 @@ Changed Contributed by Nick Maludy (Encore Technologies). + +Fixed +~~~~~ + +* Fixed a bug where ``secret: true`` was not applying to full object and array trees. (bugfix) #4234 + Reported by @jjm + + Contributed by Nick Maludy (Encore Technologies). + 2.8.0 - July 10, 2018 --------------------- diff --git a/st2common/st2common/util/secrets.py b/st2common/st2common/util/secrets.py index af12aa92b6..57d8a0b08f 100644 --- a/st2common/st2common/util/secrets.py +++ b/st2common/st2common/util/secrets.py @@ -66,6 +66,17 @@ def get_secret_parameters(parameters): secret_parameters = {} parameters_type = parameters.get('type') + # If the parameter itself is secret, then skip all processing below it + # and return the type of this parameter. + # + # **This causes the _full_ object / array tree to be secret (no children will be shown).** + # + # **Important** that we do this check first, so in case this parameter + # is an `object` or `array`, and the user wants the full thing + # to be secret, that it is marked as secret. + if parameters.get('secret', False): + return parameters_type + iterator = None if parameters_type == 'object': # if this is an object, then iterate over the properties within @@ -95,7 +106,24 @@ def get_secret_parameters(parameters): continue parameter_type = options.get('type') - if parameter_type in ['object', 'array']: + if options.get('secret', False): + # If this parameter is secret, then add it our secret parameters + # + # **This causes the _full_ object / array tree to be secret + # (no children will be shown)** + # + # **Important** that we do this check first, so in case this parameter + # is an `object` or `array`, and the user wants the full thing + # to be secret, that it is marked as secret. + if isinstance(secret_parameters, list): + secret_parameters.append(parameter_type) + elif isinstance(secret_parameters, dict): + secret_parameters[parameter] = parameter_type + else: + return parameter_type + elif parameter_type in ['object', 'array']: + # otherwise recursively dive into the `object`/`array` and + # find individual parameters marked as secret sub_params = get_secret_parameters(options) if sub_params: if isinstance(secret_parameters, list): @@ -104,14 +132,6 @@ def get_secret_parameters(parameters): secret_parameters[parameter] = sub_params else: return sub_params - elif options.get('secret', False): - # if this parameter is secret, then add it our secret parameters - if isinstance(secret_parameters, list): - secret_parameters.append(parameter_type) - elif isinstance(secret_parameters, dict): - secret_parameters[parameter] = parameter_type - else: - return parameter_type return secret_parameters diff --git a/st2common/tests/unit/test_util_secrets.py b/st2common/tests/unit/test_util_secrets.py index 848b372574..b110492180 100644 --- a/st2common/tests/unit/test_util_secrets.py +++ b/st2common/tests/unit/test_util_secrets.py @@ -145,6 +145,32 @@ ################################################################################ +TEST_ROOT_OBJECT_SCHEMA = { + 'description': 'root', + 'type': 'object', + 'properties': { + 'arg_level_one': { + 'description': 'down', + 'type': 'object', + 'properties': { + 'secret_field_in_object': { + 'type': 'string', + 'secret': True + } + } + } + } +} + +TEST_ROOT_OBJECT_SECRET_PARAMS = { + 'arg_level_one': + { + 'secret_field_in_object': 'string' + } +} + +################################################################################ + TEST_NESTED_ARRAYS_SCHEMA = { 'arg_optional_array': { 'description': 'Mirror', @@ -319,6 +345,208 @@ ] } +################################################################################ + +TEST_SECRET_ARRAY_SCHEMA = { + 'arg_secret_array': { + 'description': 'Mirror', + 'type': 'array', + 'secret': True, + } +} + +TEST_SECRET_ARRAY_SECRET_PARAMS = { + 'arg_secret_array': 'array' +} + +################################################################################ + +TEST_SECRET_OBJECT_SCHEMA = { + 'arg_secret_object': { + 'type': 'object', + 'secret': True, + } +} + +TEST_SECRET_OBJECT_SECRET_PARAMS = { + 'arg_secret_object': 'object' +} + + +################################################################################ + +TEST_SECRET_ROOT_ARRAY_SCHEMA = { + 'description': 'secret array', + 'type': 'array', + 'secret': True, + 'items': { + 'description': 'down', + 'type': 'object', + 'properties': { + 'secret_field_in_object': { + 'type': 'string', + 'secret': True + } + } + } +} + +TEST_SECRET_ROOT_ARRAY_SECRET_PARAMS = 'array' + +################################################################################ + +TEST_SECRET_ROOT_OBJECT_SCHEMA = { + 'description': 'secret object', + 'type': 'object', + 'secret': True, + 'proeprteis': { + 'arg_level_one': { + 'description': 'down', + 'type': 'object', + 'properties': { + 'secret_field_in_object': { + 'type': 'string', + 'secret': True + } + } + } + } +} + +TEST_SECRET_ROOT_OBJECT_SECRET_PARAMS = 'object' + +################################################################################ + +TEST_SECRET_NESTED_OBJECTS_SCHEMA = { + 'arg_object': { + 'description': 'Mirror', + 'type': 'object', + 'properties': { + 'arg_nested_object': { + 'description': 'Mirror mirror', + 'type': 'object', + 'secret': True, + 'properties': { + 'arg_double_nested_secret': { + 'description': 'Deep, deep down', + 'type': 'string', + 'secret': True + } + } + }, + 'arg_nested_secret': { + 'description': 'Deep down', + 'type': 'string', + 'secret': True + } + } + }, + 'arg_secret_object': { + 'description': 'Mirror', + 'type': 'object', + 'secret': True, + 'properties': { + 'arg_nested_object': { + 'description': 'Mirror mirror', + 'type': 'object', + 'secret': True, + 'properties': { + 'arg_double_nested_secret': { + 'description': 'Deep, deep down', + 'type': 'string', + 'secret': True + } + } + }, + 'arg_nested_secret': { + 'description': 'Deep down', + 'type': 'string', + 'secret': True + } + } + } +} + +TEST_SECRET_NESTED_OBJECTS_SECRET_PARAMS = { + 'arg_object': { + 'arg_nested_secret': 'string', + 'arg_nested_object': 'object' + }, + 'arg_secret_object': 'object' +} + + +################################################################################ + +TEST_SECRET_NESTED_ARRAYS_SCHEMA = { + 'arg_optional_array': { + 'description': 'Mirror', + 'type': 'array', + 'secret': True, + 'items': { + 'description': 'Deep down', + 'type': 'string' + } + }, + 'arg_optional_double_array': { + 'description': 'Mirror', + 'type': 'array', + 'secret': True, + 'items': { + 'type': 'array', + 'items': { + 'description': 'Deep down', + 'type': 'string', + } + } + }, + 'arg_optional_tripple_array': { + 'description': 'Mirror', + 'type': 'array', + 'items': { + 'type': 'array', + 'secret': True, + 'items': { + 'type': 'array', + 'items': { + 'description': 'Deep down', + 'type': 'string', + } + } + } + }, + 'arg_optional_quad_array': { + 'description': 'Mirror', + 'type': 'array', + 'items': { + 'type': 'array', + 'items': { + 'type': 'array', + 'secret': True, + 'items': { + 'type': 'array', + 'items': { + 'description': 'Deep down', + 'type': 'string', + } + } + } + } + } +} + +TEST_SECRET_NESTED_ARRAYS_SECRET_PARAMS = { + 'arg_optional_array': 'array', + 'arg_optional_double_array': 'array', + 'arg_optional_tripple_array': [ + 'array' + ], + 'arg_optional_quad_array': [ + [ + 'array' + ] + ] +} ################################################################################ @@ -345,6 +573,10 @@ def test_get_secret_parameters_root_array(self): result = secrets.get_secret_parameters(TEST_ROOT_ARRAY_SCHEMA) self.assertEqual(TEST_ROOT_ARRAY_SECRET_PARAMS, result) + def test_get_secret_parameters_root_object(self): + result = secrets.get_secret_parameters(TEST_ROOT_OBJECT_SCHEMA) + self.assertEqual(TEST_ROOT_OBJECT_SECRET_PARAMS, result) + def test_get_secret_parameters_nested_arrays(self): result = secrets.get_secret_parameters(TEST_NESTED_ARRAYS_SCHEMA) self.assertEqual(TEST_NESTED_ARRAYS_SECRET_PARAMS, result) @@ -361,8 +593,31 @@ def test_get_secret_parameters_nested_array_with_object(self): result = secrets.get_secret_parameters(TEST_NESTED_ARRAY_WITH_OBJECT_SCHEMA) self.assertEqual(TEST_NESTED_ARRAY_WITH_OBJECT_SECRET_PARAMS, result) + def test_get_secret_parameters_secret_array(self): + result = secrets.get_secret_parameters(TEST_SECRET_ARRAY_SCHEMA) + self.assertEqual(TEST_SECRET_ARRAY_SECRET_PARAMS, result) + + def test_get_secret_parameters_secret_object(self): + result = secrets.get_secret_parameters(TEST_SECRET_OBJECT_SCHEMA) + self.assertEqual(TEST_SECRET_OBJECT_SECRET_PARAMS, result) + + def test_get_secret_parameters_secret_root_array(self): + result = secrets.get_secret_parameters(TEST_SECRET_ROOT_ARRAY_SCHEMA) + self.assertEqual(TEST_SECRET_ROOT_ARRAY_SECRET_PARAMS, result) + + def test_get_secret_parameters_secret_root_object(self): + result = secrets.get_secret_parameters(TEST_SECRET_ROOT_OBJECT_SCHEMA) + self.assertEqual(TEST_SECRET_ROOT_OBJECT_SECRET_PARAMS, result) + + def test_get_secret_parameters_secret_nested_arrays(self): + result = secrets.get_secret_parameters(TEST_SECRET_NESTED_ARRAYS_SCHEMA) + self.assertEqual(TEST_SECRET_NESTED_ARRAYS_SECRET_PARAMS, result) + + def test_get_secret_parameters_secret_nested_objects(self): + result = secrets.get_secret_parameters(TEST_SECRET_NESTED_OBJECTS_SCHEMA) + self.assertEqual(TEST_SECRET_NESTED_OBJECTS_SECRET_PARAMS, result) + ############################################################################ - # TODO unit tests for mask_secret_parameters def test_mask_secret_parameters_flat(self): parameters = { @@ -459,6 +714,23 @@ def test_mask_secret_parameters_root_array(self): ] self.assertEqual(expected, result) + def test_mask_secret_parameters_root_object(self): + parameters = { + 'arg_level_one': + { + 'secret_field_in_object': 'Secret $tr!ng' + } + } + + result = secrets.mask_secret_parameters(parameters, TEST_ROOT_OBJECT_SECRET_PARAMS) + expected = { + 'arg_level_one': + { + 'secret_field_in_object': MASKED_ATTRIBUTE_VALUE + } + } + self.assertEqual(expected, result) + def test_mask_secret_parameters_nested_arrays(self): parameters = { 'arg_optional_array': [ @@ -669,3 +941,159 @@ def test_mask_secret_parameters_nested_array_with_object(self): ] } self.assertEqual(expected, result) + + def test_mask_secret_parameters_secret_array(self): + parameters = { + 'arg_secret_array': [ + "abc", + 123, + True + ] + } + result = secrets.mask_secret_parameters(parameters, + TEST_SECRET_ARRAY_SECRET_PARAMS) + expected = { + 'arg_secret_array': MASKED_ATTRIBUTE_VALUE + } + self.assertEqual(expected, result) + + def test_mask_secret_parameters_secret_object(self): + parameters = { + 'arg_secret_object': + { + "abc": 123, + "key": "value", + "bool": True, + "array": ["x", "y", "z"], + "obj": + { + "x": "deep" + } + } + } + result = secrets.mask_secret_parameters(parameters, + TEST_SECRET_OBJECT_SECRET_PARAMS) + expected = { + 'arg_secret_object': MASKED_ATTRIBUTE_VALUE + } + self.assertEqual(expected, result) + + def test_mask_secret_parameters_secret_root_array(self): + parameters = [ + "abc", + 123, + True + ] + result = secrets.mask_secret_parameters(parameters, + TEST_SECRET_ROOT_ARRAY_SECRET_PARAMS) + expected = MASKED_ATTRIBUTE_VALUE + self.assertEqual(expected, result) + + def test_mask_secret_parameters_secret_root_object(self): + parameters = { + 'arg_level_one': + { + 'secret_field_in_object': 'Secret $tr!ng' + } + } + result = secrets.mask_secret_parameters(parameters, + TEST_SECRET_ROOT_OBJECT_SECRET_PARAMS) + expected = MASKED_ATTRIBUTE_VALUE + self.assertEqual(expected, result) + + def test_mask_secret_parameters_secret_nested_arrays(self): + parameters = { + 'arg_optional_array': [ + 'secret 1', + 'secret 2', + 'secret 3', + ], + 'arg_optional_double_array': [ + [ + 'secret 4', + 'secret 5', + 'secret 6', + ], + [ + 'secret 7', + 'secret 8', + 'secret 9', + ] + ], + 'arg_optional_tripple_array': [ + [ + [ + 'secret 10', + 'secret 11' + ], + [ + 'secret 12', + 'secret 13', + 'secret 14' + ] + ], + [ + [ + 'secret 15', + 'secret 16' + ] + ] + ], + 'arg_optional_quad_array': [ + [ + [ + [ + 'secret 17', + 'secret 18' + ], + [ + 'secret 19' + ] + ] + ] + ] + } + + result = secrets.mask_secret_parameters(parameters, + TEST_SECRET_NESTED_ARRAYS_SECRET_PARAMS) + + expected = { + 'arg_optional_array': MASKED_ATTRIBUTE_VALUE, + 'arg_optional_double_array': MASKED_ATTRIBUTE_VALUE, + 'arg_optional_tripple_array': [ + MASKED_ATTRIBUTE_VALUE, + MASKED_ATTRIBUTE_VALUE, + ], + 'arg_optional_quad_array': [ + [ + MASKED_ATTRIBUTE_VALUE, + ] + ] + } + self.assertEqual(expected, result) + + def test_mask_secret_parameters_secret_nested_objects(self): + parameters = { + 'arg_object': { + 'arg_nested_secret': 'nested Secret', + 'arg_nested_object': { + 'arg_double_nested_secret': 'double nested $ecret', + } + }, + 'arg_secret_object': { + 'arg_nested_secret': 'secret data', + 'arg_nested_object': { + 'arg_double_nested_secret': 'double nested $ecret', + } + } + } + result = secrets.mask_secret_parameters(parameters, + TEST_SECRET_NESTED_OBJECTS_SECRET_PARAMS) + expected = { + 'arg_object': { + 'arg_nested_secret': MASKED_ATTRIBUTE_VALUE, + 'arg_nested_object': MASKED_ATTRIBUTE_VALUE, + }, + 'arg_secret_object': MASKED_ATTRIBUTE_VALUE, + } + self.assertEqual(expected, result)