Improve access rights: KILL_MUTATION deleted, rights for corresponding ALTER commands are checked instead.

This commit is contained in:
Vitaly Baranov 2020-03-06 20:13:28 +03:00
parent c1f5f8bc89
commit f1e9e3dec0
8 changed files with 189 additions and 168 deletions

View File

@ -349,9 +349,7 @@ private:
ext::push_back(all, std::move(optimize));
auto kill_query = std::make_unique<Node>("KILL QUERY", next_flag++, GLOBAL);
auto kill_mutation = std::make_unique<Node>("KILL MUTATION", next_flag++, TABLE);
auto kill = std::make_unique<Node>("KILL", std::move(kill_query), std::move(kill_mutation));
ext::push_back(all, std::move(kill));
ext::push_back(all, std::move(kill_query));
auto create_user = std::make_unique<Node>("CREATE USER", next_flag++, GLOBAL);
auto alter_user = std::make_unique<Node>("ALTER USER", next_flag++, GLOBAL);

View File

@ -436,7 +436,7 @@ boost::shared_ptr<const AccessRights> AccessRightsContext::calculateResultAccess
| AccessType::DROP_ROLE | AccessType::DROP_POLICY | AccessType::DROP_QUOTA | AccessType::ROLE_ADMIN;
if (readonly_)
result.revoke(write_table_access | all_dcl | AccessType::SYSTEM | AccessType::KILL);
result.revoke(write_table_access | all_dcl | AccessType::SYSTEM | AccessType::KILL_QUERY);
if (readonly_ || !allow_ddl_)
result.revoke(table_and_dictionary_ddl);

View File

@ -79,8 +79,6 @@ enum class AccessType
OPTIMIZE, /// allows to execute OPTIMIZE TABLE
KILL_QUERY, /// allows to kill a query started by another user (anyone can kill his own queries)
KILL_MUTATION, /// allows to kill a mutation
KILL, /// allows to execute KILL {MUTATION|QUERY}
CREATE_USER,
ALTER_USER,
@ -244,8 +242,6 @@ namespace impl
ACCESS_TYPE_TO_KEYWORD_CASE(OPTIMIZE);
ACCESS_TYPE_TO_KEYWORD_CASE(KILL_QUERY);
ACCESS_TYPE_TO_KEYWORD_CASE(KILL_MUTATION);
ACCESS_TYPE_TO_KEYWORD_CASE(KILL);
ACCESS_TYPE_TO_KEYWORD_CASE(CREATE_USER);
ACCESS_TYPE_TO_KEYWORD_CASE(ALTER_USER);

View File

@ -13,7 +13,7 @@
#include <Storages/LiveView/StorageLiveView.h>
#include <Access/AccessRightsElement.h>
#include <Common/typeid_cast.h>
#include <boost/range/algorithm_ext/push_back.hpp>
#include <algorithm>
@ -125,155 +125,162 @@ AccessRightsElements InterpreterAlterQuery::getRequiredAccess() const
{
AccessRightsElements required_access;
const auto & alter = query_ptr->as<ASTAlterQuery &>();
for (ASTAlterCommand * command_ast : alter.command_list->commands)
{
auto column_name = [&]() -> String { return getIdentifierName(command_ast->column); };
auto column_name_from_col_decl = [&]() -> std::string_view { return command_ast->col_decl->as<ASTColumnDeclaration &>().name; };
auto column_names_from_update_assignments = [&]() -> std::vector<std::string_view>
{
std::vector<std::string_view> column_names;
for (const ASTPtr & assignment_ast : command_ast->update_assignments->children)
column_names.emplace_back(assignment_ast->as<const ASTAssignment &>().column_name);
return column_names;
};
for (ASTAlterCommand * command : alter.command_list->commands)
boost::range::push_back(required_access, getRequiredAccessForCommand(*command, alter.database, alter.table));
return required_access;
}
switch (command_ast->type)
AccessRightsElements InterpreterAlterQuery::getRequiredAccessForCommand(const ASTAlterCommand & command, const String & database, const String & table)
{
AccessRightsElements required_access;
auto column_name = [&]() -> String { return getIdentifierName(command.column); };
auto column_name_from_col_decl = [&]() -> std::string_view { return command.col_decl->as<ASTColumnDeclaration &>().name; };
auto column_names_from_update_assignments = [&]() -> std::vector<std::string_view>
{
std::vector<std::string_view> column_names;
for (const ASTPtr & assignment_ast : command.update_assignments->children)
column_names.emplace_back(assignment_ast->as<const ASTAssignment &>().column_name);
return column_names;
};
switch (command.type)
{
case ASTAlterCommand::UPDATE:
{
case ASTAlterCommand::UPDATE:
{
required_access.emplace_back(AccessType::UPDATE, alter.database, alter.table, column_names_from_update_assignments());
break;
}
case ASTAlterCommand::DELETE:
{
required_access.emplace_back(AccessType::DELETE, alter.database, alter.table);
break;
}
case ASTAlterCommand::ADD_COLUMN:
{
required_access.emplace_back(AccessType::ADD_COLUMN, alter.database, alter.table, column_name_from_col_decl());
break;
}
case ASTAlterCommand::DROP_COLUMN:
{
if (command_ast->clear_column)
required_access.emplace_back(AccessType::CLEAR_COLUMN, alter.database, alter.table, column_name());
else
required_access.emplace_back(AccessType::DROP_COLUMN, alter.database, alter.table, column_name());
break;
}
case ASTAlterCommand::MODIFY_COLUMN:
{
required_access.emplace_back(AccessType::MODIFY_COLUMN, alter.database, alter.table, column_name_from_col_decl());
break;
}
case ASTAlterCommand::COMMENT_COLUMN:
{
required_access.emplace_back(AccessType::COMMENT_COLUMN, alter.database, alter.table, column_name());
break;
}
case ASTAlterCommand::MODIFY_ORDER_BY:
{
required_access.emplace_back(AccessType::ALTER_ORDER_BY, alter.database, alter.table);
break;
}
case ASTAlterCommand::ADD_INDEX:
{
required_access.emplace_back(AccessType::ADD_INDEX, alter.database, alter.table);
break;
}
case ASTAlterCommand::DROP_INDEX:
{
if (command_ast->clear_index)
required_access.emplace_back(AccessType::CLEAR_INDEX, alter.database, alter.table);
else
required_access.emplace_back(AccessType::DROP_INDEX, alter.database, alter.table);
break;
}
case ASTAlterCommand::MATERIALIZE_INDEX:
{
required_access.emplace_back(AccessType::MATERIALIZE_INDEX, alter.database, alter.table);
break;
}
case ASTAlterCommand::ADD_CONSTRAINT:
{
required_access.emplace_back(AccessType::ADD_CONSTRAINT, alter.database, alter.table);
break;
}
case ASTAlterCommand::DROP_CONSTRAINT:
{
required_access.emplace_back(AccessType::DROP_CONSTRAINT, alter.database, alter.table);
break;
}
case ASTAlterCommand::MODIFY_TTL:
{
required_access.emplace_back(AccessType::MODIFY_TTL, alter.database, alter.table);
break;
}
case ASTAlterCommand::MATERIALIZE_TTL:
{
required_access.emplace_back(AccessType::MATERIALIZE_TTL, alter.database, alter.table);
break;
}
case ASTAlterCommand::MODIFY_SETTING:
{
required_access.emplace_back(AccessType::MODIFY_SETTING, alter.database, alter.table);
break;
}
case ASTAlterCommand::ATTACH_PARTITION:
{
required_access.emplace_back(AccessType::INSERT, alter.database, alter.table);
break;
}
case ASTAlterCommand::DROP_PARTITION: [[fallthrough]];
case ASTAlterCommand::DROP_DETACHED_PARTITION:
{
required_access.emplace_back(AccessType::DELETE, alter.database, alter.table);
break;
}
case ASTAlterCommand::MOVE_PARTITION:
{
if ((command_ast->move_destination_type == PartDestinationType::DISK)
|| (command_ast->move_destination_type == PartDestinationType::VOLUME))
{
required_access.emplace_back(AccessType::MOVE_PARTITION, alter.database, alter.table);
}
else if (command_ast->move_destination_type == PartDestinationType::TABLE)
{
required_access.emplace_back(AccessType::SELECT | AccessType::DELETE, alter.database, alter.table);
required_access.emplace_back(AccessType::INSERT, command_ast->to_database, command_ast->to_table);
}
break;
}
case ASTAlterCommand::REPLACE_PARTITION:
{
required_access.emplace_back(AccessType::SELECT, command_ast->from_database, command_ast->from_table);
required_access.emplace_back(AccessType::DELETE | AccessType::INSERT, alter.database, alter.table);
break;
}
case ASTAlterCommand::FETCH_PARTITION:
{
required_access.emplace_back(AccessType::FETCH_PARTITION, alter.database, alter.table);
break;
}
case ASTAlterCommand::FREEZE_PARTITION: [[fallthrough]];
case ASTAlterCommand::FREEZE_ALL:
{
required_access.emplace_back(AccessType::FREEZE_PARTITION, alter.database, alter.table);
break;
}
case ASTAlterCommand::MODIFY_QUERY:
{
required_access.emplace_back(AccessType::MODIFY_VIEW_QUERY, alter.database, alter.table);
break;
}
case ASTAlterCommand::LIVE_VIEW_REFRESH:
{
required_access.emplace_back(AccessType::REFRESH_VIEW, alter.database, alter.table);
break;
}
case ASTAlterCommand::NO_TYPE: break;
required_access.emplace_back(AccessType::UPDATE, database, table, column_names_from_update_assignments());
break;
}
case ASTAlterCommand::DELETE:
{
required_access.emplace_back(AccessType::DELETE, database, table);
break;
}
case ASTAlterCommand::ADD_COLUMN:
{
required_access.emplace_back(AccessType::ADD_COLUMN, database, table, column_name_from_col_decl());
break;
}
case ASTAlterCommand::DROP_COLUMN:
{
if (command.clear_column)
required_access.emplace_back(AccessType::CLEAR_COLUMN, database, table, column_name());
else
required_access.emplace_back(AccessType::DROP_COLUMN, database, table, column_name());
break;
}
case ASTAlterCommand::MODIFY_COLUMN:
{
required_access.emplace_back(AccessType::MODIFY_COLUMN, database, table, column_name_from_col_decl());
break;
}
case ASTAlterCommand::COMMENT_COLUMN:
{
required_access.emplace_back(AccessType::COMMENT_COLUMN, database, table, column_name());
break;
}
case ASTAlterCommand::MODIFY_ORDER_BY:
{
required_access.emplace_back(AccessType::ALTER_ORDER_BY, database, table);
break;
}
case ASTAlterCommand::ADD_INDEX:
{
required_access.emplace_back(AccessType::ADD_INDEX, database, table);
break;
}
case ASTAlterCommand::DROP_INDEX:
{
if (command.clear_index)
required_access.emplace_back(AccessType::CLEAR_INDEX, database, table);
else
required_access.emplace_back(AccessType::DROP_INDEX, database, table);
break;
}
case ASTAlterCommand::MATERIALIZE_INDEX:
{
required_access.emplace_back(AccessType::MATERIALIZE_INDEX, database, table);
break;
}
case ASTAlterCommand::ADD_CONSTRAINT:
{
required_access.emplace_back(AccessType::ADD_CONSTRAINT, database, table);
break;
}
case ASTAlterCommand::DROP_CONSTRAINT:
{
required_access.emplace_back(AccessType::DROP_CONSTRAINT, database, table);
break;
}
case ASTAlterCommand::MODIFY_TTL:
{
required_access.emplace_back(AccessType::MODIFY_TTL, database, table);
break;
}
case ASTAlterCommand::MATERIALIZE_TTL:
{
required_access.emplace_back(AccessType::MATERIALIZE_TTL, database, table);
break;
}
case ASTAlterCommand::MODIFY_SETTING:
{
required_access.emplace_back(AccessType::MODIFY_SETTING, database, table);
break;
}
case ASTAlterCommand::ATTACH_PARTITION:
{
required_access.emplace_back(AccessType::INSERT, database, table);
break;
}
case ASTAlterCommand::DROP_PARTITION: [[fallthrough]];
case ASTAlterCommand::DROP_DETACHED_PARTITION:
{
required_access.emplace_back(AccessType::DELETE, database, table);
break;
}
case ASTAlterCommand::MOVE_PARTITION:
{
if ((command.move_destination_type == PartDestinationType::DISK)
|| (command.move_destination_type == PartDestinationType::VOLUME))
{
required_access.emplace_back(AccessType::MOVE_PARTITION, database, table);
}
else if (command.move_destination_type == PartDestinationType::TABLE)
{
required_access.emplace_back(AccessType::SELECT | AccessType::DELETE, database, table);
required_access.emplace_back(AccessType::INSERT, command.to_database, command.to_table);
}
break;
}
case ASTAlterCommand::REPLACE_PARTITION:
{
required_access.emplace_back(AccessType::SELECT, command.from_database, command.from_table);
required_access.emplace_back(AccessType::DELETE | AccessType::INSERT, database, table);
break;
}
case ASTAlterCommand::FETCH_PARTITION:
{
required_access.emplace_back(AccessType::FETCH_PARTITION, database, table);
break;
}
case ASTAlterCommand::FREEZE_PARTITION: [[fallthrough]];
case ASTAlterCommand::FREEZE_ALL:
{
required_access.emplace_back(AccessType::FREEZE_PARTITION, database, table);
break;
}
case ASTAlterCommand::MODIFY_QUERY:
{
required_access.emplace_back(AccessType::MODIFY_VIEW_QUERY, database, table);
break;
}
case ASTAlterCommand::LIVE_VIEW_REFRESH:
{
required_access.emplace_back(AccessType::REFRESH_VIEW, database, table);
break;
}
case ASTAlterCommand::NO_TYPE: break;
}
return required_access;

View File

@ -8,6 +8,7 @@ namespace DB
{
class Context;
class AccessRightsElements;
class ASTAlterCommand;
/** Allows you add or remove a column in the table.
@ -20,6 +21,8 @@ public:
BlockIO execute() override;
static AccessRightsElements getRequiredAccessForCommand(const ASTAlterCommand & command, const String & database, const String & table);
private:
AccessRightsElements getRequiredAccess() const;

View File

@ -6,6 +6,10 @@
#include <Interpreters/ProcessList.h>
#include <Interpreters/executeQuery.h>
#include <Interpreters/CancellationCode.h>
#include <Interpreters/InterpreterAlterQuery.h>
#include <Parsers/ASTAlterQuery.h>
#include <Parsers/ParserAlterQuery.h>
#include <Parsers/parseQuery.h>
#include <Access/AccessRightsContext.h>
#include <Columns/ColumnString.h>
#include <Common/typeid_cast.h>
@ -55,7 +59,7 @@ struct QueryDescriptor
size_t source_num;
bool processed = false;
QueryDescriptor(String && query_id_, String && user_, size_t source_num_, bool processed_ = false)
QueryDescriptor(String query_id_, String user_, size_t source_num_, bool processed_ = false)
: query_id(std::move(query_id_)), user(std::move(user_)), source_num(source_num_), processed(processed_) {}
};
@ -81,6 +85,7 @@ static QueryDescriptors extractQueriesExceptMeAndCheckAccess(const Block & proce
const ClientInfo & my_client = context.getProcessListElement()->getClientInfo();
std::optional<bool> can_kill_query_started_by_another_user;
String query_user;
bool access_denied = false;
for (size_t i = 0; i < num_processes; ++i)
{
@ -95,14 +100,17 @@ static QueryDescriptors extractQueriesExceptMeAndCheckAccess(const Block & proce
{
if (!can_kill_query_started_by_another_user)
can_kill_query_started_by_another_user = context.getAccessRights()->isGranted(&Poco::Logger::get("InterpreterKillQueryQuery"), AccessType::KILL_QUERY);
if (!can_kill_query_started_by_another_user.value())
if (!*can_kill_query_started_by_another_user)
{
access_denied = true;
continue;
}
}
res.emplace_back(std::move(query_id), std::move(query_user), i, false);
res.emplace_back(std::move(query_id), query_user, i, false);
}
if (res.empty() && !query_user.empty()) // NOLINT
if (res.empty() && access_denied)
throw Exception("User " + my_client.current_user + " attempts to kill query created by " + query_user, ErrorCodes::ACCESS_DENIED);
return res;
@ -221,19 +229,22 @@ BlockIO InterpreterKillQueryQuery::execute()
}
case ASTKillQueryQuery::Type::Mutation:
{
Block mutations_block = getSelectResult("database, table, mutation_id", "system.mutations");
Block mutations_block = getSelectResult("database, table, mutation_id, command", "system.mutations");
if (!mutations_block)
return res_io;
const ColumnString & database_col = typeid_cast<const ColumnString &>(*mutations_block.getByName("database").column);
const ColumnString & table_col = typeid_cast<const ColumnString &>(*mutations_block.getByName("table").column);
const ColumnString & mutation_id_col = typeid_cast<const ColumnString &>(*mutations_block.getByName("mutation_id").column);
const ColumnString & command_col = typeid_cast<const ColumnString &>(*mutations_block.getByName("command").column);
auto header = mutations_block.cloneEmpty();
header.insert(0, {ColumnString::create(), std::make_shared<DataTypeString>(), "kill_status"});
MutableColumns res_columns = header.cloneEmptyColumns();
auto table_id = StorageID::createEmpty();
AccessRightsElements required_access_rights;
bool access_denied = false;
for (size_t i = 0; i < mutations_block.rows(); ++i)
{
@ -248,8 +259,14 @@ BlockIO InterpreterKillQueryQuery::execute()
code = CancellationCode::NotFound;
else
{
if (!context.getAccessRights()->isGranted(&Poco::Logger::get("InterpreterKillQueryQuery"), AccessType::KILL_MUTATION, table_id.database_name, table_id.table_name))
ParserAlterCommand parser;
auto command_ast = parseQuery(parser, command_col.getDataAt(i).toString(), 0);
required_access_rights = InterpreterAlterQuery::getRequiredAccessForCommand(command_ast->as<const ASTAlterCommand &>(), table_id.database_name, table_id.table_name);
if (!context.getAccessRights()->isGranted(&Poco::Logger::get("InterpreterKillQueryQuery"), required_access_rights))
{
access_denied = true;
continue;
}
code = storage->killMutation(mutation_id);
}
}
@ -257,9 +274,9 @@ BlockIO InterpreterKillQueryQuery::execute()
insertResultRow(i, code, mutations_block, header, res_columns);
}
if (res_columns[0]->empty() && table_id)
if (res_columns[0]->empty() && access_denied)
throw Exception(
"Not allowed to kill mutation on " + table_id.getNameForLogs(),
"Not allowed to kill mutation. To execute this query it's necessary to have the grant " + required_access_rights.toString(),
ErrorCodes::ACCESS_DENIED);
res_io.in = std::make_shared<OneBlockInputStream>(header.cloneWithColumns(std::move(res_columns)));
@ -295,7 +312,7 @@ AccessRightsElements InterpreterKillQueryQuery::getRequiredAccessForDDLOnCluster
if (query.type == ASTKillQueryQuery::Type::Query)
required_access.emplace_back(AccessType::KILL_QUERY);
else if (query.type == ASTKillQueryQuery::Type::Mutation)
required_access.emplace_back(AccessType::KILL_MUTATION);
required_access.emplace_back(AccessType::UPDATE | AccessType::DELETE | AccessType::MATERIALIZE_INDEX | AccessType::MATERIALIZE_TTL);
return required_access;
}

View File

@ -1,7 +1,7 @@
*** Create and kill a single invalid mutation ***
1
waiting test kill_mutation mutation_3.txt
waiting test kill_mutation mutation_3.txt DELETE WHERE toUInt32(s) = 1
*** Create and kill invalid mutation that blocks another mutation ***
1
waiting test kill_mutation mutation_4.txt
waiting test kill_mutation mutation_4.txt DELETE WHERE toUInt32(s) = 1
2001-01-01 2 b

View File

@ -1,9 +1,9 @@
*** Create and kill a single invalid mutation ***
1
Mutation 0000000000 was killed
waiting test kill_mutation_r1 0000000000
waiting test kill_mutation_r1 0000000000 DELETE WHERE toUInt32(s) = 1
0
*** Create and kill invalid mutation that blocks another mutation ***
1
waiting test kill_mutation_r1 0000000001
waiting test kill_mutation_r1 0000000001 DELETE WHERE toUInt32(s) = 1
2001-01-01 2 b