| From: | Zsolt Parragi <zsolt(dot)parragi(at)percona(dot)com> |
|---|---|
| To: | Gilles Darold <gilles(at)darold(dot)net> |
| Cc: | Japin Li <japinli(at)hotmail(dot)com>, Yuefei Shi <shiyuefei1004(at)gmail(dot)com>, songjinzhou <tsinghualucky912(at)foxmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>, Andrew Dunstan <andrew(at)dunslane(dot)net>, "Bossart, Nathan" <bossartn(at)amazon(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, liu xiaohui <liuxh(dot)zj(dot)cn(at)gmail(dot)com>, Steven Niu <niushiji(at)gmail(dot)com> |
| Subject: | Re: Pasword expiration warning |
| Date: | 2026-01-28 19:25:34 |
| Message-ID: | CAN4CZFMBj-mZq_o8BtQz_TgqDTALbn31kc-SFtWE4FR1N-4RBw@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hello!
A first question: have you looked at the GoAway patch[1]? While that
isn't exactly about the same situation, it was already considered for
password expiration checks in[2], and the same idea could be useful
for this situation too, especially in the context of my last question
in this email.
+ MyClientConnectionInfo.warning_message =
+ psprintf(_("your password will expire in %d day(s)"),
+ (int) (result / SECS_PER_DAY));
Shouldn't that use MemoryContextStrtup(TopMemoryContext, ...)?
+ /*
+ * Message to send to the client in case of connection success.
+ * When not NULL a WARNING message is sent to the client at end
+ * of the connection in src/backend/utils/init/postinit.c at
+ * enf of InitPostgres(). For example, it is use to show the
+ * password expiration warning.
+ */
+ const char *warning_message;
Handling of this new variable is missing from
EstimateClientConnectionInfoSpace and SerializeClientConnectionInfo,
which the struct explicitly asks for a few lines above this change.
Even if you think that's not necessary for some reason, it should be
explained to avoid confusing readers.
+ * Password OK, but check if rolvaliduntil is less than GUC
+ * password_expire_warning days to send a warning to the client
+ */
+ if (!isnull && password_expire_warning > 0 && vuntil < PG_INT64_MAX)
Could this use TIMESTAMP_NOT_FINITE?
And I think that "days" should be "seconds".
+ TimestampTz result = (vuntil - now) / USECS_PER_SEC; /* in seconds */
Maybe call this variable something more descriptive, like
seconds_until_expiration?
+
+ if (result <= (TimestampTz) password_expire_warning)
+ {
+ MyClientConnectionInfo.warning_message =
+ psprintf(_("your password will expire in %d day(s)"),
+ (int) (result / SECS_PER_DAY));
+ }
This is not that useful on the last day - have you considered
displaying hours if the expiration date is within a day, or maybe
HH:MM?
There are a few typos (warnin in the commit message, disable ->
disables in the documentation, enf -> end in a comment)
I would also consider long lived connections. The warning currently
only fires when a user connects, maybe it would be useful to also do
something when the user enters the expiration period during an active
connection?
[1]: https://www.postgresql.org/message-id/DDPQ1RV5FE9U.I2WW34NGRD8Z%40jeltef.nl
[2]: https://www.postgresql.org/message-id/CAER375OvH3_ONmc-SgUFpA6gv_d6eNj2KdZktzo-f_uqNwwWNw%40mail.gmail.com
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Jeff Davis | 2026-01-28 19:37:13 | A few pg_locale.c fixes |
| Previous Message | Zsolt Parragi | 2026-01-28 19:21:13 | Re: Custom oauth validator options |