Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf
Michael Paquier <michael@paquier.xyz>
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>
Date: 2023-01-12T05:32:15Z
Lists: pgsql-hackers
On Wed, Jan 11, 2023 at 03:22:35PM +0100, Jelte Fennema wrote:
> The main uncertainty I have is if the case insensitivity check is
> actually needed in check_role. It seems like a case insensitive
> check against the database user shouldn't actually be necessary.
> I only understand the need for the case insensitive check against
> the system user. But I have too little experience with LDAP/kerberos
> to say for certain. So for now I kept the existing behaviour to
> not regress.
if (!identLine->pg_user->quoted &&
+ identLine->pg_user->string[0] != '+' &&
+ !token_is_keyword(identLine->pg_user, "all") &&
+ !token_has_regexp(identLine->pg_user) &&
If we finish by allowing a regexp for the PG user in an IdentLine, I
would choose to drop "all" entirely. Simpler is better when it comes
to authentication, though we are working on getting things more..
Complicated.
+ Quoting a <replaceable>database-username</replaceable> containing
+ <literal>\1</literal> makes the <literal>\1</literal>
+ lose its special meaning.
0002 and 0003 need careful thinking.
+# Success as the regular expression matches and \1 is replaced
+reset_pg_ident($node, 'mypeermap', qq{/^$system_user(.*)\$},
+ 'test\1mapuser');
+test_role(
+ $node,
+ qq{testmapuser},
+ 'peer',
+ 0,
+ 'with regular expression in user name map with \1',
+ log_like =>
+ [qr/connection authenticated: identity="$system_user" method=peer/]);
Relying on kerberos to check the substitution pattern is a bit
annoying.. I would be really tempted to extract and commit that
independently of the rest, actually, to provide some coverage of the
substitution case in the peer test.
> The patchset also contains 3 preparatory patches with two refactoring
> passes and one small bugfix + test additions.
Applied 0001, which looked fine and was an existing issue. At the
end, I had no issues with the names you suggested.
--
Michael
Commits
-
Add description for new patterns supported in HBA and ident sample files
- 1b43743f1174 16.0 landed
-
Support the same patterns for pg-user in pg_ident.conf as in pg_hba.conf
- efb6f4a4f9b6 16.0 landed
-
Track behavior of \1 in pg_ident.conf when quoted
- 0b717432ff13 16.0 landed
-
Store IdentLine->pg_user as an AuthToken
- 02d3448f4f79 16.0 landed
-
Add tests for regex replacement with \1 in pg_ident.conf to 0003_peer.pl
- e753ae6397fe 16.0 landed
-
Rename some variables related to ident files in hba.{c,h}
- 8607630d74cd 16.0 landed