On Thu, Dec 19, 2024 at 07:25:30AM +0000, Bertrand Drouvot wrote:
> + if (pwdlen < min_password_length)
> ereport(ERROR,
> (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
> errmsg("password is too short")));
>
> Now that the minimum password length is not "hardcoded" anymore, I wonder if it
> wouldn't be better to provide more details here (pwdlen and min_password_length).
>
> Suggestion in on_top_of_0001.txt attached.
Yeah, good point.
> + /* Define custom GUC variables. */
> + DefineCustomIntVariable("passwordcheck.min_password_length",
> + "Minimum allowed password length.",
> + NULL,
> + &min_password_length,
> + 8,
> + 0, INT_MAX,
>
> Since password must contain both letters and nonletters, 0 seems too low. I
> wonder if 2 is not a better value (done in on_top_of_0001.txt attached).
>
> Also, it seems to me that INT_MAX is too large (as mentioned in [1]), but that's
> probably a nit.
I don't think we need to restrict the allowed values here. It's true that
the lower bound will be effectively 2 due to the other checks, but that's
not the responsibility of this one to enforce. Those other checks will
likely become configurable at some point, too.
> - errmsg("password is too short")));
> + errmsg("password is too short: %d (< %d)", pwdlen, min_password_length)));
I think we should explicitly point to the parameter and note its current
value.
--
nathan
