This is an automated email from the ASF dual-hosted git repository. yiguolei pushed a commit to branch branch-4.2 in repository https://gitbox.apache.org/repos/asf/doris.git
commit 8c7c9b9bf158815ce1ce174a7218802c3de93886 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]
