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 @@ -44,6 +44,9 @@ ldap_group_basedn = ou=group,dc=domain,dc=com

# ldap_user_cache_timeout_s = 5 * 60;

## 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 pool configuration
# https://docs.spring.io/spring-ldap/docs/2.3.3.RELEASE/reference/#pool-configuration
# ldap_pool_max_active = 8
Expand Down
18 changes: 18 additions & 0 deletions fe/fe-common/src/main/java/org/apache/doris/common/LdapConfig.java
Original file line number Diff line number Diff line change
Expand Up @@ -157,4 +157,22 @@ public class LdapConfig extends ConfigBase {
*/
@ConfigBase.ConfField
public static boolean ldap_pool_test_while_idle = true;

/**
* Flag to enable usage of LDAPS.
*/
@ConfigBase.ConfField
public static boolean ldap_use_ssl = 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.
* If {@code ldap_use_ssl} is true, LDAPS is used as the protocol.
* If {@code ldap_use_ssl} is false or not specified, LDAP is used as the protocol.
* @param hostPortInAccessibleFormat the host and port in accessible format (for example, "host:port")
* @return the LDAP or LDAPS connection URL string
*/
public static String getConnectionURL(String hostPortInAccessibleFormat) {
Copy link
Contributor

Choose a reason for hiding this comment

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

where to load/find certificate and CA when ldaps is enabled?

Copy link
Contributor

Choose a reason for hiding this comment

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

pls also update the corresponding docs

Copy link
Author

Choose a reason for hiding this comment

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

Hi. @gavinchou
The information about certificates location is provided within JVM arguments in fe.conf in standard parameter, like below

# For jdk 17, this JAVA_OPTS will be used as default JVM options
JAVA_OPTS_FOR_JDK_17="-Djavax.net.ssl.trustStore=/opt/apache-doris/certs/cacerts -Dfile.encoding=UTF-8 -Djavax.security.auth.useSubjectCredsOnly=false -Xmx8192m -Xms2048m -XX:+HeapDumpOnOutOfMemoryError -XX:HeapDumpPath=$LOG_DIR -Xlog:gc
*,classhisto*=trace:$LOG_DIR/fe.gc.log.$CUR_DATE:time,uptime:filecount=10,filesize=50M --add-opens=java.base/java.nio=ALL-UNNAMED --add-opens java.base/jdk.internal.ref=ALL-UNNAMED"

Also could you please confirm the documentation to be changed.
https://github.com/apache/doris-website/blob/master/docs/admin-manual/auth/authentication/ldap.md
I assume this file should be updated with new information about support of LDAPS?

Copy link
Author

Choose a reason for hiding this comment

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

Hi, @gavinchou
I've update corresponding docs in apache/doris-website#3351
Could you please take a look?

Copy link
Author

Choose a reason for hiding this comment

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

Hi, @gavinchou
Could you please continue review of this PR?

Copy link
Author

Choose a reason for hiding this comment

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

Hello, @gavinchou
Could you please return to my PR as all comments have been solved

Copy link
Author

Choose a reason for hiding this comment

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

Hello, @gavinchou
Could you please return to my PR as all comments have been solved

return ((LdapConfig.ldap_use_ssl ? "ldaps" : "ldap") + "://" + hostPortInAccessibleFormat);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,8 @@ public ClientInfo(String ldapPassword) {

private void setLdapTemplateNoPool(String ldapPassword) {
LdapContextSource contextSource = new LdapContextSource();
String url = "ldap://" + NetUtils
.getHostPortInAccessibleFormat(LdapConfig.ldap_host, LdapConfig.ldap_port);
String url = LdapConfig.getConnectionURL(
NetUtils.getHostPortInAccessibleFormat(LdapConfig.ldap_host, LdapConfig.ldap_port));

contextSource.setUrl(url);
contextSource.setUserDn(LdapConfig.ldap_admin_name);
Expand All @@ -78,8 +78,8 @@ private void setLdapTemplateNoPool(String ldapPassword) {

private void setLdapTemplatePool(String ldapPassword) {
LdapContextSource contextSource = new LdapContextSource();
String url = "ldap://" + NetUtils
.getHostPortInAccessibleFormat(LdapConfig.ldap_host, LdapConfig.ldap_port);
String url = LdapConfig.getConnectionURL(
NetUtils.getHostPortInAccessibleFormat(LdapConfig.ldap_host, LdapConfig.ldap_port));

contextSource.setUrl(url);
contextSource.setUserDn(LdapConfig.ldap_admin_name);
Expand Down Expand Up @@ -108,6 +108,7 @@ private void setLdapTemplatePool(String ldapPassword) {
public boolean checkUpdate(String ldapPassword) {
return this.ldapPassword == null || !this.ldapPassword.equals(ldapPassword);
}

}

private void init() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,11 @@

import org.apache.doris.common.Config;
import org.apache.doris.common.LdapConfig;
import org.apache.doris.common.util.NetUtils;

import mockit.Expectations;
import mockit.Tested;
import org.junit.After;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
Expand All @@ -43,6 +45,7 @@ public void setUp() {
LdapConfig.ldap_user_basedn = "dc=baidu,dc=com";
LdapConfig.ldap_group_basedn = "ou=group,dc=baidu,dc=com";
LdapConfig.ldap_user_filter = "(&(uid={login}))";
LdapConfig.ldap_use_ssl = false;
}

@Test
Expand Down Expand Up @@ -95,4 +98,28 @@ public void testGetGroups() {
};
Assert.assertEquals(1, ldapClient.getGroups("zhangsan").size());
}

@Test
public void testSecuredProtocolIsUsed() {
//testing default case with not specified property ldap_use_ssl or it is specified as false
String insecureUrl = LdapConfig.getConnectionURL(
NetUtils.getHostPortInAccessibleFormat(LdapConfig.ldap_host, LdapConfig.ldap_port));

Assert.assertNotNull("connection URL should not be null", insecureUrl);
Assert.assertTrue("with ldap_use_ssl = false or not specified URL should start with ldap, but received: " + insecureUrl,
insecureUrl.startsWith("ldap://"));

//testing new case with specified property ldap_use_ssl as true
LdapConfig.ldap_use_ssl = true;
String secureUrl = LdapConfig.getConnectionURL(
NetUtils.getHostPortInAccessibleFormat(LdapConfig.ldap_host, LdapConfig.ldap_port));
Assert.assertNotNull("connection URL should not be null", secureUrl);
Assert.assertTrue("with ldap_use_ssl = true URL should start with ldaps, but received: " + secureUrl,
secureUrl.startsWith("ldaps://"));
}

@After
public void tearDown() {
LdapConfig.ldap_use_ssl = false; // restoring default value for other tests
}
}
Loading