From ff708b6e9060d7ebc5a2803ab484914c7caf4695 Mon Sep 17 00:00:00 2001
From: Szczepan Zalega <szczepan@nitrokey.com>
Date: Sat, 12 Aug 2023 15:40:07 +0200
Subject: [PATCH] use properties struct

---
 pynitrokey/cli/nk3/secrets.py  |  2 +-
 pynitrokey/nk3/secrets_app.py  | 50 +++++++++++++++++++++++++---------
 pynitrokey/test_secrets_app.py |  4 +--
 3 files changed, 40 insertions(+), 16 deletions(-)

diff --git a/pynitrokey/cli/nk3/secrets.py b/pynitrokey/cli/nk3/secrets.py
index 53a720bd..2644bb39 100644
--- a/pynitrokey/cli/nk3/secrets.py
+++ b/pynitrokey/cli/nk3/secrets.py
@@ -161,7 +161,7 @@ def update(
         def call(app: SecretsApp) -> None:
             app.update_credential(
                 name.encode(),
-                cred_new_id=new_name,
+                new_name=new_name,
                 login=login,
                 password=password,
                 metadata=metadata,
diff --git a/pynitrokey/nk3/secrets_app.py b/pynitrokey/nk3/secrets_app.py
index 0df78fa8..b3e9da4e 100644
--- a/pynitrokey/nk3/secrets_app.py
+++ b/pynitrokey/nk3/secrets_app.py
@@ -490,22 +490,39 @@ def get_credential(self, cred_id: bytes) -> PasswordSafeEntry:
         p.properties = p.properties.hex().encode() if p.properties else None
         return p
 
-    def rename_credential(self, cred_id: bytes, cred_new_id: bytes) -> None:
-        return self.update_credential(cred_id, cred_new_id)
+    def rename_credential(self, cred_id: bytes, new_name: bytes) -> None:
+        """
+        Rename credential.
+        An alias for the update_credential() call.
+        @param cred_id: The credential ID to modify
+        @param new_name: New ID for the credential
+        """
+        return self.update_credential(cred_id, new_name)
 
     def update_credential(
         self,
         cred_id: bytes,
-        cred_new_id: Optional[bytes] = None,
+        new_name: Optional[bytes] = None,
         login: Optional[bytes] = None,
         password: Optional[bytes] = None,
         metadata: Optional[bytes] = None,
         touch_button: Optional[bool] = None,
     ) -> None:
+        """
+        Update credential fields - name, attributes, and PWS fields.
+        Unpopulated fields will not be encoded and used during the update process
+        (won't change the current value).
+        @param cred_id: The credential ID to modify
+        @param new_name: New ID for the credential
+        @param login: New login field content
+        @param password: New password field content
+        @param metadata: New metadata field content
+        @param touch_button: Set if the touch button use should be required
+        """
         structure = [
             tlv8.Entry(Tag.CredentialId.value, cred_id),
-            tlv8.Entry(Tag.CredentialId.value, cred_new_id) if cred_new_id else None,
-            tlv8.Entry(Tag.Properties.value, b"\x01" if touch_button else b"\x00")
+            tlv8.Entry(Tag.CredentialId.value, new_name) if new_name else None,
+            self.encode_properties_to_send(touch_button, False, tlv=True)
             if touch_button is not None
             else None,
             tlv8.Entry(Tag.PwsLogin.value, login) if login is not None else None,
@@ -604,15 +621,22 @@ def register(
 
     @classmethod
     def encode_properties_to_send(
-        cls, touch_button_required: bool, pin_based_encryption: bool
+        cls, touch_button_required: bool, pin_based_encryption: bool, tlv: bool = False
     ) -> RawBytes:
-        return RawBytes(
-            [
-                Tag.Properties.value,
-                (0x02 if touch_button_required else 0x00)
-                | (0x04 if pin_based_encryption else 0x00),
-            ]
-        )
+        """
+        Encode properties structure into a single byte
+        @param touch_button_required: whether the touch button use is required
+        @param pin_based_encryption: whether the PIN-encryption is requested (only during registration)
+        @param tlv: set True, if this should be encoded as TLV, as opposed to the default "TV", w/o L
+        """
+        structure = [
+            Tag.Properties.value,
+            1 if tlv else None,
+            (0x02 if touch_button_required else 0x00)
+            | (0x04 if pin_based_encryption else 0x00),
+        ]
+        structure = list(filter(lambda x: x is not None, structure))
+        return RawBytes(structure)
 
     def calculate(self, cred_id: bytes, challenge: Optional[int] = None) -> bytes:
         """
diff --git a/pynitrokey/test_secrets_app.py b/pynitrokey/test_secrets_app.py
index 89f04c47..bc032353 100644
--- a/pynitrokey/test_secrets_app.py
+++ b/pynitrokey/test_secrets_app.py
@@ -1780,7 +1780,7 @@ def test_update_credential(secretsAppResetLogin):
     app.verify_pin_raw(PIN)
     app.update_credential(
         CREDID,
-        cred_new_id=CREDID2,
+        new_name=CREDID2,
         login=b"login",
         password=b"password",
         metadata=b"metadata",
@@ -1807,7 +1807,7 @@ def test_update_credential(secretsAppResetLogin):
     # with a single blank character.
     app.verify_pin_raw(PIN)
     app.update_credential(
-        CREDID2, cred_new_id=CREDID, login=b" ", password=b" ", metadata=b" "
+        CREDID2, new_name=CREDID, login=b" ", password=b" ", metadata=b" "
     )
     app.verify_pin_raw(PIN)
     c = app.get_credential(CREDID)