Обсуждение: Pasword expiration warning
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
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
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
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
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
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
Вложения
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
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
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/
Вложения
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.
Вложения
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
Вложения
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/
Вложения
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.
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
tosee "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
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.
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/
Вложения
发送时间: 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
> 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/
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.
Вложения
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/
Вложения
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.
Le 08/01/2026 à 10:27, Japin Li a écrit : > We should probably use guc-password-expire-warning as the ID, since the GUC is > named password_expire_warning (singular). Patch updated. -- Gilles Darold http://www.darold.net/
Вложения
On Thu, 08 Jan 2026 at 13:31, Gilles Darold <gilles@darold.net> wrote: > Le 08/01/2026 à 10:27, Japin Li a écrit : >> We should probably use guc-password-expire-warning as the ID, since the GUC is >> named password_expire_warning (singular). > > Patch updated. > Thanks for updating the patch. I noticed that src/backend/libpq/crypt.c no longer needs "postmaster/postmaster.h", so I've removed it in v8. I've also added a TAP test for the new GUC parameter. The updated patch is attached. -- Regards, Japin Li ChengDu WenWu Information Technology Co., Ltd.
Вложения
+ if (password_expire_warning > 0 && vuntil < PG_INT64_MAX)
+ {
+ TimestampTz result = (vuntil - now) / USECS_PER_SEC; /* in seconds */
+
+ if (result <= (TimestampTz) password_expire_warning)
+ MyClientConnectionInfo.warning_message =
+ psprintf("your password will expire in %d day(s)", (int) (result / 86400));
+ }Please consider localization of the warning message.
2. typo fix
a. `Controls how many time ...` should be `Controls how much time ...`.
b. `Sets how many time before password expire to emit ...` should be `Sets how much time before password expires to emit ...`
On Fri, 09 Jan 2026 at 14:10, Yuefei Shi <shiyuefei1004@gmail.com> wrote:
> A few review comments for V8.
>
> ===
> 1.
> + if (password_expire_warning > 0 && vuntil < PG_INT64_MAX)
> + {
> + TimestampTz result = (vuntil - now) / USECS_PER_SEC; /* in seconds */
> +
> + if (result <= (TimestampTz) password_expire_warning)
> + MyClientConnectionInfo.warning_message =
> + psprintf("your password will expire in %d day(s)", (int) (result / 86400));
> + }
> Please consider localization of the warning message.
>
> 2. typo fix
> a. `Controls how many time ...` should be `Controls how much time ...`.
> b. `Sets how many time before password expire to emit ...` should be `Sets how much time before password expires to
emit...`
Nice catch. Updated in v9. Please take to look.
I've also replaced the magic constant 86400 with the SECS_PER_DAY macro and
enclosed the statement in braces since it now spans multiple lines.
--
Regards,
Japin Li
ChengDu WenWu Information Technology Co., Ltd.
Вложения
From: Japin Li <japinli@hotmail.com>
Sent: Friday, January 09, 2026 15:12
To: Yuefei Shi <shiyuefei1004@gmail.com>
Cc: Gilles Darold <gilles@darold.net>; songjinzhou <tsinghualucky912@foxmail.com>; PostgreSQL Hackers
<pgsql-hackers@postgresql.org>;Andrew Dunstan <andrew@dunslane.net>; Bossart, Nathan <bossartn@amazon.com>; Tom Lane
<tgl@sss.pgh.pa.us>;liu xiaohui <liuxh.zj.cn@gmail.com>
Subject: Re: Pasword expiration warning
On Fri, 09 Jan 2026 at 14:10, Yuefei Shi <shiyuefei1004@gmail.com> wrote:
> A few review comments for V8.
>
> ===
> 1.
> + if (password_expire_warning > 0 && vuntil < PG_INT64_MAX)
> + {
> + TimestampTz result = (vuntil - now) / USECS_PER_SEC; /* in seconds */
> +
> + if (result <= (TimestampTz) password_expire_warning)
> + MyClientConnectionInfo.warning_message =
> + psprintf("your password will expire in %d day(s)", (int) (result /
86400));
> + }
> Please consider localization of the warning message.
>
> 2. typo fix
> a. `Controls how many time ...` should be `Controls how much time ...`.
> b. `Sets how many time before password expire to emit ...` should be `Sets how much time before password expires to
emit...`
Nice catch. Updated in v9. Please take to look.
I've also replaced the magic constant 86400 with the SECS_PER_DAY macro and
enclosed the statement in braces since it now spans multiple lines.
--
Regards,
Japin Li
ChengDu WenWu Information Technology Co., Ltd.
________________________________________
Hi, Jiapin,
I reviewed the v9-0002-Add-TAP-test-for-password_expire_warning.patch
and here are my comments:
1. I think we should add tow more cases. One case is for the feature is disbaled. And another is for no warning when
>1dremaining.
2. The modification to pg_hba.conf is unnecessary as the default pg_hba.conf generated by initdb already allows local
connectionswith appropriate methods.
unlink($node->data_dir . '/pg_hba.conf');
$node->append_conf('pg_hba.conf', "local all all scram-sha-256");
3. Make the expected string to be more exact.
qr/your password will expire in/);
-->
qr/your password will expire in 1d/);
Thanks,
Steven
Hi, Steven
Thanks for the review.
On Fri, 09 Jan 2026 at 07:36, Steven Niu <niushiji@gmail.com> wrote:
> Hi, Jiapin,
>
> I reviewed the v9-0002-Add-TAP-test-for-password_expire_warning.patch
> and here are my comments:
>
> 1. I think we should add tow more cases. One case is for the feature is disbaled. And another is for no warning when
>1dremaining.
Add in v10.
> 2. The modification to pg_hba.conf is unnecessary as the default pg_hba.conf generated by initdb already allows local
connectionswith appropriate methods.
> unlink($node->data_dir . '/pg_hba.conf');
> $node->append_conf('pg_hba.conf', "local all all scram-sha-256");
Yes, it allows local connections, but they are always in trust mode, so no
password is required (or used).
> 3. Make the expected string to be more exact.
> qr/your password will expire in/);
> -->
> qr/your password will expire in 1d/);
>
Fixed. PFA.
v10-0001 - No changes.
v10-0002 - Address review comments.
--
Regards,
Japin Li
ChengDu WenWu Information Technology Co., Ltd.
Вложения
Le 09/01/2026 à 10:04, Japin Li a écrit :
> Hi, Steven
>
> Thanks for the review.
>
> On Fri, 09 Jan 2026 at 07:36, Steven Niu <niushiji@gmail.com> wrote:
>> Hi, Jiapin,
>>
>> I reviewed the v9-0002-Add-TAP-test-for-password_expire_warning.patch
>> and here are my comments:
>>
>> 1. I think we should add tow more cases. One case is for the feature is disbaled. And another is for no warning when
>1dremaining.
> Add in v10.
>
>> 2. The modification to pg_hba.conf is unnecessary as the default pg_hba.conf generated by initdb already allows
localconnections with appropriate methods.
>> unlink($node->data_dir . '/pg_hba.conf');
>> $node->append_conf('pg_hba.conf', "local all all scram-sha-256");
> Yes, it allows local connections, but they are always in trust mode, so no
> password is required (or used).
>
>> 3. Make the expected string to be more exact.
>> qr/your password will expire in/);
>> -->
>> qr/your password will expire in 1d/);
>>
> Fixed. PFA.
>
> v10-0001 - No changes.
> v10-0002 - Address review comments.
>
Here is a v11 version of the patch.
v11-0001 - fix a miss on the typo fixes ( s/expire/expires/ in GUC
description ) and add your name in the authors list.
v11-0002 - Add a test with Infinity in VALID UNTIL value.
--
Gilles Darold
http://hexacluster.ai/
Вложения
> Here is a v11 version of the patch. > > v11-0001 - fix a miss on the typo fixes ( s/expire/expires/ in GUC > description ) and add your name in the authors list. > > v11-0002 - Add a test with Infinity in VALID UNTIL value. > > Thanks for updating the patches. LGTM. -- Regards, Japin Li ChengDu WenWu Information Technology Co., Ltd.