Re: On login trigger: take three
Konstantin Knizhnik <k.knizhnik@postgrespro.ru>
From: Konstantin Knizhnik <k.knizhnik@postgrespro.ru>
To: Greg Nancarrow <gregn4422@gmail.com>,
Pavel Stehule <pavel.stehule@gmail.com>
Cc: PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Date: 2020-12-04T10:05:45Z
Lists: pgsql-hackers
On 04.12.2020 3:50, Greg Nancarrow wrote: > On Tue, Sep 15, 2020 at 2:12 AM Pavel Stehule <pavel.stehule@gmail.com> wrote: >>> It is always possible to login by disabling startup triggers using disable_session_start_trigger GUC: >>> >>> psql "dbname=postgres options='-c disable_session_start_trigger=true'" >> >> sure, I know. Just this behavior can be a very unpleasant surprise, and my question is if it can be fixed. Creating custom libpq variables can be the stop for people that use pgAdmin. >> > Hi, > > I thought in the case of using pgAdmin (assuming you can connect as > superuser to a database, say the default "postgres" maintenance > database, that doesn't have an EVENT TRIGGER defined for the > session_start event) you could issue the query "ALTER SYSTEM SET > disable_session_start_trigger TO true;" and then reload the > configuration? 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. > > Anyway, I am wondering if this patch is still being actively developed/improved? I do not know what else has to be improved. If you, Pavel or anybody else have some suggestions: please let me know. > > Regarding the last-posted patch, I'd like to give some feedback. I > found that the documentation part wouldn't build because of errors in > the SGML tags. There are some grammatical errors too, and some minor > inconsistencies with the current documentation, and some descriptions > could be improved. I think that a colon separator should be added to > the NOTICE message for superuser, so it's clear exactly where the text > of the underlying error message starts. Also, I think that > "client_connection" is perhaps a better and more intuitive event name > than "session_start", or the suggested "user_backend_start". > I've therefore attached an updated patch with these suggested minor > improvements, please take a look and see what you think (please > compare with the original patch). Thank you very much for detecting the problems and much more thanks for fixing them and providing your version of the patch. I have nothing against renaming "session_start" to "client_connection". -- Konstantin Knizhnik Postgres Professional: http://www.postgrespro.com The Russian Postgres Company
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