Handle properly default privileges on CAggs

If a default privilege is configured and applied to a given Continuous
Aggregate during it creation just the user view has the ACL properly
configured but the underlying materialization hypertable no leading to
permission errors.

Fixed it by copying the privileges from the user view to the
materialization hypertable during the Continous Aggregate creation.

Fixes timescale#4555

CHANGELOG.md

@@ -13,11 +13,13 @@ accidentally triggering the load of a previous DB version.**
 
 **Bugfixes**
 * #4486 Adding boolean column with default value doesn't work on compressed table
+* #4555 Handle properly default privileges on Continuous Aggregates
 * #4575 Fix use of `get_partition_hash` and `get_partition_for_key` inside an IMMUTABLE function
 
 **Thanks**
 @janko for reporting
 @AlmiS for reporting error on `get_partition_hash` executed inside an IMMUTABLE function
+@michaelkitson for reporting permission errors using default privileges on Continuous Aggregates
 
 ## 2.7.2 (2022-07-26)
 

src/chunk.c

@@ -739,69 +739,7 @@ get_am_name_for_rel(Oid relid)
 static void
 copy_hypertable_acl_to_relid(const Hypertable *ht, Oid owner_id, Oid relid)
 {
-	HeapTuple ht_tuple;
-	bool is_null;
-	Datum acl_datum;
-	Relation class_rel;
-
-	/* We open it here since there is no point in trying to update the tuples
-	 * if we cannot open the Relation catalog table */
-	class_rel = table_open(RelationRelationId, RowExclusiveLock);
-
-	ht_tuple = SearchSysCache1(RELOID, ObjectIdGetDatum(ht->main_table_relid));
-	Assert(HeapTupleIsValid(ht_tuple));
-
-	/* We only bother about setting the chunk ACL if the hypertable ACL is
-	 * non-null */
-	acl_datum = SysCacheGetAttr(RELOID, ht_tuple, Anum_pg_class_relacl, &is_null);
-	if (!is_null)
-	{
-		HeapTuple chunk_tuple, newtuple;
-		Datum new_val[Natts_pg_class] = { 0 };
-		bool new_null[Natts_pg_class] = { false };
-		bool new_repl[Natts_pg_class] = { false };
-		Acl *acl = DatumGetAclP(acl_datum);
-
-		new_repl[Anum_pg_class_relacl - 1] = true;
-		new_val[Anum_pg_class_relacl - 1] = PointerGetDatum(acl);
-
-		/* Find the tuple for the chunk in `pg_class` */
-		chunk_tuple = SearchSysCache1(RELOID, ObjectIdGetDatum(relid));
-		Assert(HeapTupleIsValid(chunk_tuple));
-
-		/* Update the relacl for the chunk tuple to use the acl from the hypertable */
-		newtuple = heap_modify_tuple(chunk_tuple,
-									 RelationGetDescr(class_rel),
-									 new_val,
-									 new_null,
-									 new_repl);
-		CatalogTupleUpdate(class_rel, &newtuple->t_self, newtuple);
-
-		/* We need to update the shared dependencies as well to indicate that
-		 * the chunk is dependent on any roles that the hypertable is
-		 * dependent on. */
-		Oid *newmembers;
-		int nnewmembers = aclmembers(acl, &newmembers);
-
-		/* The list of old members is intentionally empty since we are using
-		 * updateAclDependencies to set the ACL for the chunk. We can use NULL
-		 * because getOidListDiff, which is called from updateAclDependencies,
-		 * can handle that. */
-		updateAclDependencies(RelationRelationId,
-							  relid,
-							  0,
-							  owner_id,
-							  0,
-							  NULL,
-							  nnewmembers,
-							  newmembers);
-
-		heap_freetuple(newtuple);
-		ReleaseSysCache(chunk_tuple);
-	}
-
-	ReleaseSysCache(ht_tuple);
-	table_close(class_rel, RowExclusiveLock);
+	ts_copy_relation_acl(ht->main_table_relid, relid, owner_id);
 }
 
 /*

src/utils.c

@@ -24,6 +24,7 @@
 #include <parser/parse_coerce.h>
 #include <parser/parse_func.h>
 #include <parser/scansup.h>
+#include <utils/acl.h>
 #include <utils/builtins.h>
 #include <utils/catcache.h>
 #include <utils/date.h>
@@ -1197,3 +1198,72 @@ ts_alter_table_with_event_trigger(Oid relid, Node *cmd, List *cmds, bool recurse
 	AlterTableInternal(relid, cmds, recurse);
 	EventTriggerAlterTableEnd();
 }
+
+void
+ts_copy_relation_acl(const Oid source_relid, const Oid target_relid, const Oid owner_id)
+{
+	HeapTuple source_tuple;
+	bool is_null;
+	Datum acl_datum;
+	Relation class_rel;
+
+	/* We open it here since there is no point in trying to update the tuples
+	 * if we cannot open the Relation catalog table */
+	class_rel = table_open(RelationRelationId, RowExclusiveLock);
+
+	source_tuple = SearchSysCache1(RELOID, ObjectIdGetDatum(source_relid));
+	Assert(HeapTupleIsValid(source_tuple));
+
+	/* We only bother about setting the ACL if the source relation ACL is
+	 * non-null */
+	acl_datum = SysCacheGetAttr(RELOID, source_tuple, Anum_pg_class_relacl, &is_null);
+
+	if (!is_null)
+	{
+		HeapTuple target_tuple, newtuple;
+		Datum new_val[Natts_pg_class] = { 0 };
+		bool new_null[Natts_pg_class] = { false };
+		bool new_repl[Natts_pg_class] = { false };
+		Acl *acl = DatumGetAclP(acl_datum);
+
+		new_repl[Anum_pg_class_relacl - 1] = true;
+		new_val[Anum_pg_class_relacl - 1] = PointerGetDatum(acl);
+
+		/* Find the tuple for the target in `pg_class` */
+		target_tuple = SearchSysCache1(RELOID, ObjectIdGetDatum(target_relid));
+		Assert(HeapTupleIsValid(target_tuple));
+
+		/* Update the relacl for the target tuple to use the acl from the source */
+		newtuple = heap_modify_tuple(target_tuple,
+									 RelationGetDescr(class_rel),
+									 new_val,
+									 new_null,
+									 new_repl);
+		CatalogTupleUpdate(class_rel, &newtuple->t_self, newtuple);
+
+		/* We need to update the shared dependencies as well to indicate that
+		 * the target is dependent on any roles that the source is
+		 * dependent on. */
+		Oid *newmembers;
+		int nnewmembers = aclmembers(acl, &newmembers);
+
+		/* The list of old members is intentionally empty since we are using
+		 * updateAclDependencies to set the ACL for the target. We can use NULL
+		 * because getOidListDiff, which is called from updateAclDependencies,
+		 * can handle that. */
+		updateAclDependencies(RelationRelationId,
+							  target_relid,
+							  0,
+							  owner_id,
+							  0,
+							  NULL,
+							  nnewmembers,
+							  newmembers);
+
+		heap_freetuple(newtuple);
+		ReleaseSysCache(target_tuple);
+	}
+
+	ReleaseSysCache(source_tuple);
+	table_close(class_rel, RowExclusiveLock);
+}

src/utils.h

@@ -198,5 +198,7 @@ extern TSDLLEXPORT const char *ts_get_node_name(Node *node);
 extern TSDLLEXPORT int ts_get_relnatts(Oid relid);
 extern TSDLLEXPORT void ts_alter_table_with_event_trigger(Oid relid, Node *cmd, List *cmds,
 														  bool recurse);
+extern TSDLLEXPORT void ts_copy_relation_acl(const Oid source_relid, const Oid target_relid,
+											 const Oid owner_id);
 
 #endif /* TIMESCALEDB_UTILS_H */

tsl/src/continuous_aggs/create.c

@@ -47,6 +47,7 @@
 #include <parser/parse_type.h>
 #include <rewrite/rewriteHandler.h>
 #include <rewrite/rewriteManip.h>
+#include <utils/acl.h>
 #include <utils/rel.h>
 #include <utils/builtins.h>
 #include <utils/catcache.h>
@@ -2257,7 +2258,9 @@ cagg_create(const CreateTableAsStmt *create_stmt, ViewStmt *stmt, Query *panquer
 										   panquery,
 										   materialize_hypertable_id);
 
-	create_view_for_query(final_selquery, stmt->view);
+	/* copy view acl to materialization hypertable */
+	ObjectAddress view_address = create_view_for_query(final_selquery, stmt->view);
+	ts_copy_relation_acl(view_address.objectId, mataddress.objectId, GetUserId());
 
 	/* Step 3: create the internal view with select partialize(..)
 	 */

tsl/test/expected/cagg_permissions.out

@@ -65,8 +65,8 @@ insert into conditions
 select generate_series(0, 50, 10), 'NYC', 55, 75, 40, 70, NULL;
 CALL refresh_continuous_aggregate(' mat_refresh_test', NULL, NULL);
 SELECT id as cagg_job_id FROM _timescaledb_config.bgw_job order by id desc limit 1 \gset
-SELECT format('%I.%I', materialization_hypertable_schema, materialization_hypertable_name ) as materialization_hypertable 
-FROM timescaledb_information.continuous_aggregates 
+SELECT format('%I.%I', materialization_hypertable_schema, materialization_hypertable_name ) as materialization_hypertable
+FROM timescaledb_information.continuous_aggregates
 WHERE view_name = 'mat_refresh_test' \gset
 SELECT mat_hypertable_id FROM _timescaledb_catalog.continuous_agg WHERE user_view_name = 'mat_refresh_test' \gset
 SELECT schema_name as mat_chunk_schema, table_name as mat_chunk_table
@@ -268,3 +268,36 @@ partial_view      | _timescaledb_internal._partial_view_10
 partial_view_perm | {default_perm_user_2=arwdDxt/default_perm_user_2,default_perm_user=adDxt/default_perm_user_2}
 
 \x off
+\c :TEST_DBNAME :ROLE_SUPERUSER
+CREATE SCHEMA test;
+ERROR:  schema "test" already exists
+GRANT USAGE ON SCHEMA "test" TO :ROLE_DEFAULT_PERM_USER;
+ALTER DEFAULT PRIVILEGES IN SCHEMA "test" GRANT SELECT ON TABLES TO :ROLE_DEFAULT_PERM_USER;
+CREATE TABLE test.devices (
+   time TIMESTAMPTZ NOT NULL,
+   device INT,
+   temp DOUBLE PRECISION NULL,
+   PRIMARY KEY(time, device)
+);
+SELECT create_hypertable('test.devices', 'time');
+  create_hypertable  
+---------------------
+ (11,test,devices,t)
+(1 row)
+
+CREATE MATERIALIZED VIEW test.devices_summary
+WITH (timescaledb.continuous)
+AS
+SELECT time_bucket('1 day', time) AS bucket, device, MAX(temp)
+FROM test.devices
+GROUP BY bucket, device
+WITH NO DATA;
+-- check if user view perms has being propagated to the mat-ht
+SELECT user_view_perm IS NOT DISTINCT FROM mat_table_perm
+FROM cagg_info
+WHERE user_view = 'test.devices_summary'::regclass;
+ ?column? 
+----------
+ t
+(1 row)
+

tsl/test/sql/cagg_permissions.sql

@@ -62,8 +62,8 @@ select generate_series(0, 50, 10), 'NYC', 55, 75, 40, 70, NULL;
 CALL refresh_continuous_aggregate(' mat_refresh_test', NULL, NULL);
 
 SELECT id as cagg_job_id FROM _timescaledb_config.bgw_job order by id desc limit 1 \gset
-SELECT format('%I.%I', materialization_hypertable_schema, materialization_hypertable_name ) as materialization_hypertable 
-FROM timescaledb_information.continuous_aggregates 
+SELECT format('%I.%I', materialization_hypertable_schema, materialization_hypertable_name ) as materialization_hypertable
+FROM timescaledb_information.continuous_aggregates
 WHERE view_name = 'mat_refresh_test' \gset
 
 SELECT mat_hypertable_id FROM _timescaledb_catalog.continuous_agg WHERE user_view_name = 'mat_refresh_test' \gset
@@ -223,3 +223,30 @@ SELECT * FROM cagg_info WHERE user_view::text = 'devices_summary';
 REVOKE SELECT, UPDATE ON devices_summary FROM :ROLE_DEFAULT_PERM_USER;
 SELECT * FROM cagg_info WHERE user_view::text = 'devices_summary';
 \x off
+
+\c :TEST_DBNAME :ROLE_SUPERUSER
+CREATE SCHEMA test;
+GRANT USAGE ON SCHEMA "test" TO :ROLE_DEFAULT_PERM_USER;
+ALTER DEFAULT PRIVILEGES IN SCHEMA "test" GRANT SELECT ON TABLES TO :ROLE_DEFAULT_PERM_USER;
+
+CREATE TABLE test.devices (
+   time TIMESTAMPTZ NOT NULL,
+   device INT,
+   temp DOUBLE PRECISION NULL,
+   PRIMARY KEY(time, device)
+);
+
+SELECT create_hypertable('test.devices', 'time');
+
+CREATE MATERIALIZED VIEW test.devices_summary
+WITH (timescaledb.continuous)
+AS
+SELECT time_bucket('1 day', time) AS bucket, device, MAX(temp)
+FROM test.devices
+GROUP BY bucket, device
+WITH NO DATA;
+
+-- check if user view perms has being propagated to the mat-ht
+SELECT user_view_perm IS NOT DISTINCT FROM mat_table_perm
+FROM cagg_info
+WHERE user_view = 'test.devices_summary'::regclass;