Skip to content

Commit 7f50307

Browse files
author
Vladimir Kotal
committed
add LDAP tunables
fixes #2741 fixes #2742
1 parent 806e34b commit 7f50307

File tree

7 files changed

+136
-37
lines changed

7 files changed

+136
-37
lines changed

‎plugins/sample-ldap-plugin-config.xml

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,16 +4,25 @@
44
<void property="interval">
55
<int>900000</int>
66
</void>
7+
<void property="searchTimeout">
8+
<int>500</int>
9+
</void>
710
<void property="searchBase">
811
<string>dc=foobar,dc=com</string>
912
</void>
13+
<void property="connectTimeout">
14+
<int>5000</int>
15+
</void>
16+
<void property="countLimit">
17+
<int>100</int>
18+
</void>
1019
<void property="servers">
1120
<void method="add">
1221
<object class="opengrok.auth.plugin.ldap.LdapServer">
1322
<void property="name">
1423
<string>ldap://ldap1.foobar.com</string>
1524
</void>
16-
<void property="timeout">
25+
<void property="connectTimeout">
1726
<int>5000</int>
1827
</void>
1928
</object>
@@ -23,9 +32,6 @@
2332
<void property="name">
2433
<string>ldap://ldap2.foobar.com</string>
2534
</void>
26-
<void property="timeout">
27-
<int>5000</int>
28-
</void>
2935
</object>
3036
</void>
3137
</void>

‎plugins/src/opengrok/auth/plugin/configuration/Configuration.java

Lines changed: 33 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,17 +33,23 @@
3333
import java.io.IOException;
3434
import java.io.InputStream;
3535
import java.io.OutputStream;
36+
import java.io.Serializable;
3637
import java.util.ArrayList;
3738
import java.util.List;
3839
import opengrok.auth.plugin.ldap.LdapServer;
3940
import opengrok.auth.plugin.util.Hooks;
4041

41-
public class Configuration {
42+
public class Configuration implements Serializable {
43+
44+
private static final long serialVersionUID = -1;
4245

4346
private List<LdapServer> servers = new ArrayList<>();
4447
private int interval;
4548
private String searchBase;
4649
private Hooks hooks;
50+
private int searchTimeout;
51+
private int connectTimeout;
52+
private int countLimit;
4753

4854
public void setServers(List<LdapServer> servers) {
4955
this.servers = servers;
@@ -69,6 +75,30 @@ public void setInterval(int interval) {
6975
this.interval = interval;
7076
}
7177

78+
public int getSearchTimeout() {
79+
return this.searchTimeout;
80+
}
81+
82+
public void setSearchTimeout(int timeout) {
83+
this.searchTimeout = timeout;
84+
}
85+
86+
public int getConnectTimeout() {
87+
return this.connectTimeout;
88+
}
89+
90+
public void setConnectTimeout(int timeout) {
91+
this.connectTimeout = timeout;
92+
}
93+
94+
public int getCountLimit() {
95+
return this.countLimit;
96+
}
97+
98+
public void setCountLimit(int limit) {
99+
this.countLimit = limit;
100+
}
101+
72102
public String getSearchBase() {
73103
return searchBase;
74104
}
@@ -90,7 +120,7 @@ private void encodeObject(OutputStream out) {
90120
}
91121

92122
/**
93-
* Read a configuration from a file in xml format.
123+
* Read a configuration from a file in XML format.
94124
*
95125
* @param file input file
96126
* @return the new configuration object
@@ -124,7 +154,7 @@ private static Configuration decodeObject(InputStream in) throws IOException {
124154
}
125155

126156
if (!(ret instanceof Configuration)) {
127-
throw new IOException("Not a valid config file");
157+
throw new IOException("Not a valid configuration file");
128158
}
129159

130160
return (Configuration) ret;

‎plugins/src/opengrok/auth/plugin/ldap/LdapFacade.java

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -58,11 +58,13 @@ public class LdapFacade extends AbstractLdapProvider {
5858
private static final String LDAP_FILTER = "objectclass=*";
5959

6060
/**
61-
* Timeout for retrieving the results.
61+
* default timeout for retrieving the results
6262
*/
63-
private static final int LDAP_TIMEOUT = 5000; // ms
63+
private static final int LDAP_SEARCH_TIMEOUT = 5000; // ms
6464

6565
/**
66+
* default limit of result traversal
67+
*
6668
* @see
6769
* <a href="https://docs.oracle.com/javase/7/docs/api/javax/naming/directory/SearchControls.html#setCountLimit%28long%29">SearchControls</a>
6870
*
@@ -178,11 +180,11 @@ public Map<String, Set<String>> mapFromAttributes(Attributes attrs) throws Namin
178180
};
179181

180182
public LdapFacade(Configuration cfg) {
181-
setServers(cfg.getServers());
183+
setServers(cfg.getServers(), cfg.getConnectTimeout());
182184
setInterval(cfg.getInterval());
183185
setSearchBase(cfg.getSearchBase());
184186
setHooks(cfg.getHooks());
185-
prepareSearchControls();
187+
prepareSearchControls(cfg.getSearchTimeout(), cfg.getCountLimit());
186188
prepareServers();
187189
}
188190

@@ -218,8 +220,14 @@ public List<LdapServer> getServers() {
218220
return servers;
219221
}
220222

221-
public LdapFacade setServers(List<LdapServer> servers) {
223+
public LdapFacade setServers(List<LdapServer> servers, int timeout) {
222224
this.servers = servers;
225+
// Inherit connect timeout from server pool configuration.
226+
for (LdapServer server : servers) {
227+
if (server.getConnectTimeout() == 0 && timeout != 0) {
228+
server.setConnectTimeout(timeout);
229+
}
230+
}
223231
return this;
224232
}
225233

@@ -267,9 +275,16 @@ public Map<String, Set<String>> lookupLdapContent(User user, String filter, Stri
267275
new ContentAttributeMapper(values));
268276
}
269277

270-
private SearchControls prepareSearchControls() {
278+
private SearchControls prepareSearchControls(int ldapTimeout, int ldapCountLimit) {
271279
controls = new SearchControls();
272280
controls.setSearchScope(SearchControls.SUBTREE_SCOPE);
281+
controls.setTimeLimit(ldapTimeout > 0 ? ldapTimeout : LDAP_SEARCH_TIMEOUT);
282+
controls.setCountLimit(ldapCountLimit > 0 ? ldapCountLimit : LDAP_COUNT_LIMIT);
283+
284+
return controls;
285+
}
286+
287+
public SearchControls getSearchControls() {
273288
return controls;
274289
}
275290

‎plugins/src/opengrok/auth/plugin/ldap/LdapServer.java

Lines changed: 29 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
*/
2323
package opengrok.auth.plugin.ldap;
2424

25+
import java.io.Serializable;
2526
import java.util.Hashtable;
2627
import java.util.logging.Level;
2728
import java.util.logging.Logger;
@@ -34,21 +35,23 @@
3435
import javax.naming.ldap.InitialLdapContext;
3536
import javax.naming.ldap.LdapContext;
3637

37-
public class LdapServer {
38+
public class LdapServer implements Serializable {
39+
40+
private static final long serialVersionUID = -1;
3841

3942
private static final Logger LOGGER = Logger.getLogger(LdapServer.class.getName());
4043

41-
private static final String LDAP_TIMEOUT_PARAMETER = "com.sun.jndi.ldap.connect.timeout";
44+
private static final String LDAP_TIMEOUT_PARAMETER = "com.sun.jndi.ldap.connect.connectTimeout";
4245
private static final String LDAP_CONTEXT_FACTORY = "com.sun.jndi.ldap.LdapCtxFactory";
4346
/**
44-
* Timeout for connecting.
47+
* default connectTimeout for connecting
4548
*/
46-
private static final int LDAP_TIMEOUT = 5000; // ms
49+
private static final int LDAP_CONNECT_TIMEOUT = 5000; // ms
4750

4851
private String url;
4952
private String username;
5053
private String password;
51-
private int timeout;
54+
private int connectTimeout;
5255
private int interval = 10 * 1000;
5356

5457
private Hashtable<String, String> env;
@@ -95,12 +98,12 @@ public LdapServer setPassword(String password) {
9598
return this;
9699
}
97100

98-
public int getTimeout() {
99-
return timeout;
101+
public int getConnectTimeout() {
102+
return connectTimeout;
100103
}
101104

102-
public LdapServer setTimeout(int timeout) {
103-
this.timeout = timeout;
105+
public LdapServer setConnectTimeout(int connectTimeout) {
106+
this.connectTimeout = connectTimeout;
104107
return this;
105108
}
106109

@@ -140,19 +143,19 @@ private synchronized LdapContext connect() {
140143
}
141144

142145
if (ctx == null) {
143-
try {
144-
env.put(Context.PROVIDER_URL, this.url);
145-
146-
if (this.username != null) {
147-
env.put(Context.SECURITY_PRINCIPAL, this.username);
148-
}
149-
if (this.password != null) {
150-
env.put(Context.SECURITY_CREDENTIALS, this.password);
151-
}
152-
if (this.timeout > 0) {
153-
env.put(LDAP_TIMEOUT_PARAMETER, Integer.toString(this.timeout));
154-
}
146+
env.put(Context.PROVIDER_URL, this.url);
147+
148+
if (this.username != null) {
149+
env.put(Context.SECURITY_PRINCIPAL, this.username);
150+
}
151+
if (this.password != null) {
152+
env.put(Context.SECURITY_CREDENTIALS, this.password);
153+
}
154+
if (this.connectTimeout > 0) {
155+
env.put(LDAP_TIMEOUT_PARAMETER, Integer.toString(this.connectTimeout));
156+
}
155157

158+
try {
156159
ctx = new InitialLdapContext(env, null);
157160
ctx.reconnect(null);
158161
ctx.setRequestControls(null);
@@ -165,6 +168,7 @@ private synchronized LdapContext connect() {
165168
return ctx = null;
166169
}
167170
}
171+
168172
return ctx;
169173
}
170174

@@ -231,16 +235,18 @@ public synchronized void close() {
231235
try {
232236
ctx.close();
233237
} catch (NamingException ex) {
238+
LOGGER.log(Level.WARNING, "cannot close LDAP server {0}", getUrl());
234239
}
235240
ctx = null;
236241
}
237242
}
238243

239244
private static Hashtable<String, String> prepareEnv() {
240245
Hashtable<String, String> e = new Hashtable<String, String>();
246+
241247
e.put(Context.INITIAL_CONTEXT_FACTORY, LDAP_CONTEXT_FACTORY);
242-
e.put(LDAP_TIMEOUT_PARAMETER, Integer.toString(LDAP_TIMEOUT));
248+
e.put(LDAP_TIMEOUT_PARAMETER, Integer.toString(LDAP_CONNECT_TIMEOUT));
249+
243250
return e;
244251
}
245-
246252
}

‎plugins/test/opengrok/auth/plugin/LdapFacadeTest.java

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
package opengrok.auth.plugin;
2+
3+
import opengrok.auth.plugin.configuration.Configuration;
4+
import opengrok.auth.plugin.ldap.LdapFacade;
5+
import opengrok.auth.plugin.ldap.LdapServer;
6+
import org.junit.Test;
7+
8+
import javax.naming.directory.SearchControls;
9+
10+
import java.util.Collections;
11+
12+
import static org.junit.Assert.assertEquals;
13+
import static org.junit.Assert.assertTrue;
14+
15+
public class LdapFacadeTest {
16+
@Test
17+
public void testSearchControlsConfig() {
18+
Configuration config = new Configuration();
19+
int searchTimeout = 1234;
20+
config.setSearchTimeout(searchTimeout);
21+
int countLimit = 32;
22+
config.setCountLimit(countLimit);
23+
24+
LdapFacade facade = new LdapFacade(config);
25+
SearchControls controls = facade.getSearchControls();
26+
assertEquals(searchTimeout, controls.getTimeLimit());
27+
assertEquals(countLimit, controls.getCountLimit());
28+
}
29+
30+
@Test
31+
public void testConnectTimeoutInheritance() {
32+
Configuration config = new Configuration();
33+
config.setServers(Collections.singletonList(new LdapServer("http://foo.bar")));
34+
int timeoutValue = 42;
35+
config.setConnectTimeout(timeoutValue);
36+
LdapFacade facade = new LdapFacade(config);
37+
assertTrue(facade.getServers().stream().anyMatch(s -> s.getConnectTimeout() == timeoutValue));
38+
}
39+
}

‎plugins/test/opengrok/auth/plugin/config.xml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
<void property="name">
1414
<string>ldap://ldap.foobar.com</string>
1515
</void>
16-
<void property="timeout">
16+
<void property="connectTimeout">
1717
<int>10</int>
1818
</void>
1919
</object>

‎plugins/test/opengrok/auth/plugin/configuration/ConfigurationTest.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,9 @@ public void exceptionThrown(Exception e) {
6363

6464
Configuration configuration1 = new Configuration();
6565
configuration1.setInterval(500);
66+
configuration1.setSearchTimeout(1000);
67+
configuration1.setConnectTimeout(42);
68+
configuration1.setCountLimit(10);
6669
configuration1.setServers(new ArrayList<>(Arrays.asList(new LdapServer("http://server.com"))));
6770
Hooks hooks = new Hooks();
6871
Hook hook = new Hook();

0 commit comments

Comments
 (0)