Re: Parametrization minimum password lenght

From: Bertrand Drouvot <bertranddrouvot(dot)pg(at)gmail(dot)com>
To: Emanuele Musella <emamuse86(at)gmail(dot)com>
Cc: pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: Parametrization minimum password lenght
Date: 2024-11-19 19:28:03
Message-ID: Zzzmw4IAvrypmFO4@ip-10-97-1-34.eu-west-3.compute.internal
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

On Mon, Nov 18, 2024 at 05:21:18PM +0100, Emanuele Musella wrote:
> We notice some errors on CFBot results.

FWIW, you can run "cfbot like" tests on your own repo (see [1]).

> In attached the errors fixed

Thanks for the updated version!

A few random comments:

=== 1

trailing whitespace:

$ git apply min_password_length_v7.patch
min_password_length_v7.patch:130: trailing whitespace.
There is a configuration parameter that control the behavior
warning: 1 line adds whitespace errors.

=== 2

+ * Author: Maurizio Boriani <maurizio(at)boriani(dot)cloud>
+ * Author: Emanuele Musella <emamuse86(at)gmail(dot)com>

Same comment as in [2].

=== 3

- int pwdlen = strlen(password);
+ int pwdlen = pg_mbstrlen(password);

Sorry if I was not clear in [2], but I meant to say to keep using strlen() to be
consistent with the current behavior.

=== 4

+ GUC_UNIT_BYTE,

this is correct if strlen() is used (see above comment).

=== 5

+ 0, INT_MAX,

INT_MAX seems too large and 0 too low. Maybe we should not allow less than it
was before the patch (8). For the max, maybe something like PG_MAX_AUTH_TOKEN_LENGTH?
(see the comment in src/backend/libpq/auth.c)

=== 6

+ There is a configuration parameter that control the behavior
+ <filename>passwordcheck</filename>

s/behavior/behavior of/?

=== 7

+ <varname>passwordcheck.min_password_length</varname> is the minimum length
+ of accepted password on database users.
+ If not setted the default is 8 bytes.

What about? "is the minimum password length in bytes. The default is 8."

=== 7

+
+<programlisting>
+# postgresql.conf
+session_preload_libraries = 'passwordcheck'
+passwordcheck.min_password_length = 12
+
+</programlisting>

What about a sentence before? Something like for auto_explain means "In ordinary
usage, these parameters are set in postgresql.conf,............"

[1]: https://github.com/postgres/postgres/blob/master/src/tools/ci/README
[2]: https://www.postgresql.org/message-id/ZzsZZY3YrO6hinnT%40ip-10-97-1-34.eu-west-3.compute.internal

Regards,

--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2024-11-19 19:45:16 Re: Using Expanded Objects other than Arrays from plpgsql
Previous Message James Coleman 2024-11-19 19:16:59 Parallel safety docs for CTEs