Re: Non-superuser subscription owners

> On Nov 17, 2021, at 9:33 AM, Jeff Davis <pgsql@j-davis.com> wrote:
>
> I am still trying to understand this use case. It doesn't feel like
> "ownership" to me, it feels more like some kind of delegation.
>
> Is GRANT a better fit here? That would allow more than one user to
> REFRESH, or ENABLE/DISABLE the same subscription. It wouldn't allow
> RENAME, but I don't see why we'd separate privileges for
> CREATE/DROP/RENAME anyway.

We may eventually allow non-superusers to create subscriptions, but there are lots of details to work out.  Should
therebe limits on how many subscriptions they can create?  Should there be limits to the number of simultaneously open
connectionsthey can create out to other database servers (publishers)?  Should they need to be granted USAGE on a
databasepublisher in order to use the connection string for that publisher in a subscription they create?  Should they
needto be granted USAGE on a publication in order to replicate it?  Yes, there may be restrictions on the publisher
side,too, but the user model on subscriber and publisher might differ, and the connection string used might not match
thesubscription owner, so some restriction on the subscriber side may be needed. 

The implementation of [CREATE | ALTER] SUBSCRIPTION was designed at a time when only superusers could execute them, and
asfar as I can infer from the design, no effort to constrain the effects of those commands was made.  Since we're
tryingto make subscriptions into things that non-superusers can use, we have to deal with some things in those
functions. For example, ALTER SUBSCRIPTION can change the database connection parameter, or the publication subscribed,
orwhether 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. 

I think this is pretty analogous to how security definer functions work.  You might call those "delegation" also, but
thebasic idea is that the function will run under the privileges of the function's owner, who might be quite privileged
ifyou want the function to do highly secure things for you, but who could also intentionally be limited in privilege.
Itwouldn't make much sense to say the owner of a security definer function can arbitrarily escalate their privileges to
dothings like open connections to other database servers, or have the transactions in which they run have a different
settingof synchronous_commit.  Yet with subscriptions, if the subscription owner can run all forms of ALTER
SUBSCRIPTION,that's what they can do. 

I took a conservative position in the design of the patch to avoid giving away too much.  I suspect that we'll come
backto these design decisions and relax them at some point, but the exact way in which we relax them is not obvious.
Wecould just agree to remove them (as you seem to propose), or we might agree to create predefined roles and say that
thesubscription owner can change certain aspects of the subscription if and only if they are members of one or more of
thoseroles, or we may create new grantable privileges.  Each of those debates may be long and hard fought, so I don't
wantto invite that as part of this thread, or this patch will almost surely miss the cutoff for v15. 

> 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
superuseron such a basis, and didn't see what we should do with the subscription other than have it continue to be
ownedby the recently-non-superuser.  If you have a better idea, we can discuss it, but to some degree I think that is
alsoorthogonal to the purpose of this patch.  The only sense in which this patch depends on that issue is that this
patchproposes that non-superuser subscription owners are already an issue, and therefore that this patch isn't creating
anew issue, but rather making more sane something that already can happen. 

—
Mark Dilger
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company