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

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 47c556669f33e73dd1a6e01505886b1cb0872f01..8a2c671b6620cba461db05e4c8437a02711c2854 100644 (file)
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -542,8 +542,7 @@ static void GetForeignKeyCheckTriggers(Relation trigrel,
                                                                           Oid *insertTriggerOid,
                                                                           Oid *updateTriggerOid);
 static void ATExecDropConstraint(Relation rel, const char *constrName,
-                                                                DropBehavior behavior,
-                                                                bool recurse, bool recursing,
+                                                                DropBehavior behavior, bool recurse,
                                                                 bool missing_ok, LOCKMODE lockmode);
 static ObjectAddress dropconstraint_internal(Relation rel,
                                                                                         HeapTuple constraintTup, DropBehavior behavior,
@@ -5236,7 +5235,7 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab,
                        break;
                case AT_DropConstraint: /* DROP CONSTRAINT */
                        ATExecDropConstraint(rel, cmd->name, cmd->behavior,
-                                                                cmd->recurse, false,
+                                                                cmd->recurse,
                                                                 cmd->missing_ok, lockmode);
                        break;
                case AT_AlterColumnType:        /* ALTER COLUMN TYPE */
@@ -12263,8 +12262,7 @@ createForeignKeyCheckTriggers(Oid myRelOid, Oid refRelOid,
  */
 static void
 ATExecDropConstraint(Relation rel, const char *constrName,
-                                        DropBehavior behavior,
-                                        bool recurse, bool recursing,
+                                        DropBehavior behavior, bool recurse,
                                         bool missing_ok, LOCKMODE lockmode)
 {
        Relation        conrel;
@@ -12273,10 +12271,6 @@ ATExecDropConstraint(Relation rel, const char *constrName,
        HeapTuple       tuple;
        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);
 
        /*
@@ -12302,7 +12296,7 @@ ATExecDropConstraint(Relation rel, const char *constrName,
        {
                List       *readyRels = NIL;
 
-               dropconstraint_internal(rel, tuple, behavior, recurse, recursing,
+               dropconstraint_internal(rel, tuple, behavior, recurse, false,
                                                                missing_ok, &readyRels, lockmode);
                found = true;
        }
@@ -12353,6 +12347,10 @@ dropconstraint_internal(Relation rel, HeapTuple constraintTup, DropBehavior beha
                return InvalidObjectAddress;
        *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);
 
        con = (Form_pg_constraint) GETSTRUCT(constraintTup);

diff --git a/src/test/regress/expected/inherit.out b/src/test/regress/expected/inherit.out
index 59583e1e417c4ec89038acdfa38b0341e8ff2316..08d93884d8722e1b104a8bc1a6bdec23c0cecb22 100644 (file)
--- a/src/test/regress/expected/inherit.out
+++ b/src/test/regress/expected/inherit.out
@@ -2430,6 +2430,27 @@ NOTICE:  drop cascades to 2 other objects
 DETAIL:  drop cascades to table inh_multiparent
 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
 --
 create table inh_perm_parent (a1 int);

diff --git a/src/test/regress/sql/inherit.sql b/src/test/regress/sql/inherit.sql
index abe8602682c6a5aee5e8939320f7de928fdb2e7c..3d57c7ee950ca1b05990e4dee4c7916ff86a9ee9 100644 (file)
--- a/src/test/regress/sql/inherit.sql
+++ b/src/test/regress/sql/inherit.sql
@@ -920,6 +920,27 @@ select conrelid::regclass, contype, conname,
 
 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
 --