chore: Refactor OAuthHydration screen with unlockWallet#24572
Conversation
…/6008-abstract-auth-api-cal
…rors are encountered
| mockAlert.mockRestore(); | ||
| }); | ||
|
|
||
| it('navigates to rehydrate screen when seedless onboarding error is detected', async () => { |
There was a problem hiding this comment.
Add unit test for ensuring that Login will navigate to OAuthRehydration whenever it encounters seedless onboarding errors
| const [rehydrationFailedAttempts, setRehydrationFailedAttempts] = useState(0); | ||
| const [loading, setLoading] = useState(false); | ||
| const [error, setError] = useState<string | null>( | ||
| isSeedlessPasswordOutdated |
There was a problem hiding this comment.
Since isSeedlessPasswordOutdated is available, we can just set the error message by default
| }); | ||
|
|
||
| try { | ||
| const locked = !passwordRequirementsMet(password); |
There was a problem hiding this comment.
Since the purpose of this is to log in, we do not want to show users a password requirement error
| op: TraceOperation.Login, | ||
| }, | ||
| async () => { | ||
| await Authentication.userEntryAuth(password, authType); |
There was a problem hiding this comment.
Replace userEntryAuth with unlockWallet
| endTrace({ name: TraceName.OnboardingExistingSocialLogin }); | ||
| endTrace({ name: TraceName.OnboardingJourneyOverall }); | ||
|
|
||
| await navigateToHome(); |
There was a problem hiding this comment.
unlockWallet navigates to home automatically
|
|
||
| const newGlobalPasswordLogin = useCallback(async () => { | ||
| try { | ||
| const locked = !passwordRequirementsMet(password); |
There was a problem hiding this comment.
Same comment on removing the password requirement message
| op: TraceOperation.Login, | ||
| }, | ||
| async () => { | ||
| await Authentication.userEntryAuth(password, authType); |
There was a problem hiding this comment.
Replace userEntryAuth with unlockWallet
| }, [route.params?.onboardingTraceCtx]); | ||
|
|
||
| // Handle password outdated state | ||
| useEffect(() => { |
There was a problem hiding this comment.
Remove the need to auto clear credentials. We want to avoid clearing credentials by itself unless absolutely needed since this risks throwing the credential dependencies out of sync. If we need to clear credentials, it's safer to ensure that all necessary states are being reset together. In a happy path, by removing this reset, UX would remain the same and the credentials will eventually be reset by successfully unlocking the wallet.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| // Arrange | ||
| (Authentication.userEntryAuth as jest.Mock).mockRejectedValue( | ||
| new Error('Error: Decrypt failed'), | ||
| ); |
There was a problem hiding this comment.
Test passes for wrong reason after validation removal
Low Severity
The test "prevents login when password is too short" now passes for the wrong reason. The PR removed the passwordRequirementsMet validation, so short passwords are now allowed to be submitted to unlockWallet. The test asserts mockReplace is not called, which is true - but only because the component no longer calls navigation.replace() directly (navigation is now handled by unlockWallet). Login actually proceeds with the short password; the test title is misleading and should be removed or updated to reflect the new behavior.
🔍 Smart E2E Test Selection
click to see 🤖 AI reasoning detailsThe changes modify the Login and OAuthRehydration views, which handle authentication flows. The key changes are:
These are internal refactoring changes that don't modify UI elements or test IDs. The E2E tests that interact with the Login screen (change-password.spec.ts, auto-lock.spec.ts) use the SmokeAccounts tag. Running SmokeAccounts tests will verify that:
The risk is medium because while the changes are refactoring, they touch core authentication logic that could affect login behavior. The unit tests have been updated to cover the new behavior, but E2E validation of the login flow is prudent. |
|
|
Missing release label release-7.64.0 on PR. Adding release label release-7.64.0 on PR and removing other release labels(release-7.65.0), as PR was added to branch 7.64.0 when release was cut. |
Description
This is part of the effort to consolidate and refactor areas calling
userEntryAuthorappTriggeredAuth. This PR refactors theOAuthHydrationscreen to replaceappTriggeredAuthwithunlockWallet.Changelog
CHANGELOG entry:
Related issues
Fixes:
Manual testing steps
While using seedless onboarding and global password is outdated
While first onboarding with seedless onboarding on mobile, with existing account on extension
Screenshots/Recordings
Before
After
Pre-merge author checklist
Pre-merge reviewer checklist
Note
Modernizes authentication flows and centralizes logic via
useAuthentication.Authentication.userEntryAuth/appTriggeredAuthwithuseAuthentication.unlockWalletinLoginandOAuthRehydrationcomponentAuthenticationType; remove password requirement pre-checks andresetPasswordside effectscontainsErrorMessage, treatSeedlessOnboardingControllerErrorvia navigation toREHYDRATE, handle biometric cancellations, vault corruption, passcode-not-set, and seedless-specific errors (incorrect password, too many attempts, password recently updated)isSeedlessPasswordOutdatedand streamline loading/biometry flagsuseAuthentication, cover new seedless cases, offline handling, and error sanitizationWritten by Cursor Bugbot for commit c7d3a69. This will update automatically on new commits. Configure here.