This is an automated email from the ASF dual-hosted git repository.
morningman pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/master by this push:
new d715350c96b [enhance](auth) introduction of configuration property to
prohibit login with empty LDAP password (#61440)
d715350c96b is described below
commit d715350c96b1286d21dfde35c9d38a9c633c9e0d
Author: iaorekhov-1980 <[email protected]>
AuthorDate: Tue Aug 18 11:08:48 2026 +0300
[enhance](auth) introduction of configuration property to prohibit login
with empty LDAP password (#61440)
### What problem does this PR solve?
This PR adds new configuration property **ldap_allow_empty_pass** to
prohibit option for existing user to login into LDAP with empty
password.
It doesn't impact new approach from #60407 , because since 4.1.x new
LDAP plugin explicitly prohibits login with empty pass.
But in legacy version - 3.1.x and 4.0.x such option is still available.
If **ldap_allow_empty_pass** in ldap.conf is not specified or specified
as **false** - user can't login with empty pass (new behavior, aligned
with newly introduced plugin authentication).
If **ldap_allow_empty_pass** specified as **true** - login attempt with
empty password will be possible as legacy behavior.
**Could you please include this PR into 4.x branch, please!**
Issue Number: close #60353
Related PR: #xxx
Problem Summary:
Currently for existing user it is possible to login into LDAP with empty
password.
New configuration property disables such option, but user is still able
to activate legacy behavior and allow login without specified password.
### Release note
New **ldap_allow_empty_pass** property for legacy authentication
approach was introduced into ldap.conf to prohibit login with empty LDAP
password as it is allowed by LDAP protocol by default.
### Check List (For Author)
- Test <!-- At least one of them must be included. -->
- [ ] Regression test
- [x] Unit Test
- [ ] Manual test (add detailed scripts or steps below)
- [ ] No need to test or manual test. Explain why:
- [ ] This is a refactor/code format and no logic has been changed.
- [ ] Previous test can cover this change.
- [ ] No code files have been changed.
- [ ] Other reason <!-- Add your reason? -->
- Behavior changed:
- [ ] No.
- [x] Yes. <!-- Explain the behavior change -->
1. ldap.conf and LdapConfig.java - new configuration
**ldap_allow_empty_pass** property with default value **false** to
disable login with empty LDAP password as in latest plugin
implementation.
2. LdapManager.java - added logic to prohibit login with empty LDAP
password and placed it into separate method, which is invoked from
already existing checkUserPasswd
3.1 user has specified empty password
3.2 property **ldap_allow_empty_pass** is **false** and doesn't allow to
login with empty password
If both conditions met - authentication is failed and false is
returning, as by other check in checkUserPassword
4. LdapManagerTest.java - introduced set of test methods to validate for
existing behavior (with **ldap_allow_empty_pass** = true) and new one
(with **ldap_allow_empty_pass** property is not specified or specified
to false) to check that login is still successful in first case and
failed in the second one.
- Does this need documentation?
- [ ] No.
- [x] Yes. [apache/doris-website/pull/3403]
### Check List (For Reviewer who merge this PR)
- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label <!-- Add branch pick label that this PR
should merge into -->
---------
Co-authored-by: morningman <[email protected]>
Co-authored-by: Claude Opus 5 (1M context) <[email protected]>
---
conf/ldap.conf | 13 ++++
.../java/org/apache/doris/common/LdapConfig.java | 10 +++
.../doris/mysql/authenticate/ldap/LdapManager.java | 9 +++
.../mysql/authenticate/ldap/LdapManagerTest.java | 77 ++++++++++++++++++++++
4 files changed, 109 insertions(+)
diff --git a/conf/ldap.conf b/conf/ldap.conf
index 00647819273..39a73737164 100644
--- a/conf/ldap.conf
+++ b/conf/ldap.conf
@@ -50,6 +50,19 @@ ldap_group_basedn = ou=group,dc=domain,dc=com
## ldap_use_ssl - use secured connection to LDAP server if required (disabled
by default). Note: When enabling SSL, ensure ldap_port is set appropriately
(typically 636 for LDAPS instead of 389 for LDAP).
# ldap_use_ssl = false
+## ldap_allow_empty_pass - allow LDAP users to log in with an empty password.
Disabled by default.
+##
+## LDAP treats a bind with a non-empty DN and an empty password as an
unauthenticated bind and
+## normally reports it as successful, so with this enabled anyone who knows a
valid LDAP user
+## name can log in to Doris without a password. Keep it false unless you must
restore the
+## legacy behaviour; setting it to true reopens that hole.
+##
+## Changing this requires an FE restart (the config is not runtime-mutable).
+##
+## This setting applies to the legacy LDAP authentication path only. The
plugin-based LDAP
+## authentication (the fe-authentication LDAP plugin) always rejects empty
passwords.
+# ldap_allow_empty_pass = false
+
# LDAP pool configuration
#
https://docs.spring.io/spring-ldap/docs/2.3.3.RELEASE/reference/#pool-configuration
# ldap_pool_max_active = 8
diff --git a/fe/fe-common/src/main/java/org/apache/doris/common/LdapConfig.java
b/fe/fe-common/src/main/java/org/apache/doris/common/LdapConfig.java
index 82966af525b..6756f2ec43e 100644
--- a/fe/fe-common/src/main/java/org/apache/doris/common/LdapConfig.java
+++ b/fe/fe-common/src/main/java/org/apache/doris/common/LdapConfig.java
@@ -199,6 +199,16 @@ public class LdapConfig extends ConfigBase {
@ConfigBase.ConfField
public static boolean ldap_use_ssl = false;
+ /**
+ * Allow LDAP users to log in with an empty password. Disabled by default:
LDAP reports a bind
+ * with an empty password as a successful unauthenticated bind, so
enabling this lets anyone
+ * who knows a valid LDAP user name log in without a password. Applies to
the legacy LDAP
+ * authentication path only; the fe-authentication LDAP plugin always
rejects empty passwords.
+ * Not runtime-mutable - changing it requires an FE restart.
+ */
+ @ConfigBase.ConfField
+ public static boolean ldap_allow_empty_pass = false;
+
/**
* The method constructs the correct URL connection string for the
specified host and port depending on
* the value of the {@code ldap_use_ssl} property.
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/mysql/authenticate/ldap/LdapManager.java
b/fe/fe-core/src/main/java/org/apache/doris/mysql/authenticate/ldap/LdapManager.java
index 2e1e1a26ecb..07a2c9837ed 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/mysql/authenticate/ldap/LdapManager.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/mysql/authenticate/ldap/LdapManager.java
@@ -129,6 +129,15 @@ public class LdapManager {
|| Objects.isNull(passwd)) {
return false;
}
+
+ // Reject an empty password here, before the cached-password
comparison and the LDAP bind
+ // below: an empty password would otherwise reach the server as an
unauthenticated bind,
+ // which LDAP reports as success. Opt out with ldap_allow_empty_pass =
true.
+ if (passwd.isEmpty() && !LdapConfig.ldap_allow_empty_pass) {
+ LOG.warn("Rejected LDAP login with empty password, user={},
ldapAllowEmptyPass=false", fullName);
+ return false;
+ }
+
LdapUserInfo ldapUserInfo = getUserInfo(fullName);
if (Objects.isNull(ldapUserInfo) || !ldapUserInfo.isExists()) {
long elapsed = System.currentTimeMillis() - start;
diff --git
a/fe/fe-core/src/test/java/org/apache/doris/mysql/authenticate/ldap/LdapManagerTest.java
b/fe/fe-core/src/test/java/org/apache/doris/mysql/authenticate/ldap/LdapManagerTest.java
index 64fffd2c71d..afe9d034b57 100644
---
a/fe/fe-core/src/test/java/org/apache/doris/mysql/authenticate/ldap/LdapManagerTest.java
+++
b/fe/fe-core/src/test/java/org/apache/doris/mysql/authenticate/ldap/LdapManagerTest.java
@@ -24,6 +24,7 @@ import org.apache.doris.common.jmockit.Deencapsulation;
import org.apache.doris.mysql.privilege.Auth;
import org.apache.doris.mysql.privilege.Role;
+import org.junit.After;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
@@ -49,6 +50,11 @@ public class LdapManagerTest {
LdapConfig.ldap_default_roles = new String[0];
}
+ @After
+ public void tearDown() {
+ LdapConfig.ldap_allow_empty_pass = false;
+ }
+
private void mockClient(boolean userExist, boolean passwd) {
mockClient(userExist, passwd, new ArrayList<>());
}
@@ -108,6 +114,77 @@ public class LdapManagerTest {
Assert.assertFalse(ldapManager.checkUserPasswd(USER2, "123"));
}
+ @Test
+ public void testCheckUserEmptyPasswdAllowed() throws Exception {
+ //test checks - that user with empty ldap password can login with
ldap_allow_empty_pass = true
+ LdapConfig.ldap_allow_empty_pass = true;
+ LdapManager ldapManager = new LdapManager();
+ Deencapsulation.setField(ldapManager, "ldapClient", ldapClient);
+ mockClient(true, true);
+ Assert.assertTrue(ldapManager.checkUserPasswd(USER1, ""));
+ LdapUserInfo ldapUserInfo = ldapManager.getUserInfo(USER1);
+ Assert.assertNotNull(ldapUserInfo);
+ Assert.assertTrue(ldapUserInfo.isSetPasswd());
+ Assert.assertEquals("", ldapUserInfo.getPasswd());
+ }
+
+ @Test
+ public void testCheckUserEmptyPasswdDisabled() throws Exception {
+ //test checks - that login with empty ldap password is prohibited by
default
+ //corresponding property is set to false - so login with empty
password is not allowed
+ //if password is not empty - user can login as usual
+ LdapManager ldapManager = new LdapManager();
+ Deencapsulation.setField(ldapManager, "ldapClient", ldapClient);
+ mockClient(true, true);
+ Assert.assertFalse(ldapManager.checkUserPasswd(USER1, ""));
+
+ Assert.assertTrue(ldapManager.checkUserPasswd(USER1, "123"));
+ LdapUserInfo ldapUserInfo = ldapManager.getUserInfo(USER1);
+ Assert.assertNotNull(ldapUserInfo);
+ Assert.assertTrue(ldapUserInfo.isSetPasswd());
+ Assert.assertEquals("123", ldapUserInfo.getPasswd());
+ }
+
+ @Test
+ public void testCachedEmptyPasswordIsRejectedAfterFlagDisabled() {
+ LdapConfig.ldap_allow_empty_pass = true;
+ LdapManager ldapManager = new LdapManager();
+ Deencapsulation.setField(ldapManager, "ldapClient", ldapClient);
+ mockClient(true, true);
+ //empty password succeeds and gets cached while the flag is still
enabled.
+ Assert.assertTrue(ldapManager.checkUserPasswd(USER1, ""));
+ Assert.assertEquals("", ldapManager.getUserInfo(USER1).getPasswd());
+
+ //once disabled, the cached entry must not short-circuit the new check
+ LdapConfig.ldap_allow_empty_pass = false;
+ Assert.assertFalse(ldapManager.checkUserPasswd(USER1, ""));
+ //a non-empty password still authenticates against the same cached
entry
+ Assert.assertTrue(ldapManager.checkUserPasswd(USER1, "123"));
+ }
+
+ @Test
+ public void testEmptyPasswordIsRejectedBeforeCacheLookup() throws
Exception {
+ //the empty password check must run before getUserInfo(), which is
what keeps a cached
+ //empty password from short-circuiting the check and letting the login
through
+ LdapManager ldapManager = new LdapManager();
+ Deencapsulation.setField(ldapManager, "ldapClient", ldapClient);
+ mockClient(true, true);
+
+ LdapManager spyManager = Mockito.spy(ldapManager);
+ Assert.assertFalse(spyManager.checkUserPasswd(USER1, ""));
+ Mockito.verify(spyManager, Mockito.times(0)).getUserInfo(USER1);
+ }
+
+ @Test
+ public void testCheckUserNullPasswd() throws Exception {
+ //test check existing feature that user with null ldap password can't
login in any case
+ //because this is first check in checkUserPasswd() method
+ LdapManager ldapManager = new LdapManager();
+ Deencapsulation.setField(ldapManager, "ldapClient", ldapClient);
+ mockClient(true, true);
+ Assert.assertFalse(ldapManager.checkUserPasswd(USER1, null));
+ }
+
@Test
public void testGetUserInfoWithLdapDefaultRolesWithoutLdapGroups() {
LdapManager ldapManager = new LdapManager();
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
