[KYUUBI #4152] Enhance LDAP authentication

### _Why are the changes needed?_ This PR proposes to enhance the LDAP support, which mainly referring the code introduced in HIVE-14713. Currently, Kyuubi has very limited LDAP support, and the implementation is from the early Hive codebase. Hive enhanced the LDAP support in later versions, considering the Hive ecosystem is quite mature, I think we'd better to porting this functionality and keep the same behavior w/ Hive first, and we can improve it if meet certain requirements/issues in the future. Basically, this PR introduces the following configurations ``` kyuubi.authentication.ldap.url (since 1.0.0) kyuubi.authentication.ldap.domain (since 1.0.0) kyuubi.authentication.ldap.guidKey (since 1.2.0) kyuubi.authentication.ldap.base.dn (since 1.0.0 deprecated) kyuubi.authentication.ldap.baseDN kyuubi.authentication.ldap.groupMembershipKey kyuubi.authentication.ldap.userMembershipKey kyuubi.authentication.ldap.groupClassKey kyuubi.authentication.ldap.groupDNPattern kyuubi.authentication.ldap.userDNPattern kyuubi.authentication.ldap.groupFilter kyuubi.authentication.ldap.userFilter kyuubi.authentication.ldap.customLDAPQuery kyuubi.authentication.ldap.binddn kyuubi.authentication.ldap.bindpw ``` ### _How was this patch tested?_ - [x] Add some test cases that check the changes thoroughly including negative and positive cases if possible This PR ports all LDAP-related UT&IT from Hive codebase - [ ] Add screenshots for manual tests if appropriate - [x] [Run test](https://kyuubi.apache.org/docs/latest/develop_tools/testing.html#running-tests) locally before make a pull request Closes #4152 from pan3793/ldap. Closes #4152 d251c95 [Cheng Pan] nit 6d14f44 [Cheng Pan] nit 6b3d116 [Cheng Pan] nit ab47d82 [Cheng Pan] nit a56e870 [Cheng Pan] nit 4624619 [Cheng Pan] nit b82c0c0 [Cheng Pan] LDAP test password uses alphanumeric 86a01cc [Cheng Pan] Enhance LDAP authentication Authored-by: Cheng Pan <[email protected]> Signed-off-by: Cheng Pan <[email protected]>
apache · Feb 3, 2023 · eb1b11c · eb1b11c
1 parent c1e2e57
commit eb1b11c
Show file tree

Hide file tree

Showing 48 changed files with 4,209 additions and 104 deletions.
diff --git a/.gitignore b/.gitignore
@@ -32,11 +32,7 @@
 .ensime_lucene
 .generated-mima*
 .vscode/
-# The star is required for further !/.idea/ to work, see https://git-scm.com/docs/gitignore
-/.idea/*
-# Icon for JetBrains Toolbox
-!/.idea/icon.png
-!/.idea/vcs.xml
+.idea/
 .idea_modules/
 .project
 .pydevproject
@@ -59,7 +55,6 @@ hs_err_pid*
 spark-warehouse/
 metastore_db
 derby.log
-ldap
 **/dependency-reduced-pom.xml
 metrics/report.json
 metrics/.report.json.crc

diff --git a/LICENSE-binary b/LICENSE-binary
@@ -322,7 +322,9 @@ org.apache.zookeeper:zookeeper
 
 BSD
 ------------
+org.antlr:antlr-runtime
 org.antlr:antlr4-runtime
+org.antlr:ST4
 jline:jline
 com.thoughtworks.paranamer:paranamer
 dk.brics.automaton:automaton

diff --git a/dev/dependencyList b/dev/dependencyList
@@ -16,8 +16,10 @@
 #
 
 HikariCP/4.0.3//HikariCP-4.0.3.jar
+ST4/4.3.4//ST4-4.3.4.jar
 animal-sniffer-annotations/1.21//animal-sniffer-annotations-1.21.jar
 annotations/4.1.1.4//annotations-4.1.1.4.jar
+antlr-runtime/3.5.3//antlr-runtime-3.5.3.jar
 antlr4-runtime/4.9.3//antlr4-runtime-4.9.3.jar
 aopalliance-repackaged/2.6.1//aopalliance-repackaged-2.6.1.jar
 automaton/1.11-8//automaton-1.11-8.jar

diff --git a/docs/deployment/settings.md b/docs/deployment/settings.md
diff --git a/kyuubi-common/pom.xml b/kyuubi-common/pom.xml
@@ -88,6 +88,11 @@
             <scope>runtime</scope>
         </dependency>
 
+        <dependency>
+            <groupId>org.antlr</groupId>
+            <artifactId>ST4</artifactId>
+        </dependency>
+
         <dependency>
             <groupId>org.apache.commons</groupId>
             <artifactId>commons-lang3</artifactId>
@@ -141,6 +146,12 @@
             <scope>test</scope>
         </dependency>
 
+        <dependency>
+            <groupId>org.scalatestplus</groupId>
+            <artifactId>mockito-4-6_${scala.binary.version}</artifactId>
+            <scope>test</scope>
+        </dependency>
+
         <dependency>
             <groupId>com.google.guava</groupId>
             <artifactId>failureaccess</artifactId>

diff --git a/kyuubi-common/src/main/scala/org/apache/kyuubi/Logging.scala b/kyuubi-common/src/main/scala/org/apache/kyuubi/Logging.scala
@@ -54,12 +54,24 @@ trait Logging {
     }
   }
 
+  def debug(message: => Any, t: Throwable): Unit = {
+    if (logger.isDebugEnabled) {
+      logger.debug(message.toString, t)
+    }
+  }
+
   def info(message: => Any): Unit = {
     if (logger.isInfoEnabled) {
       logger.info(message.toString)
     }
   }
 
+  def info(message: => Any, t: Throwable): Unit = {
+    if (logger.isInfoEnabled) {
+      logger.info(message.toString, t)
+    }
+  }
+
   def warn(message: => Any): Unit = {
     if (logger.isWarnEnabled) {
       logger.warn(message.toString)

diff --git a/kyuubi-common/src/main/scala/org/apache/kyuubi/config/KyuubiConf.scala b/kyuubi-common/src/main/scala/org/apache/kyuubi/config/KyuubiConf.scala
@@ -783,10 +783,11 @@ object KyuubiConf {
       .stringConf
       .createOptional
 
-  val AUTHENTICATION_LDAP_BASEDN: OptionalConfigEntry[String] =
-    buildConf("kyuubi.authentication.ldap.base.dn")
+  val AUTHENTICATION_LDAP_BASE_DN: OptionalConfigEntry[String] =
+    buildConf("kyuubi.authentication.ldap.baseDN")
+      .withAlternative("kyuubi.authentication.ldap.base.dn")
       .doc("LDAP base DN.")
-      .version("1.0.0")
+      .version("1.7.0")
       .stringConf
       .createOptional
 
@@ -797,14 +798,109 @@ object KyuubiConf {
       .stringConf
       .createOptional
 
-  val AUTHENTICATION_LDAP_GUIDKEY: ConfigEntry[String] =
+  val AUTHENTICATION_LDAP_GROUP_DN_PATTERN: OptionalConfigEntry[String] =
+    buildConf("kyuubi.authentication.ldap.groupDNPattern")
+      .doc("COLON-separated list of patterns to use to find DNs for group entities in " +
+        "this directory. Use %s where the actual group name is to be substituted for. " +
+        "For example: CN=%s,CN=Groups,DC=subdomain,DC=domain,DC=com.")
+      .version("1.7.0")
+      .stringConf
+      .createOptional
+
+  val AUTHENTICATION_LDAP_USER_DN_PATTERN: OptionalConfigEntry[String] =
+    buildConf("kyuubi.authentication.ldap.userDNPattern")
+      .doc("COLON-separated list of patterns to use to find DNs for users in this directory. " +
+        "Use %s where the actual group name is to be substituted for. " +
+        "For example: CN=%s,CN=Users,DC=subdomain,DC=domain,DC=com.")
+      .version("1.7.0")
+      .stringConf
+      .createOptional
+
+  val AUTHENTICATION_LDAP_GROUP_FILTER: ConfigEntry[Seq[String]] =
+    buildConf("kyuubi.authentication.ldap.groupFilter")
+      .doc("COMMA-separated list of LDAP Group names (short name not full DNs). " +
+        "For example: HiveAdmins,HadoopAdmins,Administrators")
+      .version("1.7.0")
+      .stringConf
+      .toSequence()
+      .createWithDefault(Nil)
+
+  val AUTHENTICATION_LDAP_USER_FILTER: ConfigEntry[Seq[String]] =
+    buildConf("kyuubi.authentication.ldap.userFilter")
+      .doc("COMMA-separated list of LDAP usernames (just short names, not full DNs). " +
+        "For example: hiveuser,impalauser,hiveadmin,hadoopadmin")
+      .version("1.7.0")
+      .stringConf
+      .toSequence()
+      .createWithDefault(Nil)
+
+  val AUTHENTICATION_LDAP_GUID_KEY: ConfigEntry[String] =
     buildConf("kyuubi.authentication.ldap.guidKey")
-      .doc("LDAP attribute name whose values are unique in this LDAP server." +
-        "For example:uid or cn.")
+      .doc("LDAP attribute name whose values are unique in this LDAP server. " +
+        "For example: uid or CN.")
       .version("1.2.0")
       .stringConf
       .createWithDefault("uid")
 
+  val AUTHENTICATION_LDAP_GROUP_MEMBERSHIP_KEY: ConfigEntry[String] =
+    buildConf("kyuubi.authentication.ldap.groupMembershipKey")
+      .doc("LDAP attribute name on the group object that contains the list of distinguished " +
+        "names for the user, group, and contact objects that are members of the group. " +
+        "For example: member, uniqueMember or memberUid")
+      .version("1.7.0")
+      .stringConf
+      .createWithDefault("member")
+
+  val AUTHENTICATION_LDAP_USER_MEMBERSHIP_KEY: OptionalConfigEntry[String] =
+    buildConf("kyuubi.authentication.ldap.userMembershipKey")
+      .doc("LDAP attribute name on the user object that contains groups of which the user is " +
+        "a direct member, except for the primary group, which is represented by the " +
+        "primaryGroupId. For example: memberOf")
+      .version("1.7.0")
+      .stringConf
+      .createOptional
+
+  val AUTHENTICATION_LDAP_GROUP_CLASS_KEY: ConfigEntry[String] =
+    buildConf("kyuubi.authentication.ldap.groupClassKey")
+      .doc("LDAP attribute name on the group entry that is to be used in LDAP group searches. " +
+        "For example: group, groupOfNames or groupOfUniqueNames.")
+      .version("1.7.0")
+      .stringConf
+      .createWithDefault("groupOfNames")
+
+  val AUTHENTICATION_LDAP_CUSTOM_LDAP_QUERY: OptionalConfigEntry[String] =
+    buildConf("kyuubi.authentication.ldap.customLDAPQuery")
+      .doc("A full LDAP query that LDAP Atn provider uses to execute against LDAP Server. " +
+        "If this query returns a null resultset, the LDAP Provider fails the Authentication " +
+        "request, succeeds if the user is part of the resultset." +
+        "For example: `(&(objectClass=group)(objectClass=top)(instanceType=4)(cn=Domain*))`, " +
+        "`(&(objectClass=person)(|(sAMAccountName=admin)" +
+        "(|(memberOf=CN=Domain Admins,CN=Users,DC=domain,DC=com)" +
+        "(memberOf=CN=Administrators,CN=Builtin,DC=domain,DC=com))))`")
+      .version("1.7.0")
+      .stringConf
+      .createOptional
+
+  val AUTHENTICATION_LDAP_BIND_USER: OptionalConfigEntry[String] =
+    buildConf("kyuubi.authentication.ldap.binddn")
+      .doc("The user with which to bind to the LDAP server, and search for the full domain name " +
+        "of the user being authenticated. This should be the full domain name of the user, and " +
+        "should have search access across all users in the LDAP tree. If not specified, then " +
+        "the user being authenticated will be used as the bind user. " +
+        "For example: CN=bindUser,CN=Users,DC=subdomain,DC=domain,DC=com")
+      .version("1.7.0")
+      .stringConf
+      .createOptional
+
+  val AUTHENTICATION_LDAP_BIND_PASSWORD: OptionalConfigEntry[String] =
+    buildConf("kyuubi.authentication.ldap.bindpw")
+      .doc("The password for the bind user, to be used to search for the full name of the " +
+        "user being authenticated. If the username is specified, this parameter must also be " +
+        "specified.")
+      .version("1.7.0")
+      .stringConf
+      .createOptional
+
   val AUTHENTICATION_JDBC_DRIVER: OptionalConfigEntry[String] =
     buildConf("kyuubi.authentication.jdbc.driver.class")
       .doc("Driver class name for JDBC Authentication Provider.")

diff --git a/kyuubi-common/src/main/scala/org/apache/kyuubi/service/ServiceUtils.scala b/kyuubi-common/src/main/scala/org/apache/kyuubi/service/ServiceUtils.scala
@@ -17,6 +17,10 @@
 
 package org.apache.kyuubi.service
 
+import java.io.{Closeable, IOException}
+
+import org.slf4j.Logger
+
 object ServiceUtils {
 
   /**
@@ -49,4 +53,24 @@ object ServiceUtils {
       userName.substring(0, indexOfDomainMatch)
     }
   }
+
+  /**
+   * Close the Closeable objects and <b>ignore</b> any [[IOException]] or
+   * null pointers. Must only be used for cleanup in exception handlers.
+   *
+   * @param log        the log to record problems to at debug level. Can be null.
+   * @param closeables the objects to close
+   */
+  def cleanup(log: Logger, closeables: Closeable*): Unit = {
+    closeables.filter(_ != null).foreach { c =>
+      try {
+        c.close()
+      } catch {
+        case e: IOException =>
+          if (log != null && log.isDebugEnabled) {
+            log.debug(s"Exception in closing $c", e)
+          }
+      }
+    }
+  }
 }
diff --git a/.../main/scala/org/apache/kyuubi/service/authentication/LdapAuthenticationProviderImpl.scala b/.../main/scala/org/apache/kyuubi/service/authentication/LdapAuthenticationProviderImpl.scala
@@ -17,17 +17,25 @@
 
 package org.apache.kyuubi.service.authentication
 
-import javax.naming.{Context, NamingException}
-import javax.naming.directory.InitialDirContext
+import javax.naming.NamingException
 import javax.security.sasl.AuthenticationException
 
 import org.apache.commons.lang3.StringUtils
 
+import org.apache.kyuubi.Logging
 import org.apache.kyuubi.config.KyuubiConf
-import org.apache.kyuubi.config.KyuubiConf._
 import org.apache.kyuubi.service.ServiceUtils
+import org.apache.kyuubi.service.authentication.LdapAuthenticationProviderImpl.FILTER_FACTORIES
+import org.apache.kyuubi.service.authentication.ldap._
 
-class LdapAuthenticationProviderImpl(conf: KyuubiConf) extends PasswdAuthenticationProvider {
+class LdapAuthenticationProviderImpl(
+    conf: KyuubiConf,
+    searchFactory: DirSearchFactory = new LdapSearchFactory)
+  extends PasswdAuthenticationProvider with Logging {
+
+  private val filterOpt: Option[Filter] = FILTER_FACTORIES
+    .map { f => f.getInstance(conf) }
+    .collectFirst { case Some(f: Filter) => f }
 
   /**
    * The authenticate method is called by the Kyuubi Server authentication layer
@@ -41,47 +49,72 @@ class LdapAuthenticationProviderImpl(conf: KyuubiConf) extends PasswdAuthenticat
    * @throws AuthenticationException When a user is found to be invalid by the implementation
    */
   override def authenticate(user: String, password: String): Unit = {
+
+    val (usedBind, bindUser, bindPassword) = (
+      conf.get(KyuubiConf.AUTHENTICATION_LDAP_BIND_USER),
+      conf.get(KyuubiConf.AUTHENTICATION_LDAP_BIND_PASSWORD)) match {
+      case (Some(_bindUser), Some(_bindPw)) => (true, _bindUser, _bindPw)
+      case _ =>
+        // If no bind user or bind password was specified,
+        // we assume the user we are authenticating has the ability to search
+        // the LDAP tree, so we use it as the "binding" account.
+        // This is the way it worked before bind users were allowed in the LDAP authenticator,
+        // so we keep existing systems working.
+        (false, user, password)
+    }
+
+    var search: DirSearch = null
+    try {
+      search = createDirSearch(bindUser, bindPassword)
+      applyFilter(search, user)
+      if (usedBind) {
+        // If we used the bind user, then we need to authenticate again,
+        // this time using the full user name we got during the bind process.
+        createDirSearch(search.findUserDn(user), password)
+      }
+    } catch {
+      case e: NamingException =>
+        throw new AuthenticationException(
+          s"Unable to find the user in the LDAP tree. ${e.getMessage}")
+    } finally {
+      ServiceUtils.cleanup(logger, search)
+    }
+  }
+
+  @throws[AuthenticationException]
+  private def createDirSearch(user: String, password: String): DirSearch = {
     if (StringUtils.isBlank(user)) {
       throw new AuthenticationException(s"Error validating LDAP user, user is null" +
         s" or contains blank space")
     }
 
-    if (StringUtils.isBlank(password)) {
+    if (StringUtils.isBlank(password) || password.getBytes()(0) == 0) {
       throw new AuthenticationException(s"Error validating LDAP user, password is null" +
         s" or contains blank space")
     }
 
-    val env = new java.util.Hashtable[String, Any]()
-    env.put(Context.INITIAL_CONTEXT_FACTORY, "com.sun.jndi.ldap.LdapCtxFactory")
-    env.put(Context.SECURITY_AUTHENTICATION, "simple")
-
-    conf.get(AUTHENTICATION_LDAP_URL).foreach(env.put(Context.PROVIDER_URL, _))
-
-    val domain = conf.get(AUTHENTICATION_LDAP_DOMAIN)
-    val u =
-      if (!hasDomain(user) && domain.nonEmpty) {
-        user + "@" + domain.get
-      } else {
-        user
+    val principals = LdapUtils.createCandidatePrincipals(conf, user)
+    val iterator = principals.iterator
+    while (iterator.hasNext) {
+      val principal = iterator.next
+      try {
+        return searchFactory.getInstance(conf, principal, password)
+      } catch {
+        case ex: AuthenticationException => if (iterator.isEmpty) throw ex
       }
-
-    val guidKey = conf.get(AUTHENTICATION_LDAP_GUIDKEY)
-    val bindDn = conf.get(AUTHENTICATION_LDAP_BASEDN) match {
-      case Some(dn) => guidKey + "=" + u + "," + dn
-      case _ => u
     }
+    throw new AuthenticationException(s"No candidate principals for $user was found.")
+  }
 
-    env.put(Context.SECURITY_PRINCIPAL, bindDn)
-    env.put(Context.SECURITY_CREDENTIALS, password)
-
-    try {
-      val ctx = new InitialDirContext(env)
-      ctx.close()
-    } catch {
-      case e: NamingException =>
-        throw new AuthenticationException(s"Error validating LDAP user: $bindDn", e)
-    }
+  @throws[AuthenticationException]
+  private def applyFilter(client: DirSearch, user: String): Unit = filterOpt.foreach { filter =>
+    val username = if (LdapUtils.hasDomain(user)) LdapUtils.extractUserName(user) else user
+    filter.apply(client, username)
   }
+}
 
-  private def hasDomain(userName: String): Boolean = ServiceUtils.indexOfDomainMatch(userName) > 0
+object LdapAuthenticationProviderImpl {
+  val FILTER_FACTORIES: Array[FilterFactory] = Array[FilterFactory](
+    CustomQueryFilterFactory,
+    new ChainFilterFactory(UserSearchFilterFactory, UserFilterFactory, GroupFilterFactory))
 }