0001-Don-t-check-child-s-TRUNCATE-privilege-when-truncate-fujii-pg96.patch

text/plain

Filename: 0001-Don-t-check-child-s-TRUNCATE-privilege-when-truncate-fujii-pg96.patch
Type: text/plain
Part: 4
Message: Re: table partitioning and access privileges

Patch

Format: unified
Series: patch 0001
File+
src/backend/commands/tablecmds.c 49 18
src/test/regress/expected/privileges.out 21 0
src/test/regress/sql/privileges.sql 14 0
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 9c6ca7ff9c..31bc431d9d 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -271,7 +271,9 @@ struct DropRelationCallbackState
 #define		ATT_COMPOSITE_TYPE		0x0010
 #define		ATT_FOREIGN_TABLE		0x0020
 
-static void truncate_check_rel(Relation rel);
+static void truncate_check_rel(Oid relid, Form_pg_class reltuple);
+static void truncate_check_perms(Oid relid, Form_pg_class reltuple);
+static void truncate_check_activity(Relation rel);
 static List *MergeAttributes(List *schema, List *supers, char relpersistence,
 				List **supOids, List **supconstr, int *supOidCount);
 static bool MergeCheckConstraint(List *constraints, char *name, Node *expr);
@@ -1050,7 +1052,11 @@ ExecuteTruncate(TruncateStmt *stmt)
 			heap_close(rel, lockmode);
 			continue;
 		}
-		truncate_check_rel(rel);
+
+		truncate_check_rel(myrelid, rel->rd_rel);
+		truncate_check_perms(myrelid, rel->rd_rel);
+		truncate_check_activity(rel);
+
 		rels = lappend(rels, rel);
 		relids = lappend_oid(relids, myrelid);
 
@@ -1086,7 +1092,15 @@ ExecuteTruncate(TruncateStmt *stmt)
 					continue;
 				}
 
-				truncate_check_rel(rel);
+				/*
+				 * Inherited TRUNCATE commands perform access
+				 * permission checks on the parent table only.
+				 * So we skip checking the children's permissions
+				 * and don't call truncate_check_perms() here.
+				 */
+				truncate_check_rel(RelationGetRelid(rel), rel->rd_rel);
+				truncate_check_activity(rel);
+
 				rels = lappend(rels, rel);
 				relids = lappend_oid(relids, childrelid);
 			}
@@ -1120,7 +1134,9 @@ ExecuteTruncate(TruncateStmt *stmt)
 				ereport(NOTICE,
 						(errmsg("truncate cascades to table \"%s\"",
 								RelationGetRelationName(rel))));
-				truncate_check_rel(rel);
+				truncate_check_rel(relid, rel->rd_rel);
+				truncate_check_perms(relid, rel->rd_rel);
+				truncate_check_activity(rel);
 				rels = lappend(rels, rel);
 				relids = lappend_oid(relids, relid);
 			}
@@ -1328,30 +1344,45 @@ ExecuteTruncate(TruncateStmt *stmt)
  * Check that a given rel is safe to truncate.  Subroutine for ExecuteTruncate
  */
 static void
-truncate_check_rel(Relation rel)
+truncate_check_rel(Oid relid, Form_pg_class reltuple)
 {
-	AclResult	aclresult;
+	char	   *relname = NameStr(reltuple->relname);
 
 	/* Only allow truncate on regular tables */
-	if (rel->rd_rel->relkind != RELKIND_RELATION)
+	if (reltuple->relkind != RELKIND_RELATION)
 		ereport(ERROR,
 				(errcode(ERRCODE_WRONG_OBJECT_TYPE),
-				 errmsg("\"%s\" is not a table",
-						RelationGetRelationName(rel))));
+				 errmsg("\"%s\" is not a table", relname)));
 
-	/* Permissions checks */
-	aclresult = pg_class_aclcheck(RelationGetRelid(rel), GetUserId(),
-								  ACL_TRUNCATE);
-	if (aclresult != ACLCHECK_OK)
-		aclcheck_error(aclresult, ACL_KIND_CLASS,
-					   RelationGetRelationName(rel));
-
-	if (!allowSystemTableMods && IsSystemRelation(rel))
+	if (!allowSystemTableMods && IsSystemClass(relid, reltuple))
 		ereport(ERROR,
 				(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
 				 errmsg("permission denied: \"%s\" is a system catalog",
-						RelationGetRelationName(rel))));
+						relname)));
+}
 
+/*
+ * Check that current user has the permission to truncate given relation.
+ */
+static void
+truncate_check_perms(Oid relid, Form_pg_class reltuple)
+{
+	char	   *relname = NameStr(reltuple->relname);
+	AclResult	aclresult;
+
+	/* Permissions checks */
+	aclresult = pg_class_aclcheck(relid, GetUserId(), ACL_TRUNCATE);
+	if (aclresult != ACLCHECK_OK)
+		aclcheck_error(aclresult, ACL_KIND_CLASS, relname);
+}
+
+/*
+ * Set of extra sanity checks to check if a given relation is safe to
+ * truncate.
+ */
+static void
+truncate_check_activity(Relation rel)
+{
 	/*
 	 * Don't allow truncate on temp tables of other backends ... their local
 	 * buffer manager is not going to cope.
diff --git a/src/test/regress/expected/privileges.out b/src/test/regress/expected/privileges.out
index b46af7c5e6..34f1e74f6a 100644
--- a/src/test/regress/expected/privileges.out
+++ b/src/test/regress/expected/privileges.out
@@ -695,6 +695,27 @@ SELECT oid FROM atestp2; -- ok
 -----
 (0 rows)
 
+-- child's permissions do not apply when operating on parent
+SET SESSION AUTHORIZATION regress_user1;
+REVOKE ALL ON atestc FROM regress_user2;
+GRANT ALL ON atestp1 TO regress_user2;
+SET SESSION AUTHORIZATION regress_user2;
+SELECT f2 FROM atestp1; -- ok
+ f2 
+----
+(0 rows)
+
+SELECT f2 FROM atestc; -- fail
+ERROR:  permission denied for relation atestc
+DELETE FROM atestp1; -- ok
+DELETE FROM atestc; -- fail
+ERROR:  permission denied for relation atestc
+UPDATE atestp1 SET f1 = 1; -- ok
+UPDATE atestc SET f1 = 1; -- fail
+ERROR:  permission denied for relation atestc
+TRUNCATE atestp1; -- ok
+TRUNCATE atestc; -- fail
+ERROR:  permission denied for relation atestc
 -- privileges on functions, languages
 -- switch to superuser
 \c -
diff --git a/src/test/regress/sql/privileges.sql b/src/test/regress/sql/privileges.sql
index c7d7347091..c802f190d5 100644
--- a/src/test/regress/sql/privileges.sql
+++ b/src/test/regress/sql/privileges.sql
@@ -446,6 +446,20 @@ SELECT fy FROM atestp2; -- ok
 SELECT atestp2 FROM atestp2; -- ok
 SELECT oid FROM atestp2; -- ok
 
+-- child's permissions do not apply when operating on parent
+SET SESSION AUTHORIZATION regress_user1;
+REVOKE ALL ON atestc FROM regress_user2;
+GRANT ALL ON atestp1 TO regress_user2;
+SET SESSION AUTHORIZATION regress_user2;
+SELECT f2 FROM atestp1; -- ok
+SELECT f2 FROM atestc; -- fail
+DELETE FROM atestp1; -- ok
+DELETE FROM atestc; -- fail
+UPDATE atestp1 SET f1 = 1; -- ok
+UPDATE atestc SET f1 = 1; -- fail
+TRUNCATE atestp1; -- ok
+TRUNCATE atestc; -- fail
+
 -- privileges on functions, languages
 
 -- switch to superuser