Pasword expiration warning

Started by Gilles Daroldover 4 years ago65 messages
Jump to latest
#1Gilles Darold
gilles.darold@dalibo.com

Hi all,

Now that the security policy is getting stronger, it is not uncommon to
create users with a password expiration date (VALID UNTIL). The problem
is that the user is only aware that his password has expired when he can
no longer log in unless the application with which he is connecting
notifies him beforehand.

I'm wondering if we might be interested in having this feature in psql?
For example for a user whose password expires in 3 days:

gilles=# CREATE ROLE foo LOGIN PASSWORD 'foo' VALID UNTIL '2021-11-22';
CREATE ROLE
gilles=# \c - foo
Password for user foo:
psql (15devel, server 14.1 (Ubuntu 14.1-2.pgdg20.04+1))
** Warning: your password expires in 3 days **
You are now connected to database "gilles" as user "foo".

My idea is to add a psql variable that can be defined in psqlrc to
specify the number of days before the user password expires to start
printing a warning. The warning message is only diplayed in interactive
mode Example:

$ cat /etc/postgresql-common/psqlrc
\set PASSWORD_EXPIRE_WARNING 7

Default value is 0 like today no warning at all.

Of course any other client application have to write his own beforehand
expiration notice but with psql we don't have it for the moment. If
there is interest for this psql feature I can post the patch.

--
Gilles Darold

#2Dinesh Chemuduru
dinesh.kumar@migops.com
In reply to: Gilles Darold (#1)
Re: Pasword expiration warning

On Fri, 19 Nov 2021 at 20:19, Gilles Darold <gilles@migops.com> wrote:

Hi all,

Now that the security policy is getting stronger, it is not uncommon to
create users with a password expiration date (VALID UNTIL). The problem
is that the user is only aware that his password has expired when he can no
longer log in unless the application with which he is connecting notifies
him beforehand.

I'm wondering if we might be interested in having this feature in psql? For
example for a user whose password expires in 3 days:

gilles=# CREATE ROLE foo LOGIN PASSWORD 'foo' VALID UNTIL '2021-11-22';
CREATE ROLE
gilles=# \c - foo
Password for user foo:
psql (15devel, server 14.1 (Ubuntu 14.1-2.pgdg20.04+1))
** Warning: your password expires in 3 days **
You are now connected to database "gilles" as user "foo".

My idea is to add a psql variable that can be defined in psqlrc to specify
the number of days before the user password expires to start printing a
warning. The warning message is only diplayed in interactive mode Example:

$ cat /etc/postgresql-common/psqlrc
\set PASSWORD_EXPIRE_WARNING 7

+1

It is useful to notify the users about their near account expiration,
and we are doing that at client level.

Default value is 0 like today no warning at all.

Show quoted text

Of course any other client application have to write his own beforehand expiration
notice but with psql we don't have it for the moment. If there is interest
for this psql feature I can post the patch.

--
Gilles Darold

#3Tom Lane
tgl@sss.pgh.pa.us
In reply to: Gilles Darold (#1)
Re: Pasword expiration warning

Gilles Darold <gilles@migops.com> writes:

Now that the security policy is getting stronger, it is not uncommon to
create users with a password expiration date (VALID UNTIL).

TBH, I thought people were starting to realize that forced password
rotations are a net security negative. It's true that a lot of
places haven't gotten the word yet.

I'm wondering if we might be interested in having this feature in psql?

This proposal kind of seems like a hack, because
(1) not everybody uses psql
(2) psql can't really tell whether rolvaliduntil is relevant.
(It can see whether the server demanded a password, but
maybe that went to LDAP or some other auth method.)

That leads me to wonder about server-side solutions. It's easy
enough for the server to see that it's used a password with an
expiration N days away, but how could that be reported to the
client? The only idea that comes to mind that doesn't seem like
a protocol break is to issue a NOTICE message, which doesn't
seem like it squares with your desire to only do this interactively.
(Although I'm not sure I believe that's a great idea. If your
application breaks at 2AM because its password expired, you
won't be any happier than if your interactive sessions start to
fail. Maybe a message that would leave a trail in the server log
would be best after all.)

Default value is 0 like today no warning at all.

Off-by-default is pretty much guaranteed to not help most people.

regards, tom lane

#4Gilles Darold
gilles.darold@dalibo.com
In reply to: Tom Lane (#3)
Re: Pasword expiration warning

Le 19/11/2021 à 16:55, Tom Lane a écrit :

Gilles Darold <gilles@migops.com> writes:

Now that the security policy is getting stronger, it is not uncommon to
create users with a password expiration date (VALID UNTIL).

TBH, I thought people were starting to realize that forced password
rotations are a net security negative. It's true that a lot of
places haven't gotten the word yet.

I'm wondering if we might be interested in having this feature in psql?

This proposal kind of seems like a hack, because
(1) not everybody uses psql

Yes, for me it's a comfort feature. When a user connect to a PG backend
using an account that have expired you have no information that the
problem is a password expiration. The message returned to the user is
just: "FATAL: password authentication failed for user "foo".  We had to
verify in the log file that the problem is related to "DETAIL:  User
"foo" has an expired password.".  If the user was warned beforehand to
change the password it will probably saves me some time.

(2) psql can't really tell whether rolvaliduntil is relevant.
(It can see whether the server demanded a password, but
maybe that went to LDAP or some other auth method.)

I agree, I hope that in case of external authentication rolvaliduntil is
not set and in this case I guess that there is other notification
channels to inform the user that his password will expire. Otherwise yes
the warning message could be a false positive but the rolvaliduntil can
be changed to infinity to fix this case.

That leads me to wonder about server-side solutions. It's easy
enough for the server to see that it's used a password with an
expiration N days away, but how could that be reported to the
client? The only idea that comes to mind that doesn't seem like
a protocol break is to issue a NOTICE message, which doesn't
seem like it squares with your desire to only do this interactively.
(Although I'm not sure I believe that's a great idea. If your
application breaks at 2AM because its password expired, you
won't be any happier than if your interactive sessions start to
fail. Maybe a message that would leave a trail in the server log
would be best after all.)

I think that this is the responsibility of the client to display a
warning when the password is about to expire, the backend could help the
application by sending a NOTICE but the application will still have to
report the notice. I mean that it can continue to do all the work to
verify that the password is about to expire.

Default value is 0 like today no warning at all.

Off-by-default is pretty much guaranteed to not help most people.

Right, I was thinking of backward compatibility but this does not apply
here. So default to 7 days will be better.

To sum up as I said on top this is just a comfort notification dedicated
to psql and for local pg account to avoid looking at log file for
forgetting users.

--
Gilles Darold

#5Nathan Bossart
nathandbossart@gmail.com
In reply to: Gilles Darold (#4)
Re: Pasword expiration warning

On 11/19/21, 7:56 AM, "Tom Lane" <tgl@sss.pgh.pa.us> wrote:

That leads me to wonder about server-side solutions. It's easy
enough for the server to see that it's used a password with an
expiration N days away, but how could that be reported to the
client? The only idea that comes to mind that doesn't seem like
a protocol break is to issue a NOTICE message, which doesn't
seem like it squares with your desire to only do this interactively.
(Although I'm not sure I believe that's a great idea. If your
application breaks at 2AM because its password expired, you
won't be any happier than if your interactive sessions start to
fail. Maybe a message that would leave a trail in the server log
would be best after all.)

I bet it's possible to use the ClientAuthentication_hook for this. In
any case, I agree that it probably belongs server-side so that other
clients can benefit from this.

Nathan

#6Michael Paquier
michael@paquier.xyz
In reply to: Nathan Bossart (#5)
Re: Pasword expiration warning

On Sat, Nov 20, 2021 at 12:17:53AM +0000, Bossart, Nathan wrote:

I bet it's possible to use the ClientAuthentication_hook for this. In
any case, I agree that it probably belongs server-side so that other
clients can benefit from this.

ClientAuthentication_hook is called before the user is informed of the
authentication result, FWIW, so that does not seem wise.
--
Michael

#7Andrew Dunstan
andrew@dunslane.net
In reply to: Nathan Bossart (#5)
Re: Pasword expiration warning

On 11/19/21 19:17, Bossart, Nathan wrote:

On 11/19/21, 7:56 AM, "Tom Lane" <tgl@sss.pgh.pa.us> wrote:

That leads me to wonder about server-side solutions. It's easy
enough for the server to see that it's used a password with an
expiration N days away, but how could that be reported to the
client? The only idea that comes to mind that doesn't seem like
a protocol break is to issue a NOTICE message, which doesn't
seem like it squares with your desire to only do this interactively.
(Although I'm not sure I believe that's a great idea. If your
application breaks at 2AM because its password expired, you
won't be any happier than if your interactive sessions start to
fail. Maybe a message that would leave a trail in the server log
would be best after all.)

I bet it's possible to use the ClientAuthentication_hook for this. In
any case, I agree that it probably belongs server-side so that other
clients can benefit from this.

+1 for a server side solution. The people most likely to benefit from
this are the people least likely to be using psql IMNSHO.

cheers

andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com

#8Gilles Darold
gilles.darold@dalibo.com
In reply to: Andrew Dunstan (#7)
Re: Pasword expiration warning

Le 20/11/2021 à 14:48, Andrew Dunstan a écrit :

On 11/19/21 19:17, Bossart, Nathan wrote:

On 11/19/21, 7:56 AM, "Tom Lane" <tgl@sss.pgh.pa.us> wrote:

That leads me to wonder about server-side solutions. It's easy
enough for the server to see that it's used a password with an
expiration N days away, but how could that be reported to the
client? The only idea that comes to mind that doesn't seem like
a protocol break is to issue a NOTICE message, which doesn't
seem like it squares with your desire to only do this interactively.
(Although I'm not sure I believe that's a great idea. If your
application breaks at 2AM because its password expired, you
won't be any happier than if your interactive sessions start to
fail. Maybe a message that would leave a trail in the server log
would be best after all.)

I bet it's possible to use the ClientAuthentication_hook for this. In
any case, I agree that it probably belongs server-side so that other
clients can benefit from this.

+1 for a server side solution. The people most likely to benefit from
this are the people least likely to be using psql IMNSHO.

Ok, I can try to implement something at server side using a NOTICE message.

--
Gilles Darold

#9Gilles Darold
gilles.darold@dalibo.com
In reply to: Gilles Darold (#8)
Re: Pasword expiration warning

Le 21/11/2021 à 10:49, Gilles Darold a écrit :

Le 20/11/2021 à 14:48, Andrew Dunstan a écrit :

On 11/19/21 19:17, Bossart, Nathan wrote:

On 11/19/21, 7:56 AM, "Tom Lane" <tgl@sss.pgh.pa.us> wrote:

That leads me to wonder about server-side solutions. It's easy
enough for the server to see that it's used a password with an
expiration N days away, but how could that be reported to the
client? The only idea that comes to mind that doesn't seem like
a protocol break is to issue a NOTICE message, which doesn't
seem like it squares with your desire to only do this interactively.
(Although I'm not sure I believe that's a great idea. If your
application breaks at 2AM because its password expired, you
won't be any happier than if your interactive sessions start to
fail. Maybe a message that would leave a trail in the server log
would be best after all.)

I bet it's possible to use the ClientAuthentication_hook for this. In
any case, I agree that it probably belongs server-side so that other
clients can benefit from this.

+1 for a server side solution. The people most likely to benefit from
this are the people least likely to be using psql IMNSHO.

Ok, I can try to implement something at server side using a NOTICE message.

Hi,

Sorry to resurrect this old thread, but I had completely forgotten about
it. If there's still interest in this feature, then please find in
attachment a patch to emit a warning to the client and into the logs
when the password will expire within 7 days by default. A GUC,
password_expire_warning, allow to change the number of days before
sending the message or to disable this feature with setting value 0.

I have chosen to add a new field, const char *warning_message, to struct
ClientConnectionInfo so that it can be used to send other messages to
the client at end of connection ( src/backend/utils/init/postinit.c:
InitPostgres() ). Not sure sure that this is the best way to do that but
as it is a message dedicated to the connection I've though it could be
the right place. If we don't expect other warning message sent to the
client at connection time, just using an integer for the number of days
remaining will be enough. We could use notice but it is not logged by
default and also I think that warning is the good level for this message.

Output at psql connection:

        $ /usr/local/pgsql/bin/psql -h localhost -U test -d postgres
        Password for user test:
        WARNING:  your password will expire in 4 days
        psql (19devel)
        Type "help" for help.

        postgres=>

Output in the log:

        2026-01-05 23:23:13.763 CET [136001] WARNING:  your password
will expire in 4 days

Using a script:

        $ perl test_conn.pl
        WARNING:  your password will expire in 3 days

The message can be handled by any client application to warn the user if
required.

Thanks in advance for your feedback and suggestion for a better
implementation.

Best regards,

--
Gilles Darold
http://hexacluster.ai/

Attachments:

password_expire_warning-v1.patchtext/x-patch; charset=UTF-8; name=password_expire_warning-v1.patchDownload+49-0
#10Japin Li
japinli@hotmail.com
In reply to: Gilles Darold (#9)
Re: Pasword expiration warning

Hi, Gilles Darold

On Tue, 06 Jan 2026 at 15:43, Gilles Darold <gilles@darold.net> wrote:

Le 21/11/2021 à 10:49, Gilles Darold a écrit :

Le 20/11/2021 à 14:48, Andrew Dunstan a écrit :

On 11/19/21 19:17, Bossart, Nathan wrote:

On 11/19/21, 7:56 AM, "Tom Lane" <tgl@sss.pgh.pa.us> wrote:

That leads me to wonder about server-side solutions. It's easy
enough for the server to see that it's used a password with an
expiration N days away, but how could that be reported to the
client? The only idea that comes to mind that doesn't seem like
a protocol break is to issue a NOTICE message, which doesn't
seem like it squares with your desire to only do this interactively.
(Although I'm not sure I believe that's a great idea. If your
application breaks at 2AM because its password expired, you
won't be any happier than if your interactive sessions start to
fail. Maybe a message that would leave a trail in the server log
would be best after all.)

I bet it's possible to use the ClientAuthentication_hook for this. In
any case, I agree that it probably belongs server-side so that other
clients can benefit from this.

+1 for a server side solution. The people most likely to benefit from
this are the people least likely to be using psql IMNSHO.

Ok, I can try to implement something at server side using a NOTICE message.

Hi,

Sorry to resurrect this old thread, but I had completely forgotten
about it. If there's still interest in this feature, then please find
in attachment a patch to emit a warning to the client and into the
logs when the password will expire within 7 days by default. A GUC,
password_expire_warning, allow to change the number of days before
sending the message or to disable this feature with setting value 0.

I have chosen to add a new field, const char *warning_message, to
struct ClientConnectionInfo so that it can be used to send other
messages to the client at end of connection (
src/backend/utils/init/postinit.c: InitPostgres() ). Not sure sure
that this is the best way to do that but as it is a message dedicated
to the connection I've though it could be the right place. If we don't
expect other warning message sent to the client at connection time,
just using an integer for the number of days remaining will be
enough. We could use notice but it is not logged by default and also I
think that warning is the good level for this message.

Output at psql connection:

        $ /usr/local/pgsql/bin/psql -h localhost -U test -d postgres
        Password for user test:
        WARNING:  your password will expire in 4 days
        psql (19devel)
        Type "help" for help.

        postgres=>

Output in the log:

        2026-01-05 23:23:13.763 CET [136001] WARNING:  your password
will expire in 4 days

Using a script:

        $ perl test_conn.pl
        WARNING:  your password will expire in 3 days

The message can be handled by any client application to warn the user
if required.

Thanks in advance for your feedback and suggestion for a better
implementation.

Thanks for updating the patch.

1.
$ git apply ~/password_expire_warning-v1.patch
/home/japin/password_expire_warning-v1.patch:71: indent with spaces.
if (MyClientConnectionInfo.warning_message)
/home/japin/password_expire_warning-v1.patch:72: indent with spaces.
ereport(WARNING, (errmsg("%s", MyClientConnectionInfo.warning_message)));
warning: 2 lines add whitespace errors.

2.
+{ name => 'password_expire_warning', type => 'int', context => 'PGC_SIGHUP', group => 'CONN_AUTH_AUTH',
+  short_desc => 'Sets the number of days before password expire to emit a warning at client connection. Default is 7 days, 0 means no warning.',
+  flags => 'GUC_UNIT_S',
+  variable => 'password_expire_warning',
+  boot_val => '7',
+  min => '0',
+  max => '30',
+},
+

The GUC_UNIT_S flag specifies that the unit is seconds, meaning the default
value of 7 corresponds to 7 seconds rather than 7 days. For example:

# ALTER SYSTEM SET password_expire_warning TO 60;
ERROR: 60 s is outside the valid range for parameter "password_expire_warning" (0 s .. 30 s)

3.
I tested the patch and only received an empty WARNING message. After some
analysis, I found that the warning_message buffer is likely freed after
transaction commit.

Here's a new patch that resolves the issues mentioned earlier.

Furthermore, if the remaining time until expiration is less than one day,
should we display it in minutes (or hours) instead of 0 days?

--
Regards,
Japin Li
ChengDu WenWu Information Technology Co., Ltd.

Attachments:

v2-0001-Add-password_expire_warning-GUC-to-warn-clients.patchtext/x-diffDownload+49-1
#11songjinzhou
tsinghualucky912@foxmail.com
In reply to: Japin Li (#10)
Re: Pasword expiration warning

Hello fellow hackers, I've refined the print information based on the japin code above, but otherwise remained unchanged. The result is as follows:

[postgres@localhost:~/test/bin]$ ./psql -p 5432 -U test_user2 -d postgres
Password for user test_user2:
2026-01-07 00:28:25.999 PST [82198] WARNING:  your password will expire in 7 hours and 31 minutes
WARNING:  your password will expire in 7 hours and 31 minutes
psql (19devel)
Type "help" for help.

postgres=> \q
[postgres@localhost:~/test/bin]$ ./psql -p 5432 -U test_user3 -d postgres
Password for user test_user3:
2026-01-07 00:28:33.998 PST [82282] WARNING:  your password will expire in 2 days and 7 hours
WARNING:  your password will expire in 2 days and 7 hours
psql (19devel)
Type "help" for help.

postgres=> \q
[postgres@localhost:~/test/bin]$

Thanks

songjinzhou
tsinghualucky912@foxmail.com

Attachments:

v3-0001-Add-password_expire_warning-GUC-to-warn-clients.patchapplication/octet-stream; charset=utf-8; name=v3-0001-Add-password_expire_warning-GUC-to-warn-clients.patchDownload+89-1
#12Gilles Darold
gilles.darold@dalibo.com
In reply to: Japin Li (#10)
Re: Pasword expiration warning

Le 07/01/2026 à 06:12, Japin Li a écrit :

Thanks for updating the patch.

1.
$ git apply ~/password_expire_warning-v1.patch
/home/japin/password_expire_warning-v1.patch:71: indent with spaces.
if (MyClientConnectionInfo.warning_message)
/home/japin/password_expire_warning-v1.patch:72: indent with spaces.
ereport(WARNING, (errmsg("%s", MyClientConnectionInfo.warning_message)));
warning: 2 lines add whitespace errors.

2.
+{ name => 'password_expire_warning', type => 'int', context => 'PGC_SIGHUP', group => 'CONN_AUTH_AUTH',
+  short_desc => 'Sets the number of days before password expire to emit a warning at client connection. Default is 7 days, 0 means no warning.',
+  flags => 'GUC_UNIT_S',
+  variable => 'password_expire_warning',
+  boot_val => '7',
+  min => '0',
+  max => '30',
+},
+

The GUC_UNIT_S flag specifies that the unit is seconds, meaning the default
value of 7 corresponds to 7 seconds rather than 7 days. For example:

# ALTER SYSTEM SET password_expire_warning TO 60;
ERROR: 60 s is outside the valid range for parameter "password_expire_warning" (0 s .. 30 s)

3.
I tested the patch and only received an empty WARNING message. After some
analysis, I found that the warning_message buffer is likely freed after
transaction commit.

Here's a new patch that resolves the issues mentioned earlier.

Furthermore, if the remaining time until expiration is less than one day,
should we display it in minutes (or hours) instead of 0 days?

Thanks for the patch review and improvement.

I missed the GUC_UNIT_S c/p but we can use second as unit. I have fixed
your patch update because in this case the result variable must not be
turned into days but kept in seconds to be compared to the GUC value.

I have also added the missing GUC in the sample configuration file.

I'm not in favor of a high granularity in time display, number of days
for me is enough. I the user have the chance to see the 0 day remaining
he knows that he must fix that immediately. But why not, it depends of a
consensus.

Thanks.

--
Gilles Darold
http://hexaculter.ai/

Attachments:

v4-0001-Add-password_expire_warning-GUC-to-warn-clients.patchtext/x-patch; charset=UTF-8; name=v4-0001-Add-password_expire_warning-GUC-to-warn-clients.patchDownload+49-1
#13Gilles Darold
gilles.darold@dalibo.com
In reply to: songjinzhou (#11)
Re: Pasword expiration warning

Le 07/01/2026 à 09:37, songjinzhou a écrit :

Hello fellow hackers, I've refined the print information based on the japin code above, but otherwise remained unchanged. The result is as follows:

[postgres@localhost:~/test/bin]$ ./psql -p 5432 -U test_user2 -d postgres
Password for user test_user2:
2026-01-07 00:28:25.999 PST [82198] WARNING:  your password will expire in 7 hours and 31 minutes
WARNING:  your password will expire in 7 hours and 31 minutes
psql (19devel)
Type "help" for help.

postgres=> \q
[postgres@localhost:~/test/bin]$ ./psql -p 5432 -U test_user3 -d postgres
Password for user test_user3:
2026-01-07 00:28:33.998 PST [82282] WARNING:  your password will expire in 2 days and 7 hours
WARNING:  your password will expire in 2 days and 7 hours
psql (19devel)
Type "help" for help.

postgres=> \q
[postgres@localhost:~/test/bin]$

Thanks

songjinzhou
tsinghualucky912@foxmail.com

I'm not in favor of a higher granularity like explained in my previous
answer, I don't see it as a countdown to the second. But why not if
there's more people in favor of a more detailed remaining time, your
improvement will be welcome.

#14songjinzhou
tsinghualucky912@foxmail.com
In reply to: Gilles Darold (#13)
Re: Pasword expiration warning

Hi, Gilles Darold

First of all, thank you for your reply. This is indeed not a simple countdown. I did think it would be abrupt for users to see "0 days". I checked v4, and I think it's fine. (PS: Should we add relevant explanations to the SGML?) Thank you.

songjinzhou
tsinghualucky912@foxmail.com

#15Japin Li
japinli@hotmail.com
In reply to: songjinzhou (#14)
Re: Pasword expiration warning

On Thu, 08 Jan 2026 at 10:57, "songjinzhou" <tsinghualucky912@foxmail.com> wrote:

Hi, Gilles Darold

First of all, thank you for your reply. This is indeed not a simple
countdown. I did think it would be abrupt for users to see "0 days". I
checked v4, and I think it's fine. (PS: Should we add relevant
explanations to the SGML?) Thank you.

I'd like to hear more opinions on this.

--
Regards,
Japin Li
ChengDu WenWu Information Technology Co., Ltd.

#16Gilles Darold
gilles.darold@dalibo.com
In reply to: Japin Li (#15)
Re: Pasword expiration warning

Le 08/01/2026 à 04:37, Japin Li a écrit :

On Thu, 08 Jan 2026 at 10:57, "songjinzhou" <tsinghualucky912@foxmail.com> wrote:

Hi, Gilles Darold

First of all, thank you for your reply. This is indeed not a simple
countdown. I did think it would be abrupt for users to see "0 days". I
checked v4, and I think it's fine. (PS: Should we add relevant
explanations to the SGML?) Thank you.

I'd like to hear more opinions on this.

Here is a new version of the patch that adds the documentation for the
new GUC, fix the warning message (days(s) instead of days) and handle
the 'Infinity' value for the VALID UNTIL clause.

--
Gilles Darold
http://hexacluster.ai/

Attachments:

v5-0001-Add-password_expire_warning-GUC-to-warn-clients.patchtext/x-patch; charset=UTF-8; name=v5-0001-Add-password_expire_warning-GUC-to-warn-clients.patchDownload+67-2
#17liu xiaohui
liuxh.zj.cn@gmail.com
In reply to: Gilles Darold (#16)
回复: Pasword expiration warning

________________________________
发件人: Gilles Darold <gilles@darold.net>
发送时间: 2026年1月8日 14:04
收件人: Japin Li <japinli@hotmail.com>; songjinzhou <tsinghualucky912@foxmail.com>
抄送: Gilles Darold <gilles@darold.net>; PostgreSQL Hackers <pgsql-hackers@postgresql.org>; Andrew Dunstan <andrew@dunslane.net>; Bossart, Nathan <bossartn@amazon.com>; Tom Lane <tgl@sss.pgh.pa.us>
主题: Re: Pasword expiration warning

Le 08/01/2026 à 04:37, Japin Li a écrit :

On Thu, 08 Jan 2026 at 10:57, "songjinzhou" <tsinghualucky912@foxmail.com> wrote:

Hi, Gilles Darold

First of all, thank you for your reply. This is indeed not a simple
countdown. I did think it would be abrupt for users to see "0 days". I
checked v4, and I think it's fine. (PS: Should we add relevant
explanations to the SGML?) Thank you.

I'd like to hear more opinions on this.

Here is a new version of the patch that adds the documentation for the
new GUC, fix the warning message (days(s) instead of days) and handle
the 'Infinity' value for the VALID UNTIL clause.

--
Gilles Darold
http://hexacluster.ai/

Dear Gilles,

Thank you for submitting the v5 patch to add the password_expire_warning GUC.
The feature is useful and the implementation is mostly solid.
I reviewed the patch with a particular focus on the comments and documentation,
and I noticed several inconsistencies between the comments, the documentation,
and the actual code that could confuse future maintainers or users.

Here are the main issues I found:

1. /doc/src/sgml/config.sgml
There is an unrelated change in config.sgml around the password_encryption parameter:
the closing parenthesis of the <type> tag was split onto its own line, resulting in an isolated ")".

This appears to be an accidental editing artifact and is not required for the new feature.
It should be reverted to keep the documentation formatting consistent with the rest of the file.

2. Comments referring to "days" while the internal variable uses seconds:

src/backend/libpq/crypt.c:
/* Emit a warning 7 days before password expiration */

These hard-code "7 days" may be no longer accurate once the value becomes configurable.
Better comments would be:

/* Threshold (in seconds) before password expiration to emit a warning at login (0 = disabled; default 7 days) */

3. Minor typo/grammar in src/include/libpq/libpq-be.h
In the comment for warning_message:
"... at enf of InitPostgres(). ... it is use to show the password expiration warning."
Should be: "at the end of InitPostgres()" and "it is used to show".

Overall the implementation works correctly, but aligning all comments and documentation with the actual units (seconds internally, days for users) would greatly improve clarity.

Best regards,
Xiaohui Liu

#18Japin Li
japinli@hotmail.com
In reply to: Gilles Darold (#16)
Re: Pasword expiration warning

On Thu, 08 Jan 2026 at 07:04, Gilles Darold <gilles@darold.net> wrote:

Le 08/01/2026 à 04:37, Japin Li a écrit :

On Thu, 08 Jan 2026 at 10:57, "songjinzhou" <tsinghualucky912@foxmail.com> wrote:

Hi, Gilles Darold

First of all, thank you for your reply. This is indeed not a simple
countdown. I did think it would be abrupt for users to see "0 days". I
checked v4, and I think it's fine. (PS: Should we add relevant
explanations to the SGML?) Thank you.

I'd like to hear more opinions on this.

Here is a new version of the patch that adds the documentation for the
new GUC, fix the warning message (days(s) instead of days) and handle
the 'Infinity' value for the VALID UNTIL clause.

Thanks for updating the patch.

1.
I noticed a warning when applying the patch.

Applying: Add password_expire_warning GUC to warn clients
.git/rebase-apply/patch:31: trailing whitespace.
disable this behavior. The default value is <literal>7d</literal>.
warning: 1 line adds whitespace errors.

2.
      <varlistentry id="guc-password-encryption" xreflabel="password_encryption">
-      <term><varname>password_encryption</varname> (<type>enum</type>)
+      <term><varname>password_encryption</varname> (<type>enum</type>
+)

I think this modification isn't necessary.

3.
+		float8          result;
+
+		result = ((float8) (vuntil - GetCurrentTimestamp())) / 1000000.0; /* in seconds */
+

Perhaps we could use TimestampTz for the result variable and substitute
USECS_PER_SEC for 1000000.0—that would avoid the unnecessary type cast.

4.
+ if ((int) result <= password_expire_warning)

If the result exceeds INT_MAX, it triggers undefined behavior (IIRC).
Therefore, we should probably cast password_expire_warning itself.

5.
With this feature, GetCurrentTimestamp() might end up being called twice.
Perhaps we can avoid that duplication.

Attached is v6 of the patch addressing the issues above. Please take a look.

--
Regards,
Japin Li
ChengDu WenWu Information Technology Co., Ltd.

Attachments:

v6-0001-Add-password_expire_warning-GUC-to-warn-clients.patchtext/x-diffDownload+75-8
#19Gilles Darold
gilles.darold@dalibo.com
In reply to: Japin Li (#18)
Re: Pasword expiration warning

Le 08/01/2026 à 08:43, Japin Li a écrit :

On Thu, 08 Jan 2026 at 07:04, Gilles Darold <gilles@darold.net> wrote:

Le 08/01/2026 à 04:37, Japin Li a écrit :

On Thu, 08 Jan 2026 at 10:57, "songjinzhou" <tsinghualucky912@foxmail.com> wrote:

Hi, Gilles Darold

First of all, thank you for your reply. This is indeed not a simple
countdown. I did think it would be abrupt for users to see "0 days". I
checked v4, and I think it's fine. (PS: Should we add relevant
explanations to the SGML?) Thank you.

I'd like to hear more opinions on this.

Here is a new version of the patch that adds the documentation for the
new GUC, fix the warning message (days(s) instead of days) and handle
the 'Infinity' value for the VALID UNTIL clause.

Thanks for updating the patch.

1.
I noticed a warning when applying the patch.

Applying: Add password_expire_warning GUC to warn clients
.git/rebase-apply/patch:31: trailing whitespace.
disable this behavior. The default value is <literal>7d</literal>.
warning: 1 line adds whitespace errors.

2.
<varlistentry id="guc-password-encryption" xreflabel="password_encryption">
-      <term><varname>password_encryption</varname> (<type>enum</type>)
+      <term><varname>password_encryption</varname> (<type>enum</type>
+)

I think this modification isn't necessary.

3.
+		float8          result;
+
+		result = ((float8) (vuntil - GetCurrentTimestamp())) / 1000000.0; /* in seconds */
+

Perhaps we could use TimestampTz for the result variable and substitute
USECS_PER_SEC for 1000000.0—that would avoid the unnecessary type cast.

4.
+ if ((int) result <= password_expire_warning)

If the result exceeds INT_MAX, it triggers undefined behavior (IIRC).
Therefore, we should probably cast password_expire_warning itself.

5.
With this feature, GetCurrentTimestamp() might end up being called twice.
Perhaps we can avoid that duplication.

Attached is v6 of the patch addressing the issues above. Please take a look.

Thanks Japin, the implementation is fully working using the TimestampTz
cast.

I've attached a new patch to fix documentation and comments reported by
liu xiaohui and create a commitfest entry :
https://commitfest.postgresql.org/patch/6381/ Every one involved in the
review should edit the commitfest entry.

--
Gilles Darold
http://hexacluster.ai/

Attachments:

v7-0001-Add-password_expire_warning-GUC-to-warn-clients.patchtext/x-patch; charset=UTF-8; name=v7-0001-Add-password_expire_warning-GUC-to-warn-clients.patchDownload+78-8
#20Japin Li
japinli@hotmail.com
In reply to: Gilles Darold (#19)
Re: Pasword expiration warning

On Thu, 08 Jan 2026 at 09:41, Gilles Darold <gilles@darold.net> wrote:

Le 08/01/2026 à 08:43, Japin Li a écrit :

On Thu, 08 Jan 2026 at 07:04, Gilles Darold <gilles@darold.net> wrote:

Le 08/01/2026 à 04:37, Japin Li a écrit :

On Thu, 08 Jan 2026 at 10:57, "songjinzhou" <tsinghualucky912@foxmail.com> wrote:

Hi, Gilles Darold

First of all, thank you for your reply. This is indeed not a simple
countdown. I did think it would be abrupt for users to see "0 days". I
checked v4, and I think it's fine. (PS: Should we add relevant
explanations to the SGML?) Thank you.

I'd like to hear more opinions on this.

Here is a new version of the patch that adds the documentation for the
new GUC, fix the warning message (days(s) instead of days) and handle
the 'Infinity' value for the VALID UNTIL clause.

Thanks for updating the patch.

1.
I noticed a warning when applying the patch.

Applying: Add password_expire_warning GUC to warn clients
.git/rebase-apply/patch:31: trailing whitespace.
disable this behavior. The default value is <literal>7d</literal>.
warning: 1 line adds whitespace errors.

2.
<varlistentry id="guc-password-encryption" xreflabel="password_encryption">
-      <term><varname>password_encryption</varname> (<type>enum</type>)
+      <term><varname>password_encryption</varname> (<type>enum</type>
+)

I think this modification isn't necessary.

3.
+		float8          result;
+
+		result = ((float8) (vuntil - GetCurrentTimestamp())) / 1000000.0; /* in seconds */
+

Perhaps we could use TimestampTz for the result variable and substitute
USECS_PER_SEC for 1000000.0—that would avoid the unnecessary type cast.

4.
+ if ((int) result <= password_expire_warning)

If the result exceeds INT_MAX, it triggers undefined behavior (IIRC).
Therefore, we should probably cast password_expire_warning itself.

5.
With this feature, GetCurrentTimestamp() might end up being called twice.
Perhaps we can avoid that duplication.

Attached is v6 of the patch addressing the issues above. Please take a look.

Thanks Japin, the implementation is fully working using the
TimestampTz cast.

I've attached a new patch to fix documentation and comments reported
by liu xiaohui and create a commitfest entry :
https://commitfest.postgresql.org/patch/6381/ Every one involved in
the review should edit the commitfest entry.

A minor nitpick:

1.
+     <varlistentry id="guc-password-expire-warnings" xreflabel="password_expire_warning">
+      <term><varname>password_expire_warning</varname> (<type>integer</type>)

We should probably use guc-password-expire-warning as the ID, since the GUC is
named password_expire_warning (singular).

--
Regards,
Japin Li
ChengDu WenWu Information Technology Co., Ltd.

#21Gilles Darold
gilles.darold@dalibo.com
In reply to: Japin Li (#20)
#22Japin Li
japinli@hotmail.com
In reply to: Gilles Darold (#21)
#23Yuefei Shi
shiyuefei1004@gmail.com
In reply to: Japin Li (#22)
#24Japin Li
japinli@hotmail.com
In reply to: Yuefei Shi (#23)
#25Steven Niu
niushiji@gmail.com
In reply to: Japin Li (#24)
#26Japin Li
japinli@hotmail.com
In reply to: Steven Niu (#25)
#27Gilles Darold
gilles.darold@dalibo.com
In reply to: Japin Li (#26)
#28Japin Li
japinli@hotmail.com
In reply to: Gilles Darold (#27)
#29Soumya S Murali
soumyamurali.work@gmail.com
In reply to: Gilles Darold (#27)
#30Euler Taveira
euler@eulerto.com
In reply to: Gilles Darold (#27)
#31Zsolt Parragi
zsolt.parragi@percona.com
In reply to: Gilles Darold (#27)
#32Japin Li
japinli@hotmail.com
In reply to: Zsolt Parragi (#31)
#33Zsolt Parragi
zsolt.parragi@percona.com
In reply to: Japin Li (#32)
#34Gilles Darold
gilles.darold@dalibo.com
In reply to: Euler Taveira (#30)
#35Gilles Darold
gilles.darold@dalibo.com
In reply to: Zsolt Parragi (#31)
#36Nathan Bossart
nathandbossart@gmail.com
In reply to: Zsolt Parragi (#33)
#37Zsolt Parragi
zsolt.parragi@percona.com
In reply to: Gilles Darold (#35)
#38Gilles Darold
gilles.darold@dalibo.com
In reply to: Nathan Bossart (#36)
#39Nathan Bossart
nathandbossart@gmail.com
In reply to: Gilles Darold (#38)
#40Gilles Darold
gilles.darold@dalibo.com
In reply to: Nathan Bossart (#36)
#41Nathan Bossart
nathandbossart@gmail.com
In reply to: Gilles Darold (#40)
#42Gilles Darold
gilles.darold@dalibo.com
In reply to: Nathan Bossart (#41)
#43Nathan Bossart
nathandbossart@gmail.com
In reply to: Gilles Darold (#42)
#44Zsolt Parragi
zsolt.parragi@percona.com
In reply to: Nathan Bossart (#43)
#45Nathan Bossart
nathandbossart@gmail.com
In reply to: Nathan Bossart (#43)
#46Nathan Bossart
nathandbossart@gmail.com
In reply to: Nathan Bossart (#45)
#47Greg Sabino Mullane
greg@turnstep.com
In reply to: Nathan Bossart (#46)
#48Peter Eisentraut
peter_e@gmx.net
In reply to: Nathan Bossart (#46)
#49Nathan Bossart
nathandbossart@gmail.com
In reply to: Peter Eisentraut (#48)
#50Zsolt Parragi
zsolt.parragi@percona.com
In reply to: Nathan Bossart (#49)
#51Nathan Bossart
nathandbossart@gmail.com
In reply to: Zsolt Parragi (#50)
#52Jacob Champion
jacob.champion@enterprisedb.com
In reply to: Nathan Bossart (#51)
#53Nathan Bossart
nathandbossart@gmail.com
In reply to: Jacob Champion (#52)
#54Euler Taveira
euler@eulerto.com
In reply to: Nathan Bossart (#49)
#55Nathan Bossart
nathandbossart@gmail.com
In reply to: Euler Taveira (#54)
#56Euler Taveira
euler@eulerto.com
In reply to: Nathan Bossart (#55)
#57Chao Li
li.evan.chao@gmail.com
In reply to: Nathan Bossart (#55)
#58Greg Sabino Mullane
greg@turnstep.com
In reply to: Chao Li (#57)
#59Nathan Bossart
nathandbossart@gmail.com
In reply to: Chao Li (#57)
#60Peter Eisentraut
peter_e@gmx.net
In reply to: Zsolt Parragi (#50)
#61Zsolt Parragi
zsolt.parragi@percona.com
In reply to: Nathan Bossart (#59)
#62Nathan Bossart
nathandbossart@gmail.com
In reply to: Zsolt Parragi (#61)
#63Nathan Bossart
nathandbossart@gmail.com
In reply to: Nathan Bossart (#62)
#64Gilles Darold
gilles.darold@dalibo.com
In reply to: Nathan Bossart (#63)
#65Nathan Bossart
nathandbossart@gmail.com
In reply to: Gilles Darold (#64)