diff --git a/src/Access/AccessFlags.h b/src/Access/AccessFlags.h index cbba295be1a..54a8b9cde76 100644 --- a/src/Access/AccessFlags.h +++ b/src/Access/AccessFlags.h @@ -61,6 +61,10 @@ public: friend bool operator ==(const AccessFlags & left, const AccessFlags & right) { return left.flags == right.flags; } friend bool operator !=(const AccessFlags & left, const AccessFlags & right) { return !(left == right); } + friend bool operator <(const AccessFlags & left, const AccessFlags & right) { return memcmp(&left.flags, &right.flags, sizeof(Flags)) < 0; } + friend bool operator >(const AccessFlags & left, const AccessFlags & right) { return right < left; } + friend bool operator <=(const AccessFlags & left, const AccessFlags & right) { return !(right < left); } + friend bool operator >=(const AccessFlags & left, const AccessFlags & right) { return !(left < right); } void clear() { flags.reset(); } diff --git a/src/Access/AccessRights.cpp b/src/Access/AccessRights.cpp index 79d77e3f352..086d740ede1 100644 --- a/src/Access/AccessRights.cpp +++ b/src/Access/AccessRights.cpp @@ -606,69 +606,102 @@ void AccessRights::revoke(const AccessRightsElements & elements, std::string_vie } -AccessRights::Elements AccessRights::getElements() const +AccessRightsElements AccessRights::getGrants() const +{ + AccessRightsElements grants; + getGrantsAndPartialRevokesImpl(&grants, nullptr); + return grants; +} + +AccessRightsElements AccessRights::getPartialRevokes() const +{ + AccessRightsElements partial_revokes; + getGrantsAndPartialRevokesImpl(nullptr, &partial_revokes); + return partial_revokes; +} + +AccessRights::GrantsAndPartialRevokes AccessRights::getGrantsAndPartialRevokes() const +{ + GrantsAndPartialRevokes res; + getGrantsAndPartialRevokesImpl(&res.grants, &res.revokes); + return res; +} + + +void AccessRights::getGrantsAndPartialRevokesImpl(AccessRightsElements * out_grants, AccessRightsElements * out_partial_revokes) const { if (!root) - return {}; - Elements res; + return; auto global_access = root->access; - if (global_access) - res.grants.push_back({global_access}); + if (out_grants && global_access) + out_grants->push_back({global_access}); if (root->children) { for (const auto & [db_name, db_node] : *root->children) { - auto db_grants = db_node.access - global_access; - auto db_partial_revokes = global_access - db_node.access; - if (db_partial_revokes) - res.partial_revokes.push_back({db_partial_revokes, db_name}); - if (db_grants) - res.grants.push_back({db_grants, db_name}); + if (out_grants) + { + if (auto db_grants = db_node.access - global_access) + out_grants->push_back({db_grants, db_name}); + } + if (out_partial_revokes) + { + if (auto db_partial_revokes = global_access - db_node.access) + out_partial_revokes->push_back({db_partial_revokes, db_name}); + } if (db_node.children) { for (const auto & [table_name, table_node] : *db_node.children) { - auto table_grants = table_node.access - db_node.access; - auto table_partial_revokes = db_node.access - table_node.access; - if (table_partial_revokes) - res.partial_revokes.push_back({table_partial_revokes, db_name, table_name}); - if (table_grants) - res.grants.push_back({table_grants, db_name, table_name}); + if (out_grants) + { + if (auto table_grants = table_node.access - db_node.access) + out_grants->push_back({table_grants, db_name, table_name}); + } + if (out_partial_revokes) + { + if (auto table_partial_revokes = db_node.access - table_node.access) + out_partial_revokes->push_back({table_partial_revokes, db_name, table_name}); + } if (table_node.children) { for (const auto & [column_name, column_node] : *table_node.children) { - auto column_grants = column_node.access - table_node.access; - auto column_partial_revokes = table_node.access - column_node.access; - if (column_partial_revokes) - res.partial_revokes.push_back({column_partial_revokes, db_name, table_name, column_name}); - if (column_grants) - res.grants.push_back({column_grants, db_name, table_name, column_name}); + if (out_grants) + { + if (auto column_grants = column_node.access - table_node.access) + out_grants->push_back({column_grants, db_name, table_name, column_name}); + } + if (out_partial_revokes) + { + if (auto column_partial_revokes = table_node.access - column_node.access) + out_partial_revokes->push_back({column_partial_revokes, db_name, table_name, column_name}); + } } + } } } } } - return res; } String AccessRights::toString() const { - auto elements = getElements(); String res; - if (!elements.grants.empty()) + auto gr = getGrantsAndPartialRevokes(); + if (!gr.grants.empty()) { res += "GRANT "; - res += elements.grants.toString(); + res += gr.grants.toString(); } - if (!elements.partial_revokes.empty()) + if (!gr.revokes.empty()) { if (!res.empty()) res += ", "; res += "REVOKE "; - res += elements.partial_revokes.toString(); + res += gr.revokes.toString(); } if (res.empty()) res = "GRANT USAGE ON *.*"; diff --git a/src/Access/AccessRights.h b/src/Access/AccessRights.h index 133038f2d44..c32514e8feb 100644 --- a/src/Access/AccessRights.h +++ b/src/Access/AccessRights.h @@ -49,12 +49,14 @@ public: void revoke(const AccessRightsElements & elements, std::string_view current_database = {}); /// Returns the information about all the access granted. - struct Elements + struct GrantsAndPartialRevokes { AccessRightsElements grants; - AccessRightsElements partial_revokes; + AccessRightsElements revokes; }; - Elements getElements() const; + AccessRightsElements getGrants() const; + AccessRightsElements getPartialRevokes() const; + GrantsAndPartialRevokes getGrantsAndPartialRevokes() const; /// Returns the information about all the access granted as a string. String toString() const; @@ -92,6 +94,8 @@ private: template AccessFlags getAccessImpl(const Args &... args) const; + void getGrantsAndPartialRevokesImpl(AccessRightsElements * grants, AccessRightsElements * partial_revokes) const; + void logTree() const; struct Node; diff --git a/src/Access/AccessRightsElement.cpp b/src/Access/AccessRightsElement.cpp index 81237a334e2..db1ea5d3d5c 100644 --- a/src/Access/AccessRightsElement.cpp +++ b/src/Access/AccessRightsElement.cpp @@ -1,10 +1,124 @@ #include #include #include +#include +#include +#include +#include +#include namespace DB { +namespace +{ + size_t groupElements(AccessRightsElements & elements, size_t start) + { + auto & start_element = elements[start]; + auto it = std::find_if(elements.begin() + start + 1, elements.end(), + [&](const AccessRightsElement & element) + { + return (element.database != start_element.database) || + (element.any_database != start_element.any_database) || + (element.table != start_element.table) || + (element.any_table != start_element.any_table) || + (element.any_column != start_element.any_column); + }); + size_t end = it - elements.begin(); + + /// All the elements at indices from start to end here specify + /// the same database and table. + + if (start_element.any_column) + { + /// Easy case: the elements don't specify columns. + /// All we need is to combine the access flags. + for (size_t i = start + 1; i != end; ++i) + { + start_element.access_flags |= elements[i].access_flags; + elements[i].access_flags = {}; + } + return end; + } + + /// Difficult case: the elements specify columns. + /// We have to find groups of columns with common access flags. + for (size_t i = start; i != end; ++i) + { + if (!elements[i].access_flags) + continue; + + AccessFlags common_flags = elements[i].access_flags; + size_t num_elements_with_common_flags = 1; + for (size_t j = i + 1; j != end; ++j) + { + auto new_common_flags = common_flags & elements[j].access_flags; + if (new_common_flags) + { + common_flags = new_common_flags; + ++num_elements_with_common_flags; + } + } + + if (num_elements_with_common_flags == 1) + continue; + + if (elements[i].access_flags != common_flags) + { + elements.insert(elements.begin() + i + 1, elements[i]); + elements[i].access_flags = common_flags; + elements[i].columns.clear(); + ++end; + } + + for (size_t j = i + 1; j != end; ++j) + { + if ((elements[j].access_flags & common_flags) == common_flags) + { + boost::range::push_back(elements[i].columns, elements[j].columns); + elements[j].access_flags -= common_flags; + } + } + } + + return end; + } + + /// Tries to combine elements to decrease their number. + void groupElements(AccessRightsElements & elements) + { + if (!boost::range::is_sorted(elements)) + boost::range::sort(elements); /// Algorithm in groupElement() requires elements to be sorted. + for (size_t start = 0; start != elements.size();) + start = groupElements(elements, start); + } + + /// Removes unnecessary elements, sorts elements and makes them unique. + void sortElementsAndMakeUnique(AccessRightsElements & elements) + { + /// Remove empty elements. + boost::range::remove_erase_if(elements, [](const AccessRightsElement & element) + { + return !element.access_flags || (!element.any_column && element.columns.empty()); + }); + + /// Sort columns and make them unique. + for (auto & element : elements) + { + if (element.any_column) + continue; + + if (!boost::range::is_sorted(element.columns)) + boost::range::sort(element.columns); + element.columns.erase(std::unique(element.columns.begin(), element.columns.end()), element.columns.end()); + } + + /// Sort elements themselves. + boost::range::sort(elements); + elements.erase(std::unique(elements.begin(), elements.end()), elements.end()); + } +} + void AccessRightsElement::setDatabase(const String & new_database) { database = new_database; @@ -26,10 +140,29 @@ bool AccessRightsElement::isEmptyDatabase() const String AccessRightsElement::toString() const +{ + String msg = toStringWithoutON(); + msg += " ON "; + + if (any_database) + msg += "*."; + else if (!database.empty()) + msg += backQuoteIfNeed(database) + "."; + + if (any_table) + msg += "*"; + else + msg += backQuoteIfNeed(table); + return msg; +} + +String AccessRightsElement::toStringWithoutON() const { String columns_in_parentheses; if (!any_column) { + if (columns.empty()) + return "USAGE"; for (const auto & column : columns) { columns_in_parentheses += columns_in_parentheses.empty() ? "(" : ", "; @@ -38,25 +171,17 @@ String AccessRightsElement::toString() const columns_in_parentheses += ")"; } + auto keywords = access_flags.toKeywords(); + if (keywords.empty()) + return "USAGE"; + String msg; - for (const std::string_view & keyword : access_flags.toKeywords()) + for (const std::string_view & keyword : keywords) { if (!msg.empty()) msg += ", "; msg += String{keyword} + columns_in_parentheses; } - - msg += " ON "; - - if (any_database) - msg += "*."; - else if (!database.empty() && (database != IDictionary::NO_DATABASE_TAG)) - msg += backQuoteIfNeed(database) + "."; - - if (any_table) - msg += "*"; - else - msg += backQuoteIfNeed(table); return msg; } @@ -68,19 +193,41 @@ void AccessRightsElements::replaceEmptyDatabase(const String & new_database) } -String AccessRightsElements::toString() const +String AccessRightsElements::toString() { - String res; - bool need_comma = false; - for (const auto & element : *this) - { - if (std::exchange(need_comma, true)) - res += ", "; - res += element.toString(); - } + normalize(); - if (res.empty()) - res = "USAGE ON *.*"; - return res; + if (empty()) + return "USAGE ON *.*"; + + String msg; + bool need_comma = false; + for (size_t i = 0; i != size(); ++i) + { + const auto & element = (*this)[i]; + if (std::exchange(need_comma, true)) + msg += ", "; + bool next_element_on_same_db_and_table = false; + if (i != size() - 1) + { + const auto & next_element = (*this)[i + 1]; + if ((element.database == next_element.database) && (element.any_database == next_element.any_database) + && (element.table == next_element.table) && (element.any_table == next_element.any_table)) + next_element_on_same_db_and_table = true; + } + if (next_element_on_same_db_and_table) + msg += element.toStringWithoutON(); + else + msg += element.toString(); + } + return msg; } + + +void AccessRightsElements::normalize() +{ + groupElements(*this); + sortElementsAndMakeUnique(*this); +} + } diff --git a/src/Access/AccessRightsElement.h b/src/Access/AccessRightsElement.h index 3894b6f5157..70eb95c2d17 100644 --- a/src/Access/AccessRightsElement.h +++ b/src/Access/AccessRightsElement.h @@ -71,6 +71,14 @@ struct AccessRightsElement { } + auto toTuple() const { return std::tie(access_flags, database, any_database, table, any_table, columns, any_column); } + friend bool operator==(const AccessRightsElement & left, const AccessRightsElement & right) { return left.toTuple() == right.toTuple(); } + friend bool operator!=(const AccessRightsElement & left, const AccessRightsElement & right) { return left.toTuple() != right.toTuple(); } + friend bool operator<(const AccessRightsElement & left, const AccessRightsElement & right) { return left.toTuple() < right.toTuple(); } + friend bool operator>(const AccessRightsElement & left, const AccessRightsElement & right) { return left.toTuple() > right.toTuple(); } + friend bool operator<=(const AccessRightsElement & left, const AccessRightsElement & right) { return left.toTuple() <= right.toTuple(); } + friend bool operator>=(const AccessRightsElement & left, const AccessRightsElement & right) { return left.toTuple() >= right.toTuple(); } + /// Sets the database. void setDatabase(const String & new_database); @@ -82,6 +90,7 @@ struct AccessRightsElement /// Returns a human-readable representation like "SELECT, UPDATE(x, y) ON db.table". /// The returned string isn't prefixed with the "GRANT" keyword. String toString() const; + String toStringWithoutON() const; }; @@ -94,7 +103,11 @@ public: /// Returns a human-readable representation like "SELECT, UPDATE(x, y) ON db.table". /// The returned string isn't prefixed with the "GRANT" keyword. - String toString() const; + String toString() const { return AccessRightsElements(*this).toString(); } + String toString(); + + /// Reorder and group elements to show them in more readable form. + void normalize(); }; } diff --git a/src/Access/AccessType.h b/src/Access/AccessType.h index d0665a6e55f..c4fdbc46b71 100644 --- a/src/Access/AccessType.h +++ b/src/Access/AccessType.h @@ -167,50 +167,48 @@ enum class AccessType #undef DECLARE_ACCESS_TYPE_ENUM_CONST }; -std::string_view toString(AccessType type); - namespace impl { template - class AccessTypeToKeywordConverter + class AccessTypeToStringConverter { public: - static const AccessTypeToKeywordConverter & instance() + static const AccessTypeToStringConverter & instance() { - static const AccessTypeToKeywordConverter res; + static const AccessTypeToStringConverter res; return res; } std::string_view convert(AccessType type) const { - return access_type_to_keyword_mapping[static_cast(type)]; + return access_type_to_string_mapping[static_cast(type)]; } private: - AccessTypeToKeywordConverter() + AccessTypeToStringConverter() { -#define INSERT_ACCESS_TYPE_KEYWORD_PAIR_TO_MAPPING(name, aliases, node_type, parent_group_name) \ - insertToMapping(AccessType::name, #name); +#define ACCESS_TYPE_TO_STRING_CONVERTER_ADD_TO_MAPPING(name, aliases, node_type, parent_group_name) \ + addToMapping(AccessType::name, #name); - APPLY_FOR_ACCESS_TYPES(INSERT_ACCESS_TYPE_KEYWORD_PAIR_TO_MAPPING) + APPLY_FOR_ACCESS_TYPES(ACCESS_TYPE_TO_STRING_CONVERTER_ADD_TO_MAPPING) -#undef INSERT_ACCESS_TYPE_KEYWORD_PAIR_TO_MAPPING +#undef ACCESS_TYPE_TO_STRING_CONVERTER_ADD_TO_MAPPING } - void insertToMapping(AccessType type, const std::string_view & str) + void addToMapping(AccessType type, const std::string_view & str) { String str2{str}; boost::replace_all(str2, "_", " "); size_t index = static_cast(type); - access_type_to_keyword_mapping.resize(std::max(index + 1, access_type_to_keyword_mapping.size())); - access_type_to_keyword_mapping[index] = str2; + access_type_to_string_mapping.resize(std::max(index + 1, access_type_to_string_mapping.size())); + access_type_to_string_mapping[index] = str2; } - Strings access_type_to_keyword_mapping; + Strings access_type_to_string_mapping; }; } -inline std::string_view toKeyword(AccessType type) { return impl::AccessTypeToKeywordConverter<>::instance().convert(type); } +inline std::string_view toString(AccessType type) { return impl::AccessTypeToStringConverter<>::instance().convert(type); } } diff --git a/src/Access/ContextAccess.cpp b/src/Access/ContextAccess.cpp index ab504e32579..3a06a9aeead 100644 --- a/src/Access/ContextAccess.cpp +++ b/src/Access/ContextAccess.cpp @@ -134,12 +134,12 @@ void ContextAccess::setUser(const UserPtr & user_) const std::vector current_roles, current_roles_with_admin_option; if (params.use_default_roles) { - for (const UUID & id : user->granted_roles) + for (const UUID & id : user->granted_roles.roles) { if (user->default_roles.match(id)) current_roles.push_back(id); } - boost::range::set_intersection(current_roles, user->granted_roles_with_admin_option, + boost::range::set_intersection(current_roles, user->granted_roles.roles_with_admin_option, std::back_inserter(current_roles_with_admin_option)); } else @@ -147,9 +147,9 @@ void ContextAccess::setUser(const UserPtr & user_) const current_roles.reserve(params.current_roles.size()); for (const auto & id : params.current_roles) { - if (user->granted_roles.count(id)) + if (user->granted_roles.roles.contains(id)) current_roles.push_back(id); - if (user->granted_roles_with_admin_option.count(id)) + if (user->granted_roles.roles_with_admin_option.contains(id)) current_roles_with_admin_option.push_back(id); } } @@ -397,13 +397,13 @@ boost::shared_ptr ContextAccess::calculateResultAccess(bool if (grant_option) { - *merged_access = user->access_with_grant_option; + *merged_access = user->access.access_with_grant_option; if (roles_info) merged_access->merge(roles_info->access_with_grant_option); } else { - *merged_access = user->access; + *merged_access = user->access.access; if (roles_info) merged_access->merge(roles_info->access); } diff --git a/src/Access/ExtendedRoleSet.cpp b/src/Access/ExtendedRoleSet.cpp index 145bd0fe7e5..adee5fea669 100644 --- a/src/Access/ExtendedRoleSet.cpp +++ b/src/Access/ExtendedRoleSet.cpp @@ -1,3 +1,4 @@ + #include #include #include @@ -43,12 +44,6 @@ ExtendedRoleSet::ExtendedRoleSet(const std::vector & ids_) } -ExtendedRoleSet::ExtendedRoleSet(const boost::container::flat_set & ids_) -{ - add(ids_); -} - - ExtendedRoleSet::ExtendedRoleSet(const ASTExtendedRoleSet & ast) { init(ast, nullptr); @@ -126,6 +121,7 @@ std::shared_ptr ExtendedRoleSet::toAST() const ast->names.reserve(ids.size()); for (const UUID & id : ids) ast->names.emplace_back(::DB::toString(id)); + boost::range::sort(ast->names); } if (!except_ids.empty()) @@ -133,6 +129,7 @@ std::shared_ptr ExtendedRoleSet::toAST() const ast->except_names.reserve(except_ids.size()); for (const UUID & except_id : except_ids) ast->except_names.emplace_back(::DB::toString(except_id)); + boost::range::sort(ast->except_names); } return ast; @@ -244,13 +241,6 @@ void ExtendedRoleSet::add(const std::vector & ids_) } -void ExtendedRoleSet::add(const boost::container::flat_set & ids_) -{ - for (const auto & id : ids_) - add(id); -} - - bool ExtendedRoleSet::match(const UUID & id) const { return (all || ids.count(id)) && !except_ids.count(id); diff --git a/src/Access/ExtendedRoleSet.h b/src/Access/ExtendedRoleSet.h index 486b4277337..ba84ade49f8 100644 --- a/src/Access/ExtendedRoleSet.h +++ b/src/Access/ExtendedRoleSet.h @@ -28,7 +28,6 @@ struct ExtendedRoleSet ExtendedRoleSet(const UUID & id); ExtendedRoleSet(const std::vector & ids_); - ExtendedRoleSet(const boost::container::flat_set & ids_); /// The constructor from AST requires the AccessControlManager if `ast.id_mode == false`. ExtendedRoleSet(const ASTExtendedRoleSet & ast); @@ -48,7 +47,6 @@ struct ExtendedRoleSet void clear(); void add(const UUID & id); void add(const std::vector & ids_); - void add(const boost::container::flat_set & ids_); /// Checks if a specified ID matches this ExtendedRoleSet. bool match(const UUID & id) const; diff --git a/src/Access/GrantedAccess.cpp b/src/Access/GrantedAccess.cpp new file mode 100644 index 00000000000..2af1e0b44ec --- /dev/null +++ b/src/Access/GrantedAccess.cpp @@ -0,0 +1,22 @@ +#include + + +namespace DB +{ + +GrantedAccess::GrantsAndPartialRevokes GrantedAccess::getGrantsAndPartialRevokes() const +{ + GrantsAndPartialRevokes res; + res.grants_with_grant_option = access_with_grant_option.getGrants(); + AccessRights access_without_gg = access; + access_without_gg.revoke(res.grants_with_grant_option); + auto gr = access_without_gg.getGrantsAndPartialRevokes(); + res.grants = std::move(gr.grants); + res.revokes = std::move(gr.revokes); + AccessRights access_with_grant_options_without_r = access_with_grant_option; + access_with_grant_options_without_r.grant(res.revokes); + res.revokes_grant_option = access_with_grant_options_without_r.getPartialRevokes(); + return res; +} + +} diff --git a/src/Access/GrantedAccess.h b/src/Access/GrantedAccess.h new file mode 100644 index 00000000000..b8f6bdfe8fb --- /dev/null +++ b/src/Access/GrantedAccess.h @@ -0,0 +1,55 @@ +#pragma once + +#include + + +namespace DB +{ +/// Access rights as they are granted to a role or user. +/// Stores both the access rights themselves and the access rights with grant option. +struct GrantedAccess +{ + AccessRights access; + AccessRights access_with_grant_option; + + template + void grant(const Args &... args) + { + access.grant(args...); + } + + template + void grantWithGrantOption(const Args &... args) + { + access.grant(args...); + access_with_grant_option.grant(args...); + } + + template + void revoke(const Args &... args) + { + access.revoke(args...); + access_with_grant_option.revoke(args...); + } + + template + void revokeGrantOption(const Args &... args) + { + access_with_grant_option.revoke(args...); + } + + struct GrantsAndPartialRevokes + { + AccessRightsElements grants; + AccessRightsElements revokes; + AccessRightsElements grants_with_grant_option; + AccessRightsElements revokes_grant_option; + }; + + /// Retrieves the information about grants and partial revokes. + GrantsAndPartialRevokes getGrantsAndPartialRevokes() const; + + friend bool operator ==(const GrantedAccess & left, const GrantedAccess & right) { return (left.access == right.access) && (left.access_with_grant_option == right.access_with_grant_option); } + friend bool operator !=(const GrantedAccess & left, const GrantedAccess & right) { return !(left == right); } +}; +} diff --git a/src/Access/GrantedRoles.cpp b/src/Access/GrantedRoles.cpp new file mode 100644 index 00000000000..4d7007c4db6 --- /dev/null +++ b/src/Access/GrantedRoles.cpp @@ -0,0 +1,64 @@ +#include +#include + + +namespace DB +{ +void GrantedRoles::grant(const UUID & role) +{ + roles.insert(role); +} + +void GrantedRoles::grant(const std::vector & roles_) +{ + for (const UUID & role : roles_) + grant(role); +} + +void GrantedRoles::grantWithAdminOption(const UUID & role) +{ + roles.insert(role); + roles_with_admin_option.insert(role); +} + +void GrantedRoles::grantWithAdminOption(const std::vector & roles_) +{ + for (const UUID & role : roles_) + grantWithAdminOption(role); +} + + +void GrantedRoles::revoke(const UUID & role) +{ + roles.erase(role); + roles_with_admin_option.erase(role); +} + +void GrantedRoles::revoke(const std::vector & roles_) +{ + for (const UUID & role : roles_) + revoke(role); +} + +void GrantedRoles::revokeAdminOption(const UUID & role) +{ + roles_with_admin_option.erase(role); +} + +void GrantedRoles::revokeAdminOption(const std::vector & roles_) +{ + for (const UUID & role : roles_) + revokeAdminOption(role); +} + + +GrantedRoles::Grants GrantedRoles::getGrants() const +{ + Grants res; + res.grants_with_admin_option.insert(res.grants_with_admin_option.end(), roles_with_admin_option.begin(), roles_with_admin_option.end()); + res.grants.reserve(roles.size() - roles_with_admin_option.size()); + boost::range::set_difference(roles, roles_with_admin_option, std::back_inserter(res.grants)); + return res; +} + +} diff --git a/src/Access/GrantedRoles.h b/src/Access/GrantedRoles.h new file mode 100644 index 00000000000..fd091755a80 --- /dev/null +++ b/src/Access/GrantedRoles.h @@ -0,0 +1,39 @@ +#pragma once + +#include +#include +#include + + +namespace DB +{ +/// Roles when they are granted to a role or user. +/// Stores both the roles themselves and the roles with admin option. +struct GrantedRoles +{ + boost::container::flat_set roles; + boost::container::flat_set roles_with_admin_option; + + void grant(const UUID & role); + void grant(const std::vector & roles_); + void grantWithAdminOption(const UUID & role); + void grantWithAdminOption(const std::vector & roles_); + + void revoke(const UUID & role); + void revoke(const std::vector & roles_); + void revokeAdminOption(const UUID & role); + void revokeAdminOption(const std::vector & roles_); + + struct Grants + { + std::vector grants; + std::vector grants_with_admin_option; + }; + + /// Retrieves the information about grants. + Grants getGrants() const; + + friend bool operator ==(const GrantedRoles & left, const GrantedRoles & right) { return (left.roles == right.roles) && (left.roles_with_admin_option == right.roles_with_admin_option); } + friend bool operator !=(const GrantedRoles & left, const GrantedRoles & right) { return !(left == right); } +}; +} diff --git a/src/Access/Role.cpp b/src/Access/Role.cpp index f20ef9b9bfa..d7bec28c576 100644 --- a/src/Access/Role.cpp +++ b/src/Access/Role.cpp @@ -9,9 +9,6 @@ bool Role::equal(const IAccessEntity & other) const if (!IAccessEntity::equal(other)) return false; const auto & other_role = typeid_cast(other); - return (access == other_role.access) && (access_with_grant_option == other_role.access_with_grant_option) - && (granted_roles == other_role.granted_roles) && (granted_roles_with_admin_option == other_role.granted_roles_with_admin_option) - && (settings == other_role.settings); + return (access == other_role.access) && (granted_roles == other_role.granted_roles) && (settings == other_role.settings); } - } diff --git a/src/Access/Role.h b/src/Access/Role.h index 04330ba85f5..01a5c6ea2ce 100644 --- a/src/Access/Role.h +++ b/src/Access/Role.h @@ -1,10 +1,9 @@ #pragma once #include -#include +#include +#include #include -#include -#include namespace DB @@ -12,10 +11,8 @@ namespace DB struct Role : public IAccessEntity { - AccessRights access; - AccessRights access_with_grant_option; - boost::container::flat_set granted_roles; - boost::container::flat_set granted_roles_with_admin_option; + GrantedAccess access; + GrantedRoles granted_roles; SettingsProfileElements settings; bool equal(const IAccessEntity & other) const override; diff --git a/src/Access/RoleCache.cpp b/src/Access/RoleCache.cpp index 0263b793017..441e3844a07 100644 --- a/src/Access/RoleCache.cpp +++ b/src/Access/RoleCache.cpp @@ -37,10 +37,10 @@ namespace if (!role) return; - for (const auto & granted_role : role->granted_roles) + for (const auto & granted_role : role->granted_roles.roles) collectRoles(collected_roles, get_role_function, granted_role, false, false); - for (const auto & granted_role : role->granted_roles_with_admin_option) + for (const auto & granted_role : role->granted_roles.roles_with_admin_option) collectRoles(collected_roles, get_role_function, granted_role, false, true); } @@ -59,8 +59,8 @@ namespace if (collect_info.with_admin_option) new_info->enabled_roles_with_admin_option.emplace_back(role_id); new_info->names_of_roles[role_id] = role->getName(); - new_info->access.merge(role->access); - new_info->access_with_grant_option.merge(role->access_with_grant_option); + new_info->access.merge(role->access.access); + new_info->access_with_grant_option.merge(role->access.access_with_grant_option); new_info->settings_from_enabled_roles.merge(role->settings); } return new_info; diff --git a/src/Access/User.cpp b/src/Access/User.cpp index 4a751c31e25..459357731ed 100644 --- a/src/Access/User.cpp +++ b/src/Access/User.cpp @@ -10,9 +10,7 @@ bool User::equal(const IAccessEntity & other) const return false; const auto & other_user = typeid_cast(other); return (authentication == other_user.authentication) && (allowed_client_hosts == other_user.allowed_client_hosts) - && (access == other_user.access) && (access_with_grant_option == other_user.access_with_grant_option) - && (granted_roles == other_user.granted_roles) && (granted_roles_with_admin_option == other_user.granted_roles_with_admin_option) - && (default_roles == other_user.default_roles) && (settings == other_user.settings); + && (access == other_user.access) && (granted_roles == other_user.granted_roles) && (default_roles == other_user.default_roles) + && (settings == other_user.settings); } - } diff --git a/src/Access/User.h b/src/Access/User.h index 6df3b3e4d3c..b20f6538e4d 100644 --- a/src/Access/User.h +++ b/src/Access/User.h @@ -3,11 +3,10 @@ #include #include #include -#include +#include +#include #include #include -#include -#include namespace DB @@ -18,10 +17,8 @@ struct User : public IAccessEntity { Authentication authentication; AllowedClientHosts allowed_client_hosts = AllowedClientHosts::AnyHostTag{}; - AccessRights access; - AccessRights access_with_grant_option; - boost::container::flat_set granted_roles; - boost::container::flat_set granted_roles_with_admin_option; + GrantedAccess access; + GrantedRoles granted_roles; ExtendedRoleSet default_roles = ExtendedRoleSet::AllTag{}; SettingsProfileElements settings; diff --git a/src/Access/UsersConfigAccessStorage.cpp b/src/Access/UsersConfigAccessStorage.cpp index 0842839dec8..c4335640b25 100644 --- a/src/Access/UsersConfigAccessStorage.cpp +++ b/src/Access/UsersConfigAccessStorage.cpp @@ -151,30 +151,30 @@ namespace } } - user->access.grant(AccessType::ALL); /// By default all databases are accessible. + /// By default all databases are accessible + /// and the user can grant everything he has. + user->access.grantWithGrantOption(AccessType::ALL); if (databases) { user->access.revoke(AccessFlags::allFlags() - AccessFlags::allGlobalFlags()); - user->access.grant(AccessFlags::allDictionaryFlags(), IDictionary::NO_DATABASE_TAG); + user->access.grantWithGrantOption(AccessFlags::allDictionaryFlags(), IDictionary::NO_DATABASE_TAG); for (const String & database : *databases) - user->access.grant(AccessFlags::allFlags(), database); + user->access.grantWithGrantOption(AccessFlags::allFlags(), database); } if (dictionaries) { user->access.revoke(AccessFlags::allDictionaryFlags(), IDictionary::NO_DATABASE_TAG); for (const String & dictionary : *dictionaries) - user->access.grant(AccessFlags::allDictionaryFlags(), IDictionary::NO_DATABASE_TAG, dictionary); + user->access.grantWithGrantOption(AccessFlags::allDictionaryFlags(), IDictionary::NO_DATABASE_TAG, dictionary); } - user->access_with_grant_option = user->access; /// By default the user can grant everything he has. - bool access_management = config.getBool(user_config + ".access_management", false); if (!access_management) { user->access.revoke(AccessType::ACCESS_MANAGEMENT); - user->access_with_grant_option.clear(); + user->access.revokeGrantOption(AccessType::ALL); } return user; diff --git a/src/Access/ya.make b/src/Access/ya.make index fb2e23e0684..5b85330a7f4 100644 --- a/src/Access/ya.make +++ b/src/Access/ya.make @@ -18,6 +18,8 @@ SRCS( EnabledRowPolicies.cpp EnabledSettings.cpp ExtendedRoleSet.cpp + GrantedAccess.cpp + GrantedRoles.cpp IAccessEntity.cpp IAccessStorage.cpp MemoryAccessStorage.cpp diff --git a/src/Interpreters/InterpreterCreateUserQuery.cpp b/src/Interpreters/InterpreterCreateUserQuery.cpp index e4900e5c518..7c488ddf8e9 100644 --- a/src/Interpreters/InterpreterCreateUserQuery.cpp +++ b/src/Interpreters/InterpreterCreateUserQuery.cpp @@ -48,7 +48,7 @@ namespace if (default_roles) { if (!query.alter && !default_roles->all) - boost::range::copy(default_roles->getMatchingIDs(), std::inserter(user.granted_roles, user.granted_roles.end())); + user.granted_roles.grant(default_roles->getMatchingIDs()); InterpreterSetRoleQuery::updateUserSetDefaultRoles(user, *default_roles); } diff --git a/src/Interpreters/InterpreterGrantQuery.cpp b/src/Interpreters/InterpreterGrantQuery.cpp index a5f13dbbbfe..c72e48c2019 100644 --- a/src/Interpreters/InterpreterGrantQuery.cpp +++ b/src/Interpreters/InterpreterGrantQuery.cpp @@ -23,14 +23,16 @@ namespace { if (query.kind == Kind::GRANT) { - grantee.access.grant(query.access_rights_elements, current_database); if (query.grant_option) - grantee.access_with_grant_option.grant(query.access_rights_elements, current_database); + grantee.access.grantWithGrantOption(query.access_rights_elements, current_database); + else + grantee.access.grant(query.access_rights_elements, current_database); } else { - grantee.access_with_grant_option.revoke(query.access_rights_elements, current_database); - if (!query.grant_option) + if (query.grant_option) + grantee.access.revokeGrantOption(query.access_rights_elements, current_database); + else grantee.access.revoke(query.access_rights_elements, current_database); } } @@ -39,18 +41,21 @@ namespace { if (query.kind == Kind::GRANT) { - boost::range::copy(roles_from_query, std::inserter(grantee.granted_roles, grantee.granted_roles.end())); if (query.admin_option) - boost::range::copy(roles_from_query, std::inserter(grantee.granted_roles_with_admin_option, grantee.granted_roles_with_admin_option.end())); + grantee.granted_roles.grantWithAdminOption(roles_from_query); + else + grantee.granted_roles.grant(roles_from_query); } else { - for (const UUID & role_from_query : roles_from_query) + if (query.admin_option) + grantee.granted_roles.revokeAdminOption(roles_from_query); + else + grantee.granted_roles.revoke(roles_from_query); + + if constexpr (std::is_same_v) { - grantee.granted_roles_with_admin_option.erase(role_from_query); - if (!query.admin_option) - grantee.granted_roles.erase(role_from_query); - if constexpr (std::is_same_v) + for (const UUID & role_from_query : roles_from_query) grantee.default_roles.ids.erase(role_from_query); } } diff --git a/src/Interpreters/InterpreterSetRoleQuery.cpp b/src/Interpreters/InterpreterSetRoleQuery.cpp index 8f085d66c4c..f56926332ec 100644 --- a/src/Interpreters/InterpreterSetRoleQuery.cpp +++ b/src/Interpreters/InterpreterSetRoleQuery.cpp @@ -42,7 +42,7 @@ void InterpreterSetRoleQuery::setRole(const ASTSetRoleQuery & query) std::vector new_current_roles; if (roles_from_query.all) { - for (const auto & id : user->granted_roles) + for (const auto & id : user->granted_roles.roles) if (roles_from_query.match(id)) new_current_roles.push_back(id); } @@ -50,7 +50,7 @@ void InterpreterSetRoleQuery::setRole(const ASTSetRoleQuery & query) { for (const auto & id : roles_from_query.getMatchingIDs()) { - if (!user->granted_roles.count(id)) + if (!user->granted_roles.roles.contains(id)) throw Exception("Role should be granted to set current", ErrorCodes::SET_NON_GRANTED_ROLE); new_current_roles.push_back(id); } @@ -85,7 +85,7 @@ void InterpreterSetRoleQuery::updateUserSetDefaultRoles(User & user, const Exten { for (const auto & id : roles_from_query.getMatchingIDs()) { - if (!user.granted_roles.count(id)) + if (!user.granted_roles.roles.contains(id)) throw Exception("Role should be granted to set default", ErrorCodes::SET_NON_GRANTED_ROLE); } } diff --git a/src/Interpreters/InterpreterShowGrantsQuery.cpp b/src/Interpreters/InterpreterShowGrantsQuery.cpp index da1d46f0cab..aa139b8e10e 100644 --- a/src/Interpreters/InterpreterShowGrantsQuery.cpp +++ b/src/Interpreters/InterpreterShowGrantsQuery.cpp @@ -10,8 +10,6 @@ #include #include #include -#include -#include namespace DB @@ -23,37 +21,6 @@ namespace ErrorCodes namespace { - std::vector groupByTable(AccessRightsElements && elements) - { - using Key = std::tuple; - std::map grouping_map; - for (auto & element : elements) - { - Key key(element.database, element.any_database, element.table, element.any_table); - grouping_map[key].emplace_back(std::move(element)); - } - std::vector res; - res.reserve(grouping_map.size()); - boost::range::copy(grouping_map | boost::adaptors::map_values, std::back_inserter(res)); - return res; - } - - - struct GroupedGrantsAndPartialRevokes - { - std::vector grants; - std::vector partial_revokes; - }; - - GroupedGrantsAndPartialRevokes groupByTable(AccessRights::Elements && elements) - { - GroupedGrantsAndPartialRevokes res; - res.grants = groupByTable(std::move(elements.grants)); - res.partial_revokes = groupByTable(std::move(elements.partial_revokes)); - return res; - } - - template ASTs getGrantQueriesImpl( const T & grantee, @@ -65,35 +32,51 @@ namespace std::shared_ptr to_roles = std::make_shared(); to_roles->names.push_back(grantee.getName()); - for (bool grant_option : {true, false}) - { - if (!grant_option && (grantee.access == grantee.access_with_grant_option)) - continue; - const auto & access_rights = grant_option ? grantee.access_with_grant_option : grantee.access; - const auto grouped_elements = groupByTable(access_rights.getElements()); + auto grants_and_partial_revokes = grantee.access.getGrantsAndPartialRevokes(); + for (bool grant_option : {false, true}) + { using Kind = ASTGrantQuery::Kind; for (Kind kind : {Kind::GRANT, Kind::REVOKE}) { - for (const auto & elements : (kind == Kind::GRANT ? grouped_elements.grants : grouped_elements.partial_revokes)) + AccessRightsElements * elements = nullptr; + if (grant_option) + elements = (kind == Kind::GRANT) ? &grants_and_partial_revokes.grants_with_grant_option : &grants_and_partial_revokes.revokes_grant_option; + else + elements = (kind == Kind::GRANT) ? &grants_and_partial_revokes.grants : &grants_and_partial_revokes.revokes; + elements->normalize(); + + std::shared_ptr grant_query = nullptr; + for (size_t i = 0; i != elements->size(); ++i) { - auto grant_query = std::make_shared(); - grant_query->kind = kind; - grant_query->attach = attach_mode; - grant_query->grant_option = grant_option; - grant_query->to_roles = to_roles; - grant_query->access_rights_elements = elements; - res.push_back(std::move(grant_query)); + const auto & element = (*elements)[i]; + bool prev_element_on_same_db_and_table = false; + if (grant_query) + { + const auto & prev_element = grant_query->access_rights_elements.back(); + if ((element.database == prev_element.database) && (element.any_database == prev_element.any_database) + && (element.table == prev_element.table) && (element.any_table == prev_element.any_table)) + prev_element_on_same_db_and_table = true; + } + if (!prev_element_on_same_db_and_table) + { + grant_query = std::make_shared(); + grant_query->kind = kind; + grant_query->attach = attach_mode; + grant_query->grant_option = grant_option; + grant_query->to_roles = to_roles; + res.push_back(grant_query); + } + grant_query->access_rights_elements.emplace_back(std::move(element)); } } } - for (bool admin_option : {true, false}) - { - if (!admin_option && (grantee.granted_roles == grantee.granted_roles_with_admin_option)) - continue; + auto grants_roles = grantee.granted_roles.getGrants(); - const auto & roles = admin_option ? grantee.granted_roles_with_admin_option : grantee.granted_roles; + for (bool admin_option : {false, true}) + { + const auto & roles = admin_option ? grants_roles.grants_with_admin_option : grants_roles.grants; if (roles.empty()) continue; diff --git a/src/Interpreters/tests/users.cpp b/src/Interpreters/tests/users.cpp index acd0cfd0519..5c7d66ed7ed 100644 --- a/src/Interpreters/tests/users.cpp +++ b/src/Interpreters/tests/users.cpp @@ -218,7 +218,7 @@ void runOneTest(const TestDescriptor & test_descriptor) try { - res = acl_manager.read(entry.user_name)->access.isGranted(DB::AccessType::ALL, entry.database_name); + res = acl_manager.read(entry.user_name)->access.access.isGranted(DB::AccessType::ALL, entry.database_name); } catch (const Poco::Exception &) { diff --git a/src/Parsers/ASTGrantQuery.cpp b/src/Parsers/ASTGrantQuery.cpp index e6764fc067a..8114bef0766 100644 --- a/src/Parsers/ASTGrantQuery.cpp +++ b/src/Parsers/ASTGrantQuery.cpp @@ -1,10 +1,6 @@ #include #include #include -#include -#include -#include -#include namespace DB @@ -16,61 +12,13 @@ namespace ErrorCodes namespace { - using KeywordToColumnsMap = std::map /* columns */>; - using TableToAccessMap = std::map; - - TableToAccessMap prepareTableToAccessMap(const AccessRightsElements & elements) + void formatColumnNames(const Strings & columns, const IAST::FormatSettings & settings) { - TableToAccessMap res; - for (const auto & element : elements) - { - String database_and_table_name; - if (element.any_database) - { - if (element.any_table) - database_and_table_name = "*.*"; - else - database_and_table_name = "*." + backQuoteIfNeed(element.table); - } - else if (element.database.empty()) - { - if (element.any_table) - database_and_table_name = "*"; - else - database_and_table_name = backQuoteIfNeed(element.table); - } - else - { - if (element.any_table) - database_and_table_name = backQuoteIfNeed(element.database) + ".*"; - else - database_and_table_name = backQuoteIfNeed(element.database) + "." + backQuoteIfNeed(element.table); - } - - KeywordToColumnsMap & keyword_to_columns = res[database_and_table_name]; - for (const auto & keyword : element.access_flags.toKeywords()) - boost::range::push_back(keyword_to_columns[keyword], element.columns); - } - - for (auto & keyword_to_columns : res | boost::adaptors::map_values) - { - for (auto & columns : keyword_to_columns | boost::adaptors::map_values) - boost::range::sort(columns); - } - return res; - } - - - void formatColumnNames(const std::vector & columns, const IAST::FormatSettings & settings) - { - if (columns.empty()) - return; - settings.ostr << "("; - bool need_comma_after_column_name = false; + bool need_comma = false; for (const auto & column : columns) { - if (std::exchange(need_comma_after_column_name, true)) + if (std::exchange(need_comma, true)) settings.ostr << ", "; settings.ostr << backQuoteIfNeed(column); } @@ -80,20 +28,50 @@ namespace void formatAccessRightsElements(const AccessRightsElements & elements, const IAST::FormatSettings & settings) { - bool need_comma = false; - for (const auto & [database_and_table, keyword_to_columns] : prepareTableToAccessMap(elements)) + bool no_output = true; + for (size_t i = 0; i != elements.size(); ++i) { - for (const auto & [keyword, columns] : keyword_to_columns) + 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(need_comma, true)) + if (!std::exchange(no_output, false)) settings.ostr << ", "; settings.ostr << (settings.hilite ? IAST::hilite_keyword : "") << keyword << (settings.hilite ? IAST::hilite_none : ""); - formatColumnNames(columns, settings); + if (!element.any_column) + formatColumnNames(element.columns, settings); } - settings.ostr << (settings.hilite ? IAST::hilite_keyword : "") << " ON " << (settings.hilite ? IAST::hilite_none : "") << database_and_table; + bool next_element_on_same_db_and_table = false; + if (i != elements.size() - 1) + { + const auto & next_element = elements[i + 1]; + if ((element.database == next_element.database) && (element.any_database == next_element.any_database) + && (element.table == next_element.table) && (element.any_table == next_element.any_table)) + next_element_on_same_db_and_table = true; + } + + if (!next_element_on_same_db_and_table) + { + settings.ostr << (settings.hilite ? IAST::hilite_keyword : "") << " ON " << (settings.hilite ? IAST::hilite_none : ""); + 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); + } } + + if (no_output) + settings.ostr << (settings.hilite ? IAST::hilite_keyword : "") << "USAGE ON " << (settings.hilite ? IAST::hilite_none : "") << "*.*"; } @@ -134,7 +112,7 @@ void ASTGrantQuery::formatImpl(const FormatSettings & settings, FormatState &, F settings.ostr << (settings.hilite ? hilite_keyword : "") << " ADMIN OPTION FOR" << (settings.hilite ? hilite_none : ""); } - if ((!!roles + !access_rights_elements.empty()) != 1) + if (roles && !access_rights_elements.empty()) throw Exception("Either roles or access rights elements should be set", ErrorCodes::LOGICAL_ERROR); settings.ostr << " "; diff --git a/tests/queries/0_stateless/01073_grant_and_revoke.reference b/tests/queries/0_stateless/01073_grant_and_revoke.reference index d7d97fa28fe..134256c8113 100644 --- a/tests/queries/0_stateless/01073_grant_and_revoke.reference +++ b/tests/queries/0_stateless/01073_grant_and_revoke.reference @@ -1,11 +1,11 @@ CREATE USER test_user_01073 A B -GRANT ALTER DELETE, INSERT ON *.* TO test_user_01073 GRANT SELECT ON db1.* TO test_user_01073 GRANT SELECT ON db2.table TO test_user_01073 GRANT SELECT(col1) ON db3.table TO test_user_01073 GRANT SELECT(col1, col2) ON db4.table TO test_user_01073 +GRANT INSERT, ALTER DELETE ON *.* TO test_user_01073 C -GRANT ALTER DELETE ON *.* TO test_user_01073 GRANT SELECT(col1) ON db4.table TO test_user_01073 +GRANT ALTER DELETE ON *.* TO test_user_01073