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

Add password length validation - take 2 #138

Merged
merged 4 commits into from
Oct 27, 2020
Merged

Add password length validation - take 2 #138

merged 4 commits into from
Oct 27, 2020

Conversation

hwwhww
Copy link
Contributor

@hwwhww hwwhww commented Oct 21, 2020

Address #99
Replace #118

I haven't figured out how to test hidden input (password) with the test script, but #123 gave me a hint of utilizing click callback function. This is a workaround of #118.

Example output

  1. enter the length=3 password and confirmation -> got rejected
  2. enter the length=5 password -> got rejected
  3. enter the length=8 password and confirmation -> got accepted
. venv/bin/activate && python ./eth2deposit/deposit.py
Please choose how many validators you wish to run: 1
Please choose your mnemonic language (czech, chinese_traditional, chinese_simplified, english, spanish, italian, korean) [english]:
Please choose the (mainnet or testnet) network/chain name (mainnet, witti, altona, medalla, spadina, zinken) [mainnet]:
Type the password that secures your validator keystore(s):
Repeat for confirmation:
Error: The password length should be at least 8. Got 3. Please retype again.
Type the password that secures your validator keystore(s):
Error: The password length should be at least 8. Got 5. Please retype again.
Type the password that secures your validator keystore(s):
Repeat for confirmation:

Compare with #118

#118 This PR
1. --password flag is disallowed --password flag is allowed
2. Validate the password strength once the user enters the password Validate the password strength after it passing the confirmation. After that, it validates the strength once the user enters the password. (another while loop)

I do want to avoid (1), but (1) helps us writing the test script. 😓

@hwwhww hwwhww requested a review from CarlBeek October 21, 2020 06:22
Copy link
Collaborator

@CarlBeek CarlBeek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is a better solution overall than the one presented in #118. 👍

I do with the order was verify length then check for matching passwords, but this may be an example of perfect being the enemy of the good.

@hwwhww hwwhww mentioned this pull request Oct 21, 2020
@hwwhww
Copy link
Contributor Author

hwwhww commented Oct 21, 2020

edited PR description:

#118: --password flag is disallowed
This PR: --password flag is allowed

eth2deposit/deposit.py Outdated Show resolved Hide resolved
eth2deposit/deposit.py Outdated Show resolved Hide resolved
@hwwhww hwwhww merged commit 076469a into dev Oct 27, 2020
@hwwhww hwwhww deleted the tob_audit_03_2 branch October 27, 2020 06:05
@hwwhww hwwhww mentioned this pull request Oct 27, 2020
2 tasks
CarlBeek added a commit that referenced this pull request Nov 3, 2020
@hwwhww hwwhww mentioned this pull request Nov 3, 2020
everhusk pushed a commit to earthbitcoin/earth-wallet-cli that referenced this pull request Aug 3, 2023
* [WIP] Add password length validation

* Workaround: use click callback function

* Minor rewording
everhusk pushed a commit to earthbitcoin/earth-wallet-cli that referenced this pull request Aug 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants