Skip to content

Commit 59a60c2

Browse files
added test for secure / insecure protocol
1 parent 87df608 commit 59a60c2

4 files changed

Lines changed: 30 additions & 6 deletions

File tree

conf/ldap.conf

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,9 +42,10 @@ ldap_user_basedn = ou=people,dc=domain,dc=com
4242
ldap_user_filter = (&(uid={login}))
4343
ldap_group_basedn = ou=group,dc=domain,dc=com
4444

45+
# ldap_user_cache_timeout_s = 5 * 60;
46+
4547
## ldap_use_ssl - use secured connection to LDAP server if required (disabled by default)
4648
# ldap_use_ssl = false
47-
# ldap_user_cache_timeout_s = 5 * 60;
4849

4950
# LDAP pool configuration
5051
# https://docs.spring.io/spring-ldap/docs/2.3.3.RELEASE/reference/#pool-configuration

fe/fe-common/src/main/java/org/apache/doris/common/LdapConfig.java

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,6 @@ public class LdapConfig extends ConfigBase {
161161
/**
162162
* Flag to enable usage of LDAPS.
163163
*/
164-
@Deprecated
165164
@ConfigBase.ConfField
166165
public static boolean ldap_use_ssl = false;
167166
}

fe/fe-core/src/main/java/org/apache/doris/mysql/authenticate/ldap/LdapClient.java

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -65,8 +65,7 @@ public ClientInfo(String ldapPassword) {
6565

6666
private void setLdapTemplateNoPool(String ldapPassword) {
6767
LdapContextSource contextSource = new LdapContextSource();
68-
String url = (LdapConfig.ldap_use_ssl ? "ldaps://" : "ldap://") + NetUtils
69-
.getHostPortInAccessibleFormat(LdapConfig.ldap_host, LdapConfig.ldap_port);
68+
String url = this.getURL();
7069

7170
contextSource.setUrl(url);
7271
contextSource.setUserDn(LdapConfig.ldap_admin_name);
@@ -78,8 +77,7 @@ private void setLdapTemplateNoPool(String ldapPassword) {
7877

7978
private void setLdapTemplatePool(String ldapPassword) {
8079
LdapContextSource contextSource = new LdapContextSource();
81-
String url = (LdapConfig.ldap_use_ssl ? "ldaps://" : "ldap://") + NetUtils
82-
.getHostPortInAccessibleFormat(LdapConfig.ldap_host, LdapConfig.ldap_port);
80+
String url = this.getURL();
8381

8482
contextSource.setUrl(url);
8583
contextSource.setUserDn(LdapConfig.ldap_admin_name);
@@ -108,6 +106,11 @@ private void setLdapTemplatePool(String ldapPassword) {
108106
public boolean checkUpdate(String ldapPassword) {
109107
return this.ldapPassword == null || !this.ldapPassword.equals(ldapPassword);
110108
}
109+
110+
public String getURL() {
111+
String url = (LdapConfig.ldap_use_ssl ? "ldaps" : "ldap") + "://" + NetUtils
112+
.getHostPortInAccessibleFormat(LdapConfig.ldap_host, LdapConfig.ldap_port);
113+
}
111114
}
112115

113116
private void init() {

fe/fe-core/src/test/java/org/apache/doris/mysql/authenticate/ldap/LdapClientTest.java

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,4 +95,25 @@ public void testGetGroups() {
9595
};
9696
Assert.assertEquals(1, ldapClient.getGroups("zhangsan").size());
9797
}
98+
99+
@Test
100+
public void testSecuredProtocolIsUsed() {
101+
//testing default case with not specified property ldap_use_ssl or it is specified as false
102+
String insecureUrl = ldapClient.getURL();
103+
Assert.assertNotNull("connection URL should not be null", insecureUrl);
104+
Assert.assertTrue("with ldap_use_ssl connection = false or not specified URL should start with ldap, but received: " + insecureUrl,
105+
insecureUrl.startsWith("ldap://"));
106+
107+
//testing new case with specified property ldap_use_ssl as true
108+
LdapConfig.ldap_use_ssl = true;
109+
String secureUrl = ldapClient.getURL();
110+
Assert.assertNotNull("connection URL should not be null", secureUrl);
111+
Assert.assertTrue("with ldap_use_ssl = true URL connection should start with ldaps, but received: " + secureUrl,
112+
secureUrl.startsWith("ldaps://"));
113+
}
114+
115+
@After
116+
public void tearDown() {
117+
LdapConfig.ldap_use_ssl = false; // restoring default value for other tests
118+
}
98119
}

0 commit comments

Comments
 (0)