Add permission checks to run_job()

There were no permission checks when calling run_job(), so it was
possible to execute any job regardless of who owned it. This commit
adds such checks.

CHANGELOG.md

@@ -27,6 +27,7 @@ accidentally triggering the load of a previous DB version.**
 * #5570 Improve interpolate error message on datatype mismatch
 * #5583 Fix parameterization in DecompressChunk path generation
 * #5602 Fix broken CAgg with JOIN repair function
+* #5615 Add permission checks to run_job()
 
 **Thanks**
 * @kovetskiy and @DZDomi for reporting peformance regression in Realtime Continuous Aggregates

src/bgw/job.c

@@ -974,12 +974,21 @@ ts_bgw_job_check_max_retries(BgwJob *job)
 }
 
 void
-ts_bgw_job_permission_check(BgwJob *job)
+ts_bgw_job_permission_check(BgwJob *job, const char *cmd)
 {
 	if (!has_privs_of_role(GetUserId(), job->fd.owner))
+	{
+		const char *owner_name = GetUserNameFromId(job->fd.owner, false);
+		const char *user_name = GetUserNameFromId(GetUserId(), false);
 		ereport(ERROR,
 				(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
-				 errmsg("insufficient permissions to alter job %d", job->fd.id)));
+				 errmsg("insufficient permissions to %s job %d", cmd, job->fd.id),
+				 errdetail("Job %d is owned by role \"%s\" but user \"%s\" does not belong to that "
+						   "role.",
+						   job->fd.id,
+						   owner_name,
+						   user_name)));
+	}
 }
 
 void

src/bgw/job.h

@@ -47,7 +47,7 @@ extern TSDLLEXPORT int32 ts_bgw_job_insert_relation(
 	Interval *retry_period, Name proc_schema, Name proc_name, Name check_schema, Name check_name,
 	Oid owner, bool scheduled, bool fixed_schedule, int32 hypertable_id, Jsonb *config,
 	TimestampTz initial_start, const char *timezone);
-extern TSDLLEXPORT void ts_bgw_job_permission_check(BgwJob *job);
+extern TSDLLEXPORT void ts_bgw_job_permission_check(BgwJob *job, const char *cmd);
 
 extern TSDLLEXPORT void ts_bgw_job_validate_job_owner(Oid owner);
 

tsl/src/bgw_policy/job_api.c

@@ -263,6 +263,8 @@ job_run(PG_FUNCTION_ARGS)
 	int32 job_id = PG_GETARG_INT32(0);
 	BgwJob *job = find_job(job_id, PG_ARGISNULL(0), false);
 
+	ts_bgw_job_permission_check(job, "run");
+
 	job_execute(job);
 
 	PG_RETURN_VOID();
@@ -326,7 +328,7 @@ job_alter(PG_FUNCTION_ARGS)
 	if (job == NULL)
 		PG_RETURN_NULL();
 
-	ts_bgw_job_permission_check(job);
+	ts_bgw_job_permission_check(job, "alter");
 
 	if (!PG_ARGISNULL(1))
 		job->fd.schedule_interval = *PG_GETARG_INTERVAL_P(1);
@@ -463,7 +465,8 @@ job_alter_set_hypertable_id(PG_FUNCTION_ARGS)
 	BgwJob *job = find_job(job_id, PG_ARGISNULL(0), false /* missing_ok */);
 	if (job == NULL)
 		PG_RETURN_NULL();
-	ts_bgw_job_permission_check(job);
+
+	ts_bgw_job_permission_check(job, "alter");
 
 	if (!PG_ARGISNULL(1))
 	{

tsl/test/expected/bgw_security.out

@@ -0,0 +1,32 @@
+-- This file and its contents are licensed under the Timescale License.
+-- Please see the included NOTICE for copyright information and
+-- LICENSE-TIMESCALE for a copy of the license.
+\set ROLE_ADMIN :TEST_DBNAME _admin
+\c :TEST_DBNAME :ROLE_SUPERUSER
+CREATE ROLE :ROLE_ADMIN;
+GRANT :ROLE_ADMIN TO :ROLE_DEFAULT_PERM_USER;
+\c :TEST_DBNAME :ROLE_SUPERUSER
+CREATE TABLE custom_log (ts integer, msg text);
+GRANT ALL ON custom_log TO PUBLIC;
+CREATE PROCEDURE custom_job(integer, jsonb) AS $$
+  INSERT INTO custom_log values($1, 'custom_job');
+$$ LANGUAGE SQL;
+SELECT add_job('custom_job', '1h') AS job_id \gset
+-- Set the owner of the job to the admin role
+UPDATE _timescaledb_config.bgw_job SET owner = :'ROLE_ADMIN' WHERE id = :job_id;
+SELECT id, proc_name, owner FROM _timescaledb_config.bgw_job WHERE id = :job_id;
+  id  | proc_name  |         owner         
+------+------------+-----------------------
+ 1000 | custom_job | db_bgw_security_admin
+(1 row)
+
+\c :TEST_DBNAME :ROLE_DEFAULT_PERM_USER_2
+-- We should fail to execute the job since we do not own it or belong
+-- to the group that owns it.
+\set ON_ERROR_STOP 0
+CALL run_job(:job_id);
+ERROR:  insufficient permissions to run job 1000
+\set ON_ERROR_STOP 1
+\c :TEST_DBNAME :ROLE_DEFAULT_PERM_USER
+-- This should succeed since the role belongs to the job owner group.
+CALL run_job(:job_id);

tsl/test/sql/CMakeLists.txt

@@ -4,6 +4,7 @@ include(GenerateTestSchedule)
 # so unless you have a good reason, add new test files here.
 set(TEST_FILES
     bgw_custom.sql
+    bgw_security.sql
     bgw_policy.sql
     cagg_errors.sql
     cagg_invalidation.sql

tsl/test/sql/bgw_security.sql

@@ -0,0 +1,38 @@
+-- This file and its contents are licensed under the Timescale License.
+-- Please see the included NOTICE for copyright information and
+-- LICENSE-TIMESCALE for a copy of the license.
+
+\set ROLE_ADMIN :TEST_DBNAME _admin
+\c :TEST_DBNAME :ROLE_SUPERUSER
+
+CREATE ROLE :ROLE_ADMIN;
+GRANT :ROLE_ADMIN TO :ROLE_DEFAULT_PERM_USER;
+
+\c :TEST_DBNAME :ROLE_SUPERUSER
+
+CREATE TABLE custom_log (ts integer, msg text);
+GRANT ALL ON custom_log TO PUBLIC;
+
+CREATE PROCEDURE custom_job(integer, jsonb) AS $$
+  INSERT INTO custom_log values($1, 'custom_job');
+$$ LANGUAGE SQL;
+
+SELECT add_job('custom_job', '1h') AS job_id \gset
+
+-- Set the owner of the job to the admin role
+UPDATE _timescaledb_config.bgw_job SET owner = :'ROLE_ADMIN' WHERE id = :job_id;
+
+SELECT id, proc_name, owner FROM _timescaledb_config.bgw_job WHERE id = :job_id;
+
+\c :TEST_DBNAME :ROLE_DEFAULT_PERM_USER_2
+
+-- We should fail to execute the job since we do not own it or belong
+-- to the group that owns it.
+\set ON_ERROR_STOP 0
+CALL run_job(:job_id);
+\set ON_ERROR_STOP 1
+
+\c :TEST_DBNAME :ROLE_DEFAULT_PERM_USER
+
+-- This should succeed since the role belongs to the job owner group.
+CALL run_job(:job_id);