Skip to content

Commit ac22a95

Browse files
alvherrealvherre
committed
Move privilege check to the right place
Now that ATExecDropConstraint doesn't recurse anymore, so it's wrong to test privileges "during recursion" there. Move the check to dropconstraint_internal, which is the place where recursion occurs. In passing, remove now-useless 'recursing' argument to ATExecDropConstraint. Discussion: https://postgr.es/m/202309051744.y4mndw5gwzhh@alvherre.pgsql
1 parent 3af7217 commit ac22a95

File tree

3 files changed

+50
-10
lines changed

3 files changed

+50
-10
lines changed

‎src/backend/commands/tablecmds.c

+8-10
Original file line numberDiff line numberDiff line change
@@ -542,8 +542,7 @@ static void GetForeignKeyCheckTriggers(Relation trigrel,
542542
Oid *insertTriggerOid,
543543
Oid *updateTriggerOid);
544544
static void ATExecDropConstraint(Relation rel, const char *constrName,
545-
DropBehavior behavior,
546-
bool recurse, bool recursing,
545+
DropBehavior behavior, bool recurse,
547546
bool missing_ok, LOCKMODE lockmode);
548547
static ObjectAddress dropconstraint_internal(Relation rel,
549548
HeapTuple constraintTup, DropBehavior behavior,
@@ -5236,7 +5235,7 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab,
52365235
break;
52375236
case AT_DropConstraint: /* DROP CONSTRAINT */
52385237
ATExecDropConstraint(rel, cmd->name, cmd->behavior,
5239-
cmd->recurse, false,
5238+
cmd->recurse,
52405239
cmd->missing_ok, lockmode);
52415240
break;
52425241
case AT_AlterColumnType: /* ALTER COLUMN TYPE */
@@ -12263,8 +12262,7 @@ createForeignKeyCheckTriggers(Oid myRelOid, Oid refRelOid,
1226312262
*/
1226412263
static void
1226512264
ATExecDropConstraint(Relation rel, const char *constrName,
12266-
DropBehavior behavior,
12267-
bool recurse, bool recursing,
12265+
DropBehavior behavior, bool recurse,
1226812266
bool missing_ok, LOCKMODE lockmode)
1226912267
{
1227012268
Relation conrel;
@@ -12273,10 +12271,6 @@ ATExecDropConstraint(Relation rel, const char *constrName,
1227312271
HeapTuple tuple;
1227412272
bool found = false;
1227512273

12276-
/* At top level, permission check was done in ATPrepCmd, else do it */
12277-
if (recursing)
12278-
ATSimplePermissions(AT_DropConstraint, rel, ATT_TABLE | ATT_FOREIGN_TABLE);
12279-
1228012274
conrel = table_open(ConstraintRelationId, RowExclusiveLock);
1228112275

1228212276
/*
@@ -12302,7 +12296,7 @@ ATExecDropConstraint(Relation rel, const char *constrName,
1230212296
{
1230312297
List *readyRels = NIL;
1230412298

12305-
dropconstraint_internal(rel, tuple, behavior, recurse, recursing,
12299+
dropconstraint_internal(rel, tuple, behavior, recurse, false,
1230612300
missing_ok, &readyRels, lockmode);
1230712301
found = true;
1230812302
}
@@ -12353,6 +12347,10 @@ dropconstraint_internal(Relation rel, HeapTuple constraintTup, DropBehavior beha
1235312347
return InvalidObjectAddress;
1235412348
*readyRels = lappend_oid(*readyRels, RelationGetRelid(rel));
1235512349

12350+
/* At top level, permission check was done in ATPrepCmd, else do it */
12351+
if (recursing)
12352+
ATSimplePermissions(AT_DropConstraint, rel, ATT_TABLE | ATT_FOREIGN_TABLE);
12353+
1235612354
conrel = table_open(ConstraintRelationId, RowExclusiveLock);
1235712355

1235812356
con = (Form_pg_constraint) GETSTRUCT(constraintTup);

‎src/test/regress/expected/inherit.out

+21
Original file line numberDiff line numberDiff line change
@@ -2430,6 +2430,27 @@ NOTICE: drop cascades to 2 other objects
24302430
DETAIL: drop cascades to table inh_multiparent
24312431
drop cascades to table inh_multiparent2
24322432
--
2433+
-- Mixed ownership inheritance tree
2434+
--
2435+
create role regress_alice;
2436+
create role regress_bob;
2437+
grant all on schema public to regress_alice, regress_bob;
2438+
grant regress_alice to regress_bob;
2439+
set session authorization regress_alice;
2440+
create table inh_parent (a int not null);
2441+
set session authorization regress_bob;
2442+
create table inh_child () inherits (inh_parent);
2443+
set session authorization regress_alice;
2444+
-- alice can't do this: she doesn't own inh_child
2445+
alter table inh_parent alter a drop not null;
2446+
ERROR: must be owner of table inh_child
2447+
set session authorization regress_bob;
2448+
alter table inh_parent alter a drop not null;
2449+
reset session authorization;
2450+
drop table inh_parent, inh_child;
2451+
revoke all on schema public from regress_alice, regress_bob;
2452+
drop role regress_alice, regress_bob;
2453+
--
24332454
-- Check use of temporary tables with inheritance trees
24342455
--
24352456
create table inh_perm_parent (a1 int);

‎src/test/regress/sql/inherit.sql

+21
Original file line numberDiff line numberDiff line change
@@ -920,6 +920,27 @@ select conrelid::regclass, contype, conname,
920920

921921
drop table inh_p1, inh_p2, inh_p3, inh_p4 cascade;
922922

923+
--
924+
-- Mixed ownership inheritance tree
925+
--
926+
create role regress_alice;
927+
create role regress_bob;
928+
grant all on schema public to regress_alice, regress_bob;
929+
grant regress_alice to regress_bob;
930+
set session authorization regress_alice;
931+
create table inh_parent (a int not null);
932+
set session authorization regress_bob;
933+
create table inh_child () inherits (inh_parent);
934+
set session authorization regress_alice;
935+
-- alice can't do this: she doesn't own inh_child
936+
alter table inh_parent alter a drop not null;
937+
set session authorization regress_bob;
938+
alter table inh_parent alter a drop not null;
939+
reset session authorization;
940+
drop table inh_parent, inh_child;
941+
revoke all on schema public from regress_alice, regress_bob;
942+
drop role regress_alice, regress_bob;
943+
923944
--
924945
-- Check use of temporary tables with inheritance trees
925946
--

0 commit comments

Comments
 (0)