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

#123 & #124 Feature/askpassphrase #127

Merged
merged 2 commits into from
Oct 8, 2014

Conversation

jd1123
Copy link
Contributor

@jd1123 jd1123 commented Oct 8, 2014

This fixes Issue #123 and closes #124 (request to merge into wrong branch)

jd1123 added 2 commits October 4, 2014 16:55
* context.go - added global variable gAskPassphrase and added
  PromptForPassword() to get passphrase
* cmd/trousseau/before.go - updateCheckPassphrase() to update global
  variable
* cmd/trousseau/main.go - added Blooflag struct to give comments in help
* gPassphrase in context.go
* helpers in context.go
* calling AskPassphrase from before.go sets gPassphrase
@jd1123
Copy link
Contributor Author

jd1123 commented Oct 8, 2014

@oleiade - did as you suggested in #124. Let me know if this is what your were thinking...

@oleiade
Copy link
Owner

oleiade commented Oct 8, 2014

Super good!

Matches exactly what I was thinking (ref #123) ! Have tested it locally and it works as expected. I'm gonna merge it now.

I've noticed two things though:

  • The stdout prints Password:. It should print Passphrase: in order to be coherent and not to lost the user. I think the helper function could be renamed to something more explicit/generic too like PromptForHiddenInput. I've created a ticket for this: Refactor the ask-passphrase option behavior to be more explicit #129
  • When an invalid passphrase is provided, trousseau outputs on stderr the following message:
Error: unable to decrypt trousseau data store. No private key able to decrypt it found in your keyring.

which in this case is not precise enough, and might be confusing for the user trying to understand why it's not working and actually being pointed to a wrong problem description. I've created another ticket for this: #130

Thank you very much for this addition and the work done!
I'll add you to the AUTHORS file :-)

oleiade pushed a commit that referenced this pull request Oct 8, 2014
@oleiade oleiade merged commit 2206a2e into oleiade:develop Oct 8, 2014
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