Sorry, attached the output file.
From 2165513e858a5a6c7a620b1499b2f634c4f2ab44 Mon Sep 17 00:00:00 2001 From: steve-chavez <stevechavez...@gmail.com> Date: Sun, 20 Apr 2025 19:46:00 -0500 Subject: [PATCH] Allow regular users to create event triggers
--- src/backend/commands/event_trigger.c | 33 +++------ src/backend/utils/cache/evtcache.c | 1 + src/include/utils/evtcache.h | 1 + src/test/regress/expected/event_trigger.out | 13 +--- .../expected/event_trigger_nosuper.out | 74 +++++++++++++++++++ src/test/regress/parallel_schedule | 4 + src/test/regress/sql/event_trigger.sql | 11 +-- .../regress/sql/event_trigger_nosuper.sql | 65 ++++++++++++++++ 8 files changed, 156 insertions(+), 46 deletions(-) create mode 100644 src/test/regress/expected/event_trigger_nosuper.out create mode 100644 src/test/regress/sql/event_trigger_nosuper.sql diff --git a/src/backend/commands/event_trigger.c b/src/backend/commands/event_trigger.c index edc2c988e29..9dc813adb6a 100644 --- a/src/backend/commands/event_trigger.c +++ b/src/backend/commands/event_trigger.c @@ -126,18 +126,6 @@ CreateEventTrigger(CreateEventTrigStmt *stmt) ListCell *lc; List *tags = NULL; - /* - * It would be nice to allow database owners or even regular users to do - * this, but there are obvious privilege escalation risks which would have - * to somehow be plugged first. - */ - if (!superuser()) - ereport(ERROR, - (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), - errmsg("permission denied to create event trigger \"%s\"", - stmt->trigname), - errhint("Must be superuser to create an event trigger."))); - /* Validate event name. */ if (strcmp(stmt->eventname, "ddl_command_start") != 0 && strcmp(stmt->eventname, "ddl_command_end") != 0 && @@ -545,14 +533,6 @@ AlterEventTriggerOwner_internal(Relation rel, HeapTuple tup, Oid newOwnerId) aclcheck_error(ACLCHECK_NOT_OWNER, OBJECT_EVENT_TRIGGER, NameStr(form->evtname)); - /* New owner must be a superuser */ - if (!superuser_arg(newOwnerId)) - ereport(ERROR, - (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), - errmsg("permission denied to change owner of event trigger \"%s\"", - NameStr(form->evtname)), - errhint("The owner of an event trigger must be a superuser."))); - form->evtowner = newOwnerId; CatalogTupleUpdate(rel, &tup->t_self, tup); @@ -698,7 +678,7 @@ EventTriggerCommonSetup(Node *parsetree, if (unfiltered || filter_event_trigger(tag, item)) { /* We must plan to fire this trigger. */ - runlist = lappend_oid(runlist, item->fnoid); + runlist = lappend(runlist, item); } } @@ -1084,11 +1064,16 @@ EventTriggerInvoke(List *fn_oid_list, EventTriggerData *trigdata) foreach(lc, fn_oid_list) { LOCAL_FCINFO(fcinfo, 0); - Oid fnoid = lfirst_oid(lc); + EventTriggerCacheItem *item = (EventTriggerCacheItem*) lfirst_oid(lc); FmgrInfo flinfo; PgStat_FunctionCallUsage fcusage; + Oid current_user = GetUserId(); + + if (!is_member_of_role_nosuper(current_user, item->owneroid)) { + continue; + } - elog(DEBUG1, "EventTriggerInvoke %u", fnoid); + elog(DEBUG1, "EventTriggerInvoke %u", item->fnoid); /* * We want each event trigger to be able to see the results of the @@ -1102,7 +1087,7 @@ EventTriggerInvoke(List *fn_oid_list, EventTriggerData *trigdata) CommandCounterIncrement(); /* Look up the function */ - fmgr_info(fnoid, &flinfo); + fmgr_info(item->fnoid, &flinfo); /* Call the function, passing no arguments but setting a context. */ InitFunctionCallInfoData(*fcinfo, &flinfo, 0, diff --git a/src/backend/utils/cache/evtcache.c b/src/backend/utils/cache/evtcache.c index ce596bf5638..1dc9a864034 100644 --- a/src/backend/utils/cache/evtcache.c +++ b/src/backend/utils/cache/evtcache.c @@ -175,6 +175,7 @@ BuildEventTriggerCache(void) item = palloc0(sizeof(EventTriggerCacheItem)); item->fnoid = form->evtfoid; item->enabled = form->evtenabled; + item->owneroid = form->evtowner; /* Decode and sort tags array. */ evttags = heap_getattr(tup, Anum_pg_event_trigger_evttags, diff --git a/src/include/utils/evtcache.h b/src/include/utils/evtcache.h index 9d9fcb8657b..0ecd6a54c5f 100644 --- a/src/include/utils/evtcache.h +++ b/src/include/utils/evtcache.h @@ -31,6 +31,7 @@ typedef struct Oid fnoid; /* function to be called */ char enabled; /* as SESSION_REPLICATION_ROLE_* */ Bitmapset *tagset; /* command tags, or NULL if empty */ + Oid owneroid; /* owner of the event trigger */ } EventTriggerCacheItem; extern List *EventCacheLookup(EventTriggerEvent event); diff --git a/src/test/regress/expected/event_trigger.out b/src/test/regress/expected/event_trigger.out index 7b2198eac6f..0ca16ec5e38 100644 --- a/src/test/regress/expected/event_trigger.out +++ b/src/test/regress/expected/event_trigger.out @@ -84,14 +84,6 @@ create event trigger regress_event_trigger2 on ddl_command_start execute procedure test_event_trigger(); -- OK comment on event trigger regress_event_trigger is 'test comment'; --- drop as non-superuser should fail -create role regress_evt_user; -set role regress_evt_user; -create event trigger regress_event_trigger_noperms on ddl_command_start - execute procedure test_event_trigger(); -ERROR: permission denied to create event trigger "regress_event_trigger_noperms" -HINT: Must be superuser to create an event trigger. -reset role; -- test enabling and disabling alter event trigger regress_event_trigger disable; -- fires _trigger2 and _trigger_end should fire, but not _trigger @@ -168,15 +160,12 @@ create foreign data wrapper useless; NOTICE: test_event_trigger: ddl_command_end CREATE FOREIGN DATA WRAPPER create server useless_server foreign data wrapper useless; NOTICE: test_event_trigger: ddl_command_end CREATE SERVER +create role regress_evt_user; create user mapping for regress_evt_user server useless_server; NOTICE: test_event_trigger: ddl_command_end CREATE USER MAPPING alter default privileges for role regress_evt_user revoke delete on tables from regress_evt_user; NOTICE: test_event_trigger: ddl_command_end ALTER DEFAULT PRIVILEGES --- alter owner to non-superuser should fail -alter event trigger regress_event_trigger owner to regress_evt_user; -ERROR: permission denied to change owner of event trigger "regress_event_trigger" -HINT: The owner of an event trigger must be a superuser. -- alter owner to superuser should work alter role regress_evt_user superuser; alter event trigger regress_event_trigger owner to regress_evt_user; diff --git a/src/test/regress/expected/event_trigger_nosuper.out b/src/test/regress/expected/event_trigger_nosuper.out new file mode 100644 index 00000000000..38c4c1d5301 --- /dev/null +++ b/src/test/regress/expected/event_trigger_nosuper.out @@ -0,0 +1,74 @@ +-- setup roles and privileges +create role evtrig_owner; +create role member_1; +create role member_2; +grant evtrig_owner to member_1; +grant evtrig_owner to member_2; +create role non_member; +grant all on schema public to evtrig_owner, member_1, member_2, non_member; +\echo + +-- create nonsuper event trigger +set role evtrig_owner; +create function show_current_user() + returns event_trigger + language plpgsql as +$$ +begin + raise notice 'the event trigger is executed for %', current_user; +end; +$$; +create event trigger evtrig_show_current_user_1 + on ddl_command_start + execute procedure show_current_user(); +reset role; +\echo + +-- create super event trigger and alter it to be nonsuper +create event trigger evtrig_show_current_user_2 + on ddl_command_end + execute procedure show_current_user(); +alter event trigger evtrig_show_current_user_2 owner to evtrig_owner; +\echo + +-- evtrig should not fire for superusers +select current_setting('is_superuser'); + current_setting +----------------- + on +(1 row) + +create table evtrig_quux(); +\echo + +-- evtrig should fire for members +set role member_1; +create table evtrig_foo(); +NOTICE: the event trigger is executed for member_1 +NOTICE: the event trigger is executed for member_1 +\echo + +set role member_2; +create table evtrig_bar(); +NOTICE: the event trigger is executed for member_2 +NOTICE: the event trigger is executed for member_2 +\echo + +-- evtrig should not fire for non-members +set role non_member; +create table evtrig_qux(); +\echo + +-- cleanup +reset role; +drop table evtrig_qux; +drop table evtrig_bar; +drop table evtrig_foo; +revoke all on schema public from member_1, member_2, non_member, evtrig_owner; +drop event trigger evtrig_show_current_user_1; +drop event trigger evtrig_show_current_user_2; +drop function show_current_user(); +drop role member_1; +drop role member_2; +drop role non_member; +drop role evtrig_owner; diff --git a/src/test/regress/parallel_schedule b/src/test/regress/parallel_schedule index a424be2a6bf..fd3ef719f5e 100644 --- a/src/test/regress/parallel_schedule +++ b/src/test/regress/parallel_schedule @@ -130,6 +130,10 @@ test: partition_join partition_prune reloptions hash_part indexing partition_agg # oidjoins is read-only, though, and should run late for best coverage test: oidjoins event_trigger +# event_trigger_nosuper cannot run concurrently +# with other tests that runs DDL +test: event_trigger_nosuper + # event_trigger_login cannot run concurrently with any other tests because # on-login event handling could catch connection of a concurrent test. test: event_trigger_login diff --git a/src/test/regress/sql/event_trigger.sql b/src/test/regress/sql/event_trigger.sql index 013546b8305..6833e991762 100644 --- a/src/test/regress/sql/event_trigger.sql +++ b/src/test/regress/sql/event_trigger.sql @@ -85,13 +85,6 @@ create event trigger regress_event_trigger2 on ddl_command_start -- OK comment on event trigger regress_event_trigger is 'test comment'; --- drop as non-superuser should fail -create role regress_evt_user; -set role regress_evt_user; -create event trigger regress_event_trigger_noperms on ddl_command_start - execute procedure test_event_trigger(); -reset role; - -- test enabling and disabling alter event trigger regress_event_trigger disable; -- fires _trigger2 and _trigger_end should fire, but not _trigger @@ -139,13 +132,11 @@ revoke all on table event_trigger_fire1 from public; drop table event_trigger_fire1; create foreign data wrapper useless; create server useless_server foreign data wrapper useless; +create role regress_evt_user; create user mapping for regress_evt_user server useless_server; alter default privileges for role regress_evt_user revoke delete on tables from regress_evt_user; --- alter owner to non-superuser should fail -alter event trigger regress_event_trigger owner to regress_evt_user; - -- alter owner to superuser should work alter role regress_evt_user superuser; alter event trigger regress_event_trigger owner to regress_evt_user; diff --git a/src/test/regress/sql/event_trigger_nosuper.sql b/src/test/regress/sql/event_trigger_nosuper.sql new file mode 100644 index 00000000000..4023575bf44 --- /dev/null +++ b/src/test/regress/sql/event_trigger_nosuper.sql @@ -0,0 +1,65 @@ +-- setup roles and privileges +create role evtrig_owner; +create role member_1; +create role member_2; +grant evtrig_owner to member_1; +grant evtrig_owner to member_2; +create role non_member; +grant all on schema public to evtrig_owner, member_1, member_2, non_member; +\echo + +-- create nonsuper event trigger +set role evtrig_owner; +create function show_current_user() + returns event_trigger + language plpgsql as +$$ +begin + raise notice 'the event trigger is executed for %', current_user; +end; +$$; +create event trigger evtrig_show_current_user_1 + on ddl_command_start + execute procedure show_current_user(); +reset role; +\echo + +-- create super event trigger and alter it to be nonsuper +create event trigger evtrig_show_current_user_2 + on ddl_command_end + execute procedure show_current_user(); +alter event trigger evtrig_show_current_user_2 owner to evtrig_owner; +\echo + +-- evtrig should not fire for superusers +select current_setting('is_superuser'); +create table evtrig_quux(); +\echo + +-- evtrig should fire for members +set role member_1; +create table evtrig_foo(); +\echo + +set role member_2; +create table evtrig_bar(); +\echo + +-- evtrig should not fire for non-members +set role non_member; +create table evtrig_qux(); +\echo + +-- cleanup +reset role; +drop table evtrig_qux; +drop table evtrig_bar; +drop table evtrig_foo; +revoke all on schema public from member_1, member_2, non_member, evtrig_owner; +drop event trigger evtrig_show_current_user_1; +drop event trigger evtrig_show_current_user_2; +drop function show_current_user(); +drop role member_1; +drop role member_2; +drop role non_member; +drop role evtrig_owner; -- 2.42.0