On 2020/01/23 22:14, Fujii Masao wrote:
On 2020/01/22 16:54, Amit Langote wrote:
Fujii-san,
Thanks for taking a look.
On Fri, Jan 10, 2020 at 10:29 AM Fujii Masao <masao.fu...@gmail.com>
wrote:
On Tue, Jan 7, 2020 at 5:15 PM Amit Langote <amitlangot...@gmail.com>
wrote:
I tend to agree that TRUNCATE's permission model for inheritance
should be consistent with that for the other commands. How about the
attached patch toward that end?
Thanks for the patch!
The patch basically looks good to me.
+GRANT SELECT (f1, fz), UPDATE (fz) ON atestc TO regress_priv_user2;
+REVOKE TRUNCATE ON atestc FROM regress_priv_user2;
These seem not to be necessary for the test.
You're right. Removed in the attached updated patch.
Thanks for updating the patch! Barring any objection,
I will commit this fix and backport it to all supported versions.
Attached are the back-port versions of the patches.
- patch for master and v12
0001-Don-t-check-child-s-TRUNCATE-privilege-when-truncate-fujii-pg12-13.patch
- patch for v11
0001-Don-t-check-child-s-TRUNCATE-privilege-when-truncate-fujii-pg11.patch
- patch for v10
0001-Don-t-check-child-s-TRUNCATE-privilege-when-truncate-fujii-pg10.patch
- patch for v9.6
0001-Don-t-check-child-s-TRUNCATE-privilege-when-truncate-fujii-pg96.patch
- patch for v9.5 and v9.4
0001-Don-t-check-child-s-TRUNCATE-privilege-when-truncate-fujii-pg94-95.patch
The patch for master branch separates truncate_check_activity() into two
functions, but in v11 or before, truncate_check_activity() didn't exist and
its code was in truncate_check_rel(). So I had to write the back-port
version
of the patch for the previous versions and separate truncate_check_rel()
into three functions, i.e., truncate_check_rel(),
truncate_check_activity() and
truncate_check_perms().
Also the names of users that the regression test for privileges use were
different between PostgreSQL versions. This is another reason
why I had to write several back-port versions of the patches.
Regards,
--
Fujii Masao
NTT DATA CORPORATION
Advanced Platform Technology Group
Research and Development Headquarters
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 1a005760d8..204d25aeb3 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -291,7 +291,9 @@ struct DropRelationCallbackState
#define child_dependency_type(child_is_partition) \
((child_is_partition) ? DEPENDENCY_AUTO : DEPENDENCY_NORMAL)
-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,
bool is_partition, List **supOids, List
**supconstr,
int *supOidCount);
@@ -1242,7 +1244,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);
@@ -1278,7 +1284,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);
}
@@ -1317,7 +1331,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);
}
@@ -1530,35 +1546,50 @@ 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 and partitioned tables
(although,
* the latter are only being included here for the following checks; no
* physical truncation will occur in their case.)
*/
- if (rel->rd_rel->relkind != RELKIND_RELATION &&
- rel->rd_rel->relkind != RELKIND_PARTITIONED_TABLE)
+ if (reltuple->relkind != RELKIND_RELATION &&
+ reltuple->relkind != RELKIND_PARTITIONED_TABLE)
ereport(ERROR,
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
- errmsg("\"%s\" is not a table",
- RelationGetRelationName(rel))));
-
- /* Permissions checks */
- aclresult = pg_class_aclcheck(RelationGetRelid(rel), GetUserId(),
- ACL_TRUNCATE);
- if (aclresult != ACLCHECK_OK)
- aclcheck_error(aclresult, ACL_KIND_CLASS,
- RelationGetRelationName(rel));
+ errmsg("\"%s\" is not a table", relname)));
- 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 dacc984505..e242137549 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 4263315a2d..973dde8143 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
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 38386fb9cf..9b0dc3fd10 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -300,7 +300,9 @@ struct DropRelationCallbackState
#define child_dependency_type(child_is_partition) \
((child_is_partition) ? DEPENDENCY_AUTO : DEPENDENCY_NORMAL)
-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,
bool is_partition, List **supOids, List
**supconstr,
int *supOidCount);
@@ -1381,7 +1383,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);
/* Log this relation only if needed for logical decoding */
@@ -1420,7 +1426,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);
/* Log this relation only if needed for logical
decoding */
@@ -1503,7 +1517,9 @@ ExecuteTruncateGuts(List *explicit_rels, List *relids,
List *relids_logged,
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);
/* Log this relation only if needed for logical
decoding */
@@ -1759,35 +1775,51 @@ ExecuteTruncateGuts(List *explicit_rels, List *relids,
List *relids_logged,
* 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 and partitioned tables
(although,
* the latter are only being included here for the following checks; no
* physical truncation will occur in their case.)
*/
- if (rel->rd_rel->relkind != RELKIND_RELATION &&
- rel->rd_rel->relkind != RELKIND_PARTITIONED_TABLE)
+ if (reltuple->relkind != RELKIND_RELATION &&
+ reltuple->relkind != RELKIND_PARTITIONED_TABLE)
ereport(ERROR,
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
- errmsg("\"%s\" is not a table",
- RelationGetRelationName(rel))));
-
- /* Permissions checks */
- aclresult = pg_class_aclcheck(RelationGetRelid(rel), GetUserId(),
- ACL_TRUNCATE);
- if (aclresult != ACLCHECK_OK)
- aclcheck_error(aclresult,
get_relkind_objtype(rel->rd_rel->relkind),
- RelationGetRelationName(rel));
+ errmsg("\"%s\" is not a table", relname)));
- 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,
get_relkind_objtype(reltuple->relkind),
+ 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 a8346e1717..6221601c5d 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_priv_user1;
+REVOKE ALL ON atestc FROM regress_priv_user2;
+GRANT ALL ON atestp1 TO regress_priv_user2;
+SET SESSION AUTHORIZATION regress_priv_user2;
+SELECT f2 FROM atestp1; -- ok
+ f2
+----
+(0 rows)
+
+SELECT f2 FROM atestc; -- fail
+ERROR: permission denied for table atestc
+DELETE FROM atestp1; -- ok
+DELETE FROM atestc; -- fail
+ERROR: permission denied for table atestc
+UPDATE atestp1 SET f1 = 1; -- ok
+UPDATE atestc SET f1 = 1; -- fail
+ERROR: permission denied for table atestc
+TRUNCATE atestp1; -- ok
+TRUNCATE atestc; -- fail
+ERROR: permission denied for table 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 c1e42d1be2..c25157b32d 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_priv_user1;
+REVOKE ALL ON atestc FROM regress_priv_user2;
+GRANT ALL ON atestp1 TO regress_priv_user2;
+SET SESSION AUTHORIZATION regress_priv_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
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 7c23968f2d..9afdbc5d53 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -304,6 +304,7 @@ struct DropRelationCallbackState
((child_is_partition) ? DEPENDENCY_AUTO : DEPENDENCY_NORMAL)
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 void RangeVarCallbackForTruncate(const RangeVar *relation,
Oid relId, Oid oldRelId, void *arg);
@@ -1615,6 +1616,12 @@ ExecuteTruncate(TruncateStmt *stmt)
continue;
}
+ /*
+ * 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);
@@ -1701,6 +1708,7 @@ ExecuteTruncateGuts(List *explicit_rels, List *relids,
List *relids_logged,
(errmsg("truncate cascades to
table \"%s\"",
RelationGetRelationName(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);
@@ -1951,7 +1959,6 @@ ExecuteTruncateGuts(List *explicit_rels, List *relids,
List *relids_logged,
static void
truncate_check_rel(Oid relid, Form_pg_class reltuple)
{
- AclResult aclresult;
char *relname = NameStr(reltuple->relname);
/*
@@ -1965,12 +1972,6 @@ truncate_check_rel(Oid relid, Form_pg_class reltuple)
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
errmsg("\"%s\" is not a table", relname)));
- /* Permissions checks */
- aclresult = pg_class_aclcheck(relid, GetUserId(), ACL_TRUNCATE);
- if (aclresult != ACLCHECK_OK)
- aclcheck_error(aclresult,
get_relkind_objtype(reltuple->relkind),
- relname);
-
if (!allowSystemTableMods && IsSystemClass(relid, reltuple))
ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
@@ -1980,6 +1981,22 @@ truncate_check_rel(Oid relid, Form_pg_class reltuple)
InvokeObjectTruncateHook(relid);
}
+/*
+ * 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,
get_relkind_objtype(reltuple->relkind),
+ relname);
+}
+
/*
* Set of extra sanity checks to check if a given relation is safe to
* truncate. This is split with truncate_check_rel() as
@@ -15287,6 +15304,7 @@ RangeVarCallbackForTruncate(const RangeVar *relation,
elog(ERROR, "cache lookup failed for relation %u", relId);
truncate_check_rel(relId, (Form_pg_class) GETSTRUCT(tuple));
+ truncate_check_perms(relId, (Form_pg_class) GETSTRUCT(tuple));
ReleaseSysCache(tuple);
}
diff --git a/src/test/regress/expected/privileges.out
b/src/test/regress/expected/privileges.out
index 0ddbd8e89f..bc8e198097 100644
--- a/src/test/regress/expected/privileges.out
+++ b/src/test/regress/expected/privileges.out
@@ -695,6 +695,27 @@ SELECT tableoid FROM atestp2; -- ok
----------
(0 rows)
+-- child's permissions do not apply when operating on parent
+SET SESSION AUTHORIZATION regress_priv_user1;
+REVOKE ALL ON atestc FROM regress_priv_user2;
+GRANT ALL ON atestp1 TO regress_priv_user2;
+SET SESSION AUTHORIZATION regress_priv_user2;
+SELECT f2 FROM atestp1; -- ok
+ f2
+----
+(0 rows)
+
+SELECT f2 FROM atestc; -- fail
+ERROR: permission denied for table atestc
+DELETE FROM atestp1; -- ok
+DELETE FROM atestc; -- fail
+ERROR: permission denied for table atestc
+UPDATE atestp1 SET f1 = 1; -- ok
+UPDATE atestc SET f1 = 1; -- fail
+ERROR: permission denied for table atestc
+TRUNCATE atestp1; -- ok
+TRUNCATE atestc; -- fail
+ERROR: permission denied for table 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 f15d1f3773..dfe2603fe2 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 tableoid FROM atestp2; -- ok
+-- child's permissions do not apply when operating on parent
+SET SESSION AUTHORIZATION regress_priv_user1;
+REVOKE ALL ON atestc FROM regress_priv_user2;
+GRANT ALL ON atestp1 TO regress_priv_user2;
+SET SESSION AUTHORIZATION regress_priv_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
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 7fcc3db14c..5f46aaa9da 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -269,7 +269,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);
@@ -1046,7 +1048,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);
@@ -1082,7 +1088,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);
}
@@ -1116,7 +1130,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);
}
@@ -1324,30 +1340,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 2d92698c4d..41645ad6cb 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 regressuser1;
+REVOKE ALL ON atestc FROM regressuser2;
+GRANT ALL ON atestp1 TO regressuser2;
+SET SESSION AUTHORIZATION regressuser2;
+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 3ad2e39930..33955d559c 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 regressuser1;
+REVOKE ALL ON atestc FROM regressuser2;
+GRANT ALL ON atestp1 TO regressuser2;
+SET SESSION AUTHORIZATION regressuser2;
+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
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
