Fix ALTER QUERY MODIFY SQL SECURITY (#61480)

This commit is contained in:
pufit 2024-03-25 20:03:02 -04:00 committed by GitHub
parent 7282c5dbf6
commit cad66f3e68
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
8 changed files with 35 additions and 14 deletions

View File

@ -71,6 +71,17 @@ void applyMetadataChangesToCreateQuery(const ASTPtr & query, const StorageInMemo
query->replace(ast_create_query.refresh_strategy, metadata.refresh);
}
if (metadata.sql_security_type)
{
auto new_sql_security = std::make_shared<ASTSQLSecurity>();
new_sql_security->type = metadata.sql_security_type;
if (metadata.definer)
new_sql_security->definer = std::make_shared<ASTUserNameWithHost>(*metadata.definer);
ast_create_query.sql_security = std::move(new_sql_security);
}
/// MaterializedView, Dictionary are types of CREATE query without storage.
if (ast_create_query.storage)
{

View File

@ -1881,7 +1881,7 @@ void InterpreterCreateQuery::addColumnsDescriptionToCreateQueryIfNecessary(ASTCr
void InterpreterCreateQuery::processSQLSecurityOption(ContextPtr context_, ASTSQLSecurity & sql_security, bool is_attach, bool is_materialized_view)
{
/// If no SQL security is specified, apply default from default_*_view_sql_security setting.
if (!sql_security.type.has_value())
if (!sql_security.type)
{
SQLSecurityType default_security;

View File

@ -14,7 +14,7 @@ namespace DB
void ASTSQLSecurity::formatImpl(const FormatSettings & settings, FormatState & state, FormatStateStacked frame) const
{
if (!type.has_value())
if (!type)
return;
if (definer || is_definer_current_user)

View File

@ -299,6 +299,7 @@ namespace DB
MR_MACROS(MOD, "MOD") \
MR_MACROS(MODIFY_COLUMN, "MODIFY COLUMN") \
MR_MACROS(MODIFY_COMMENT, "MODIFY COMMENT") \
MR_MACROS(MODIFY_DEFINER, "MODIFY DEFINER") \
MR_MACROS(MODIFY_ORDER_BY, "MODIFY ORDER BY") \
MR_MACROS(MODIFY_QUERY, "MODIFY QUERY") \
MR_MACROS(MODIFY_REFRESH, "MODIFY REFRESH") \

View File

@ -41,6 +41,7 @@ bool ParserAlterCommand::parseImpl(Pos & pos, ASTPtr & node, Expected & expected
ParserKeyword s_reset_setting(Keyword::RESET_SETTING);
ParserKeyword s_modify_query(Keyword::MODIFY_QUERY);
ParserKeyword s_modify_sql_security(Keyword::MODIFY_SQL_SECURITY);
ParserKeyword s_modify_definer(Keyword::MODIFY_DEFINER);
ParserKeyword s_modify_refresh(Keyword::MODIFY_REFRESH);
ParserKeyword s_add_index(Keyword::ADD_INDEX);
@ -862,11 +863,16 @@ bool ParserAlterCommand::parseImpl(Pos & pos, ASTPtr & node, Expected & expected
return false;
command->type = ASTAlterCommand::MODIFY_QUERY;
}
else if (s_modify_sql_security.ignore(pos, expected))
else if (s_modify_sql_security.checkWithoutMoving(pos, expected))
{
/// This is a hack so we can reuse parser from create and don't have to write `MODIFY SQL SECURITY SQL SECURITY INVOKER`
--pos;
--pos;
s_modify.ignore(pos, expected);
if (!sql_security_p.parse(pos, command_sql_security, expected))
return false;
command->type = ASTAlterCommand::MODIFY_SQL_SECURITY;
}
else if (s_modify_definer.checkWithoutMoving(pos, expected))
{
s_modify.ignore(pos, expected);
if (!sql_security_p.parse(pos, command_sql_security, expected))
return false;
command->type = ASTAlterCommand::MODIFY_SQL_SECURITY;

View File

@ -94,6 +94,8 @@ void StorageInMemoryMetadata::setSQLSecurity(const ASTSQLSecurity & sql_security
{
if (sql_security.definer)
definer = sql_security.definer->toString();
else
definer = std::nullopt;
sql_security_type = sql_security.type;
}

View File

@ -12,8 +12,10 @@ OK
2
2
OK
1
===== MaterializedView =====
OK
1
0
0
OK

View File

@ -1,18 +1,17 @@
#!/usr/bin/env bash
# Tags: no-replicated-database
CURDIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)
# shellcheck source=../shell_config.sh
. "$CURDIR"/../shell_config.sh
user1="user02884_1_$RANDOM$RANDOM"
user2="user02884_2_$RANDOM$RANDOM"
user3="user02884_3_$RANDOM$RANDOM"
db="db02884_$RANDOM$RANDOM"
user1="user02884_1_${CLICKHOUSE_DATABASE}_$RANDOM"
user2="user02884_2_${CLICKHOUSE_DATABASE}_$RANDOM"
user3="user02884_3_${CLICKHOUSE_DATABASE}_$RANDOM"
db=${CLICKHOUSE_DATABASE}
${CLICKHOUSE_CLIENT} --multiquery <<EOF
DROP DATABASE IF EXISTS $db;
CREATE DATABASE $db;
CREATE TABLE $db.test_table (s String) ENGINE = MergeTree ORDER BY s;
DROP USER IF EXISTS $user1, $user2, $user3;
@ -92,6 +91,7 @@ ${CLICKHOUSE_CLIENT} --user $user2 --query "SELECT count() FROM $db.test_view_10
${CLICKHOUSE_CLIENT} --query "ALTER TABLE $db.test_view_10 MODIFY SQL SECURITY INVOKER"
(( $(${CLICKHOUSE_CLIENT} --user $user2 --query "SELECT * FROM $db.test_view_10" 2>&1 | grep -c "Not enough privileges") >= 1 )) && echo "OK" || echo "UNEXPECTED"
${CLICKHOUSE_CLIENT} --query "SHOW CREATE TABLE $db.test_view_10" | grep -c "SQL SECURITY INVOKER"
echo "===== MaterializedView ====="
@ -136,6 +136,7 @@ ${CLICKHOUSE_CLIENT} --query "GRANT SELECT ON $db.test_mv_5 TO $user2"
${CLICKHOUSE_CLIENT} --query "ALTER TABLE $db.test_mv_5 MODIFY SQL SECURITY NONE"
${CLICKHOUSE_CLIENT} --user $user2 --query "SELECT * FROM $db.test_mv_5"
${CLICKHOUSE_CLIENT} --query "SHOW CREATE TABLE $db.test_mv_5" | grep -c "SQL SECURITY NONE"
${CLICKHOUSE_CLIENT} --query "GRANT SELECT ON $db.test_mv_1 TO $user2"
${CLICKHOUSE_CLIENT} --query "GRANT SELECT ON $db.test_mv_3 TO $user2"
@ -221,6 +222,4 @@ EOF
${CLICKHOUSE_CLIENT} --user $user2 --query "SELECT * FROM $db.test_mv_row_2"
${CLICKHOUSE_CLIENT} --query "DROP DATABASE IF EXISTS $db;"
${CLICKHOUSE_CLIENT} --query "DROP USER IF EXISTS $user1, $user2, $user3";