Dear colleagues,
Please see my suggestions below and the updated patch attached.
>Среда, 29 сентября 2021, 15:14 +03:00 от Teodor Sigaev < [email protected] >:
>
>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 < [email protected] > 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: [email protected]
> 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
+
