Merge pull request #33249 from nicelulu/fix_acl

clickhouse-keeper acl consistent with zookeeper, accept generated digest
ClickHouse · Dec 29, 2021 · 7e17675 · 7e17675
2 parents 24df9de + daa4c9e
commit 7e17675
Show file tree

Hide file tree

Showing 2 changed files with 6 additions and 10 deletions.
diff --git a/src/Coordination/KeeperStorage.cpp b/src/Coordination/KeeperStorage.cpp
@@ -91,8 +91,7 @@ static bool checkACL(int32_t permission, const Coordination::ACLs & node_acls, c
 static bool fixupACL(
     const std::vector<Coordination::ACL> & request_acls,
     const std::vector<KeeperStorage::AuthID> & current_ids,
-    std::vector<Coordination::ACL> & result_acls,
-    bool hash_acls)
+    std::vector<Coordination::ACL> & result_acls)
 {
     if (request_acls.empty())
         return true;
@@ -125,8 +124,6 @@ static bool fixupACL(
                 return false;
 
             valid_found = true;
-            if (hash_acls)
-                new_acl.id = generateDigest(new_acl.id);
             result_acls.push_back(new_acl);
         }
     }
@@ -310,7 +307,7 @@ struct KeeperStorageCreateRequestProcessor final : public KeeperStorageRequestPr
         KeeperStorage::Node created_node;
 
         Coordination::ACLs node_acls;
-        if (!fixupACL(request.acls, session_auth_ids, node_acls, !request.restored_from_zookeeper_log))
+        if (!fixupACL(request.acls, session_auth_ids, node_acls))
         {
             response.error = Coordination::Error::ZINVALIDACL;
             return {response_ptr, {}};
@@ -778,7 +775,7 @@ struct KeeperStorageSetACLRequestProcessor final : public KeeperStorageRequestPr
             auto & session_auth_ids = storage.session_and_auth[session_id];
             Coordination::ACLs node_acls;
 
-            if (!fixupACL(request.acls, session_auth_ids, node_acls, !request.restored_from_zookeeper_log))
+            if (!fixupACL(request.acls, session_auth_ids, node_acls))
             {
                 response.error = Coordination::Error::ZINVALIDACL;
                 return {response_ptr, {}};

diff --git a/tests/integration/test_keeper_auth/test.py b/tests/integration/test_keeper_auth/test.py
@@ -43,12 +43,11 @@ def test_digest_auth_basic(started_cluster, get_zk):
 
     auth_connection.create("/test_no_acl", b"")
     auth_connection.create("/test_all_acl", b"data", acl=[make_acl("auth", "", all=True)])
-    # for some reason original zookeeper accepts this ACL, but doesn't allow to do anything with this node
-    # even with correct credentials.
-    auth_connection.create("/test_all_digest_acl", b"dataX", acl=[make_acl("digest", "user1:password1", all=True)])
+    # Consistent with zookeeper, accept generated digest
+    auth_connection.create("/test_all_digest_acl", b"dataX", acl=[make_acl("digest", "user1:XDkd2dsEuhc9ImU3q8pa8UOdtpI=", all=True)])
 
     assert auth_connection.get("/test_all_acl")[0] == b"data"
-    #assert auth_connection.get("/test_all_digest_acl")[0] == b"dataX"
+    assert auth_connection.get("/test_all_digest_acl")[0] == b"dataX"
 
     no_auth_connection = get_zk()
     no_auth_connection.set("/test_no_acl", b"hello")