On 5/26/20 4:25 AM, Peter Eisentraut wrote:
> On 2020-05-25 17:57, Jonathan S. Katz wrote:
>> I took a look over, it looks good. One question on the initdb.c diff:
>>
>> - if (strcmp(authmethodlocal, "scram-sha-256") == 0 ||
>> - strcmp(authmethodhost, "scram-sha-256") == 0)
>> - {
>> - conflines = replace_token(conflines,
>> - "#password_encryption = md5",
>> - "password_encryption =
>> scram-sha-256");
>> - }
>> -
>>
>> Would we reverse this, i.e. if someone chooses authmethodlocal to be
>> "md5", we would then set "password_encryption = md5"?
>
> Yeah, I was too enthusiastic about removing that. Here is a better patch.
Did some testing. Overall it looks good. Here are my test cases and what
happened:
$ initdb -D data
Deferred password_encryption to the default, confirmed it was indeed scram
$ initdb -D data --auth-local=md5
Set password_encryption to md5
$ initdb -D data --auth-host=md5
Set password_encryption to md5
$ initdb -D data --auth-local=md5 --auth-host=scram-sha-256
Got an error message:
initdb: error: must specify a password for the superuser to enable
scram-sha-256 authentication
$ initdb -D data --auth-local=scram-sha-256 --auth-host=md5
Got an error message:
"initdb: error: must specify a password for the superuser to enable md5
authentication"
For the last two, that behavior is to be expected (after all, you've set
the two login vectors to require passwords), but the error message seems
odd now. Perhaps we tweak it to be:
"initdb: error: must specify a password for the superuser when requiring
passwords for both local and host authentication."
Another option could be to set the message based on whether both
local/host have the same setting, and then default to something like the
above when they differ.
Other than that, looks great!
Jonathan