From 8030b2a8b235a9d0b5881a150c8f697b5504103d Mon Sep 17 00:00:00 2001 From: unashi Date: Mon, 26 Aug 2024 18:12:51 +0800 Subject: [PATCH 01/10] [feature] Add 'check grant' to see if (current user has been granted certain access rights elements && the elements exist). --- .../Access/InterpreterCheckGrantQuery.cpp | 114 +++++++++ .../Access/InterpreterCheckGrantQuery.h | 26 ++ src/Interpreters/InterpreterFactory.cpp | 5 + src/Interpreters/registerInterpreters.cpp | 2 + src/Parsers/Access/ASTCheckGrantQuery.cpp | 30 +++ src/Parsers/Access/ASTCheckGrantQuery.h | 27 ++ src/Parsers/Access/ParserCheckGrantQuery.cpp | 232 ++++++++++++++++++ src/Parsers/Access/ParserCheckGrantQuery.h | 17 ++ src/Parsers/CommonParsers.h | 1 + src/Parsers/ParserQuery.cpp | 3 + 10 files changed, 457 insertions(+) create mode 100644 src/Interpreters/Access/InterpreterCheckGrantQuery.cpp create mode 100644 src/Interpreters/Access/InterpreterCheckGrantQuery.h create mode 100644 src/Parsers/Access/ASTCheckGrantQuery.cpp create mode 100644 src/Parsers/Access/ASTCheckGrantQuery.h create mode 100644 src/Parsers/Access/ParserCheckGrantQuery.cpp create mode 100644 src/Parsers/Access/ParserCheckGrantQuery.h diff --git a/src/Interpreters/Access/InterpreterCheckGrantQuery.cpp b/src/Interpreters/Access/InterpreterCheckGrantQuery.cpp new file mode 100644 index 00000000000..7a01380e343 --- /dev/null +++ b/src/Interpreters/Access/InterpreterCheckGrantQuery.cpp @@ -0,0 +1,114 @@ +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include "Databases/IDatabase.h" +#include "Storages/IStorage.h" + +namespace DB +{ + +namespace +{ + /// Extracts access rights elements which are going to check grant + void collectAccessRightsElementsToGrantOrRevoke( + const ASTCheckGrantQuery & query, + AccessRightsElements & elements_to_check_grant) + { + elements_to_check_grant.clear(); + /// GRANT + elements_to_check_grant = query.access_rights_elements; + } +} + + +BlockIO InterpreterCheckGrantQuery::execute() +{ + auto & query = query_ptr->as(); + query.access_rights_elements.eraseNonGrantable(); + + auto current_user_access = getContext()->getAccess(); + + /// Collect access rights elements which will be checked. + AccessRightsElements elements_to_check_grant; + collectAccessRightsElementsToGrantOrRevoke(query, elements_to_check_grant); + + + /// Replacing empty database with the default. This step must be done before replication to avoid privilege escalation. + String current_database = getContext()->getCurrentDatabase(); + elements_to_check_grant.replaceEmptyDatabase(current_database); + query.access_rights_elements.replaceEmptyDatabase(current_database); + auto *logger = &::Poco::Logger::get("CheckGrantQuery"); + + /// Check If Table/Columns exist. + for (const auto & elem : elements_to_check_grant) + { + try + { + DatabasePtr database; + database = DatabaseCatalog::instance().getDatabase(elem.database); + if (!database->isTableExist(elem.table, getContext())) + { + /// Table not found. + return executeQuery("SELECT 0 AS CHECK_GRANT", getContext(), QueryFlags{.internal = true}).second; + } + auto table = database->getTable(elem.table, getContext()); + + auto column_name_with_sizes = table->getColumnSizes(); + for (const auto & elem_col : elem.columns) + { + bool founded = false; + for (const auto & col_in_table : column_name_with_sizes) + { + if (col_in_table.first == elem_col) + { + founded = true; + break; + } + } + if (!founded) + { + /// Column not found. + return executeQuery("SELECT 0 AS CHECK_GRANT", getContext(), QueryFlags{.internal = true}).second; + } + } + } + catch (...) + { + tryLogCurrentException(logger); + return executeQuery("SELECT 0 AS CHECK_GRANT", getContext(), QueryFlags{.internal = true}).second; + } + } + bool user_is_granted = current_user_access->isGranted(elements_to_check_grant); + if (!user_is_granted) + { + return executeQuery("SELECT 0 AS CHECK_GRANT", getContext(), QueryFlags{.internal = true}).second; + } + + return executeQuery("SELECT 1 AS CHECK_GRANT", getContext(), QueryFlags{.internal = true}).second; +} + +void registerInterpreterCheckGrantQuery(InterpreterFactory & factory) +{ + auto create_fn = [] (const InterpreterFactory::Arguments & args) + { + return std::make_unique(args.query, args.context); + }; + factory.registerInterpreter("InterpreterCheckGrantQuery", create_fn); +} + +} diff --git a/src/Interpreters/Access/InterpreterCheckGrantQuery.h b/src/Interpreters/Access/InterpreterCheckGrantQuery.h new file mode 100644 index 00000000000..e79d6925c93 --- /dev/null +++ b/src/Interpreters/Access/InterpreterCheckGrantQuery.h @@ -0,0 +1,26 @@ +#pragma once + +#include +#include +#include + + +namespace DB +{ + +class ASTCheckGrantQuery; +struct User; +struct Role; + +class InterpreterCheckGrantQuery : public IInterpreter, WithMutableContext +{ +public: + InterpreterCheckGrantQuery(const ASTPtr & query_ptr_, ContextMutablePtr context_) : WithMutableContext(context_), query_ptr(query_ptr_) {} + + BlockIO execute() override; + +private: + ASTPtr query_ptr; +}; + +} diff --git a/src/Interpreters/InterpreterFactory.cpp b/src/Interpreters/InterpreterFactory.cpp index 12b3b510098..477e2425c80 100644 --- a/src/Interpreters/InterpreterFactory.cpp +++ b/src/Interpreters/InterpreterFactory.cpp @@ -60,6 +60,7 @@ #include #include #include +#include "Parsers/Access/ASTCheckGrantQuery.h" #include @@ -298,6 +299,10 @@ InterpreterFactory::InterpreterPtr InterpreterFactory::get(ASTPtr & query, Conte { interpreter_name = "InterpreterShowGrantsQuery"; } + else if (query->as()) + { + interpreter_name = "InterpreterCheckGrantQuery"; + } else if (query->as()) { interpreter_name = "InterpreterShowAccessEntitiesQuery"; diff --git a/src/Interpreters/registerInterpreters.cpp b/src/Interpreters/registerInterpreters.cpp index 481d0597a85..7d8ed17a11f 100644 --- a/src/Interpreters/registerInterpreters.cpp +++ b/src/Interpreters/registerInterpreters.cpp @@ -45,6 +45,7 @@ void registerInterpreterDropNamedCollectionQuery(InterpreterFactory & factory); void registerInterpreterGrantQuery(InterpreterFactory & factory); void registerInterpreterShowCreateAccessEntityQuery(InterpreterFactory & factory); void registerInterpreterShowGrantsQuery(InterpreterFactory & factory); +void registerInterpreterCheckGrantQuery(InterpreterFactory & factory); void registerInterpreterShowAccessEntitiesQuery(InterpreterFactory & factory); void registerInterpreterShowAccessQuery(InterpreterFactory & factory); void registerInterpreterShowPrivilegesQuery(InterpreterFactory & factory); @@ -104,6 +105,7 @@ void registerInterpreters() registerInterpreterGrantQuery(factory); registerInterpreterShowCreateAccessEntityQuery(factory); registerInterpreterShowGrantsQuery(factory); + registerInterpreterCheckGrantQuery(factory); registerInterpreterShowAccessEntitiesQuery(factory); registerInterpreterShowAccessQuery(factory); registerInterpreterShowPrivilegesQuery(factory); diff --git a/src/Parsers/Access/ASTCheckGrantQuery.cpp b/src/Parsers/Access/ASTCheckGrantQuery.cpp new file mode 100644 index 00000000000..364e73920ab --- /dev/null +++ b/src/Parsers/Access/ASTCheckGrantQuery.cpp @@ -0,0 +1,30 @@ +#include +#include +#include "Common/logger_useful.h" +#include +#include + + +namespace DB +{ + +String ASTCheckGrantQuery::getID(char) const +{ + return "CheckGrantQuery"; +} + + +ASTPtr ASTCheckGrantQuery::clone() const +{ + auto res = std::make_shared(*this); + + return res; +} + + +void ASTCheckGrantQuery::replaceEmptyDatabase(const String & current_database) +{ + access_rights_elements.replaceEmptyDatabase(current_database); +} + +} diff --git a/src/Parsers/Access/ASTCheckGrantQuery.h b/src/Parsers/Access/ASTCheckGrantQuery.h new file mode 100644 index 00000000000..7e363d587ce --- /dev/null +++ b/src/Parsers/Access/ASTCheckGrantQuery.h @@ -0,0 +1,27 @@ +#pragma once + +#include +#include +#include +#include "Parsers/ASTQueryParameter.h" + + +namespace DB +{ +class ASTRolesOrUsersSet; + + +/** Parses queries like + * CHECK GRANT access_type[(column_name [,...])] [,...] ON {db.table|db.*|*.*|table|*} + */ +class ASTCheckGrantQuery : public IAST +{ +public: + AccessRightsElements access_rights_elements; + + String getID(char) const override; + ASTPtr clone() const override; + void replaceEmptyDatabase(const String & current_database); + QueryKind getQueryKind() const override { return QueryKind::Check; } +}; +} diff --git a/src/Parsers/Access/ParserCheckGrantQuery.cpp b/src/Parsers/Access/ParserCheckGrantQuery.cpp new file mode 100644 index 00000000000..c1daa435ad2 --- /dev/null +++ b/src/Parsers/Access/ParserCheckGrantQuery.cpp @@ -0,0 +1,232 @@ +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + + +namespace DB +{ +namespace ErrorCodes +{ + extern const int INVALID_GRANT; + extern const int SYNTAX_ERROR; +} + +namespace +{ + bool parseAccessFlags(IParser::Pos & pos, Expected & expected, AccessFlags & access_flags) + { + static constexpr auto is_one_of_access_type_words = [](IParser::Pos & pos_) + { + if (pos_->type != TokenType::BareWord) + return false; + std::string_view word{pos_->begin, pos_->size()}; + return !(boost::iequals(word, toStringView(Keyword::ON)) || boost::iequals(word, toStringView(Keyword::TO)) || boost::iequals(word, toStringView(Keyword::FROM))); + }; + + expected.add(pos, "access type"); + + return IParserBase::wrapParseImpl(pos, [&] + { + if (!is_one_of_access_type_words(pos)) + return false; + + String str; + do + { + if (!str.empty()) + str += " "; + str += std::string_view(pos->begin, pos->size()); + ++pos; + } + while (is_one_of_access_type_words(pos)); + + try + { + access_flags = AccessFlags{str}; + } + catch (...) + { + return false; + } + + return true; + }); + } + + + bool parseColumnNames(IParser::Pos & pos, Expected & expected, Strings & columns) + { + return IParserBase::wrapParseImpl(pos, [&] + { + if (!ParserToken{TokenType::OpeningRoundBracket}.ignore(pos, expected)) + return false; + + ASTPtr ast; + if (!ParserList{std::make_unique(), std::make_unique(TokenType::Comma), false}.parse(pos, ast, expected)) + return false; + + Strings res_columns; + for (const auto & child : ast->children) + res_columns.emplace_back(getIdentifierName(child)); + + if (!ParserToken{TokenType::ClosingRoundBracket}.ignore(pos, expected)) + return false; + + columns = std::move(res_columns); + return true; + }); + } + + bool parseAccessFlagsWithColumns(IParser::Pos & pos, Expected & expected, + std::vector> & access_and_columns) + { + std::vector> res; + + auto parse_access_and_columns = [&] + { + AccessFlags access_flags; + if (!parseAccessFlags(pos, expected, access_flags)) + return false; + + Strings columns; + parseColumnNames(pos, expected, columns); + res.emplace_back(access_flags, std::move(columns)); + return true; + }; + + if (!ParserList::parseUtil(pos, expected, parse_access_and_columns, false)) + return false; + + access_and_columns = std::move(res); + return true; + } + + + bool parseElementsWithoutOptions(IParser::Pos & pos, Expected & expected, AccessRightsElements & elements) + { + return IParserBase::wrapParseImpl(pos, [&] + { + AccessRightsElements res_elements; + + auto parse_around_on = [&] + { + std::vector> access_and_columns; + if (!parseAccessFlagsWithColumns(pos, expected, access_and_columns)) + return false; + + String database_name, table_name, parameter; + bool any_database = false, any_table = false, any_parameter = false; + + size_t is_global_with_parameter = 0; + for (const auto & elem : access_and_columns) + { + if (elem.first.isGlobalWithParameter()) + ++is_global_with_parameter; + } + + if (!ParserKeyword{Keyword::ON}.ignore(pos, expected)) + return false; + + if (is_global_with_parameter && is_global_with_parameter == access_and_columns.size()) + { + ASTPtr parameter_ast; + if (ParserToken{TokenType::Asterisk}.ignore(pos, expected)) + { + any_parameter = true; + } + else if (ParserIdentifier{}.parse(pos, parameter_ast, expected)) + { + any_parameter = false; + parameter = getIdentifierName(parameter_ast); + } + else + return false; + + any_database = any_table = true; + } + else if (!parseDatabaseAndTableNameOrAsterisks(pos, expected, database_name, any_database, table_name, any_table)) + { + return false; + } + + for (auto & [access_flags, columns] : access_and_columns) + { + AccessRightsElement element; + element.access_flags = access_flags; + element.any_column = columns.empty(); + element.columns = std::move(columns); + element.any_database = any_database; + element.database = database_name; + element.any_table = any_table; + element.any_parameter = any_parameter; + element.table = table_name; + element.parameter = parameter; + res_elements.emplace_back(std::move(element)); + } + + return true; + }; + + if (!ParserList::parseUtil(pos, expected, parse_around_on, false)) + return false; + + elements = std::move(res_elements); + return true; + }); + } + + void throwIfNotGrantable(AccessRightsElements & elements) + { + std::erase_if(elements, [](AccessRightsElement & element) + { + if (element.empty()) + return true; + auto old_flags = element.access_flags; + element.eraseNonGrantable(); + if (!element.empty()) + return false; + + if (!element.any_column) + throw Exception(ErrorCodes::INVALID_GRANT, "{} cannot check grant on the column level", old_flags.toString()); + else if (!element.any_table) + throw Exception(ErrorCodes::INVALID_GRANT, "{} cannot check grant on the table level", old_flags.toString()); + else if (!element.any_database) + throw Exception(ErrorCodes::INVALID_GRANT, "{} cannot check grant on the database level", old_flags.toString()); + else if (!element.any_parameter) + throw Exception(ErrorCodes::INVALID_GRANT, "{} cannot check grant on the global with parameter level", old_flags.toString()); + else + throw Exception(ErrorCodes::INVALID_GRANT, "{} cannot check grant", old_flags.toString()); + }); + } +} + + +bool ParserCheckGrantQuery::parseImpl(Pos & pos, ASTPtr & node, Expected & expected) +{ + if (!ParserKeyword{Keyword::CHECK_GRANT}.ignore(pos, expected)) + return false; + + + AccessRightsElements elements; + + if (!parseElementsWithoutOptions(pos, expected, elements) ) + return false; + + throwIfNotGrantable(elements); + + auto query = std::make_shared(); + node = query; + + query->access_rights_elements = std::move(elements); + + return true; +} +} diff --git a/src/Parsers/Access/ParserCheckGrantQuery.h b/src/Parsers/Access/ParserCheckGrantQuery.h new file mode 100644 index 00000000000..d58ea6002a8 --- /dev/null +++ b/src/Parsers/Access/ParserCheckGrantQuery.h @@ -0,0 +1,17 @@ +#pragma once + +#include + + +namespace DB +{ +/** Parses queries like + * CHECK GRANT access_type[(column_name [,...])] [,...] ON {db.table|db.*|*.*|table|*} + */ +class ParserCheckGrantQuery : public IParserBase +{ +protected: + const char * getName() const override { return "CHECK GRANT"; } + bool parseImpl(Pos & pos, ASTPtr & node, Expected & expected) override; +}; +} diff --git a/src/Parsers/CommonParsers.h b/src/Parsers/CommonParsers.h index ab0e70eb0e5..9918c13e382 100644 --- a/src/Parsers/CommonParsers.h +++ b/src/Parsers/CommonParsers.h @@ -78,6 +78,7 @@ namespace DB MR_MACROS(CHARACTER, "CHARACTER") \ MR_MACROS(CHECK_ALL_TABLES, "CHECK ALL TABLES") \ MR_MACROS(CHECK_TABLE, "CHECK TABLE") \ + MR_MACROS(CHECK_GRANT, "CHECK GRANT")\ MR_MACROS(CHECK, "CHECK") \ MR_MACROS(CLEANUP, "CLEANUP") \ MR_MACROS(CLEAR_COLUMN, "CLEAR COLUMN") \ diff --git a/src/Parsers/ParserQuery.cpp b/src/Parsers/ParserQuery.cpp index 22ddc25019f..92a83fe99f1 100644 --- a/src/Parsers/ParserQuery.cpp +++ b/src/Parsers/ParserQuery.cpp @@ -28,6 +28,7 @@ #include #include #include +#include "Parsers/Access/ParserCheckGrantQuery.h" namespace DB @@ -56,6 +57,7 @@ bool ParserQuery::parseImpl(Pos & pos, ASTPtr & node, Expected & expected) ParserDropAccessEntityQuery drop_access_entity_p; ParserMoveAccessEntityQuery move_access_entity_p; ParserGrantQuery grant_p; + ParserCheckGrantQuery check_grant_p; ParserSetRoleQuery set_role_p; ParserExternalDDLQuery external_ddl_p; ParserTransactionControl transaction_control_p; @@ -82,6 +84,7 @@ bool ParserQuery::parseImpl(Pos & pos, ASTPtr & node, Expected & expected) || drop_access_entity_p.parse(pos, node, expected) || move_access_entity_p.parse(pos, node, expected) || grant_p.parse(pos, node, expected) + || check_grant_p.parse(pos, node, expected) || external_ddl_p.parse(pos, node, expected) || transaction_control_p.parse(pos, node, expected) || delete_p.parse(pos, node, expected); From a3cd89a0b0598d118cf0678b8a36459261ae56dc Mon Sep 17 00:00:00 2001 From: unashi Date: Mon, 26 Aug 2024 18:13:40 +0800 Subject: [PATCH 02/10] [test] Add test case --- .../integration/test_check_grant/__init__.py | 0 .../configs/remote_servers.xml | 17 +++ .../test_check_grant/configs/users.xml | 121 ++++++++++++++++++ tests/integration/test_check_grant/test.py | 60 +++++++++ 4 files changed, 198 insertions(+) create mode 100644 tests/integration/test_check_grant/__init__.py create mode 100644 tests/integration/test_check_grant/configs/remote_servers.xml create mode 100644 tests/integration/test_check_grant/configs/users.xml create mode 100644 tests/integration/test_check_grant/test.py diff --git a/tests/integration/test_check_grant/__init__.py b/tests/integration/test_check_grant/__init__.py new file mode 100644 index 00000000000..e69de29bb2d diff --git a/tests/integration/test_check_grant/configs/remote_servers.xml b/tests/integration/test_check_grant/configs/remote_servers.xml new file mode 100644 index 00000000000..ea4769f55e1 --- /dev/null +++ b/tests/integration/test_check_grant/configs/remote_servers.xml @@ -0,0 +1,17 @@ + + + + + true + + node1 + 9000 + + + node2 + 9000 + + + + + diff --git a/tests/integration/test_check_grant/configs/users.xml b/tests/integration/test_check_grant/configs/users.xml new file mode 100644 index 00000000000..4a56df660df --- /dev/null +++ b/tests/integration/test_check_grant/configs/users.xml @@ -0,0 +1,121 @@ + + + + + + + + + + + + 1 + + + + + + + + + + + + + ::/0 + + + + default + + + default + + + 1 + + + 1 + 1 + 1 + + + + + + + + + + + + + 3600 + + + 0 + 0 + 0 + 0 + 0 + + + + diff --git a/tests/integration/test_check_grant/test.py b/tests/integration/test_check_grant/test.py new file mode 100644 index 00000000000..e22d5dcaa5f --- /dev/null +++ b/tests/integration/test_check_grant/test.py @@ -0,0 +1,60 @@ +#!/usr/bin/env python3 + +import logging +import pytest +from helpers.cluster import ClickHouseCluster + +@pytest.fixture(scope="module") +def cluster(): + try: + cluster = ClickHouseCluster(__file__) + cluster.add_instance( + "node1", + user_configs=[ + "configs/users.xml" + ] + ) + + cluster.add_instance( + "node2", + user_configs=[ + "configs/users.xml", + ] + ) + logging.info("Starting cluster...") + cluster.start() + logging.info("Cluster started") + + yield cluster + finally: + cluster.shutdown() + + +def test_check_grant(cluster): + node1 = cluster.instances["node1"] + + node1.query("DROP user IF EXISTS tuser") + node1.query("CREATE USER tuser") + node1.query("GRANT SELECT ON tb TO tuser") + # Has been granted but not table not exists + res, _ = node1.query("CHECK GRANT SELECT ON tb", user = "tuser") + assert res == "0" + + node1.query("CREATE TABLE tb (`content` UInt64) ENGINE = MergeTree ORDER BY content") + node1.query("INSERT INTO tb VALUES (1)") + # Has been granted and table exists + res, _ = node1.query("CHECK GRANT SELECT ON tb", user = "tuser") + assert res == "1" + + node1.query("REVOKE SELECT ON tb FROM tuser") + # Has not been granted but table exists + res, _ = node1.query("CHECK GRANT SELECT ON tb", user = "tuser") + assert res == "0" + + # Role + node1.query("CREATE ROLE trole") + node1.query("GRANT SELECT ON tb TO trole") + node1.query("GRANT trole TO tuser") + node1.query("SET ROLE trole", user = "tuser") + res, _ = node1.query("CHECK GRANT SELECT ON tb", user = "tuser") + assert res == "1" From fcfeb45d34f403006c0cec612958874981196bec Mon Sep 17 00:00:00 2001 From: unashi Date: Mon, 26 Aug 2024 20:33:09 +0800 Subject: [PATCH 03/10] [fix] Add formatImpl in AST --- src/Parsers/Access/ASTCheckGrantQuery.cpp | 98 ++++++++++++++++++++++- src/Parsers/Access/ASTCheckGrantQuery.h | 2 +- 2 files changed, 98 insertions(+), 2 deletions(-) diff --git a/src/Parsers/Access/ASTCheckGrantQuery.cpp b/src/Parsers/Access/ASTCheckGrantQuery.cpp index 364e73920ab..0f23fcce48a 100644 --- a/src/Parsers/Access/ASTCheckGrantQuery.cpp +++ b/src/Parsers/Access/ASTCheckGrantQuery.cpp @@ -1,6 +1,5 @@ #include #include -#include "Common/logger_useful.h" #include #include @@ -8,6 +7,92 @@ namespace DB { +namespace +{ + void formatColumnNames(const Strings & columns, const IAST::FormatSettings & settings) + { + settings.ostr << "("; + bool need_comma = false; + for (const auto & column : columns) + { + if (std::exchange(need_comma, true)) + settings.ostr << ", "; + settings.ostr << backQuoteIfNeed(column); + } + settings.ostr << ")"; + } + + + void formatONClause(const AccessRightsElement & element, const IAST::FormatSettings & settings) + { + settings.ostr << (settings.hilite ? IAST::hilite_keyword : "") << "ON " << (settings.hilite ? IAST::hilite_none : ""); + if (element.isGlobalWithParameter()) + { + if (element.any_parameter) + settings.ostr << "*"; + else + settings.ostr << backQuoteIfNeed(element.parameter); + } + else if (element.any_database) + { + settings.ostr << "*.*"; + } + else + { + if (!element.database.empty()) + settings.ostr << backQuoteIfNeed(element.database) << "."; + if (element.any_table) + settings.ostr << "*"; + else + settings.ostr << backQuoteIfNeed(element.table); + } + } + + + void formatElementsWithoutOptions(const AccessRightsElements & elements, const IAST::FormatSettings & settings) + { + bool no_output = true; + for (size_t i = 0; i != elements.size(); ++i) + { + const auto & element = elements[i]; + auto keywords = element.access_flags.toKeywords(); + if (keywords.empty() || (!element.any_column && element.columns.empty())) + continue; + + for (const auto & keyword : keywords) + { + if (!std::exchange(no_output, false)) + settings.ostr << ", "; + + settings.ostr << (settings.hilite ? IAST::hilite_keyword : "") << keyword << (settings.hilite ? IAST::hilite_none : ""); + if (!element.any_column) + formatColumnNames(element.columns, settings); + } + + bool next_element_on_same_db_and_table = false; + if (i != elements.size() - 1) + { + const auto & next_element = elements[i + 1]; + if (element.sameDatabaseAndTableAndParameter(next_element)) + { + next_element_on_same_db_and_table = true; + } + } + + if (!next_element_on_same_db_and_table) + { + settings.ostr << " "; + formatONClause(element, settings); + } + } + + if (no_output) + settings.ostr << (settings.hilite ? IAST::hilite_keyword : "") << "USAGE ON " << (settings.hilite ? IAST::hilite_none : "") << "*.*"; + } + +} + + String ASTCheckGrantQuery::getID(char) const { return "CheckGrantQuery"; @@ -22,6 +107,17 @@ ASTPtr ASTCheckGrantQuery::clone() const } +void ASTCheckGrantQuery::formatImpl(const FormatSettings & settings, FormatState &, FormatStateStacked) const +{ + settings.ostr << (settings.hilite ? IAST::hilite_keyword : "") << "CHECK GRANT" + << (settings.hilite ? IAST::hilite_none : ""); + + settings.ostr << " "; + + formatElementsWithoutOptions(access_rights_elements, settings); +} + + void ASTCheckGrantQuery::replaceEmptyDatabase(const String & current_database) { access_rights_elements.replaceEmptyDatabase(current_database); diff --git a/src/Parsers/Access/ASTCheckGrantQuery.h b/src/Parsers/Access/ASTCheckGrantQuery.h index 7e363d587ce..567ffdb289a 100644 --- a/src/Parsers/Access/ASTCheckGrantQuery.h +++ b/src/Parsers/Access/ASTCheckGrantQuery.h @@ -3,7 +3,6 @@ #include #include #include -#include "Parsers/ASTQueryParameter.h" namespace DB @@ -21,6 +20,7 @@ public: String getID(char) const override; ASTPtr clone() const override; + void formatImpl(const FormatSettings & settings, FormatState &, FormatStateStacked) const override; void replaceEmptyDatabase(const String & current_database); QueryKind getQueryKind() const override { return QueryKind::Check; } }; From cc6b40c5b17e8ae7b35514c6453e63bb3bb37521 Mon Sep 17 00:00:00 2001 From: unashi Date: Mon, 26 Aug 2024 20:40:04 +0800 Subject: [PATCH 04/10] [doc] Add doc --- .../sql-reference/statements/check-grant.md | 43 +++++++++++++++++++ .../sql-reference/statements/check-grant.mdx | 10 +++++ 2 files changed, 53 insertions(+) create mode 100644 docs/en/sql-reference/statements/check-grant.md create mode 100644 docs/zh/sql-reference/statements/check-grant.mdx diff --git a/docs/en/sql-reference/statements/check-grant.md b/docs/en/sql-reference/statements/check-grant.md new file mode 100644 index 00000000000..a1971eb59d1 --- /dev/null +++ b/docs/en/sql-reference/statements/check-grant.md @@ -0,0 +1,43 @@ +--- +slug: /en/sql-reference/statements/check-grant +sidebar_position: 56 +sidebar_label: CHECK GRANT +title: "CHECK GRANT Statement" +--- + +The `CHECK GRANT` query is used to check whether the current user/role has been granted a specific privilege, and whether the corresponding table/column exists in the memory. + +## Syntax + +The basic syntax of the query is as follows: + +```sql +CHECK GRANT privilege[(column_name [,...])] [,...] ON {db.table|db.*|*.*|table|*} +``` + +- `privilege` — Type of privilege. + +## Examples + +If the user used to be granted the privilege, or the role (which is granted with the privilege), and the db.table(column) exists on this node, the response`check_grant` will be `1`. Otherwise, the response `check_grant` will be `0`. + +If `table_1.col1` exists and current user is granted by privilege `SELECT`/`SELECT(con)` or role(with privilege), the response is `1`. +```sql +CHECK GRANT SELECT(col1) ON table_1; +``` + +```text +┌─CHECK_GRANT─┐ +│ 1 │ +└─────────────┘ +``` +If `table_2.col2` doesn't exists, or current user is not granted by privilege `SELECT`/`SELECT(con)` or role(with privilege), the response is `0`. +```sql +CHECK GRANT SELECT(col2) ON table_2; +``` + +```text +┌─CHECK_GRANT─┐ +│ 0 │ +└─────────────┘ +``` diff --git a/docs/zh/sql-reference/statements/check-grant.mdx b/docs/zh/sql-reference/statements/check-grant.mdx new file mode 100644 index 00000000000..60c95699a5e --- /dev/null +++ b/docs/zh/sql-reference/statements/check-grant.mdx @@ -0,0 +1,10 @@ +--- +slug: /zh/sql-reference/statements/check-grant +sidebar_position: 56 +sidebar_label: CHECK +title: "CHECK GRANT Statement" +--- + +import Content from '@site/docs/en/sql-reference/statements/check-grant.md'; + + From e0ae66ea77ce28d7264290483f49879ea52f7a22 Mon Sep 17 00:00:00 2001 From: unashi Date: Mon, 26 Aug 2024 20:46:04 +0800 Subject: [PATCH 05/10] [fix] Fix include format --- src/Interpreters/InterpreterFactory.cpp | 2 +- src/Parsers/ParserQuery.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Interpreters/InterpreterFactory.cpp b/src/Interpreters/InterpreterFactory.cpp index 477e2425c80..cae43d98560 100644 --- a/src/Interpreters/InterpreterFactory.cpp +++ b/src/Interpreters/InterpreterFactory.cpp @@ -40,6 +40,7 @@ #include #include #include +#include #include #include #include @@ -60,7 +61,6 @@ #include #include #include -#include "Parsers/Access/ASTCheckGrantQuery.h" #include diff --git a/src/Parsers/ParserQuery.cpp b/src/Parsers/ParserQuery.cpp index 92a83fe99f1..e939cdd07ba 100644 --- a/src/Parsers/ParserQuery.cpp +++ b/src/Parsers/ParserQuery.cpp @@ -26,9 +26,9 @@ #include #include #include +#include #include #include -#include "Parsers/Access/ParserCheckGrantQuery.h" namespace DB From b92c9da8590766954eba4bc626f5a4e575a84b59 Mon Sep 17 00:00:00 2001 From: unashi Date: Tue, 27 Aug 2024 10:25:07 +0800 Subject: [PATCH 06/10] [fix] Fix style check --- src/Parsers/Access/ParserCheckGrantQuery.cpp | 1 - tests/integration/test_check_grant/test.py | 24 +++++++++----------- 2 files changed, 11 insertions(+), 14 deletions(-) diff --git a/src/Parsers/Access/ParserCheckGrantQuery.cpp b/src/Parsers/Access/ParserCheckGrantQuery.cpp index c1daa435ad2..4be3d2a0d9f 100644 --- a/src/Parsers/Access/ParserCheckGrantQuery.cpp +++ b/src/Parsers/Access/ParserCheckGrantQuery.cpp @@ -16,7 +16,6 @@ namespace DB namespace ErrorCodes { extern const int INVALID_GRANT; - extern const int SYNTAX_ERROR; } namespace diff --git a/tests/integration/test_check_grant/test.py b/tests/integration/test_check_grant/test.py index e22d5dcaa5f..c20dfe86b16 100644 --- a/tests/integration/test_check_grant/test.py +++ b/tests/integration/test_check_grant/test.py @@ -4,22 +4,18 @@ import logging import pytest from helpers.cluster import ClickHouseCluster + @pytest.fixture(scope="module") def cluster(): try: cluster = ClickHouseCluster(__file__) - cluster.add_instance( - "node1", - user_configs=[ - "configs/users.xml" - ] - ) + cluster.add_instance("node1", user_configs=["configs/users.xml"]) cluster.add_instance( "node2", user_configs=[ "configs/users.xml", - ] + ], ) logging.info("Starting cluster...") cluster.start() @@ -37,24 +33,26 @@ def test_check_grant(cluster): node1.query("CREATE USER tuser") node1.query("GRANT SELECT ON tb TO tuser") # Has been granted but not table not exists - res, _ = node1.query("CHECK GRANT SELECT ON tb", user = "tuser") + res, _ = node1.query("CHECK GRANT SELECT ON tb", user="tuser") assert res == "0" - node1.query("CREATE TABLE tb (`content` UInt64) ENGINE = MergeTree ORDER BY content") + node1.query( + "CREATE TABLE tb (`content` UInt64) ENGINE = MergeTree ORDER BY content" + ) node1.query("INSERT INTO tb VALUES (1)") # Has been granted and table exists - res, _ = node1.query("CHECK GRANT SELECT ON tb", user = "tuser") + res, _ = node1.query("CHECK GRANT SELECT ON tb", user="tuser") assert res == "1" node1.query("REVOKE SELECT ON tb FROM tuser") # Has not been granted but table exists - res, _ = node1.query("CHECK GRANT SELECT ON tb", user = "tuser") + res, _ = node1.query("CHECK GRANT SELECT ON tb", user="tuser") assert res == "0" # Role node1.query("CREATE ROLE trole") node1.query("GRANT SELECT ON tb TO trole") node1.query("GRANT trole TO tuser") - node1.query("SET ROLE trole", user = "tuser") - res, _ = node1.query("CHECK GRANT SELECT ON tb", user = "tuser") + node1.query("SET ROLE trole", user="tuser") + res, _ = node1.query("CHECK GRANT SELECT ON tb", user="tuser") assert res == "1" From 844cf8d2528ef907d15d8e2950da7e16728039a6 Mon Sep 17 00:00:00 2001 From: unashi Date: Tue, 27 Aug 2024 12:32:48 +0800 Subject: [PATCH 07/10] [Fix] Fix code style --- src/Parsers/Access/ParserCheckGrantQuery.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Parsers/Access/ParserCheckGrantQuery.cpp b/src/Parsers/Access/ParserCheckGrantQuery.cpp index 4be3d2a0d9f..c7433b7d4df 100644 --- a/src/Parsers/Access/ParserCheckGrantQuery.cpp +++ b/src/Parsers/Access/ParserCheckGrantQuery.cpp @@ -216,7 +216,7 @@ bool ParserCheckGrantQuery::parseImpl(Pos & pos, ASTPtr & node, Expected & expec AccessRightsElements elements; - if (!parseElementsWithoutOptions(pos, expected, elements) ) + if (!parseElementsWithoutOptions(pos, expected, elements)) return false; throwIfNotGrantable(elements); From 4c62be2e0d4dea44d5245c6d850183c8c5dd83c7 Mon Sep 17 00:00:00 2001 From: unashi Date: Sun, 8 Sep 2024 16:09:01 +0800 Subject: [PATCH 08/10] [Update] Use stateless test instead of integration test --- .../0_stateless/03234_check_grant.reference | 4 ++++ .../queries/0_stateless/03234_check_grant.sh | 20 +++++++++++++++++++ 2 files changed, 24 insertions(+) create mode 100644 tests/queries/0_stateless/03234_check_grant.reference create mode 100755 tests/queries/0_stateless/03234_check_grant.sh diff --git a/tests/queries/0_stateless/03234_check_grant.reference b/tests/queries/0_stateless/03234_check_grant.reference new file mode 100644 index 00000000000..5565ed6787f --- /dev/null +++ b/tests/queries/0_stateless/03234_check_grant.reference @@ -0,0 +1,4 @@ +0 +1 +0 +1 diff --git a/tests/queries/0_stateless/03234_check_grant.sh b/tests/queries/0_stateless/03234_check_grant.sh new file mode 100755 index 00000000000..922f8fa96ac --- /dev/null +++ b/tests/queries/0_stateless/03234_check_grant.sh @@ -0,0 +1,20 @@ +#!/usr/bin/env bash +# Tags: no-parallel + +${CLICKHOUSE_CLIENT} --query "DROP USER IF EXISTS user_03234; DROP TABLE IF EXISTS tb;CREATE USER user_03234; GRANT SELECT ON tb TO user_03234;" + +# Has been granted but not table not exists +${CLICKHOUSE_CURL} -sS "${CLICKHOUSE_URL}&user=user_03234" --data-binary "CHECK GRANT SELECT ON tb" + +${CLICKHOUSE_CLIENT} --query "CREATE TABLE tb (\`content\` UInt64) ENGINE = MergeTree ORDER BY content; INSERT INTO tb VALUES (1);" +# Has been granted and table exists +${CLICKHOUSE_CURL} -sS "${CLICKHOUSE_URL}&user=user_03234" --data-binary "CHECK GRANT SELECT ON tb" + +${CLICKHOUSE_CLIENT} --query "REVOKE SELECT ON tb FROM user_03234;" +# Has not been granted but table exists +${CLICKHOUSE_CURL} -sS "${CLICKHOUSE_URL}&user=user_03234" --data-binary "CHECK GRANT SELECT ON tb" + +# Role +${CLICKHOUSE_CLIENT} --query "DROP ROLE IF EXISTS role_03234;CREATE ROLE role_03234;GRANT SELECT ON tb TO role_03234;GRANT role_03234 TO user_03234" +${CLICKHOUSE_CURL} -sS "${CLICKHOUSE_URL}&user=user_03234" --data-binary "SET ROLE role_03234" +${CLICKHOUSE_CURL} -sS "${CLICKHOUSE_URL}&user=user_03234" --data-binary "CHECK GRANT SELECT ON tb" From 9f8a29825d81c54e2bbe62c60fea37fc902de504 Mon Sep 17 00:00:00 2001 From: unashi Date: Sun, 8 Sep 2024 16:11:30 +0800 Subject: [PATCH 09/10] [rm] Remove integration test --- .../integration/test_check_grant/__init__.py | 0 .../configs/remote_servers.xml | 17 --- .../test_check_grant/configs/users.xml | 121 ------------------ tests/integration/test_check_grant/test.py | 58 --------- 4 files changed, 196 deletions(-) delete mode 100644 tests/integration/test_check_grant/__init__.py delete mode 100644 tests/integration/test_check_grant/configs/remote_servers.xml delete mode 100644 tests/integration/test_check_grant/configs/users.xml delete mode 100644 tests/integration/test_check_grant/test.py diff --git a/tests/integration/test_check_grant/__init__.py b/tests/integration/test_check_grant/__init__.py deleted file mode 100644 index e69de29bb2d..00000000000 diff --git a/tests/integration/test_check_grant/configs/remote_servers.xml b/tests/integration/test_check_grant/configs/remote_servers.xml deleted file mode 100644 index ea4769f55e1..00000000000 --- a/tests/integration/test_check_grant/configs/remote_servers.xml +++ /dev/null @@ -1,17 +0,0 @@ - - - - - true - - node1 - 9000 - - - node2 - 9000 - - - - - diff --git a/tests/integration/test_check_grant/configs/users.xml b/tests/integration/test_check_grant/configs/users.xml deleted file mode 100644 index 4a56df660df..00000000000 --- a/tests/integration/test_check_grant/configs/users.xml +++ /dev/null @@ -1,121 +0,0 @@ - - - - - - - - - - - - 1 - - - - - - - - - - - - - ::/0 - - - - default - - - default - - - 1 - - - 1 - 1 - 1 - - - - - - - - - - - - - 3600 - - - 0 - 0 - 0 - 0 - 0 - - - - diff --git a/tests/integration/test_check_grant/test.py b/tests/integration/test_check_grant/test.py deleted file mode 100644 index c20dfe86b16..00000000000 --- a/tests/integration/test_check_grant/test.py +++ /dev/null @@ -1,58 +0,0 @@ -#!/usr/bin/env python3 - -import logging -import pytest -from helpers.cluster import ClickHouseCluster - - -@pytest.fixture(scope="module") -def cluster(): - try: - cluster = ClickHouseCluster(__file__) - cluster.add_instance("node1", user_configs=["configs/users.xml"]) - - cluster.add_instance( - "node2", - user_configs=[ - "configs/users.xml", - ], - ) - logging.info("Starting cluster...") - cluster.start() - logging.info("Cluster started") - - yield cluster - finally: - cluster.shutdown() - - -def test_check_grant(cluster): - node1 = cluster.instances["node1"] - - node1.query("DROP user IF EXISTS tuser") - node1.query("CREATE USER tuser") - node1.query("GRANT SELECT ON tb TO tuser") - # Has been granted but not table not exists - res, _ = node1.query("CHECK GRANT SELECT ON tb", user="tuser") - assert res == "0" - - node1.query( - "CREATE TABLE tb (`content` UInt64) ENGINE = MergeTree ORDER BY content" - ) - node1.query("INSERT INTO tb VALUES (1)") - # Has been granted and table exists - res, _ = node1.query("CHECK GRANT SELECT ON tb", user="tuser") - assert res == "1" - - node1.query("REVOKE SELECT ON tb FROM tuser") - # Has not been granted but table exists - res, _ = node1.query("CHECK GRANT SELECT ON tb", user="tuser") - assert res == "0" - - # Role - node1.query("CREATE ROLE trole") - node1.query("GRANT SELECT ON tb TO trole") - node1.query("GRANT trole TO tuser") - node1.query("SET ROLE trole", user="tuser") - res, _ = node1.query("CHECK GRANT SELECT ON tb", user="tuser") - assert res == "1" From 73850421a216df25e7718cc4ad9bde94bf384212 Mon Sep 17 00:00:00 2001 From: unashi Date: Sun, 8 Sep 2024 17:19:36 +0800 Subject: [PATCH 10/10] [Fix] Add source env build in stateless test --- tests/queries/0_stateless/03234_check_grant.sh | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/queries/0_stateless/03234_check_grant.sh b/tests/queries/0_stateless/03234_check_grant.sh index 922f8fa96ac..29b33e64f2e 100755 --- a/tests/queries/0_stateless/03234_check_grant.sh +++ b/tests/queries/0_stateless/03234_check_grant.sh @@ -1,5 +1,8 @@ #!/usr/bin/env bash # Tags: no-parallel +CURDIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd) +# shellcheck source=../shell_config.sh +. "$CURDIR"/../shell_config.sh ${CLICKHOUSE_CLIENT} --query "DROP USER IF EXISTS user_03234; DROP TABLE IF EXISTS tb;CREATE USER user_03234; GRANT SELECT ON tb TO user_03234;"