Re: Proposal: allow database-specific role memberships

From: Ibrar Ahmed <ibrar(dot)ahmad(at)gmail(dot)com>
To: Kenaniah Cerny <kenaniah(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Cc: Antonin Houska <ah(at)cybertec(dot)at>
Subject: Re: Proposal: allow database-specific role memberships
Date: 2022-09-07 07:50:32
Message-ID: CALtqXTdnCFYf-62+u3H+P9t9Ah79F==OJ5i4xtgaRz9MfS=xUg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, Jul 25, 2022 at 4:03 AM Kenaniah Cerny <kenaniah(at)gmail(dot)com> wrote:

> Hi Antonin,
>
> Thank you again for the detailed review and questions. It was encouraging
> to see the increasing level of nuance in this latest round.
>
> It's not clear from the explanation of the GRANT ... IN DATABASE ... /
>> GRANT
>> ... IN CURRENT DATABASE ... that, even if "membership in ... will be
>> effective only when the recipient is connected to the database ...", the
>> ADMIN option might not be "fully effective".
>
>
> While I'm not entirely sure what you mean by fully effective, it sounds
> like you may have expected a database-specific WITH ADMIN OPTION grant to
> be able to take effect when connected to a different database (such as
> being able to use db_4's database-specific grants when connected to db_3).
> The documentation updated in this patch specifies that membership (for
> database-specific grants) would be effective only when the grantee is
> connected to the same database that the grant was issued for.
>
> In the case of attempting to make a role grant to db_4 from within db_3,
> the user would need to have a cluster-wide admin option for the role being
> granted, as the test case you referenced in your example aims to verify.
>
> I have added a couple of lines to the documentation included with this
> patch in order to clarify.
>
>
>> Specifically on the regression tests:
>>
>> * The function check_memberships() has no parameters - is there a
>> reason not to use a view?
>>
>
> I believe a view would work just as well -- this was an implementation
> detail that was fashioned to match the pre-existing rolenames.sql file's
> test format.
>
>
>> * I'm not sure if the pg_auth_members catalog can contain InvalidOid
>> in
>> other columns than dbid. Thus I think that the query in
>> check_memberships() only needs an outer JOIN for the pg_database
>> table,
>> while the other joins can be inner.
>>
>
> This is probably true. The tests run just as well using inner joins for
> pg_roles, as this latest version of the patch reflects.
>
>
>> * In this part
>>
>> SET SESSION AUTHORIZATION role_read_12_noinherit;
>> SELECT * FROM data; -- error
>> SET ROLE role_read_12; -- error
>> SELECT * FROM data; -- error
>>
>> I think you don't need to query the table again if the SET ROLE
>> statement
>> failed and the same query had been executed before the SET ROLE.
>
>
> I left that last query in place as a sanity check to ensure that
> role_read_12's privileges were indeed not in effect after the call to SET
> ROLE.
>
> As we appear to now be working through the minutiae, it is my hope that
> this will soon be ready for merge.
>
> - Kenaniah
>

The patch requires a rebase, please do that.

Hunk #5 succeeded at 454 (offset 28 lines). 1 out of 5 hunks FAILED
-- saving rejects to file doc/src/sgml/ref/grant.sgml.rej
...
...

--
Ibrar Ahmed

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Ibrar Ahmed 2022-09-07 07:53:07 Re: BufferAlloc: don't take two simultaneous locks
Previous Message Ibrar Ahmed 2022-09-07 07:45:58 Re: Add last_vacuum_index_scans in pg_stat_all_tables