Skip to content

Commit 56068e8

Browse files
alvherrepull[bot]
alvherre
authored and
pull[bot]
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 ee7ff25 commit 56068e8

File tree

3 files changed

+50
-10
lines changed

3 files changed

+50
-10
lines changed

‎src/backend/commands/tablecmds.c

Lines changed: 8 additions & 10 deletions
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

Lines changed: 21 additions & 0 deletions
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

Lines changed: 21 additions & 0 deletions
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)