Special role for subscriptions

classic Classic list List threaded Threaded
11 messages Options
Reply | Threaded
Open this post in threaded view
|

Special role for subscriptions

Evgeniy Efimkin
Hi hackers!
In postgresql 10 and 11 only superuser can create/alter subscriptions.
If there was a special role (like pg_monitor), it would be more easy to grant control on subscriptions.
I can make a patch if there are no objections against it.

Reply | Threaded
Open this post in threaded view
|

Re: Special role for subscriptions

Stephen Frost
Greetings,

* Evgeniy Efimkin ([hidden email]) wrote:
> In postgresql 10 and 11 only superuser can create/alter subscriptions.
> If there was a special role (like pg_monitor), it would be more easy to grant control on subscriptions.
> I can make a patch if there are no objections against it.

I think the short answer is 'yes, we should let non-superusers do that',
but the longer answer is:

What level of access makes sense for managing subscriptions?  Should
there be a way to say "user X is allowed to create a subscription for
remote system Y, but only for tables that exist in schema Q"?

My general feeling is 'yes', though, of course, I don't want to say that
we have to have all of that before we move forward with allowing
non-superusers to create subscriptions, but I do think we want to make
sure that we have a well thought-out path for how to get from where we
are now to a system which has a lot more granularity, and to do our best
to try avoiding any paths that might paint us into a corner.

Thanks!

Stephen

signature.asc (836 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Special role for subscriptions

Evgeniy Efimkin
Hi!
As a first step I suggest we allow CREATE SUBSCRIPTION for table owner only.

03.11.2018, 19:20, "Stephen Frost" <[hidden email]>:

> Greetings,
>
> * Evgeniy Efimkin ([hidden email]) wrote:
>>  In postgresql 10 and 11 only superuser can create/alter subscriptions.
>>  If there was a special role (like pg_monitor), it would be more easy to grant control on subscriptions.
>>  I can make a patch if there are no objections against it.
>
> I think the short answer is 'yes, we should let non-superusers do that',
> but the longer answer is:
>
> What level of access makes sense for managing subscriptions? Should
> there be a way to say "user X is allowed to create a subscription for
> remote system Y, but only for tables that exist in schema Q"?
>
> My general feeling is 'yes', though, of course, I don't want to say that
> we have to have all of that before we move forward with allowing
> non-superusers to create subscriptions, but I do think we want to make
> sure that we have a well thought-out path for how to get from where we
> are now to a system which has a lot more granularity, and to do our best
> to try avoiding any paths that might paint us into a corner.
>
> Thanks!
>
> Stephen



Reply | Threaded
Open this post in threaded view
|

Re: Special role for subscriptions

Stephen Frost
Greetings,

* Evgeniy Efimkin ([hidden email]) wrote:
> As a first step I suggest we allow CREATE SUBSCRIPTION for table owner only.

That's a nice idea but seems like we would want to have a way to filter
what tables a subscription follows then..?  Just failing if the
publication publishes tables that we don't have access to or are not the
owner of seems like a poor solution..

Thanks!

Stephen

signature.asc (836 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Special role for subscriptions

Evgeniy Efimkin
Hi!
I think we can add FOR TABLES clause for create/refresh subscription, for example: CREATE SUBSCRIPTION my_sub CONNECTION ... PUBLICATION my_pub [WITH ...] [ FOR TABLES t1, t2 | ALL TABLES ]. ALL TABLES is avalibale only for superuser. FOR TABLES t1, t2 is available to owner of tables and superuser.

07.11.2018, 00:52, "Stephen Frost" <[hidden email]>:

> Greetings,
>
> * Evgeniy Efimkin ([hidden email]) wrote:
>>  As a first step I suggest we allow CREATE SUBSCRIPTION for table owner only.
>
> That's a nice idea but seems like we would want to have a way to filter
> what tables a subscription follows then..? Just failing if the
> publication publishes tables that we don't have access to or are not the
> owner of seems like a poor solution..
>
> Thanks!
>
> Stephen

--------
Ефимкин Евгений



Reply | Threaded
Open this post in threaded view
|

Re: Special role for subscriptions

Evgeniy Efimkin
In reply to this post by Stephen Frost
Hi!
In order to support create subscription from non-superuser, we need to make it possible to choose tables on the subscriber side:
    1. add `FOR TABLE` clause in `CREATE SUBSCRIPTION`:
       ```
        CREATE SUBSCRIPTION subscription_name
            CONNECTION 'conninfo'
            PUBLICATION publication_name [, ...]
            [ FOR TABLE [ ONLY ] table_name [ * ] [, ...]| FOR ALL TABLES ]
            [ WITH ( subscription_parameter [= value] [, ... ] ) ]
       ```
       ... where `FOR ALL TABLES` is only allowed for superuser.
       and table list in `FOR TABLES` clause will be stored in pg_subscription_rel table (maybe another place?)

    2. Each subscription should have "all tables" attribute.
       For example via a new column in pg_subscription "suballtables".

    3. Add `ALTER SUBSCRIPTION (ADD TABLE | DROP TABLE)`:
       ```
        ALTER SUBSCRIPTION subscription_name ADD TABLE [ ONLY ] table_name [WITH copy_data];
        ALTER SUBSCRIPTION subscription_name DROP TABLE [ ONLY ] table_name;
       ```
    4. On `ALTER SUBSCRIPTION <name> REFRESH PUBLICATION` should check if table owner equals subscription owner. The check is ommited if subscription owner is superuser.
    5. If superuser calls `ALTER SUBSCRIPTION REFRESH PUBLICATION` on subscription with table list and non-superuser owner, we should filter tables which owner is not subscription's owner or maybe we need to raise error?

What do you think about it? Any objections?

07.11.2018, 00:52, "Stephen Frost" <[hidden email]>:

> Greetings,
>
> * Evgeniy Efimkin ([hidden email]) wrote:
>>  As a first step I suggest we allow CREATE SUBSCRIPTION for table owner only.
>
> That's a nice idea but seems like we would want to have a way to filter
> what tables a subscription follows then..? Just failing if the
> publication publishes tables that we don't have access to or are not the
> owner of seems like a poor solution..
>
> Thanks!
>
> Stephen

--------
Ефимкин Евгений



Reply | Threaded
Open this post in threaded view
|

Re: [WIP] Special role for subscriptions

Evgeniy Efimkin
Hello!
I started work on patch (draft attached). Draft has changes related only to `CREATE SUBSCRIPTION`.
I also introduce a new status (DEFFERED)  for tables in `FOR TABLE` clause (but not in publication).
New column in pg_subscription (suballtables) will be used in `REFRESH` clause

09.11.2018, 15:24, "Evgeniy Efimkin" <[hidden email]>:

> Hi!
> In order to support create subscription from non-superuser, we need to make it possible to choose tables on the subscriber side:
>     1. add `FOR TABLE` clause in `CREATE SUBSCRIPTION`:
>        ```
>         CREATE SUBSCRIPTION subscription_name
>             CONNECTION 'conninfo'
>             PUBLICATION publication_name [, ...]
>             [ FOR TABLE [ ONLY ] table_name [ * ] [, ...]| FOR ALL TABLES ]
>             [ WITH ( subscription_parameter [= value] [, ... ] ) ]
>        ```
>        ... where `FOR ALL TABLES` is only allowed for superuser.
>        and table list in `FOR TABLES` clause will be stored in pg_subscription_rel table (maybe another place?)
>
>     2. Each subscription should have "all tables" attribute.
>        For example via a new column in pg_subscription "suballtables".
>
>     3. Add `ALTER SUBSCRIPTION (ADD TABLE | DROP TABLE)`:
>        ```
>         ALTER SUBSCRIPTION subscription_name ADD TABLE [ ONLY ] table_name [WITH copy_data];
>         ALTER SUBSCRIPTION subscription_name DROP TABLE [ ONLY ] table_name;
>        ```
>     4. On `ALTER SUBSCRIPTION <name> REFRESH PUBLICATION` should check if table owner equals subscription owner. The check is ommited if subscription owner is superuser.
>     5. If superuser calls `ALTER SUBSCRIPTION REFRESH PUBLICATION` on subscription with table list and non-superuser owner, we should filter tables which owner is not subscription's owner or maybe we need to raise error?
>
> What do you think about it? Any objections?
>
> 07.11.2018, 00:52, "Stephen Frost" <[hidden email]>:
>>  Greetings,
>>
>>  * Evgeniy Efimkin ([hidden email]) wrote:
>>>   As a first step I suggest we allow CREATE SUBSCRIPTION for table owner only.
>>
>>  That's a nice idea but seems like we would want to have a way to filter
>>  what tables a subscription follows then..? Just failing if the
>>  publication publishes tables that we don't have access to or are not the
>>  owner of seems like a poor solution..
>>
>>  Thanks!
>>
>>  Stephen
>
> --------
> Ефимкин Евгений
--------
Ефимкин Евгений

create_subscription.patch (14K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [WIP] CREATE SUBSCRIPTION with FOR TABLES clause (table filter)

Evgeniy Efimkin

Hello!
New draft attached with filtering table in subscription (ADD/DROP) and allow non-superusers use` CREATE SUBSCRIPTION` for own tables.

14.11.2018, 18:10, "Evgeniy Efimkin" <[hidden email]>:

> Hello!
> I started work on patch (draft attached). Draft has changes related only to `CREATE SUBSCRIPTION`.
> I also introduce a new status (DEFFERED) for tables in `FOR TABLE` clause (but not in publication).
> New column in pg_subscription (suballtables) will be used in `REFRESH` clause
>
> 09.11.2018, 15:24, "Evgeniy Efimkin" <[hidden email]>:
>>  Hi!
>>  In order to support create subscription from non-superuser, we need to make it possible to choose tables on the subscriber side:
>>      1. add `FOR TABLE` clause in `CREATE SUBSCRIPTION`:
>>         ```
>>          CREATE SUBSCRIPTION subscription_name
>>              CONNECTION 'conninfo'
>>              PUBLICATION publication_name [, ...]
>>              [ FOR TABLE [ ONLY ] table_name [ * ] [, ...]| FOR ALL TABLES ]
>>              [ WITH ( subscription_parameter [= value] [, ... ] ) ]
>>         ```
>>         ... where `FOR ALL TABLES` is only allowed for superuser.
>>         and table list in `FOR TABLES` clause will be stored in pg_subscription_rel table (maybe another place?)
>>
>>      2. Each subscription should have "all tables" attribute.
>>         For example via a new column in pg_subscription "suballtables".
>>
>>      3. Add `ALTER SUBSCRIPTION (ADD TABLE | DROP TABLE)`:
>>         ```
>>          ALTER SUBSCRIPTION subscription_name ADD TABLE [ ONLY ] table_name [WITH copy_data];
>>          ALTER SUBSCRIPTION subscription_name DROP TABLE [ ONLY ] table_name;
>>         ```
>>      4. On `ALTER SUBSCRIPTION <name> REFRESH PUBLICATION` should check if table owner equals subscription owner. The check is ommited if subscription owner is superuser.
>>      5. If superuser calls `ALTER SUBSCRIPTION REFRESH PUBLICATION` on subscription with table list and non-superuser owner, we should filter tables which owner is not subscription's owner or maybe we need to raise error?
>>
>>  What do you think about it? Any objections?
>>
>>  07.11.2018, 00:52, "Stephen Frost" <[hidden email]>:
>>>   Greetings,
>>>
>>>   * Evgeniy Efimkin ([hidden email]) wrote:
>>>>    As a first step I suggest we allow CREATE SUBSCRIPTION for table owner only.
>>>
>>>   That's a nice idea but seems like we would want to have a way to filter
>>>   what tables a subscription follows then..? Just failing if the
>>>   publication publishes tables that we don't have access to or are not the
>>>   owner of seems like a poor solution..
>>>
>>>   Thanks!
>>>
>>>   Stephen
>>
>>  --------
>>  Ефимкин Евгений
>
> --------
> Ефимкин Евгений
--------
Ефимкин Евгений

subscription.diff (28K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [WIP] CREATE SUBSCRIPTION with FOR TABLES clause (table filter)

Evgeniy Efimkin
Hello!
I wrote some tests(it's just 01_rep_changes.pl but for non superuser) and fix `DROP TABLE` from subscription. Now old and new tests pass.

22.11.2018, 16:23, "Evgeniy Efimkin" <[hidden email]>:

> Hello!
> New draft attached with filtering table in subscription (ADD/DROP) and allow non-superusers use` CREATE SUBSCRIPTION` for own tables.
>
> 14.11.2018, 18:10, "Evgeniy Efimkin" <[hidden email]>:
>>  Hello!
>>  I started work on patch (draft attached). Draft has changes related only to `CREATE SUBSCRIPTION`.
>>  I also introduce a new status (DEFFERED) for tables in `FOR TABLE` clause (but not in publication).
>>  New column in pg_subscription (suballtables) will be used in `REFRESH` clause
>>
>>  09.11.2018, 15:24, "Evgeniy Efimkin" <[hidden email]>:
>>>   Hi!
>>>   In order to support create subscription from non-superuser, we need to make it possible to choose tables on the subscriber side:
>>>       1. add `FOR TABLE` clause in `CREATE SUBSCRIPTION`:
>>>          ```
>>>           CREATE SUBSCRIPTION subscription_name
>>>               CONNECTION 'conninfo'
>>>               PUBLICATION publication_name [, ...]
>>>               [ FOR TABLE [ ONLY ] table_name [ * ] [, ...]| FOR ALL TABLES ]
>>>               [ WITH ( subscription_parameter [= value] [, ... ] ) ]
>>>          ```
>>>          ... where `FOR ALL TABLES` is only allowed for superuser.
>>>          and table list in `FOR TABLES` clause will be stored in pg_subscription_rel table (maybe another place?)
>>>
>>>       2. Each subscription should have "all tables" attribute.
>>>          For example via a new column in pg_subscription "suballtables".
>>>
>>>       3. Add `ALTER SUBSCRIPTION (ADD TABLE | DROP TABLE)`:
>>>          ```
>>>           ALTER SUBSCRIPTION subscription_name ADD TABLE [ ONLY ] table_name [WITH copy_data];
>>>           ALTER SUBSCRIPTION subscription_name DROP TABLE [ ONLY ] table_name;
>>>          ```
>>>       4. On `ALTER SUBSCRIPTION <name> REFRESH PUBLICATION` should check if table owner equals subscription owner. The check is ommited if subscription owner is superuser.
>>>       5. If superuser calls `ALTER SUBSCRIPTION REFRESH PUBLICATION` on subscription with table list and non-superuser owner, we should filter tables which owner is not subscription's owner or maybe we need to raise error?
>>>
>>>   What do you think about it? Any objections?
>>>
>>>   07.11.2018, 00:52, "Stephen Frost" <[hidden email]>:
>>>>    Greetings,
>>>>
>>>>    * Evgeniy Efimkin ([hidden email]) wrote:
>>>>>     As a first step I suggest we allow CREATE SUBSCRIPTION for table owner only.
>>>>
>>>>    That's a nice idea but seems like we would want to have a way to filter
>>>>    what tables a subscription follows then..? Just failing if the
>>>>    publication publishes tables that we don't have access to or are not the
>>>>    owner of seems like a poor solution..
>>>>
>>>>    Thanks!
>>>>
>>>>    Stephen
>>>
>>>   --------
>>>   Ефимкин Евгений
>>
>>  --------
>>  Ефимкин Евгений
>
> --------
> Ефимкин Евгений
--------
Ефимкин Евгений

subscription_with_tests.diff (50K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [WIP] CREATE SUBSCRIPTION with FOR TABLES clause (table filter)

Andrey Borodin-2
Hi, Evgeniy!

Thanks for working on the feature.
> 28 нояб. 2018 г., в 21:41, Evgeniy Efimkin <[hidden email]> написал(а):
>
> Hello!
> I wrote some tests(it's just 01_rep_changes.pl but for non superuser) and fix `DROP TABLE` from subscription. Now old and new tests pass.
>
> 22.11.2018, 16:23, "Evgeniy Efimkin" <[hidden email]>:
>> Hello!
>> New draft attached with filtering table in subscription (ADD/DROP) and allow non-superusers use` CREATE SUBSCRIPTION` for own tables.

I've looked into the patch. The code looks good and coherent to nearby code.
There are no docs, obviously, there is WiP.

I've got few questions:
1. How will the subscription work for inherited tables? Do we need tests for that?
2. ALTER PUBLICATION has ADD\DROP and SET. Should we add SET too? Or is there a reason not to do that?
3. Message "Must be superuser to create FOR ALL TABLES subscriptions" seems a bit strange to me. Also, this literal is embedded into translations. I do not know how we deal with it, how do we deal for example with "måste vara superanvändare för att skapa prenumerationer" or "для создания подписок нужно быть суперпользователем"? Where do we insert FOR ALL TABLES?
4. How does default behavior differ from FOR ALL TABLES?
5. Can we alter subscription FOR ALL TABLES? Drop some tables out of the subscription?

Best regards, Andrey Borodin.
Reply | Threaded
Open this post in threaded view
|

Re: [WIP] CREATE SUBSCRIPTION with FOR TABLES clause (table filter)

Evgeniy Efimkin
Hello!

Thank you for questions!

> I've got few questions:
> 1. How will the subscription work for inherited tables? Do we need tests for that?
For subscription created with `FOR TABLE` we can't support inherit tables because subscriber don't know anything about inherit. In new patch i remove `ONLY` for `FOR TABLE` in subscription related statements
> 2. ALTER PUBLICATION has ADD\DROP and SET. Should we add SET too? Or is there a reason not to do that?
Added it in new patch
> 3. Message "Must be superuser to create FOR ALL TABLES subscriptions" seems a bit strange to me. Also, this literal is embedded into translations. I do not know how we deal with it, how do we deal for example with "måste vara superanvändare för att skapa prenumerationer" or "для создания подписок нужно быть суперпользователем"? Where do we insert FOR ALL TABLES?
I add hint `Use CREATE SUBSCRIPTION ... FOR TABLE ...`
> 4. How does default behavior differ from FOR ALL TABLES?
The same with default implementation
> 5. Can we alter subscription FOR ALL TABLES? Drop some tables out of the subscription?
For subscriptions created with `FOR ALL TABLES` (default), you can't change subscribed tables by `ALTER SUBSCRIPTION ADD/DROP` table, you should use `ALTER SUBSCRIPTION REFRESH PUBLICATION`

And i don't know how do export for user created subscriptions, because now non superuser can't select subconninfo column

03.12.2018, 09:06, "Andrey Borodin" <[hidden email]>:

> Hi, Evgeniy!
>
> Thanks for working on the feature.
>>  28 нояб. 2018 г., в 21:41, Evgeniy Efimkin <[hidden email]> написал(а):
>>
>>  Hello!
>>  I wrote some tests(it's just 01_rep_changes.pl but for non superuser) and fix `DROP TABLE` from subscription. Now old and new tests pass.
>>
>>  22.11.2018, 16:23, "Evgeniy Efimkin" <[hidden email]>:
>>>  Hello!
>>>  New draft attached with filtering table in subscription (ADD/DROP) and allow non-superusers use` CREATE SUBSCRIPTION` for own tables.
>
> I've looked into the patch. The code looks good and coherent to nearby code.
> There are no docs, obviously, there is WiP.
>
> I've got few questions:
> 1. How will the subscription work for inherited tables? Do we need tests for that?
> 2. ALTER PUBLICATION has ADD\DROP and SET. Should we add SET too? Or is there a reason not to do that?
> 3. Message "Must be superuser to create FOR ALL TABLES subscriptions" seems a bit strange to me. Also, this literal is embedded into translations. I do not know how we deal with it, how do we deal for example with "måste vara superanvändare för att skapa prenumerationer" or "для создания подписок нужно быть суперпользователем"? Where do we insert FOR ALL TABLES?
> 4. How does default behavior differ from FOR ALL TABLES?
> 5. Can we alter subscription FOR ALL TABLES? Drop some tables out of the subscription?
>
> Best regards, Andrey Borodin.
--------
Ефимкин Евгений

subscription_with_set_table.diff (61K) Download Attachment