Re: On login trigger: take three

Hi,

On 2022-03-13 23:31:03 +0100, Daniel Gustafsson wrote:
> > On 12 Mar 2022, at 03:46, Andres Freund <andres@anarazel.de> wrote:
> 
> >> +   <para>
> >> +     The <literal>login</literal> event occurs when a user logs in to the
> >> +     system.
> >> +     Any bugs in a trigger procedure for this event may prevent successful
> >> +     login to the system. Such bugs may be fixed after first restarting the
> >> +     system in single-user mode (as event triggers are disabled in this mode).
> >> +     See the <xref linkend="app-postgres"/> reference page for details about
> >> +     using single-user mode.
> >> +   </para>
> > 
> > I'm strongly against adding any new dependencies on single user mode.
> > 
> > A saner approach might be a superuser-only GUC that can be set as part of the
> > connection data (e.g. PGOPTIONS='-c ignore_login_event_trigger=true').
> 
> This, and similar approaches, has been discussed in this thread.  I'm not a fan
> of holes punched with GUC's like this, but if you plan on removing single-user
> mode (as I recall seeing in an initdb thread) altogether then that kills the
> discussion. So.

Even if we end up not removing single user mode, I think it's not OK to add
new failure modes that require single user mode to resolve after not-absurd
operations (I'm ok with needing single user mode if somebody does delete from
pg_authid). It's too hard to reach for most.

We already have GUCs like row_security, so it doesn't seem insane to add one
that disables login event triggers. What is the danger that you see?


> Since we already recommend single-user mode to handle broken event triggers, we
> should IMO do something to cover all of them rather than risk ending up with
> one disabling GUC per each event type.  Right now we have this on the CREATE
> EVENT TRIGGER reference page:

IMO the other types of event triggers make it a heck of a lot harder to get
yourself into a situation that you can't get out of...


>     "Event triggers are disabled in single-user mode (see postgres).  If an
>     erroneous event trigger disables the database so much that you can't even
>     drop the trigger, restart in single-user mode and you'll be able to do
>     that."
> Something like a '-c ignore_event_trigger=<event|all>' GUC perhaps? 

Did you mean login instead of event?

Something like it would work for me. It probably should be
GUC_DISALLOW_IN_FILE?

Greetings,

Andres Freund