Skip to content

Commit

Permalink
KSM-458 Remove core dep on helper module
Browse files Browse the repository at this point in the history
Refactor add_custom_field not to use the helper module. The method
will accept an instance of FieldType, however it is not tied to the
keeper_secrets_manager_helper.v3.field_type.FieldType module.

Allow the ability to set the type, label, and value not using the
FieldType instance. The method will still take the param `field`,
however it will also take `field_type`, `label`, and `value`. This
allows adding a custom field without have to use the helper module.

Removed references `keeper-secrets-manager-helper` from setup.py
and requirements.txt. This should break the circular reference.

Added unit tests for the method.
  • Loading branch information
jwalstra-keeper committed Aug 22, 2023
1 parent 4b71c4e commit ee89659
Show file tree
Hide file tree
Showing 5 changed files with 112 additions and 12 deletions.
2 changes: 1 addition & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -64,4 +64,4 @@ keeper.ini*
.secrets
ansible.cfg

/.gradle
.gradle/
33 changes: 24 additions & 9 deletions sdk/python/core/keeper_secrets_manager_core/dto/dtos.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
from keeper_secrets_manager_core import utils, helpers
from keeper_secrets_manager_core.crypto import CryptoUtils
from keeper_secrets_manager_core.exceptions import KeeperError
from keeper_secrets_manager_helper.v3.field_type import FieldType


class Record:
Expand Down Expand Up @@ -184,16 +183,32 @@ def set_custom_field_value(self, field_type, value):
field["value"] = value
self._update()

def add_custom_field(self, field: FieldType) -> bool:
if issubclass(type(field), FieldType):
if self.dict.get('custom', None) is None:
self.dict['custom'] = []
custom = self.dict['custom']
def add_custom_field(self, field=None, field_type=None, label=None, value=None) -> bool:
if self.dict.get('custom', None) is None:
self.dict['custom'] = []
custom = self.dict['custom']

# Make backward compatible. Assumes keeper_secrets_manager_helper.v#.field_type.FieldType is passed in.
if field is not None:
if field.__class__.__name__ != "FieldType":
raise ValueError("The field is not an instance of FieldType")
fdict = field.to_dict()
custom.append(fdict)
self._update()
return True
return False
else:
if field_type is None:
return False
if isinstance(value, list) is False:
value = [value]
field_dict = {
"type": field_type,
"value": value
}
if label is not None:
field_dict["label"] = label
custom.append(field_dict)

self._update()
return True

# TODO: Deprecate this for better getter and setters
def field(self, field_type, value=None, single=False):
Expand Down
1 change: 0 additions & 1 deletion sdk/python/core/requirements.txt
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
keeper-secrets-manager-helper>=1.0.4
ecdsa
cryptography>=39.0.1
requests>=2.28.2
Expand Down
1 change: 0 additions & 1 deletion sdk/python/core/setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
long_description = fp.read()

install_requires = [
'keeper-secrets-manager-helper>=1.0.4',
'ecdsa',
'requests',
'cryptography>=39.0.1',
Expand Down
87 changes: 87 additions & 0 deletions sdk/python/core/tests/record_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -100,3 +100,90 @@ 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_add_custom_field_by_param(self):

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()
mock_record = res.add_record(title="Good Record", record_type='login')
mock_record.field("login", "My Login")
mock_record.field("password", "My Password")

res_queue = mock.ResponseQueue(client=secrets_manager)
res_queue.add_response(res)

records = secrets_manager.get_secrets()
record = records[0]

record.add_custom_field(
field_type='text',
label="My Label",
value="My Value"
)
new_value = record.get_custom_field("My Label")
self.assertEqual("text", new_value.get("type"))
self.assertEqual("My Label", new_value.get("label"))
self.assertEqual(['My Value'], new_value.get("value"))
finally:
try:
os.unlink(fh.name)
except OSError:
pass

def test_add_custom_field_by_field_type(self):

class FieldType:

def __init__(self, field_type, label, value):
self.field_type = field_type
self.label = label
self.value = value

def to_dict(self):
return {
"type": self.field_type,
"label": self.label,
"value": self.value
}

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()
mock_record = res.add_record(title="Good Record", record_type='login')
mock_record.field("login", "My Login")
mock_record.field("password", "My Password")

res_queue = mock.ResponseQueue(client=secrets_manager)
res_queue.add_response(res)

records = secrets_manager.get_secrets()
record = records[0]

field = FieldType(
field_type="text",
label="My Label",
value=["My Value"]
)

record.add_custom_field(field=field)

new_value = record.get_custom_field("My Label")
self.assertEqual("text", new_value.get("type"))
self.assertEqual("My Label", new_value.get("label"))
self.assertEqual(['My Value'], new_value.get("value"))
finally:
try:
os.unlink(fh.name)
except OSError:
pass


0 comments on commit ee89659

Please sign in to comment.