Re: Non-superuser subscription owners
Amit Kapila <amit.kapila16@gmail.com>
From: Amit Kapila <amit.kapila16@gmail.com>
To: Mark Dilger <mark.dilger@enterprisedb.com>
Cc: Jeff Davis <pgsql@j-davis.com>, Andrew Dunstan <andrew@dunslane.net>, PostgreSQL-development <pgsql-hackers@postgresql.org>,
Robert Haas <robertmhaas@gmail.com>
Date: 2021-12-09T04:58:24Z
Lists: pgsql-hackers
On Tue, Dec 7, 2021 at 8:25 PM Mark Dilger <mark.dilger@enterprisedb.com> wrote: > > > On Dec 7, 2021, at 2:29 AM, Amit Kapila <amit.kapila16@gmail.com> wrote: > > > > Okay, let me try to explain again. Following is the text from docs > > [1]: " (a) To create a subscription, the user must be a superuser. (b) > > The subscription apply process will run in the local database with the > > privileges of a superuser. (c) Privileges are only checked once at the > > start of a replication connection. They are not re-checked as each > > change record is read from the publisher, nor are they re-checked for > > each change when applied. > > > > My understanding is that we want to improve what is written as (c) > > which I think is the same as what you mentioned later as "Fix the > > current bug wherein subscription changes are applied with superuser > > force after the subscription owner has superuser privileges revoked.". > > Am I correct till here? If so, I think what I am suggesting should fix > > this with the assumption that we still want to follow (b) at least for > > the first patch. > > Ok, that's a point of disagreement. I was trying to fix both (b) and (c) in the first patch. > But, I think as soon as we are trying to fix (b), we seem to be allowing non-superusers to apply changes. If we want to do that then we should be even allowed to change the owners to non-superusers. I was thinking of the below order: 1. First fix (c) from the above description "Privileges are only checked once at the start of a replication connection." 2A. Allow the transfer of subscriptions to non-superuser owners. This will be allowed only on disabled subscriptions to make this action predictable. 2B. The apply worker should be able to apply the changes provided the user has appropriate privileges on the objects being accessed by apply worker. 3) Allow the creation of subscriptions by non-superusers who are members of some as yet to be created predefined role, say "pg_create_subscriptions" We all seem to agree that (3) can be done later as an independent project. 2A, 2B can be developed as separate patches but they need to be considered together for commit. After 2A, 2B, the first one (1) won't be required so, in fact, we can just ignore (1) but the only benefit I see is that if we stuck with some design problem during the development of 2A, 2B, we would have at least something better than what we have now. You seem to be indicating let's do 2B first as that will anyway be used later after 2A and 1 won't be required if we do that. I see that but I personally feel either we should follow 1, 2(A, B) or just do 2(A, B). -- With Regards, Amit Kapila.
Commits
Same data as JSON:
GET /api/v1/messages/:b64id/commits
the thread's linked commits as JSON, with link sources.
API reference →
-
Fix possible crash in tablesync worker.
- b5c517379a40 16.0 landed
-
Display 'password_required' option for \dRs+ command.
- 19e65dff38bd 16.0 landed
-
Restart the apply worker if the 'password_required' option is changed.
- c1cc4e688b60 16.0 landed
-
Fix possible logical replication crash.
- e7e7da2f8d57 16.0 landed
-
Add new predefined role pg_create_subscription.
- c3afe8cf5a1e 16.0 landed
-
Expand AclMode to 64 bits
- 7b378237aa80 16.0 cited
-
More cleanup of a2ab9c06ea.
- 96a6f11c0625 15.0 landed
-
Respect permissions within logical replication.
- a2ab9c06ea15 15.0 landed
-
Improve table locking behavior in the face of current DDL.
- 2ad36c4e44c8 9.2.0 cited