Re: On login trigger: take three

On 11.12.2020 19:27, Pavel Stehule wrote:
>
>
> pá 11. 12. 2020 v 17:05 odesílatel Konstantin Knizhnik 
> <k.knizhnik@postgrespro.ru <mailto:k.knizhnik@postgrespro.ru>> napsal:
>
>
>
>     On 11.12.2020 18:40, Pavel Stehule wrote:
>>
>>         is not correct. It makes it not possible to superuser to
>>         disable triggers for all users.
>>
>>
>>     pg_database_ownercheck returns true for superuser always.
>
>     Sorry, but I consider different case: when normal user is
>     connected to the database.
>     In this case pg_database_ownercheck returns false and trigger is
>     not disabled, isn't it?
>
>
> My idea was to reduce necessary rights to database owners.  But you 
> have a true, so only superuser can create event trigger, so this 
> feature cannot be used in DBaaS environments, and then my original 
> idea was wrong.
>
>
>>
>>         Also GUCs are not associated with any database. So I do not
>>         understand why  this check of database ownership is relevant
>>         at all?
>>
>>         What kind of protection violation we want to prevent?
>>
>>         It seems to be obvious that normal user should not be able to
>>         prevent trigger execution because this triggers may be used
>>         to enforce some security policies.
>>         If trigger was created by user itself, then it can drop or
>>         disable it using ALTER statement. GUC is not needed for it.
>>
>>
>>     when you cannot connect to the database, then you cannot do
>>     ALTER. In DBaaS environments lot of users has not superuser rights.
>
>
>     But only superusers can set login triggers, right?
>     So only superuser can make a mistake in this trigger. But he have
>     enough rights to recover this error. Normal users are not able to
>     define on connection triggers and
>     should not have rights to disable them.
>
>
> yes, it is true
>
> Pavel
>
>
>     -- 
>     Konstantin Knizhnik
>     Postgres Professional:http://www.postgrespro.com
>     The Russian Postgres Company
>

So what's next?
I see three options:

1. Do not introduce GUC for disabling all event triggers (especially 
taken in account that them are  disabled by default).
Return back to the patch 
on_connect_event_trigger_WITH_SUGGESTED_UPDATES.patch with 
"disable_client_connection_trigger"
and make it true by default (to eliminate any overhead for users which 
are not using on logintriggers).

2. Have two GUCS: "disable_client_connection_trigger" and 
"disable_event_triggers".

3. Implement some mechanism for caching presence of event triggers in 
shared memory.


-- 
Konstantin Knizhnik
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company