On Fri, Dec 27, 2019 at 4:26 AM Tom Lane <t...@sss.pgh.pa.us> wrote:
> Fujii Masao <masao.fu...@gmail.com> writes:
> > My customer reported me that the queries through a partitioned table
> > ignore each partition's SELECT, INSERT, UPDATE, and DELETE privileges,
> > on the other hand, only TRUNCATE privilege specified for each partition
> > is applied. I'm not sure if this behavior is expected or not. But anyway
> > is it better to document that? For example,
>
> > Access privileges may be defined and removed separately for each
> > partition.
> > But note that queries through a partitioned table ignore each
> > partition's
> > SELECT, INSERT, UPDATE and DELETE privileges, and apply only TRUNCATE
> > one.
>
> I believe it's intentional that we only check access privileges on
> the table explicitly named in the query. So I'd say SELECT etc
> are doing the right thing, and if TRUNCATE isn't in step with them
> that's a bug to fix, not something to document.
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,
Amit
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 1c4394abea..43377e24c4 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -298,6 +298,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);
@@ -1575,6 +1576,10 @@ ExecuteTruncate(TruncateStmt *stmt)
}
truncate_check_rel(RelationGetRelid(rel),
rel->rd_rel);
+ /*
+ * We skip checking the child's permission,
because we
+ * already checked the parent's.
+ */
truncate_check_activity(rel);
rels = lappend(rels, rel);
@@ -1660,6 +1665,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);
@@ -1910,7 +1916,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);
/*
@@ -1924,12 +1929,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),
@@ -1939,6 +1938,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
@@ -14799,6 +14814,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..ac5c5e921c 100644
--- a/src/test/regress/expected/privileges.out
+++ b/src/test/regress/expected/privileges.out
@@ -695,6 +695,29 @@ 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 SELECT (f1, fz), UPDATE (fz) ON atestc TO regress_priv_user2;
+REVOKE TRUNCATE 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..757bbc4983 100644
--- a/src/test/regress/sql/privileges.sql
+++ b/src/test/regress/sql/privileges.sql
@@ -446,6 +446,22 @@ 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 SELECT (f1, fz), UPDATE (fz) ON atestc TO regress_priv_user2;
+REVOKE TRUNCATE 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
