This commit is contained in:
pufit 2024-11-20 15:24:57 -08:00 committed by GitHub
commit 8ee949cfbd
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 51 additions and 16 deletions

View File

@ -358,8 +358,7 @@ AccessRightsElements AccessRestorerFromBackup::getRequiredAccess() const
auto elements = user.access.getElements();
for (auto & element : elements)
{
if (element.is_partial_revoke)
continue;
if (!element.is_partial_revoke)
element.grant_option = true;
res.emplace_back(element);
}
@ -375,8 +374,7 @@ AccessRightsElements AccessRestorerFromBackup::getRequiredAccess() const
auto elements = role.access.getElements();
for (auto & element : elements)
{
if (element.is_partial_revoke)
continue;
if (!element.is_partial_revoke)
element.grant_option = true;
res.emplace_back(element);
}

View File

@ -1209,7 +1209,13 @@ AccessRights::AccessRights(const AccessRightsElement & element)
AccessRights::AccessRights(const AccessRightsElements & elements)
{
grant(elements);
for (const auto & element : elements)
{
if (element.is_partial_revoke)
revoke(element);
else
grant(element);
}
}
@ -1457,18 +1463,18 @@ bool AccessRights::isGrantedImpl(const AccessFlags & flags, const Args &... args
template <bool grant_option>
bool AccessRights::containsImpl(const AccessRights & other) const
{
auto helper = [&](const std::unique_ptr<Node> & root_node) -> bool
auto helper = [&](const std::unique_ptr<Node> & root_node, const std::unique_ptr<Node> & other_root_node) -> bool
{
if (!root_node)
return !other.root;
if (!other.root)
if (!other_root_node)
return true;
return root_node->contains(*other.root);
return root_node->contains(*other_root_node);
};
if constexpr (grant_option)
return helper(root_with_grant_option);
return helper(root_with_grant_option, other.root);
else
return helper(root);
return helper(root, other.root) && helper(root_with_grant_option, other.root_with_grant_option);
}

View File

@ -8,6 +8,7 @@
#include <Backups/DDLAdjustingForBackupVisitor.h>
#include <Access/AccessBackup.h>
#include <Access/AccessRights.h>
#include <Access/ContextAccess.h>
#include <Parsers/ParserCreateQuery.h>
#include <Parsers/parseQuery.h>
#include <Parsers/formatAST.h>
@ -704,9 +705,14 @@ void RestorerFromBackup::checkAccessForObjectsFoundInBackup() const
insertAtEnd(required_access, access_restorer->getRequiredAccess());
}
/// We convert to AccessRights and back to check access rights in a predictable way
/// (some elements could be duplicated or not sorted).
required_access = AccessRights{required_access}.getElements();
/// We convert to AccessRights to check if we have the rights contained in our current access.
/// This handles the situation when restoring partial revoked grants:
/// GRANT SELECT ON *.*
/// REVOKE SELECT FROM systems.*
const auto & required_access_rights = AccessRights{required_access};
const auto & access_rights = context->getAccess()->getAccessRights();
if (access_rights->contains(required_access_rights))
return;
context->checkAccess(required_access);
}

View File

@ -8,6 +8,7 @@ from collections import namedtuple
import pytest
from helpers.client import QueryRuntimeException
from helpers.cluster import ClickHouseCluster
from helpers.test_tools import TSV, assert_eq_with_retry
@ -49,7 +50,7 @@ def cleanup_after_test():
instance.query("DROP DATABASE IF EXISTS test")
instance.query("DROP DATABASE IF EXISTS test2")
instance.query("DROP DATABASE IF EXISTS test3")
instance.query("DROP USER IF EXISTS u1")
instance.query("DROP USER IF EXISTS u1, u2")
instance.query("DROP ROLE IF EXISTS r1, r2")
instance.query("DROP SETTINGS PROFILE IF EXISTS prof1")
instance.query("DROP ROW POLICY IF EXISTS rowpol1 ON test.table")
@ -1845,6 +1846,30 @@ def test_tables_dependency():
drop()
def test_required_privileges_with_partial_revokes():
backup_name = new_backup_name()
instance.query("CREATE USER u1")
instance.query("GRANT SELECT ON *.* TO u1")
instance.query("REVOKE SELECT ON system.zookeeper* FROM u1")
instance.query("REVOKE SELECT ON foo.* FROM u1")
instance.query(f"BACKUP ALL FROM system.users TO {backup_name}")
instance.query("DROP USER u1")
instance.query("CREATE USER u2")
instance.query("GRANT SELECT ON *.* TO u2 WITH GRANT OPTION")
instance.query("GRANT CREATE USER ON *.* TO u2")
instance.query("REVOKE SELECT ON system.zookeeper* FROM u2")
instance.query("REVOKE SELECT ON foo.* FROM u2")
instance.query(f"RESTORE ALL FROM {backup_name}", user="u2")
instance.query("DROP USER u1")
instance.query("REVOKE SELECT ON foo* FROM u2")
with pytest.raises(QueryRuntimeException):
instance.query(f"RESTORE ALL FROM {backup_name}", user="u2")
# Test for the "clickhouse_backupview" utility.
test_backupview_dir = os.path.abspath(