Merge pull request #555 from trezor/andrewkozlik/pin-dialogs

Show success and failure dialogs in change-pin.

core/src/apps/common/request_pin.py

@@ -112,3 +112,11 @@ async def verify_user_pin(
             prompt = "Wrong PIN, enter again"
 
     raise PinInvalid
+
+
+async def show_pin_invalid(ctx: wire.Context) -> None:
+    from apps.common.confirm import confirm
+
+    text = Text("Wrong PIN", ui.ICON_WRONG, ui.RED)
+    text.normal("The PIN you entered is", "invalid.")
+    await confirm(ctx, text, confirm=None, cancel="Close")

core/src/apps/common/sd_salt.py

@@ -29,9 +29,9 @@ async def wrong_card_dialog(ctx: Optional[wire.Context]) -> None:
     text.br_half()
     text.normal("Please unplug the", "device and insert a", "different card.")
     if ctx is None:
-        await Confirm(text, confirm=None)
+        await Confirm(text, confirm=None, cancel="Close")
     else:
-        await require_confirm(ctx, text, confirm=None)
+        await require_confirm(ctx, text, confirm=None, cancel="Close")
 
 
 async def insert_card_dialog(ctx: Optional[wire.Context]) -> None:
@@ -40,9 +40,9 @@ async def insert_card_dialog(ctx: Optional[wire.Context]) -> None:
     text.br_half()
     text.normal("Please unplug the", "device and insert your", "SD card.")
     if ctx is None:
-        await Confirm(text, confirm=None)
+        await Confirm(text, confirm=None, cancel="Close")
     else:
-        await require_confirm(ctx, text, confirm=None)
+        await require_confirm(ctx, text, confirm=None, cancel="Close")
 
 
 async def request_sd_salt(

core/src/apps/management/change_pin.py

@@ -4,7 +4,12 @@ from trezor.pin import pin_to_int
 from trezor.ui.text import Text
 
 from apps.common.confirm import require_confirm
-from apps.common.request_pin import request_pin_and_sd_salt, request_pin_confirm
+from apps.common.layout import show_success
+from apps.common.request_pin import (
+    request_pin_and_sd_salt,
+    request_pin_confirm,
+    show_pin_invalid,
+)
 
 if False:
     from trezor.messages.ChangePin import ChangePin
@@ -20,6 +25,7 @@ async def change_pin(ctx: wire.Context, msg: ChangePin) -> Success:
     # if changing pin, pre-check the entered pin before getting new pin
     if curpin and not msg.remove:
         if not config.check_pin(pin_to_int(curpin), salt):
+            await show_pin_invalid(ctx)
             raise wire.PinInvalid("PIN invalid")
 
     # get new pin
@@ -30,12 +36,22 @@ async def change_pin(ctx: wire.Context, msg: ChangePin) -> Success:
 
     # write into storage
     if not config.change_pin(pin_to_int(curpin), pin_to_int(newpin), salt, salt):
+        await show_pin_invalid(ctx)
         raise wire.PinInvalid("PIN invalid")
 
     if newpin:
-        return Success(message="PIN changed")
+        if curpin:
+            msg_screen = "changed your PIN."
+            msg_wire = "PIN changed"
         else:
-        return Success(message="PIN removed")
+            msg_screen = "enabled PIN protection."
+            msg_wire = "PIN enabled"
+    else:
+        msg_screen = "disabled PIN protection."
+        msg_wire = "PIN removed"
+
+    await show_success(ctx, ("You have successfully", msg_screen))
+    return Success(message=msg_wire)
 
 
 def require_confirm_change_pin(ctx: wire.Context, msg: ChangePin) -> None:

core/src/apps/management/recovery_device/__init__.py

@@ -6,7 +6,11 @@ from trezor.ui.text import Text
 
 from apps.common import storage
 from apps.common.confirm import require_confirm
-from apps.common.request_pin import request_pin_and_sd_salt, request_pin_confirm
+from apps.common.request_pin import (
+    request_pin_and_sd_salt,
+    request_pin_confirm,
+    show_pin_invalid,
+)
 from apps.management.recovery_device.homescreen import recovery_process
 
 if False:
@@ -28,6 +32,7 @@ async def recovery_device(ctx: wire.Context, msg: RecoveryDevice) -> Success:
     if msg.dry_run:
         curpin, salt = await request_pin_and_sd_salt(ctx, "Enter PIN")
         if not config.check_pin(pin_to_int(curpin), salt):
+            await show_pin_invalid(ctx)
             raise wire.PinInvalid("PIN invalid")
 
     # set up pin if requested

core/src/apps/management/sd_protect.py

@@ -7,7 +7,12 @@ from trezor.pin import pin_to_int
 from trezor.ui.text import Text
 
 from apps.common.confirm import require_confirm
-from apps.common.request_pin import request_pin_ack, request_pin_and_sd_salt
+from apps.common.layout import show_success
+from apps.common.request_pin import (
+    request_pin_ack,
+    request_pin_and_sd_salt,
+    show_pin_invalid,
+)
 from apps.common.sd_salt import (
     SD_SALT_AUTH_KEY_LEN_BYTES,
     SD_SALT_AUTH_TAG_LEN_BYTES,
@@ -71,10 +76,12 @@ async def sd_protect_enable(ctx: wire.Context, msg: SdProtect) -> Success:
             # SD-protection. If it fails for any reason, we suppress the
             # exception, because primarily we need to raise wire.PinInvalid.
             pass
+        await show_pin_invalid(ctx)
         raise wire.PinInvalid("PIN invalid")
 
     device.set_sd_salt_auth_key(salt_auth_key)
 
+    await show_success(ctx, ("You have successfully", "enabled SD protection."))
     return Success(message="SD card protection enabled")
 
 
@@ -90,6 +97,7 @@ async def sd_protect_disable(ctx: wire.Context, msg: SdProtect) -> Success:
 
     # Check PIN and remove salt.
     if not config.change_pin(pin_to_int(pin), pin_to_int(pin), salt, None):
+        await show_pin_invalid(ctx)
         raise wire.PinInvalid("PIN invalid")
 
     device.set_sd_salt_auth_key(None)
@@ -103,6 +111,7 @@ async def sd_protect_disable(ctx: wire.Context, msg: SdProtect) -> Success:
         # because overall SD-protection was successfully disabled.
         pass
 
+    await show_success(ctx, ("You have successfully", "disabled SD protection."))
     return Success(message="SD card protection disabled")
 
 
@@ -128,6 +137,7 @@ async def sd_protect_refresh(ctx: wire.Context, msg: SdProtect) -> Success:
         raise wire.ProcessError("Failed to write to SD card")
 
     if not config.change_pin(pin_to_int(pin), pin_to_int(pin), old_salt, new_salt):
+        await show_pin_invalid(ctx)
         raise wire.PinInvalid("PIN invalid")
 
     device.set_sd_salt_auth_key(new_salt_auth_key)
@@ -141,6 +151,7 @@ async def sd_protect_refresh(ctx: wire.Context, msg: SdProtect) -> Success:
         # SD-protection was successfully refreshed.
         pass
 
+    await show_success(ctx, ("You have successfully", "refreshed SD protection."))
     return Success(message="SD card protection refreshed")
 
 

core/src/apps/webauthn/__init__.py

@@ -599,7 +599,7 @@ class U2fConfirmRegister(U2fState):
             text.normal(
                 "Another U2F device", "was used to register", "in this application."
             )
-            return await confirm(text, confirm=None)
+            return await confirm(text, confirm=None, cancel="Close")
         else:
             content = ConfirmContent(self)
             return await confirm(content)
@@ -724,7 +724,7 @@ class Fido2ConfirmExcluded(Fido2ConfirmMakeCredential):
 
         text = Text("FIDO2 Register", ui.ICON_WRONG, ui.RED)
         text.normal("This device is already", "registered with", self._cred.rp_id + ".")
-        await confirm(text, confirm=None)
+        await confirm(text, confirm=None, cancel="Close")
 
 
 class Fido2ConfirmGetAssertion(Fido2State, ConfirmInfo, Pageable):
@@ -797,7 +797,7 @@ class Fido2ConfirmNoPin(State):
     async def confirm_dialog(self) -> bool:
         text = Text("FIDO2 Verify User", ui.ICON_WRONG, ui.RED)
         text.normal("Unable to verify user.", "Please enable PIN", "protection.")
-        return await confirm(text, confirm=None)
+        return await confirm(text, confirm=None, cancel="Close")
 
 
 class Fido2ConfirmNoCredentials(Fido2ConfirmGetAssertion):
@@ -814,7 +814,7 @@ class Fido2ConfirmNoCredentials(Fido2ConfirmGetAssertion):
         text.normal(
             "This device is not", "registered with", self._creds[0].app_name() + "."
         )
-        await confirm(text, confirm=None)
+        await confirm(text, confirm=None, cancel="Close")
 
 
 class Fido2ConfirmReset(Fido2State):

core/src/apps/webauthn/add_resident_credential.py

@@ -42,7 +42,7 @@ async def add_resident_credential(
             "not belong to this",
             "authenticator.",
         )
-        await require_confirm(ctx, text, confirm=None)
+        await require_confirm(ctx, text, confirm=None, cancel="Close")
         raise wire.ActionCancelled("Cancelled")
 
     content = ConfirmContent(ConfirmAddCredential(cred))

tests/device_tests/test_msg_changepin_t2.py

@@ -35,6 +35,9 @@ def _input_flow_set_pin(debug, pin):
     yield  # enter new pin again
     print(f"reenter pin {pin}")
     debug.input(pin)
+    yield  # success
+    print("success")
+    debug.press_yes()
 
 
 def _input_flow_change_pin(debug, old_pin, new_pin):
@@ -46,6 +49,8 @@ def _input_flow_change_pin(debug, old_pin, new_pin):
     debug.input(new_pin)
     yield  # enter new pin again
     debug.input(new_pin)
+    yield  # success
+    debug.press_yes()
 
 
 def _input_flow_clear_pin(debug, old_pin):
@@ -53,6 +58,8 @@ def _input_flow_clear_pin(debug, old_pin):
     debug.press_yes()
     yield  # enter current pin
     debug.input(old_pin)
+    yield  # success
+    debug.press_yes()
 
 
 def _check_pin(client, pin):
@@ -61,7 +68,7 @@ def _check_pin(client, pin):
 
     with client:
         client.set_expected_responses(
-            [messages.ButtonRequest()] * 4 + [messages.Success(), messages.Features()]
+            [messages.ButtonRequest()] * 5 + [messages.Success(), messages.Features()]
         )
         client.set_input_flow(_input_flow_change_pin(client.debug, pin, pin))
         device.change_pin(client)
@@ -77,9 +84,9 @@ def _check_no_pin(client):
 
     with client:
         client.set_expected_responses(
-            [messages.ButtonRequest()] * 3
+            [messages.ButtonRequest()] * 4
             + [messages.Success(), messages.Features()]
-            + [messages.ButtonRequest()] * 2
+            + [messages.ButtonRequest()] * 3
             + [messages.Success(), messages.Features()]
         )
         client.set_input_flow(input_flow)
@@ -98,7 +105,7 @@ def test_set_pin(client):
     # Let's set new PIN
     with client:
         client.set_expected_responses(
-            [messages.ButtonRequest()] * 3 + [messages.Success(), messages.Features()]
+            [messages.ButtonRequest()] * 4 + [messages.Success(), messages.Features()]
         )
         client.set_input_flow(_input_flow_set_pin(client.debug, PIN6))
 
@@ -119,7 +126,7 @@ def test_change_pin(client):
     # Let's change PIN
     with client:
         client.set_expected_responses(
-            [messages.ButtonRequest()] * 4 + [messages.Success(), messages.Features()]
+            [messages.ButtonRequest()] * 5 + [messages.Success(), messages.Features()]
         )
         client.set_input_flow(_input_flow_change_pin(client.debug, PIN4, PIN6))
 
@@ -142,7 +149,7 @@ def test_remove_pin(client):
     # Let's remove PIN
     with client:
         client.set_expected_responses(
-            [messages.ButtonRequest()] * 2 + [messages.Success(), messages.Features()]
+            [messages.ButtonRequest()] * 3 + [messages.Success(), messages.Features()]
         )
         client.set_input_flow(_input_flow_clear_pin(client.debug, PIN4))