Re: Parametrization minimum password lenght

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