Dear colleagues, Please see my suggestions below and the updated patch attached. >Среда, 29 сентября 2021, 15:14 +03:00 от Teodor Sigaev < teo...@sigaev.ru >: > >Hi! > >Nice feature, but, sorry, I see some design problem in suggested feature. >AFAIK, >there is two use cases for this feature: >1 A permission / prohibition to login some users >2 Just a logging of facts of user's login > >Suggested patch proposes prohibition of login only by failing of login trigger >and it has at least two issues: >1 In case of prohibition to login, there is no clean way to store information >about unsuccessful login. Ok, it could be solved by dblink module but it seems >to scary. This is a common problem of logging errors and unsuccessful transactions of any kind. It can be solved by: - logging to external log storage (stupid logging to files or syslog or whatever you can imagine with PL/Perl (sorry)) - logging inside the database by db_link or through background worker (like described in https://dpavlin.wordpress.com/2017/05/09/david-rader-autonomous-transactions-using-pg_background/ ) - or by implementing autonomous transactions in PostgreSQL, which is already under development by some of my and Teodor’s colleagues. So I propose not to invent another solution to this common problem here. >2 For logging purpose failing of trigger will cause impossibility to login, it >could be workarounded by catching error in trigger function, but it's not so >obvious for DBA. If the trigger contains an error, nobody can login. The database is bricked. Previous variant of the patch proposed to fix this with going to single-user mode. For faster recovery I propose to have also a GUC variable to turn on/off the login triggers. It should be 'on' by default. > >some other issues: >3 It's easy to create security hole, see attachment where non-privileged user >can close access to database even for superuser. Such cases can be avoided by careful design of the login triggers and related permissions. Added such note to the documentation. >4 Feature is not applicable for logging unsuccessful login with wrong >username/password or not matched by pg_hba.conf. Oracle could operate with such >cases. But I don't think that this issue is a stopper for the patch. Yes. Btw, note that pg_hba functionality can be implemented completely inside the login trigger :) ! > >May be, to solve that issues we could introduce return value of trigger and/or >add something like IGNORE ERROR to CREATE EVENT TRIGGER command. Any solutions which make syntax more complicated can lead Postgres to become Oracle (in the worst sense). I do not like this.
A new version of the patch is attached. It applies to the current master and contains the above mentioned GUC code, relevant tests and docs. Regards, Ivan Panchenko > >On 15.09.2021 16:32, Greg Nancarrow wrote: >> On Wed, Sep 8, 2021 at 10:56 PM Daniel Gustafsson < dan...@yesql.se > wrote: >>> >>> I took a look at this, and while I like the proposed feature I think the >>> patch >>> has a bit more work required. >>> >> >> Thanks for reviewing the patch. >> I am not the original patch author (who no longer seems active) but >> I've been contributing a bit and keeping the patch alive because I >> think it's a worthwhile feature. >> >>> >>> + >>> +-- 2) Initialize some user session data >>> + CREATE TEMP TABLE session_storage (x float, y integer); >>> The example in the documentation use a mix of indentations, neither of >>> which is >>> the 2-space indentation used elsewhere in the docs. >>> >> >> Fixed, using 2-space indentation. >> (to be honest, the indentation seems inconsistent elsewhere in the >> docs e.g. I'm seeing a nearby case of 2-space on 1st indent, 6-space >> on 2nd indent) >> >>> >>> + /* Get the command tag. */ >>> + tag = parsetree ? CreateCommandTag(parsetree) : CMDTAG_CONNECT; >>> This is hardcoding knowledge that currently only this trigger wont have a >>> parsetree, and setting the tag accordingly. This should instead check the >>> event and set CMDTAG_UNKNOWN if it isn't the expected one. >>> >> >> I updated that, but maybe not exactly how you expected? >> >>> >>> + /* database has on-login triggers */ >>> + bool dathaslogontriggers; >>> This patch uses three different names for the same thing: client connection, >>> logontrigger and login trigger. Since this trigger only fires after >>> successful >>> authentication it’s not accurate to name it client connection, as that >>> implies >>> it running on connections rather than logins. The nomenclature used in the >>> tree is "login" so the patch should be adjusted everywhere to use that. >>> >> >> Fixed. >> >>> >>> +/* Hook for plugins to get control at start of session */ >>> +client_connection_hook_type client_connection_hook = EventTriggerOnConnect; >>> I don't see the reason for adding core functionality by hooks. Having a hook >>> might be a useful thing on its own (to be discussed in a new thread, not >>> hidden >>> here), but core functionality should not depend on it IMO. >>> >> >> Fair enough, I removed the hook. >> >>> >>> + {"enable_client_connection_trigger", PGC_SU_BACKEND, DEVELOPER_OPTIONS, >>> + gettext_noop("Enables the client_connection event trigger."), >>> + gettext_noop("In case of errors in the ON client_connection EVENT TRIGGER >>> procedure, " >>> ..and.. >>> + /* >>> + * Try to ignore error for superuser to make it possible to login even in >>> case of errors >>> + * during trigger execution >>> + */ >>> + if (!is_superuser) >>> + PG_RE_THROW(); >>> This patch adds two ways for superusers to bypass this event trigger in >>> case of >>> it being faulty, but for every other event trigger we've documented to >>> restart >>> in single-user mode and fixing it there. Why does this need to be different? >>> This clearly has a bigger chance of being a footgun but I don't see that as >>> a >>> reason to add a GUC and a bypass that other footguns lack. >>> >> >> OK, I removed those bypasses. We'll see what others think. >> >>> >>> + elog(NOTICE, "client_connection trigger failed with message: %s", >>> error->message); >>> Calling elog() in a PG_CATCH() block isn't allowed is it? >>> >> >> I believe it is allowed (e.g. there's a case in libpq), but I removed >> this anyway as part of the superuser bypass. >> >>> >>> + /* Runtlist is empty: clear dathaslogontriggers flag >>> + */ >>> s/Runtlist/Runlist/ and also commenting style. >>> >> >> Fixed. >> >>> >>> The logic for resetting the pg_database flag when firing event trigger in >>> case >>> the trigger has gone away is messy and a problem waiting to happen IMO. For >>> normal triggers we don't bother with that on the grounds of it being racy, >>> and >>> instead perform relhastrigger removal during VACUUM. Another approach is >>> using >>> a counter as propose upthread, since updating that can be made safer. The >>> current coding also isn't instructing concurrent backends to perform >>> relcache >>> rebuild. >>> >> >> I think there are pros and cons of each possible approach, but I think >> I prefer the current way (which I have tweaked a bit) for similar >> reasons to those explained by the original patch author when debating >> whether to use reference-counting instead, in the discussion upthread >> (e.g. it keeps it all in one place). Also, it seems to be more inline >> with the documented reason why that pg_database flag was added in the >> first place. I have debugged a few concurrent scenarios with the >> current mechanism in place. If you still dislike the logic for >> resetting the flag, please elaborate on the issues you foresee and one >> of the alternative approaches can be tried. >> >> I've attached the updated patch. >> >> Regards, >> Greg Nancarrow >> Fujitsu Australia >> >-- >Teodor Sigaev E-mail: teo...@sigaev.ru > WWW: http://www.sigaev.ru/
diff --git a/doc/src/sgml/bki.sgml b/doc/src/sgml/bki.sgml index f900a01b82..c312165f71 100644 --- a/doc/src/sgml/bki.sgml +++ b/doc/src/sgml/bki.sgml @@ -182,7 +182,7 @@ { oid => '1', oid_symbol => 'TemplateDbOid', descr => 'database\'s default template', datname => 'template1', encoding => 'ENCODING', datcollate => 'LC_COLLATE', - datctype => 'LC_CTYPE', datistemplate => 't', datallowconn => 't', + datctype => 'LC_CTYPE', datistemplate => 't', datallowconn => 't', dathaslogintriggers => 'f', datconnlimit => '-1', datlastsysoid => '0', datfrozenxid => '0', datminmxid => '1', dattablespace => 'pg_default', datacl => '_null_' }, diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml index c1d11be73f..a2d75687c9 100644 --- a/doc/src/sgml/catalogs.sgml +++ b/doc/src/sgml/catalogs.sgml @@ -2979,6 +2979,17 @@ SCRAM-SHA-256$<replaceable><iteration count></replaceable>:<replaceable>&l </para></entry> </row> + <row> + <entry role="catalog_table_entry"><para role="column_definition"> + <structfield>dathaslogintriggers</structfield> <type>bool</type> + </para> + <para> + Indicates that there are login triggers defined for this database. + This flag is used to avoid extra lookups on the pg_event_trigger table during each backend startup. + This flag is used internally by Postgres and should not be manually changed by DBA or application. + </para></entry> + </row> + <row> <entry role="catalog_table_entry"><para role="column_definition"> <structfield>datconnlimit</structfield> <type>int4</type> diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index de77f14573..4584c077a9 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -1035,6 +1035,22 @@ include_dir 'conf.d' </listitem> </varlistentry> + <varlistentry id="guc-login-triggers" xreflabel="login_triggers"> + <term><varname>login_triggers</varname> (<type>boolean</type>) + <indexterm> + <primary><varname>login_triggers</varname> configuration parameter</primary> + </indexterm> + </term> + + <listitem> + <para> + Enables the <xref linkend="event-trigger-login"/>. The default value is <literal>on</literal>. + </para> + <para> + This parameter can only be set by superusers. + </para> + </listitem> + </varlistentry> </variablelist> </sect2> diff --git a/doc/src/sgml/ecpg.sgml b/doc/src/sgml/ecpg.sgml index 04d4a6bdb2..565f88d298 100644 --- a/doc/src/sgml/ecpg.sgml +++ b/doc/src/sgml/ecpg.sgml @@ -4731,6 +4731,7 @@ datdba = 10 (type: 1) encoding = 0 (type: 5) datistemplate = t (type: 1) datallowconn = t (type: 1) +dathaslogintriggers = f (type: 1) datconnlimit = -1 (type: 5) datlastsysoid = 11510 (type: 1) datfrozenxid = 379 (type: 1) @@ -4756,6 +4757,7 @@ datdba = 10 (type: 1) encoding = 0 (type: 5) datistemplate = f (type: 1) datallowconn = t (type: 1) +dathaslogintriggers = f (type: 1) datconnlimit = -1 (type: 5) datlastsysoid = 11510 (type: 1) datfrozenxid = 379 (type: 1) diff --git a/doc/src/sgml/event-trigger.sgml b/doc/src/sgml/event-trigger.sgml index 60366a950e..2ff7ce75c3 100644 --- a/doc/src/sgml/event-trigger.sgml +++ b/doc/src/sgml/event-trigger.sgml @@ -28,6 +28,7 @@ An event trigger fires whenever the event with which it is associated occurs in the database in which it is defined. Currently, the only supported events are + <literal>login</literal>, <literal>ddl_command_start</literal>, <literal>ddl_command_end</literal>, <literal>table_rewrite</literal> @@ -35,6 +36,19 @@ Support for additional events may be added in future releases. </para> + <para id="event-trigger-login" xreflabel="login event triggers"> + The <literal>login</literal> event occurs when a user logs in to the + system. + If the trigger fails, user will not log in. The side effect of such behaviour is that a bug in a trigger procedure can prevent any user from logging to the system and bricking it. + If this happens, access can be restored by connecting to the system in + single-user mode (as event triggers are disabled in this mode) and fixing the buggy trigger + or by disabling login triggers by the <xref linkend="guc-login-triggers"/> GUC variable. + See the <xref linkend="app-postgres"/> reference page for details about + using single-user mode. + Such cases can be avoided by careful design of trigger procedures and related permissions. + See also <xref linkend="event-trigger-database-login-example"/>. + </para> + <para> The <literal>ddl_command_start</literal> event occurs just before the execution of a <literal>CREATE</literal>, <literal>ALTER</literal>, <literal>DROP</literal>, @@ -1140,7 +1154,7 @@ typedef struct EventTriggerData </sect1> <sect1 id="event-trigger-example"> - <title>A Complete Event Trigger Example</title> + <title>A C language Event Trigger Example</title> <para> Here is a very simple example of an event trigger function written in C. @@ -1280,6 +1294,63 @@ $$; CREATE EVENT TRIGGER no_rewrite_allowed ON table_rewrite EXECUTE FUNCTION no_rewrite(); +</programlisting> + </para> + </sect1> + + <sect1 id="event-trigger-database-login-example" xreflabel="a login trigger example"> + <title>A Database Login Event Trigger Example</title> + + <para> + The event trigger on the <literal>login</literal> event can be + useful for logging user logins, for verifying the connection and + assigning roles according to current circumstances, or for some session data + initialization. + </para> + + <para> + The following example demonstrates these options. +<programlisting> +-- create test tables and roles +CREATE TABLE user_login_log ( + "user" text, + "session_start" timestamp with time zone +); +CREATE ROLE day_worker; +CREATE ROLE night_worker; + +-- the example trigger function +CREATE OR REPLACE FUNCTION init_session() + RETURNS event_trigger SECURITY DEFINER + LANGUAGE plpgsql AS +$$ +DECLARE + hour integer = EXTRACT('hour' FROM current_time); +BEGIN +-- 1) Assign some roles +IF hour BETWEEN 8 AND 20 THEN -- at daytime grant the day_worker role + EXECUTE 'REVOKE night_worker FROM ' || quote_ident(session_user); + EXECUTE 'GRANT day_worker TO ' || quote_ident(session_user); +ELSIF hour BETWEEN 2 AND 4 THEN + RAISE EXCEPTION 'Login forbidden'; -- do not allow to login these hours +ELSE -- at other time grant the night_worker role + EXECUTE 'REVOKE day_worker FROM ' || quote_ident(session_user); + EXECUTE 'GRANT night_worker TO ' || quote_ident(session_user); +END IF; + +-- 2) Initialize some user session data +CREATE TEMP TABLE session_storage (x float, y integer); + +-- 3) Log the connection time +INSERT INTO user_login_log VALUES (session_user, current_timestamp); + +END; +$$; + +-- trigger definition +CREATE EVENT TRIGGER init_session + ON login + EXECUTE FUNCTION init_session(); </programlisting> </para> </sect1> diff --git a/src/backend/commands/dbcommands.c b/src/backend/commands/dbcommands.c index 029fab48df..a658d476ca 100644 --- a/src/backend/commands/dbcommands.c +++ b/src/backend/commands/dbcommands.c @@ -530,6 +530,7 @@ createdb(ParseState *pstate, const CreatedbStmt *stmt) new_record[Anum_pg_database_datctype - 1] = DirectFunctionCall1(namein, CStringGetDatum(dbctype)); new_record[Anum_pg_database_datistemplate - 1] = BoolGetDatum(dbistemplate); + new_record[Anum_pg_database_dathaslogintriggers - 1] = BoolGetDatum(false); new_record[Anum_pg_database_datallowconn - 1] = BoolGetDatum(dballowconnections); new_record[Anum_pg_database_datconnlimit - 1] = Int32GetDatum(dbconnlimit); new_record[Anum_pg_database_datlastsysoid - 1] = ObjectIdGetDatum(src_lastsysoid); @@ -1585,7 +1586,7 @@ AlterDatabase(ParseState *pstate, AlterDatabaseStmt *stmt, bool isTopLevel) new_record[Anum_pg_database_datconnlimit - 1] = Int32GetDatum(dbconnlimit); new_record_repl[Anum_pg_database_datconnlimit - 1] = true; } - + new_record[Anum_pg_database_dathaslogintriggers - 1] = BoolGetDatum(datform->dathaslogintriggers); newtuple = heap_modify_tuple(tuple, RelationGetDescr(rel), new_record, new_record_nulls, new_record_repl); CatalogTupleUpdate(rel, &tuple->t_self, newtuple); diff --git a/src/backend/commands/event_trigger.c b/src/backend/commands/event_trigger.c index df264329d8..34a95c7513 100644 --- a/src/backend/commands/event_trigger.c +++ b/src/backend/commands/event_trigger.c @@ -20,6 +20,7 @@ #include "catalog/dependency.h" #include "catalog/indexing.h" #include "catalog/objectaccess.h" +#include "catalog/pg_database.h" #include "catalog/pg_event_trigger.h" #include "catalog/pg_namespace.h" #include "catalog/pg_opclass.h" @@ -43,11 +44,15 @@ #include "utils/builtins.h" #include "utils/evtcache.h" #include "utils/fmgroids.h" +#include "utils/inval.h" #include "utils/lsyscache.h" #include "utils/memutils.h" #include "utils/rel.h" +#include "utils/snapmgr.h" #include "utils/syscache.h" +bool login_triggers_enabled = false; + typedef struct EventTriggerQueryState { /* memory context for this state's objects */ @@ -101,6 +106,8 @@ static void EventTriggerInvoke(List *fn_oid_list, EventTriggerData *trigdata); static const char *stringify_grant_objtype(ObjectType objtype); static const char *stringify_adefprivs_objtype(ObjectType objtype); + + /* * Create an event trigger. */ @@ -130,6 +137,7 @@ CreateEventTrigger(CreateEventTrigStmt *stmt) if (strcmp(stmt->eventname, "ddl_command_start") != 0 && strcmp(stmt->eventname, "ddl_command_end") != 0 && strcmp(stmt->eventname, "sql_drop") != 0 && + strcmp(stmt->eventname, "login") != 0 && strcmp(stmt->eventname, "table_rewrite") != 0) ereport(ERROR, (errcode(ERRCODE_SYNTAX_ERROR), @@ -293,6 +301,26 @@ insert_event_trigger_tuple(const char *trigname, const char *eventname, Oid evtO CatalogTupleInsert(tgrel, tuple); heap_freetuple(tuple); + if (strcmp(eventname, "login") == 0) + { + Form_pg_database db; + Relation pg_db = table_open(DatabaseRelationId, RowExclusiveLock); + /* Set dathaslogintriggers flag in pg_database */ + tuple = SearchSysCacheCopy1(DATABASEOID, ObjectIdGetDatum(MyDatabaseId)); + if (!HeapTupleIsValid(tuple)) + elog(ERROR, "cache lookup failed for database %u", MyDatabaseId); + db = (Form_pg_database) GETSTRUCT(tuple); + if (!db->dathaslogintriggers) + { + db->dathaslogintriggers = true; + CatalogTupleUpdate(pg_db, &tuple->t_self, tuple); + } + else + CacheInvalidateRelcacheByTuple(tuple); + table_close(pg_db, RowExclusiveLock); + heap_freetuple(tuple); + } + /* Depend on owner. */ recordDependencyOnOwner(EventTriggerRelationId, trigoid, evtOwner); @@ -562,6 +590,9 @@ EventTriggerCommonSetup(Node *parsetree, ListCell *lc; List *runlist = NIL; + /* Get the command tag. */ + tag = (event == EVT_Login) ? CMDTAG_LOGIN : CreateCommandTag(parsetree); + /* * We want the list of command tags for which this procedure is actually * invoked to match up exactly with the list that CREATE EVENT TRIGGER @@ -577,22 +608,18 @@ EventTriggerCommonSetup(Node *parsetree, * relevant command tag. */ #ifdef USE_ASSERT_CHECKING + if (event == EVT_DDLCommandStart || + event == EVT_DDLCommandEnd || + event == EVT_SQLDrop || + event == EVT_Login) { - CommandTag dbgtag; - - dbgtag = CreateCommandTag(parsetree); - if (event == EVT_DDLCommandStart || - event == EVT_DDLCommandEnd || - event == EVT_SQLDrop) - { - if (!command_tag_event_trigger_ok(dbgtag)) - elog(ERROR, "unexpected command tag \"%s\"", GetCommandTagName(dbgtag)); - } - else if (event == EVT_TableRewrite) - { - if (!command_tag_table_rewrite_ok(dbgtag)) - elog(ERROR, "unexpected command tag \"%s\"", GetCommandTagName(dbgtag)); - } + if (!command_tag_event_trigger_ok(tag)) + elog(ERROR, "unexpected command tag \"%s\"", GetCommandTagName(tag)); + } + else if (event == EVT_TableRewrite) + { + if (!command_tag_table_rewrite_ok(tag)) + elog(ERROR, "unexpected command tag \"%s\"", GetCommandTagName(tag)); } #endif @@ -601,9 +628,6 @@ EventTriggerCommonSetup(Node *parsetree, if (cachelist == NIL) return NIL; - /* Get the command tag. */ - tag = CreateCommandTag(parsetree); - /* * Filter list of event triggers by command tag, and copy them into our * memory context. Once we start running the command triggers, or indeed @@ -800,6 +824,111 @@ EventTriggerSQLDrop(Node *parsetree) list_free(runlist); } +/* + * Return true if this database has login triggers, false otherwise. + */ +static bool +DatabaseHasLoginTriggers(void) +{ + bool has_login_triggers; + HeapTuple tuple = SearchSysCache1(DATABASEOID, ObjectIdGetDatum(MyDatabaseId)); + + if (!HeapTupleIsValid(tuple)) + elog(ERROR, "cache lookup failed for database %u", MyDatabaseId); + + has_login_triggers = ((Form_pg_database) GETSTRUCT(tuple))->dathaslogintriggers; + ReleaseSysCache(tuple); + return has_login_triggers; +} + +/* + * Fire login triggers. + */ +void +EventTriggerOnLogin(void) +{ + List *runlist; + EventTriggerData trigdata; + + /* + * See EventTriggerDDLCommandStart for a discussion about why event + * triggers are disabled in single user mode. + */ + if (!IsUnderPostmaster || !OidIsValid(MyDatabaseId)) + return; + + StartTransactionCommand(); + + if (DatabaseHasLoginTriggers()) + { + runlist = EventTriggerCommonSetup(NULL, + EVT_Login, "login", + &trigdata); + + if (runlist != NIL) + { + if(login_triggers_enabled) { + /* + * Make sure anything the main command did will be visible to the event + * triggers. + */ + CommandCounterIncrement(); + + /* + * Event trigger execution may require an active snapshot. + */ + PushActiveSnapshot(GetTransactionSnapshot()); + + /* Run the triggers. */ + EventTriggerInvoke(runlist, &trigdata); + + /* Cleanup. */ + list_free(runlist); + + PopActiveSnapshot(); + } + } + else + { + /* + * Runlist is empty: clear dathaslogintriggers flag + */ + Relation pg_db = table_open(DatabaseRelationId, RowExclusiveLock); + HeapTuple tuple = SearchSysCacheCopy1(DATABASEOID, ObjectIdGetDatum(MyDatabaseId)); + Form_pg_database db; + + if (!HeapTupleIsValid(tuple)) + elog(ERROR, "cache lookup failed for database %u", MyDatabaseId); + + db = (Form_pg_database) GETSTRUCT(tuple); + if (db->dathaslogintriggers) + { + /* + * There can be a race condition: a login event trigger may have + * been added after the pg_event_trigger table was scanned, and + * we don't want to erroneously clear the dathaslogintriggers + * flag in this case. To be sure that this hasn't happened, + * repeat the scan under the pg_database table lock. + */ + AcceptInvalidationMessages(); + runlist = EventTriggerCommonSetup(NULL, + EVT_Login, "login", + &trigdata); + if (runlist == NULL) /* list is still empty, so clear the flag */ + { + db->dathaslogintriggers = false; + CatalogTupleUpdate(pg_db, &tuple->t_self, tuple); + } + else + CacheInvalidateRelcacheByTuple(tuple); + } + table_close(pg_db, RowExclusiveLock); + heap_freetuple(tuple); + } + } + CommitTransactionCommand(); +} + /* * Fire table_rewrite triggers. diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c index 0775abe35d..273611caf9 100644 --- a/src/backend/tcop/postgres.c +++ b/src/backend/tcop/postgres.c @@ -42,6 +42,7 @@ #include "catalog/pg_type.h" #include "commands/async.h" #include "commands/prepare.h" +#include "commands/event_trigger.h" #include "executor/spi.h" #include "jit/jit.h" #include "libpq/libpq.h" @@ -4178,6 +4179,9 @@ PostgresMain(const char *dbname, const char *username) initStringInfo(&row_description_buf); MemoryContextSwitchTo(TopMemoryContext); + /* Fire any defined login triggers, if appropriate */ + EventTriggerOnLogin(); + /* * POSTGRES main processing loop begins here * diff --git a/src/backend/utils/cache/evtcache.c b/src/backend/utils/cache/evtcache.c index 460b720a65..b9b935f1cc 100644 --- a/src/backend/utils/cache/evtcache.c +++ b/src/backend/utils/cache/evtcache.c @@ -167,6 +167,8 @@ BuildEventTriggerCache(void) event = EVT_SQLDrop; else if (strcmp(evtevent, "table_rewrite") == 0) event = EVT_TableRewrite; + else if (strcmp(evtevent, "login") == 0) + event = EVT_Login; else continue; diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c index e91d5a3cfd..a41c3cd7d5 100644 --- a/src/backend/utils/misc/guc.c +++ b/src/backend/utils/misc/guc.c @@ -129,7 +129,7 @@ */ #define REALTYPE_PRECISION 17 -/* XXX these should appear in other modules' header files */ +/* XXX these should appear in other modules or their header files */ extern bool Log_disconnections; extern int CommitDelay; extern int CommitSiblings; @@ -138,6 +138,7 @@ extern char *temp_tablespaces; extern bool ignore_checksum_failure; extern bool ignore_invalid_pages; extern bool synchronize_seqscans; +extern bool login_triggers_enabled; #ifdef TRACE_SYNCSCAN extern bool trace_syncscan; @@ -2119,6 +2120,15 @@ static struct config_bool ConfigureNamesBool[] = NULL, NULL, NULL }, + { + {"login_triggers", PGC_SIGHUP, CLIENT_CONN_OTHER, + gettext_noop("Enables login triggers."), + }, + &login_triggers_enabled, + true, + NULL, NULL, NULL + }, + /* End-of-list marker */ { {NULL, 0, 0, NULL, NULL}, NULL, false, NULL, NULL, NULL diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample index 1cbc9feeb6..28b8992cfc 100644 --- a/src/backend/utils/misc/postgresql.conf.sample +++ b/src/backend/utils/misc/postgresql.conf.sample @@ -89,6 +89,7 @@ #client_connection_check_interval = 0 # time between checks for client # disconnection while running queries; # 0 for never +#login_triggers = on # - Authentication - diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c index b9635a95b6..bd7a21699a 100644 --- a/src/bin/pg_dump/pg_dump.c +++ b/src/bin/pg_dump/pg_dump.c @@ -2798,6 +2798,7 @@ dumpDatabase(Archive *fout) i_datacl, i_rdatacl, i_datistemplate, + i_dathaslogintriggers, i_datconnlimit, i_tablespace; CatalogId dbCatId; @@ -2810,6 +2811,7 @@ dumpDatabase(Archive *fout) *datacl, *rdatacl, *datistemplate, + *dathaslogintriggers, *datconnlimit, *tablespace; uint32 frozenxid, @@ -2828,7 +2830,7 @@ dumpDatabase(Archive *fout) * (pg_init_privs) are not supported on databases, so this logic cannot * make use of buildACLQueries(). */ - if (fout->remoteVersion >= 90600) + if (fout->remoteVersion >= 150000) { appendPQExpBuffer(dbQry, "SELECT tableoid, oid, datname, " "(%s datdba) AS dba, " @@ -2854,7 +2856,41 @@ dumpDatabase(Archive *fout) " AS permp(orig_acl) " " WHERE acl = orig_acl)) AS rdatacls) " " AS rdatacl, " - "datistemplate, datconnlimit, " + "datistemplate, datconnlimit, dathaslogintriggers, " + "(SELECT spcname FROM pg_tablespace t WHERE t.oid = dattablespace) AS tablespace, " + "shobj_description(oid, 'pg_database') AS description " + + "FROM pg_database " + "WHERE datname = current_database()", + username_subquery); + } + else if (fout->remoteVersion >= 90600) + { + appendPQExpBuffer(dbQry, "SELECT tableoid, oid, datname, " + "(%s datdba) AS dba, " + "pg_encoding_to_char(encoding) AS encoding, " + "datcollate, datctype, datfrozenxid, datminmxid, " + "(SELECT array_agg(acl ORDER BY row_n) FROM " + " (SELECT acl, row_n FROM " + " unnest(coalesce(datacl,acldefault('d',datdba))) " + " WITH ORDINALITY AS perm(acl,row_n) " + " WHERE NOT EXISTS ( " + " SELECT 1 " + " FROM unnest(acldefault('d',datdba)) " + " AS init(init_acl) " + " WHERE acl = init_acl)) AS datacls) " + " AS datacl, " + "(SELECT array_agg(acl ORDER BY row_n) FROM " + " (SELECT acl, row_n FROM " + " unnest(acldefault('d',datdba)) " + " WITH ORDINALITY AS initp(acl,row_n) " + " WHERE NOT EXISTS ( " + " SELECT 1 " + " FROM unnest(coalesce(datacl,acldefault('d',datdba))) " + " AS permp(orig_acl) " + " WHERE acl = orig_acl)) AS rdatacls) " + " AS rdatacl, " + "datistemplate, datconnlimit, false as dathaslogintriggers, " "(SELECT spcname FROM pg_tablespace t WHERE t.oid = dattablespace) AS tablespace, " "shobj_description(oid, 'pg_database') AS description " @@ -2868,7 +2904,7 @@ dumpDatabase(Archive *fout) "(%s datdba) AS dba, " "pg_encoding_to_char(encoding) AS encoding, " "datcollate, datctype, datfrozenxid, datminmxid, " - "datacl, '' as rdatacl, datistemplate, datconnlimit, " + "datacl, '' as rdatacl, datistemplate, datconnlimit, false as dathaslogintriggers" "(SELECT spcname FROM pg_tablespace t WHERE t.oid = dattablespace) AS tablespace, " "shobj_description(oid, 'pg_database') AS description " @@ -2882,7 +2918,7 @@ dumpDatabase(Archive *fout) "(%s datdba) AS dba, " "pg_encoding_to_char(encoding) AS encoding, " "datcollate, datctype, datfrozenxid, 0 AS datminmxid, " - "datacl, '' as rdatacl, datistemplate, datconnlimit, " + "datacl, '' as rdatacl, datistemplate, datconnlimit, false as dathaslogintriggers, " "(SELECT spcname FROM pg_tablespace t WHERE t.oid = dattablespace) AS tablespace, " "shobj_description(oid, 'pg_database') AS description " @@ -2896,7 +2932,7 @@ dumpDatabase(Archive *fout) "(%s datdba) AS dba, " "pg_encoding_to_char(encoding) AS encoding, " "NULL AS datcollate, NULL AS datctype, datfrozenxid, 0 AS datminmxid, " - "datacl, '' as rdatacl, datistemplate, datconnlimit, " + "datacl, '' as rdatacl, datistemplate, datconnlimit, false as dathaslogintriggers, " "(SELECT spcname FROM pg_tablespace t WHERE t.oid = dattablespace) AS tablespace, " "shobj_description(oid, 'pg_database') AS description " @@ -2911,7 +2947,7 @@ dumpDatabase(Archive *fout) "pg_encoding_to_char(encoding) AS encoding, " "NULL AS datcollate, NULL AS datctype, datfrozenxid, 0 AS datminmxid, " "datacl, '' as rdatacl, datistemplate, " - "-1 as datconnlimit, " + "-1 as datconnlimit, false as dathaslogintriggers, " "(SELECT spcname FROM pg_tablespace t WHERE t.oid = dattablespace) AS tablespace " "FROM pg_database " "WHERE datname = current_database()", @@ -2933,6 +2969,7 @@ dumpDatabase(Archive *fout) i_rdatacl = PQfnumber(res, "rdatacl"); i_datistemplate = PQfnumber(res, "datistemplate"); i_datconnlimit = PQfnumber(res, "datconnlimit"); + i_dathaslogintriggers = PQfnumber(res, "dathaslogintriggers"); i_tablespace = PQfnumber(res, "tablespace"); dbCatId.tableoid = atooid(PQgetvalue(res, 0, i_tableoid)); @@ -2947,6 +2984,7 @@ dumpDatabase(Archive *fout) datacl = PQgetvalue(res, 0, i_datacl); rdatacl = PQgetvalue(res, 0, i_rdatacl); datistemplate = PQgetvalue(res, 0, i_datistemplate); + dathaslogintriggers = PQgetvalue(res, 0, i_dathaslogintriggers); datconnlimit = PQgetvalue(res, 0, i_datconnlimit); tablespace = PQgetvalue(res, 0, i_tablespace); @@ -3120,6 +3158,8 @@ dumpDatabase(Archive *fout) appendPQExpBufferStr(delQry, ";\n"); } + /* We do not restore the pg_database.dathaslogintriggers because it is set automatically on trigger creation */ + /* Add database-specific SET options */ dumpDatabaseConfig(fout, creaQry, datname, dbCatId.oid); diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c index 8e01f54500..17e1a058a1 100644 --- a/src/bin/psql/tab-complete.c +++ b/src/bin/psql/tab-complete.c @@ -3099,7 +3099,8 @@ psql_completion(const char *text, int start, int end) COMPLETE_WITH("ON"); /* Complete CREATE EVENT TRIGGER <name> ON with event_type */ else if (Matches("CREATE", "EVENT", "TRIGGER", MatchAny, "ON")) - COMPLETE_WITH("ddl_command_start", "ddl_command_end", "sql_drop"); + COMPLETE_WITH("ddl_command_start", "ddl_command_end", + "login", "sql_drop"); /* * Complete CREATE EVENT TRIGGER <name> ON <event_type>. EXECUTE FUNCTION diff --git a/src/include/catalog/pg_database.dat b/src/include/catalog/pg_database.dat index b8aa1364a0..2b804a47c3 100644 --- a/src/include/catalog/pg_database.dat +++ b/src/include/catalog/pg_database.dat @@ -15,7 +15,7 @@ { oid => '1', oid_symbol => 'TemplateDbOid', descr => 'default template for new databases', datname => 'template1', encoding => 'ENCODING', datcollate => 'LC_COLLATE', - datctype => 'LC_CTYPE', datistemplate => 't', datallowconn => 't', + datctype => 'LC_CTYPE', datistemplate => 't', dathaslogintriggers => 'f', datallowconn => 't', datconnlimit => '-1', datlastsysoid => '0', datfrozenxid => '0', datminmxid => '1', dattablespace => 'pg_default', datacl => '_null_' }, diff --git a/src/include/catalog/pg_database.h b/src/include/catalog/pg_database.h index 43f3beb6a3..a6e0309bb3 100644 --- a/src/include/catalog/pg_database.h +++ b/src/include/catalog/pg_database.h @@ -52,6 +52,9 @@ CATALOG(pg_database,1262,DatabaseRelationId) BKI_SHARED_RELATION BKI_ROWTYPE_OID /* new connections allowed? */ bool datallowconn; + /* database has login triggers? */ + bool dathaslogintriggers; + /* max connections allowed (-1=no limit) */ int32 datconnlimit; diff --git a/src/include/commands/event_trigger.h b/src/include/commands/event_trigger.h index e765e67fd1..a9f9954597 100644 --- a/src/include/commands/event_trigger.h +++ b/src/include/commands/event_trigger.h @@ -54,6 +54,7 @@ extern void EventTriggerDDLCommandStart(Node *parsetree); extern void EventTriggerDDLCommandEnd(Node *parsetree); extern void EventTriggerSQLDrop(Node *parsetree); extern void EventTriggerTableRewrite(Node *parsetree, Oid tableOid, int reason); +extern void EventTriggerOnLogin(void); extern bool EventTriggerBeginCompleteQuery(void); extern void EventTriggerEndCompleteQuery(void); diff --git a/src/include/tcop/cmdtaglist.h b/src/include/tcop/cmdtaglist.h index 9ba24d4ca9..b0e7df3d81 100644 --- a/src/include/tcop/cmdtaglist.h +++ b/src/include/tcop/cmdtaglist.h @@ -186,6 +186,7 @@ PG_CMDTAG(CMDTAG_INSERT, "INSERT", false, false, true) PG_CMDTAG(CMDTAG_LISTEN, "LISTEN", false, false, false) PG_CMDTAG(CMDTAG_LOAD, "LOAD", false, false, false) PG_CMDTAG(CMDTAG_LOCK_TABLE, "LOCK TABLE", false, false, false) +PG_CMDTAG(CMDTAG_LOGIN, "LOGIN", true, false, false) PG_CMDTAG(CMDTAG_MOVE, "MOVE", false, false, true) PG_CMDTAG(CMDTAG_NOTIFY, "NOTIFY", false, false, false) PG_CMDTAG(CMDTAG_PREPARE, "PREPARE", false, false, false) diff --git a/src/include/utils/evtcache.h b/src/include/utils/evtcache.h index 58ddb71cb1..29d73afe3d 100644 --- a/src/include/utils/evtcache.h +++ b/src/include/utils/evtcache.h @@ -22,7 +22,8 @@ typedef enum EVT_DDLCommandStart, EVT_DDLCommandEnd, EVT_SQLDrop, - EVT_TableRewrite + EVT_TableRewrite, + EVT_Login, } EventTriggerEvent; typedef struct diff --git a/src/test/recovery/t/001_stream_rep.pl b/src/test/recovery/t/001_stream_rep.pl index c70c08e27b..59fa30a3ae 100644 --- a/src/test/recovery/t/001_stream_rep.pl +++ b/src/test/recovery/t/001_stream_rep.pl @@ -46,6 +46,26 @@ $node_standby_2->start; $node_primary->safe_psql('postgres', "CREATE TABLE tab_int AS SELECT generate_series(1,1002) AS a"); +$node_primary->safe_psql('postgres', q{ +CREATE ROLE regress_user LOGIN PASSWORD 'pass'; + +CREATE TABLE user_logins(id serial, who text); + +CREATE FUNCTION on_login_proc() RETURNS EVENT_TRIGGER AS $$ +BEGIN + IF NOT pg_is_in_recovery() THEN + INSERT INTO user_logins (who) VALUES (session_user); + END IF; + IF session_user = 'regress_hacker' THEN + RAISE EXCEPTION 'You are not welcome!'; + END IF; +END; +$$ LANGUAGE plpgsql SECURITY DEFINER; + +CREATE EVENT TRIGGER on_login_trigger ON login EXECUTE FUNCTION on_login_proc(); +ALTER EVENT TRIGGER on_login_trigger ENABLE ALWAYS; +}); + # Wait for standbys to catch up $node_primary->wait_for_catchup($node_standby_1, 'replay', $node_primary->lsn('insert')); @@ -371,6 +391,9 @@ sub replay_check replay_check(); +$node_standby_1->safe_psql('postgres', "SELECT 1", extra_params => [ '-U', 'regress_user', '-w' ]); +$node_standby_2->safe_psql('postgres', "SELECT 2", extra_params => [ '-U', 'regress_user', '-w' ]); + note "enabling hot_standby_feedback"; # Enable hs_feedback. The slot should gain an xmin. We set the status interval diff --git a/src/test/regress/expected/event_trigger.out b/src/test/regress/expected/event_trigger.out index 44d545de25..af3564d532 100644 --- a/src/test/regress/expected/event_trigger.out +++ b/src/test/regress/expected/event_trigger.out @@ -604,3 +604,84 @@ SELECT DROP EVENT TRIGGER start_rls_command; DROP EVENT TRIGGER end_rls_command; DROP EVENT TRIGGER sql_drop_command; +-- On login triggers +CREATE TABLE user_logins(id serial, who text); +GRANT SELECT ON user_logins TO public; +CREATE USER login_tester; +CREATE FUNCTION on_login_proc() RETURNS event_trigger SECURITY DEFINER AS $$ +BEGIN + INSERT INTO user_logins (who) VALUES (SESSION_USER); + RAISE NOTICE 'You are welcome!'; +END; +$$ LANGUAGE plpgsql; +CREATE EVENT TRIGGER on_login_trigger ON LOGIN EXECUTE PROCEDURE on_login_proc(); +ALTER EVENT TRIGGER on_login_trigger ENABLE ALWAYS; +\c +NOTICE: You are welcome! +SELECT count(*) FROM user_logins; + count +------- + 1 +(1 row) + +\set superuser :USER +\c - login_tester +NOTICE: You are welcome! +SELECT count(*) FROM user_logins; + count +------- + 2 +(1 row) + +CREATE EVENT TRIGGER on_login_trigger ON LOGIN EXECUTE PROCEDURE on_login_proc(); -- non-superuser should not be able to! +ERROR: permission denied to create event trigger "on_login_trigger" +HINT: Must be superuser to create an event trigger. +\c - :superuser +NOTICE: You are welcome! +SELECT id, CASE WHEN who = :'superuser' THEN 'superuser' ELSE who END AS who FROM user_logins; + id | who +----+-------------- + 1 | superuser + 2 | login_tester + 3 | superuser +(3 rows) + +-- Turn off login triggers and make an erroneous one. We check that it will not be called. +ALTER SYSTEM SET login_triggers = 'off'; +CREATE FUNCTION on_login_fail() RETURNS event_trigger SECURITY DEFINER AS $$ +BEGIN + RAISE EXCEPTION 'login trigger failure'; +END; +$$ LANGUAGE plpgsql; +CREATE EVENT TRIGGER on_login_fail ON LOGIN EXECUTE PROCEDURE on_login_fail(); +ALTER EVENT TRIGGER on_login_fail ENABLE ALWAYS; +SELECT pg_reload_conf(); + pg_reload_conf +---------------- + t +(1 row) + +\c +SELECT id, CASE WHEN who = :'superuser' THEN 'superuser' ELSE who END AS who FROM user_logins; + id | who +----+-------------- + 1 | superuser + 2 | login_tester + 3 | superuser +(3 rows) + +-- check dathaslogintriggers in system catalog +SELECT dathaslogintriggers FROM pg_database WHERE datname= :'DBNAME'; + dathaslogintriggers +--------------------- + t +(1 row) + +-- Cleanup +DROP TABLE user_logins; +DROP EVENT TRIGGER on_login_trigger; +DROP EVENT TRIGGER on_login_fail; +DROP FUNCTION on_login_proc(); +DROP FUNCTION on_login_fail(); +DROP USER login_tester; +-- End diff --git a/src/test/regress/sql/event_trigger.sql b/src/test/regress/sql/event_trigger.sql index 1446cf8cc8..b54b4d33ce 100644 --- a/src/test/regress/sql/event_trigger.sql +++ b/src/test/regress/sql/event_trigger.sql @@ -462,3 +462,48 @@ SELECT DROP EVENT TRIGGER start_rls_command; DROP EVENT TRIGGER end_rls_command; DROP EVENT TRIGGER sql_drop_command; + +-- On login triggers +CREATE TABLE user_logins(id serial, who text); +GRANT SELECT ON user_logins TO public; +CREATE USER login_tester; +CREATE FUNCTION on_login_proc() RETURNS event_trigger SECURITY DEFINER AS $$ +BEGIN + INSERT INTO user_logins (who) VALUES (SESSION_USER); + RAISE NOTICE 'You are welcome!'; +END; +$$ LANGUAGE plpgsql; +CREATE EVENT TRIGGER on_login_trigger ON LOGIN EXECUTE PROCEDURE on_login_proc(); +ALTER EVENT TRIGGER on_login_trigger ENABLE ALWAYS; +\c +SELECT count(*) FROM user_logins; +\set superuser :USER +\c - login_tester +SELECT count(*) FROM user_logins; +CREATE EVENT TRIGGER on_login_trigger ON LOGIN EXECUTE PROCEDURE on_login_proc(); -- non-superuser should not be able to! +\c - :superuser +SELECT id, CASE WHEN who = :'superuser' THEN 'superuser' ELSE who END AS who FROM user_logins; + +-- Turn off login triggers and make an erroneous one. We check that it will not be called. +ALTER SYSTEM SET login_triggers = 'off'; +CREATE FUNCTION on_login_fail() RETURNS event_trigger SECURITY DEFINER AS $$ +BEGIN + RAISE EXCEPTION 'login trigger failure'; +END; +$$ LANGUAGE plpgsql; +CREATE EVENT TRIGGER on_login_fail ON LOGIN EXECUTE PROCEDURE on_login_fail(); +ALTER EVENT TRIGGER on_login_fail ENABLE ALWAYS; +SELECT pg_reload_conf(); +\c +SELECT id, CASE WHEN who = :'superuser' THEN 'superuser' ELSE who END AS who FROM user_logins; +-- check dathaslogintriggers in system catalog +SELECT dathaslogintriggers FROM pg_database WHERE datname= :'DBNAME'; +-- Cleanup +DROP TABLE user_logins; +DROP EVENT TRIGGER on_login_trigger; +DROP EVENT TRIGGER on_login_fail; +DROP FUNCTION on_login_proc(); +DROP FUNCTION on_login_fail(); +DROP USER login_tester; +-- End +