Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf

From: Jelte Fennema <postgres(at)jeltef(dot)nl>
To: Michael Paquier <michael(at)paquier(dot)xyz>
Cc: Jelte Fennema <Jelte(dot)Fennema(at)microsoft(dot)com>, "isaac(dot)morland(at)gmail(dot)com" <isaac(dot)morland(at)gmail(dot)com>, "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Nathan Bossart <nathandbossart(at)gmail(dot)com>, Andrew Dunstan <andrew(at)dunslane(dot)net>
Subject: Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf
Date: 2023-01-16 10:53:57
Message-ID: CAGECzQTuW=JwY6-eVRXMxP=_UeH7K_asAOOXXH8-1fHGm-Gbsg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> Still, I am having a few second thoughts about 0003 after thinking
> about it over the weekend. Except if I am missing something, there
> are no issues with 0004 if we keep the current behavior of always
> replacing \1 even if pg-user is quoted? I would certainly add a new
> test case either way.

Yes, 0004 is not dependent on 003 at all. I attached a new version
of 0003 where only a test and some documentation is added.

> Perhaps it would be simpler to use copy_auth_token() in this code path
> and always free the resulting token?

I initially tried that when working on the patch, but copy_auth_token
(surprisingly) doesn't copy the regex field into the new AuthToken.
So we'd have to regenerate it conditionally. Making the copy
conditional seemed just as simple code-wise, with the added
bonus that it's not doing a useless copy.

> In the code path where system-user is a regexp, could it be better
> to skip the replacement of \1 in the new AuthToken if pg-user is
> itself a regexp? The compiled regexp would be the same, but it could
> be considered as a bit confusing, as it can be thought that the
> compiled regexp of pg-user happened after the replacement?

I updated 0004 to prioritize membership checks and regexes over
substitution of \1. I also added tests for this. Prioritizing "all" over
substitution of \1 is not necessary, since by definition "all" does
not include \1.

Attachment Content-Type Size
v5-0003-Only-expand-1-in-pg_ident.conf-when-not-quoted.patch application/octet-stream 2.2 KB
v5-0004-Support-same-user-patterns-in-pg_ident.conf-as-in.patch application/octet-stream 15.9 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Eisentraut 2023-01-16 11:01:47 Re: Refactor recordExtObjInitPriv()
Previous Message Aleksander Alekseev 2023-01-16 10:35:39 [PATCH] Clarify the behavior of the system when approaching XID wraparound