| From: | Gilles Darold <gilles(at)darold(dot)net> |
|---|---|
| To: | Nathan Bossart <nathandbossart(at)gmail(dot)com>, Zsolt Parragi <zsolt(dot)parragi(at)percona(dot)com> |
| 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>, 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-29 21:48:39 |
| Message-ID: | 6eca3872-18e2-4692-851a-14d1bea30760@darold.net |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Le 29/01/2026 à 18:53, Nathan Bossart a écrit :
> Sorry, I haven't been following the discussion, but I took a brief look at
> the latest patch in the thread.
>
> + Controls how much time (in seconds) before a role's password expiration
> + a <literal>WARNING</literal> message is sent to the client upon successful
> + connection. It requires that a <command>VALID UNTIL</command> date is set
> + for the role. A value of <literal>0d</literal> disable this behavior. The
> + default value is <literal>7d</literal> and the maximum value <literal>30d</literal>.
>
> I'm not sure we should subject folks to these warnings by default, and I
> don't see a reason to restrict the maximum value to 30 days. IMHO we
> should have this disabled by default and the maximum value should be
> INT_MAX.
This was my first though but I agree with Tom comment "Off-by-default is
pretty much guaranteed to not help most people.". I will use INT_MAX.
> + 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 / SECS_PER_DAY));
> + }
> + }
>
> nitpick: I suspect we could simplify this code a bit, but I haven't tried.
>
> Also, IMO we should be more precise about the expiration time. There is a
> reasonable difference between a password expiring in 1 second as opposed to
> 23 hours, 59 minutes, 59 seconds, but in both cases this message would say
> "0 days". You might be able to borrow from psql/common.c's PrintTiming()
> function to add more detail here.
Ok, there's now more vote for being more precise about the expiration
time, I will add it.
> + /*
> + * Emit a warning message to the client when set, for example
> + * to warn the user that the password will expire.
> + */
> + if (MyClientConnectionInfo.warning_message)
> + ereport(WARNING, (errmsg("%s", MyClientConnectionInfo.warning_message)));
>
> Having a variable for warning messages could come in handy later. For
> example, we might add a warning about using MD5 passwords at some point.
> In my draft patch for this [0], I put the warning after closing the
> transaction, whereas this patch puts it just before. I'm not sure I had a
> principled reason for doing so, but it's an interesting difference between
> the two patches.
>
> [0] https://postgr.es/m/attachment/177167/v2-0002-WIP-add-warning-upon-authentication-with-MD5-pass.patch
Understood, I will rewrite the patch to use a int variable.
Thanks.
--
Gilles Darold
http://www.darold.net/
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Andres Freund | 2026-01-29 21:49:36 | Re: Buffer locking is special (hints, checksums, AIO writes) |
| Previous Message | Corey Huinker | 2026-01-29 21:40:28 | Re: Optional skipping of unchanged relations during ANALYZE? |