From 51ad976a5522df973124284e3b7d47819510fb62 Mon Sep 17 00:00:00 2001 From: moelhoussein Date: Tue, 9 Jun 2026 15:32:23 -0400 Subject: [PATCH] [AUTHENTICATION] Add UnboundID-backed LDAP connection pool with failover and auth cache Adds an opt-in UnboundID LDAP SDK authentication path (kyuubi.authentication.ldap.pool.enabled, default false) alongside the existing JNDI implementation. When enabled it provides a health-checked connection pool, ordered multi-URL failover with bounded mid-request retry, an optional successful-auth cache (HMAC-keyed, TTL/size-bounded, no negative caching, request coalescing), Prometheus auth/cache/pool metrics with failure classification, and RFC 4515 escaping of user-supplied filter values. The legacy JNDI path is unchanged when the pool is disabled. Proposed and discussed in https://github.com/apache/kyuubi/discussions/7497 Assisted-by: Claude Opus 4.8 (claude-opus-4-8) Assisted-by: OpenAI GPT-5.5 Assisted-by: GitHub Copilot Enterprise --- LICENSE-binary | 1 + dev/dependencyList | 1 + docs/configuration/settings.md | 63 +- kyuubi-common/pom.xml | 1 - .../org/apache/kyuubi/config/KyuubiConf.scala | 130 ++++ .../AuthenticationProviderFactory.scala | 176 ++++- .../LdapAuthMetricsRecorder.scala | 81 ++ .../LdapAuthenticationProviderImpl.scala | 39 +- .../authentication/ldap/LdapAuthCache.scala | 275 +++++++ .../ldap/LdapAuthFailureClassifier.scala | 87 +++ .../authentication/ldap/LdapSearch.scala | 38 +- .../authentication/ldap/LdapServerSets.scala | 83 +++ .../authentication/ldap/LdapUrlParser.scala | 47 ++ .../authentication/ldap/LdapUtils.scala | 59 +- .../service/authentication/ldap/Query.scala | 93 ++- .../authentication/ldap/QueryFactory.scala | 10 +- .../ldap/UnboundIdConnectionPool.scala | 289 ++++++++ .../ldap/UnboundIdDirSearch.scala | 314 ++++++++ .../ldap/UnboundIdDirSearchFactory.scala | 139 ++++ .../AuthenticationProviderFactorySuite.scala | 53 +- .../ldap/CapturingLdapAuthRecorder.scala | 51 ++ .../ldap/LdapAuthCacheSuite.scala | 436 +++++++++++ .../ldap/LdapAuthFailureClassifierSuite.scala | 98 +++ .../authentication/ldap/LdapSearchSuite.scala | 68 +- .../ldap/LdapUrlParserSuite.scala | 96 +++ .../authentication/ldap/LdapUtilsSuite.scala | 84 +++ .../ldap/QueryFactorySuite.scala | 52 +- .../authentication/ldap/QuerySuite.scala | 37 +- .../ldap/UnboundIdDirSearchFactorySuite.scala | 690 ++++++++++++++++++ .../ldap/UnboundIdDirSearchSuite.scala | 151 ++++ .../kyuubi/metrics/MetricsConstants.scala | 12 + .../apache/kyuubi/server/KyuubiServer.scala | 145 ++++ 32 files changed, 3775 insertions(+), 124 deletions(-) create mode 100644 kyuubi-common/src/main/scala/org/apache/kyuubi/service/authentication/LdapAuthMetricsRecorder.scala create mode 100644 kyuubi-common/src/main/scala/org/apache/kyuubi/service/authentication/ldap/LdapAuthCache.scala create mode 100644 kyuubi-common/src/main/scala/org/apache/kyuubi/service/authentication/ldap/LdapAuthFailureClassifier.scala create mode 100644 kyuubi-common/src/main/scala/org/apache/kyuubi/service/authentication/ldap/LdapServerSets.scala create mode 100644 kyuubi-common/src/main/scala/org/apache/kyuubi/service/authentication/ldap/LdapUrlParser.scala create mode 100644 kyuubi-common/src/main/scala/org/apache/kyuubi/service/authentication/ldap/UnboundIdConnectionPool.scala create mode 100644 kyuubi-common/src/main/scala/org/apache/kyuubi/service/authentication/ldap/UnboundIdDirSearch.scala create mode 100644 kyuubi-common/src/main/scala/org/apache/kyuubi/service/authentication/ldap/UnboundIdDirSearchFactory.scala create mode 100644 kyuubi-common/src/test/scala/org/apache/kyuubi/service/authentication/ldap/CapturingLdapAuthRecorder.scala create mode 100644 kyuubi-common/src/test/scala/org/apache/kyuubi/service/authentication/ldap/LdapAuthCacheSuite.scala create mode 100644 kyuubi-common/src/test/scala/org/apache/kyuubi/service/authentication/ldap/LdapAuthFailureClassifierSuite.scala create mode 100644 kyuubi-common/src/test/scala/org/apache/kyuubi/service/authentication/ldap/LdapUrlParserSuite.scala create mode 100644 kyuubi-common/src/test/scala/org/apache/kyuubi/service/authentication/ldap/UnboundIdDirSearchFactorySuite.scala create mode 100644 kyuubi-common/src/test/scala/org/apache/kyuubi/service/authentication/ldap/UnboundIdDirSearchSuite.scala diff --git a/LICENSE-binary b/LICENSE-binary index 26ed25c7246..2c65e89d2de 100644 --- a/LICENSE-binary +++ b/LICENSE-binary @@ -213,6 +213,7 @@ org.apache.commons:commons-lang3 com.google.errorprone:error_prone_annotations dev.failsafe:failsafe com.jakewharton.fliptables:fliptables +com.unboundid:unboundid-ldapsdk io.grpc:grpc-api io.grpc:grpc-context io.grpc:grpc-core diff --git a/dev/dependencyList b/dev/dependencyList index 37031789847..a37331d72f2 100644 --- a/dev/dependencyList +++ b/dev/dependencyList @@ -184,6 +184,7 @@ swagger-integration/2.2.1//swagger-integration-2.2.1.jar swagger-jaxrs2/2.2.1//swagger-jaxrs2-2.2.1.jar swagger-models/2.2.1//swagger-models-2.2.1.jar trino-client/411//trino-client-411.jar +unboundid-ldapsdk/6.0.5//unboundid-ldapsdk-6.0.5.jar units/1.7//units-1.7.jar vertx-core/4.5.3//vertx-core-4.5.3.jar vertx-grpc/4.5.3//vertx-grpc-4.5.3.jar diff --git a/docs/configuration/settings.md b/docs/configuration/settings.md index 43a002a9630..886b11a398a 100644 --- a/docs/configuration/settings.md +++ b/docs/configuration/settings.md @@ -31,32 +31,43 @@ You can configure the Kyuubi properties in `$KYUUBI_HOME/conf/kyuubi-defaults.co ### Authentication -| Key | Default | Meaning | Type | Since | -|-----------------------------------------------|-------------------|--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|--------|--------| -| kyuubi.authentication | NONE | A comma-separated list of client authentication types.The following tree describes the catalog of each option. Note that: for SASL authentication, KERBEROS and PLAIN auth types are supported at the same time, and only the first specified PLAIN auth type is valid. | seq | 1.0.0 | -| kyuubi.authentication.custom.basic.class | <undefined> | User-defined authentication implementation of org.apache.kyuubi.service.authentication.PasswdAuthenticationProvider for http basic authentication. | string | 1.10.0 | -| kyuubi.authentication.custom.bearer.class | <undefined> | User-defined authentication implementation of org.apache.kyuubi.service.authentication.TokenAuthenticationProvider for http bearer authentication. | string | 1.10.0 | -| kyuubi.authentication.custom.class | <undefined> | User-defined authentication implementation of org.apache.kyuubi.service.authentication.PasswdAuthenticationProvider | string | 1.3.0 | -| kyuubi.authentication.jdbc.driver.class | <undefined> | Driver class name for JDBC Authentication Provider. | string | 1.6.0 | -| kyuubi.authentication.jdbc.password | <undefined> | Database password for JDBC Authentication Provider. | string | 1.6.0 | -| kyuubi.authentication.jdbc.query | <undefined> | Query SQL template with placeholders for JDBC Authentication Provider to execute. Authentication passes if the result set is not empty.The SQL statement must start with the `SELECT` clause. Available placeholders are `${user}` and `${password}`. | string | 1.6.0 | -| kyuubi.authentication.jdbc.url | <undefined> | JDBC URL for JDBC Authentication Provider. | string | 1.6.0 | -| kyuubi.authentication.jdbc.user | <undefined> | Database user for JDBC Authentication Provider. | string | 1.6.0 | -| kyuubi.authentication.ldap.baseDN | <undefined> | LDAP base DN. | string | 1.7.0 | -| kyuubi.authentication.ldap.binddn | <undefined> | 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 | string | 1.7.0 | -| kyuubi.authentication.ldap.bindpw | <undefined> | 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. | string | 1.7.0 | -| kyuubi.authentication.ldap.customLDAPQuery | <undefined> | 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))))` | string | 1.7.0 | -| kyuubi.authentication.ldap.domain | <undefined> | LDAP domain. | string | 1.0.0 | -| kyuubi.authentication.ldap.groupClassKey | groupOfNames | LDAP attribute name on the group entry that is to be used in LDAP group searches. For example: group, groupOfNames or groupOfUniqueNames. | string | 1.7.0 | -| kyuubi.authentication.ldap.groupDNPattern | <undefined> | 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. | string | 1.7.0 | -| kyuubi.authentication.ldap.groupFilter || COMMA-separated list of LDAP Group names (short name not full DNs). For example: HiveAdmins,HadoopAdmins,Administrators | set | 1.7.0 | -| kyuubi.authentication.ldap.groupMembershipKey | member | 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 | string | 1.7.0 | -| kyuubi.authentication.ldap.guidKey | uid | LDAP attribute name whose values are unique in this LDAP server. For example: uid or CN. | string | 1.2.0 | -| kyuubi.authentication.ldap.url | <undefined> | SPACE character separated LDAP connection URL(s). | string | 1.0.0 | -| kyuubi.authentication.ldap.userDNPattern | <undefined> | 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. | string | 1.7.0 | -| kyuubi.authentication.ldap.userFilter || COMMA-separated list of LDAP usernames (just short names, not full DNs). For example: hiveuser,impalauser,hiveadmin,hadoopadmin | set | 1.7.0 | -| kyuubi.authentication.ldap.userMembershipKey | <undefined> | 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 | string | 1.7.0 | -| kyuubi.authentication.sasl.qop | auth | Sasl QOP enable higher levels of protection for Kyuubi communication with clients. | string | 1.0.0 | +| Key | Default | Meaning | Type | Since | +|------------------------------------------------------|-------------------|--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|----------|--------| +| kyuubi.authentication | NONE | A comma-separated list of client authentication types.The following tree describes the catalog of each option. Note that: for SASL authentication, KERBEROS and PLAIN auth types are supported at the same time, and only the first specified PLAIN auth type is valid. | seq | 1.0.0 | +| kyuubi.authentication.custom.basic.class | <undefined> | User-defined authentication implementation of org.apache.kyuubi.service.authentication.PasswdAuthenticationProvider for http basic authentication. | string | 1.10.0 | +| kyuubi.authentication.custom.bearer.class | <undefined> | User-defined authentication implementation of org.apache.kyuubi.service.authentication.TokenAuthenticationProvider for http bearer authentication. | string | 1.10.0 | +| kyuubi.authentication.custom.class | <undefined> | User-defined authentication implementation of org.apache.kyuubi.service.authentication.PasswdAuthenticationProvider | string | 1.3.0 | +| kyuubi.authentication.jdbc.driver.class | <undefined> | Driver class name for JDBC Authentication Provider. | string | 1.6.0 | +| kyuubi.authentication.jdbc.password | <undefined> | Database password for JDBC Authentication Provider. | string | 1.6.0 | +| kyuubi.authentication.jdbc.query | <undefined> | Query SQL template with placeholders for JDBC Authentication Provider to execute. Authentication passes if the result set is not empty.The SQL statement must start with the `SELECT` clause. Available placeholders are `${user}` and `${password}`. | string | 1.6.0 | +| kyuubi.authentication.jdbc.url | <undefined> | JDBC URL for JDBC Authentication Provider. | string | 1.6.0 | +| kyuubi.authentication.jdbc.user | <undefined> | Database user for JDBC Authentication Provider. | string | 1.6.0 | +| kyuubi.authentication.ldap.baseDN | <undefined> | LDAP base DN. | string | 1.7.0 | +| kyuubi.authentication.ldap.binddn | <undefined> | 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 | string | 1.7.0 | +| kyuubi.authentication.ldap.bindpw | <undefined> | 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. | string | 1.7.0 | +| kyuubi.authentication.ldap.cache.enabled | true | Whether to cache successful LDAP authentications within a short TTL to reduce LDAP round-trips for repeated requests from the same user. Only successful authentications are cached; failed attempts always reach LDAP. | boolean | 1.12.0 | +| kyuubi.authentication.ldap.cache.maxSize | 1000 | Maximum number of entries in the LDAP authentication cache. When the limit is reached, least-recently-used entries are evicted. Size-based eviction applies in addition to the TTL configured by kyuubi.authentication.ldap.cache.ttl. | int | 1.12.0 | +| kyuubi.authentication.ldap.cache.ttl | PT30S | Time-to-live for a cached successful LDAP authentication result. After this period the entry is evicted and the next request re-authenticates against LDAP. Time-based eviction applies in addition to the size cap configured by kyuubi.authentication.ldap.cache.maxSize. Operational note: after an LDAP-side password rotation, account lockout, or user disable, the cached (user, password) pair continues to be accepted by Kyuubi until this TTL expires. Lower this value in environments with strict revocation SLAs; raise it to reduce LDAP load. | duration | 1.12.0 | +| kyuubi.authentication.ldap.customLDAPQuery | <undefined> | 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))))` | string | 1.7.0 | +| kyuubi.authentication.ldap.domain | <undefined> | LDAP domain. | string | 1.0.0 | +| kyuubi.authentication.ldap.groupClassKey | groupOfNames | LDAP attribute name on the group entry that is to be used in LDAP group searches. For example: group, groupOfNames or groupOfUniqueNames. | string | 1.7.0 | +| kyuubi.authentication.ldap.groupDNPattern | <undefined> | 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. | string | 1.7.0 | +| kyuubi.authentication.ldap.groupFilter || COMMA-separated list of LDAP Group names (short name not full DNs). For example: HiveAdmins,HadoopAdmins,Administrators | set | 1.7.0 | +| kyuubi.authentication.ldap.groupMembershipKey | member | 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 | string | 1.7.0 | +| kyuubi.authentication.ldap.guidKey | uid | LDAP attribute name whose values are unique in this LDAP server. For example: uid or CN. | string | 1.2.0 | +| kyuubi.authentication.ldap.pool.checkoutTimeout | PT5S | Maximum time a request thread will wait for a connection to become available when the LDAP pool is fully utilised. The UnboundID SDK default is to skip waiting and immediately try to grow the pool: if a new connection can be created the request succeeds, otherwise it fails fast (typically with CONNECT_ERROR, or with the result code from the underlying connection attempt if creation was tried). A positive value here adds a grace window so brief contention spikes do not surface as authentication failures. Set to 0 to revert to the SDK default no-wait behaviour. The default is intentionally short -- long waits pile up front-end Thrift/HTTP handler threads under a degraded LDAP. Operators who raise this should also raise kyuubi.authentication.ldap.pool.maxConnections. | duration | 1.12.0 | +| kyuubi.authentication.ldap.pool.connectTimeout | PT5S | Timeout for establishing a TCP connection to an LDAP server. | duration | 1.12.0 | +| kyuubi.authentication.ldap.pool.enabled | false | Whether to use a persistent UnboundID LDAP connection pool for LDAP authentication. When enabled, the server maintains warm pre-authenticated connections to LDAP, eliminating per-request TCP/TLS/BIND overhead. Requires bind user credentials to be configured. If the bind user is not set the pool is silently skipped and ephemeral connections are used. Setting this to false falls back to the legacy JNDI implementation; that path is scheduled for removal in a future release. Both paths derive SSL purely from the URL scheme (ldap:// vs ldaps://) -- StartTLS and LDAP referrals are not supported on either path, matching the behaviour of the legacy JNDI implementation. All kyuubi.authentication.ldap.* settings are read once on the first authentication request after server start and cached for the JVM lifetime; changes require a Kyuubi restart to take effect. | boolean | 1.12.0 | +| kyuubi.authentication.ldap.pool.healthCheck.interval | PT30S | Interval between background health checks on idle LDAP pool connections. Each check fetches the root DSE; connections that fail are discarded and replaced. | duration | 1.12.0 | +| kyuubi.authentication.ldap.pool.maxConnections | 10 | Maximum number of connections in the LDAP connection pool. | int | 1.12.0 | +| kyuubi.authentication.ldap.pool.minConnections | 3 | Minimum number of connections to keep open in the LDAP connection pool. | int | 1.12.0 | +| kyuubi.authentication.ldap.pool.responseTimeout | PT10S | Timeout for an LDAP operation response after the connection is established. | duration | 1.12.0 | +| kyuubi.authentication.ldap.pool.search.maxRetries | 1 | Maximum number of times a pooled LDAP search is retried against a freshly checked-out connection when the original connection fails mid-request with a connection-level error (server down, dropped socket, idle connection reset). Bounded retry covers the common case where one LDAP server in the failover list rotates or briefly disconnects, without amplifying load during a real outage. Set to 0 to disable retry and propagate the first failure. Only applies to the pooled (bind-user) path; the ephemeral end-user bind path does not retry because its connection is tied to specific credentials that cannot be re-established transparently. | int | 1.12.0 | +| kyuubi.authentication.ldap.url | <undefined> | SPACE character separated LDAP connection URL(s). | string | 1.0.0 | +| kyuubi.authentication.ldap.userDNPattern | <undefined> | 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. | string | 1.7.0 | +| kyuubi.authentication.ldap.userFilter || COMMA-separated list of LDAP usernames (just short names, not full DNs). For example: hiveuser,impalauser,hiveadmin,hadoopadmin | set | 1.7.0 | +| kyuubi.authentication.ldap.userMembershipKey | <undefined> | 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 | string | 1.7.0 | +| kyuubi.authentication.sasl.qop | auth | Sasl QOP enable higher levels of protection for Kyuubi communication with clients. | string | 1.0.0 | ### Backend diff --git a/kyuubi-common/pom.xml b/kyuubi-common/pom.xml index 45cf6388ed9..b94df2c3eb8 100644 --- a/kyuubi-common/pom.xml +++ b/kyuubi-common/pom.xml @@ -150,7 +150,6 @@ com.unboundid unboundid-ldapsdk - test 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 index f9b6956fc0b..bfb53e23180 100644 --- a/kyuubi-common/src/main/scala/org/apache/kyuubi/config/KyuubiConf.scala +++ b/kyuubi-common/src/main/scala/org/apache/kyuubi/config/KyuubiConf.scala @@ -1089,6 +1089,136 @@ object KyuubiConf { .stringConf .createOptional + val AUTHENTICATION_LDAP_POOL_ENABLED: ConfigEntry[Boolean] = + buildConf("kyuubi.authentication.ldap.pool.enabled") + .doc("Whether to use a persistent UnboundID LDAP connection pool for LDAP authentication. " + + "When enabled, the server maintains warm pre-authenticated connections to LDAP, " + + "eliminating per-request TCP/TLS/BIND overhead. Requires bind user credentials to be " + + "configured. If the bind user is not set the pool is silently skipped and ephemeral " + + "connections are used. " + + "Setting this to false falls back to the legacy JNDI implementation; that path is " + + "scheduled for removal in a future release. Both paths derive SSL purely from the URL " + + "scheme (ldap:// vs ldaps://) -- StartTLS and LDAP referrals are not supported on " + + "either path, matching the behaviour of the legacy JNDI implementation. " + + "All kyuubi.authentication.ldap.* settings are read once on the first authentication " + + "request after server start and cached for the JVM lifetime; changes require a Kyuubi " + + "restart to take effect.") + .version("1.12.0") + .serverOnly + .booleanConf + .createWithDefault(false) + + val AUTHENTICATION_LDAP_POOL_MIN_CONNECTIONS: ConfigEntry[Int] = + buildConf("kyuubi.authentication.ldap.pool.minConnections") + .doc("Minimum number of connections to keep open in the LDAP connection pool.") + .version("1.12.0") + .serverOnly + .intConf + .createWithDefault(3) + + val AUTHENTICATION_LDAP_POOL_MAX_CONNECTIONS: ConfigEntry[Int] = + buildConf("kyuubi.authentication.ldap.pool.maxConnections") + .doc("Maximum number of connections in the LDAP connection pool.") + .version("1.12.0") + .serverOnly + .intConf + .createWithDefault(10) + + val AUTHENTICATION_LDAP_POOL_CONNECT_TIMEOUT: ConfigEntry[Long] = + buildConf("kyuubi.authentication.ldap.pool.connectTimeout") + .doc("Timeout for establishing a TCP connection to an LDAP server.") + .version("1.12.0") + .serverOnly + .timeConf + .createWithDefaultString("PT5S") + + val AUTHENTICATION_LDAP_POOL_RESPONSE_TIMEOUT: ConfigEntry[Long] = + buildConf("kyuubi.authentication.ldap.pool.responseTimeout") + .doc("Timeout for an LDAP operation response after the connection is established.") + .version("1.12.0") + .serverOnly + .timeConf + .createWithDefaultString("PT10S") + + val AUTHENTICATION_LDAP_POOL_HEALTH_CHECK_INTERVAL: ConfigEntry[Long] = + buildConf("kyuubi.authentication.ldap.pool.healthCheck.interval") + .doc("Interval between background health checks on idle LDAP pool connections. " + + "Each check fetches the root DSE; connections that fail are discarded and replaced.") + .version("1.12.0") + .serverOnly + .timeConf + .createWithDefaultString("PT30S") + + val AUTHENTICATION_LDAP_POOL_CHECKOUT_TIMEOUT: ConfigEntry[Long] = + buildConf("kyuubi.authentication.ldap.pool.checkoutTimeout") + .doc("Maximum time a request thread will wait for a connection to become available " + + "when the LDAP pool is fully utilised. The UnboundID SDK default is to skip waiting " + + "and immediately try to grow the pool: if a new connection can be created the " + + "request succeeds, otherwise it fails fast (typically with CONNECT_ERROR, or with " + + "the result code from the underlying connection attempt if creation was tried). " + + "A positive value here adds a grace window so brief contention spikes do not " + + "surface as authentication failures. Set to 0 to revert to the SDK default no-wait " + + "behaviour. The default is intentionally short -- long waits pile up front-end " + + "Thrift/HTTP handler threads under a degraded LDAP. Operators who raise this should " + + s"also raise ${"kyuubi.authentication.ldap.pool.maxConnections"}.") + .version("1.12.0") + .serverOnly + .timeConf + .createWithDefaultString("PT5S") + + val AUTHENTICATION_LDAP_POOL_SEARCH_MAX_RETRIES: ConfigEntry[Int] = + buildConf("kyuubi.authentication.ldap.pool.search.maxRetries") + .doc("Maximum number of times a pooled LDAP search is retried against a freshly " + + "checked-out connection when the original connection fails mid-request with a " + + "connection-level error (server down, dropped socket, idle connection reset). " + + "Bounded retry covers the common case where one LDAP server in the failover list " + + "rotates or briefly disconnects, without amplifying load during a real outage. " + + "Set to 0 to disable retry and propagate the first failure. Only applies to the " + + "pooled (bind-user) path; the ephemeral end-user bind path does not retry because " + + "its connection is tied to specific credentials that cannot be re-established " + + "transparently.") + .version("1.12.0") + .serverOnly + .intConf + .checkValue(_ >= 0, "must be non-negative") + .createWithDefault(1) + + val AUTHENTICATION_LDAP_CACHE_ENABLED: ConfigEntry[Boolean] = + buildConf("kyuubi.authentication.ldap.cache.enabled") + .doc("Whether to cache successful LDAP authentications within a short TTL to reduce " + + "LDAP round-trips for repeated requests from the same user. " + + "Only successful authentications are cached; failed attempts always reach LDAP.") + .version("1.12.0") + .serverOnly + .booleanConf + .createWithDefault(true) + + val AUTHENTICATION_LDAP_CACHE_TTL: ConfigEntry[Long] = + buildConf("kyuubi.authentication.ldap.cache.ttl") + .doc("Time-to-live for a cached successful LDAP authentication result. " + + "After this period the entry is evicted and the next request re-authenticates " + + "against LDAP. Time-based eviction applies in addition to the size cap configured " + + "by kyuubi.authentication.ldap.cache.maxSize. " + + "Operational note: after an LDAP-side password rotation, account lockout, or user " + + "disable, the cached (user, password) pair continues to be accepted by Kyuubi until " + + "this TTL expires. Lower this value in environments with strict revocation SLAs; " + + "raise it to reduce LDAP load.") + .version("1.12.0") + .serverOnly + .timeConf + .createWithDefaultString("PT30S") + + val AUTHENTICATION_LDAP_CACHE_MAX_SIZE: ConfigEntry[Int] = + buildConf("kyuubi.authentication.ldap.cache.maxSize") + .doc("Maximum number of entries in the LDAP authentication cache. " + + "When the limit is reached, least-recently-used entries are evicted. " + + "Size-based eviction applies in addition to the TTL configured by " + + "kyuubi.authentication.ldap.cache.ttl.") + .version("1.12.0") + .serverOnly + .intConf + .createWithDefault(1000) + 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/authentication/AuthenticationProviderFactory.scala b/kyuubi-common/src/main/scala/org/apache/kyuubi/service/authentication/AuthenticationProviderFactory.scala index b1453985337..6854f609c10 100644 --- a/kyuubi-common/src/main/scala/org/apache/kyuubi/service/authentication/AuthenticationProviderFactory.scala +++ b/kyuubi-common/src/main/scala/org/apache/kyuubi/service/authentication/AuthenticationProviderFactory.scala @@ -17,18 +17,28 @@ package org.apache.kyuubi.service.authentication +import java.util.concurrent.ConcurrentHashMap +import java.util.concurrent.atomic.AtomicLong 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.service.authentication.AuthMethods.AuthMethod +import org.apache.kyuubi.service.authentication.ldap.{CachingLdapAuthenticationProvider, LdapAuthCache, LdapAuthFailureClassifier, LdapSearchFactory, UnboundIdConnectionPool, UnboundIdDirSearchFactory} import org.apache.kyuubi.util.ClassUtils /** * This class helps select a [[PasswdAuthenticationProvider]] for a given [[AuthMethods]] */ -object AuthenticationProviderFactory { +object AuthenticationProviderFactory extends Logging { + + // Singletons keyed by auth scope ("server" or "http"). The LDAP stack must be a singleton + // so the connection pool and auth cache are shared across all authentication requests -- not + // recreated per request. JDBC providers are stateless, so per-call creation is fine there. + private val ldapProviders = new ConcurrentHashMap[String, PasswdAuthenticationProvider]() + @throws[AuthenticationException] def getAuthenticationProvider( method: AuthMethod, @@ -45,7 +55,7 @@ object AuthenticationProviderFactory { method: AuthMethod, conf: KyuubiConf): PasswdAuthenticationProvider = method match { case AuthMethods.NONE => new AnonymousAuthenticationProviderImpl - case AuthMethods.LDAP => new LdapAuthenticationProviderImpl(conf) + case AuthMethods.LDAP => ldapProviders.computeIfAbsent("server", _ => buildLdapProvider(conf)) case AuthMethods.JDBC => new JdbcAuthenticationProviderImpl(conf) case AuthMethods.CUSTOM => val className = conf.get(KyuubiConf.AUTHENTICATION_CUSTOM_CLASS) @@ -68,7 +78,7 @@ object AuthenticationProviderFactory { method: AuthMethod, conf: KyuubiConf): PasswdAuthenticationProvider = method match { case AuthMethods.NONE => new AnonymousAuthenticationProviderImpl - case AuthMethods.LDAP => new LdapAuthenticationProviderImpl(conf) + case AuthMethods.LDAP => ldapProviders.computeIfAbsent("http", _ => buildLdapProvider(conf)) case AuthMethods.JDBC => new JdbcAuthenticationProviderImpl(conf) case AuthMethods.CUSTOM => val className = conf.get(KyuubiConf.AUTHENTICATION_CUSTOM_BASIC_CLASS) @@ -87,4 +97,164 @@ object AuthenticationProviderFactory { "kyuubi.authentication.custom.bearer.class must be set for http bearer authentication.") ClassUtils.createInstance(providerClass, classOf[TokenAuthenticationProvider], conf) } + + /** + * Builds a [[PasswdAuthenticationProvider]] for LDAP. + * + * When pool is enabled, the stack is: + * CachingLdapAuthenticationProvider + * -> LdapAuthenticationProviderImpl(UnboundIdDirSearchFactory + connection pool) + * + * When pool is disabled (the default), uses the legacy JNDI path without caching. Enabling + * the pool opts into warm, health-checked, failover-aware connections; the legacy path is + * retained for environments where per-request connections are required (e.g. firewall + * rules that disallow long-lived connections). + */ + private def buildLdapProvider(conf: KyuubiConf): PasswdAuthenticationProvider = { + if (conf.get(KyuubiConf.AUTHENTICATION_LDAP_POOL_ENABLED)) { + val poolOpt = UnboundIdConnectionPool.getOrCreate(conf) + val factory = new UnboundIdDirSearchFactory(conf, poolOpt) + val core = new LdapAuthenticationProviderImpl(conf, factory) + if (conf.get(KyuubiConf.AUTHENTICATION_LDAP_CACHE_ENABLED)) { + new CachingLdapAuthenticationProvider(core, conf) + } else { + core + } + } else { + // The auth cache is only wired into the UnboundID/pool path; warn loudly when an + // operator turns the pool off but leaves caching enabled, so they're not silently + // running without the cache they thought they had configured. + if (conf.get(KyuubiConf.AUTHENTICATION_LDAP_CACHE_ENABLED)) { + warn( + s"${KyuubiConf.AUTHENTICATION_LDAP_CACHE_ENABLED.key} has no effect when " + + s"${KyuubiConf.AUTHENTICATION_LDAP_POOL_ENABLED.key}=false: " + + "the legacy JNDI auth path does not use the LDAP auth cache.") + } + new LdapAuthenticationProviderImpl(conf, new LdapSearchFactory) + } + } + + /** Called on server shutdown to release the LDAP connection pool. */ + def close(): Unit = { + UnboundIdConnectionPool.close() + LdapAuthCache.resetCache() + ldapProviders.clear() + // Reset auth counters so a re-init after close() (e.g. between test cases) starts + // from a clean slate. In production close() runs once at JVM shutdown, so the reset + // is a no-op there. + ldapAuthSuccessCounter.set(0L) + ldapAuthFailureInvalidCredentialsCounter.set(0L) + ldapAuthFailureInvalidInputCounter.set(0L) + ldapAuthFailureInfrastructureCounter.set(0L) + ldapAuthCacheHitCounter.set(0L) + ldapAuthCacheMissCounter.set(0L) + // Drop any installed metrics recorder so subsequent server starts in the same JVM + // (notably tests) do not see events emitted to a stale destination. + ldapAuthMetricsRecorder = LdapAuthMetricsRecorder.NoOp + } + + /** + * Live gauge accessors for the LDAP connection pool. Each method returns 0 when the pool + * has not yet been created (e.g. before the first authentication request, or when LDAP + * pooling is disabled). Designed to be wired into [[org.apache.kyuubi.metrics.MetricsSystem]] + * gauges from `kyuubi-server`, which depends on `kyuubi-metrics`; `kyuubi-common` does not. + */ + def ldapPoolNumAvailableConnections: Long = ldapPoolStat(_.numAvailableConnections) + def ldapPoolNumFailedCheckouts: Long = ldapPoolStat(_.numFailedCheckouts) + def ldapPoolNumSuccessfulCheckouts: Long = ldapPoolStat(_.numSuccessfulCheckouts) + def ldapPoolNumConnectionsClosedDefunct: Long = ldapPoolStat(_.numConnectionsClosedDefunct) + def ldapPoolNumConnectionsClosedExpired: Long = ldapPoolStat(_.numConnectionsClosedExpired) + + private def ldapPoolStat(read: UnboundIdConnectionPool => Long): Long = + UnboundIdConnectionPool.currentInstance.map(read).getOrElse(0L) + + // --------------------------------------------------------------------------------------- + // LDAP authentication request counters. + // + // Held as AtomicLong here so they can be incremented from kyuubi-common code paths + // (CachingLdapAuthenticationProvider, LdapAuthenticationProviderImpl) without taking a + // dependency on kyuubi-metrics. KyuubiServer reads them via the Long getters below and + // registers them as MetricsSystem gauges, which DropwizardExports surfaces 1:1 in + // Prometheus. Counters are reset on close() so test isolation is automatic. + // + // Semantics: + // ldapAuthSuccess = ldapAuthCacheHit + (cache-miss attempts that succeeded) + // sum(ldapAuthFailure_*) = cache-miss attempts that failed + // ldapAuthCacheMiss = (success - cache-hit) + sum(failure_*) [caching ON] + // ldapAuthCacheHit / cache-miss = both 0 when caching is disabled + // --------------------------------------------------------------------------------------- + + private val ldapAuthSuccessCounter = new AtomicLong(0L) + private val ldapAuthFailureInvalidCredentialsCounter = new AtomicLong(0L) + private val ldapAuthFailureInvalidInputCounter = new AtomicLong(0L) + private val ldapAuthFailureInfrastructureCounter = new AtomicLong(0L) + private val ldapAuthCacheHitCounter = new AtomicLong(0L) + private val ldapAuthCacheMissCounter = new AtomicLong(0L) + + // Installable callback for surfacing auth events to a metrics system. See + // [[LdapAuthMetricsRecorder]] for why this is a hook rather than a direct call -- short + // version: cumulative counters must surface in Prometheus as Counter (not Gauge) so + // rate() / increase() handle process restarts correctly, and kyuubi-common does not + // depend on kyuubi-metrics. KyuubiServer installs a real implementation at startup. + @volatile private var ldapAuthMetricsRecorder: LdapAuthMetricsRecorder = + LdapAuthMetricsRecorder.NoOp + + /** + * Installs the recorder that surfaces LDAP auth events to the metrics subsystem. + * Passing `null` resets to the no-op default. Idempotent across calls. + */ + def setLdapAuthMetricsRecorder(recorder: LdapAuthMetricsRecorder): Unit = { + ldapAuthMetricsRecorder = if (recorder == null) LdapAuthMetricsRecorder.NoOp else recorder + } + + /** Records a successful LDAP authentication (cache hit OR cache-miss success). */ + private[authentication] def recordLdapAuthSuccess(): Unit = { + ldapAuthSuccessCounter.incrementAndGet() + ldapAuthMetricsRecorder.recordSuccess() + } + + /** + * Records a failed LDAP authentication, classified by `reason`. Unknown reasons fall + * into the `infrastructure` bucket so a typo at a call site does not silently drop the + * event. + */ + private[authentication] def recordLdapAuthFailure(reason: String): Unit = { + val normalised = reason match { + case LdapAuthFailureClassifier.INVALID_CREDENTIALS => + ldapAuthFailureInvalidCredentialsCounter.incrementAndGet() + LdapAuthFailureClassifier.INVALID_CREDENTIALS + case LdapAuthFailureClassifier.INVALID_INPUT => + ldapAuthFailureInvalidInputCounter.incrementAndGet() + LdapAuthFailureClassifier.INVALID_INPUT + case _ => + ldapAuthFailureInfrastructureCounter.incrementAndGet() + LdapAuthFailureClassifier.INFRASTRUCTURE + } + ldapAuthMetricsRecorder.recordFailure(normalised) + } + + /** Records a cache hit at the LDAP authentication caching layer. */ + private[authentication] def recordLdapAuthCacheHit(): Unit = { + ldapAuthCacheHitCounter.incrementAndGet() + ldapAuthMetricsRecorder.recordCacheHit() + } + + /** Records a cache miss at the LDAP authentication caching layer. */ + private[authentication] def recordLdapAuthCacheMiss(): Unit = { + ldapAuthCacheMissCounter.incrementAndGet() + ldapAuthMetricsRecorder.recordCacheMiss() + } + + // In-process counter accessors retained for test inspection. Tightened to + // `private[authentication]` because production metric exposure now flows through the + // recorder above -- no caller outside this package needs these. + private[authentication] def ldapAuthSuccess: Long = ldapAuthSuccessCounter.get() + private[authentication] def ldapAuthFailureInvalidCredentials: Long = + ldapAuthFailureInvalidCredentialsCounter.get() + private[authentication] def ldapAuthFailureInvalidInput: Long = + ldapAuthFailureInvalidInputCounter.get() + private[authentication] def ldapAuthFailureInfrastructure: Long = + ldapAuthFailureInfrastructureCounter.get() + private[authentication] def ldapAuthCacheHit: Long = ldapAuthCacheHitCounter.get() + private[authentication] def ldapAuthCacheMiss: Long = ldapAuthCacheMissCounter.get() } diff --git a/kyuubi-common/src/main/scala/org/apache/kyuubi/service/authentication/LdapAuthMetricsRecorder.scala b/kyuubi-common/src/main/scala/org/apache/kyuubi/service/authentication/LdapAuthMetricsRecorder.scala new file mode 100644 index 00000000000..5935517e38c --- /dev/null +++ b/kyuubi-common/src/main/scala/org/apache/kyuubi/service/authentication/LdapAuthMetricsRecorder.scala @@ -0,0 +1,81 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.kyuubi.service.authentication + +/** + * Callback hook so the LDAP auth path (in `kyuubi-common`) can publish authentication + * events to a metrics system (in `kyuubi-server`, via `kyuubi-metrics`) without + * `kyuubi-common` taking a build-time dependency on `kyuubi-metrics`. + * + * Why this is a trait rather than a direct call to `MetricsSystem.incCount`: cumulative + * LDAP auth counters must surface in Prometheus as `# TYPE counter`, not `# TYPE gauge`, + * so that `rate()` and `increase()` correctly handle process restarts. The earlier design + * exposed `AtomicLong` values as `Gauge`s, which produced misleading negative rates on + * Kyuubi restarts. With this recorder, `kyuubi-server` plugs in an implementation that + * delegates to `MetricsSystem.incCount`, which goes through Dropwizard's `Counter` type. + * + * Implementations must be thread-safe; `recordXxx` is invoked from request threads on + * every authentication call. + * + * The default registered implementation is [[LdapAuthMetricsRecorder.NoOp]]; the real one + * is installed by `KyuubiServer.start` after `MetricsSystem` has started. + */ +// Visibility is `private[kyuubi]` rather than `private[authentication]` because the +// production implementation lives in `kyuubi-server` (KyuubiServer.installLdapAuthMetricsRecorder) +// where it can delegate to `MetricsSystem.incCount`. The auth path that invokes it lives +// here in `kyuubi-common`. +private[kyuubi] trait LdapAuthMetricsRecorder { + + /** Successful authentication (cache hit OR cache-miss success). */ + def recordSuccess(): Unit + + /** + * Failed authentication. `reason` is one of the constants in + * [[org.apache.kyuubi.service.authentication.ldap.LdapAuthFailureClassifier]] so it + * becomes a Prometheus metric-name suffix. Unknown reasons map to `infrastructure` at + * the caller. + */ + def recordFailure(reason: String): Unit + + /** Cache short-circuited an LDAP round-trip for this request. */ + def recordCacheHit(): Unit + + /** + * This request observed an empty cache entry when it arrived. Per-request, not + * per-LDAP-roundtrip: under request coalescing, multiple concurrent identical requests + * each see an empty entry and each fire recordCacheMiss, even though Guava executes + * only one underlying LDAP authentication. Use cache.miss for request-rate sizing and + * the success / failure counters for actual auth volume. + */ + def recordCacheMiss(): Unit +} + +private[kyuubi] object LdapAuthMetricsRecorder { + + /** + * The recorder installed by default. All `recordXxx` calls are no-ops, so the LDAP auth + * path is metric-agnostic and remains usable in `kyuubi-common`-only contexts (tests, + * `kyuubi-ctl`, etc.) where `MetricsSystem` is not present. + */ + object NoOp extends LdapAuthMetricsRecorder { + override def recordSuccess(): Unit = () + override def recordFailure(reason: String): Unit = () + override def recordCacheHit(): Unit = () + override def recordCacheMiss(): Unit = () + } +} diff --git a/kyuubi-common/src/main/scala/org/apache/kyuubi/service/authentication/LdapAuthenticationProviderImpl.scala b/kyuubi-common/src/main/scala/org/apache/kyuubi/service/authentication/LdapAuthenticationProviderImpl.scala index d885da55b23..26f969b922a 100644 --- a/kyuubi-common/src/main/scala/org/apache/kyuubi/service/authentication/LdapAuthenticationProviderImpl.scala +++ b/kyuubi-common/src/main/scala/org/apache/kyuubi/service/authentication/LdapAuthenticationProviderImpl.scala @@ -50,7 +50,20 @@ class LdapAuthenticationProviderImpl( * @throws AuthenticationException When a user is found to be invalid by the implementation */ override def authenticate(user: String, password: String): Unit = { + // The actual auth flow is delegated to doAuthenticate; this wrapper is the single + // place where the kyuubi.authentication.success / failure metric counters are + // updated, classified by [[LdapAuthFailureClassifier]] for failures. + try { + doAuthenticate(user, password) + AuthenticationProviderFactory.recordLdapAuthSuccess() + } catch { + case e: AuthenticationException => + AuthenticationProviderFactory.recordLdapAuthFailure(LdapAuthFailureClassifier.classify(e)) + throw e + } + } + private def doAuthenticate(user: String, password: String): Unit = { val (usedBind, bindUser, bindPassword) = ( conf.get(KyuubiConf.AUTHENTICATION_LDAP_BIND_USER), conf.get(KyuubiConf.AUTHENTICATION_LDAP_BIND_PASSWORD)) match { @@ -65,6 +78,10 @@ class LdapAuthenticationProviderImpl( } var search: DirSearch = null + // The end-user credential-verification DirSearch (bind-user flow only) wraps an + // ephemeral LDAPConnection -- its close() is the only path that frees the underlying + // socket. Tracked separately so the finally block can close both. + var userSearch: DirSearch = null try { search = createDirSearch(bindUser, bindPassword) applyFilter(search, user) @@ -72,14 +89,30 @@ class LdapAuthenticationProviderImpl( // 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. val username = getUserName(user) - createDirSearch(search.findUserDn(username), password) + userSearch = createDirSearch(search.findUserDn(username), password) } } catch { case e: NamingException => + // Chain the cause so the original LDAPException (result code, server address) is + // visible in audit logs and stack traces. Classification is unaffected: the message + // prefix falls through to classifyByMessage -> INFRASTRUCTURE regardless, but + // preserving the cause allows classifyByCause to also reach that result via the + // NamingException branch, and keeps the full diagnostic chain intact for operators. + throw new AuthenticationException( + s"Unable to find the user in the LDAP tree. ${e.getMessage}", + e) + case e: IllegalStateException => + // Query.QueryBuilder.build wraps an unparseable rendered filter as + // IllegalStateException. The most common trigger is a username containing + // LDAP filter metacharacters ("*", "(", ")", "\") that break RFC 4515 after + // template substitution. Surface this as an AuthenticationException so the + // metric recorder/classifier sees it (invalid_input bucket) instead of letting + // an unclassified RuntimeException escape the auth contract. throw new AuthenticationException( - s"Unable to find the user in the LDAP tree. ${e.getMessage}") + s"Error validating LDAP user, invalid filter for $user: ${e.getMessage}", + e) } finally { - ServiceUtils.cleanup(logger, search) + ServiceUtils.cleanup(logger, search, userSearch) } } diff --git a/kyuubi-common/src/main/scala/org/apache/kyuubi/service/authentication/ldap/LdapAuthCache.scala b/kyuubi-common/src/main/scala/org/apache/kyuubi/service/authentication/ldap/LdapAuthCache.scala new file mode 100644 index 00000000000..b2000bacfac --- /dev/null +++ b/kyuubi-common/src/main/scala/org/apache/kyuubi/service/authentication/ldap/LdapAuthCache.scala @@ -0,0 +1,275 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.kyuubi.service.authentication.ldap + +import java.nio.charset.StandardCharsets +import java.security.SecureRandom +import java.util.concurrent.{Callable, ExecutionException, TimeUnit} +import javax.crypto.Mac +import javax.crypto.spec.SecretKeySpec +import javax.security.sasl.AuthenticationException + +import com.google.common.cache.{Cache, CacheBuilder} +import com.google.common.util.concurrent.UncheckedExecutionException + +import org.apache.kyuubi.Logging +import org.apache.kyuubi.config.KyuubiConf +import org.apache.kyuubi.service.authentication.{AuthenticationProviderFactory, PasswdAuthenticationProvider} + +/** + * [[PasswdAuthenticationProvider]] decorator that short-circuits repeated LDAP round-trips + * for the same (username, password) pair within a configurable TTL. + * + * Cache key: HMAC-SHA256(username + NUL + password) encoded as a hex string. Using HMAC + * rather than a plain hash prevents offline dictionary attacks against the in-memory cache; + * the key material (secret) is generated once at JVM startup and never leaves the process. + * Passwords themselves are not stored anywhere in the cache. + * + * Only successful authentications are cached. Failed attempts always reach LDAP, which + * allows clients to correct credentials without waiting for a TTL to expire, and avoids + * the security risk of caching a "bad credentials" result that would block a subsequent + * correct password. + * + * The backing Guava [[Cache]] lives on the [[LdapAuthCache]] companion object so that the + * "server" and "http" auth scopes (which AuthenticationProviderFactory builds independently + * via computeIfAbsent) share a single cache. Both scopes hit the same LDAP backend with + * the same user identity, so cross-protocol cache reuse is correct and removes the second + * LDAP round-trip a user pays when they connect over both Thrift and HTTP. + */ +private[authentication] class CachingLdapAuthenticationProvider( + delegate: PasswdAuthenticationProvider, + conf: KyuubiConf) + extends PasswdAuthenticationProvider with Logging { + + // The cache itself is shared across all CachingLdapAuthenticationProvider instances -- + // see the class doc for why. Conf-derived TTL is also kept here only for the debug log + // formatting; the cache's actual TTL is set by getOrCreateCache from the first conf. + private val cache: Cache[String, java.lang.Boolean] = LdapAuthCache.getOrCreateCache(conf) + private val ttlMs = conf.get(KyuubiConf.AUTHENTICATION_LDAP_CACHE_TTL) + + override def authenticate(user: String, password: String): Unit = { + // Null inputs: bypass cacheKey entirely and delegate directly. String concatenation + // converts null to the literal "null", so cacheKey(null, p) would hash "nullp" + // -- producing a semantically wrong key that could collide with a real username + // "null". The delegate validates blank/null inputs and throws AuthenticationException + // with proper invalid_input classification; no caching is needed for that path. + if (user == null || password == null) { + AuthenticationProviderFactory.recordLdapAuthCacheMiss() + delegate.authenticate(user, password) + return + } + val key = LdapAuthCache.cacheKey(user, password) + + // Fast path: avoid Callable allocation when the entry is already present. A cache hit + // is itself a successful authentication from the user's perspective, so it increments + // the success counter alongside the cache.hit counter -- the delegate is NOT called, + // so the impl-layer success counter is intentionally not bumped here. + if (cache.getIfPresent(key) != null) { + AuthenticationProviderFactory.recordLdapAuthCacheHit() + AuthenticationProviderFactory.recordLdapAuthSuccess() + debug(s"LDAP auth cache hit for user $user") + return + } + + // Cache miss. cache.get(key, Callable) coalesces concurrent identical requests so a + // burst of authentications for the same (user, password) results in one LDAP + // round-trip rather than N: exactly one thread (the "leader") invokes the callable + // while the others ("waiters") block until the leader's result is available. + // + // Per-request accounting under coalescing: every request observed an empty cache when + // it arrived, so every request bumps cache.miss (this matches the documented semantics + // of cache.miss as "this request did not find a cached entry"). For success/failure + // counters, however, the impl inside the callable only records once -- for the leader. + // To keep success/failure counts per-request (so 8 successful concurrent auths show as + // 8 successes, not 1), waiters must record their own outcome on the wrapper side. The + // `wasLeader` flag below distinguishes the leader (whose impl already recorded) from + // waiters (who need wrapper-side recording). + // + // Failure path symmetric: Guava propagates the leader's exception to all waiters via + // the same ExecutionException, and the impl recorded the leader's failure once -- but + // every request observed a failed authentication, so waiters classify and record their + // own failure. Classification uses the cause chain when present, falling back to the + // ExecutionException itself. + // + // Guava propagates the callable's exception WITHOUT populating the cache, so failed + // authentications are never cached -- preserving the no-negative-caching contract. + AuthenticationProviderFactory.recordLdapAuthCacheMiss() + var wasLeader = false + try { + cache.get( + key, + new Callable[java.lang.Boolean] { + override def call(): java.lang.Boolean = { + wasLeader = true + delegate.authenticate(user, password) + java.lang.Boolean.TRUE + } + }) + if (!wasLeader) { + // Waited on the leader's successful authentication; record my own success. + AuthenticationProviderFactory.recordLdapAuthSuccess() + } + debug(s"LDAP auth cache populated for user $user (ttl=${ttlMs}ms)") + } catch { + case e: ExecutionException => + if (!wasLeader) recordCoalescedFailure(e) + CachingLdapAuthenticationProvider.rethrowAsAuth(e) + case e: UncheckedExecutionException => + if (!wasLeader) recordCoalescedFailure(e) + CachingLdapAuthenticationProvider.rethrowAsAuth(e) + } + } + + /** + * Classifies and records a failure observed by a coalesced waiter. The leader's impl + * has already recorded its own failure with the classification it derived from the + * underlying exception; here we classify from whatever Guava handed us so that the + * waiter's failure bucket matches the leader's as closely as possible. + */ + private def recordCoalescedFailure(wrapped: Exception): Unit = { + val cause = Option(wrapped.getCause).getOrElse(wrapped) + AuthenticationProviderFactory.recordLdapAuthFailure( + LdapAuthFailureClassifier.classify(cause)) + } + + // Test-only helpers: scoped to the ldap package so unit tests can drive the cache, but + // hidden from production call sites that should never need to peek at the cache state. + private[ldap] def invalidate(user: String, password: String): Unit = + cache.invalidate(LdapAuthCache.cacheKey(user, password)) + + private[ldap] def cacheSize(): Long = cache.size() +} + +private[authentication] object CachingLdapAuthenticationProvider { + + /** + * Unwraps a Guava cache-loader wrapper exception and rethrows it as an + * [[AuthenticationException]]. The original cause is preserved either by rethrowing it + * directly (if it already is an [[AuthenticationException]]) or by chaining it inside a + * fresh [[AuthenticationException]] -- so callers see the same exception type they would + * have seen without the cache wrapper. + */ + private def rethrowAsAuth(wrapped: Exception): Nothing = wrapped.getCause match { + case ae: AuthenticationException => throw ae + case other => + val cause = if (other != null) other else wrapped + val ae = new AuthenticationException(cause.getMessage) + ae.initCause(cause) + throw ae + } +} + +// Visibility is `private[authentication]` rather than `private[ldap]` because +// `AuthenticationProviderFactory.close()` needs to reset the shared cache between server +// lifecycles (and between test cases) -- the factory lives one package up. +private[authentication] object LdapAuthCache extends Logging { + + // Per-process secret generated once at class load time. HMAC key is 32 random bytes. + private val hmacSecret: Array[Byte] = { + val bytes = new Array[Byte](32) + new SecureRandom().nextBytes(bytes) + bytes + } + + // One initialised Mac per thread -- amortises JCA provider lookup and SecretKeySpec + // allocation across requests. Mac.doFinal() resets state per JCA spec, so the instance + // is safe to reuse after each call without an explicit reset(). + private val threadLocalMac: ThreadLocal[Mac] = ThreadLocal.withInitial[Mac](() => { + val mac = Mac.getInstance("HmacSHA256") + mac.init(new SecretKeySpec(hmacSecret, "HmacSHA256")) + mac + }) + + // NUL (\u0000) separator between user and password makes the HMAC input unambiguous -- + // without it, ("a", "bc") and ("ab", "c") would hash identically. The Unicode escape is + // used in the source (rather than a literal 0x00 byte) so the file stays plain ASCII + // and tools like git diff, GitHub, scalafmt, and grep handle it correctly. + private val UserPasswordSeparator: String = "\u0000" + + def cacheKey(user: String, password: String): String = { + val raw = threadLocalMac + .get() + .doFinal((user + UserPasswordSeparator + password).getBytes(StandardCharsets.UTF_8)) + toHex(raw) + } + + // Shared cache backing every CachingLdapAuthenticationProvider in the process. Lazy init + // is gated by getOrCreateCache below; resetCache nulls it out for test isolation. The + // ttl / max-size snapshot of the first conf is kept alongside so subsequent callers with + // divergent conf get a visible warning rather than a silent reuse. + @volatile private var sharedCache: Cache[String, java.lang.Boolean] = _ + @volatile private var initialTtlMs: Long = 0L + @volatile private var initialMaxSize: Int = 0 + private val cacheInitLock = new Object() + + /** + * Returns the singleton auth-success cache. The first caller's conf determines the cache + * size and TTL; subsequent callers reuse the same instance regardless of their conf. + * That assumption is safe in production because AuthenticationProviderFactory always + * builds providers from the same KyuubiConf within a process lifetime, but it can drift + * in embedded contexts or tests that build multiple providers from different confs. + * To make that drift observable we log on init and warn on every divergent-conf reuse. + */ + private[ldap] def getOrCreateCache(conf: KyuubiConf): Cache[String, java.lang.Boolean] = { + if (sharedCache == null) { + cacheInitLock.synchronized { + if (sharedCache == null) { + val ttlMs = conf.get(KyuubiConf.AUTHENTICATION_LDAP_CACHE_TTL) + val maxSize = conf.get(KyuubiConf.AUTHENTICATION_LDAP_CACHE_MAX_SIZE) + sharedCache = CacheBuilder.newBuilder() + .maximumSize(maxSize) + .expireAfterWrite(ttlMs, TimeUnit.MILLISECONDS) + .build() + initialTtlMs = ttlMs + initialMaxSize = maxSize + info(s"LDAP auth cache initialised: ttlMs=$ttlMs maxSize=$maxSize") + } + } + } else { + // Already initialised; the existing cache is not reconfigured. Warn the caller if + // their conf disagrees so the silent first-caller-wins behaviour is observable. + val ttlMs = conf.get(KyuubiConf.AUTHENTICATION_LDAP_CACHE_TTL) + val maxSize = conf.get(KyuubiConf.AUTHENTICATION_LDAP_CACHE_MAX_SIZE) + if (ttlMs != initialTtlMs || maxSize != initialMaxSize) { + warn( + s"LDAP auth cache reuse with mismatched config: initial " + + s"(ttlMs=$initialTtlMs maxSize=$initialMaxSize) vs requested " + + s"(ttlMs=$ttlMs maxSize=$maxSize). The existing cache is unchanged. " + + s"Restart Kyuubi to apply the new values.") + } + } + sharedCache + } + + /** + * Discards the shared cache so the next [[getOrCreateCache]] call rebuilds it from fresh + * conf. Invoked from [[AuthenticationProviderFactory.close]]; effectively a no-op in + * production (close runs once at JVM shutdown) and used between tests for isolation. + */ + private[authentication] def resetCache(): Unit = cacheInitLock.synchronized { + sharedCache = null + initialTtlMs = 0L + initialMaxSize = 0 + } + + private def toHex(bytes: Array[Byte]): String = { + val sb = new StringBuilder(bytes.length * 2) + bytes.foreach(b => sb.append(f"${b & 0xFF}%02x")) + sb.toString() + } +} diff --git a/kyuubi-common/src/main/scala/org/apache/kyuubi/service/authentication/ldap/LdapAuthFailureClassifier.scala b/kyuubi-common/src/main/scala/org/apache/kyuubi/service/authentication/ldap/LdapAuthFailureClassifier.scala new file mode 100644 index 00000000000..72133fb643c --- /dev/null +++ b/kyuubi-common/src/main/scala/org/apache/kyuubi/service/authentication/ldap/LdapAuthFailureClassifier.scala @@ -0,0 +1,87 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.kyuubi.service.authentication.ldap + +import javax.naming.{AuthenticationException => JndiAuthenticationException, NamingException} +import javax.security.sasl.AuthenticationException + +import com.unboundid.ldap.sdk.{LDAPException, ResultCode} + +/** + * Classifies an LDAP authentication failure into one of three operational categories so + * that dashboards and alerts can distinguish credential rejection from misconfiguration + * from infrastructure outage. + * + * Categories: + * - `invalid_credentials`: the LDAP server explicitly rejected the bind with + * [[ResultCode.INVALID_CREDENTIALS]] (LDAP error 49). Expected and high-baseline. + * - `invalid_input`: the request itself was malformed -- blank user/password, or the + * user format produced no candidate principals against the configured DN patterns. + * Almost always client-side; baseline near zero in healthy production. + * - `infrastructure`: anything else -- connection failures, timeouts, search errors, + * server-internal errors. Baseline zero; spikes warrant paging. + * + * Classification first inspects the cause chain (most reliable) and falls back to message + * prefixes for the cases where our own code threw [[AuthenticationException]] with no + * cause attached (notably the blank-input checks in [[LdapAuthenticationProviderImpl]]). + */ +// Visibility is `private[kyuubi]` rather than `private[authentication]` because the +// classification strings are used as metric-name suffixes by kyuubi-server (see +// org.apache.kyuubi.server.KyuubiServer.installLdapAuthMetricsRecorder). +private[kyuubi] object LdapAuthFailureClassifier { + + val INVALID_CREDENTIALS: String = "invalid_credentials" + val INVALID_INPUT: String = "invalid_input" + val INFRASTRUCTURE: String = "infrastructure" + + // Message prefixes thrown by our own code for the invalid_input category. Kept here + // (not duplicated from the throw sites) so a future change to those messages will need + // to update this list -- the test suite asserts the mapping holds. + private val InvalidInputPrefixes: Seq[String] = Seq( + "Error validating LDAP user, user is null", + "Error validating LDAP user, password is null", + "Error validating LDAP user, invalid filter", + "No candidate principals for") + + def classify(t: Throwable): String = t match { + case ae: AuthenticationException => + classifyByCause(ae.getCause).getOrElse(classifyByMessage(ae.getMessage)) + case _: NamingException => INFRASTRUCTURE + case _ => INFRASTRUCTURE + } + + private def classifyByCause(cause: Throwable): Option[String] = cause match { + case le: LDAPException if le.getResultCode == ResultCode.INVALID_CREDENTIALS => + Some(INVALID_CREDENTIALS) + case _: LDAPException => Some(INFRASTRUCTURE) + // Legacy JNDI path: a failed bind surfaces as javax.naming.AuthenticationException, + // the JNDI equivalent of LDAP result code 49. It is a NamingException subtype, so this + // case must precede the generic NamingException case below; otherwise wrong-password + // attempts on the pool-disabled path would be bucketed as `infrastructure` and page + // operators as an outage. + case _: JndiAuthenticationException => Some(INVALID_CREDENTIALS) + case _: NamingException => Some(INFRASTRUCTURE) + case _ => None + } + + private def classifyByMessage(message: String): String = { + val msg = Option(message).getOrElse("") + if (InvalidInputPrefixes.exists(msg.startsWith)) INVALID_INPUT + else INFRASTRUCTURE + } +} diff --git a/kyuubi-common/src/main/scala/org/apache/kyuubi/service/authentication/ldap/LdapSearch.scala b/kyuubi-common/src/main/scala/org/apache/kyuubi/service/authentication/ldap/LdapSearch.scala index 09dca1d5c3a..cf25f9b7d5f 100644 --- a/kyuubi-common/src/main/scala/org/apache/kyuubi/service/authentication/ldap/LdapSearch.scala +++ b/kyuubi-common/src/main/scala/org/apache/kyuubi/service/authentication/ldap/LdapSearch.scala @@ -17,8 +17,8 @@ package org.apache.kyuubi.service.authentication.ldap -import javax.naming.{NamingEnumeration, NamingException} -import javax.naming.directory.{DirContext, SearchResult} +import javax.naming.{NameNotFoundException, NamingEnumeration, NamingException} +import javax.naming.directory.{DirContext, SearchControls, SearchResult} import scala.collection.mutable.ArrayBuffer @@ -80,7 +80,10 @@ class LdapSearch(conf: KyuubiConf, ctx: DirContext) extends DirSearch with Loggi private def findDnByPattern(patterns: Seq[String], name: String): Array[String] = { for (pattern <- patterns) { val baseDnFromPattern: String = LdapUtils.extractBaseDn(pattern) - val rdn = LdapUtils.extractFirstRdn(pattern).replaceAll("%s", name) + // Use replace (literal) not replaceAll (regex): a name containing '$' or '\' is + // otherwise interpreted as a regex replacement back-reference/escape, corrupting the + // RDN or throwing IndexOutOfBoundsException. + val rdn = LdapUtils.extractFirstRdn(pattern).replace("%s", name) val names = execute(Array(baseDnFromPattern), queries.findDnByPattern(rdn)).getAllLdapNames if (!names.isEmpty) return names } @@ -108,17 +111,36 @@ class LdapSearch(conf: KyuubiConf, ctx: DirContext) extends DirSearch with Loggi execute(Array(baseDn), queries.customQuery(query)).getAllLdapNamesAndAttributes private def execute(baseDns: Array[String], query: Query): SearchResultHandler = { + // Reconstruct JNDI SearchControls from the Query fields. Query no longer holds a + // SearchControls directly; the UnboundID path uses query.filter (Filter object) and + // query.attributes / query.sizeLimit natively, while this JNDI path builds its own. + val controls = new SearchControls() + controls.setSearchScope(SearchControls.SUBTREE_SCOPE) + // Always set returning attributes. An empty array means "return no attributes" (DN only) + // -- the pre-UnboundID default -- so DN-only lookups do not fetch every attribute; the + // SearchControls default of null would instead mean "return all". customQuery is the only + // query that populates attributes, so it still receives its requested attribute. + controls.setReturningAttributes(query.attributes.toArray) + if (query.sizeLimit > 0) controls.setCountLimit(query.sizeLimit) + val searchResults = new ArrayBuffer[NamingEnumeration[SearchResult]] - debug(s"Executing a query: '${query.filter}' with base DNs ${baseDns.mkString(",")}") + debug(s"Executing a query: '${query.filterString}' with base DNs ${baseDns.mkString(",")}") baseDns.foreach { baseDn => try { - val searchResult = ctx.search(baseDn, query.filter, query.controls) + val searchResult = ctx.search(baseDn, query.filterString, controls) if (searchResult != null) searchResults += searchResult } catch { + case _: NameNotFoundException => + // Base DN does not exist in the directory -- not an error, try the next base. + // This is the JNDI equivalent of UnboundIdDirSearch swallowing NO_SUCH_OBJECT. + debug(s"Base DN '$baseDn' not found for query '${query.filterString}', skipping.") case ex: NamingException => - debug( - s"Exception happened for query '${query.filter}' with base DN '$baseDn'", - ex) + // Any other JNDI failure (CommunicationException, ServiceUnavailableException, + // AuthenticationException, etc.) means the LDAP server is unreachable or the + // request was rejected at the protocol level. Propagate so callers can + // distinguish "user not found" from "LDAP is down" -- the original broad catch + // silently turned infrastructure failures into empty results, masking outages. + throw ex } } new SearchResultHandler(searchResults.toArray) diff --git a/kyuubi-common/src/main/scala/org/apache/kyuubi/service/authentication/ldap/LdapServerSets.scala b/kyuubi-common/src/main/scala/org/apache/kyuubi/service/authentication/ldap/LdapServerSets.scala new file mode 100644 index 00000000000..65531991eb2 --- /dev/null +++ b/kyuubi-common/src/main/scala/org/apache/kyuubi/service/authentication/ldap/LdapServerSets.scala @@ -0,0 +1,83 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.kyuubi.service.authentication.ldap + +import javax.net.SocketFactory + +import com.unboundid.ldap.sdk._ + +import org.apache.kyuubi.config.KyuubiConf + +/** + * Shared construction helpers for [[LDAPConnectionOptions]] and [[ServerSet]] so the pool + * path ([[UnboundIdConnectionPool]]) and the ephemeral path ([[UnboundIdDirSearchFactory]]) + * cannot drift apart on timeouts, SSL handling, or failover topology. + * + * The only intentional difference between the two paths is `autoReconnect`: pooled + * connections benefit from automatic reconnect on detected closures, while ephemeral + * connections are short-lived and discarded after a single bind, so reconnect is wasted work. + */ +private[ldap] object LdapServerSets { + + /** + * Build an [[LDAPConnectionOptions]] from Kyuubi conf. + * @param autoReconnect true for pooled connections, false for ephemeral. + */ + def buildConnectionOptions(conf: KyuubiConf, autoReconnect: Boolean): LDAPConnectionOptions = { + val opts = new LDAPConnectionOptions() + // The config is a Long of milliseconds; setConnectTimeoutMillis takes an int. Clamp to + // Int.MaxValue (~24 days) so an out-of-range value cannot silently overflow into a + // negative/garbage timeout. Time configs are non-negative, so no lower-bound clamp. + opts.setConnectTimeoutMillis( + math.min( + conf.get(KyuubiConf.AUTHENTICATION_LDAP_POOL_CONNECT_TIMEOUT), + Int.MaxValue.toLong).toInt) + opts.setResponseTimeoutMillis( + conf.get(KyuubiConf.AUTHENTICATION_LDAP_POOL_RESPONSE_TIMEOUT)) + opts.setAutoReconnect(autoReconnect) + opts + } + + /** + * Build a [[ServerSet]] from one or more [[LDAPURL]]s. A single URL produces a + * [[SingleServerSet]]; multiple URLs produce a [[FailoverServerSet]] that tries each + * server in order, advancing only on connection-level failures (so that bind errors + * such as INVALID_CREDENTIALS surface immediately without retrying other servers). + * + * @throws IllegalArgumentException if `urls` is empty -- callers must validate config. + */ + def buildServerSet( + urls: Seq[LDAPURL], + options: LDAPConnectionOptions, + sslSocketFactory: SocketFactory): ServerSet = { + require(urls.nonEmpty, "LDAP URL list must not be empty") + + def makeSingle(url: LDAPURL): SingleServerSet = { + if (url.getScheme.equalsIgnoreCase("ldaps")) { + new SingleServerSet(url.getHost, url.getPort, sslSocketFactory, options) + } else { + new SingleServerSet(url.getHost, url.getPort, options) + } + } + + urls match { + case Seq(url) => makeSingle(url) + case multi => new FailoverServerSet(multi.map(u => makeSingle(u): ServerSet): _*) + } + } +} diff --git a/kyuubi-common/src/main/scala/org/apache/kyuubi/service/authentication/ldap/LdapUrlParser.scala b/kyuubi-common/src/main/scala/org/apache/kyuubi/service/authentication/ldap/LdapUrlParser.scala new file mode 100644 index 00000000000..855b9d3f9ef --- /dev/null +++ b/kyuubi-common/src/main/scala/org/apache/kyuubi/service/authentication/ldap/LdapUrlParser.scala @@ -0,0 +1,47 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.kyuubi.service.authentication.ldap + +import com.unboundid.ldap.sdk.{LDAPException, LDAPURL} + +/** + * Thin helper for parsing the whitespace-separated LDAP URL list stored in + * [[org.apache.kyuubi.config.KyuubiConf.AUTHENTICATION_LDAP_URL]]. + * + * Delegates all URL parsing to [[LDAPURL]], which is RFC 4516-compliant and handles + * default ports (389 for ldap, 636 for ldaps) without custom regex logic. + */ +private[ldap] object LdapUrlParser { + + /** + * Splits `urlString` on whitespace and parses each token as an [[LDAPURL]]. + * + * @throws IllegalArgumentException if any token is not a valid LDAP URL. + */ + def parseAll(urlString: String): Seq[LDAPURL] = + urlString.trim.split("\\s+").iterator + .filter(_.nonEmpty) + .map { raw => + try new LDAPURL(raw) + catch { + case e: LDAPException => + throw new IllegalArgumentException(s"Invalid LDAP URL: $raw", e) + } + } + .toSeq +} diff --git a/kyuubi-common/src/main/scala/org/apache/kyuubi/service/authentication/ldap/LdapUtils.scala b/kyuubi-common/src/main/scala/org/apache/kyuubi/service/authentication/ldap/LdapUtils.scala index e304e96f733..00fda5417ee 100644 --- a/kyuubi-common/src/main/scala/org/apache/kyuubi/service/authentication/ldap/LdapUtils.scala +++ b/kyuubi-common/src/main/scala/org/apache/kyuubi/service/authentication/ldap/LdapUtils.scala @@ -19,6 +19,8 @@ package org.apache.kyuubi.service.authentication.ldap import scala.collection.mutable.ArrayBuffer +import com.unboundid.ldap.sdk.{DN, LDAPException} + import org.apache.kyuubi.Logging import org.apache.kyuubi.config.{KyuubiConf, OptionalConfigEntry} import org.apache.kyuubi.service.ServiceUtils @@ -35,16 +37,21 @@ object LdapUtils extends Logging { *
* "ou=CORP,dc=mycompany,dc=com" is the base DN for "cn=user1,ou=CORP,dc=mycompany,dc=com" * - * @param dn distinguished name - * @return base DN + * Uses [[DN]] for correct parsing (handles escaped commas and multi-valued RDNs). Falls + * back to a simple comma-split for pattern strings containing "%s" placeholders, which are + * not valid DNs but are also passed through this method during filter construction. + * + * @param dn distinguished name or DN pattern + * @return base DN, or null if the input has no parent (single-RDN or unparseable) */ - def extractBaseDn(dn: String): String = { - val indexOfFirstDelimiter = dn.indexOf(",") - if (indexOfFirstDelimiter > -1) { - return dn.substring(indexOfFirstDelimiter + 1) + def extractBaseDn(dn: String): String = + try { + Option(new DN(dn).getParent).map(_.toString).orNull + } catch { + case _: LDAPException => + val idx = dn.indexOf(",") + if (idx > -1) dn.substring(idx + 1) else null } - null - } /** * Extracts the first Relative Distinguished Name (RDN). @@ -53,10 +60,20 @@ object LdapUtils extends Logging { *
* For DN "cn=user1,ou=CORP,dc=mycompany,dc=com" this method will return "cn=user1" * - * @param dn distinguished name - * @return first RDN + * Uses [[DN]] for correct parsing (handles escaped commas and multi-valued RDNs). Falls + * back to a simple comma-split for pattern strings containing "%s" placeholders. + * + * @param dn distinguished name or DN pattern + * @return first RDN string */ - def extractFirstRdn(dn: String): String = dn.substring(0, dn.indexOf(",")) + def extractFirstRdn(dn: String): String = + try { + new DN(dn).getRDNString + } catch { + case _: LDAPException => + val idx = dn.indexOf(",") + if (idx > -1) dn.substring(0, idx) else dn + } /** * Extracts username from user DN. @@ -132,14 +149,22 @@ object LdapUtils extends Logging { * 
    * LdapUtils.isDn("cn=UserName,dc=mycompany,dc=com") = true
    * LdapUtils.isDn("user1")                           = false
+   * LdapUtils.isDn(null)                              = false
    *
* + * Uses [[DN.isValidDN]] for RFC 4514-compliant validation (handles multi-valued RDNs, + * escaped special characters, etc.) instead of a naive `contains("=")` check that would + * produce false positives for arbitrary "key=value" strings. + * + * The empty string is explicitly rejected even though it is the (valid) "null DN" per + * RFC 4514, because for our use case (deciding whether a value is a DN-typed login or a + * uid-style login) an empty value is never a DN. + * * @param name name to be checked - * @return true if the provided name is a distinguished name + * @return true if the provided name is a non-empty, valid distinguished name */ - def isDn(name: String): Boolean = { - name.contains("=") - } + def isDn(name: String): Boolean = + name != null && name.nonEmpty && DN.isValidDN(name) /** * Reads and parses DN patterns from Kyuubi configuration. @@ -206,7 +231,9 @@ object LdapUtils extends Logging { if (userPatterns.isEmpty) { return Array(user) } - userPatterns.map(_.replaceAll("%s", user)) + // Use replace (literal) not replaceAll (regex): a user containing '$' or '\' is + // otherwise interpreted as a regex replacement back-reference/escape. + userPatterns.map(_.replace("%s", user)) } } } diff --git a/kyuubi-common/src/main/scala/org/apache/kyuubi/service/authentication/ldap/Query.scala b/kyuubi-common/src/main/scala/org/apache/kyuubi/service/authentication/ldap/Query.scala index 3c0321c748c..5305419c1ec 100644 --- a/kyuubi-common/src/main/scala/org/apache/kyuubi/service/authentication/ldap/Query.scala +++ b/kyuubi-common/src/main/scala/org/apache/kyuubi/service/authentication/ldap/Query.scala @@ -17,18 +17,16 @@ package org.apache.kyuubi.service.authentication.ldap -import java.util -import javax.naming.directory.SearchControls +import scala.collection.mutable.ArrayBuffer +import com.unboundid.ldap.sdk.{Filter => LdapFilter, LDAPException} import org.stringtemplate.v4.{ST, STGroup} /** * The object that encompasses all components of a Directory Service search query. * - * The caller must and can only call each [[filter]] and [[build]] once, - * otherwise [[ST]] internal cache may leak and cause heap OOM. - * * @see [[LdapSearch]] + * @see [[UnboundIdDirSearch]] */ object Query { @@ -43,17 +41,9 @@ object Query { * A builder of the [[Query]]. */ final class QueryBuilder { - - /** The [[STGroup]] used only for this builder's filter template; unloaded in [[build]]. */ - private var filterTemplateGroup: Option[STGroup] = None private var filterTemplate: ST = _ - private val controls: SearchControls = { - val _controls = new SearchControls - _controls.setSearchScope(SearchControls.SUBTREE_SCOPE) - _controls.setReturningAttributes(new Array[String](0)) - _controls - } - private val returningAttributes: util.List[String] = new util.ArrayList[String] + private val attributes: ArrayBuffer[String] = ArrayBuffer.empty + private var sizeLimit: Int = 0 /** * Sets search filter template. @@ -62,8 +52,11 @@ object Query { * @return the current instance of the builder */ def filter(filterTemplate: String): Query.QueryBuilder = { + // Use a dedicated STGroup per filter so LDAP filter compile/render is not concurrent on the + // same group. Sharwell (ST4 maintainer) advises against using one STGroup from multiple + // threads at once; options are one STGroup per thread, a pool, or external synchronization. + // See https://github.com/antlr/stringtemplate4/issues/61#issuecomment-43803970 val group = new STGroup() - this.filterTemplateGroup = Some(group) this.filterTemplate = new ST(group, filterTemplate) this } @@ -80,6 +73,28 @@ object Query { this } + /** + * Sets mapping between names in the search filter template and an RFC 4515-escaped + * attribute value. Use this for any user-supplied or externally-sourced string that + * ends up in an attribute value assertion position (e.g. `(uid=)`). + * + * Escaping converts the five special characters (`*`, `(`, `)`, `\`, NUL) to their + * `\xx` hex forms per RFC 4515 section 3. This prevents filter injection: a username + * like `jsmith)(uid=admin` is rendered as `jsmith\29\28uid\3dadmin` and treated as a + * literal string rather than injected filter syntax. + * + * Do NOT use this for parameters in filter-fragment positions (e.g. `()`) or + * attribute-name positions -- only attribute value literals should be escaped. + * + * @param key marker in the search filter template. + * @param value user-controlled attribute value to escape before substitution + * @return the current instance of the builder + */ + def mapEscaped(key: String, value: String): Query.QueryBuilder = { + filterTemplate.add(key, LdapFilter.encodeValue(value)) + this + } + /** * Sets mapping between names in the search filter template and actual values. * @@ -99,7 +114,7 @@ object Query { * @return the current instance of the builder */ def returnAttribute(attributeName: String): Query.QueryBuilder = { - returningAttributes.add(attributeName) + attributes += attributeName this } @@ -112,7 +127,7 @@ object Query { * @return the current instance of the builder */ def limit(limit: Int): Query.QueryBuilder = { - controls.setCountLimit(limit) + sizeLimit = limit this } @@ -120,27 +135,45 @@ object Query { require(filterTemplate != null, "filter is required for LDAP search query") } - private def createFilter(): String = filterTemplate.render() - - private def updateControls(): Unit = { - if (!returningAttributes.isEmpty) controls.setReturningAttributes( - returningAttributes.toArray(new Array[String](returningAttributes.size))) - } - /** * Builds an instance of [[Query]]. * * @return configured directory service query + * @throws IllegalStateException if the rendered filter string is not valid RFC 4515 */ def build: Query = { validate() - val filter: String = createFilter() + val filterStr: String = filterTemplate.render() // Unload template cache after render to avoid CompiledST/STToken retention - filterTemplateGroup.foreach(_.unload()) - updateControls() - new Query(filter, controls) + Option(filterTemplate.groupThatCreatedThisInstance).foreach(_.unload()) + val filter: LdapFilter = + try LdapFilter.create(filterStr) + catch { + case e: LDAPException => + throw new IllegalStateException( + s"Rendered LDAP filter is not valid RFC 4515: '$filterStr'", + e) + } + Query(filterStr, filter, attributes.toSeq, sizeLimit) } } } -case class Query(filter: String, controls: SearchControls) +/** + * An immutable LDAP search query. + * + * @param filterString The RFC 4515 filter string as rendered from the template. Used by the + * JNDI path ([[LdapSearch]]) which passes a raw string to + * [[javax.naming.directory.DirContext#search]]. + * @param filter The parsed [[LdapFilter]] object. Used by the UnboundID path + * ([[UnboundIdDirSearch]]) which passes it directly to [[SearchRequest]], + * eliminating a redundant parse step inside the SDK. + * @param attributes Attribute names to return in search results. Empty means return no + * attributes (DN only); see [[UnboundIdDirSearch]] and [[LdapSearch]]. + * @param sizeLimit Maximum entries to return per base DN; 0 means no limit. + */ +case class Query( + filterString: String, + filter: LdapFilter, + attributes: Seq[String], + sizeLimit: Int) diff --git a/kyuubi-common/src/main/scala/org/apache/kyuubi/service/authentication/ldap/QueryFactory.scala b/kyuubi-common/src/main/scala/org/apache/kyuubi/service/authentication/ldap/QueryFactory.scala index 849006e3845..c5ec1211322 100644 --- a/kyuubi-common/src/main/scala/org/apache/kyuubi/service/authentication/ldap/QueryFactory.scala +++ b/kyuubi-common/src/main/scala/org/apache/kyuubi/service/authentication/ldap/QueryFactory.scala @@ -80,7 +80,7 @@ final class QueryFactory(conf: KyuubiConf) { .filter("(&(|)}>)" + "(|(uid=)(sAMAccountName=)))") .map("classes", USER_OBJECT_CLASSES) - .map("userName", userName) + .mapEscaped("userName", userName) // user-supplied; escape to prevent filter injection .limit(2) .build @@ -96,8 +96,8 @@ final class QueryFactory(conf: KyuubiConf) { "(|(=)(=)))") .map("groupClassAttr", groupClassAttr) .map("groupMembershipAttr", groupMembershipAttr) - .map("userName", userName) - .map("userDn", userDn) + .mapEscaped("userName", userName) // user-derived; escape to prevent filter injection + .mapEscaped("userDn", userDn) // directory-resolved DN; escaping is harmless and defensive .build /** @@ -122,8 +122,8 @@ final class QueryFactory(conf: KyuubiConf) { .map("classes", USER_OBJECT_CLASSES) .map("guidAttr", guidAttr) .map("userMembershipAttr", userMembershipAttrOpt.get) - .map("userId", userId) - .map("groupDn", groupDn) + .mapEscaped("userId", userId) // user-derived identifier; escape to prevent filter injection + .mapEscaped("groupDn", groupDn) // directory-resolved DN; escaping is harmless and defensive .limit(2) .build } diff --git a/kyuubi-common/src/main/scala/org/apache/kyuubi/service/authentication/ldap/UnboundIdConnectionPool.scala b/kyuubi-common/src/main/scala/org/apache/kyuubi/service/authentication/ldap/UnboundIdConnectionPool.scala new file mode 100644 index 00000000000..008bd5e05a0 --- /dev/null +++ b/kyuubi-common/src/main/scala/org/apache/kyuubi/service/authentication/ldap/UnboundIdConnectionPool.scala @@ -0,0 +1,289 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.kyuubi.service.authentication.ldap + +import java.io.Closeable +import java.util.concurrent.atomic.AtomicBoolean +import javax.net.ssl.SSLContext + +import com.unboundid.ldap.sdk._ + +import org.apache.kyuubi.Logging +import org.apache.kyuubi.config.KyuubiConf + +/** + * Owns a persistent [[LDAPConnectionPool]] backed by an UnboundID [[FailoverServerSet]]. + * + * One instance is created at server startup and shared across all authentication requests. + * The pool holds pre-authenticated connections for the configured bind user so that each + * authentication request borrows a warm connection rather than performing a full TCP + TLS + * + BIND round-trip. A background health-check thread validates idle connections against the + * LDAP root DSE and replaces any that are broken or stale. + * + * Only instantiated when both binddn and bindpw are configured -- if no bind user is set the + * factory falls back to ephemeral per-request connections. + */ +private[authentication] class UnboundIdConnectionPool(conf: KyuubiConf) + extends Closeable with Logging { + + private val urls: Seq[LDAPURL] = + conf.get(KyuubiConf.AUTHENTICATION_LDAP_URL) + .map(LdapUrlParser.parseAll) + .getOrElse(Seq.empty) + + private val bindDn = conf.get(KyuubiConf.AUTHENTICATION_LDAP_BIND_USER).get + private val bindPassword = conf.get(KyuubiConf.AUTHENTICATION_LDAP_BIND_PASSWORD).get + + private val minConns = conf.get(KyuubiConf.AUTHENTICATION_LDAP_POOL_MIN_CONNECTIONS) + private val maxConns = conf.get(KyuubiConf.AUTHENTICATION_LDAP_POOL_MAX_CONNECTIONS) + private val healthCheckIntervalMs = + conf.get(KyuubiConf.AUTHENTICATION_LDAP_POOL_HEALTH_CHECK_INTERVAL) + private val checkoutTimeoutMs = + conf.get(KyuubiConf.AUTHENTICATION_LDAP_POOL_CHECKOUT_TIMEOUT) + + // autoReconnect=true: the pool detects closed sockets between requests and reconnects + // transparently, so a server bounce does not drain the pool to zero on the next checkout. + private val connectionOptions: LDAPConnectionOptions = + LdapServerSets.buildConnectionOptions(conf, autoReconnect = true) + + private val sslSocketFactory = SSLContext.getDefault.getSocketFactory + + val serverSet: ServerSet = { + if (urls.isEmpty) { + throw new IllegalStateException( + "kyuubi.authentication.ldap.url must be set when pool is enabled") + } + // FailoverServerSet tries servers in order, advancing only on connection-level failure. + // Ordered failover (rather than round-robin) is intentional: it prefers the first + // configured URL and only moves to the next when a connection cannot be established, + // which suits deployments where each URL fronts its own load-balanced LDAP endpoint. + LdapServerSets.buildServerSet(urls, connectionOptions, sslSocketFactory) + } + + // Primary server is the first URL in the ordered list. A checkout that lands on a + // different host means FailoverServerSet has advanced past a failed primary -- we + // surface that as a warn so operators know the primary is unhealthy without having to + // cross-reference pool defunct-connection counters in Prometheus. + private val primaryHost: String = urls.head.getHost + private val primaryPort: Int = urls.head.getPort + + // Tracks whether checkouts are currently landing on a secondary server, so the failover + // warning fires only on state transitions rather than once per checkout. Logging per + // checkout would flood the auth hot path during a sustained primary outage. + private val onFailover = new AtomicBoolean(false) + + private val pool: LDAPConnectionPool = buildPool() + + private def buildPool(): LDAPConnectionPool = { + val bindRequest = new SimpleBindRequest(bindDn, bindPassword) + val pool = new LDAPConnectionPool(serverSet, bindRequest, minConns, maxConns) + pool.setHealthCheckIntervalMillis(healthCheckIntervalMs) + // The SDK default for setMaxWaitTimeMillis is 0: skip the wait queue, attempt to grow + // the pool, and fail fast if not (typically CONNECT_ERROR when already at maxConnections, + // or the result code from the underlying connection attempt if creation was tried). + // A positive value smooths brief contention spikes so they do not surface as auth + // failures during normal bursts. + pool.setMaxWaitTimeMillis(checkoutTimeoutMs) + // Validate idle connections by fetching the root DSE (empty base DN, always readable). + // invokeOnCheckout=false avoids an extra round-trip on every borrow; the background + // thread and release validation keep the pool healthy without adding per-request latency. + pool.setHealthCheck(new LoggingHealthCheck(new GetEntryLDAPConnectionPoolHealthCheck( + "", // root DSE -- readable by the bind user on any LDAP server + 0L, // max response time: use connection-level response timeout + false, // invokeOnCreate: connection just authenticated, skip redundant check + true, // invokeAfterAuthentication: verify bind succeeded + false, // invokeOnCheckout: skip to avoid per-request latency + true, // invokeOnRelease: detect connections broken during use + true // invokeForBackgroundChecks: evict stale idle connections + ))) + val urlStr = urls + .map(u => s"${u.getScheme}://${u.getHost}:${u.getPort}") + .mkString(", ") + info( + s"LDAP connection pool initialised: [$urlStr]" + + s" min=$minConns max=$maxConns" + + s" healthCheck=${healthCheckIntervalMs}ms checkoutTimeout=${checkoutTimeoutMs}ms") + pool + } + + /** + * Returns a connection borrowed from the pool. Caller must call [[releaseConnection]] or + * [[releaseConnectionAfterException]] when done. + * + * Detects when a borrowed connection is on a server other than the primary (first + * configured URL), which means [[FailoverServerSet]] has advanced past the primary due to + * a connection-level failure. [[FailoverServerSet]] itself is silent about which server it + * chose, so this is the only signal of a live failover. + * + * Logging is edge-triggered to keep the auth hot path quiet during a sustained outage: + * a single WARN on the primary -> secondary transition, a single INFO on recovery, and the + * per-connection detail at DEBUG. A per-checkout WARN would otherwise emit one line per + * authentication request for the entire duration of an outage. + */ + def checkoutConnection(): LDAPConnection = { + val conn = pool.getConnection() + val connHost = conn.getConnectedAddress + val connPort = conn.getConnectedPort + val onSecondary = connHost != primaryHost || connPort != primaryPort + if (onSecondary) { + if (onFailover.compareAndSet(false, true)) { + warn( + s"LDAP pool failover active: connections now landing on secondary server" + + s" $connHost:$connPort (primary is $primaryHost:$primaryPort). The primary LDAP" + + s" server may be down. Per-checkout detail is logged at DEBUG until recovery.") + } else { + debug(s"LDAP pool checkout on secondary server $connHost:$connPort") + } + } else if (onFailover.compareAndSet(true, false)) { + info(s"LDAP pool failover recovered: connections back on primary $primaryHost:$primaryPort") + } + conn + } + + /** + * Returns a healthy connection to the pool. + * Call [[releaseConnectionAfterException]] instead if the connection experienced an + * [[LDAPException]] so the pool can decide whether to reuse or replace it. + */ + def releaseConnection(conn: LDAPConnection): Unit = pool.releaseConnection(conn) + + /** + * Releases a connection after an [[LDAPException]] was thrown during its use. + * + * Delegates to [[LDAPConnectionPool.releaseConnectionAfterException]], which is the + * SDK-recommended approach: it inspects the exception's result code and the pool's + * configured health check to decide automatically whether to return the connection for + * reuse or discard it. This is preferable to manually branching on a boolean `defunct` + * flag because the pool's health check has more context than our call site does. + */ + def releaseConnectionAfterException(conn: LDAPConnection, e: LDAPException): Unit = + pool.releaseConnectionAfterException(conn, e) + + /** + * Live gauge accessors for [[org.apache.kyuubi.metrics.MetricsSystem]]. Each call reads the + * current value from [[LDAPConnectionPoolStatistics]] held by the SDK pool -- no copying or + * caching, so the values reflect the pool state at the moment the metric is scraped. + * + * Cumulative counters (failed/successful checkouts, defunct/expired closures) are exposed + * as [[Long]] gauges; dashboards compute deltas from successive scrapes. + */ + def numAvailableConnections: Long = pool.getConnectionPoolStatistics.getNumAvailableConnections + def numFailedCheckouts: Long = pool.getConnectionPoolStatistics.getNumFailedCheckouts + def numSuccessfulCheckouts: Long = pool.getConnectionPoolStatistics.getNumSuccessfulCheckouts + def numConnectionsClosedDefunct: Long = + pool.getConnectionPoolStatistics.getNumConnectionsClosedDefunct + def numConnectionsClosedExpired: Long = + pool.getConnectionPoolStatistics.getNumConnectionsClosedExpired + + override def close(): Unit = { + pool.close() + info("LDAP connection pool closed") + } +} + +/** + * Decorates [[LDAPConnectionPoolHealthCheck]] to emit a [[Logging.warn]] whenever the + * delegate decides a connection must be discarded. + * + * Only the two discard paths are instrumented: + * - [[ensureConnectionValidForContinuedUse]]: called on release and by background checks. + * - [[ensureConnectionValidAfterException]]: called via [[releaseConnectionAfterException]]. + * + * [[ensureNewConnectionValid]] is delegated silently -- a healthy replacement connection + * is expected behaviour and not worth logging at warn. + */ +final private[ldap] class LoggingHealthCheck(delegate: LDAPConnectionPoolHealthCheck) + extends LDAPConnectionPoolHealthCheck with Logging { + + override def ensureConnectionValidForContinuedUse(conn: LDAPConnection): Unit = + try delegate.ensureConnectionValidForContinuedUse(conn) + catch { + case e: LDAPException => + warn( + s"LDAP pool discarding connection to " + + s"${conn.getConnectedAddress}:${conn.getConnectedPort}" + + s" (${e.getResultCode}): ${e.getMessage}") + throw e + } + + override def ensureConnectionValidAfterException( + conn: LDAPConnection, + e: LDAPException): Unit = + try delegate.ensureConnectionValidAfterException(conn, e) + catch { + case ex: LDAPException => + // Log both the original exception that triggered the health check (e -- the "why + // did the operation fail") and the health check's own rejection (ex -- the "why is + // the connection being discarded"). Previously only `e` was logged, which meant + // operators saw the triggering failure but not the health check's discard reason. + warn( + s"LDAP pool discarding connection to " + + s"${conn.getConnectedAddress}:${conn.getConnectedPort}" + + s" after exception (${e.getResultCode}): ${e.getMessage}" + + s"; health check rejected with (${ex.getResultCode}): ${ex.getMessage}") + throw ex + } + + override def ensureNewConnectionValid(conn: LDAPConnection): Unit = + delegate.ensureNewConnectionValid(conn) +} + +/** + * Singleton holder for the [[UnboundIdConnectionPool]]. + * + * Initialised on first use (double-checked locking) so the pool is created once and shared + * for the lifetime of the Kyuubi server process. [[close]] is called on server shutdown via + * [[org.apache.kyuubi.service.authentication.AuthenticationProviderFactory.close]]. + */ +private[authentication] object UnboundIdConnectionPool extends Logging { + + @volatile private var instance: Option[UnboundIdConnectionPool] = None + private val lock = new Object() + + /** + * Returns the live pool instance without forcing creation. Used by metric gauges that + * read pool statistics: gauges registered before the pool exists return their default + * value until the first authentication request triggers [[getOrCreate]]. + */ + def currentInstance: Option[UnboundIdConnectionPool] = instance + + def getOrCreate(conf: KyuubiConf): Option[UnboundIdConnectionPool] = { + if (instance.isEmpty) { + lock.synchronized { + if (instance.isEmpty) { + val hasBindUser = + conf.get(KyuubiConf.AUTHENTICATION_LDAP_BIND_USER).isDefined && + conf.get(KyuubiConf.AUTHENTICATION_LDAP_BIND_PASSWORD).isDefined + instance = + if (hasBindUser) Some(new UnboundIdConnectionPool(conf)) + else { + info( + "LDAP bind user not configured: pool disabled, ephemeral connections will be used") + None + } + } + } + } + instance + } + + def close(): Unit = lock.synchronized { + instance.foreach(_.close()) + instance = None + } +} diff --git a/kyuubi-common/src/main/scala/org/apache/kyuubi/service/authentication/ldap/UnboundIdDirSearch.scala b/kyuubi-common/src/main/scala/org/apache/kyuubi/service/authentication/ldap/UnboundIdDirSearch.scala new file mode 100644 index 00000000000..1067ed06da3 --- /dev/null +++ b/kyuubi-common/src/main/scala/org/apache/kyuubi/service/authentication/ldap/UnboundIdDirSearch.scala @@ -0,0 +1,314 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.kyuubi.service.authentication.ldap + +import javax.naming.NamingException + +import scala.collection.JavaConverters._ + +import com.unboundid.ldap.sdk._ + +import org.apache.kyuubi.Logging +import org.apache.kyuubi.config.KyuubiConf + +/** + * [[DirSearch]] implementation backed by an UnboundID [[LDAPConnection]]. + * + * Two modes are supported: + * - Pooled: the connection was borrowed from [[UnboundIdConnectionPool]]. On [[close]], + * [[LDAPConnectionPool.releaseConnectionAfterException]] is used when an exception was + * recorded during the request, letting the pool's health check decide whether the + * connection is still usable or should be discarded and replaced. + * - Ephemeral: the connection was created for a single credential-verification BIND. On + * [[close]], the connection is closed and discarded unconditionally. + * + * Unlike the JNDI-based [[LdapSearch]], search failures are propagated as + * [[NamingException]] rather than silently swallowed. This allows callers to detect + * mid-query connection drops and surface them as authentication failures instead of + * returning empty search results that would masquerade as "user not found". + * + * Partial results from [[LDAPSearchException]] (e.g. SIZE_LIMIT_EXCEEDED) are returned + * rather than thrown away, matching the intent of the sizeLimit setting in [[QueryFactory]]. + * Whether the connection is reused or discarded after such an exception is delegated to + * [[ResultCode.isConnectionUsable]], which returns true for server-side soft limits and + * false only for result codes that indicate a broken socket or session. + */ +private[ldap] class UnboundIdDirSearch( + conf: KyuubiConf, + initialConnection: LDAPConnection, + poolOpt: Option[UnboundIdConnectionPool]) + extends DirSearch with Logging { + + // The active connection used for searches. Mutable so that the mid-request retry path + // (see searchWithAttributes) can swap a broken pooled connection for a fresh one without + // bubbling the failure up to the caller. Only mutated on the pooled path; ephemeral + // searches do not retry because their connection is bound to specific end-user + // credentials that we cannot re-establish from inside this class. + private var connection: LDAPConnection = initialConnection + + private val baseDn = conf.get(KyuubiConf.AUTHENTICATION_LDAP_BASE_DN).orNull + private val groupBases: Array[String] = + LdapUtils.patternsToBaseDns( + LdapUtils.parseDnPatterns(conf, KyuubiConf.AUTHENTICATION_LDAP_GROUP_DN_PATTERN)) + private val userPatterns: Array[String] = + LdapUtils.parseDnPatterns(conf, KyuubiConf.AUTHENTICATION_LDAP_USER_DN_PATTERN) + private val userBases: Array[String] = LdapUtils.patternsToBaseDns(userPatterns) + private val queries: QueryFactory = new QueryFactory(conf) + + // Bounded number of swap-and-retry attempts on connection-level failures. One retry is + // enough to cover a single broken pooled connection (rotated server, dropped socket, + // idle reset by an intermediary) while keeping the request latency bounded and avoiding + // retry storms during a real outage. + // Tunable via kyuubi.authentication.ldap.pool.search.maxRetries. + private val maxSearchRetries: Int = + conf.get(KyuubiConf.AUTHENTICATION_LDAP_POOL_SEARCH_MAX_RETRIES) + + // The last LDAPException thrown during this request, if any. Stored so that + // close() can delegate the reuse-or-discard decision to the pool's health check via + // releaseConnectionAfterException rather than making that call at the exception site. + // DirSearch is used within a single request thread, so no volatile needed. + private var lastException: Option[LDAPException] = None + + // Set true when the retry path has already returned the active connection to the pool + // via releaseConnectionAfterException AND a replacement checkout failed -- so close() + // must not release the same connection a second time. Reset to false once a fresh + // connection is successfully checked out (the new connection is unreleased). + private var connectionAlreadyReleased: Boolean = false + + override def findUserDn(user: String): String = { + val allNames = + if (LdapUtils.isDn(user)) { + val userBaseDn = LdapUtils.extractBaseDn(user) + val userRdn = LdapUtils.extractFirstRdn(user) + search(Array(userBaseDn), queries.findUserDnByRdn(userRdn)) + } else { + val byPattern = findDnByPattern(userPatterns, user) + if (byPattern.nonEmpty) byPattern + else search(userBases, queries.findUserDnByName(user)) + } + + if (allNames.length == 1) allNames.head + else { + info( + s"Expected exactly one user result for $user but got ${allNames.length}. Returning null") + null + } + } + + override def findGroupDn(group: String): String = { + val results = search(groupBases, queries.findGroupDnById(group)) + if (results.length == 1) results.head + else throw new NamingException("Single result was expected") + } + + override def isUserMemberOfGroup(user: String, groupDn: String): Boolean = { + val userId = LdapUtils.extractUserName(user) + search(userBases, queries.isUserMemberOfGroup(userId, groupDn)).nonEmpty + } + + override def findGroupsForUser(userDn: String): Array[String] = { + val userName = LdapUtils.extractUserName(userDn) + search(groupBases, queries.findGroupsForUser(userName, userDn)) + } + + override def executeCustomQuery(query: String): Array[String] = { + val results = searchWithAttributes(Array(baseDn), queries.customQuery(query)) + results.flatMap { entry => + Seq(entry.getDN) ++ entry.getAttributes.asScala.flatMap(_.getValues.toSeq) + } + } + + private def findDnByPattern(patterns: Seq[String], name: String): Array[String] = { + for (pattern <- patterns) { + val patternBaseDn = LdapUtils.extractBaseDn(pattern) + // Use replace (literal) not replaceAll (regex): usernames containing '$' or '\' + // are treated as regex back-references / escape sequences in the replacement string + // by String.replaceAll, causing unexpected rewriting or IndexOutOfBoundsException. + val rdn = LdapUtils.extractFirstRdn(pattern).replace("%s", name) + val names = search(Array(patternBaseDn), queries.findDnByPattern(rdn)) + if (names.nonEmpty) return names + } + Array.empty + } + + private def search(baseDns: Array[String], query: Query): Array[String] = + searchWithAttributes(baseDns, query).map(_.getDN) + + private def searchWithAttributes( + baseDns: Array[String], + query: Query): Array[SearchResultEntry] = { + // query.filter is already a parsed Filter object -- no redundant re-parse inside the SDK. + // query.attributes / query.sizeLimit replace the JNDI SearchControls bridge. + // + // Every query except customQuery reads only entry.getDN (see `search`), so request no + // attributes via the RFC 4511 "1.1" OID (SearchRequest.NO_ATTRIBUTES) to avoid pulling + // every attribute over the wire for DN-only lookups. customQuery is the only caller that + // populates query.attributes (the group-membership attribute) and is honored as-is. + // This matches both the pre-UnboundID default (empty returningAttributes) and the JNDI + // path in LdapSearch. + val requestedAttrs: Array[String] = + if (query.attributes.isEmpty) Array(SearchRequest.NO_ATTRIBUTES) else query.attributes.toArray + + debug(s"Executing LDAP query '${query.filterString}' on bases [${baseDns.mkString(", ")}]") + + baseDns.toSeq.flatMap { base => + if (base == null) Seq.empty[SearchResultEntry] + else searchBaseWithRetry(base, query, requestedAttrs, maxSearchRetries) + }.toArray + } + + /** + * Executes one base-DN search with bounded retry on connection-level failures. + * + * Retry contract: + * - Only the pooled path retries. On retry, the broken connection is released to the + * pool with [[LDAPConnectionPool.releaseConnectionAfterException]] (which discards it + * via the configured health check), and a fresh one is checked out. If the pool's + * [[com.unboundid.ldap.sdk.FailoverServerSet]] is configured with multiple URLs, the + * fresh connection will land on a healthy server, closing the gap where JNDI fails + * over only at initial context creation. + * - The ephemeral path does not retry: its connection was bound with end-user + * credentials that this class does not retain. + * - Retry is restricted to result codes where [[ResultCode.isConnectionUsable]] returns + * false (SERVER_DOWN, CONNECT_ERROR, LOCAL_ERROR, etc.). Protocol-level errors that + * leave the connection usable (e.g. INVALID_DN_SYNTAX) are propagated without retry -- + * retrying them on a different connection would not change the outcome. + * - Soft-limit codes (SIZE_LIMIT_EXCEEDED etc.) are not retried because partial results + * are intentionally returned per the [[QueryFactory]] sizeLimit contract. + */ + private def searchBaseWithRetry( + base: String, + query: Query, + requestedAttrs: Array[String], + attemptsRemaining: Int): Seq[SearchResultEntry] = { + try { + val request = + new SearchRequest(base, SearchScope.SUB, query.filter, requestedAttrs: _*) + if (query.sizeLimit > 0) request.setSizeLimit(query.sizeLimit) + connection.search(request).getSearchEntries.asScala.toSeq + } catch { + case e: LDAPSearchException + if e.getResultCode == ResultCode.NO_SUCH_OBJECT || + e.getResultCode == ResultCode.NO_SUCH_ATTRIBUTE => + // Base DN does not exist or attribute absent -- not an error, move to next base. + Seq.empty[SearchResultEntry] + + case e: LDAPSearchException if e.getResultCode.isConnectionUsable => + // Server responded but truncated results (e.g. SIZE_LIMIT_EXCEEDED, TIME_LIMIT_EXCEEDED, + // ADMIN_LIMIT_EXCEEDED). Return whatever entries arrived before the limit; the + // connection is still alive for soft-limit codes -- isConnectionUsable separates the + // "server told us to stop" outcomes (handled here) from the "connection died" ones + // (which fall through to the retry/propagate cases below). + // + // Auth-safety note: returning a truncated result set cannot create false positives. + // LDAP only includes entries that already matched the search filter, so truncation + // can shrink the matching set but never invent matches. The worst behaviour change + // is that an ambiguous result (multiple matches expected) appears unique -- callers + // that enforce uniqueness (e.g. findUserDn requires length == 1) handle that case + // explicitly. Configure sizeLimit >= 2 in QueryFactory so duplicates remain + // observable when truncation occurs. + debug(s"LDAP search truncated (${e.getResultCode}) on base '$base': ${e.getMessage}") + Option(e.getSearchEntries).fold(Seq.empty[SearchResultEntry])(_.asScala.toSeq) + + case e: LDAPException + if attemptsRemaining > 0 && + !e.getResultCode.isConnectionUsable && + poolOpt.isDefined => + // Connection-level failure on the pooled path. Discard the broken connection, + // check out a fresh one, and retry. This is what closes the JNDI failover gap: + // a server failing mid-search (not just at initial connect) is recovered + // transparently to the caller as long as a healthy peer is available. + val pool = poolOpt.get + warn( + s"LDAP search failed mid-request on ${connection.getConnectedAddress}:" + + s"${connection.getConnectedPort} (${e.getResultCode}): ${e.getMessage}. " + + s"Retrying with a fresh connection (attempts left: ${attemptsRemaining - 1}).") + // Mark BEFORE releasing. Either ordering has an edge case: + // - Set before: if release throws *before* the pool accepts the connection, + // close() will skip releasing and the connection may leak. + // - Set after: if release throws *after* the pool accepts the connection, + // close() will release it a second time and corrupt pool accounting. + // Picking "before" because LDAPConnectionPool.releaseConnectionAfterException + // delegates to releaseDefunctConnection / releaseConnection internally and both + // are no-throw on a non-null conn against a live pool (the two preconditions + // this code already satisfies by construction). The double-release case, by + // contrast, is genuinely reachable when every peer in the FailoverServerSet is + // down on the replacement checkout. Revisit if a future SDK upgrade widens the + // throw contract of releaseConnectionAfterException. + connectionAlreadyReleased = true + pool.releaseConnectionAfterException(connection, e) + try { + connection = pool.checkoutConnection() + // Replacement is healthy and now owned by this DirSearch -- subsequent close() + // must release this new connection normally. + connectionAlreadyReleased = false + } catch { + case ce: LDAPException => + // Could not get a replacement (pool exhausted, all servers down). Leave + // connectionAlreadyReleased=true so close() does NOT release the previous + // (already-released) connection a second time. Surface the ORIGINAL search + // failure for diagnostic clarity; the checkout error chains under it. + lastException = Some(e) + val ne = toNamingException(e) + ne.addSuppressed(ce) + throw ne + } + searchBaseWithRetry(base, query, requestedAttrs, attemptsRemaining - 1) + + case e: LDAPException => + // Either non-recoverable, no retries left, or ephemeral path. Record and propagate. + lastException = Some(e) + throw toNamingException(e) + } + } + + /** + * Returns the connection to the pool or closes it, depending on whether an exception + * was recorded during the request. + * + * When an exception was recorded, [[UnboundIdConnectionPool.releaseConnectionAfterException]] + * is called instead of manually choosing between [[releaseConnection]] and + * [[releaseDefunctConnection]]. This delegates the reuse-or-discard decision to + * [[LDAPConnectionPool.releaseConnectionAfterException]], which consults the pool's + * configured health check -- the same check already in use for background validation. + */ + override def close(): Unit = poolOpt match { + case Some(pool) => + if (connectionAlreadyReleased) { + // Retry path already returned this connection to the pool (with the failure + // attached) and could not obtain a replacement. Releasing again would either + // double-return a discarded connection or hand the pool a re-released + // healthy one -- both corrupt the pool's accounting. Nothing to do. + debug("LDAP DirSearch close(): connection already released during retry path") + } else { + lastException match { + case Some(e) => pool.releaseConnectionAfterException(connection, e) + case None => pool.releaseConnection(connection) + } + } + case None => + connection.close() + } + + private def toNamingException(e: LDAPException): NamingException = { + val ne = new NamingException( + s"LDAP operation failed (${e.getResultCode}): ${e.getMessage}") + ne.initCause(e) + ne + } +} diff --git a/kyuubi-common/src/main/scala/org/apache/kyuubi/service/authentication/ldap/UnboundIdDirSearchFactory.scala b/kyuubi-common/src/main/scala/org/apache/kyuubi/service/authentication/ldap/UnboundIdDirSearchFactory.scala new file mode 100644 index 00000000000..39fdf80d49a --- /dev/null +++ b/kyuubi-common/src/main/scala/org/apache/kyuubi/service/authentication/ldap/UnboundIdDirSearchFactory.scala @@ -0,0 +1,139 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.kyuubi.service.authentication.ldap + +import javax.naming.NamingException +import javax.net.ssl.SSLContext +import javax.security.sasl.AuthenticationException + +import com.unboundid.ldap.sdk._ + +import org.apache.kyuubi.Logging +import org.apache.kyuubi.config.KyuubiConf + +/** + * [[DirSearchFactory]] backed by UnboundID SDK. + * + * Two connection strategies are applied depending on who is being authenticated: + * + * - Bind user (service account): borrows a pre-authenticated connection from the + * [[UnboundIdConnectionPool]]. The pool holds warm, health-checked connections so the + * borrow is effectively free -- no TCP handshake, no TLS negotiation, no BIND round-trip. + * On [[DirSearch.close]] the connection is returned (healthy) or discarded (defunct). + * + * - End user (credential verification): obtains a short-lived [[LDAPConnection]] from the + * [[ServerSet]] and issues a BIND request with the user's DN and password. + * [[FailoverServerSet.getConnection]] tries each configured server in order, skipping to + * the next only on connection-level failures (DNS, refused, timeout). BIND failure with + * INVALID_CREDENTIALS (LDAP error 49) is thrown immediately without failover -- bad + * credentials should fail fast and retrying other servers would risk triggering AD lockout. + * + * When no bind user is configured (unusual in AD environments but supported), every call + * uses the end-user path -- the caller's own credentials are used to search LDAP. + */ +private[authentication] class UnboundIdDirSearchFactory( + conf: KyuubiConf, + poolOpt: Option[UnboundIdConnectionPool]) + extends DirSearchFactory with Logging { + + private val bindDn: Option[String] = conf.get(KyuubiConf.AUTHENTICATION_LDAP_BIND_USER) + + // Reuse the pool's ServerSet when available -- ephemeral connections share the same + // server topology and SSL configuration without duplicating the setup. When no pool is + // configured, build a dedicated ServerSet from the same URL list (but without + // autoReconnect, which is pool-specific and inappropriate for short-lived connections). + private val serverSet: ServerSet = + poolOpt.map(_.serverSet).getOrElse(buildEphemeralServerSet()) + + override def getInstance(conf: KyuubiConf, principal: String, password: String): DirSearch = { + val isBindUser = bindDn.exists(_ == principal) && poolOpt.isDefined + if (isBindUser) { + borrowFromPool(poolOpt.get) + } else { + ephemeralBind(principal, password) + } + } + + private def borrowFromPool(pool: UnboundIdConnectionPool): DirSearch = { + try { + val conn = pool.checkoutConnection() + new UnboundIdDirSearch(conf, conn, poolOpt) + } catch { + case e: LDAPException => + throw new AuthenticationException( + s"Failed to borrow connection from LDAP pool: ${e.getMessage}", + e) + } + } + + /** + * Obtains a short-lived connection from the [[ServerSet]] and issues a BIND. + * + * [[FailoverServerSet.getConnection]] handles server-level failover natively: it tries + * each server in order and only advances to the next on a connection-level failure. + * INVALID_CREDENTIALS is raised during the BIND step -- after a connection is already + * established -- so it is never retried across servers, preventing AD lockout events. + */ + private def ephemeralBind(principal: String, password: String): DirSearch = { + val conn = + try { + serverSet.getConnection() + } catch { + case e: LDAPException => + val ne = new NamingException(s"LDAP connect failed: ${e.getMessage}") + ne.initCause(e) + throw ne + } + try { + // bind() throws LDAPException for any non-SUCCESS result; it never returns a + // non-SUCCESS BindResult, so no extra check on the result code is needed. + conn.bind(new SimpleBindRequest(principal, password)) + new UnboundIdDirSearch(conf, conn, poolOpt = None) + } catch { + case e: LDAPException if e.getResultCode == ResultCode.INVALID_CREDENTIALS => + conn.close() + throw new AuthenticationException( + s"Error validating LDAP user: $principal (InvalidCredentials: ${e.getMessage})", + e) + case e: LDAPException => + conn.close() + val ne = new NamingException(s"LDAP bind failed: ${e.getMessage}") + ne.initCause(e) + throw ne + case t: Throwable => + conn.close() + throw t + } + } + + private def buildEphemeralServerSet(): ServerSet = { + val urls = conf.get(KyuubiConf.AUTHENTICATION_LDAP_URL) + .map(LdapUrlParser.parseAll) + .getOrElse(Seq.empty) + + if (urls.isEmpty) { + throw new IllegalStateException( + "kyuubi.authentication.ldap.url must be set for LDAP authentication") + } + + // autoReconnect=false: ephemeral connections are used for one bind then discarded; + // reconnect logic is wasted work and would mask transient failures we want to surface. + val opts = LdapServerSets.buildConnectionOptions(conf, autoReconnect = false) + LdapServerSets.buildServerSet(urls, opts, SSLContext.getDefault.getSocketFactory) + } +} diff --git a/kyuubi-common/src/test/scala/org/apache/kyuubi/service/authentication/AuthenticationProviderFactorySuite.scala b/kyuubi-common/src/test/scala/org/apache/kyuubi/service/authentication/AuthenticationProviderFactorySuite.scala index 0bd29ac56a9..84977284df3 100644 --- a/kyuubi-common/src/test/scala/org/apache/kyuubi/service/authentication/AuthenticationProviderFactorySuite.scala +++ b/kyuubi-common/src/test/scala/org/apache/kyuubi/service/authentication/AuthenticationProviderFactorySuite.scala @@ -17,6 +17,7 @@ package org.apache.kyuubi.service.authentication +import java.net.ServerSocket import javax.security.sasl.AuthenticationException import org.apache.kyuubi.{KyuubiFunSuite, Utils} @@ -26,16 +27,66 @@ class AuthenticationProviderFactorySuite extends KyuubiFunSuite { import AuthenticationProviderFactory._ + // Reset the singleton ldapProviders map and the underlying UnboundID pool between tests + // so any LDAP provider built in one test cannot be observed by the next (the singleton + // is keyed by auth scope, not by KyuubiConf, so different conf in successive tests would + // otherwise resolve to the cached provider from the first run). + override def afterEach(): Unit = { + try AuthenticationProviderFactory.close() + finally super.afterEach() + } + + /** + * Allocates a TCP port, immediately closes the listener, and returns the port number. + * Subsequent connections to `127.0.0.1:port` are deterministically refused for the + * lifetime of the test -- no DNS, no timing dependence, no hostname-resolution variance. + */ + private def refusedLocalPort(): Int = { + val s = new ServerSocket(0) + try s.getLocalPort + finally s.close() + } + test("get auth provider") { val conf = KyuubiConf() + // UnboundIdDirSearchFactory checks at construction that kyuubi.authentication.ldap.url + // is non-empty and parseable as an LDAPURL -- it does NOT open a TCP connection. Using + // a freshly-allocated-then-closed loopback port gives us a guaranteed-refused TCP + // target so the failure path is deterministic. + .set(KyuubiConf.AUTHENTICATION_LDAP_URL, s"ldap://127.0.0.1:${refusedLocalPort()}") val p1 = getAuthenticationProvider(AuthMethods.withName("NONE"), conf) p1.authenticate(Utils.currentUser, "") val p2 = getAuthenticationProvider(AuthMethods.withName("LDAP"), conf) val e1 = intercept[AuthenticationException](p2.authenticate("test", "test")) - assert(e1.getMessage.contains("Error validating LDAP user:")) + // With the pool disabled (the default), the factory builds the legacy JNDI-backed + // provider. A refused connection surfaces from LdapSearchFactory.getInstance, which + // wraps the NamingException as an AuthenticationException with this prefix. + assert( + e1.getMessage.contains("Error validating LDAP user:"), + s"unexpected AuthenticationException message: ${e1.getMessage}") val e2 = intercept[AuthenticationException]( AuthenticationProviderFactory.getAuthenticationProvider(null, conf)) assert(e2.getMessage === "Not a valid authentication method") } + test("close() releases LDAP singletons so subsequent calls rebuild fresh providers") { + val conf = KyuubiConf() + // Construction-time URL check parses the value as an LDAPURL; it does not connect. + // A refused loopback port works fine -- this test exercises provider caching and + // close(), never calling authenticate(). + .set(KyuubiConf.AUTHENTICATION_LDAP_URL, s"ldap://127.0.0.1:${refusedLocalPort()}") + + // Force the LDAP singleton to be created and cached under the "server" key. + val p1 = getAuthenticationProvider(AuthMethods.withName("LDAP"), conf) + val p2 = getAuthenticationProvider(AuthMethods.withName("LDAP"), conf) + assert(p1 eq p2, "LDAP provider must be cached as a singleton between calls") + + // close() must drop the cached provider and any owned LDAP pool. The next call + // returns a fresh instance built from the (potentially updated) conf. + AuthenticationProviderFactory.close() + val p3 = getAuthenticationProvider(AuthMethods.withName("LDAP"), conf) + assert(p3 ne p1, "close() should evict the cached LDAP provider") + // afterEach() runs close() again as belt-and-braces for the next test. + } + } diff --git a/kyuubi-common/src/test/scala/org/apache/kyuubi/service/authentication/ldap/CapturingLdapAuthRecorder.scala b/kyuubi-common/src/test/scala/org/apache/kyuubi/service/authentication/ldap/CapturingLdapAuthRecorder.scala new file mode 100644 index 00000000000..75ff063dd63 --- /dev/null +++ b/kyuubi-common/src/test/scala/org/apache/kyuubi/service/authentication/ldap/CapturingLdapAuthRecorder.scala @@ -0,0 +1,51 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.kyuubi.service.authentication.ldap + +import java.util.concurrent.atomic.AtomicInteger + +import org.apache.kyuubi.service.authentication.LdapAuthMetricsRecorder + +/** + * Test-only [[LdapAuthMetricsRecorder]] that tracks every call. Shared across suites that + * want to assert which recorder methods were invoked (e.g. classifier-dispatch tests that + * verify a bad-password authenticate lands as `invalid_credentials`). + * + * Lives in the `ldap` test package so any test suite in this package can use it without + * each suite redefining the same helper. + */ +final private[ldap] class CapturingLdapAuthRecorder extends LdapAuthMetricsRecorder { + val successes = new AtomicInteger(0) + val cacheHits = new AtomicInteger(0) + val cacheMisses = new AtomicInteger(0) + val invalidCreds = new AtomicInteger(0) + val invalidInput = new AtomicInteger(0) + val infrastructure = new AtomicInteger(0) + + override def recordSuccess(): Unit = { successes.incrementAndGet(); () } + override def recordCacheHit(): Unit = { cacheHits.incrementAndGet(); () } + override def recordCacheMiss(): Unit = { cacheMisses.incrementAndGet(); () } + override def recordFailure(reason: String): Unit = { + reason match { + case LdapAuthFailureClassifier.INVALID_CREDENTIALS => invalidCreds.incrementAndGet() + case LdapAuthFailureClassifier.INVALID_INPUT => invalidInput.incrementAndGet() + case _ => infrastructure.incrementAndGet() + } + () + } +} diff --git a/kyuubi-common/src/test/scala/org/apache/kyuubi/service/authentication/ldap/LdapAuthCacheSuite.scala b/kyuubi-common/src/test/scala/org/apache/kyuubi/service/authentication/ldap/LdapAuthCacheSuite.scala new file mode 100644 index 00000000000..58baf817742 --- /dev/null +++ b/kyuubi-common/src/test/scala/org/apache/kyuubi/service/authentication/ldap/LdapAuthCacheSuite.scala @@ -0,0 +1,436 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.kyuubi.service.authentication.ldap + +import java.util.concurrent.{Callable, CountDownLatch, CyclicBarrier, Executors, TimeUnit} +import java.util.concurrent.atomic.AtomicInteger +import javax.security.sasl.AuthenticationException + +import org.apache.kyuubi.KyuubiFunSuite +import org.apache.kyuubi.config.KyuubiConf +import org.apache.kyuubi.service.authentication.{AuthenticationProviderFactory, PasswdAuthenticationProvider} + +class LdapAuthCacheSuite extends KyuubiFunSuite { + + // Counter state lives on the AuthenticationProviderFactory singleton; reset between + // tests so cache.hit / cache.miss / success assertions are independent of execution + // order. The singleton only resets through close(), which is safe to call here because + // no LDAP pool has been initialised in this suite. + override def afterEach(): Unit = { + try AuthenticationProviderFactory.close() + finally super.afterEach() + } + + private def makeDelegate(onAuthenticate: (String, String) => Unit): PasswdAuthenticationProvider = + (user: String, password: String) => onAuthenticate(user, password) + + test("cacheKey is deterministic for same inputs") { + val k1 = LdapAuthCache.cacheKey("alice", "secret") + val k2 = LdapAuthCache.cacheKey("alice", "secret") + assert(k1 === k2) + } + + test("cacheKey differs for different passwords") { + val k1 = LdapAuthCache.cacheKey("alice", "secret1") + val k2 = LdapAuthCache.cacheKey("alice", "secret2") + assert(k1 !== k2) + } + + test("cacheKey differs for different users") { + val k1 = LdapAuthCache.cacheKey("alice", "secret") + val k2 = LdapAuthCache.cacheKey("bob", "secret") + assert(k1 !== k2) + } + + test("successful authentication is cached and skips delegate on repeat") { + val callCount = new AtomicInteger(0) + val delegate = makeDelegate((_, _) => { callCount.incrementAndGet(); () }) + val conf = new KyuubiConf() + val provider = new CachingLdapAuthenticationProvider(delegate, conf) + + provider.authenticate("alice", "secret") + provider.authenticate("alice", "secret") + provider.authenticate("alice", "secret") + + assert(callCount.get() === 1) + assert(provider.cacheSize() === 1) + } + + test("failed authentication is not cached") { + val callCount = new AtomicInteger(0) + val delegate = makeDelegate { (_, _) => + callCount.incrementAndGet() + throw new AuthenticationException("bad credentials") + } + val conf = new KyuubiConf() + val provider = new CachingLdapAuthenticationProvider(delegate, conf) + + intercept[AuthenticationException](provider.authenticate("alice", "wrong")) + intercept[AuthenticationException](provider.authenticate("alice", "wrong")) + + assert(callCount.get() === 2) + assert(provider.cacheSize() === 0) + } + + test("different users are cached independently") { + val callCount = new AtomicInteger(0) + val delegate = makeDelegate((_, _) => { callCount.incrementAndGet(); () }) + val conf = new KyuubiConf() + val provider = new CachingLdapAuthenticationProvider(delegate, conf) + + provider.authenticate("alice", "pw1") + provider.authenticate("bob", "pw2") + provider.authenticate("alice", "pw1") + provider.authenticate("bob", "pw2") + + assert(callCount.get() === 2) + assert(provider.cacheSize() === 2) + } + + test("password change after cache hit still allows re-auth after invalidation") { + var storedPassword = "initial" + val delegate = makeDelegate { (_, password) => + if (password != storedPassword) { + throw new AuthenticationException("wrong password") + } + } + val conf = new KyuubiConf() + val provider = new CachingLdapAuthenticationProvider(delegate, conf) + + provider.authenticate("alice", "initial") + // Cache hit -- old password still works from cache + provider.authenticate("alice", "initial") + + // After invalidation, new password must be used + provider.invalidate("alice", "initial") + storedPassword = "newpassword" + + intercept[AuthenticationException](provider.authenticate("alice", "initial")) + provider.authenticate("alice", "newpassword") + } + + test("concurrent identical authentications collapse to a single delegate call") { + // Setup: a delegate that blocks inside authenticate until released, so we can be sure + // multiple threads are simultaneously waiting on the same cache key. Without Guava's + // per-key coalescing in cache.get(key, Callable) all threads would each invoke the + // delegate; with coalescing only one does, and the rest receive the cached result. + val callCount = new AtomicInteger(0) + val release = new CountDownLatch(1) + val firstCallEntered = new CountDownLatch(1) + val delegate = makeDelegate { (_, _) => + callCount.incrementAndGet() + firstCallEntered.countDown() + // Hold this delegate invocation open so other threads pile up at cache.get. + assert(release.await(5, TimeUnit.SECONDS), "test gate never released") + } + val provider = new CachingLdapAuthenticationProvider(delegate, new KyuubiConf()) + + val threads = 8 + val pool = Executors.newFixedThreadPool(threads) + try { + // submit() returns immediately; invokeAll() would block here forever because the + // first task is parked inside the delegate awaiting `release`. + val futures = (1 to threads).map { _ => + pool.submit(new Callable[Unit] { + override def call(): Unit = provider.authenticate("alice", "secret") + }) + } + + // Wait until at least one thread is inside the delegate, then let it finish. + assert( + firstCallEntered.await(5, TimeUnit.SECONDS), + "no thread reached the delegate") + release.countDown() + + // All futures must complete without timing out. + futures.foreach(_.get(5, TimeUnit.SECONDS)) + } finally { + pool.shutdownNow() + } + + // Coalescing means exactly one LDAP round-trip for the burst. + assert(callCount.get() === 1) + assert(provider.cacheSize() === 1) + } + + test("max size config is respected") { + val conf = new KyuubiConf() + conf.set(KyuubiConf.AUTHENTICATION_LDAP_CACHE_MAX_SIZE, 2) + val delegate = makeDelegate((_, _) => ()) + val provider = new CachingLdapAuthenticationProvider(delegate, conf) + + provider.authenticate("user1", "pw") + provider.authenticate("user2", "pw") + provider.authenticate("user3", "pw") + + // Cache size should not exceed max + assert(provider.cacheSize() <= 2) + } + + test("cache hit increments cache.hit and success counters") { + val delegate = makeDelegate((_, _) => ()) + val provider = new CachingLdapAuthenticationProvider(delegate, new KyuubiConf()) + + // First call -> miss (delegate invoked, success recorded by delegate is bypassed + // because the delegate here is a stub, not the impl). The wrapper records the miss. + provider.authenticate("alice", "secret") + assert(AuthenticationProviderFactory.ldapAuthCacheMiss === 1L) + assert(AuthenticationProviderFactory.ldapAuthCacheHit === 0L) + assert(AuthenticationProviderFactory.ldapAuthSuccess === 0L) + + // Second + third calls -> cache hits, both bump cache.hit and success. + provider.authenticate("alice", "secret") + provider.authenticate("alice", "secret") + assert(AuthenticationProviderFactory.ldapAuthCacheMiss === 1L) + assert(AuthenticationProviderFactory.ldapAuthCacheHit === 2L) + assert(AuthenticationProviderFactory.ldapAuthSuccess === 2L) + } + + test("cache miss increments cache.miss but not success on its own") { + // Stub delegate that succeeds; the wrapper records cache.miss only. The delegate + // here does NOT record success (production delegate, LdapAuthenticationProviderImpl, + // does that itself; this assertion verifies the wrapper does not double-count). + val delegate = makeDelegate((_, _) => ()) + val provider = new CachingLdapAuthenticationProvider(delegate, new KyuubiConf()) + + provider.authenticate("alice", "secret") + assert(AuthenticationProviderFactory.ldapAuthCacheMiss === 1L) + assert(AuthenticationProviderFactory.ldapAuthCacheHit === 0L) + assert( + AuthenticationProviderFactory.ldapAuthSuccess === 0L, + "wrapper must not record success on cache miss; delegate is responsible") + } + + test("metrics recorder receives the same events the in-process counters do") { + // Install a recorder that captures every call; verify it sees exactly the same set + // of events the in-process AtomicLong counters observe. KyuubiServer installs an + // equivalent recorder in production that delegates to MetricsSystem.incCount, which + // surfaces as Prometheus type `counter`. + val recorder = new CapturingLdapAuthRecorder + AuthenticationProviderFactory.setLdapAuthMetricsRecorder(recorder) + + val delegate = makeDelegate((_, _) => ()) + val provider = new CachingLdapAuthenticationProvider(delegate, new KyuubiConf()) + + provider.authenticate("alice", "secret") // miss + provider.authenticate("alice", "secret") // hit (cache.hit + success) + provider.authenticate("alice", "secret") // hit (cache.hit + success) + + // Recorder calls match the in-process counters exactly. + assert(recorder.cacheMisses.get === 1) + assert(recorder.cacheHits.get === 2) + assert(recorder.successes.get === 2) + assert(AuthenticationProviderFactory.ldapAuthCacheMiss === 1L) + assert(AuthenticationProviderFactory.ldapAuthCacheHit === 2L) + assert(AuthenticationProviderFactory.ldapAuthSuccess === 2L) + } + + test("concurrent identical authentications: success counter matches request count" + + " under coalescing") { + // Pins the per-request invariant: when N concurrent identical requests are coalesced + // by Guava (one delegate call, N-1 waiters), the success counter must report N, not 1. + // The stub delegate mimics LdapAuthenticationProviderImpl's contract of recording its + // own success when its callable actually runs; the wrapper records success for the + // waiters via its `wasLeader` logic. + val callCount = new AtomicInteger(0) + val release = new CountDownLatch(1) + val firstCallEntered = new CountDownLatch(1) + val delegate = makeDelegate { (_, _) => + callCount.incrementAndGet() + firstCallEntered.countDown() + assert(release.await(5, TimeUnit.SECONDS), "test gate never released") + // Match the impl's recording behaviour: success on actual delegate execution. + AuthenticationProviderFactory.recordLdapAuthSuccess() + } + val provider = new CachingLdapAuthenticationProvider(delegate, new KyuubiConf()) + + val threads = 8 + val pool = Executors.newFixedThreadPool(threads) + // Pre-warm so all 8 worker threads exist before the real tasks queue; otherwise the + // last-arriving worker can be late enough to find the cache already populated and + // observe a hit instead of becoming a coalescing waiter. + val warmup = (1 to threads).map { _ => + pool.submit(new Callable[Unit] { override def call(): Unit = () }) + } + warmup.foreach(_.get(5, TimeUnit.SECONDS)) + + // CyclicBarrier releases all task threads into authenticate at the same instant so + // they race into cache.get together rather than serially. + val barrier = new CyclicBarrier(threads) + try { + val futures = (1 to threads).map { _ => + pool.submit(new Callable[Unit] { + override def call(): Unit = { + barrier.await(5, TimeUnit.SECONDS) + provider.authenticate("alice", "secret") + } + }) + } + assert( + firstCallEntered.await(5, TimeUnit.SECONDS), + "no thread reached the delegate") + // Brief pause so the other 7 threads have time to enter cache.get and block as + // waiters before the leader's callable releases. Without this, the leader can + // complete the callable before slow-arriving waiters reach cache.get, leaving + // them to observe a populated cache and record a hit instead of a miss. + Thread.sleep(200) + release.countDown() + futures.foreach(_.get(5, TimeUnit.SECONDS)) + } finally { + pool.shutdownNow() + } + + assert(callCount.get() === 1, "coalescing must collapse to exactly one delegate call") + assert( + AuthenticationProviderFactory.ldapAuthCacheMiss === 8L, + "every concurrent request observed an empty cache when it arrived") + assert(AuthenticationProviderFactory.ldapAuthCacheHit === 0L) + assert( + AuthenticationProviderFactory.ldapAuthSuccess === 8L, + s"every request must contribute one success increment; got " + + s"${AuthenticationProviderFactory.ldapAuthSuccess} for 8 requests with 1 delegate call") + } + + test("concurrent identical authentications: failure counter matches request count" + + " under coalescing") { + // Symmetric to the success test. When the leader's delegate throws, Guava propagates + // the exception to every waiter via the same ExecutionException. The impl recorded + // the leader's failure once; waiters classify and record their own failure on the + // wrapper side, so the failure bucket totals equal the request count. + // + // Note: Guava does NOT cache failures, so a thread that arrives after the leader's + // callable has failed will trigger a fresh delegate invocation. The pool warm-up + + // CyclicBarrier + post-firstCallEntered sleep ensures all 8 waiters are inside + // cache.get before the leader's failure releases, so all 8 are coalesced under the + // leader's single callable invocation. + val callCount = new AtomicInteger(0) + val release = new CountDownLatch(1) + val firstCallEntered = new CountDownLatch(1) + val delegate = makeDelegate { (_, _) => + callCount.incrementAndGet() + firstCallEntered.countDown() + assert(release.await(5, TimeUnit.SECONDS), "test gate never released") + // Match the impl's recording behaviour: failure with classification on delegate + // execution. The wrapper records its own failure for waiters via its own classifier. + AuthenticationProviderFactory.recordLdapAuthFailure( + LdapAuthFailureClassifier.INVALID_CREDENTIALS) + throw new AuthenticationException("simulated invalid credentials") + } + val provider = new CachingLdapAuthenticationProvider(delegate, new KyuubiConf()) + + val threads = 8 + val pool = Executors.newFixedThreadPool(threads) + val warmup = (1 to threads).map { _ => + pool.submit(new Callable[Unit] { override def call(): Unit = () }) + } + warmup.foreach(_.get(5, TimeUnit.SECONDS)) + + val barrier = new CyclicBarrier(threads) + try { + val futures = (1 to threads).map { _ => + pool.submit(new Callable[Unit] { + override def call(): Unit = { + barrier.await(5, TimeUnit.SECONDS) + provider.authenticate("alice", "wrong") + } + }) + } + assert( + firstCallEntered.await(5, TimeUnit.SECONDS), + "no thread reached the delegate") + Thread.sleep(200) + release.countDown() + // Each future fails: Future.get wraps the AuthenticationException in + // java.util.concurrent.ExecutionException, regardless of whether this thread was + // leader or waiter. + futures.foreach { f => + val ex = intercept[java.util.concurrent.ExecutionException](f.get(5, TimeUnit.SECONDS)) + assert(ex.getCause.isInstanceOf[AuthenticationException]) + } + } finally { + pool.shutdownNow() + } + + assert(callCount.get() === 1, "coalescing must collapse to exactly one delegate call") + assert(AuthenticationProviderFactory.ldapAuthCacheMiss === 8L) + assert(AuthenticationProviderFactory.ldapAuthCacheHit === 0L) + assert(AuthenticationProviderFactory.ldapAuthSuccess === 0L) + + // Total failures across all classification buckets must be 8 -- one per request. + // Bucket attribution is implementation-defined: the leader's stub recorded one + // invalid_credentials directly, and the waiter-side classifier maps the unwrapped + // AuthenticationException (no cause chain attached) to whatever its message-based + // fallback dictates. We pin the total, not the split, because the test exists to + // catch undercount regressions, not to overspecify classifier behaviour. + val totalFailures = + AuthenticationProviderFactory.ldapAuthFailureInvalidCredentials + + AuthenticationProviderFactory.ldapAuthFailureInvalidInput + + AuthenticationProviderFactory.ldapAuthFailureInfrastructure + val invCreds = AuthenticationProviderFactory.ldapAuthFailureInvalidCredentials + val invInput = AuthenticationProviderFactory.ldapAuthFailureInvalidInput + val infra = AuthenticationProviderFactory.ldapAuthFailureInfrastructure + assert( + totalFailures === 8L, + s"every request must contribute one failure increment; got total=$totalFailures " + + s"(invalid_credentials=$invCreds, invalid_input=$invInput, infrastructure=$infra)") + } + + test("server and http LDAP scopes share a single cache instance") { + // AuthenticationProviderFactory builds independent CachingLdapAuthenticationProvider + // instances for the "server" and "http" auth scopes. Both must consult the same + // backing cache, otherwise a user who authenticates over Thrift then HTTP re-pays + // the LDAP round-trip within the TTL window. + val callCount = new AtomicInteger(0) + val delegate1 = makeDelegate { (_, _) => callCount.incrementAndGet() } + val delegate2 = makeDelegate { (_, _) => callCount.incrementAndGet() } + + val conf = new KyuubiConf() + val serverScope = new CachingLdapAuthenticationProvider(delegate1, conf) + val httpScope = new CachingLdapAuthenticationProvider(delegate2, conf) + + // First auth via "server" scope populates the shared cache. + serverScope.authenticate("alice", "secret") + assert(callCount.get() === 1, "first authentication must hit the delegate") + + // Same credentials via "http" scope must hit the cache and skip the delegate. + httpScope.authenticate("alice", "secret") + assert( + callCount.get() === 1, + "http scope must reuse the cache entry populated by the server scope") + + // The shared cache has exactly one entry, regardless of which wrapper queries. + assert(serverScope.cacheSize() === 1) + assert(httpScope.cacheSize() === 1) + } + + test("cache miss with delegate failure: cache.miss increments, no success or failure") { + // The wrapper records cache.miss and rethrows. The delegate (stub here, impl in + // production) is responsible for recording success/failure -- the wrapper does not. + val delegate = makeDelegate { (_, _) => throw new AuthenticationException("nope") } + val provider = new CachingLdapAuthenticationProvider(delegate, new KyuubiConf()) + + intercept[AuthenticationException](provider.authenticate("alice", "secret")) + assert(AuthenticationProviderFactory.ldapAuthCacheMiss === 1L) + assert(AuthenticationProviderFactory.ldapAuthCacheHit === 0L) + assert(AuthenticationProviderFactory.ldapAuthSuccess === 0L) + // Failure counters are owned by LdapAuthenticationProviderImpl, not the wrapper. + assert(AuthenticationProviderFactory.ldapAuthFailureInvalidCredentials === 0L) + assert(AuthenticationProviderFactory.ldapAuthFailureInvalidInput === 0L) + assert(AuthenticationProviderFactory.ldapAuthFailureInfrastructure === 0L) + } +} diff --git a/kyuubi-common/src/test/scala/org/apache/kyuubi/service/authentication/ldap/LdapAuthFailureClassifierSuite.scala b/kyuubi-common/src/test/scala/org/apache/kyuubi/service/authentication/ldap/LdapAuthFailureClassifierSuite.scala new file mode 100644 index 00000000000..e39bc4f0942 --- /dev/null +++ b/kyuubi-common/src/test/scala/org/apache/kyuubi/service/authentication/ldap/LdapAuthFailureClassifierSuite.scala @@ -0,0 +1,98 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.kyuubi.service.authentication.ldap + +import javax.naming.{AuthenticationException => JndiAuthenticationException, CommunicationException, NamingException} +import javax.security.sasl.AuthenticationException + +import com.unboundid.ldap.sdk.{LDAPException, ResultCode} + +import org.apache.kyuubi.KyuubiFunSuite + +class LdapAuthFailureClassifierSuite extends KyuubiFunSuite { + + import LdapAuthFailureClassifier._ + + test("INVALID_CREDENTIALS via cause chain -> invalid_credentials") { + val ldapEx = new LDAPException(ResultCode.INVALID_CREDENTIALS, "bad password") + val authEx = new AuthenticationException("Error validating LDAP user: alice", ldapEx) + assert(classify(authEx) === INVALID_CREDENTIALS) + } + + test("non-INVALID_CREDENTIALS LDAPException cause -> infrastructure") { + val ldapEx = new LDAPException(ResultCode.SERVER_DOWN, "no route") + val authEx = new AuthenticationException("LDAP bind failed", ldapEx) + assert(classify(authEx) === INFRASTRUCTURE) + } + + test("blank user message -> invalid_input") { + val ex = new AuthenticationException( + "Error validating LDAP user, user is null or contains blank space") + assert(classify(ex) === INVALID_INPUT) + } + + test("blank password message -> invalid_input") { + val ex = new AuthenticationException( + "Error validating LDAP user, password is null or contains blank space") + assert(classify(ex) === INVALID_INPUT) + } + + test("no candidate principals message -> invalid_input") { + val ex = new AuthenticationException("No candidate principals for alice was found.") + assert(classify(ex) === INVALID_INPUT) + } + + test("Unable to find the user message (no cause) -> infrastructure") { + // This is the message LdapAuthenticationProviderImpl emits when it catches a + // NamingException; the cause is intentionally not chained in production code. + val ex = new AuthenticationException( + "Unable to find the user in the LDAP tree. LDAP connect failed: timeout") + assert(classify(ex) === INFRASTRUCTURE) + } + + test("AuthenticationException with null message and null cause -> infrastructure") { + val ex = new AuthenticationException() + assert(classify(ex) === INFRASTRUCTURE) + } + + test("plain NamingException -> infrastructure") { + assert(classify(new NamingException("bad query")) === INFRASTRUCTURE) + } + + test("CommunicationException (NamingException subtype) -> infrastructure") { + assert(classify(new CommunicationException("connection refused")) === INFRASTRUCTURE) + } + + test("RuntimeException -> infrastructure") { + assert(classify(new RuntimeException("bug")) === INFRASTRUCTURE) + } + + test("javax.naming.AuthenticationException cause -> invalid_credentials (JNDI path)") { + // Legacy JNDI path: LdapSearchFactory wraps a failed bind (wrong password) as a + // sasl.AuthenticationException whose cause is a javax.naming.AuthenticationException. + // This must classify as invalid_credentials, not infrastructure. + val jndiEx = new JndiAuthenticationException("[LDAP: error code 49 - Invalid Credentials]") + val authEx = new AuthenticationException("Error validating LDAP user: alice", jndiEx) + assert(classify(authEx) === INVALID_CREDENTIALS) + } + + test("NamingException cause inside AuthenticationException -> infrastructure") { + val authEx = new AuthenticationException("LDAP bind failed", new NamingException("boom")) + assert(classify(authEx) === INFRASTRUCTURE) + } +} diff --git a/kyuubi-common/src/test/scala/org/apache/kyuubi/service/authentication/ldap/LdapSearchSuite.scala b/kyuubi-common/src/test/scala/org/apache/kyuubi/service/authentication/ldap/LdapSearchSuite.scala index 3bf27127ba3..8c7e80aad32 100644 --- a/kyuubi-common/src/test/scala/org/apache/kyuubi/service/authentication/ldap/LdapSearchSuite.scala +++ b/kyuubi-common/src/test/scala/org/apache/kyuubi/service/authentication/ldap/LdapSearchSuite.scala @@ -17,9 +17,10 @@ package org.apache.kyuubi.service.authentication.ldap -import javax.naming.{NamingEnumeration, NamingException} +import javax.naming.{CommunicationException, NameNotFoundException, NamingEnumeration, NamingException} import javax.naming.directory.{DirContext, SearchControls, SearchResult} +import org.mockito.ArgumentCaptor import org.mockito.ArgumentMatchers.{any, anyString, contains, eq => mockEq} import org.mockito.Mockito.{atLeastOnce, verify, when} import org.scalatestplus.mockito.MockitoSugar.mock @@ -208,6 +209,49 @@ class LdapSearchSuite extends KyuubiFunSuite { assert(expected.sorted === actual.sorted) } + test("DnOnlyQueryRequestsNoAttributes") { + // Regression: query.attributes is empty for every lookup except customQuery. The JNDI + // path must set an EMPTY returning-attributes array so the server returns the DN only; + // leaving it unset (null) makes JNDI return ALL attributes -- a needless payload/load + // increase for DN-only lookups such as findUserDn / findGroupDn. + conf.set(KyuubiConf.AUTHENTICATION_LDAP_GROUP_DN_PATTERN, "CN=%s,OU=org1,DC=foo,DC=bar") + // Build the enumeration first: mockNamingEnumeration stubs mocks internally, so inlining + // it inside thenReturn(...) would nest stubbing and throw UnfinishedStubbingException. + val result = mockNamingEnumeration("CN=Group1") + when(ctx.search(anyString, anyString, any(classOf[SearchControls]))).thenReturn(result) + search = new LdapSearch(conf, ctx) + search.findGroupDn("grp1") + val controls = ArgumentCaptor.forClass(classOf[SearchControls]) + verify(ctx).search(anyString, anyString, controls.capture()) + val returning = controls.getValue.getReturningAttributes + assert(returning != null, "returningAttributes must be an empty array, not null") + assert( + returning.isEmpty, + s"DN-only query must request no attributes; got: ${returning.mkString(",")}") + } + + test("CustomQueryRequestsOnlyMembershipAttribute") { + // customQuery is the only query that requests an attribute (the group-membership key, + // default "member"). The returning-attributes array must contain exactly that attribute + // -- not all attributes -- so the result carries only the membership values plus the DN. + conf.set(KyuubiConf.AUTHENTICATION_LDAP_BASE_DN, "dc=example,dc=com") + // Build the enumeration first: mockSearchResult/mockNamingEnumeration stub mocks + // internally, so inlining inside thenReturn(...) throws UnfinishedStubbingException. + val result = mockNamingEnumeration(Array( + mockSearchResult( + "uid=group1,ou=Groups,dc=example,dc=com", + mockAttributes("member", "uid=user1,ou=People,dc=example,dc=com")))) + when(ctx.search(mockEq("dc=example,dc=com"), anyString, any(classOf[SearchControls]))) + .thenReturn(result) + search = new LdapSearch(conf, ctx) + search.executeCustomQuery("(objectClass=groupOfNames)") + val controls = ArgumentCaptor.forClass(classOf[SearchControls]) + verify(ctx).search(anyString, anyString, controls.capture()) + val returning = controls.getValue.getReturningAttributes + assert(returning != null, "returningAttributes must be set for customQuery") + assert(returning.toSeq === Seq("member")) + } + test("FindGroupDnPositive") { conf.set( KyuubiConf.AUTHENTICATION_LDAP_GROUP_DN_PATTERN, @@ -250,20 +294,38 @@ class LdapSearchSuite extends KyuubiFunSuite { } - test("FindGroupDNWhenExceptionInSearch") { + test("FindGroupDNWhenNameNotFoundExceptionOnSecondBase") { + // NameNotFoundException (base DN does not exist) must be swallowed and the next + // base DN tried. Previously any NamingException was swallowed, which masked real + // infrastructure failures. Only NameNotFoundException is the correct carve-out. conf.set( KyuubiConf.AUTHENTICATION_LDAP_GROUP_DN_PATTERN, Array("CN=%s,OU=org1,DC=foo,DC=bar", "CN=%s,OU=org2,DC=foo,DC=bar").mkString(":")) val result: NamingEnumeration[SearchResult] = LdapTestUtils.mockNamingEnumeration("CN=Group1") when(ctx.search(anyString, anyString, any(classOf[SearchControls]))) .thenReturn(result) - .thenThrow(classOf[NamingException]) + .thenThrow(new NameNotFoundException("ou=org2 does not exist")) search = new LdapSearch(conf, ctx) val expected: String = "CN=Group1" val actual: String = search.findGroupDn("grp1") assert(expected === actual) } + test("FindGroupDNWhenCommunicationExceptionPropagates") { + // Infrastructure failures (CommunicationException, ServiceUnavailableException, etc.) + // must propagate so callers can distinguish "group not found" from "LDAP is down". + // The original broad NamingException catch silently turned outages into empty results. + conf.set( + KyuubiConf.AUTHENTICATION_LDAP_GROUP_DN_PATTERN, + "CN=%s,OU=org1,DC=foo,DC=bar") + when(ctx.search(anyString, anyString, any(classOf[SearchControls]))) + .thenThrow(new CommunicationException("connection refused")) + search = new LdapSearch(conf, ctx) + intercept[NamingException] { + search.findGroupDn("grp1") + } + } + test("IsUserMemberOfGroupWhenUserId") { conf.set( KyuubiConf.AUTHENTICATION_LDAP_USER_DN_PATTERN, diff --git a/kyuubi-common/src/test/scala/org/apache/kyuubi/service/authentication/ldap/LdapUrlParserSuite.scala b/kyuubi-common/src/test/scala/org/apache/kyuubi/service/authentication/ldap/LdapUrlParserSuite.scala new file mode 100644 index 00000000000..70c060c36fa --- /dev/null +++ b/kyuubi-common/src/test/scala/org/apache/kyuubi/service/authentication/ldap/LdapUrlParserSuite.scala @@ -0,0 +1,96 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.kyuubi.service.authentication.ldap + +import org.apache.kyuubi.KyuubiFunSuite + +/** + * Tests for [[LdapUrlParser]], which delegates to [[com.unboundid.ldap.sdk.LDAPURL]] + * for RFC 4516-compliant URL parsing (default ports, case-insensitive scheme, etc.). + */ +class LdapUrlParserSuite extends KyuubiFunSuite { + + test("plain ldap with explicit port") { + val Seq(url) = LdapUrlParser.parseAll("ldap://ldap.example.com:389") + assert(url.getHost === "ldap.example.com") + assert(url.getPort === 389) + assert(url.getScheme.equalsIgnoreCase("ldap")) + } + + test("ldaps with explicit port") { + val Seq(url) = LdapUrlParser.parseAll("ldaps://ldap.example.com:636") + assert(url.getHost === "ldap.example.com") + assert(url.getPort === 636) + assert(url.getScheme.equalsIgnoreCase("ldaps")) + } + + test("ldap without port defaults to 389") { + val Seq(url) = LdapUrlParser.parseAll("ldap://ldap.example.com") + assert(url.getPort === 389) + assert(url.getScheme.equalsIgnoreCase("ldap")) + } + + test("ldaps without port defaults to 636") { + val Seq(url) = LdapUrlParser.parseAll("ldaps://ldap.example.com") + assert(url.getPort === 636) + assert(url.getScheme.equalsIgnoreCase("ldaps")) + } + + test("scheme matching is case-insensitive") { + val Seq(url) = LdapUrlParser.parseAll("LDAPS://ldap.example.com:636") + assert(url.getScheme.equalsIgnoreCase("ldaps")) + assert(url.getHost === "ldap.example.com") + } + + test("URL with base DN is accepted") { + val Seq(url) = LdapUrlParser.parseAll("ldap://ldap.example.com:389/dc=example,dc=com") + assert(url.getHost === "ldap.example.com") + assert(url.getPort === 389) + } + + test("parseAll splits on whitespace and returns all URLs") { + val urls = LdapUrlParser.parseAll( + "ldaps://server1.example.com:636 ldaps://server2.example.com:636 ldap://server3.example.com") + assert(urls.size === 3) + assert(urls(0).getHost === "server1.example.com") + assert(urls(1).getHost === "server2.example.com") + assert(urls(2).getHost === "server3.example.com") + assert(urls(0).getScheme.equalsIgnoreCase("ldaps")) + assert(urls(1).getScheme.equalsIgnoreCase("ldaps")) + assert(urls(2).getScheme.equalsIgnoreCase("ldap")) + } + + test("parseAll handles extra whitespace between URLs") { + val urls = LdapUrlParser.parseAll(" ldap://a.com ldap://b.com ") + assert(urls.size === 2) + assert(urls(0).getHost === "a.com") + assert(urls(1).getHost === "b.com") + } + + test("parseAll returns empty sequence for blank input") { + assert(LdapUrlParser.parseAll(" ").isEmpty) + assert(LdapUrlParser.parseAll("").isEmpty) + } + + test("invalid URL throws IllegalArgumentException") { + val ex = intercept[IllegalArgumentException] { + LdapUrlParser.parseAll("not-a-url") + } + assert(ex.getMessage.contains("Invalid LDAP URL")) + } +} diff --git a/kyuubi-common/src/test/scala/org/apache/kyuubi/service/authentication/ldap/LdapUtilsSuite.scala b/kyuubi-common/src/test/scala/org/apache/kyuubi/service/authentication/ldap/LdapUtilsSuite.scala index 1ef371051e4..6799f0c3891 100644 --- a/kyuubi-common/src/test/scala/org/apache/kyuubi/service/authentication/ldap/LdapUtilsSuite.scala +++ b/kyuubi-common/src/test/scala/org/apache/kyuubi/service/authentication/ldap/LdapUtilsSuite.scala @@ -67,6 +67,21 @@ class LdapUtilsSuite extends KyuubiFunSuite { assert(actual.sorted === expected.sorted) } + test("CreateCandidatePrincipalsPreservesRegexSpecialCharsInUserName") { + // Regression: substitution into the DN pattern must be literal. A user name containing + // '$' (or '\') is interpreted as a regex replacement back-reference/escape by + // String.replaceAll -- throwing IndexOutOfBoundsException or corrupting the value -- + // whereas String.replace substitutes it verbatim. + val conf = new KyuubiConf() + .set(KyuubiConf.AUTHENTICATION_LDAP_BASE_DN, "dc=mycompany,dc=com") + .set( + KyuubiConf.AUTHENTICATION_LDAP_USER_DN_PATTERN, + "cn=%s,ou=CORP,dc=mycompany,dc=com") + val expected = Array("cn=user$1,ou=CORP,dc=mycompany,dc=com") + val actual = LdapUtils.createCandidatePrincipals(conf, "user$1") + assert(actual === expected) + } + test("ExtractFirstRdn") { val dn = "cn=user1,ou=CORP1,dc=mycompany,dc=com" val expected = "cn=user1" @@ -85,4 +100,73 @@ class LdapUtilsSuite extends KyuubiFunSuite { val dn: String = "cn=user1" assert(LdapUtils.extractBaseDn(dn) === null) } + + test("extractBaseDn handles DN with escaped comma in value") { + // "Smith, John" contains a comma that must be escaped in a DN; indexOf would split here + // incorrectly but DN.getParent() handles it correctly. + val dn = "cn=Smith\\, John,ou=People,dc=example,dc=com" + assert(LdapUtils.extractBaseDn(dn) === "ou=People,dc=example,dc=com") + } + + test("extractFirstRdn handles DN with escaped comma in value") { + val dn = "cn=Smith\\, John,ou=People,dc=example,dc=com" + assert(LdapUtils.extractFirstRdn(dn) === "cn=Smith\\, John") + } + + test("extractBaseDn falls back for pattern strings containing %s") { + val pattern = "cn=%s,ou=CORP,dc=mycompany,dc=com" + assert(LdapUtils.extractBaseDn(pattern) === "ou=CORP,dc=mycompany,dc=com") + } + + test("extractFirstRdn falls back for pattern strings containing %s") { + val pattern = "cn=%s,ou=CORP,dc=mycompany,dc=com" + assert(LdapUtils.extractFirstRdn(pattern) === "cn=%s") + } + + test("isDn returns true for valid distinguished name") { + assert(LdapUtils.isDn("cn=user1,ou=CORP,dc=mycompany,dc=com")) + } + + test("isDn returns false for plain username") { + assert(!LdapUtils.isDn("user1")) + } + + test("isDn returns false for null without throwing") { + assert(!LdapUtils.isDn(null)) + } + + // Behaviour-change guards against regressions to the legacy `contains("=")` heuristic + // that pre-dated the switch to UnboundID DN.isValidDN. Strings that contain '=' but are + // malformed (invalid RDN, missing attribute name, etc.) must NOT be treated as DNs -- + // they should fall through to the candidate-principal pattern matcher instead. This is + // the documented behavior change in F6 of the review. + + test("isDn returns false for malformed DN with invalid second RDN") { + // Legacy heuristic returned true (contains '='); new strict check correctly returns + // false because the second component "bad" has no attribute=value form. + assert(!LdapUtils.isDn("cn=alice,bad")) + } + + test("isDn returns false for malformed DN with empty attribute name") { + assert(!LdapUtils.isDn("=foo")) + } + + test("isDn returns false for email-style usernames") { + assert(!LdapUtils.isDn("alice@example.com")) + } + + test("isDn returns false for empty string") { + // UnboundID DN.isValidDN("") returns true (the empty / null DN per RFC 4514), but our + // isDn explicitly rejects it -- an empty string is never a meaningful DN login. + assert(!LdapUtils.isDn("")) + } + + test("isDn returns true for valid single-RDN DN") { + assert(LdapUtils.isDn("uid=alice")) + } + + test("isDn returns true for valid DN with escaped commas in attribute values") { + assert(LdapUtils.isDn("cn=Doe\\, John,ou=people,dc=example,dc=com")) + } + } diff --git a/kyuubi-common/src/test/scala/org/apache/kyuubi/service/authentication/ldap/QueryFactorySuite.scala b/kyuubi-common/src/test/scala/org/apache/kyuubi/service/authentication/ldap/QueryFactorySuite.scala index 56800968000..32a8880dea0 100644 --- a/kyuubi-common/src/test/scala/org/apache/kyuubi/service/authentication/ldap/QueryFactorySuite.scala +++ b/kyuubi-common/src/test/scala/org/apache/kyuubi/service/authentication/ldap/QueryFactorySuite.scala @@ -37,23 +37,20 @@ class QueryFactorySuite extends KyuubiFunSuite { test("FindGroupDnById") { val q = queries.findGroupDnById("unique_group_id") val expected = "(&(objectClass=superGroups)(guid=unique_group_id))" - val actual = q.filter - assert(expected === actual) + assert(expected === q.filterString) } test("FindUserDnByRdn") { val q = queries.findUserDnByRdn("cn=User1") val expected = "(&(|(objectClass=person)(objectClass=user)(objectClass=inetOrgPerson))(cn=User1))" - val actual = q.filter - assert(expected === actual) + assert(expected === q.filterString) } test("FindDnByPattern") { val q = queries.findDnByPattern("cn=User1") val expected = "(cn=User1)" - val actual = q.filter - assert(expected === actual) + assert(expected === q.filterString) } test("FindUserDnByName") { @@ -61,15 +58,13 @@ class QueryFactorySuite extends KyuubiFunSuite { val expected = "(&(|(objectClass=person)(objectClass=user)(objectClass=inetOrgPerson))" + "(|(uid=unique_user_id)(sAMAccountName=unique_user_id)))" - val actual = q.filter - assert(expected === actual) + assert(expected === q.filterString) } test("FindGroupsForUser") { val q = queries.findGroupsForUser("user_name", "user_Dn") val expected = "(&(objectClass=superGroups)(|(member=user_Dn)(member=user_name)))" - val actual = q.filter - assert(expected === actual) + assert(expected === q.filterString) } test("IsUserMemberOfGroup") { @@ -77,8 +72,7 @@ class QueryFactorySuite extends KyuubiFunSuite { val expected = "(&(|(objectClass=person)(objectClass=user)(objectClass=inetOrgPerson))" + "(partOf=cn=MyGroup,ou=Groups,dc=mycompany,dc=com)(guid=unique_user))" - val actual = q.filter - assert(expected === actual) + assert(expected === q.filterString) } test("IsUserMemberOfGroupWhenMisconfigured") { @@ -91,7 +85,37 @@ class QueryFactorySuite extends KyuubiFunSuite { test("FindGroupDNByID") { val q = queries.findGroupDnById("unique_group_id") val expected = "(&(objectClass=superGroups)(guid=unique_group_id))" - val actual = q.filter - assert(expected === actual) + assert(expected === q.filterString) + } + + test("findUserDnByName escapes LDAP filter metacharacters in userName") { + // userName is in an attribute value position: (uid=). Without escaping, a + // username like "jsmith)(uid=admin" would inject extra filter clauses. With escaping, + // the special characters are converted to \xx hex form and treated as literals. + val injectionAttempt = "jsmith)(uid=admin" + val q = queries.findUserDnByName(injectionAttempt) + // Parentheses become \28 and \29 per RFC 4515. + assert(!q.filterString.contains(")(uid=admin"), "filter must not contain injected clause") + assert( + q.filterString.contains("\\28") || q.filterString.contains("jsmith"), + s"escaped metacharacters expected in filter; got: ${q.filterString}") + // The parsed filter must also be valid (LdapFilter.create succeeded inside build()). + assert(q.filter != null) + } + + test("findUserDnByName with wildcard username escapes asterisk") { + val q = queries.findUserDnByName("*") + // Unescaped * would be a presence assertion matching all users. + // Escaped \2a is a literal search for the string "*". + assert(!q.filterString.contains("uid=*"), s"wildcard must be escaped; got: ${q.filterString}") + assert(q.filterString.contains("\\2a"), s"expected \\2a in filter; got: ${q.filterString}") + } + + test("filter field is a parsed Filter object matching filterString") { + val q = queries.findGroupDnById("unique_group_id") + assert(q.filter != null) + // Filter.toString() returns the normalized RFC 4515 representation; for our simple + // filter strings there should be no normalization difference. + assert(q.filterString === q.filter.toString) } } diff --git a/kyuubi-common/src/test/scala/org/apache/kyuubi/service/authentication/ldap/QuerySuite.scala b/kyuubi-common/src/test/scala/org/apache/kyuubi/service/authentication/ldap/QuerySuite.scala index ffe330cce8b..de33e359376 100644 --- a/kyuubi-common/src/test/scala/org/apache/kyuubi/service/authentication/ldap/QuerySuite.scala +++ b/kyuubi-common/src/test/scala/org/apache/kyuubi/service/authentication/ldap/QuerySuite.scala @@ -21,34 +21,43 @@ import org.apache.kyuubi.KyuubiFunSuite class QuerySuite extends KyuubiFunSuite { - test("QueryBuilderFilter") { + test("QueryBuilderFilter renders template into filterString and parses Filter") { val q = Query.builder - .filter("test = query") - .map("uid_attr", "uid") - .map("value", "Hello!") + .filter("(uid=)") + .map("value", "alice") .build - assert("test uid=Hello! query" === q.filter) - assert(0 === q.controls.getCountLimit) + // filterString holds the rendered template; filter is the parsed UnboundID Filter object. + assert("(uid=alice)" === q.filterString) + assert(q.filter != null) + assert("(uid=alice)" === q.filter.toString) + assert(0 === q.sizeLimit) } test("QueryBuilderLimit") { val q = Query.builder - .filter(",") - .map("key1", "value1") - .map("key2", "value2") + .filter("(uid=)") + .map("value", "alice") .limit(8) .build - assert("value1,value2" === q.filter) - assert(8 === q.controls.getCountLimit) + assert("(uid=alice)" === q.filterString) + assert(8 === q.sizeLimit) } test("QueryBuilderReturningAttributes") { val q = Query.builder - .filter("(query)") + .filter("(objectClass=*)") .returnAttribute("attr1") .returnAttribute("attr2") .build - assert("(query)" === q.filter) - assert(Array("attr1", "attr2") === q.controls.getReturningAttributes) + assert("(objectClass=*)" === q.filterString) + assert(Seq("attr1", "attr2") === q.attributes) + } + + test("build throws IllegalStateException for non-LDAP filter string") { + intercept[IllegalStateException] { + Query.builder + .filter("not a valid ldap filter") + .build + } } } diff --git a/kyuubi-common/src/test/scala/org/apache/kyuubi/service/authentication/ldap/UnboundIdDirSearchFactorySuite.scala b/kyuubi-common/src/test/scala/org/apache/kyuubi/service/authentication/ldap/UnboundIdDirSearchFactorySuite.scala new file mode 100644 index 00000000000..201cb1ffea7 --- /dev/null +++ b/kyuubi-common/src/test/scala/org/apache/kyuubi/service/authentication/ldap/UnboundIdDirSearchFactorySuite.scala @@ -0,0 +1,690 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.kyuubi.service.authentication.ldap + +import java.net.ServerSocket +import javax.security.sasl.AuthenticationException + +import com.unboundid.ldap.listener.{InMemoryDirectoryServer, InMemoryDirectoryServerConfig} +import com.unboundid.ldap.sdk.{Attribute, Entry} + +import org.apache.kyuubi.KyuubiFunSuite +import org.apache.kyuubi.config.KyuubiConf +import org.apache.kyuubi.service.authentication.{AuthenticationProviderFactory, LdapAuthenticationProviderImpl, PasswdAuthenticationProvider, WithLdapServer} + +/** + * Integration tests for [[UnboundIdDirSearchFactory]] against an in-process LDAP server. + * + * Tests the two code paths: + * - Bind-user path: borrows connection from pool + * - End-user path: ephemeral BIND with failover across multiple URLs + */ +class UnboundIdDirSearchFactorySuite extends KyuubiFunSuite with WithLdapServer { + + private var conf: KyuubiConf = _ + + override protected def beforeEach(): Unit = { + super.beforeEach() + conf = new KyuubiConf() + conf.set(KyuubiConf.AUTHENTICATION_LDAP_URL, ldapUrl) + conf.set(KyuubiConf.AUTHENTICATION_LDAP_BASE_DN, "ou=users") + } + + override protected def afterEach(): Unit = { + // Reset the AuthenticationProviderFactory singleton state (auth counters, installed + // metrics recorder, cached LDAP providers, owned pool) so tests that drive the + // recorder cannot leak observations into subsequent tests. + try AuthenticationProviderFactory.close() + finally super.afterEach() + } + + test("getInstance returns DirSearch for valid end-user credentials") { + val factory = new UnboundIdDirSearchFactory(conf, poolOpt = None) + val bindDn = s"uid=$ldapUser,ou=users" + val search = factory.getInstance(conf, bindDn, ldapUserPasswd) + try { + assert(search != null) + } finally { + search.close() + } + } + + test("getInstance throws AuthenticationException for invalid credentials (LDAP 49)") { + val factory = new UnboundIdDirSearchFactory(conf, poolOpt = None) + val bindDn = s"uid=$ldapUser,ou=users" + intercept[AuthenticationException] { + factory.getInstance(conf, bindDn, "wrong-password") + } + } + + test("invalid credentials do not failover to second server") { + // Start a second in-memory server + val config2 = new InMemoryDirectoryServerConfig("ou=users") + config2.setSchema(null) + config2.addAdditionalBindCredentials(s"uid=$ldapUser,ou=users", ldapUserPasswd) + val server2 = new InMemoryDirectoryServer(config2) + server2.startListening() + try { + val twoServerUrl = s"$ldapUrl ldap://localhost:${server2.getListenPort}" + conf.set(KyuubiConf.AUTHENTICATION_LDAP_URL, twoServerUrl) + val factory = new UnboundIdDirSearchFactory(conf, poolOpt = None) + + // With wrong password, must fail immediately without trying server2 + val ex = intercept[AuthenticationException] { + factory.getInstance(conf, s"uid=$ldapUser,ou=users", "wrong-password") + } + assert(ex.getMessage.contains("InvalidCredentials")) + } finally { + server2.close() + } + } + + test("connection failure on first server falls over to second server") { + // Start a second in-memory server; point first URL at a port that will refuse connections + val config2 = new InMemoryDirectoryServerConfig("ou=users") + config2.setSchema(null) + config2.addAdditionalBindCredentials(s"uid=$ldapUser,ou=users", ldapUserPasswd) + val server2 = new InMemoryDirectoryServer(config2) + server2.startListening() + try { + // Allocate-and-close a port to get one that's guaranteed-refused on this host. + // Using a hardcoded port like 19999 produces a misleading green if a developer has + // an unrelated service bound to it; ServerSocket(0)+close is deterministic. + val refusedPort = { + val s = new ServerSocket(0) + try s.getLocalPort + finally s.close() + } + val twoServerUrl = + s"ldap://localhost:$refusedPort ldap://localhost:${server2.getListenPort}" + conf.set(KyuubiConf.AUTHENTICATION_LDAP_URL, twoServerUrl) + val factory = new UnboundIdDirSearchFactory(conf, poolOpt = None) + val search = factory.getInstance(conf, s"uid=$ldapUser,ou=users", ldapUserPasswd) + try { + assert(search != null) + } finally { + search.close() + } + } finally { + server2.close() + } + } + + test("bind-user path borrows from pool") { + conf.set(KyuubiConf.AUTHENTICATION_LDAP_BIND_USER, s"uid=$ldapUser,ou=users") + conf.set(KyuubiConf.AUTHENTICATION_LDAP_BIND_PASSWORD, ldapUserPasswd) + val pool = new UnboundIdConnectionPool(conf) + try { + val factory = new UnboundIdDirSearchFactory(conf, poolOpt = Some(pool)) + val search = factory.getInstance(conf, s"uid=$ldapUser,ou=users", ldapUserPasswd) + try { + assert(search != null) + } finally { + search.close() + } + } finally { + pool.close() + } + } + + test("counter attribution across cache wrapper + impl: miss then hit") { + // Joint integration of CachingLdapAuthenticationProvider + LdapAuthenticationProviderImpl + // against the in-memory directory. Locks in the per-layer responsibilities: + // - first call (cache miss): wrapper bumps cache.miss; impl runs and bumps success + // - second call (cache hit): wrapper bumps cache.hit AND success; impl is not invoked + // Total over the two calls: cache.miss=1, cache.hit=1, success=2, failures=0. + // A future refactor that double-counts success (e.g. wrapper also bumps it on miss) + // would push success to 3 and fail this test. + val recorder = new CapturingLdapAuthRecorder + AuthenticationProviderFactory.setLdapAuthMetricsRecorder(recorder) + + val factory = new UnboundIdDirSearchFactory(conf, poolOpt = None) + val impl = new LdapAuthenticationProviderImpl(conf, factory) + val cached: PasswdAuthenticationProvider = + new CachingLdapAuthenticationProvider(impl, conf) + + val bindDn = s"uid=$ldapUser,ou=users" + + cached.authenticate(bindDn, ldapUserPasswd) // miss -> impl runs -> impl records success + assert(recorder.cacheMisses.get === 1) + assert(recorder.cacheHits.get === 0) + assert(recorder.successes.get === 1, "impl must record success on cache-miss success") + + cached.authenticate(bindDn, ldapUserPasswd) // hit -> wrapper records success; impl skipped + assert(recorder.cacheMisses.get === 1, "second call must not bump cache.miss") + assert(recorder.cacheHits.get === 1) + assert( + recorder.successes.get === 2, + "wrapper must bump success on cache hit; total success = miss-success + hit-success") + + // Failure buckets must remain at zero -- this path is two clean successes. + assert(recorder.invalidCreds.get === 0) + assert(recorder.invalidInput.get === 0) + assert(recorder.infrastructure.get === 0) + } + + test("end-to-end failure metrics: bad credentials classify as invalid_credentials") { + // Drives the full production stack -- LdapAuthenticationProviderImpl -> + // UnboundIdDirSearchFactory -> ephemeralBind -> LdapAuthFailureClassifier -> recorder + // -- against the in-memory directory, with a wrong password, and asserts the + // recorder observed exactly one `invalid_credentials` event. Guards against future + // regressions in the classifier's cause-chain dispatch path (e.g. the + // INVALID_CREDENTIALS LDAPException cause check) that the cache-suite tests, which + // use a stub delegate, would not catch. + val recorder = new CapturingLdapAuthRecorder + AuthenticationProviderFactory.setLdapAuthMetricsRecorder(recorder) + + val factory = new UnboundIdDirSearchFactory(conf, poolOpt = None) + val provider = new LdapAuthenticationProviderImpl(conf, factory) + + intercept[AuthenticationException] { + provider.authenticate(ldapUser, "wrong-password") + } + + // The classifier should map the chained LDAPException(INVALID_CREDENTIALS) cause to + // `invalid_credentials`. Asserting zero on the other buckets pins the dispatch to + // exactly one bucket -- a regression that maps everything to `infrastructure` would + // increment infrastructure and leave invalidCreds at 0. + assert( + recorder.invalidCreds.get === 1, + s"expected exactly one invalid_credentials event, got " + + s"invalidCreds=${recorder.invalidCreds.get}, " + + s"infrastructure=${recorder.infrastructure.get}, " + + s"invalidInput=${recorder.invalidInput.get}") + assert(recorder.successes.get === 0) + assert(recorder.infrastructure.get === 0) + assert(recorder.invalidInput.get === 0) + } + + test("executeCustomQuery output parity: JNDI vs UnboundID against same server") { + // F8 from the review: confirm that the new UnboundID-backed executeCustomQuery + // produces the same set of strings the legacy JNDI implementation does for the same + // query against the same data. Both flatten the search results into a single array + // of [DN, attribute_value, attribute_value, ..., DN, ...]; LDAP gives no guarantee on + // attribute iteration order, so this asserts SET equality rather than exact sequence. + // The in-memory server's base DN exists in its config but is not auto-populated as + // an entry, so we add it (idempotently, since other test invocations may have done + // so) before any child entry. + if (ldapServer.getEntry("ou=users") == null) { + ldapServer.add(new Entry( + "ou=users", + new Attribute("objectClass", "top", "organizationalUnit"), + new Attribute("ou", "users"))) + } + val parityDn = "uid=parityuser,ou=users" + val parityEntry = new Entry( + parityDn, + new Attribute("objectClass", "top", "person", "inetOrgPerson"), + new Attribute("uid", "parityuser"), + new Attribute("cn", "Parity User"), + new Attribute("sn", "User"), + // QueryFactory.customQuery requests only the `groupMembershipAttr` (default + // "member"). Multi-valued member here exercises the inner flatMap over attribute + // values, and matches how executeCustomQuery is actually used in production + // (custom-query group membership lookups). + new Attribute("member", "cn=group1,ou=groups", "cn=group2,ou=groups")) + ldapServer.add(parityEntry) + + val query = "(uid=parityuser)" + val bindDn = s"uid=$ldapUser,ou=users" + + val jndiFactory = new LdapSearchFactory + val jndiSearch = jndiFactory.getInstance(conf, bindDn, ldapUserPasswd) + val jndiResult = + try jndiSearch.executeCustomQuery(query) + finally jndiSearch.close() + + val unboundFactory = new UnboundIdDirSearchFactory(conf, poolOpt = None) + val unboundSearch = unboundFactory.getInstance(conf, bindDn, ldapUserPasswd) + val unboundResult = + try unboundSearch.executeCustomQuery(query) + finally unboundSearch.close() + + // Both must observe the matching DN and both values of the multi-valued member attribute. + assert(jndiResult.contains(parityDn)) + assert(unboundResult.contains(parityDn)) + assert(jndiResult.contains("cn=group1,ou=groups")) + assert(unboundResult.contains("cn=group1,ou=groups")) + assert(jndiResult.contains("cn=group2,ou=groups")) + assert(unboundResult.contains("cn=group2,ou=groups")) + + // Set equality is the meaningful invariant -- the JNDI and UnboundID iteration + // orders over attributes are implementation-defined and need not match. A divergence + // in the SET would indicate one path is dropping or duplicating data. + val jndiSet = jndiResult.toSet + val unboundSet = unboundResult.toSet + assert( + jndiSet === unboundSet, + s"executeCustomQuery diverged.\nOnly in JNDI: ${jndiSet -- unboundSet}\n" + + s"Only in UnboundID: ${unboundSet -- jndiSet}") + + // Length parity guards against one path silently de-duplicating. + assert( + jndiResult.length === unboundResult.length, + s"length mismatch -- JNDI=${jndiResult.length} (${jndiResult.toSeq}), " + + s"UnboundID=${unboundResult.length} (${unboundResult.toSeq})") + } + + test("pool initialises with first URL refused and second URL healthy") { + // Coverage gap: UnboundIdDirSearchFactorySuite already exercised ephemeral-path + // failover (line 97), but the pooled path -- which is the main new code -- had no + // failover-on-init test. Verifies that UnboundIdConnectionPool boots cleanly when the + // first configured server refuses connections, because FailoverServerSet skips ahead + // to the next URL during pool warmup rather than failing the whole pool construction. + val refusedPort = { + val s = new ServerSocket(0) + try s.getLocalPort + finally s.close() + } + val twoServerUrl = s"ldap://localhost:$refusedPort $ldapUrl" + conf.set(KyuubiConf.AUTHENTICATION_LDAP_URL, twoServerUrl) + conf.set(KyuubiConf.AUTHENTICATION_LDAP_BIND_USER, s"uid=$ldapUser,ou=users") + conf.set(KyuubiConf.AUTHENTICATION_LDAP_BIND_PASSWORD, ldapUserPasswd) + conf.set(KyuubiConf.AUTHENTICATION_LDAP_POOL_MIN_CONNECTIONS, 1) + conf.set(KyuubiConf.AUTHENTICATION_LDAP_POOL_MAX_CONNECTIONS, 2) + + val pool = new UnboundIdConnectionPool(conf) + try { + val factory = new UnboundIdDirSearchFactory(conf, poolOpt = Some(pool)) + val search = factory.getInstance(conf, s"uid=$ldapUser,ou=users", ldapUserPasswd) + try assert(search != null) + finally search.close() + // Pool must have at least one healthy connection -- failover succeeded. + assert( + pool.numAvailableConnections >= 1, + s"expected >=1 healthy connection after pool boot with refused first URL, got " + + s"${pool.numAvailableConnections}") + } finally { + pool.close() + } + } + + /** + * Ensures the shared in-memory directory has a real searchable entry for `ldapUser`. + * WithLdapServer only registers the credentials via addAdditionalBindCredentials, which + * lets BIND succeed but leaves no entry that findUserDn / search can return. Tests that + * drive the full LdapAuthenticationProviderImpl bind-user flow (impl.authenticate -> + * findUserDn -> userSearch) need the entry present. Idempotent so multiple tests can call it. + */ + private def ensureUserEntry(server: InMemoryDirectoryServer = ldapServer): Unit = { + if (server.getEntry("ou=users") == null) { + server.add(new Entry( + "ou=users", + new Attribute("objectClass", "top", "organizationalUnit"), + new Attribute("ou", "users"))) + } + val userDn = s"uid=$ldapUser,ou=users" + if (server.getEntry(userDn) == null) { + server.add(new Entry( + userDn, + new Attribute("objectClass", "top", "person", "inetOrgPerson"), + new Attribute("uid", ldapUser), + new Attribute("cn", ldapUser), + new Attribute("sn", ldapUser), + new Attribute("userPassword", ldapUserPasswd))) + } + } + + test("pool retries mid-request when active connection breaks server-side") { + // Verifies the bounded retry path added to UnboundIdDirSearch.searchBaseWithRetry: + // when a pooled connection dies mid-SEARCH (server kill, dropped socket), the in-flight + // request is not surfaced as a failure -- the broken connection is discarded and a + // fresh one (from a healthy peer in FailoverServerSet) is used to complete the search. + // This closes the JNDI gap where failover applied only at initial-context creation. + // + // Strategy: two in-memory servers; pool configured with [server1, server2] in failover + // order. Warm the pool against server1, kill server1, then drive an auth. The first + // checkout returns a dead-to-server1 connection (we did not enable invokeOnCheckout + // health checks because they add per-request latency), so the SEARCH fails, the retry + // discards it, and FailoverServerSet advances the next checkout to server2. + val server1Config = new InMemoryDirectoryServerConfig("ou=users") + server1Config.setSchema(null) + server1Config.addAdditionalBindCredentials(s"uid=$ldapUser,ou=users", ldapUserPasswd) + val server1 = new InMemoryDirectoryServer(server1Config) + server1.startListening() + ensureUserEntry(server1) + val server2Config = new InMemoryDirectoryServerConfig("ou=users") + server2Config.setSchema(null) + server2Config.addAdditionalBindCredentials(s"uid=$ldapUser,ou=users", ldapUserPasswd) + val server2 = new InMemoryDirectoryServer(server2Config) + server2.startListening() + ensureUserEntry(server2) + try { + val twoServerUrl = + s"ldap://localhost:${server1.getListenPort} ldap://localhost:${server2.getListenPort}" + conf.set(KyuubiConf.AUTHENTICATION_LDAP_URL, twoServerUrl) + conf.set(KyuubiConf.AUTHENTICATION_LDAP_BIND_USER, s"uid=$ldapUser,ou=users") + conf.set(KyuubiConf.AUTHENTICATION_LDAP_BIND_PASSWORD, ldapUserPasswd) + conf.set(KyuubiConf.AUTHENTICATION_LDAP_POOL_MIN_CONNECTIONS, 1) + conf.set(KyuubiConf.AUTHENTICATION_LDAP_POOL_MAX_CONNECTIONS, 2) + + val pool = new UnboundIdConnectionPool(conf) + try { + val factory = new UnboundIdDirSearchFactory(conf, poolOpt = Some(pool)) + + // Warm: borrow + release one connection so the pool holds an idle (now-doomed) + // connection to server1. + val warm = factory.getInstance(conf, s"uid=$ldapUser,ou=users", ldapUserPasswd) + warm.close() + + // Kill server1. All idle pool connections are now broken at the socket level. + server1.shutDown(true) + + // Drive an auth that triggers a SEARCH (findUserDn). The first SearchRequest fails + // with a connection-level result code; the retry path must discard, re-checkout, + // fail over to server2, and complete successfully. + val search = factory.getInstance(conf, s"uid=$ldapUser,ou=users", ldapUserPasswd) + try { + val dn = search.findUserDn(ldapUser) + assert( + dn != null, + "mid-request failover did not recover the search -- expected findUserDn to " + + "succeed against server2 after server1 was killed") + } finally { + search.close() + } + } finally { + pool.close() + } + } finally { + try server1.close() + catch { case _: Throwable => () } + server2.close() + } + } + + test("bind-user flow closes every DirSearch it opens (no end-user verification leak)") { + // Regression guard for the second-DirSearch leak previously present in + // LdapAuthenticationProviderImpl.doAuthenticate: createDirSearch(userDn, password) + // returned a DirSearch wrapping an ephemeral LDAPConnection whose close() was the only + // path that freed the socket -- and the returned value was discarded, not closed. + // + // Strategy: wrap the real factory with one that hands out tracking decorators counting + // open() and close() calls. If the fix is in place, opens == closes for any number of + // auths. The decorator is local to this test, so the assertion is exact -- no flakiness + // from external FD accounting or directory-server stats. + ensureUserEntry() + conf.set(KyuubiConf.AUTHENTICATION_LDAP_BIND_USER, s"uid=$ldapUser,ou=users") + conf.set(KyuubiConf.AUTHENTICATION_LDAP_BIND_PASSWORD, ldapUserPasswd) + conf.set(KyuubiConf.AUTHENTICATION_LDAP_POOL_MIN_CONNECTIONS, 1) + conf.set(KyuubiConf.AUTHENTICATION_LDAP_POOL_MAX_CONNECTIONS, 2) + + val pool = new UnboundIdConnectionPool(conf) + try { + val opens = new java.util.concurrent.atomic.AtomicInteger(0) + val closes = new java.util.concurrent.atomic.AtomicInteger(0) + val realFactory = new UnboundIdDirSearchFactory(conf, poolOpt = Some(pool)) + val countingFactory: DirSearchFactory = new DirSearchFactory { + override def getInstance( + conf: KyuubiConf, + principal: String, + password: String): DirSearch = { + val inner = realFactory.getInstance(conf, principal, password) + opens.incrementAndGet() + new DirSearch { + override def findUserDn(user: String): String = inner.findUserDn(user) + override def findGroupDn(group: String): String = inner.findGroupDn(group) + override def isUserMemberOfGroup(user: String, groupDn: String): Boolean = + inner.isUserMemberOfGroup(user, groupDn) + override def findGroupsForUser(userDn: String): Array[String] = + inner.findGroupsForUser(userDn) + override def executeCustomQuery(query: String): Array[String] = + inner.executeCustomQuery(query) + override def close(): Unit = { + try inner.close() + finally { closes.incrementAndGet(); () } + } + } + } + } + val impl = new LdapAuthenticationProviderImpl(conf, countingFactory) + + val nAuths = 20 + (1 to nAuths).foreach { _ => impl.authenticate(ldapUser, ldapUserPasswd) } + + // The bind-user flow opens TWO DirSearches per auth: one for the bind-user pool + // checkout, one for the end-user credential-verification ephemeral bind. Both must + // be closed. opens/closes are an exact pairing -- not a bound. + assert( + opens.get === 2 * nAuths, + s"expected ${2 * nAuths} DirSearch opens for $nAuths bind-user auths, " + + s"got ${opens.get}") + assert( + closes.get === opens.get, + s"DirSearch leak detected: ${opens.get} opened, ${closes.get} closed " + + s"(diff ${opens.get - closes.get}). The end-user credential-verification " + + s"DirSearch is the historical leak site.") + } finally { + pool.close() + } + } + + test("pool reuse: burst of N auths opens <= max.connections pool sockets") { + // Asserts the central pool-reuse claim. With a bind user configured and pool capped at + // poolMax, a burst of nAuths (>> poolMax) must NOT open nAuths pool connections -- the + // pool must reuse warm connections across requests. Each iteration also opens exactly + // one ephemeral end-user verification connection that is closed inside the auth path + // (verified separately by the leak-detection test above). + ensureUserEntry() + conf.set(KyuubiConf.AUTHENTICATION_LDAP_BIND_USER, s"uid=$ldapUser,ou=users") + conf.set(KyuubiConf.AUTHENTICATION_LDAP_BIND_PASSWORD, ldapUserPasswd) + val poolMin = 1 + val poolMax = 3 + conf.set(KyuubiConf.AUTHENTICATION_LDAP_POOL_MIN_CONNECTIONS, poolMin) + conf.set(KyuubiConf.AUTHENTICATION_LDAP_POOL_MAX_CONNECTIONS, poolMax) + + val pool = new UnboundIdConnectionPool(conf) + try { + val factory = new UnboundIdDirSearchFactory(conf, poolOpt = Some(pool)) + val impl = new LdapAuthenticationProviderImpl(conf, factory) + val nAuths = 50 + (1 to nAuths).foreach { _ => impl.authenticate(ldapUser, ldapUserPasswd) } + + // numSuccessfulCheckouts grows with each pool borrow; the count of distinct + // connections actually created is bounded by poolMax. Health-check replacements can + // add a small slack but not a per-request multiplier. + assert( + pool.numAvailableConnections <= poolMax, + s"pool exceeded max -- numAvailable=${pool.numAvailableConnections} > $poolMax") + assert( + pool.numSuccessfulCheckouts >= nAuths, + s"expected >=$nAuths successful checkouts, got ${pool.numSuccessfulCheckouts}") + } finally { + pool.close() + } + } + + test("retry does not double-release pool connection when replacement checkout fails") { + // Regression guard for the retry edge case: when searchBaseWithRetry releases the + // broken connection via releaseConnectionAfterException and the immediately-following + // pool.checkoutConnection() also fails (e.g. pool exhausted, every server down), the + // DirSearch.close() that fires as the exception propagates must NOT release the same + // connection a second time. Tracked via UnboundIdDirSearch.connectionAlreadyReleased. + // + // Strategy: use a counting pool subclass. Borrow a connection while the server is up, + // kill the server (so a search on that connection will fail), arm the pool to refuse + // the replacement checkout, then drive a search. We assert that exactly ONE + // releaseConnectionAfterException call lands at the pool -- the retry path's release. + // Without the fix, close() would release the same connection a second time and the + // counter would tick to two. + import java.util.concurrent.atomic.AtomicInteger + import com.unboundid.ldap.sdk.{LDAPConnection, LDAPException => SdkLdapEx, ResultCode} + + val standaloneConfig = new InMemoryDirectoryServerConfig("ou=users") + standaloneConfig.setSchema(null) + standaloneConfig.addAdditionalBindCredentials(s"uid=$ldapUser,ou=users", ldapUserPasswd) + val standalone = new InMemoryDirectoryServer(standaloneConfig) + standalone.startListening() + ensureUserEntry(standalone) + try { + val isolatedConf = new KyuubiConf() + isolatedConf.set( + KyuubiConf.AUTHENTICATION_LDAP_URL, + s"ldap://localhost:${standalone.getListenPort}") + isolatedConf.set(KyuubiConf.AUTHENTICATION_LDAP_BASE_DN, "ou=users") + isolatedConf.set(KyuubiConf.AUTHENTICATION_LDAP_BIND_USER, s"uid=$ldapUser,ou=users") + isolatedConf.set(KyuubiConf.AUTHENTICATION_LDAP_BIND_PASSWORD, ldapUserPasswd) + isolatedConf.set(KyuubiConf.AUTHENTICATION_LDAP_POOL_MIN_CONNECTIONS, 1) + isolatedConf.set(KyuubiConf.AUTHENTICATION_LDAP_POOL_MAX_CONNECTIONS, 1) + + val releaseAfterExceptionCount = new AtomicInteger(0) + val releaseHealthyCount = new AtomicInteger(0) + val refuseCheckout = new AtomicInteger(0) + val countingPool = new UnboundIdConnectionPool(isolatedConf) { + override def checkoutConnection(): LDAPConnection = { + if (refuseCheckout.get() > 0) { + // Simulate "no replacement available" (pool exhausted, all servers down). + throw new SdkLdapEx(ResultCode.CONNECT_ERROR, "test-induced checkout failure") + } + super.checkoutConnection() + } + override def releaseConnection(conn: LDAPConnection): Unit = { + releaseHealthyCount.incrementAndGet() + super.releaseConnection(conn) + } + override def releaseConnectionAfterException( + conn: LDAPConnection, + e: SdkLdapEx): Unit = { + releaseAfterExceptionCount.incrementAndGet() + super.releaseConnectionAfterException(conn, e) + } + } + try { + // Borrow a connection while the server is still up so the search request actually + // travels onto a real (now-doomed) socket. + val borrowed = countingPool.checkoutConnection() + // Kill the server. The borrowed connection is now broken at the socket level; + // its next operation will throw an LDAPException with !isConnectionUsable. + standalone.shutDown(true) + // Arm the pool so the retry path's replacement checkout fails -- forcing the + // exception-propagation branch of searchBaseWithRetry to fire. + refuseCheckout.incrementAndGet() + + val search = new UnboundIdDirSearch(isolatedConf, borrowed, Some(countingPool)) + intercept[javax.naming.NamingException] { + search.findUserDn(ldapUser) + } + // close() runs after the failed retry path. The fix means it must observe + // connectionAlreadyReleased=true and skip the release. + search.close() + + // Exactly ONE release-with-exception: the one from inside the retry path. + // Two would indicate close() did a duplicate release on an already-returned + // connection. + assert( + releaseAfterExceptionCount.get === 1, + s"expected exactly 1 releaseConnectionAfterException call, got " + + s"${releaseAfterExceptionCount.get}. >1 implies the retry path released " + + s"and then close() released the same connection again.") + // Healthy-release path must not be touched: the connection was never healthy at + // close() time. + assert( + releaseHealthyCount.get === 0, + s"healthy releaseConnection must not be called when the failed retry leaves " + + s"the connection already released; got ${releaseHealthyCount.get}.") + } finally { + countingPool.close() + } + } finally { + try standalone.close() + catch { case _: Throwable => () } + } + } + + test("checkoutConnection emits warn when FailoverServerSet lands on secondary server") { + // Verifies the failover-visibility warning added to UnboundIdConnectionPool.checkoutConnection: + // when the primary server is unreachable and FailoverServerSet quietly advances to a secondary, + // the checkout must emit a WARN log naming both servers so operators can detect live failover + // without having to correlate Prometheus defunct-connection counters. + // + // Setup: primary URL points at a refused port; secondary URL is the real in-memory server. + // All pool connections land on the secondary, so every checkout fires the warn. + import org.apache.logging.log4j.Level + val refusedPort = { + val s = new ServerSocket(0) + try s.getLocalPort + finally s.close() + } + // primary=refused, secondary=real; minConns=1 so the pool warms during construction + val twoServerUrl = s"ldap://localhost:$refusedPort $ldapUrl" + conf.set(KyuubiConf.AUTHENTICATION_LDAP_URL, twoServerUrl) + conf.set(KyuubiConf.AUTHENTICATION_LDAP_BIND_USER, s"uid=$ldapUser,ou=users") + conf.set(KyuubiConf.AUTHENTICATION_LDAP_BIND_PASSWORD, ldapUserPasswd) + conf.set(KyuubiConf.AUTHENTICATION_LDAP_POOL_MIN_CONNECTIONS, 1) + conf.set(KyuubiConf.AUTHENTICATION_LDAP_POOL_MAX_CONNECTIONS, 2) + + val logAppender = new LogAppender("checkoutConnection failover warn") + logAppender.setThreshold(Level.WARN) + withLogAppender(logAppender, loggerNames = Seq(classOf[UnboundIdConnectionPool].getName)) { + val pool = new UnboundIdConnectionPool(conf) + try { + val conn = pool.checkoutConnection() + pool.releaseConnection(conn) + } finally { + pool.close() + } + } + val warnMessages = logAppender.loggingEvents + .filter(_.getLevel == Level.WARN) + .map(_.getMessage.getFormattedMessage) + assert( + warnMessages.exists(_.contains("failover active")), + s"expected a WARN containing 'failover active' when all connections land on secondary; " + + s"got: ${warnMessages.mkString(", ")}") + assert( + warnMessages.exists(_.contains(s"localhost:$refusedPort")), + s"warn message should name the primary (refused) server localhost:$refusedPort; " + + s"got: ${warnMessages.mkString(", ")}") + } + + test("malformed username with filter metacharacters classifies as invalid_input") { + // Regression guard for the Query.QueryBuilder.build IllegalStateException bypass: a + // username containing LDAP filter metacharacters renders to an unparseable RFC 4515 + // filter (e.g. "(uid=*)(uid=*)"), build() throws ISE, and -- before the fix -- that + // ISE escaped the auth contract uncaught, skipping both the auth-failure conversion + // and the metric recorder. The catch in LdapAuthenticationProviderImpl.doAuthenticate + // now wraps it as AuthenticationException with a prefix the classifier maps to + // invalid_input. + conf.set(KyuubiConf.AUTHENTICATION_LDAP_BIND_USER, s"uid=$ldapUser,ou=users") + conf.set(KyuubiConf.AUTHENTICATION_LDAP_BIND_PASSWORD, ldapUserPasswd) + + val recorder = new CapturingLdapAuthRecorder + AuthenticationProviderFactory.setLdapAuthMetricsRecorder(recorder) + + val factory = new UnboundIdDirSearchFactory(conf, poolOpt = None) + val provider = new LdapAuthenticationProviderImpl(conf, factory) + + val badUser = "*)(uid=*" + val ex = intercept[AuthenticationException] { + provider.authenticate(badUser, ldapUserPasswd) + } + // The wrapped message must start with the invalid_input prefix recognised by the + // classifier so the metric lands in the right bucket. + assert( + ex.getMessage.startsWith("Error validating LDAP user, invalid filter"), + s"unexpected exception message: ${ex.getMessage}") + assert( + recorder.invalidInput.get === 1, + s"expected one invalid_input event, got invalidInput=${recorder.invalidInput.get}, " + + s"infrastructure=${recorder.infrastructure.get}, " + + s"invalidCreds=${recorder.invalidCreds.get}") + assert(recorder.invalidCreds.get === 0) + assert(recorder.infrastructure.get === 0) + assert(recorder.successes.get === 0) + } +} diff --git a/kyuubi-common/src/test/scala/org/apache/kyuubi/service/authentication/ldap/UnboundIdDirSearchSuite.scala b/kyuubi-common/src/test/scala/org/apache/kyuubi/service/authentication/ldap/UnboundIdDirSearchSuite.scala new file mode 100644 index 00000000000..719c049510b --- /dev/null +++ b/kyuubi-common/src/test/scala/org/apache/kyuubi/service/authentication/ldap/UnboundIdDirSearchSuite.scala @@ -0,0 +1,151 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.kyuubi.service.authentication.ldap + +import com.unboundid.ldap.sdk.{Attribute, Entry} + +import org.apache.kyuubi.KyuubiFunSuite +import org.apache.kyuubi.config.KyuubiConf +import org.apache.kyuubi.service.authentication.WithLdapServer + +/** + * Integration tests for [[UnboundIdDirSearch]] against an in-process LDAP server. + * + * Each test gets its own fresh connection to avoid state leakage between test cases. + */ +class UnboundIdDirSearchSuite extends KyuubiFunSuite with WithLdapServer { + + private var conf: KyuubiConf = _ + + override protected def beforeEach(): Unit = { + super.beforeEach() + conf = new KyuubiConf() + conf.set(KyuubiConf.AUTHENTICATION_LDAP_BASE_DN, "ou=users") + } + + private def openSearch(): UnboundIdDirSearch = { + val conn = ldapServer.getConnection + new UnboundIdDirSearch(conf, conn, poolOpt = None) + } + + /** + * Ensures the shared in-memory directory has a real searchable entry for [[ldapUser]]. + * [[WithLdapServer]] only registers bind credentials via addAdditionalBindCredentials, + * which lets BIND succeed but does not create any entry that search can return. + * Tests that drive findUserDn / findDnByPattern need the entry present. Idempotent. + */ + private def ensureUserEntry(): Unit = { + if (ldapServer.getEntry("ou=users") == null) { + ldapServer.add(new Entry( + "ou=users", + new Attribute("objectClass", "top", "organizationalUnit"), + new Attribute("ou", "users"))) + } + val userDn = s"uid=$ldapUser,ou=users" + if (ldapServer.getEntry(userDn) == null) { + ldapServer.add(new Entry( + userDn, + new Attribute("objectClass", "top", "person", "inetOrgPerson"), + new Attribute("uid", ldapUser), + new Attribute("cn", ldapUser), + new Attribute("sn", ldapUser), + new Attribute("userPassword", ldapUserPasswd))) + } + } + + test("findUserDn returns null when user not found") { + val search = openSearch() + try { + assert(search.findUserDn("nosuchuser") === null) + } finally { + search.close() + } + } + + test("findUserDn with bind credentials finds configured user") { + // Requires a real directory entry: WithLdapServer.beforeAll only registers bind + // credentials which are not searchable. findDnByPattern runs a filter search so + // it needs an actual entry under ou=users to return a non-empty result. + ensureUserEntry() + conf.set(KyuubiConf.AUTHENTICATION_LDAP_USER_DN_PATTERN, s"uid=%s,ou=users") + val search = openSearch() + try { + val dn = search.findUserDn(ldapUser) + assert(dn === s"uid=$ldapUser,ou=users") + } finally { + search.close() + } + } + + test("executeCustomQuery returns only the configured attribute, not all attributes") { + // Regression for the attribute over-fetch: searches must request only the attributes the + // caller needs. customQuery requests the group-membership attribute; previously the empty + // attribute list was sent to the server as "*" (all attributes), so executeCustomQuery + // returned every attribute value (e.g. sn) instead of just the configured one. + ensureUserEntry() + val attrUserDn = "uid=attrtest,ou=users" + if (ldapServer.getEntry(attrUserDn) == null) { + ldapServer.add(new Entry( + attrUserDn, + new Attribute("objectClass", "top", "person", "inetOrgPerson"), + new Attribute("uid", "attrtest"), + new Attribute("cn", "AttrTest Common Name"), + new Attribute("sn", "AttrTestSurname"))) + } + // customQuery returns the group-membership attribute; point it at cn for this entry. + conf.set(KyuubiConf.AUTHENTICATION_LDAP_GROUP_MEMBERSHIP_KEY, "cn") + val search = openSearch() + try { + val results = search.executeCustomQuery("(uid=attrtest)").toSeq + assert(results.contains(attrUserDn), s"DN should be returned; got: $results") + assert(results.contains("AttrTest Common Name"), s"cn should be returned; got: $results") + assert( + !results.contains("AttrTestSurname"), + s"sn was not requested and must not be returned; got: $results") + } finally { + search.close() + } + } + + test("close on ephemeral connection (poolOpt = None) closes the connection") { + val conn = ldapServer.getConnection + val search = new UnboundIdDirSearch(conf, conn, poolOpt = None) + search.close() + assert(!conn.isConnected) + } + + test("close on pooled connection releases back to pool") { + val conf2 = new KyuubiConf() + conf2.set(KyuubiConf.AUTHENTICATION_LDAP_URL, ldapUrl) + conf2.set(KyuubiConf.AUTHENTICATION_LDAP_BIND_USER, s"uid=$ldapUser,ou=users") + conf2.set(KyuubiConf.AUTHENTICATION_LDAP_BIND_PASSWORD, ldapUserPasswd) + val pool = new UnboundIdConnectionPool(conf2) + try { + val conn = pool.checkoutConnection() + val search = new UnboundIdDirSearch(conf, conn, poolOpt = Some(pool)) + // Should release healthy connection back to pool, not close it + search.close() + // Pool should still be functional -- checkout a second connection + val conn2 = pool.checkoutConnection() + assert(conn2.isConnected) + pool.releaseConnection(conn2) + } finally { + pool.close() + } + } +} diff --git a/kyuubi-metrics/src/main/scala/org/apache/kyuubi/metrics/MetricsConstants.scala b/kyuubi-metrics/src/main/scala/org/apache/kyuubi/metrics/MetricsConstants.scala index 61a5d8d31d5..b05bae87271 100644 --- a/kyuubi-metrics/src/main/scala/org/apache/kyuubi/metrics/MetricsConstants.scala +++ b/kyuubi-metrics/src/main/scala/org/apache/kyuubi/metrics/MetricsConstants.scala @@ -103,4 +103,16 @@ object MetricsConstants { final private val JETTY = KYUUBI + "jetty." final val JETTY_API_V1 = JETTY + "api.v1" + + // Authentication metrics. Dimensions (auth method, failure reason) are encoded as + // dot-suffixes on the base name via MetricRegistry.name(BASE, dim, ...) -- the same + // pattern used by OPERATION_STATE and CONN_OPEN. Registered names look like + // "kyuubi.authentication.success.ldap" or + // "kyuubi.authentication.failure.ldap.invalid_credentials" and surface in Prometheus + // with dot-to-underscore translation by DropwizardExports. + final private val AUTHENTICATION = KYUUBI + "authentication." + final val AUTHENTICATION_SUCCESS: String = AUTHENTICATION + "success" + final val AUTHENTICATION_FAILURE: String = AUTHENTICATION + "failure" + final val AUTHENTICATION_CACHE_HIT: String = AUTHENTICATION + "cache.hit" + final val AUTHENTICATION_CACHE_MISS: String = AUTHENTICATION + "cache.miss" } diff --git a/kyuubi-server/src/main/scala/org/apache/kyuubi/server/KyuubiServer.scala b/kyuubi-server/src/main/scala/org/apache/kyuubi/server/KyuubiServer.scala index 0996472653b..a441c63ed6c 100644 --- a/kyuubi-server/src/main/scala/org/apache/kyuubi/server/KyuubiServer.scala +++ b/kyuubi-server/src/main/scala/org/apache/kyuubi/server/KyuubiServer.scala @@ -19,6 +19,7 @@ package org.apache.kyuubi.server import scala.util.Properties +import com.codahale.metrics.{Metric, MetricRegistry} import org.apache.commons.lang3.StringUtils import org.apache.hadoop.conf.Configuration import org.apache.hadoop.security.UserGroupInformation @@ -33,6 +34,7 @@ import org.apache.kyuubi.ha.client.{AuthTypes, ServiceDiscovery} import org.apache.kyuubi.metrics.{MetricsConf, MetricsSystem} import org.apache.kyuubi.server.metadata.jdbc.JDBCMetadataStoreConf import org.apache.kyuubi.service.{AbstractBackendService, AbstractFrontendService, Serverable, ServiceState} +import org.apache.kyuubi.service.authentication.{AuthenticationProviderFactory, LdapAuthMetricsRecorder} import org.apache.kyuubi.session.KyuubiSessionManager import org.apache.kyuubi.util.{KyuubiHadoopUtils, SignalRegister} import org.apache.kyuubi.zookeeper.EmbeddedZookeeper @@ -222,10 +224,122 @@ class KyuubiServer(name: String) extends Serverable(name) { override def start(): Unit = { super.start() + // MetricsSystem is a child service started by super.start(); registering after that + // call guarantees the metrics are picked up. The accessors return 0 until the LDAP pool + // is lazily created on the first authentication request, so registration is order-safe. + installLdapAuthMetricsRecorder() + registerLdapPoolMetrics() KyuubiServer.kyuubiServer = this KyuubiServerInfoEvent(this, ServiceState.STARTED).foreach(EventBus.post) } + /** + * Plugs the auth-event hook in `kyuubi-common` into `MetricsSystem.incCount`, which uses + * Dropwizard's `Counter` underneath. `DropwizardExports` emits these as + * `# TYPE name counter` in Prometheus, so `rate()` / `increase()` queries handle process + * restarts correctly. Dimensions (auth method, failure reason) are dot-suffixes on the + * base name -- matches the convention of `CONN_OPEN`, `OPERATION_STATE`, etc. + * + * The `MetricsSystem` reference is captured once via `tracing` at install time rather + * than dispatched per call: auth is a hot path under load, and per-event `Option`-check + * lookups are wasteful when the reference is stable for the JVM lifetime. If metrics is + * not running at install time (i.e. `MetricsSystem.tracing` is a no-op), the recorder + * remains at its default `NoOp`; the alternative -- installing a closure that captures a + * stale reference -- would silently write to a dead registry after a restart. + * + * Shutdown ordering invariant (do not break): in [[stop()]] / [[stopServer()]] the + * teardown order is `super.stop()` then `stopServer()`. `super.stop()` is + * [[org.apache.kyuubi.service.CompositeService.stop]], which tears down child services + * in reverse start order. Because [[initialize]] adds [[MetricsSystem]] before + * `super.initialize(conf)` (which adds the backend and frontend services), the + * frontends stop first and MetricsSystem stops afterwards within `super.stop()`. By the + * time MetricsSystem stops, no auth request can arrive at the recorder. Only after + * `super.stop()` returns does `stopServer()` call [[AuthenticationProviderFactory.close]], + * which resets the recorder to NoOp. The closure's captured `ms` reference is therefore + * never invoked after MetricsSystem has stopped. + * + * Moving `AuthenticationProviderFactory.close` to run before `super.stop()` would close + * the LDAP connection pool while the frontends are still accepting requests, causing + * in-flight auths to fail with pool-closed errors -- a real regression. Do NOT change + * the order without also addressing that. + */ + private def installLdapAuthMetricsRecorder(): Unit = MetricsSystem.tracing { ms => + import org.apache.kyuubi.metrics.MetricsConstants._ + val ldap = "ldap" + val successName = MetricRegistry.name(AUTHENTICATION_SUCCESS, ldap) + val cacheHitName = MetricRegistry.name(AUTHENTICATION_CACHE_HIT, ldap) + val cacheMissName = MetricRegistry.name(AUTHENTICATION_CACHE_MISS, ldap) + AuthenticationProviderFactory.setLdapAuthMetricsRecorder(new LdapAuthMetricsRecorder { + override def recordSuccess(): Unit = ms.incCount(successName) + override def recordCacheHit(): Unit = ms.incCount(cacheHitName) + override def recordCacheMiss(): Unit = ms.incCount(cacheMissName) + override def recordFailure(reason: String): Unit = + // The `reason` dimension is variable so this single metric name is built per call, + // but `MetricRegistry.name` is just a string join -- nothing expensive. + ms.incCount(MetricRegistry.name(AUTHENTICATION_FAILURE, ldap, reason)) + }) + } + + /** + * LDAP pool metrics. Of the five values [[LDAPConnectionPoolStatistics]] exposes, four + * are cumulative (checkouts, defunct/expired closures) -- those must be Prometheus + * counters for `rate()` / `increase()` to be correct across Kyuubi restarts. The fifth + * (`connections.available`) is a point-in-time queue size and stays a gauge. + * + * The SDK owns the underlying values, so we register a [[LdapPoolStatCounter]] whose + * `getCount()` reads from the SDK statistics on each scrape; `inc`/`dec` are + * [[UnsupportedOperationException]] so a future caller cannot silently drop an + * increment by going through [[org.apache.kyuubi.metrics.MetricsSystem.incCount]] on + * one of the names below (which would otherwise resolve to the existing + * [[LdapPoolStatCounter]] via `MetricRegistry.counter(name)`). + * + * Contract: the four cumulative metric names below are READ-ONLY -- their values flow + * one-way from `LDAPConnectionPoolStatistics` into the registry on every scrape. Do not + * call `MetricsSystem.incCount` on any of them; the registered counter will throw. + */ + private def registerLdapPoolMetrics(): Unit = MetricsSystem.tracing { ms => + val prefix = "kyuubi.authentication.ldap.pool" + val connectionsAvailable = s"$prefix.connections.available" + + // Read-only counter names -- backed by LdapPoolStatCounter, never call .incCount on these. + val checkoutsSuccessful = s"$prefix.checkouts.successful" + val checkoutsFailed = s"$prefix.checkouts.failed" + val connectionsClosedDefunct = s"$prefix.connections.closed.defunct" + val connectionsClosedExpired = s"$prefix.connections.closed.expired" + + MetricsSystem.getMetricsRegistry.foreach { registry => + // start() may run more than once against the same MetricRegistry (embedded use, or a + // restart after a partial shutdown). Dropwizard's register(...) throws + // IllegalArgumentException on a duplicate name, so register each metric only when it is + // absent to keep a re-start idempotent. + def registerIfAbsent(name: String, metric: => Metric): Unit = + if (!registry.getNames.contains(name)) registry.register(name, metric) + + if (!registry.getNames.contains(connectionsAvailable)) { + ms.registerGauge( + connectionsAvailable, + AuthenticationProviderFactory.ldapPoolNumAvailableConnections, + 0L) + } + registerIfAbsent( + checkoutsSuccessful, + new LdapPoolStatCounter(() => + AuthenticationProviderFactory.ldapPoolNumSuccessfulCheckouts)) + registerIfAbsent( + checkoutsFailed, + new LdapPoolStatCounter(() => + AuthenticationProviderFactory.ldapPoolNumFailedCheckouts)) + registerIfAbsent( + connectionsClosedDefunct, + new LdapPoolStatCounter(() => + AuthenticationProviderFactory.ldapPoolNumConnectionsClosedDefunct)) + registerIfAbsent( + connectionsClosedExpired, + new LdapPoolStatCounter(() => + AuthenticationProviderFactory.ldapPoolNumConnectionsClosedExpired)) + } + } + override def stop(): Unit = { KyuubiServerInfoEvent(this, ServiceState.STOPPED).foreach(EventBus.post) super.stop() @@ -236,6 +350,37 @@ class KyuubiServer(name: String) extends Serverable(name) { } override protected def stopServer(): Unit = { + // Release the LDAP connection pool (and any cached auth providers) before the JVM exits. + // CompositeService.stop has already drained the frontend / backend services, so no + // authentication request can race with this teardown. + AuthenticationProviderFactory.close() EventBus.deregisterAll() } } + +/** + * A Dropwizard [[com.codahale.metrics.Counter]] whose value is read from a supplier rather + * than maintained internally. Used to register the cumulative counters of + * [[com.unboundid.ldap.sdk.LDAPConnectionPoolStatistics]] -- which the SDK already owns -- + * as Prometheus type `counter`. The supplier is authoritative; inc/dec are explicitly + * unsupported so a future call site that tries `MetricsSystem.incCount` against one of the + * registered names fails loudly instead of silently dropping the increment (the + * MetricRegistry would otherwise hand back this instance and the no-op .inc() would mask + * the bug). + * + * Lives in this file because `kyuubi-common` does not depend on `metrics-core`; promoting + * this class to a shared location would require adding that dependency. + */ +final private class LdapPoolStatCounter(read: () => Long) extends com.codahale.metrics.Counter { + // Empty parens (not nullary) to match Java's `long getCount()`; a nullary override + // triggers -Xlint:nullary-override, which is fatal under -Xfatal-warnings on Scala 2.13. + override def getCount(): Long = read() + override def inc(): Unit = throw new UnsupportedOperationException( + "LdapPoolStatCounter is read-only; value sourced from LDAPConnectionPoolStatistics") + override def inc(n: Long): Unit = throw new UnsupportedOperationException( + "LdapPoolStatCounter is read-only; value sourced from LDAPConnectionPoolStatistics") + override def dec(): Unit = throw new UnsupportedOperationException( + "LdapPoolStatCounter is read-only; value sourced from LDAPConnectionPoolStatistics") + override def dec(n: Long): Unit = throw new UnsupportedOperationException( + "LdapPoolStatCounter is read-only; value sourced from LDAPConnectionPoolStatistics") +}