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
This commit is contained in:
Alvaro Herrera 2023-09-07 12:15:18 +02:00
parent 3af7217942
commit ac22a9545c
No known key found for this signature in database
GPG Key ID: 1C20ACB9D5C564AE
3 changed files with 50 additions and 10 deletions

View File

@ -542,8 +542,7 @@ static void GetForeignKeyCheckTriggers(Relation trigrel,
Oid *insertTriggerOid, Oid *insertTriggerOid,
Oid *updateTriggerOid); Oid *updateTriggerOid);
static void ATExecDropConstraint(Relation rel, const char *constrName, static void ATExecDropConstraint(Relation rel, const char *constrName,
DropBehavior behavior, DropBehavior behavior, bool recurse,
bool recurse, bool recursing,
bool missing_ok, LOCKMODE lockmode); bool missing_ok, LOCKMODE lockmode);
static ObjectAddress dropconstraint_internal(Relation rel, static ObjectAddress dropconstraint_internal(Relation rel,
HeapTuple constraintTup, DropBehavior behavior, HeapTuple constraintTup, DropBehavior behavior,
@ -5236,7 +5235,7 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab,
break; break;
case AT_DropConstraint: /* DROP CONSTRAINT */ case AT_DropConstraint: /* DROP CONSTRAINT */
ATExecDropConstraint(rel, cmd->name, cmd->behavior, ATExecDropConstraint(rel, cmd->name, cmd->behavior,
cmd->recurse, false, cmd->recurse,
cmd->missing_ok, lockmode); cmd->missing_ok, lockmode);
break; break;
case AT_AlterColumnType: /* ALTER COLUMN TYPE */ case AT_AlterColumnType: /* ALTER COLUMN TYPE */
@ -12263,8 +12262,7 @@ createForeignKeyCheckTriggers(Oid myRelOid, Oid refRelOid,
*/ */
static void static void
ATExecDropConstraint(Relation rel, const char *constrName, ATExecDropConstraint(Relation rel, const char *constrName,
DropBehavior behavior, DropBehavior behavior, bool recurse,
bool recurse, bool recursing,
bool missing_ok, LOCKMODE lockmode) bool missing_ok, LOCKMODE lockmode)
{ {
Relation conrel; Relation conrel;
@ -12273,10 +12271,6 @@ ATExecDropConstraint(Relation rel, const char *constrName,
HeapTuple tuple; HeapTuple tuple;
bool found = false; bool found = false;
/* At top level, permission check was done in ATPrepCmd, else do it */
if (recursing)
ATSimplePermissions(AT_DropConstraint, rel, ATT_TABLE | ATT_FOREIGN_TABLE);
conrel = table_open(ConstraintRelationId, RowExclusiveLock); conrel = table_open(ConstraintRelationId, RowExclusiveLock);
/* /*
@ -12302,7 +12296,7 @@ ATExecDropConstraint(Relation rel, const char *constrName,
{ {
List *readyRels = NIL; List *readyRels = NIL;
dropconstraint_internal(rel, tuple, behavior, recurse, recursing, dropconstraint_internal(rel, tuple, behavior, recurse, false,
missing_ok, &readyRels, lockmode); missing_ok, &readyRels, lockmode);
found = true; found = true;
} }
@ -12353,6 +12347,10 @@ dropconstraint_internal(Relation rel, HeapTuple constraintTup, DropBehavior beha
return InvalidObjectAddress; return InvalidObjectAddress;
*readyRels = lappend_oid(*readyRels, RelationGetRelid(rel)); *readyRels = lappend_oid(*readyRels, RelationGetRelid(rel));
/* At top level, permission check was done in ATPrepCmd, else do it */
if (recursing)
ATSimplePermissions(AT_DropConstraint, rel, ATT_TABLE | ATT_FOREIGN_TABLE);
conrel = table_open(ConstraintRelationId, RowExclusiveLock); conrel = table_open(ConstraintRelationId, RowExclusiveLock);
con = (Form_pg_constraint) GETSTRUCT(constraintTup); con = (Form_pg_constraint) GETSTRUCT(constraintTup);

View File

@ -2430,6 +2430,27 @@ NOTICE: drop cascades to 2 other objects
DETAIL: drop cascades to table inh_multiparent DETAIL: drop cascades to table inh_multiparent
drop cascades to table inh_multiparent2 drop cascades to table inh_multiparent2
-- --
-- Mixed ownership inheritance tree
--
create role regress_alice;
create role regress_bob;
grant all on schema public to regress_alice, regress_bob;
grant regress_alice to regress_bob;
set session authorization regress_alice;
create table inh_parent (a int not null);
set session authorization regress_bob;
create table inh_child () inherits (inh_parent);
set session authorization regress_alice;
-- alice can't do this: she doesn't own inh_child
alter table inh_parent alter a drop not null;
ERROR: must be owner of table inh_child
set session authorization regress_bob;
alter table inh_parent alter a drop not null;
reset session authorization;
drop table inh_parent, inh_child;
revoke all on schema public from regress_alice, regress_bob;
drop role regress_alice, regress_bob;
--
-- Check use of temporary tables with inheritance trees -- Check use of temporary tables with inheritance trees
-- --
create table inh_perm_parent (a1 int); create table inh_perm_parent (a1 int);

View File

@ -920,6 +920,27 @@ select conrelid::regclass, contype, conname,
drop table inh_p1, inh_p2, inh_p3, inh_p4 cascade; drop table inh_p1, inh_p2, inh_p3, inh_p4 cascade;
--
-- Mixed ownership inheritance tree
--
create role regress_alice;
create role regress_bob;
grant all on schema public to regress_alice, regress_bob;
grant regress_alice to regress_bob;
set session authorization regress_alice;
create table inh_parent (a int not null);
set session authorization regress_bob;
create table inh_child () inherits (inh_parent);
set session authorization regress_alice;
-- alice can't do this: she doesn't own inh_child
alter table inh_parent alter a drop not null;
set session authorization regress_bob;
alter table inh_parent alter a drop not null;
reset session authorization;
drop table inh_parent, inh_child;
revoke all on schema public from regress_alice, regress_bob;
drop role regress_alice, regress_bob;
-- --
-- Check use of temporary tables with inheritance trees -- Check use of temporary tables with inheritance trees
-- --