Feedback-ID: ia2694551:Fastmail
Date: Tue, 5 Sep 2023 19:45:27 +0200
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
To: Pg Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: missing privilege check after not-null constraint rework
Message-ID: <202309051745.jk3bqxm26fbx@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="22fwpudqxyrxnub5"
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <202309051744.y4mndw5gwzhh@alvherre.pgsql>
Archived-At: 
 <https://www.postgresql.org/message-id/202309051745.jk3bqxm26fbx%40alvherre.pgsql>
Precedence: bulk


--22fwpudqxyrxnub5
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit

On 2023-Sep-05, Alvaro Herrera wrote:

> Here's a fix to move the privilege check on constraint dropping from
> ATExecDropConstraint to dropconstraint_internal.


-- 
Álvaro Herrera         PostgreSQL Developer  —  https://www.EnterpriseDB.com/
"No renuncies a nada. No te aferres a nada."

--22fwpudqxyrxnub5
Content-Type: text/x-diff; charset=utf-8
Content-Disposition: attachment;
	filename="0001-Move-privilege-check-to-the-right-place.patch"

From f58d4532d39bc39edc3841c1ee5c3e3e9e9d153a Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
Date: Tue, 5 Sep 2023 17:06:23 +0200
Subject: [PATCH] Move privilege check to the right place

Now that ATExecDropConstraint doesn't recurse anymore, there's no point
in testing the 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.
---
 src/backend/commands/tablecmds.c | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 47c556669f..8a2c671b66 100644
--- 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);
-- 
2.30.2


--22fwpudqxyrxnub5--