Message-ID: <0774acfb4e613b9959c4e4995ba163cdadc454ef.camel@j-davis.com>
Subject: Re: Non-superuser subscription owners
From: Jeff Davis <pgsql@j-davis.com>
To: Mark Dilger <mark.dilger@enterprisedb.com>
Cc: Andrew Dunstan <andrew@dunslane.net>, PostgreSQL-development
	 <pgsql-hackers@postgresql.org>, Robert Haas <robertmhaas@gmail.com>
Date: Wed, 17 Nov 2021 13:06:32 -0800
In-Reply-To: <CF56AC0D-7495-4E8D-A48F-FF38BD8074EB@enterprisedb.com>
References: <9DFC88D3-1300-4DE8-ACBC-4CEF84399A53@enterprisedb.com>
	 <c1c334a2-5d85-687f-f0dc-0ac8c422ca1a@dunslane.net>
	 <CD02D159-7260-4247-888F-2CFA7AD3798D@enterprisedb.com>
	 <6BB4451E-7B1B-474C-BD1F-DB7531E720C6@enterprisedb.com>
	 <aa28945f9f348307c8ac833a937b5fca732668b8.camel@j-davis.com>
	 <6A2B0FF6-CC86-48CE-B0D3-5401AA5CFEA9@enterprisedb.com>
	 <b6f6ed921b6e219875801857204b14bbc8782e5e.camel@j-davis.com>
	 <CF56AC0D-7495-4E8D-A48F-FF38BD8074EB@enterprisedb.com>
Content-Type: text/plain; charset="UTF-8"
Mime-Version: 1.0
Content-Transfer-Encoding: 7bit
Archived-At: 
 <https://www.postgresql.org/message-id/0774acfb4e613b9959c4e4995ba163cdadc454ef.camel%40j-davis.com>
Precedence: bulk

On Wed, 2021-11-17 at 10:25 -0800, Mark Dilger wrote:
> We may eventually allow non-superusers to create subscriptions, but
> there are lots of details to work out.

I am setting aside the idea of subscriptions created by non-superusers.

My comments were about your idea for "low-power users" that can still
do things with subscriptions. And for that, GRANT seems like a better
fit than ownership.

With v2-0001, there are several things that seem weird to me:

  * Why can there be only one low-power user per subscription?
  * Why is RENAME a separate capability from CREATE/DROP?
  * What if you want to make the privileges more fine-grained, or make
changes in the future? Ownership is a single bit, so it requires that
everyone agree. Maybe some people want RENAME to be a part of that, and
others don't.

GRANT seems to provide better answers here.

> Since we're trying to make subscriptions into things that non-
> superusers can use, we have to deal with some things in those
> functions.

I understand the use case where a superuser isn't required anywhere in
the process, and some special users can create and own subscriptions. I
also understand that's not what these patches are trying to accomplish
(though v2-0003 seems like a good step in that direction).

I don't understand the use case as well where a non-superuser can
merely "use" a subscription. I'm sure such use cases exist and I'm fine
to go along with that idea, but I'd like to understand why ownership
(partial ownership?) is the right way to do this and GRANT is the wrong
way.

> For example, ALTER SUBSCRIPTION can change the database connection
> parameter, or the publication subscribed, or whether
> synchronous_commit is used.  I don't see that a subscription owner
> should necessarily be allowed to mess with that, at least not without
> some other privilege checks beyond mere ownership.

That violates my expectations of what "ownership" means.

> I think this is pretty analogous to how security definer functions
> work.

The analogy to SECURITY DEFINER functions seems to support my
suggestion for GRANT at least as much as your modified definition of
ownership.

> > This would not address the weirdness of the existing code where a
> > superuser loses their superuser privileges but still owns a
> > subscription. But perhaps we can solve that a different way, like
> > just
> > performing a check when someone loses their superuser privileges
> > that
> > they don't own any subscriptions.
> 
> I gave that a slight amount of thought during the design of this
> patch, but didn't think we could refuse to revoke superuser on such a
> basis,

I don't necessarily see a problem there, but I could be missing
something.

>  and didn't see what we should do with the subscription other than
> have it continue to be owned by the recently-non-superuser.  If you
> have a better idea, we can discuss it, but to some degree I think
> that is also orthogonal to the purpose of this patch.  The only sense
> in which this patch depends on that issue is that this patch proposes
> that non-superuser subscription owners are already an issue, and
> therefore that this patch isn't creating a new issue, but rather
> making more sane something that already can happen.

By introducing and documenting a way to get non-superusers to own a
subscription, it makes it more likely that people will do it, and
harder for us to change. That means the standard should be "this is
what we really want", rather than just "more sane than before".

Regards,
	Jeff Davis