This commit is contained in:
jsc0218 2024-04-17 00:31:15 +00:00
parent a1e3a4a58c
commit 80fa1ab89a
6 changed files with 29 additions and 40 deletions

View File

@ -205,7 +205,7 @@ namespace
}
/// There is overlap between AccessType sources and table engines, so the following code avoids user granting twice.
std::vector<std::tuple<AccessFlags, std::string>> source_and_table_engines = {
static const std::vector<std::tuple<AccessFlags, std::string>> source_and_table_engines = {
{AccessType::FILE, "File"},
{AccessType::URL, "URL"},
{AccessType::REMOTE, "Distributed"},
@ -222,14 +222,31 @@ namespace
{AccessType::AZURE, "AzureBlobStorage"}
};
for (const auto & source_and_table_engine : source_and_table_engines)
/// Sync SOURCE and TABLE_ENGINE, so only need to check TABLE_ENGINE later.
if (access_control.doesTableEnginesRequireGrant())
{
const auto & source = std::get<0>(source_and_table_engine);
const auto & table_engine = std::get<1>(source_and_table_engine);
if (res.isGranted(source) || res.isGranted(AccessType::TABLE_ENGINE, table_engine))
for (const auto & source_and_table_engine : source_and_table_engines)
{
res.grant(source);
res.grant(AccessType::TABLE_ENGINE, table_engine);
const auto & source = std::get<0>(source_and_table_engine);
if (res.isGranted(source))
{
const auto & table_engine = std::get<1>(source_and_table_engine);
res.grant(AccessType::TABLE_ENGINE, table_engine);
}
}
}
else
{
/// Add TABLE_ENGINE on * and then remove TABLE_ENGINE on particular engines.
res.grant(AccessType::TABLE_ENGINE);
for (const auto & source_and_table_engine : source_and_table_engines)
{
const auto & source = std::get<0>(source_and_table_engine);
if (!res.isGranted(source))
{
const auto & table_engine = std::get<1>(source_and_table_engine);
res.revoke(AccessType::TABLE_ENGINE, table_engine);
}
}
}
@ -576,9 +593,6 @@ bool ContextAccess::checkAccessImplHelper(AccessFlags flags, const Args &... arg
if (flags & AccessType::CLUSTER && !access_control->doesOnClusterQueriesRequireClusterGrant())
flags &= ~AccessType::CLUSTER;
if (flags & AccessType::TABLE_ENGINE && !access_control->doesTableEnginesRequireGrant())
flags &= ~AccessType::TABLE_ENGINE;
if (!flags)
return true;

View File

@ -723,13 +723,7 @@ InterpreterCreateQuery::TableProperties InterpreterCreateQuery::getTableProperti
/// We have to check access rights again (in case engine was changed).
if (create.storage && create.storage->engine)
{
auto source_access_type = StorageFactory::instance().getSourceAccessType(create.storage->engine->name);
const auto & access_control = getContext()->getAccessControl();
if (source_access_type != AccessType::NONE && !access_control.doesTableEnginesRequireGrant())
getContext()->checkAccess(source_access_type);
getContext()->checkAccess(AccessType::TABLE_ENGINE, create.storage->engine->name);
}
TableProperties properties;
TableLockHolder as_storage_lock;
@ -1835,14 +1829,7 @@ AccessRightsElements InterpreterCreateQuery::getRequiredAccess() const
required_access.emplace_back(AccessType::SELECT | AccessType::INSERT, create.to_table_id.database_name, create.to_table_id.table_name);
if (create.storage && create.storage->engine)
{
auto source_access_type = StorageFactory::instance().getSourceAccessType(create.storage->engine->name);
const auto & access_control = getContext()->getAccessControl();
/// We just need to check GRANT TABLE ENGINE for sources if grant of table engine is enabled.
if (source_access_type != AccessType::NONE && !access_control.doesTableEnginesRequireGrant())
required_access.emplace_back(source_access_type);
required_access.emplace_back(AccessType::TABLE_ENGINE, create.storage->engine->name);
}
return required_access;
}

View File

@ -10,6 +10,7 @@
<allow_databases>
<database>db1</database>
<database>Memory</database>
</allow_databases>
</restricted_user>
</users>

View File

@ -291,29 +291,19 @@ def test_allowed_databases(test_cluster):
instance.query("CREATE DATABASE IF NOT EXISTS db2 ON CLUSTER cluster")
instance.query(
"CREATE TABLE IF NOT EXISTS db1.t1 ON CLUSTER cluster (i Int8) ENGINE = Memory"
)
instance.query(
"CREATE TABLE IF NOT EXISTS db2.t2 ON CLUSTER cluster (i Int8) ENGINE = Memory"
)
instance.query(
"CREATE TABLE IF NOT EXISTS t3 ON CLUSTER cluster (i Int8) ENGINE = Memory"
)
instance.query(
"SELECT * FROM db1.t1",
"CREATE TABLE db1.t1 ON CLUSTER cluster (i Int8) ENGINE = Memory",
settings={"user": "restricted_user"},
)
with pytest.raises(Exception):
instance.query(
"SELECT * FROM db2.t2",
"CREATE TABLE db2.t2 ON CLUSTER cluster (i Int8) ENGINE = Memory",
settings={"user": "restricted_user"},
)
with pytest.raises(Exception):
instance.query(
"SELECT * FROM t3",
"CREATE TABLE t3 ON CLUSTER cluster (i Int8) ENGINE = Memory",
settings={"user": "restricted_user"},
)

View File

@ -989,7 +989,6 @@ def test_predefined_connection_configuration(started_cluster):
instance.query("GRANT CREATE ON *.* TO user")
instance.query("GRANT SOURCES ON *.* TO user")
instance.query("GRANT SELECT ON *.* TO user")
instance.query("GRANT TABLE ENGINE ON S3 TO user")
instance.query(f"drop table if exists {name}", user="user")
error = instance.query_and_get_error(

View File

@ -9,9 +9,7 @@ ${CLICKHOUSE_CLIENT} -q "create table mute_stylecheck (x UInt32) engine = Replic
${CLICKHOUSE_CLIENT} -q "CREATE USER user_${CLICKHOUSE_DATABASE} settings database_replicated_allow_only_replicated_engine=1"
${CLICKHOUSE_CLIENT} -q "GRANT CREATE TABLE ON ${CLICKHOUSE_DATABASE}_db.* TO user_${CLICKHOUSE_DATABASE}"
${CLICKHOUSE_CLIENT} -q "GRANT TABLE ENGINE ON Memory TO user_${CLICKHOUSE_DATABASE}"
${CLICKHOUSE_CLIENT} -q "GRANT TABLE ENGINE ON MergeTree TO user_${CLICKHOUSE_DATABASE}"
${CLICKHOUSE_CLIENT} -q "GRANT TABLE ENGINE ON ReplicatedMergeTree TO user_${CLICKHOUSE_DATABASE}"
${CLICKHOUSE_CLIENT} -q "GRANT TABLE ENGINE ON Memory, TABLE ENGINE ON MergeTree, TABLE ENGINE ON ReplicatedMergeTree TO user_${CLICKHOUSE_DATABASE}"
${CLICKHOUSE_CLIENT} --allow_experimental_database_replicated=1 --query "CREATE DATABASE ${CLICKHOUSE_DATABASE}_db engine = Replicated('/clickhouse/databases/${CLICKHOUSE_TEST_ZOOKEEPER_PREFIX}/${CLICKHOUSE_DATABASE}_db', '{shard}', '{replica}')"
${CLICKHOUSE_CLIENT} --distributed_ddl_output_mode=none --user "user_${CLICKHOUSE_DATABASE}" --query "CREATE TABLE ${CLICKHOUSE_DATABASE}_db.tab_memory (x UInt32) engine = Memory;"
${CLICKHOUSE_CLIENT} --distributed_ddl_output_mode=none --user "user_${CLICKHOUSE_DATABASE}" -n --query "CREATE TABLE ${CLICKHOUSE_DATABASE}_db.tab_mt (x UInt32) engine = MergeTree order by x;" 2>&1 | grep -o "Only tables with a Replicated engine"