Re: SYSTEM_USER reserved word implementation

Jacob Champion <jchampion@timescale.com>

From: Jacob Champion <jchampion@timescale.com>
To: Michael Paquier <michael@paquier.xyz>
Cc: "Drouvot, Bertrand" <bdrouvot@amazon.com>, Alvaro Herrera <alvherre@alvh.no-ip.org>, Tom Lane <tgl@sss.pgh.pa.us>, Joe Conway <mail@joeconway.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2022-09-08T17:37:01Z
Lists: pgsql-hackers
On Wed, Sep 7, 2022 at 6:17 PM Michael Paquier <michael@paquier.xyz> wrote:
> >> +       /* Initialize SystemUser now that MyClientConnectionInfo is restored. */
> >> +       InitializeSystemUser(MyClientConnectionInfo.authn_id,
> >> +                                                hba_authname(MyClientConnectionInfo.auth_method));
> >
> > It makes me a little nervous to call hba_authname(auth_method) without
> > checking to see that auth_method is actually valid (which is only true
> > if authn_id is not NULL).
>
> You have mentioned that a couple of months ago if I recall correctly,
> and we pass down an enum value.

Ah, sorry. Do you remember which thread?

I am probably misinterpreting you, but I don't see why auth_method's
being an enum helps. uaReject (and the "reject" string) is not a sane
value to be using in SYSTEM_USER, and the more call stacks away we get
from MyClientConnectionInfo, the easier it is to forget that that
value is junk. As long as the code doesn't get more complicated, I
suppose there's no real harm being done, but it'd be cleaner not to
access auth_method at all if authn_id is NULL. I won't die on that
hill, though.

> There is actually a second and much deeper issue
> here, in the shape of a collation problem.

Oh, none of that sounds fun. :/

--Jacob



Commits

  1. Introduce SYSTEM_USER

  2. Add some information about authenticated identity via log_connections