This is an automated email from the ASF dual-hosted git repository.
yiguolei pushed a commit to branch branch-4.1
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/branch-4.1 by this push:
new 35791e3b692 branch-4.1: [enhance](auth) introduction of configuration
property to prohibit login with empty LDAP password #61440 (#68098)
35791e3b692 is described below
commit 35791e3b692482b0b2974b758e18374cac56503d
Author: Mingyu Chen (Rayner) <[email protected]>
AuthorDate: Sun Sep 20 14:24:23 2026 +0800
branch-4.1: [enhance](auth) introduction of configuration property to
prohibit login with empty LDAP password #61440 (#68098)
Cherry-picked from #61440 (master commit d715350c96b) to branch-4.1.
### What problem does this PR solve?
Issue Number: close #60353
Related PR: #61440
Problem Summary:
The legacy LDAP authentication path accepted an empty password: LDAP
reports a bind with a non-empty DN and an empty password as a successful
*unauthenticated* bind, so anyone who knew a valid LDAP user name could
log in to Doris without a password.
This adds the `ldap_allow_empty_pass` FE config (`ldap.conf`, default
`false`). `LdapManager.checkUserPasswd` now rejects an empty password up
front, before the cached-password comparison and the LDAP bind, so a
previously cached empty password cannot short-circuit the check either.
Setting `ldap_allow_empty_pass = true` restores the legacy behaviour.
The plugin-based LDAP authentication (fe-authentication LDAP plugin)
already rejects empty passwords and is not affected.
Conflict resolution: only `LdapManagerTest.java` conflicted. branch-4.1
carries two extra tests
(`testCheckUserPasswdCachedPasswdMatchLogsInfoWithoutThreshold`,
`testGetUserInfoLogsInfoWithoutThreshold`) at the position where master
inserted the new tests; the new tests are placed right after
`testCheckUserPasswd()` and the branch tests are left untouched. The
added lines are identical to the master commit.
### Release note
New `ldap_allow_empty_pass` property in `ldap.conf` for the legacy LDAP
authentication path. Login with an empty LDAP password is now rejected
by default; set it to `true` to restore the previous behaviour.
### 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. Empty LDAP passwords are rejected by default on the legacy
LDAP path; `ldap_allow_empty_pass = true` opts back in.
- Does this need documentation?
- [ ] No.
- [x] Yes. https://github.com/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 -->
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-authored-by: iaorekhov-1980 <[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 bfafb0ac686..6ab0398d315 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
@@ -130,6 +130,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 37492c799be..3971d332b4c 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
@@ -26,6 +26,7 @@ import org.apache.doris.mysql.privilege.Auth;
import org.apache.doris.mysql.privilege.Role;
import org.apache.logging.log4j.Level;
+import org.junit.After;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
@@ -51,6 +52,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<>());
}
@@ -110,6 +116,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 testCheckUserPasswdCachedPasswdMatchLogsInfoWithoutThreshold()
{
LdapManager ldapManager = new LdapManager();
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
