Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions conf/ldap.conf
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,9 @@ 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 to connect to ldap with empty pass (enabled by default)
# ldap_allow_empty_pass = true

# LDAP pool configuration
# https://docs.spring.io/spring-ldap/docs/2.3.3.RELEASE/reference/#pool-configuration
# ldap_pool_max_active = 8
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,14 @@ under the License.
<version>6.0.7</version>
<scope>test</scope>
</dependency>

<!-- reference for fe-common for compatibility integration tests for PR #61440 -->
<dependency>
<groupId>org.apache.doris</groupId>
<artifactId>fe-common</artifactId>
<version>${project.version}</version>
<scope>test</scope>
</dependency>
</dependencies>

<build>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import org.apache.doris.authentication.AuthenticationResult;
import org.apache.doris.authentication.CredentialType;
import org.apache.doris.authentication.Principal;
import org.apache.doris.common.LdapConfig;
Comment thread
iaorekhov-1980 marked this conversation as resolved.

import com.unboundid.ldap.listener.InMemoryDirectoryServer;
import com.unboundid.ldap.listener.InMemoryDirectoryServerConfig;
Expand All @@ -30,6 +31,7 @@
import com.unboundid.ldif.LDIFException;
import com.unboundid.ldif.LDIFReader;
import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.BeforeEach;
Expand Down Expand Up @@ -111,8 +113,17 @@ static void stopLdapServer() {
}
}

@AfterEach
void tearDown() {
//running test with specified value - ldap_allow_empty_pass is be true
LdapConfig.ldap_allow_empty_pass = true;
}

@BeforeEach
void setUp() {
//running test with specified value - ldap_allow_empty_pass is be true
LdapConfig.ldap_allow_empty_pass = true;

plugin = new LdapAuthenticationPlugin();

// Create integration configuration pointing to embedded LDAP
Expand Down Expand Up @@ -220,8 +231,43 @@ void testAuthenticateNonExistentUser() throws Exception {
}

@Test
@DisplayName("Should reject empty password")
void testAuthenticateEmptyPassword() throws Exception {
@DisplayName("Should reject empty password with default value of ldap_allow_empty_pass")
void testAuthenticateEmptyPasswordAndAllowEmptyPassDefault() throws Exception {
//running test with default value - ldap_allow_empty_pass should be true
LdapConfig.ldap_allow_empty_pass = true;
AuthenticationRequest request = AuthenticationRequest.builder()
.username("alice")
.credentialType(CredentialType.CLEAR_TEXT_PASSWORD)
.credential("".getBytes(StandardCharsets.UTF_8))
.build();

AuthenticationResult result = plugin.authenticate(request, integration);

Assertions.assertTrue(result.isFailure());
}

@Test
@DisplayName("Should reject empty password with true value of ldap_allow_empty_pass")
void testAuthenticateEmptyPasswordAndAllowEmptyPassTrue() throws Exception {
//running test with obvious value - ldap_allow_empty_pass is true
LdapConfig.ldap_allow_empty_pass = true;
AuthenticationRequest request = AuthenticationRequest.builder()
.username("alice")
.credentialType(CredentialType.CLEAR_TEXT_PASSWORD)
.credential("".getBytes(StandardCharsets.UTF_8))
.build();

AuthenticationResult result = plugin.authenticate(request, integration);

Assertions.assertTrue(result.isFailure());
}

@Test
@DisplayName("Should reject empty password with false value of ldap_allow_empty_pass")
void testAuthenticateEmptyPasswordAndAllowEmptyPassFalse() throws Exception {
//running test with obvious value - ldap_allow_empty_pass is false
LdapConfig.ldap_allow_empty_pass = false;

AuthenticationRequest request = AuthenticationRequest.builder()
.username("alice")
.credentialType(CredentialType.CLEAR_TEXT_PASSWORD)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1235,7 +1235,10 @@ public enum ErrorCode {
ERR_NO_CLUSTER_ERROR(5099, new byte[]{'4', '2', '0', '0', '0'}, "No compute group (cloud cluster) selected"),

ERR_NOT_CLOUD_MODE(6000, new byte[]{'4', '2', '0', '0', '0'},
"Command only support in cloud mode.");
"Command only support in cloud mode."),

ERR_EMPTY_PASSWORD(6001, new byte[]{'H', 'Y', '0', '0', '0'},
"Access with empty password is prohibited for LDAP user '%s'. Set ldap_allow_empty_pass=true to allow.");

Comment thread
iaorekhov-1980 marked this conversation as resolved.
// This is error code
private final int code;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -210,4 +210,10 @@ public class LdapConfig extends ConfigBase {
public static String getConnectionURL(String hostPortInAccessibleFormat) {
return ((LdapConfig.ldap_use_ssl ? "ldaps" : "ldap") + "://" + hostPortInAccessibleFormat);
}

/**
* Flag to enable login with empty pass.
*/
@ConfigBase.ConfField
public static boolean ldap_allow_empty_pass = true;
Comment thread
iaorekhov-1980 marked this conversation as resolved.
Comment thread
iaorekhov-1980 marked this conversation as resolved.
Comment thread
iaorekhov-1980 marked this conversation as resolved.
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import org.apache.doris.catalog.Env;
import org.apache.doris.common.ErrorCode;
import org.apache.doris.common.ErrorReport;
import org.apache.doris.common.LdapConfig;
import org.apache.doris.mysql.authenticate.AuthenticateRequest;
import org.apache.doris.mysql.authenticate.AuthenticateResponse;
import org.apache.doris.mysql.authenticate.Authenticator;
Expand Down Expand Up @@ -94,7 +95,7 @@ public boolean canDeal(String qualifiedUser) {

/**
* The LDAP authentication process is as follows:
* step1: Check the LDAP password.
* step1: Check the LDAP password (if ldap_allow_empty_pass is false login with empty pass is prohibited).
* step2: Get the LDAP groups privileges as a role, saved into ConnectContext.
* step3: Set current userIdentity. If the user account does not exist in Doris, login as a temporary user.
* Otherwise, login to the Doris account.
Expand All @@ -107,11 +108,17 @@ private AuthenticateResponse internalAuthenticate(String password, String qualif
}

// check user password by ldap server.
// extra check for login with empty LDAP password was added in checkUserPasswd.
try {
if (!Env.getCurrentEnv().getAuth().getLdapManager().checkUserPasswd(qualifiedUser, password)) {
if (LOG.isDebugEnabled()) {
LOG.debug("internalAuthenticate: user={}, success=false", userName);
}
// extra log to identify case covered by PR 61440 - login with empty LDAP password is not allowed
if (Strings.isNullOrEmpty(password) && !LdapConfig.ldap_allow_empty_pass) {
LOG.info(ErrorCode.ERR_EMPTY_PASSWORD, username +"@" + remoteIp);
}

ErrorReport.report(ErrorCode.ERR_ACCESS_DENIED_ERROR, qualifiedUser, remoteIp, usePasswd);
return AuthenticateResponse.failedResponse;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,13 +122,30 @@ public boolean doesUserExist(String fullName) {
return exists;
}

//not allow to login in case when empty password is specified but such mode is disabled by configuration
public boolean checkLoginWithEmptyPasswordForLdapIsAllowed(String fullName, String passwd) {
if (Strings.isNullOrEmpty(passwd) && !LdapConfig.ldap_allow_empty_pass) {
LOG.info("User:{} login rejected: empty LDAP password is prohibited (ldap_allow_empty_pass=false)",
fullName);
return false;
} else {
return true;
}
}

public boolean checkUserPasswd(String fullName, String passwd) {
long start = System.currentTimeMillis();
String userName = fullName;
if (AuthenticateType.getAuthTypeConfig() != AuthenticateType.LDAP || Strings.isNullOrEmpty(userName)
|| Objects.isNull(passwd)) {
return false;
}

// extra check for PR 61440 to disable login with empty LDAP password in case when specific property is true
if (!checkLoginWithEmptyPasswordForLdapIsAllowed(fullName, passwd)) {
return false;
}

LdapUserInfo ldapUserInfo = getUserInfo(fullName);
if (Objects.isNull(ldapUserInfo) || !ldapUserInfo.isExists()) {
long elapsed = System.currentTimeMillis() - start;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
import org.apache.doris.common.ErrorCode;
import org.apache.doris.common.ErrorReport;
import org.apache.doris.common.FeConstants;
import org.apache.doris.common.LdapConfig;
import org.apache.doris.common.Pair;
import org.apache.doris.common.PatternMatcherException;
import org.apache.doris.common.UserException;
Expand Down Expand Up @@ -237,6 +238,10 @@ public void checkPlainPassword(String remoteUser, String remoteHost, String remo
// Check the LDAP password when the user exists in the LDAP service.
if (ldapManager.doesUserExist(remoteUser)) {
if (!ldapManager.checkUserPasswd(remoteUser, remotePasswd, remoteHost, currentUser)) {
// extra log to identify case covered by PR 61440 - login with empty LDAP password is not allowed
if (Strings.isNullOrEmpty(remotePasswd) && !LdapConfig.ldap_allow_empty_pass) {
LOG.info(ErrorCode.ERR_EMPTY_PASSWORD, remoteUser +"@" + remoteHost);
}
throw new AuthenticationException(ErrorCode.ERR_ACCESS_DENIED_ERROR, remoteUser + "@" + remoteHost,
Strings.isNullOrEmpty(remotePasswd) ? "NO" : "YES");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
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;
Expand All @@ -47,6 +48,12 @@ public class LdapManagerTest {
public void setUp() {
Config.authentication_type = "ldap";
LdapConfig.ldap_default_roles = new String[0];
LdapConfig.ldap_allow_empty_pass = true;
}

@After
public void tearDown() {
LdapConfig.ldap_allow_empty_pass = true;
}

private void mockClient(boolean userExist, boolean passwd) {
Expand Down Expand Up @@ -108,6 +115,32 @@ public void testCheckUserPasswd() {
Assert.assertFalse(ldapManager.checkUserPasswd(USER2, "123"));
}

@Test
public void testUserLoginWithEmptyLDAPPasswordDefault() {
LdapManager ldapManager = new LdapManager();
Assert.assertTrue(ldapManager.checkLoginWithEmptyPasswordForLdapIsAllowed("username", null));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Exercise the production empty-password chokepoint

These new tests call the helper directly, so they still pass if its invocation is deleted from LdapManager.checkUserPasswd or moved after the cached-password/LDAP-bind success paths. The existing manager test uses only nonempty passwords, LdapAuthenticatorTest mocks checkUserPasswd, and no Auth.checkPlainPassword test is changed despite the PR description claiming both caller suites. Please test an empty credential through checkUserPasswd with cached/client success under both flag values, assert the disabled case returns before those success paths, and cover both callers. Those caller assertions should distinguish a flag-disabled policy rejection from a flag-enabled empty-bind failure and keep LDAP-present/nonempty failures message-neutral with LDAP-absent/native failures.

Assert.assertTrue(ldapManager.checkLoginWithEmptyPasswordForLdapIsAllowed("username", ""));
Assert.assertTrue(ldapManager.checkLoginWithEmptyPasswordForLdapIsAllowed("username", "password"));
}

@Test
public void testUserLoginWithEmptyLDAPPasswordEnabled() {
LdapManager ldapManager = new LdapManager();
LdapConfig.ldap_allow_empty_pass = true;
Assert.assertTrue(ldapManager.checkLoginWithEmptyPasswordForLdapIsAllowed("username", null));
Assert.assertTrue(ldapManager.checkLoginWithEmptyPasswordForLdapIsAllowed("username", ""));
Assert.assertTrue(ldapManager.checkLoginWithEmptyPasswordForLdapIsAllowed("username", "password"));
}

@Test
public void testUserLoginWithEmptyLDAPPasswordDisabled() {
LdapManager ldapManager = new LdapManager();
LdapConfig.ldap_allow_empty_pass = false;
Assert.assertFalse(ldapManager.checkLoginWithEmptyPasswordForLdapIsAllowed("username", null));
Assert.assertFalse(ldapManager.checkLoginWithEmptyPasswordForLdapIsAllowed("username", ""));
Assert.assertTrue(ldapManager.checkLoginWithEmptyPasswordForLdapIsAllowed("username", "password"));
}

@Test
public void testGetUserInfoWithLdapDefaultRolesWithoutLdapGroups() {
LdapManager ldapManager = new LdapManager();
Expand Down
Empty file added git
Empty file.
Loading