-
-
Notifications
You must be signed in to change notification settings - Fork 273
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(suite): change copy for Device authentication ghost modal #16737
Conversation
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 ESLint
error Error: http://10.0.0.28:4873/@trezor%2feslint: no such package available WalkthroughThis pull request enhances the device authentication failure handling in the Trezor Suite application. The changes involve adding a new localized message for device authentication failures and updating the Changes
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
⏰ Context from checks skipped due to timeout of 90000ms (9)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/suite/src/views/onboarding/steps/SecurityCheck/DeviceAuthenticity.tsx (1)
116-119
: LGTM! Consider extracting the translation key as a constant.The text prop addition maintains consistency with AuthenticateDeviceFailModal.tsx. Consider extracting "TR_DEVICE_COMPROMISED_DEVICE_AUTHENTICITY_TEXT" as a constant since it's used in multiple components.
+const DEVICE_AUTHENTICITY_FAIL_MESSAGE = 'TR_DEVICE_COMPROMISED_DEVICE_AUTHENTICITY_TEXT'; + export const DeviceAuthenticity = ({ goToNext }: DeviceAuthenticityProps) => { // ... if (isCheckFailed) { return ( <StyledCard> <SecurityCheckFail supportUrl={TREZOR_SUPPORT_DEVICE_AUTHENTICATION_FAILED_URL} - text="TR_DEVICE_COMPROMISED_DEVICE_AUTHENTICITY_TEXT" + text={DEVICE_AUTHENTICITY_FAIL_MESSAGE} /> </StyledCard> ); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/suite/src/components/suite/modals/ReduxModal/UserContextModal/AuthenticateDeviceFailModal.tsx
(1 hunks)packages/suite/src/support/messages.ts
(1 hunks)packages/suite/src/views/onboarding/steps/SecurityCheck/DeviceAuthenticity.tsx
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (20)
- GitHub Check: e2e-test-suite-web (@group=wallet, trezor-user-env-unix bitcoin-regtest)
- GitHub Check: e2e-test-suite-web (@group=other, trezor-user-env-unix)
- GitHub Check: e2e-test-suite-web (@group=metadata, trezor-user-env-unix)
- GitHub Check: e2e-test-suite-web (@group=settings, trezor-user-env-unix)
- GitHub Check: e2e-test-suite-web (@group=device-management, trezor-user-env-unix)
- GitHub Check: e2e-test-suite-web (@group=suite, trezor-user-env-unix)
- GitHub Check: e2e-test-suite-web (@group_wallet, trezor-user-env-unix bitcoin-regtest, 1)
- GitHub Check: e2e-test-suite-web (@group_other, trezor-user-env-unix, 1)
- GitHub Check: e2e-test-suite-web (@group_passphrase, trezor-user-env-unix, 1)
- GitHub Check: e2e-test-suite-web (@group_device-management, trezor-user-env-unix, 1)
- GitHub Check: e2e-test-suite-web (@group_suite, trezor-user-env-unix, 1)
- GitHub Check: Linting and formatting
- GitHub Check: Type Checking
- GitHub Check: Unit Tests
- GitHub Check: run-desktop-tests (@group=wallet, trezor-user-env-unix bitcoin-regtest)
- GitHub Check: run-desktop-tests (@group=other, trezor-user-env-unix)
- GitHub Check: run-desktop-tests (@group=settings, trezor-user-env-unix bitcoin-regtest)
- GitHub Check: run-desktop-tests (@group=device-management, trezor-user-env-unix)
- GitHub Check: run-desktop-tests (@group=suite, trezor-user-env-unix)
- GitHub Check: Analyze with CodeQL (javascript)
🔇 Additional comments (2)
packages/suite/src/components/suite/modals/ReduxModal/UserContextModal/AuthenticateDeviceFailModal.tsx (1)
14-17
: LGTM! Verify translation key exists.The addition of the text prop with the translation key aligns with the PR objectives for consistent device authentication messaging.
Let's verify the translation key exists:
✅ Verification successful
Translation key verified and properly implemented
The key
TR_DEVICE_COMPROMISED_DEVICE_AUTHENTICITY_TEXT
exists in the messages file with appropriate default text and is consistently used across device authentication components.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for the translation key in messages files rg "TR_DEVICE_COMPROMISED_DEVICE_AUTHENTICITY_TEXT" -A 2Length of output: 1170
packages/suite/src/support/messages.ts (1)
6881-6884
: LGTM! Critical security message added for device authentication failures.The new message is clear, concise and appropriately communicates the security issue to users without causing unnecessary panic.
7e10610
to
0368e47
Compare
Description
Followup to #16599 - make subtitle copies consistent for FW revcheck, FW hashcheck-mismatch, and now also device authenticity. Or "authentication", as I just learned is the official term 🧐
Screenshots:
Device authentication failure during onboarding
![image](https://private-user-images.githubusercontent.com/5287903/408222742-4c62ef3a-3219-4818-9a71-33afbbd5a430.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3Mzg5MTQzNTYsIm5iZiI6MTczODkxNDA1NiwicGF0aCI6Ii81Mjg3OTAzLzQwODIyMjc0Mi00YzYyZWYzYS0zMjE5LTQ4MTgtOWE3MS0zM2FmYmJkNWE0MzAucG5nP1gtQW16LUFsZ29yaXRobT1BV1M0LUhNQUMtU0hBMjU2JlgtQW16LUNyZWRlbnRpYWw9QUtJQVZDT0RZTFNBNTNQUUs0WkElMkYyMDI1MDIwNyUyRnVzLWVhc3QtMSUyRnMzJTJGYXdzNF9yZXF1ZXN0JlgtQW16LURhdGU9MjAyNTAyMDdUMDc0MDU2WiZYLUFtei1FeHBpcmVzPTMwMCZYLUFtei1TaWduYXR1cmU9MGJiYWIzZTQ0M2IyMzMzODhlOGNiODU3ZDliNTE1N2Y1NGVkMTlkNzdkNTE2YmJkMzg0OTYzMDk1YmI2MTFlNSZYLUFtei1TaWduZWRIZWFkZXJzPWhvc3QifQ.e8_PogqHKGDR0jnoVZiqLRB5X6cotoCibItHWt-s5no)
Device authentication failure prompted by button in Settings > Device
![image](https://private-user-images.githubusercontent.com/5287903/408222473-c2bbe192-755b-490a-9638-d28a0a1d6693.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3Mzg5MTQzNTYsIm5iZiI6MTczODkxNDA1NiwicGF0aCI6Ii81Mjg3OTAzLzQwODIyMjQ3My1jMmJiZTE5Mi03NTViLTQ5MGEtOTYzOC1kMjhhMGExZDY2OTMucG5nP1gtQW16LUFsZ29yaXRobT1BV1M0LUhNQUMtU0hBMjU2JlgtQW16LUNyZWRlbnRpYWw9QUtJQVZDT0RZTFNBNTNQUUs0WkElMkYyMDI1MDIwNyUyRnVzLWVhc3QtMSUyRnMzJTJGYXdzNF9yZXF1ZXN0JlgtQW16LURhdGU9MjAyNTAyMDdUMDc0MDU2WiZYLUFtei1FeHBpcmVzPTMwMCZYLUFtei1TaWduYXR1cmU9MTU4MDBmMGQyYjY2MGI4OTU4NmMwNmU4MGY3ZTM1OWFmNTk0NjZkOGRlZTkzOWYyNzFkNzMzNDA1ZWU1YjM2MiZYLUFtei1TaWduZWRIZWFkZXJzPWhvc3QifQ.21DllJRqSKNI2uQAfEL_qNZDvVb0FbcwnpKW_0Stk9Q)
Summary by CodeRabbit
New Features
Documentation
The changes improve user awareness and provide clearer information when a device authentication check fails.