From 0fa3fb359286054a0c2c97c7b68ffd1e0180f6fe Mon Sep 17 00:00:00 2001 From: kssenii Date: Fri, 3 Mar 2023 14:04:32 +0100 Subject: [PATCH] Fix show grants for user which has all grants --- src/Access/AccessRights.cpp | 23 +++++++++++++------ src/Access/Common/AccessRightsElement.cpp | 5 ++++ src/Access/Common/AccessRightsElement.h | 16 ++++++++++--- .../integration/test_grant_and_revoke/test.py | 3 +++ .../test_named_collections/test.py | 16 ++++++++++--- 5 files changed, 50 insertions(+), 13 deletions(-) diff --git a/src/Access/AccessRights.cpp b/src/Access/AccessRights.cpp index 37597552a41..cfa14e6c88b 100644 --- a/src/Access/AccessRights.cpp +++ b/src/Access/AccessRights.cpp @@ -124,20 +124,29 @@ namespace const auto & element = sorted[i]; if (element.access_flags) { - auto per_parameter = element.access_flags.splitIntoParameterTypes(); - if (per_parameter.size() == 1) + const bool all_granted = sorted.size() == 1 && element.access_flags.contains(AccessFlags::allFlags()); + if (all_granted) { /// Easy case: one Element is converted to one AccessRightsElement. res.emplace_back(element.getResult()); } else { - /// Difficult case: one element is converted into multiple AccessRightsElements. - for (const auto & [_, parameter_flags] : per_parameter) + auto per_parameter = element.access_flags.splitIntoParameterTypes(); + if (per_parameter.size() == 1) { - auto current_element{element}; - current_element.access_flags = parameter_flags; - res.emplace_back(current_element.getResult()); + /// Easy case: one Element is converted to one AccessRightsElement. + res.emplace_back(element.getResult()); + } + else + { + /// Difficult case: one element is converted into multiple AccessRightsElements. + for (const auto & [_, parameter_flags] : per_parameter) + { + auto current_element{element}; + current_element.access_flags = parameter_flags; + res.emplace_back(current_element.getResult()); + } } } } diff --git a/src/Access/Common/AccessRightsElement.cpp b/src/Access/Common/AccessRightsElement.cpp index 81cebd68b4c..e11d43634ec 100644 --- a/src/Access/Common/AccessRightsElement.cpp +++ b/src/Access/Common/AccessRightsElement.cpp @@ -233,6 +233,11 @@ bool AccessRightsElements::sameDatabaseAndTableAndParameter() const return (size() < 2) || std::all_of(std::next(begin()), end(), [this](const AccessRightsElement & e) { return e.sameDatabaseAndTableAndParameter(front()); }); } +bool AccessRightsElements::sameDatabaseAndTable() const +{ + return (size() < 2) || std::all_of(std::next(begin()), end(), [this](const AccessRightsElement & e) { return e.sameDatabaseAndTable(front()); }); +} + bool AccessRightsElements::sameOptions() const { return (size() < 2) || std::all_of(std::next(begin()), end(), [this](const AccessRightsElement & e) { return e.sameOptions(front()); }); diff --git a/src/Access/Common/AccessRightsElement.h b/src/Access/Common/AccessRightsElement.h index 96850f0880e..ba625fc43df 100644 --- a/src/Access/Common/AccessRightsElement.h +++ b/src/Access/Common/AccessRightsElement.h @@ -55,13 +55,22 @@ struct AccessRightsElement bool sameDatabaseAndTableAndParameter(const AccessRightsElement & other) const { - return (database == other.database) && (any_database == other.any_database) - && (table == other.table) && (any_table == other.any_table) - && (parameter == other.parameter) && (any_parameter == other.any_parameter) + return sameDatabaseAndTable(other) && sameParameter(other); + } + + bool sameParameter(const AccessRightsElement & other) const + { + return (parameter == other.parameter) && (any_parameter == other.any_parameter) && (access_flags.getParameterType() == other.access_flags.getParameterType()) && (isGlobalWithParameter() == other.isGlobalWithParameter()); } + bool sameDatabaseAndTable(const AccessRightsElement & other) const + { + return (database == other.database) && (any_database == other.any_database) + && (table == other.table) && (any_table == other.any_table); + } + bool sameOptions(const AccessRightsElement & other) const { return (grant_option == other.grant_option) && (is_partial_revoke == other.is_partial_revoke); @@ -92,6 +101,7 @@ public: bool empty() const; bool sameDatabaseAndTableAndParameter() const; + bool sameDatabaseAndTable() const; bool sameOptions() const; /// Resets flags which cannot be granted. diff --git a/tests/integration/test_grant_and_revoke/test.py b/tests/integration/test_grant_and_revoke/test.py index 8d48f7449e4..4d89e6255d3 100644 --- a/tests/integration/test_grant_and_revoke/test.py +++ b/tests/integration/test_grant_and_revoke/test.py @@ -402,6 +402,9 @@ def test_introspection(): assert instance.query("SHOW GRANTS FOR B") == TSV( ["GRANT CREATE ON *.* TO B WITH GRANT OPTION"] ) + assert instance.query("SHOW GRANTS FOR default") == TSV( + ["GRANT ALL ON *.* TO default WITH GRANT OPTION"] + ) assert instance.query("SHOW GRANTS FOR A,B") == TSV( [ "GRANT SELECT ON test.table TO A", diff --git a/tests/integration/test_named_collections/test.py b/tests/integration/test_named_collections/test.py index 1f27826d213..5574c77b886 100644 --- a/tests/integration/test_named_collections/test.py +++ b/tests/integration/test_named_collections/test.py @@ -100,7 +100,9 @@ def test_default_access(cluster): ) node.restart_clickhouse() assert ( - node.query("select collection['key1'] from system.named_collections").strip() + node.query( + "select collection['key1'] from system.named_collections where name = 'collection1'" + ).strip() == "value1" ) replace_in_users_config( @@ -111,7 +113,9 @@ def test_default_access(cluster): ) node.restart_clickhouse() assert ( - node.query("select collection['key1'] from system.named_collections").strip() + node.query( + "select collection['key1'] from system.named_collections where name = 'collection1'" + ).strip() == "[HIDDEN]" ) replace_in_users_config( @@ -122,13 +126,19 @@ def test_default_access(cluster): ) node.restart_clickhouse() assert ( - node.query("select collection['key1'] from system.named_collections").strip() + node.query( + "select collection['key1'] from system.named_collections where name = 'collection1'" + ).strip() == "value1" ) def test_granular_access_show_query(cluster): node = cluster.instances["node"] + assert ( + "GRANT ALL ON *.* TO default WITH GRANT OPTION" + == node.query("SHOW GRANTS FOR default").strip() + ) # includes named collections control assert 1 == int(node.query("SELECT count() FROM system.named_collections")) assert ( "collection1" == node.query("SELECT name FROM system.named_collections").strip()