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-11 14:22:35
Message-ID: CAGECzQTkwELHUOAKhvdA+m3tWbUQySHHkExJV8GAZ1pwgbEgXg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> couldn't we also use a regexp for the pg-role rather than
> just a hardcoded keyword here then, so as it would be possible to
> allow a mapping to pass for a group of role names? "all" is just a
> pattern to allow everything, at the end.

That's a good point. I hadn't realised that you added support for
regexes in pg_hba.conf in 8fea868. Attached is a patchset
where I reuse the pg_hba.conf code path to add support to
pg_ident.conf for: all, +group and regexes.

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.

The patchset also contains 3 preparatory patches with two refactoring
passes and one small bugfix + test additions.

> - renaming "systemuser" to "system_user_token" to outline that this is
> not a simple string but an AuthToken with potentially a regexp?

I decided against this, since now both system user and database user
are tokens. Furthermore, compiler warnings should avoid any confusion
against using this as a normal string. If you feel strongly about this
though, I'm happy to change this.

On Wed, 11 Jan 2023 at 14:34, Michael Paquier <michael(at)paquier(dot)xyz> wrote:
>
> On Wed, Jan 11, 2023 at 09:04:56AM +0000, Jelte Fennema wrote:
> > It's very different. I think easiest is to explain by example:
> >
> > If there exist three users on the postgres server: admin, jelte and michael
> >
> > Then this rule (your suggested rule):
> > mapname /^(.*)$ \1
> >
> > Is equivalent to:
> > mapname admin admin
> > mapname jelte jelte
> > mapname michael michael
> >
> > While with the "all" keyword you can create a rule like this:
> > mapname admin all
> >
> > which is equivalent to:
> > mapname admin admin
> > mapname admin jelte
> > mapname admin michael
>
> Thanks for the explanation, I was missing your point. Hmm. On top
> of my mind, couldn't we also use a regexp for the pg-role rather than
> just a hardcoded keyword here then, so as it would be possible to
> allow a mapping to pass for a group of role names? "all" is just a
> pattern to allow everything, at the end.
> --
> Michael

Attachment Content-Type Size
v3-0004-Support-same-user-patterns-in-pg_ident.conf-as-in.patch application/octet-stream 14.0 KB
v3-0002-Store-pg_user-as-AuthToken-in-IdentLine.patch application/octet-stream 4.7 KB
v3-0003-Only-expand-1-in-pg_ident.conf-when-not-quoted.patch application/octet-stream 3.4 KB
v3-0001-Make-naming-in-code-for-username-maps-consistent.patch application/octet-stream 11.1 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Eisentraut 2023-01-11 14:22:43 Re: doc: add missing "id" attributes to extension packaging page
Previous Message Peter Eisentraut 2023-01-11 14:08:46 Re: Rework of collation code, extensibility