Merge pull request ClickHouse#68355 from zvonand/tryfix-65762-again

Some fixes for LDAP

src/Access/LDAPAccessStorage.cpp

@@ -191,8 +191,8 @@ void LDAPAccessStorage::applyRoleChangeNoLock(bool grant, const UUID & role_id,
     }
     else
     {
-        granted_role_names.erase(role_id);
         granted_role_ids.erase(role_name);
+        granted_role_names.erase(role_id);
     }
 }
 

tests/integration/helpers/cluster.py

@@ -4687,6 +4687,9 @@ def write_embedded_config(name, dest_dir, fix_log_level=False):
         if self.with_kerberized_hdfs:
             depends_on.append("kerberizedhdfs1")
 
+        if self.with_ldap:
+            depends_on.append("openldap")
+
         if self.with_rabbitmq:
             depends_on.append("rabbitmq1")
 

tests/integration/test_ldap_external_user_directory/test.py

@@ -48,9 +48,32 @@ def add_ldap_group(ldap_cluster, group_cn, member_cn):
     assert code == 0
 
 
+def delete_ldap_group(ldap_cluster, group_cn):
+    code, (stdout, stderr) = ldap_cluster.ldap_container.exec_run(
+        [
+            "sh",
+            "-c",
+            """ldapdelete -r 'cn={group_cn},dc=example,dc=org' \
+-H ldap://{host}:{port} -D "{admin_bind_dn}" -x -w {admin_password}
+            """.format(
+                host=ldap_cluster.ldap_host,
+                port=ldap_cluster.ldap_port,
+                admin_bind_dn=LDAP_ADMIN_BIND_DN,
+                admin_password=LDAP_ADMIN_PASSWORD,
+                group_cn=group_cn,
+            ),
+        ],
+        demux=True,
+    )
+    logging.debug(
+        f"test_ldap_external_user_directory code:{code} stdout:{stdout}, stderr:{stderr}"
+    )
+    assert code == 0
+
+
 def test_authentication_pass():
     assert instance.query(
-        "select currentUser()", user="janedoe", password="qwerty"
+        "SELECT currentUser()", user="janedoe", password="qwerty"
     ) == TSV([["janedoe"]])
 
 
@@ -67,6 +90,9 @@ def test_authentication_fail():
 
 
 def test_role_mapping(ldap_cluster):
+    instance.query("DROP ROLE IF EXISTS role_1")
+    instance.query("DROP ROLE IF EXISTS role_2")
+    instance.query("DROP ROLE IF EXISTS role_3")
     instance.query("CREATE ROLE role_1")
     instance.query("CREATE ROLE role_2")
     add_ldap_group(ldap_cluster, group_cn="clickhouse-role_1", member_cn="johndoe")
@@ -93,3 +119,12 @@ def test_role_mapping(ldap_cluster):
         user="johndoe",
         password="qwertz",
     ) == TSV([["role_1"], ["role_2"], ["role_3"]])
+
+    instance.query("DROP ROLE role_1")
+    instance.query("DROP ROLE role_2")
+    instance.query("DROP ROLE role_3")
+
+    delete_ldap_group(ldap_cluster, group_cn="clickhouse-role_1")
+    delete_ldap_group(ldap_cluster, group_cn="clickhouse-role_2")
+    delete_ldap_group(ldap_cluster, group_cn="clickhouse-role_3")
+    delete_ldap_group(ldap_cluster, group_cn="clickhouse-role_4")