Re: Pasword expiration warning - Mailing list pgsql-hackers

From Nathan Bossart
Subject Re: Pasword expiration warning
Date
Msg-id aXuehKtwtUXWVFYp@nathan
Whole thread Raw
In response to Re: Pasword expiration warning  (Zsolt Parragi <zsolt.parragi@percona.com>)
Responses Re: Pasword expiration warning
Re: Pasword expiration warning
List pgsql-hackers
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.

+        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.

+    /*
+     * 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

-- 
nathan



pgsql-hackers by date:

Previous
From: Peter Geoghegan
Date:
Subject: Re: Buffer locking is special (hints, checksums, AIO writes)
Next
From: Andres Freund
Date:
Subject: Re: Buffer locking is special (hints, checksums, AIO writes)