Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add check access database for system tables #2856

Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 8 additions & 0 deletions dbms/src/Interpreters/Context.cpp
Expand Up @@ -618,6 +618,13 @@ void Context::checkDatabaseAccessRights(const std::string & database_name) const
checkDatabaseAccessRightsImpl(database_name);
}

bool Context::isDatabaseAccessRights(const String & database_name) const
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isDatabaseAccessRights -> hasDatabaseAccessRights

{
auto lock = getLock();
return client_info.current_user.empty() || (database_name == "system") ||
shared->security_manager->hasAccessToDatabase(client_info.current_user, database_name);
}

void Context::checkDatabaseAccessRightsImpl(const std::string & database_name) const
{
if (client_info.current_user.empty() || (database_name == "system"))
Expand Down Expand Up @@ -1793,6 +1800,7 @@ std::shared_ptr<ActionLocksManager> Context::getActionLocksManager()
return shared->action_locks_manager;
}


SessionCleaner::~SessionCleaner()
{
try
Expand Down
1 change: 1 addition & 0 deletions dbms/src/Interpreters/Context.h
Expand Up @@ -180,6 +180,7 @@ class Context
bool isTableExist(const String & database_name, const String & table_name) const;
bool isDatabaseExist(const String & database_name) const;
bool isExternalTableExist(const String & table_name) const;
bool isDatabaseAccessRights(const String & database_name) const;
void assertTableExists(const String & database_name, const String & table_name) const;

/** The parameter check_database_access_rights exists to not check the permissions of the database again,
Expand Down
6 changes: 5 additions & 1 deletion dbms/src/Storages/System/StorageSystemColumns.cpp
Expand Up @@ -42,7 +42,11 @@ void StorageSystemColumns::fillData(MutableColumns & res_columns, const Context
/// Add `database` column.
MutableColumnPtr database_column_mut = ColumnString::create();
for (const auto & database : databases)
database_column_mut->insert(database.first);
{
if (context.isDatabaseAccessRights(database.first))
database_column_mut->insert(database.first);
}

block_to_filter.insert(ColumnWithTypeAndName(std::move(database_column_mut), std::make_shared<DataTypeString>(), "database"));

/// Filter block with `database` column.
Expand Down
11 changes: 7 additions & 4 deletions dbms/src/Storages/System/StorageSystemDatabases.cpp
Expand Up @@ -22,10 +22,13 @@ void StorageSystemDatabases::fillData(MutableColumns & res_columns, const Contex
auto databases = context.getDatabases();
for (const auto & database : databases)
{
res_columns[0]->insert(database.first);
res_columns[1]->insert(database.second->getEngineName());
res_columns[2]->insert(database.second->getDataPath());
res_columns[3]->insert(database.second->getMetadataPath());
if (context.isDatabaseAccessRights(database.first))
{
res_columns[0]->insert(database.first);
res_columns[1]->insert(database.second->getEngineName());
res_columns[2]->insert(database.second->getDataPath());
res_columns[3]->insert(database.second->getMetadataPath());
}
}
}

Expand Down
37 changes: 20 additions & 17 deletions dbms/src/Storages/System/StorageSystemMerges.cpp
Expand Up @@ -33,23 +33,26 @@ void StorageSystemMerges::fillData(MutableColumns & res_columns, const Context &
{
for (const auto & merge : context.getMergeList().get())
{
size_t i = 0;
res_columns[i++]->insert(merge.database);
res_columns[i++]->insert(merge.table);
res_columns[i++]->insert(merge.elapsed);
res_columns[i++]->insert(merge.progress);
res_columns[i++]->insert(merge.num_parts);
res_columns[i++]->insert(merge.source_part_names);
res_columns[i++]->insert(merge.result_part_name);
res_columns[i++]->insert(merge.total_size_bytes_compressed);
res_columns[i++]->insert(merge.total_size_marks);
res_columns[i++]->insert(merge.bytes_read_uncompressed);
res_columns[i++]->insert(merge.rows_read);
res_columns[i++]->insert(merge.bytes_written_uncompressed);
res_columns[i++]->insert(merge.rows_written);
res_columns[i++]->insert(merge.columns_written);
res_columns[i++]->insert(merge.memory_usage);
res_columns[i++]->insert(merge.thread_number);
if (context.isDatabaseAccessRights(merge.database))
{
size_t i = 0;
res_columns[i++]->insert(merge.database);
res_columns[i++]->insert(merge.table);
res_columns[i++]->insert(merge.elapsed);
res_columns[i++]->insert(merge.progress);
res_columns[i++]->insert(merge.num_parts);
res_columns[i++]->insert(merge.source_part_names);
res_columns[i++]->insert(merge.result_part_name);
res_columns[i++]->insert(merge.total_size_bytes_compressed);
res_columns[i++]->insert(merge.total_size_marks);
res_columns[i++]->insert(merge.bytes_read_uncompressed);
res_columns[i++]->insert(merge.rows_read);
res_columns[i++]->insert(merge.bytes_written_uncompressed);
res_columns[i++]->insert(merge.rows_written);
res_columns[i++]->insert(merge.columns_written);
res_columns[i++]->insert(merge.memory_usage);
res_columns[i++]->insert(merge.thread_number);
}
}
}

Expand Down
11 changes: 7 additions & 4 deletions dbms/src/Storages/System/StorageSystemMutations.cpp
Expand Up @@ -38,12 +38,15 @@ void StorageSystemMutations::fillData(MutableColumns & res_columns, const Contex
std::map<String, std::map<String, StoragePtr>> merge_tree_tables;
for (const auto & db : context.getDatabases())
{
for (auto iterator = db.second->getIterator(context); iterator->isValid(); iterator->next())
if (context.isDatabaseAccessRights(db.first))
{
if (dynamic_cast<const StorageMergeTree *>(iterator->table().get())
|| dynamic_cast<const StorageReplicatedMergeTree *>(iterator->table().get()))
for (auto iterator = db.second->getIterator(context); iterator->isValid(); iterator->next())
{
merge_tree_tables[db.first][iterator->name()] = iterator->table();
if (dynamic_cast<const StorageMergeTree *>(iterator->table().get())
|| dynamic_cast<const StorageReplicatedMergeTree *>(iterator->table().get()))
{
merge_tree_tables[db.first][iterator->name()] = iterator->table();
}
}
}
}
Expand Down
5 changes: 4 additions & 1 deletion dbms/src/Storages/System/StorageSystemPartsBase.cpp
Expand Up @@ -59,7 +59,10 @@ class StoragesInfoStream
/// Add column 'database'.
MutableColumnPtr database_column_mut = ColumnString::create();
for (const auto & database : databases)
database_column_mut->insert(database.first);
{
if (context.isDatabaseAccessRights(database.first))
database_column_mut->insert(database.first);
}
block_to_filter.insert(ColumnWithTypeAndName(
std::move(database_column_mut), std::make_shared<DataTypeString>(), "database"));

Expand Down
12 changes: 9 additions & 3 deletions dbms/src/Storages/System/StorageSystemReplicas.cpp
Expand Up @@ -65,9 +65,15 @@ BlockInputStreams StorageSystemReplicas::read(
/// We collect a set of replicated tables.
std::map<String, std::map<String, StoragePtr>> replicated_tables;
for (const auto & db : context.getDatabases())
for (auto iterator = db.second->getIterator(context); iterator->isValid(); iterator->next())
if (dynamic_cast<const StorageReplicatedMergeTree *>(iterator->table().get()))
replicated_tables[db.first][iterator->name()] = iterator->table();
{
if (context.isDatabaseAccessRights(db.first))
{
for (auto iterator = db.second->getIterator(context); iterator->isValid(); iterator->next())
if (dynamic_cast<const StorageReplicatedMergeTree *>(iterator->table().get()))
replicated_tables[db.first][iterator->name()] = iterator->table();
}
}


/// Do you need columns that require a walkthrough in ZooKeeper to compute.
bool with_zk_fields = false;
Expand Down
12 changes: 9 additions & 3 deletions dbms/src/Storages/System/StorageSystemReplicationQueue.cpp
Expand Up @@ -50,9 +50,15 @@ void StorageSystemReplicationQueue::fillData(MutableColumns & res_columns, const
{
std::map<String, std::map<String, StoragePtr>> replicated_tables;
for (const auto & db : context.getDatabases())
for (auto iterator = db.second->getIterator(context); iterator->isValid(); iterator->next())
if (dynamic_cast<const StorageReplicatedMergeTree *>(iterator->table().get()))
replicated_tables[db.first][iterator->name()] = iterator->table();
{
if (context.isDatabaseAccessRights(db.first))
{
for (auto iterator = db.second->getIterator(context); iterator->isValid(); iterator->next())
if (dynamic_cast<const StorageReplicatedMergeTree *>(iterator->table().get()))
replicated_tables[db.first][iterator->name()] = iterator->table();
}
}


MutableColumnPtr col_database_mut = ColumnString::create();
MutableColumnPtr col_table_mut = ColumnString::create();
Expand Down
2 changes: 1 addition & 1 deletion dbms/src/Storages/System/StorageSystemTables.cpp
Expand Up @@ -95,7 +95,7 @@ BlockInputStreams StorageSystemTables::read(

auto database = context.tryGetDatabase(database_name);

if (!database)
if (!database || !context.isDatabaseAccessRights(database_name))
{
/// Database was deleted just now.
continue;
Expand Down