Re: On login trigger: take three
Pavel Stehule <pavel.stehule@gmail.com>
From: Pavel Stehule <pavel.stehule@gmail.com>
To: Greg Nancarrow <gregn4422@gmail.com>
Cc: Konstantin Knizhnik <k.knizhnik@postgrespro.ru>, PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Date: 2020-12-08T04:26:08Z
Lists: pgsql-hackers
út 8. 12. 2020 v 1:17 odesílatel Greg Nancarrow <gregn4422@gmail.com> napsal: > On Fri, Dec 4, 2020 at 9:05 PM Konstantin Knizhnik > <k.knizhnik@postgrespro.ru> wrote: > > > > As far as I understand Pavel concern was about the case when superuser > > defines wrong login trigger which prevents login to the system > > all user including himself. Right now solution of this problem is to > > include "options='-c disable_session_start_trigger=true'" in connection > > string. > > I do not know if it can be done with pgAdmin. > > > > > As an event trigger is tied to a particular database, and a GUC is > global to the cluster, as long as there is one database in the cluster > for which an event trigger for the "client_connection" event is NOT > defined (say the default "postgres" maintenance database), then the > superuser can always connect to that database, issue "ALTER SYSTEM SET > disable_client_connection_trigger TO true" and reload the > configuration. I tested this with pgAdmin4 and it worked fine for me, > to allow login to a database for which login was previously prevented > due to a badly-defined logon trigger. > yes, it can work .. Maybe for this operation only database owner rights should be necessary. The super user is maybe too strong. There are two maybe generic questions? 1. Maybe we can introduce more generic GUC for all event triggers like disable_event_triggers? This GUC can be checked only by the database owner or super user. It can be an alternative ALTER TABLE DISABLE TRIGGER ALL. It can be protection against necessity to restart to single mode to repair the event trigger. I think so more generic solution is better than special disable_client_connection_trigger GUC. 2. I have no objection against client_connection. It is probably better for the mentioned purpose - possibility to block connection to database. Can be interesting, and I am not sure how much work it is to introduce the second event - session_start. This event should be started after connecting - so the exception there doesn't block connect, and should be started also after the new statement "DISCARD SESSION", that will be started automatically after DISCARD ALL. This feature should not be implemented in first step, but it can be a plan for support pooled connections Regards Pavel > Pavel, is this an acceptable solution or do you still see problems > with this approach? > > > Regards, > Greg Nancarrow > Fujitsu Australia >
Commits
-
Fix some typos in event trigger docs
- 5fce30e77fe1 17.0 landed
-
Use heap_inplace_update() to unset pg_database.dathasloginevt
- 8be93177c46b 17.0 landed
-
Remove the flaky check in event_trigger_login regression test
- 4b885d01f967 17.0 landed
-
Fix instable 006_login_trigger.pl test
- 06be01eb266b 17.0 landed
-
Add support event triggers on authenticated login
- e83d1b0c40cc 17.0 landed
-
Add GUC for temporarily disabling event triggers
- 7750fefdb2b8 17.0 landed
-
Fix typo in reference to __FreeBSD__.
- e52f8b301ed5 16.0 cited
-
Restore robustness of TAP tests that wait for postmaster restart.
- f452aaf7d4a9 14.0 cited
-
Restore the portal-level snapshot after procedure COMMIT/ROLLBACK.
- 84f5c2908dad 14.0 cited