entry: Do not set protected=True on setters (#381)

* entry: Do not set protected=True on setters

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

* update docstring

---------

Co-authored-by: evan <[email protected]>

pykeepass/entry.py

@@ -92,20 +92,35 @@ 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:
             key (str): name of field
             value (str): value of field
-            protected (bool): mark whether the field should be protected in memory
-                in other tools.  This property is ignored in PyKeePass and all
-                fields are decrypted immediately upon opening the database.
+            protected (bool or None): mark whether the field should be protected in memory
+                in other tools.  If None, value is either copied from existing field or field
+                is created with protected property unset.
+
+        Note: pykeepass does not support memory protection
         """
         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')]
@@ -183,7 +198,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):
@@ -231,7 +249,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):
@@ -338,6 +359,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"

tests/tests.py

@@ -1047,6 +1047,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