Skip to content

Commit

Permalink
entry: Do not set protected=True on setters
Browse files Browse the repository at this point in the history
Setting a property would call _set_string_field which by default would
set the protected status of the attribute as "True".

In order to verify in tests if a property is protected we add a private
method _is_property_protected which shares the code with
is_custom_property_protected.

Fixes: #376
  • Loading branch information
A6GibKm committed Mar 19, 2024
1 parent e43ca6c commit 498bfcd
Show file tree
Hide file tree
Showing 2 changed files with 59 additions and 4 deletions.
31 changes: 27 additions & 4 deletions pykeepass/entry.py
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ def _get_string_field(self, key):
if field is not None:
return field.text

def _set_string_field(self, key, value, protected=True):
def _set_string_field(self, key, value, protected=None):
"""Create or overwrite a string field in an Entry
Args:
Expand All @@ -103,9 +103,22 @@ def _set_string_field(self, key, value, protected=True):
fields are decrypted immediately upon opening the database.
"""
field = self._xpath('String/Key[text()="{}"]/..'.format(key), history=True, first=True)

protected_str = None
if protected is None:
protected_field = self._xpath('String/Key[text()="{}"]/../Value'.format(key), first=True)
if protected_field is not None:
protected_str = protected_field.attrib.get("Protected")
else:
protected_str = str(protected)

if field is not None:
self._element.remove(field)
self._element.append(E.String(E.Key(key), E.Value(value, Protected=str(protected))))

if protected_str is None:
self._element.append(E.String(E.Key(key), E.Value(value)))
else:
self._element.append(E.String(E.Key(key), E.Value(value, Protected=protected_str)))

def _get_string_field_keys(self, exclude_reserved=False):
results = [x.find('Key').text for x in self._element.findall('String')]
Expand Down Expand Up @@ -183,7 +196,10 @@ def password(self):

@password.setter
def password(self, value):
return self._set_string_field('Password', value)
if self.password:
return self._set_string_field('Password', value)
else:
return self._set_string_field('Password', value, True)

@property
def url(self):
Expand Down Expand Up @@ -231,7 +247,10 @@ def otp(self):

@otp.setter
def otp(self, value):
return self._set_string_field('otp', value)
if self.otp:
return self._set_string_field('otp', value)
else:
return self._set_string_field('otp', value, True)

@property
def history(self):
Expand Down Expand Up @@ -338,6 +357,10 @@ def is_custom_property_protected(self, key):
"""
assert key not in reserved_keys, '{} is a reserved key'.format(key)
return self._is_property_protected(key)

def _is_property_protected(self, key):
"""Whether a property is protected."""
field = self._xpath('String/Key[text()="{}"]/../Value'.format(key), first=True)
if field is not None:
return field.attrib.get("Protected", "False") == "True"
Expand Down
32 changes: 32 additions & 0 deletions tests/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -1019,6 +1019,38 @@ def test_issue344(self):
e._element.xpath('Times/ExpiryTime')[0].text = None
self.assertEqual(e.expiry_time, None)

def test_issue376(self):
# Setting the properties of an entry should not change the Protected
# property
subgroup = self.kp.root_group
e = self.kp.add_entry(subgroup, 'banana_entry', 'user', 'pass')

self.assertEqual(e._is_property_protected('Password'), True)
self.assertEqual(e._is_property_protected('Title'), False)
self.assertEqual(e.otp, None)
self.assertEqual(e._is_property_protected('otp'), False)

e.title = 'pineapple'
e.password = 'pizza'
e.otp = 'aa'

self.assertEqual(e._is_property_protected('Password'), True)
self.assertEqual(e._is_property_protected('Title'), False)
self.assertEqual(e._is_property_protected('otp'), True)

# Using protected=None should not change the current status
e._set_string_field('XYZ', '1', protected=None)
self.assertEqual(e._is_property_protected('XYZ'), False)

e._set_string_field('XYZ', '1', protected=True)
self.assertEqual(e._is_property_protected('XYZ'), True)

e._set_string_field('XYZ', '1', protected=None)
self.assertEqual(e._is_property_protected('XYZ'), True)

e._set_string_field('XYZ', '1', protected=False)
self.assertEqual(e._is_property_protected('XYZ'), False)

class EntryFindTests4(KDBX4Tests, EntryFindTests3):
pass

Expand Down

0 comments on commit 498bfcd

Please sign in to comment.