From ae2f586170b49e40d1f93688336c45e0fb1a712f Mon Sep 17 00:00:00 2001 From: Vitaly Baranov Date: Fri, 1 Jul 2022 12:18:09 +0200 Subject: [PATCH] Fix crash when granting ALL on cluster. --- src/Access/ContextAccess.cpp | 22 ++++++++++++------- .../test_access_control_on_cluster/test.py | 10 +++++++++ 2 files changed, 24 insertions(+), 8 deletions(-) diff --git a/src/Access/ContextAccess.cpp b/src/Access/ContextAccess.cpp index 221113cb425..995a46d07ca 100644 --- a/src/Access/ContextAccess.cpp +++ b/src/Access/ContextAccess.cpp @@ -412,15 +412,18 @@ bool ContextAccess::checkAccessImplHelper(AccessFlags flags, const Args &... arg return false; }; + if (is_full_access) + return access_granted(); + + if (user_was_dropped) + return access_denied("User has been dropped", ErrorCodes::UNKNOWN_USER); + if (flags & AccessType::CLUSTER && !access_control->doesOnClusterQueriesRequireClusterGrant()) flags &= ~AccessType::CLUSTER; - if (!flags || is_full_access) + if (!flags) return access_granted(); - if (!tryGetUser()) - return access_denied("User has been dropped", ErrorCodes::UNKNOWN_USER); - /// Access to temporary tables is controlled in an unusual way, not like normal tables. /// Creating of temporary tables is controlled by AccessType::CREATE_TEMPORARY_TABLES grant, /// and other grants are considered as always given. @@ -600,9 +603,6 @@ void ContextAccess::checkGrantOption(const AccessRightsElements & elements) cons template bool ContextAccess::checkAdminOptionImplHelper(const Container & role_ids, const GetNameFunction & get_name_function) const { - if (!std::size(role_ids) || is_full_access) - return true; - auto show_error = [this](const String & msg, int error_code [[maybe_unused]]) { UNUSED(this); @@ -610,12 +610,18 @@ bool ContextAccess::checkAdminOptionImplHelper(const Container & role_ids, const throw Exception(getUserName() + ": " + msg, error_code); }; - if (!tryGetUser()) + if (is_full_access) + return true; + + if (user_was_dropped) { show_error("User has been dropped", ErrorCodes::UNKNOWN_USER); return false; } + if (!std::size(role_ids)) + return true; + if (isGranted(AccessType::ROLE_ADMIN)) return true; diff --git a/tests/integration/test_access_control_on_cluster/test.py b/tests/integration/test_access_control_on_cluster/test.py index 6c2331178e0..db76233a35f 100644 --- a/tests/integration/test_access_control_on_cluster/test.py +++ b/tests/integration/test_access_control_on_cluster/test.py @@ -49,3 +49,13 @@ def test_access_control_on_cluster(): assert "There is no user `Alex`" in ch1.query_and_get_error("SHOW CREATE USER Alex") assert "There is no user `Alex`" in ch2.query_and_get_error("SHOW CREATE USER Alex") assert "There is no user `Alex`" in ch3.query_and_get_error("SHOW CREATE USER Alex") + + +def test_grant_all_on_cluster(): + ch1.query("CREATE USER IF NOT EXISTS Alex ON CLUSTER 'cluster'") + ch1.query("GRANT ALL ON *.* TO Alex ON CLUSTER 'cluster'") + + assert ch1.query("SHOW GRANTS FOR Alex") == "GRANT ALL ON *.* TO Alex\n" + assert ch2.query("SHOW GRANTS FOR Alex") == "GRANT ALL ON *.* TO Alex\n" + + ch1.query("DROP USER Alex ON CLUSTER 'cluster'")