diff --git a/tsl/src/chunk_copy.c b/tsl/src/chunk_copy.c index c9207b79f..2cb22af3e 100644 --- a/tsl/src/chunk_copy.c +++ b/tsl/src/chunk_copy.c @@ -269,12 +269,6 @@ chunk_copy_setup(ChunkCopy *cc, Oid chunk_relid, const char *src_node, const cha Cache *hcache; MemoryContext old, mcxt; - if (!superuser() && !has_rolreplication(GetUserId())) - ereport(ERROR, - (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), - (errmsg( - "must be superuser or replication role to copy/move chunk to data node")))); - if (dist_util_membership() != DIST_MEMBER_ACCESS_NODE) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), @@ -300,9 +294,16 @@ chunk_copy_setup(ChunkCopy *cc, Oid chunk_relid, const char *src_node, const cha CACHE_FLAG_NONE, &hcache); + if (!superuser() && !has_rolreplication(GetUserId()) && + (ts_rel_get_owner(ht->main_table_relid) != GetUserId())) + ereport(ERROR, + (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), + (errmsg("must be superuser, replication role, or hypertable owner to copy/move " + "chunk to data node")))); + /* - * We have already checked above for superuser or replication perms. There's no - * need to check if the user has "ownership" on the hypertable now. + * We have already checked above for superuser/replication/owner perms. There's no + * need to check again if the user has "ownership" on the hypertable now. */ if (!hypertable_is_distributed(ht)) ereport(ERROR, @@ -1051,6 +1052,7 @@ chunk_copy_execute(ChunkCopy *cc) { int sec_ctx; Oid saved_uid; + bool is_superuser; /* * A chunk copy/move operation involves a lot of stages. Many of these @@ -1062,13 +1064,22 @@ chunk_copy_execute(ChunkCopy *cc) * * The move_chunk/copy_chunk functions can only be invoked by superuser * or REPLICATION users. To keep things manageable, we switch to the - * bootstrap superuser for each stage of the execution. Care is taken to - * create the chunks with original hypertable ownership. + * bootstrap superuser (or if the current logged in user is a superuser) + * for each stage of the execution. Care is taken to create the chunks + * with original hypertable ownership. */ SPI_start_transaction(); - GetUserIdAndSecContext(&saved_uid, &sec_ctx); - if (BOOTSTRAP_SUPERUSERID != saved_uid) + is_superuser = superuser(); + + /* + * Note that if the current logged in user is superuser then we use those + * credentials instead of implicit bootstrap superuser + */ + if (!is_superuser) + { + GetUserIdAndSecContext(&saved_uid, &sec_ctx); SetUserIdAndSecContext(BOOTSTRAP_SUPERUSERID, sec_ctx | SECURITY_LOCAL_USERID_CHANGE); + } cc->stage = stage; @@ -1080,7 +1091,7 @@ chunk_copy_execute(ChunkCopy *cc) DEBUG_ERROR_INJECTION(stage->name); - if (BOOTSTRAP_SUPERUSERID != saved_uid) + if (!is_superuser) SetUserIdAndSecContext(saved_uid, sec_ctx); SPI_commit(); } @@ -1223,6 +1234,7 @@ chunk_copy_cleanup_internal(ChunkCopy *cc, int stage_idx) { int sec_ctx; Oid saved_uid; + bool is_superuser = superuser(); /* * A chunk copy/move cleanup operation involves a lot of stages. Many of these @@ -1236,9 +1248,16 @@ chunk_copy_cleanup_internal(ChunkCopy *cc, int stage_idx) * bootstrap superuser for each stage of the execution. */ SPI_start_transaction(); - GetUserIdAndSecContext(&saved_uid, &sec_ctx); - if (BOOTSTRAP_SUPERUSERID != saved_uid) + + /* + * Note that if the current logged in user is superuser then we use those + * credentials instead of implicit bootstrap superuser + */ + if (!is_superuser) + { + GetUserIdAndSecContext(&saved_uid, &sec_ctx); SetUserIdAndSecContext(BOOTSTRAP_SUPERUSERID, sec_ctx | SECURITY_LOCAL_USERID_CHANGE); + } cc->stage = &chunk_copy_stages[stage_idx]; if (cc->stage->function_cleanup) @@ -1250,7 +1269,7 @@ chunk_copy_cleanup_internal(ChunkCopy *cc, int stage_idx) else first = false; - if (BOOTSTRAP_SUPERUSERID != saved_uid) + if (!is_superuser) SetUserIdAndSecContext(saved_uid, sec_ctx); SPI_commit(); } while (--stage_idx >= 0); @@ -1265,12 +1284,6 @@ chunk_copy_cleanup(const char *operation_id) bool found = false; int stage_idx; - if (!superuser() && !has_rolreplication(GetUserId())) - ereport(ERROR, - (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), - (errmsg( - "must be superuser or replication role to cleanup a chunk copy operation")))); - if (dist_util_membership() != DIST_MEMBER_ACCESS_NODE) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), @@ -1302,6 +1315,13 @@ chunk_copy_cleanup(const char *operation_id) } } + if (!superuser() && !has_rolreplication(GetUserId()) && + (ts_rel_get_owner(cc->chunk->hypertable_relid) != GetUserId())) + ereport(ERROR, + (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), + (errmsg("must be superuser, replication role, or hypertable owner to cleanup a " + "chunk copy/move operation")))); + /* should always find an entry, add ereport to quell compiler warning */ Assert(found == true); if (!found) diff --git a/tsl/test/expected/dist_move_chunk.out b/tsl/test/expected/dist_move_chunk.out index 92982a939..83cf01f9f 100644 --- a/tsl/test/expected/dist_move_chunk.out +++ b/tsl/test/expected/dist_move_chunk.out @@ -133,7 +133,7 @@ ROLLBACK; SET ROLE :ROLE_DEFAULT_PERM_USER; \set ON_ERROR_STOP 0 CALL timescaledb_experimental.move_chunk(chunk=>'_timescaledb_internal._dist_hyper_1_1_chunk', source_node=> 'data_node_1', destination_node => 'data_node_2'); -ERROR: must be superuser or replication role to copy/move chunk to data node +ERROR: must be superuser, replication role, or hypertable owner to copy/move chunk to data node \set ON_ERROR_STOP 1 SET ROLE :ROLE_1; -- can't run copy/move chunk on a data node diff --git a/tsl/test/t/002_chunk_copy_move.pl b/tsl/test/t/002_chunk_copy_move.pl index 52de60490..9f4e6729a 100644 --- a/tsl/test/t/002_chunk_copy_move.pl +++ b/tsl/test/t/002_chunk_copy_move.pl @@ -8,7 +8,7 @@ use warnings; use AccessNode; use DataNode; use TestLib; -use Test::More tests => 286; +use Test::More tests => 287; #Initialize all the multi-node instances my $an = AccessNode->create('an'); @@ -18,10 +18,18 @@ my $dn2 = DataNode->create('dn2', allows_streaming => 'logical'); $an->add_data_node($dn1); $an->add_data_node($dn2); +$an->safe_psql('postgres', + "GRANT USAGE ON FOREIGN SERVER dn1, dn2 TO PUBLIC;"); + +for my $node ($an, $dn1, $dn2) +{ + $node->safe_psql('postgres', "CREATE ROLE htowner LOGIN"); +} #Create few distributed hypertables with default and specified schema names and insert a few rows $an->safe_psql( 'postgres', qq[ + SET ROLE htowner; CREATE TABLE test(time timestamp NOT NULL, device int, temp float); SELECT create_distributed_hypertable('test', 'time', 'device', 3); INSERT INTO test SELECT t, (abs(timestamp_hash(t::timestamp)) % 10) + 1, 0.10 FROM generate_series('2018-03-02 1:00'::TIMESTAMPTZ, '2018-03-08 1:00', '1 hour') t; @@ -65,7 +73,7 @@ while ($curr_index < $arrSize) #We provide the operation id ourselves $operation_id = "ts_cloud_" . $curr_index . "_1"; ($ret, $stdout, $stderr) = $an->psql('postgres', - "SELECT error_injection_on('$stages[$curr_index]'); CALL timescaledb_experimental.move_chunk(chunk=>'_timescaledb_internal._dist_hyper_1_1_chunk', source_node=> 'dn1', destination_node => 'dn2', operation_id => '$operation_id');" + "SELECT error_injection_on('$stages[$curr_index]'); SET ROLE htowner; CALL timescaledb_experimental.move_chunk(chunk=>'_timescaledb_internal._dist_hyper_1_1_chunk', source_node=> 'dn1', destination_node => 'dn2', operation_id => '$operation_id');" ); is($ret, 3, "move_chunk fails as expected in stage '$stages[$curr_index]'"); @@ -77,7 +85,7 @@ while ($curr_index < $arrSize) #The earlier debug error point gets released automatically since it's a session lock #Call the cleanup procedure to make things right $an->safe_psql('postgres', - "CALL timescaledb_experimental.cleanup_copy_chunk_operation(operation_id=>'$operation_id');" + "SET ROLE htowner; CALL timescaledb_experimental.cleanup_copy_chunk_operation(operation_id=>'$operation_id');" ); #Check chunk state is as before the move @@ -103,7 +111,7 @@ for my $node ($an, $dn1, $dn2) like( $stderr, - qr/must be superuser or replication role to copy\/move chunk to data node/, + qr/must be superuser, replication role, or hypertable owner to copy\/move chunk to data node/, 'Expected failure due to no credentials'); #Provide REPLICATION creds to this user now @@ -141,6 +149,17 @@ $an->psql_is( "ts_copy_1_1|complete|dn1|dn2|t", "AN catalog is as expected"); +# Cleanup with a role which doesn't have superuser, replication or ownernership should fail +$an->safe_psql('postgres', "CREATE ROLE testrole2 LOGIN;"); +($ret, $stdout, $stderr) = $an->psql('postgres', + "SET ROLE testrole2; CALL timescaledb_experimental.move_chunk(chunk=>'_timescaledb_internal._dist_hyper_1_1_chunk', source_node=> 'dn1', destination_node => 'dn2')" +); + +like( + $stderr, + qr/must be superuser, replication role, or hypertable owner to copy\/move chunk to data node/, + 'Expected failure due to no credentials'); +# #Run cleanup on this operstion. It should just delete the catalog entry since the #activity has completed successfully. Rest of the checks below should succeed #Try this cleanup as the REPLICATION user