Skip to content
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

fix: loop on mismatched passphrase entry #5396

Merged

Conversation

AaronFeickert
Copy link
Collaborator

Description

If the user enters a new or changed passphrase and fails to confirm it, an exit error is immediately returned. This PR updates to prompt the user to try again, up to a sanity limit.

Closes issue 5391.

Motivation and Context

The user is prompted to enter and confirm a new passphrase in two cases: when creating a new wallet, and when changing the passphrase of an existing wallet. In both cases, failure to confirm the new passphrase correctly returns an exit error.

This PR prompts the user again if the new passphrase and its confirmation do not match.

We probably don't want the user to get stuck in an infinite loop and go insane, so there is a sanity limit of three tries. If the user fails this many times, we return the exit error.

How Has This Been Tested?

Tested manually.

What process can a PR reviewer use to test or verify this change?

Test the following scenarios manually:

  • Create a new wallet. Fail to confirm a new passphrase three times. Confirm that the process exits.
  • Create a new wallet. Confirm a new passphrase within three tries. Confirm that the process succeeds.
  • Change an existing wallet's passphrase. Fail to confirm a new passphrase three times. Confirm that the process exits.
  • Change an existing wallet's passphrase. Confirm a new passphrase within three tries. Confirm that the process succeeds.

@AaronFeickert AaronFeickert marked this pull request as draft May 18, 2023 21:29
@AaronFeickert AaronFeickert marked this pull request as ready for review May 18, 2023 21:48
@ghpbot-tari-project ghpbot-tari-project added the P-acks_required Process - Requires more ACKs or utACKs label May 19, 2023
@github-actions
Copy link

Test Results (CI)

1 141 tests  ±0   1 141 ✔️ ±0   9m 14s ⏱️ +6s
     37 suites ±0          0 💤 ±0 
       1 files   ±0          0 ±0 

Results for commit 76ebf7f. ± Comparison against base commit 9ad6144.

@github-actions
Copy link

Test Results (Integration tests)

  2 files  +  2    1 errors  10 suites  +10   30m 11s ⏱️ + 30m 11s
22 tests +22  21 ✔️ +21  0 💤 ±0  1 +1 
24 runs  +24  22 ✔️ +22  0 💤 ±0  2 +2 

For more details on these parsing errors and failures, see this check.

Results for commit 76ebf7f. ± Comparison against base commit 9ad6144.

@SWvheerden SWvheerden merged commit ed120b2 into tari-project:development May 19, 2023
@AaronFeickert AaronFeickert deleted the password-matching branch May 19, 2023 13:45
SWvheerden added a commit that referenced this pull request May 29, 2023
##
[0.50.0-pre.2](v0.50.0-pre.1...v0.50.0-pre.2)
(2023-05-29)

### ⚠ BREAKING CHANGES

* add optional range proof types (5372)

### Features

* add metadata signature check
([5411](#5411))
([9c2bf41](9c2bf41))
* add optional range proof types
([5372](#5372))
([f24784f](f24784f))
* added burn feature to the console wallet
([5322](#5322))
([45685b9](45685b9))
* improved base node monitoring
([5390](#5390))
([c704890](c704890))


### Bug Fixes

* **comms:** only set final forward address if configured to port 0
([5406](#5406))
([ff7fb6d](ff7fb6d))
* deeplink to rfc spec
([5342](#5342))
([806d3b8](806d3b8))
* don't use in memory datastores for chat client dht in integration
tests ([#5399](#5399))
([cbdca6f](cbdca6f))
* fix panic when no public addresses
([5367](#5367))
([49be2a2](49be2a2))
* loop on mismatched passphrase entry
([5396](#5396))
([ed120b2](ed120b2))
* use domain separation for wallet message signing
([5400](#5400))
([7d71f8b](7d71f8b))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P-acks_required Process - Requires more ACKs or utACKs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

New wallet exists on password mismatch
3 participants