feat: ask for password if -p is not provided #4153
Conversation
| @@ -67,6 +69,11 @@ public static void main(final String[] args) throws IOException { | |||
| System.exit(-1); | |||
| } | |||
|
|
|||
| // ask for password if not set through command parameters | |||
| if (!options.getUserName().isEmpty() && !options.isPasswordSet()) { | |||
| options.setPassword(readPassword()); | |||
There was a problem hiding this comment.
Should we add a unit test for this functionality?
There was a problem hiding this comment.
I don't see any tests cases that verify the ksql command using parameters. The KsqlTest uses mocks and executes ksql.run() directly.
I tried using unit tests, but there are a few static classes used in the main class. I could reorganize the code to set stubs, but how do I set stubs or mocks on static classes?
Do you know if there are tests that verify ksql commands are executed using the cli?
There was a problem hiding this comment.
Do you know if there are tests that verify ksql commands are executed using the cli?
Have you looked at the ones in https://github.com/confluentinc/ksql/blob/master/ksql-cli/src/test/java/io/confluent/ksql/cli/CliTest.java? Not exactly what you're looking for but many come close.
| @@ -49,4 +49,21 @@ public void shouldReturnEmptyOptionWhenUserAndPassNotPresent() throws Exception | |||
| assertFalse(options.getUserNameAndPassword().isPresent()); | |||
| } | |||
|
|
|||
| @Test | |||
| public void shouldReturnFalseIfPasswordIsNull() throws Exception { | |||
There was a problem hiding this comment.
nit: rename to shouldReturnPasswordNotSetIfPasswordIsNull() or something similar to clarify what's returning false, and similarly for the other new tests.
spena
force-pushed
the
ask_password_console
branch
from
fe396cb to
62595b3
Compare
spena
force-pushed
the
ask_password_console
branch
from
62595b3 to
10854c0
Compare
|
Jenkins is failing with unrelated changes to this code. The build and tests passed, but some packaging errors, like I will merge this PR for now. |
Description
What behavior do you want to change, why, how does your patch achieve the changes?
If the
-por--passwordparameters are not provided when using authentication, then the CLI will ask the user to enter the password on the prompt:BREAKING CHANGE
The old behavior requires a user to type
--userand--passwordparameters for authentication. This change makes--passwordoptional.This behavior provides an extra security to users who do not want to type the password in plaintext in the console. The
Enter password:prompt will not echo the password.Testing done
Describe the testing strategy. Unit and integration tests are expected for any behavior changes.
Reviewer checklist