change default value of column 'enable_parent', small refactoring of function check_security_policy(), check_security_policy_internal() now takes role as parameter

funbringer · funbringer · commit 2ad0cf0b9e46 · 2016-10-08T16:31:45.000+03:00
diff --git a/expected/pathman_permissions.out b/expected/pathman_permissions.out
@@ -17,7 +17,8 @@ INSERT INTO permissions.user1_table SELECT g, g FROM generate_series(1, 20) as g
 SET ROLE user2;
 SELECT create_range_partitions('permissions.user1_table', 'id', 1, 10, 2);
 NOTICE:  sequence "user1_table_seq" does not exist, skipping
-ERROR:  only the owner or superuser can change partitioning configuration of table "user1_table"
+WARNING:  only the owner or superuser can change partitioning configuration of table "user1_table"
+ERROR:  new row violates row-level security policy for table "pathman_config"
 /* Should be ok */
 SET ROLE user1;
 SELECT create_range_partitions('permissions.user1_table', 'id', 1, 10, 2);
@@ -44,14 +45,16 @@ SELECT * FROM pathman_config_params;
 /* Should fail */
 SET ROLE user2;
 SELECT set_enable_parent('permissions.user1_table', true);
-ERROR:  only the owner or superuser can change partitioning configuration of table "user1_table"
+WARNING:  only the owner or superuser can change partitioning configuration of table "user1_table"
+ERROR:  new row violates row-level security policy for table "pathman_config_params"
 SELECT set_auto('permissions.user1_table', false);
-ERROR:  only the owner or superuser can change partitioning configuration of table "user1_table"
+WARNING:  only the owner or superuser can change partitioning configuration of table "user1_table"
+ERROR:  new row violates row-level security policy for table "pathman_config_params"
 /* Should fail */
 SET ROLE user2;
 DELETE FROM pathman_config
 WHERE partrel = 'permissions.user1_table'::regclass;
-ERROR:  only the owner or superuser can change partitioning configuration of table "user1_table"
+WARNING:  only the owner or superuser can change partitioning configuration of table "user1_table"
 /* No rights to insert, should fail */
 SET ROLE user2;
 INSERT INTO permissions.user1_table (id, a) VALUES (35, 0);
diff --git a/init.sql b/init.sql
@@ -35,7 +35,7 @@ CREATE TABLE IF NOT EXISTS @extschema@.pathman_config (
  */
 CREATE TABLE IF NOT EXISTS @extschema@.pathman_config_params (
 	partrel			REGCLASS NOT NULL PRIMARY KEY,
-	enable_parent	BOOLEAN NOT NULL DEFAULT TRUE,
+	enable_parent	BOOLEAN NOT NULL DEFAULT FALSE,
 	auto			BOOLEAN NOT NULL DEFAULT TRUE,
 	init_callback	REGPROCEDURE NOT NULL DEFAULT 0
 );
diff --git a/src/pl_funcs.c b/src/pl_funcs.c
@@ -538,7 +538,7 @@ build_check_constraint_name_attname(PG_FUNCTION_ARGS)
 		elog(ERROR, "Invalid relation %u", relid);
 
 	if (attnum == InvalidAttrNumber)
-		elog(ERROR, "Relation \"%s\" has no column '%s'",
+		elog(ERROR, "relation \"%s\" has no column \"%s\"",
 			 get_rel_name_or_relid(relid), text_to_cstring(attname));
 
 	result = build_check_constraint_name_internal(relid, attnum);
@@ -870,7 +870,19 @@ invoke_on_partition_created_callback(PG_FUNCTION_ARGS)
 Datum
 check_security_policy(PG_FUNCTION_ARGS)
 {
-	PG_RETURN_BOOL(check_security_policy_internal(PG_GETARG_OID(0)));
+	Oid relid = PG_GETARG_OID(0);
+
+	if (!check_security_policy_internal(relid, GetUserId()))
+	{
+		elog(WARNING, "only the owner or superuser can change "
+					  "partitioning configuration of table \"%s\"",
+			 get_rel_name_or_relid(relid));
+
+		PG_RETURN_BOOL(false);
+	}
+
+	/* Else return TRUE */
+	PG_RETURN_BOOL(true);
 }
 
 
diff --git a/src/utils.c b/src/utils.c
@@ -759,27 +759,28 @@ validate_on_part_init_cb(Oid procid, bool emit_error)
  * better to check user permissions in order to let other users participate.
  */
 bool
-check_security_policy_internal(Oid relid)
+check_security_policy_internal(Oid relid, Oid role)
 {
-	Oid		owner;
+	Oid owner;
 
 	/* Superuser is allowed to do anything */
 	if (superuser())
 		return true;
 
+	/* Fetch the owner */
+	owner = get_rel_owner(relid);
+
 	/*
 	 * Sometimes the relation doesn't exist anymore but there is still
 	 * a record in config. For instance, it happens in DDL event trigger.
 	 * Still we should be able to remove this record.
 	 */
-	if ((owner = get_rel_owner(relid)) == InvalidOid)
+	if (owner == InvalidOid)
 		return true;
 
 	/* Check if current user is the owner of the relation */
-	if (owner != GetUserId())
-		elog(ERROR, "only the owner or superuser can change "
-					"partitioning configuration of table \"%s\"",
-			 get_rel_name_or_relid(relid));
+	if (owner != role)
+		return false;
 
 	return true;
 }
diff --git a/src/utils.h b/src/utils.h
@@ -50,7 +50,7 @@ bool clause_contains_params(Node *clause);
 bool is_date_type_internal(Oid typid);
 bool is_string_type_internal(Oid typid);
 bool validate_on_part_init_cb(Oid procid, bool emit_error);
-bool check_security_policy_internal(Oid relid);
+bool check_security_policy_internal(Oid relid, Oid role);
 
 /*
  * Misc.
