On Wed, Feb 1, 2023 at 4:02 PM Andres Freund <and...@anarazel.de> wrote: > On 2023-01-30 15:32:34 -0500, Robert Haas wrote: > > I had a long think about what to do with ALTER SUBSCRIPTION ... OWNER > > TO in terms of permissions checks. > > As long as owner and run-as are the same, I think it's strongly > preferrable to *not* require pg_create_subscription.
OK. > > Another question around ALTER SUBSCRIPTION ... OWNER TO and also ALTER > > SUBSCRIPTION .. RENAME is whether they ought to fail if you're not a > > superuser and password_required false is set. > > I don't really see a benefit in allowing it, so I'm inclined to go for > the more restrictive option. But this is a really weakly held opinion. I went back and forth on this and ended up with what you propose here. It's simpler to explain this way. > > + /* Is the use of a password mandatory? */ > > + must_use_password = MySubscription->passwordrequired && > > + !superuser_arg(MySubscription->owner); > > There's a few repetitions of this - perhaps worth putting into a helper? I don't think so. It's slightly different each time, because it's pulling data out of different data structures. > This still leaks the connection on error, no? I've attempted to fix this in v4, attached. -- Robert Haas EDB: http://www.enterprisedb.com
v4-0001-Add-new-predefined-role-pg_create_subscriptions.patch
Description: Binary data