Re: Allow placeholders in ALTER ROLE w/o superuser

Alexander Korotkov <aekorotkov@gmail.com>

From: Alexander Korotkov <aekorotkov@gmail.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Kyotaro Horiguchi <horikyota.ntt@gmail.com>, nathandbossart@gmail.com, steve@supabase.io, pgsql-hackers@postgresql.org
Date: 2022-11-18T21:26:27Z
Lists: pgsql-hackers
Hi!

I'd like to resume this discussion.

On Wed, Jul 20, 2022 at 6:50 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Kyotaro Horiguchi <horikyota.ntt@gmail.com> writes:
> > At Tue, 19 Jul 2022 09:53:39 -0700, Nathan Bossart <nathandbossart@gmail.com> wrote in
> >> Hm.  I would expect ALTER ROLE to store the PGC_SUSET context when executed
> >> by a superuser or a role with privileges via pg_parameter_acl.  Storing all
> >> placeholder GUC settings as PGC_USERSET would make things more restrictive
> >> than they are today.  For example, it would no longer be possible to apply
> >> any ALTER ROLE settings from superusers for placeholders that later become
> >> custom GUCS.
>
> > Returning to the topic, that operation can be allowed in PG15, having
> > being granted by superuser using the GRANT SET ON PARMETER command.
>
> I think that 13d838815 has completely changed the terms that this
> discussion needs to be conducted under.  It seems clear to me now
> that if you want to relax this only-superusers restriction, what
> you have to do is store the OID of the role that issued ALTER ROLE/DB SET,
> and then apply the same checks that would be used in the ordinary case
> where a placeholder is being filled in after being set intra-session.
> That is, we'd no longer assume that a value coming from pg_db_role_setting
> was set with superuser privileges, but we'd know exactly who did set it.

This makes sense.  But do we really need to store the OID of the role?
 validate_option_array_item() already checks if the placeholder option
passes validation for PGC_SUSET.  So, we can just save a flag
indicating that this check was not successful.  If so, then the value
stored can be only used for PGC_USERSET.  Do you think this would be
correct?

------
Regards,
Alexander Korotkov



Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →
  1. Remove extra regress check arguments from test_pg_db_role_setting

  2. meson: Add 'running' test setup, as a replacement for installcheck

  3. Add USER SET parameter values for pg_db_role_setting

  4. Fix sloppy cleanup of roles in privileges.sql.

  5. Drop test user when done with it.

  6. GRANT rights to CURRENT_USER instead of adding roles

  7. Clean up roles from roleattributes test

  8. Make repeated 'make installcheck' runs work