Update credential support

Add tests for credential update with PWS and touch button requirement

pynitrokey/nk3/secrets_app.py

@@ -217,6 +217,7 @@ class Instruction(Enum):
     SetPIN = 0xB4
     GetCredential = 0xB5
     RenameCredential = 0xB6
+    UpdateCredential = 0xB7
 
 
 class Tag(Enum):
@@ -490,11 +491,32 @@ def get_credential(self, cred_id: bytes) -> PasswordSafeEntry:
         return p
 
     def rename_credential(self, cred_id: bytes, cred_new_id: bytes) -> None:
+        return self.update_credential(cred_id, cred_new_id)
+
+    def update_credential(
+        self,
+        cred_id: bytes,
+        cred_new_id: Optional[bytes] = None,
+        login: Optional[bytes] = None,
+        password: Optional[bytes] = None,
+        metadata: Optional[bytes] = None,
+        touch_button: Optional[bool] = None,
+    ) -> None:
         structure = [
             tlv8.Entry(Tag.CredentialId.value, cred_id),
-            tlv8.Entry(Tag.CredentialId.value, cred_new_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 is not None
+            else None,
+            tlv8.Entry(Tag.PwsLogin.value, login) if login is not None else None,
+            tlv8.Entry(Tag.PwsPassword.value, password)
+            if password is not None
+            else None,
+            tlv8.Entry(Tag.PwsMetadata.value, metadata)
+            if metadata is not None
+            else None,
         ]
-        self._send_receive(Instruction.RenameCredential, structure=structure)
+        self._send_receive(Instruction.UpdateCredential, structure=structure)
 
     def delete(self, cred_id: bytes) -> None:
         """
@@ -566,13 +588,7 @@ def register(
             tlv8.Entry(
                 Tag.Key.value, bytes([kind.value | algo.value, digits]) + secret
             ),
-            RawBytes(
-                [
-                    Tag.Properties.value,
-                    (0x02 if touch_button_required else 0x00)
-                    | (0x04 if pin_based_encryption else 0x00),
-                ]
-            ),
+            self.encode_properties_to_send(touch_button_required, pin_based_encryption),
             tlv8.Entry(
                 Tag.InitialCounter.value, initial_counter_value.to_bytes(4, "big")
             )
@@ -585,6 +601,19 @@ def register(
         structure = list(filter(lambda x: x is not None, structure))
         self._send_receive(Instruction.Put, structure)
 
+    @classmethod
+    def encode_properties_to_send(
+        cls, touch_button_required: bool, pin_based_encryption: bool
+    ) -> bytes:
+        r = RawBytes(
+            [
+                Tag.Properties.value,
+                (0x02 if touch_button_required else 0x00)
+                | (0x04 if pin_based_encryption else 0x00),
+            ]
+        )
+        return r
+
     def calculate(self, cred_id: bytes, challenge: Optional[int] = None) -> bytes:
         """
         Calculate the OTP code for the credential named `cred_id`, and with challenge `challenge`.

pynitrokey/test_secrets_app.py

@@ -1703,6 +1703,11 @@ def test_rename_credential(secretsAppResetLogin):
     assert len(l) == 1
     assert l[0].decode() == CREDID2
 
+    # Old name should not be accessible
+    app.verify_pin_raw(PIN)
+    with pytest.raises(SecretsAppException, match="NotFound"):
+        app.get_credential(CREDID)
+
 
 @pytest.mark.parametrize(
     "cred1_encryption", [True, False], ids=lambda x: "cred1" + ("_enc" if x else "")
@@ -1743,3 +1748,69 @@ def test_rename_credential_to_existing(
     app.verify_pin_raw(PIN)
     # There should be still 2 credentials left
     assert set([CREDID.encode(), CREDID2.encode()]) == set(app.list())
+
+
+def test_update_credential(secretsAppResetLogin):
+    """
+    Credential should change its properties. Test both PIN- and HW-encrypted credentials.
+    """
+    app = secretsAppResetLogin
+    app.register(CREDID, SECRET, DIGITS, kind=Kind.Hotp)
+    app.verify_pin_raw(PIN)
+    l = app.list_with_properties()
+    assert len(l) == 1
+    assert l[0].label.decode() == CREDID
+    assert not l[0].properties.touch_required
+    app.verify_pin_raw(PIN)
+    app.update_credential(CREDID, touch_button=True)
+    # There should be only one credential left, with the same name
+    app.verify_pin_raw(PIN)
+    l = app.list_with_properties()
+    assert len(l) == 1
+    assert l[0].label.decode() == CREDID
+    # This credential should be listed now as requiring a touch button for use
+    assert l[0].properties.touch_required
+
+    # Now add some PWS fields, and rename it too
+    app.verify_pin_raw(PIN)
+    c = app.get_credential(CREDID)
+    assert not c.login
+    assert not c.password
+    assert not c.metadata
+    app.verify_pin_raw(PIN)
+    app.update_credential(
+        CREDID,
+        cred_new_id=CREDID2,
+        login=b"login",
+        password=b"password",
+        metadata=b"metadata",
+    )
+
+    # Check if PWS fields are there, and the "touch button required" flag is still present
+    app.verify_pin_raw(PIN)
+    c = app.get_credential(CREDID2)
+    assert c.login == b"login"
+    assert c.password == b"password"
+    assert c.metadata == b"metadata"
+    app.verify_pin_raw(PIN)
+    l = app.list_with_properties()
+    assert len(l) == 1
+    assert l[0].properties.touch_required
+
+    # Old name should not be accessible
+    app.verify_pin_raw(PIN)
+    with pytest.raises(SecretsAppException, match="NotFound"):
+        app.get_credential(CREDID)
+
+    # Try to remove the PWS data, and rename again
+    # It's not possible currently to remove the PWS fields; hence we need to overwrite them
+    # with a single blank character.
+    app.verify_pin_raw(PIN)
+    app.update_credential(
+        CREDID2, cred_new_id=CREDID, login=b" ", password=b" ", metadata=b" "
+    )
+    app.verify_pin_raw(PIN)
+    c = app.get_credential(CREDID)
+    assert c.login == b" "
+    assert c.password == b" "
+    assert c.metadata == b" "