From 9ce6056cd3db28307ca7c3a82afca66982bf89ee Mon Sep 17 00:00:00 2001 From: Ivan Dimov <78815270+idimov-keeper@users.noreply.github.com> Date: Wed, 13 Sep 2023 16:53:12 -0500 Subject: [PATCH 1/7] fix for record's data JSON where "fields": null added test for clients that do not store "fields":[] or set it to null --- .../keeper_secrets_manager_core/dto/dtos.py | 2 ++ .../core/keeper_secrets_manager_core/mock.py | 2 +- sdk/python/core/tests/record_test.py | 25 +++++++++++++++++++ 3 files changed, 28 insertions(+), 1 deletion(-) diff --git a/sdk/python/core/keeper_secrets_manager_core/dto/dtos.py b/sdk/python/core/keeper_secrets_manager_core/dto/dtos.py index f346393c..a71e425c 100644 --- a/sdk/python/core/keeper_secrets_manager_core/dto/dtos.py +++ b/sdk/python/core/keeper_secrets_manager_core/dto/dtos.py @@ -54,6 +54,8 @@ def __init__(self, record_dict, secret_key, folder_uid = ''): self.raw_json = record_data_json self.dict = utils.json_to_dict(self.raw_json) + if self.dict and self.dict.get('fields') is None: + self.dict['fields'] = [] self.title = self.dict.get('title') self.type = self.dict.get('type') self.revision = record_dict.get('revision') diff --git a/sdk/python/core/keeper_secrets_manager_core/mock.py b/sdk/python/core/keeper_secrets_manager_core/mock.py index ebe7b605..0665590b 100644 --- a/sdk/python/core/keeper_secrets_manager_core/mock.py +++ b/sdk/python/core/keeper_secrets_manager_core/mock.py @@ -390,7 +390,7 @@ def add_file(self, name, title=None, content_type=None, url=None, content=None, def dump(self, secret, flags=None): - fields = list(self._fields) + fields = self._fields # If no files, the JSON has null files = None diff --git a/sdk/python/core/tests/record_test.py b/sdk/python/core/tests/record_test.py index 0f36b53e..b4ed062d 100644 --- a/sdk/python/core/tests/record_test.py +++ b/sdk/python/core/tests/record_test.py @@ -100,3 +100,28 @@ def test_record_field(self): assert "privacyScreen" not in value, "privacyScreen exists in dictionary" self.assertIsNone(value.get("privacyScreen"), "privacyScreen is not correct") self.assertIsNone(value.get("complexity"), "complexity is not correct") + + def test_missing_fields_section(self): + + """ Test for clients that may set "fields": null in JSON data """ + + try: + with tempfile.NamedTemporaryFile("w", delete=False) as fh: + fh.write(MockConfig.make_json()) + fh.seek(0) + secrets_manager = SecretsManager(config=FileKeyValueStorage(config_file_location=fh.name)) + + res = mock.Response() + rec = res.add_record(title="MyLogin", record_type='login') + res.records[rec.uid]._fields = None + res_queue = mock.ResponseQueue(client=secrets_manager) + res_queue.add_response(res) + + records = secrets_manager.get_secrets() + self.assertEqual(1, len(records), "didn't get 1 record for MyLogin") + self.assertEqual([], records[0].dict.get('fields')) + finally: + try: + os.unlink(fh.name) + except OSError: + pass From 1b3268932f8054205cd5f6216a401f5fc81be81d Mon Sep 17 00:00:00 2001 From: Ivan Dimov <78815270+idimov-keeper@users.noreply.github.com> Date: Wed, 13 Sep 2023 18:12:03 -0500 Subject: [PATCH 2/7] auto-format record tests --- sdk/python/core/tests/record_test.py | 72 ++++++++++++++++++---------- 1 file changed, 46 insertions(+), 26 deletions(-) diff --git a/sdk/python/core/tests/record_test.py b/sdk/python/core/tests/record_test.py index b4ed062d..b5012d5d 100644 --- a/sdk/python/core/tests/record_test.py +++ b/sdk/python/core/tests/record_test.py @@ -1,6 +1,6 @@ -import unittest -import tempfile import os +import tempfile +import unittest from keeper_secrets_manager_core.storage import FileKeyValueStorage from keeper_secrets_manager_core import SecretsManager @@ -20,34 +20,40 @@ def tearDown(self): os.chdir(self.orig_working_dir) def test_the_login_record_password(self): - - """ If the record type is login, the password will be placed in the instance attribute. + """ If the record type is login, the password will be placed + in the instance attribute. """ try: with tempfile.NamedTemporaryFile("w", delete=False) as fh: fh.write(MockConfig.make_json()) fh.seek(0) - secrets_manager = SecretsManager(config=FileKeyValueStorage(config_file_location=fh.name)) + secrets_manager = SecretsManager( + config=FileKeyValueStorage(config_file_location=fh.name)) # A good record. - # 'fields': [...{'type': 'password', 'value': ['My Password']}...] + # 'fields':[{'type': 'password', 'value': ['My Password']}...] good_res = mock.Response() - good = good_res.add_record(title="Good Record", record_type='login') + good = good_res.add_record( + title="Good Record", record_type='login') good.field("login", "My Login") good.field("password", "My Password") - # A bad record. This would be like if someone removed a password text from an existing field. + # A bad record. This would be like if someone removed + # a password text from an existing field. # 'fields': [...{'type': 'password', 'value': []}...] bad_res = mock.Response() - bad = bad_res.add_record(title="Bad Record", record_type='login') + bad = bad_res.add_record( + title="Bad Record", record_type='login') bad.field("login", "My Login") bad.field("password", []) - # An ugly record. The application didn't even add the field. We need to set flags to prune empty fields. + # An ugly record. The application didn't even add the field. + # We need to set flags to prune empty fields. # 'fields': [...] ugly_res = mock.Response(flags={"prune_empty_fields": True}) - ugly = ugly_res.add_record(title="Ugly Record", record_type='login') + ugly = ugly_res.add_record( + title="Ugly Record", record_type='login') ugly.field("login", "My Login") # this will be removed from the fields array. @@ -59,16 +65,23 @@ def test_the_login_record_password(self): res_queue.add_response(ugly_res) records = secrets_manager.get_secrets() - self.assertEqual(1, len(records), "didn't get 1 record for the good") - self.assertEqual("My Password", records[0].password, "did not get correct password for the good") + self.assertEqual( + 1, len(records), "didn't get 1 record for the good") + self.assertEqual( + "My Password", records[0].password, + "did not get correct password for the good") records = secrets_manager.get_secrets() - self.assertEqual(1, len(records), "didn't get 1 record for the bad") - self.assertIsNone(records[0].password, "password is defined for the bad") + self.assertEqual( + 1, len(records), "didn't get 1 record for the bad") + self.assertIsNone(records[0].password, + "password is defined for the bad") records = secrets_manager.get_secrets() - self.assertEqual(1, len(records), "didn't get 1 record for the ugly") - self.assertIsNone(records[0].password, "password is defined for the ugly") + self.assertEqual( + 1, len(records), "didn't get 1 record for the ugly") + self.assertIsNone(records[0].password, + "password is defined for the ugly") finally: try: os.unlink(fh.name) @@ -77,7 +90,8 @@ def test_the_login_record_password(self): def test_record_field(self): - rf = RecordField(field_type="login", value="test", label="Test", required=True, enforceGeneration=False, + rf = RecordField(field_type="login", value="test", label="Test", + required=True, enforceGeneration=False, privacyScreen=True, complexity={"foo": "bar"}) value = helpers.obj_to_dict(rf) @@ -85,9 +99,12 @@ def test_record_field(self): self.assertEqual(["test"], value.get("value"), "value is not correct") self.assertEqual("Test", value.get("label"), "label is not correct") self.assertTrue(value.get("required"), "required is not correct") - self.assertFalse(value.get("enforceGeneration"), "enforceGeneration is not correct") - self.assertTrue(value.get("privacyScreen"), "privacyScreen is not correct") - self.assertIsNotNone(value.get("complexity"), "complexity is not correct") + self.assertFalse(value.get("enforceGeneration"), + "enforceGeneration is not correct") + self.assertTrue(value.get("privacyScreen"), + "privacyScreen is not correct") + self.assertIsNotNone(value.get("complexity"), + "complexity is not correct") rf = RecordField(field_type="login", value="test", privacyScreen=None) @@ -96,20 +113,22 @@ def test_record_field(self): self.assertEqual(["test"], value.get("value"), "value is not correct") self.assertIsNone(value.get("label"), "label is not correct") self.assertIsNone(value.get("required"), "required is not correct") - self.assertIsNone(value.get("enforceGeneration"), "enforceGeneration is not correct") + self.assertIsNone(value.get("enforceGeneration"), + "enforceGeneration is not correct") assert "privacyScreen" not in value, "privacyScreen exists in dictionary" - self.assertIsNone(value.get("privacyScreen"), "privacyScreen is not correct") + self.assertIsNone(value.get("privacyScreen"), + "privacyScreen is not correct") self.assertIsNone(value.get("complexity"), "complexity is not correct") def test_missing_fields_section(self): - """ Test for clients that may set "fields": null in JSON data """ try: with tempfile.NamedTemporaryFile("w", delete=False) as fh: fh.write(MockConfig.make_json()) fh.seek(0) - secrets_manager = SecretsManager(config=FileKeyValueStorage(config_file_location=fh.name)) + secrets_manager = SecretsManager( + config=FileKeyValueStorage(config_file_location=fh.name)) res = mock.Response() rec = res.add_record(title="MyLogin", record_type='login') @@ -118,7 +137,8 @@ def test_missing_fields_section(self): res_queue.add_response(res) records = secrets_manager.get_secrets() - self.assertEqual(1, len(records), "didn't get 1 record for MyLogin") + self.assertEqual( + 1, len(records), "didn't get 1 record for MyLogin") self.assertEqual([], records[0].dict.get('fields')) finally: try: From 0aa95a31495abe5465345dec5cbeab358738767a Mon Sep 17 00:00:00 2001 From: Ivan Dimov <78815270+idimov-keeper@users.noreply.github.com> Date: Thu, 14 Sep 2023 17:07:34 -0500 Subject: [PATCH 3/7] fixed windows tests --- integration/keeper_secrets_manager_cli/tests/misc_test.py | 2 +- integration/keeper_secrets_manager_cli/tests/secret_test.py | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/integration/keeper_secrets_manager_cli/tests/misc_test.py b/integration/keeper_secrets_manager_cli/tests/misc_test.py index c9cf87f3..fdbe4dc6 100644 --- a/integration/keeper_secrets_manager_cli/tests/misc_test.py +++ b/integration/keeper_secrets_manager_cli/tests/misc_test.py @@ -66,7 +66,7 @@ def test_config_mode_dog_food(self): sp = subprocess.run(["icacls.exe", Config.default_ini_file], capture_output=True) if sp.stderr is not None and sp.stderr.decode() != "": self.fail("Could not icacls.exe {}: {}".format(Config.default_ini_file,sp.stderr.decode())) - allowed_users = [user.lower(), "Administrators".lower()] + allowed_users = [user.decode().lower(), "Administrators".lower()] for line in sp.stdout.decode().split("\n"): parts = line[len(Config.default_ini_file):].split(":") if len(parts) == 2: diff --git a/integration/keeper_secrets_manager_cli/tests/secret_test.py b/integration/keeper_secrets_manager_cli/tests/secret_test.py index b187fc19..16c0f279 100644 --- a/integration/keeper_secrets_manager_cli/tests/secret_test.py +++ b/integration/keeper_secrets_manager_cli/tests/secret_test.py @@ -159,6 +159,7 @@ def test_get(self): ], catch_exceptions=False) self.assertEqual(0, result.exit_code, "the exit code was not 0") + print(f'result.output: {result.output}') # TODO: remove fields = json.loads(result.output) self.assertEqual(4, len(fields), "didn't find 4 objects in array") @@ -958,7 +959,7 @@ def test_template_record_types(self): self.assertIsInstance(data.get("fields"), list, "fields is not a list") - field = data.get("fields")[1] + field = data.get("fields")[0] self.assertEqual("login", field.get("type"), "field type is not login") self.assertIsNotNone(field.get("value"), "value was None") From d6fe1e051e10b013d3c336a2f5bd5094266f6b7d Mon Sep 17 00:00:00 2001 From: Ivan Dimov <78815270+idimov-keeper@users.noreply.github.com> Date: Thu, 14 Sep 2023 17:22:13 -0500 Subject: [PATCH 4/7] fixed broken test --- integration/keeper_secrets_manager_cli/tests/secret_test.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/integration/keeper_secrets_manager_cli/tests/secret_test.py b/integration/keeper_secrets_manager_cli/tests/secret_test.py index 16c0f279..b9b6ba67 100644 --- a/integration/keeper_secrets_manager_cli/tests/secret_test.py +++ b/integration/keeper_secrets_manager_cli/tests/secret_test.py @@ -154,12 +154,11 @@ def test_get(self): # JSON Output w/ JQ to stdout runner = CliRunner() result = runner.invoke(cli, [ - 'secret', 'get', '-u', one.uid, '--json', + 'secret', 'get', '-u', one.uid, '--json', '--deflate', '--query', 'fields[*]' ], catch_exceptions=False) self.assertEqual(0, result.exit_code, "the exit code was not 0") - print(f'result.output: {result.output}') # TODO: remove fields = json.loads(result.output) self.assertEqual(4, len(fields), "didn't find 4 objects in array") From b7e35988c2a4a278ccc68fb641c0e129ebee912e Mon Sep 17 00:00:00 2001 From: Ivan Dimov <78815270+idimov-keeper@users.noreply.github.com> Date: Thu, 14 Sep 2023 17:35:19 -0500 Subject: [PATCH 5/7] fixed broken tests --- integration/keeper_secrets_manager_cli/tests/secret_test.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/integration/keeper_secrets_manager_cli/tests/secret_test.py b/integration/keeper_secrets_manager_cli/tests/secret_test.py index b9b6ba67..0eccbf10 100644 --- a/integration/keeper_secrets_manager_cli/tests/secret_test.py +++ b/integration/keeper_secrets_manager_cli/tests/secret_test.py @@ -154,13 +154,13 @@ def test_get(self): # JSON Output w/ JQ to stdout runner = CliRunner() result = runner.invoke(cli, [ - 'secret', 'get', '-u', one.uid, '--json', '--deflate', + 'secret', 'get', '-u', one.uid, '--json', '--inflate', '--query', 'fields[*]' ], catch_exceptions=False) self.assertEqual(0, result.exit_code, "the exit code was not 0") fields = json.loads(result.output) - self.assertEqual(4, len(fields), "didn't find 4 objects in array") + self.assertEqual(6, len(fields), "didn't find 4 objects in array") # Text Output to file tf_name = self._make_temp_file() @@ -958,7 +958,7 @@ def test_template_record_types(self): self.assertIsInstance(data.get("fields"), list, "fields is not a list") - field = data.get("fields")[0] + field = data.get("fields")[1] self.assertEqual("login", field.get("type"), "field type is not login") self.assertIsNotNone(field.get("value"), "value was None") From 9d93cdb166b209c66d29b80f735a13164606b42b Mon Sep 17 00:00:00 2001 From: Ivan Dimov <78815270+idimov-keeper@users.noreply.github.com> Date: Thu, 14 Sep 2023 17:47:28 -0500 Subject: [PATCH 6/7] fixing broken test --- integration/keeper_secrets_manager_cli/tests/secret_test.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/integration/keeper_secrets_manager_cli/tests/secret_test.py b/integration/keeper_secrets_manager_cli/tests/secret_test.py index 0eccbf10..bcea820d 100644 --- a/integration/keeper_secrets_manager_cli/tests/secret_test.py +++ b/integration/keeper_secrets_manager_cli/tests/secret_test.py @@ -183,8 +183,9 @@ def test_get(self): result = runner.invoke(cli, [ 'secret', 'get', '-u', one.uid, '--query', '[*].fields[*].type', - '--force-array' + '--force-array', '--inflate', ], catch_exceptions=True) + print(f'result.output: {result.output}') # TODO: remove after test data = json.loads(result.output) self.assertEqual(4, len(data), "found 4 rows") self.assertEqual(0, result.exit_code, "the exit code was not 0") From 3ade3cd9dc820608cb94bc07f21bedecb5bfdcac Mon Sep 17 00:00:00 2001 From: Ivan Dimov <78815270+idimov-keeper@users.noreply.github.com> Date: Thu, 14 Sep 2023 18:01:57 -0500 Subject: [PATCH 7/7] fixing tests --- integration/keeper_secrets_manager_cli/tests/secret_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/integration/keeper_secrets_manager_cli/tests/secret_test.py b/integration/keeper_secrets_manager_cli/tests/secret_test.py index bcea820d..6c3dc57d 100644 --- a/integration/keeper_secrets_manager_cli/tests/secret_test.py +++ b/integration/keeper_secrets_manager_cli/tests/secret_test.py @@ -183,7 +183,7 @@ def test_get(self): result = runner.invoke(cli, [ 'secret', 'get', '-u', one.uid, '--query', '[*].fields[*].type', - '--force-array', '--inflate', + '--force-array', '--deflate', ], catch_exceptions=True) print(f'result.output: {result.output}') # TODO: remove after test data = json.loads(result.output)