Add a common method for asking for a new password #251
Conversation
agrare
force-pushed
the
add_common_method_for_prompting_new_password
branch
from
299176d
to
18ae029
Compare
| end | ||
| self.database_name = just_ask("cluster database name", database_name) | ||
| self.database_user = just_ask("cluster database username", database_user) | ||
| self.database_password = ask_for_new_password("cluster database password", :default => database_password, :allow_empty => true) |
There was a problem hiding this comment.
NOTE I'm not sure if this was intentional but the old method did allow for an empty password
There was a problem hiding this comment.
Or maybe because it has a default, allow empty allows it to use the default?
There was a problem hiding this comment.
https://github.com/ManageIQ/manageiq-appliance_console/pull/251/files#diff-58f7d485041385eb768bd8d25d7e83a11ccf1a1bbf2bc4fb136fcdcf9cd6b590L126 had a default as well but that checks for a 0 length password. I think the default is only present if you've already configured a password but if this is the first time through there would be no default.
There was a problem hiding this comment.
Looks like it was like that back to the initial open source commit.
It had a concept of no password, which I assume means the default, likely already configured with a password. I'm not sure though.
+ def ask_for_database_credentials
+ self.host = ask_for_ip_or_hostname("database hostname or IP address", host) if host.blank? || !local?
+ self.database = just_ask("name of the database on #{host}", database) unless local?
+ self.username = just_ask("username", username) unless local?
+ count = 0
+ loop do
+ count += 1
+ password1 = ask_for_password_or_none("database password on #{host}", password)
+ # if they took the default, just bail
+ break if (password1 == password)
+ password2 = ask_for_password("database password again")
+ if password1 == password2
+ self.password = password1
+ break
+ elsif count > 1 # only reprompt password once
+ raise ArgumentError, "passwords did not match"
+ else
+ say("\nThe passwords did not match, please try again")
+ end
+ end
+ endThere was a problem hiding this comment.
Okay I think I'm going to keep the behavior the same as the internal database configuration and not allow a non-default empty password.
agrare
force-pushed
the
add_common_method_for_prompting_new_password
branch
from
18ae029
to
b0708b2
Compare
agrare
force-pushed
the
add_common_method_for_prompting_new_password
branch
2 times, most recently
from
4544ca6
to
de20f41
Compare
When entering an existing password it is sufficient to prompt once since if the user mistyped the password it will fail later on, but when creating a new password it is common to re-prompt them to confirm that they didn't typo the new password.
agrare
force-pushed
the
add_common_method_for_prompting_new_password
branch
from
de20f41
to
7892d83
Compare
|
Checked commit agrare@7892d83 with ruby 2.7.8, rubocop 1.56.3, haml-lint 0.51.0, and yamllint |
When entering an existing password it is sufficient to prompt once since if the user mistyped the password it will fail later on, but when creating a new password it is common to re-prompt them to confirm that they didn't typo the new password.
We had this "prompt twice, check non-empty" loop in two places and were going to need it in a third so it seemed better to move this to a common method.
TODO
Database configuration testing:
Message Server Configuration testing
Fixes ManageIQ/manageiq#23031