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

cmd/clef: replace password arg with prompt #17856

Closed

Conversation

johnsBeharry
Copy link
Contributor

Feature: cmd/clef

Scenario: addpw prompts user to type password
  When a user runs "clef addpw 0x0000000000000000000000000000000000000000"
  Then the user is prompted to enter a password
  And then the user is prompted to enter the same password again to confirm

NOTES

  • As specified in Clef addpw should not take password via command line  #17829 the problem is passing a password to clef addpw <account> <password> exposes the password to bash_history. The solution is to prompt the user to enter the password instead.
  • This PR borrows the getPassword function from cmd/geth/accountcmd.go with some simplification for clef.

@johnsBeharry johnsBeharry changed the title [WIP] cmd/clef: replace password arg with prompt (#17829) cmd/clef: replace password arg with prompt (#17829) Oct 7, 2018
@holiman
Copy link
Contributor

holiman commented Oct 8, 2018

Oh, my bad. I forgot that the PR to have a master password is not in master yet, the idea was to reuse the same password dialog that's already in that PR (#17704) . If made on top of that one, it would be pretty simple to make it work also in non-CLI mode, with an external UI.

@johnsBeharry johnsBeharry changed the title cmd/clef: replace password arg with prompt (#17829) cmd/clef: replace password arg with prompt Oct 12, 2018
@johnsBeharry
Copy link
Contributor Author

@holiman I'm closing this in favor of PR #17897 which was branched off of master containing #17704

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