Feedback-ID: i0fe9450f:Fastmail
Date: Tue, 17 Jan 2023 14:10:18 +0900
From: Michael Paquier <michael@paquier.xyz>
To: Jelte Fennema <postgres@jeltef.nl>
Cc: Jelte Fennema <Jelte.Fennema@microsoft.com>,
	"isaac.morland@gmail.com" <isaac.morland@gmail.com>,
	"pgsql-hackers@lists.postgresql.org" <pgsql-hackers@lists.postgresql.org>,
	Nathan Bossart <nathandbossart@gmail.com>,
	Andrew Dunstan <andrew@dunslane.net>
Subject: Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in
 pg_ident.conf
Message-ID: <Y8YtunC3vL+sfr4r@paquier.xyz>
References: <DBBPR83MB0507FEC2E8965012990A80D0F7FC9@DBBPR83MB0507.EURPRD83.prod.outlook.com>
 <Y76YEHolPL4LIAtO@paquier.xyz>
 <CAGECzQTkwELHUOAKhvdA+m3tWbUQySHHkExJV8GAZ1pwgbEgXg@mail.gmail.com>
 <Y7+bXzP5ubRHgoyf@paquier.xyz>
 <CAGECzQRNow4MwkBjgPxywXdJU_K3a9+Pm78JB7De3yQwwkTDew@mail.gmail.com>
 <Y8C9R4dIv5iHH0gF@paquier.xyz>
 <CAGECzQQ_h_-kUg_xy=mm8b6wQ=TSMV=-5h37T32=qODxi+GOMA@mail.gmail.com>
 <Y8TfE699nFya24fp@paquier.xyz>
 <CAGECzQTuW=JwY6-eVRXMxP=_UeH7K_asAOOXXH8-1fHGm-Gbsg@mail.gmail.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="noGynxPu+d65PpwZ"
Content-Disposition: inline
In-Reply-To: <CAGECzQTuW=JwY6-eVRXMxP=_UeH7K_asAOOXXH8-1fHGm-Gbsg@mail.gmail.com>
Archived-At: <https://www.postgresql.org/message-id/Y8YtunC3vL%2Bsfr4r%40paquier.xyz>
Precedence: bulk


--noGynxPu+d65PpwZ
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Mon, Jan 16, 2023 at 11:53:57AM +0100, Jelte Fennema wrote:
>> Perhaps it would be simpler to use copy_auth_token() in this code path
>> and always free the resulting token?
>=20
> 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.

Okay, I can live with that.

>> 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?
>=20
> 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.

Thanks, 0003 is OK, so applied now.

0004 looks fine as well, be it for the tests (I am hesitating to tweak
things a bit here actually for the role names), the code or the docs,
still I am planning a second lookup.
--
Michael

--noGynxPu+d65PpwZ
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAmPGLboACgkQnvQgOdby
QH0BtRAApqHVWkc9LhiiNzhoeZ3uiX/wOpP1H8v+LwIGURt4E+Mpt9TJ/nRioSD1
SMbae6vqpwx05wDL/iIkvF8iN75yfrLN0t8qtlL0wCHU/qwYOFZ++ydoK+mCTUvD
44IeARtS/ZAjWzksFQVN0JBSJ1ESdYk146hWaY89zx2PjlKvT+Erz2jBDivRov3V
99uFbqR+ZgTrTNHRJnkU57fdRkJlKgWJaY69rGW+n/9d60BKtZzPsnVIwAE+Myzg
ZVu+DwNNREPBp0XbGaFu4ICAc1+l2dLtGeE0xFKLLsoOofxqY9RcIrm0yCaDvWZk
VXg2nxqi8/BIGiZGIKWLzgvBLumw9MWsLuVloAU9NC0ORK7497UV3vYbmgPXsGxN
KtzfdE4A0DLOX+yhU9NSoCt5Vqp/ehxf36x+KEKIH0xB9R2zLMkylO7oWvVNXAOg
W/9wOjXOaIbFjLkDNPjrWbwTOYCZ7jum8ArRU8VusZQk9f5Ga3UB5RxhpQFnqdZR
syt323Y4duV2Uz1yVz90nxt0HO7XS467D45MEXEwITwGHwKBX3WNPfu2ZtB6bapa
i3/hqni6wiNm2X1CvpfkOrh0NdWQoPtGW6rVfj3v4wHtuAhde237tu9ZSNaLIaqc
0k8fSgZtJ1SepxMyfVlrx2Dl7yKDsOXTBDdbfHOoP6k0PtVYiTo=
=k+Pf
-----END PGP SIGNATURE-----

--noGynxPu+d65PpwZ--