Closed steveatinfincia closed 7 years ago
@steveatinfincia Hey ! Thanks for your contribution ! I am happy to review this PR :smiley:
Backspace works for me in the current implementation. Not for you ? If not, that's a bug.
As for the additions that show the password, they make sense. Just 2 things that need fixing before we can merge this:
Windows build
We need to fix what's breaking the build on Windows — @equalsraf are you available to check it out please ?
Function names
I'd like the library to have the option of echoing the password on prompt_password_*
functions also. To keep things consistent, we'd need to rename some functions.
Could you please rename the function read_response
to read_password_with_output
and add 2 functions named prompt_password_stderr_with_output
and prompt_password_stdout_with_output
, that are just like prompt_password_stderr
/prompt_password_stdout
but that echo the password ?
If this is unclear, please let me know, and I'll try to rephrase.
Best regards, Conrad
I just realised you named the PR add-non-password-responses
. I suppose that means you just want to read text, but without the newline at the end. Is that right ? If that's the case, then I don't think rpassword
is meant to help you. rpassword
does one thing: read a password (without the newline) from the command line. That's it. For other things, rpassword
is not it.
We need to fix what's breaking the build on Windows — @equalsraf are you available to check it out please ?
It will be a while before I get home, quick glimpse shows
src\lib.rs:178:82: 178:99 error: unresolved name
ENABLE_ECHO_INPUT
src\lib.rs:178 false => winapi::ENABLE_LINE_INPUT | winapi::ENABLE_PROCESSED_INPUT| ENABLE_ECHO_INPUT,
Quick browse in the winapi docs shows a winapi::wincon::ENABLE_ECHO_INPUT
, so maybe its just a reference that needs to be fixed (the same applies to the other ones).
@conradkleinespel I did make the implementation a little more general, as it may have been useful for other things, however the primary use case I was thinking of was the following:
1) Entering passwords that are simply too hard to enter correctly without seeing what you're typing
For example, long pass phrases often look like this:
quality useless orient offer pole host amazing title only clog sight wild anxiety gloom market rescue fan language entry fan oyster
Addition of special characters or even just capitalization can mean the user has to try multiple times with no feedback on what part they entered incorrectly.
2) Entering a new password
Includes cases where typing the intended password incorrectly (even twice) could be very hard to fix or undo, like deriving encryption keys
In other words, a way to allow applications to do in the console what this checkbox does in a GUI:
We need to fix what's breaking the build on Windows — @equalsraf are you available to check it out please ?
Confirmed here, a minor fix to add the missing part of the path builds successfuly in Windows
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -175,7 +175,7 @@ mod windows {
}
let new_mode_flags = match hide {
true => winapi::ENABLE_LINE_INPUT | winapi::ENABLE_PROCESSED_INPUT,
- false => winapi::ENABLE_LINE_INPUT | winapi::ENABLE_PROCESSED_INPUT| ENABLE_ECHO_INPUT,
+ false => winapi::ENABLE_LINE_INPUT | winapi::ENABLE_PROCESSED_INPUT| winapi::ENABLE_ECHO_INPUT,
};
// We want to be able to read line by line, and we still want backspace to work
@steveatinfincia Makes sense, thanks for explaining that !
@equalsraf Thanks for checking it out !
I will merge, apply the Windows fix and then publish to crates.io.
@steveatinfincia Just published to crates.io under version 0.4.0
. Thanks again !
@conradkleinespel awesome, thank you :)
This adds support for optionally echoing what users type, which is useful for various tasks including some that are necessary right before the user enters a password, like entering an account name. It can also be useful to permit users to correct their own mistakes when entering very long pass phrases, or secrets that are difficult to type, rather than waiting to see if they got it right later on.
Changes
I have moved the bulk of the
read_password()
functions for each platform to genericread_input()
functions that take a parameter to optionally hide what the user types.The existing
read_password()
andprompt_password_*()
functions should behave the same as before, but there are nowread_response()
andprompt_response_*()
variants as well.The current public API should continue working for existing users of the library, and the existing test still passes both on Mac and Linux.