Skip to content

Commit e556799

Browse files
author
Vladimir Kotal
committed
better LDAP query error reporting
1 parent e173e36 commit e556799

File tree

3 files changed

+22
-4
lines changed

3 files changed

+22
-4
lines changed

plugins/src/main/java/opengrok/auth/plugin/LdapUserPlugin.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -166,8 +166,8 @@ public void fillSession(HttpServletRequest req, User user) {
166166
if ((res = getLdapProvider().lookupLdapContent(dn, expandedFilter,
167167
attributes.toArray(new String[0]))) == null) {
168168
LOGGER.log(Level.WARNING, "failed to get LDAP attributes ''{2}'' for user {0} " +
169-
"with filter ''{1}''",
170-
new Object[]{user, expandedFilter, attributes});
169+
"with filter ''{1}'' from LDAP provider {3}",
170+
new Object[]{user, expandedFilter, attributes, getLdapProvider()});
171171
return;
172172
}
173173

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

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,8 @@ public LdapFacade(Configuration cfg) {
176176
setInterval(cfg.getInterval());
177177
setSearchBase(cfg.getSearchBase());
178178
setWebHooks(cfg.getWebHooks());
179+
180+
// Anti-pattern: do some non trivial stuff in the constructor.
179181
prepareSearchControls(cfg.getSearchTimeout(), cfg.getCountLimit());
180182
prepareServers();
181183
}
@@ -425,7 +427,8 @@ private <T> T processResult(SearchResult result, AttributeMapper<T> mapper) thro
425427
}
426428

427429
public String toString() {
428-
return String.join(",",
429-
getServers().stream().map(LdapServer::getUrl).collect(Collectors.toList()));
430+
return "{servers=" + String.join(",",
431+
getServers().stream().map(LdapServer::getUrl).collect(Collectors.toList())) +
432+
", searchBase=" + getSearchBase() + "}";
430433
}
431434
}

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

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@
77

88
import javax.naming.directory.SearchControls;
99

10+
import java.util.ArrayList;
11+
import java.util.Arrays;
1012
import java.util.Collections;
1113

1214
import static org.junit.Assert.assertEquals;
@@ -36,4 +38,17 @@ public void testConnectTimeoutInheritance() {
3638
LdapFacade facade = new LdapFacade(config);
3739
assertTrue(facade.getServers().stream().anyMatch(s -> s.getConnectTimeout() == timeoutValue));
3840
}
41+
42+
@Test
43+
public void testToString() {
44+
Configuration config = new Configuration();
45+
config.setServers(Arrays.asList(new LdapServer("http://foo.foo"),
46+
new LdapServer("http://bar.bar")));
47+
config.setSearchBase("dc=foo,dc=com");
48+
int timeoutValue = 42;
49+
config.setConnectTimeout(timeoutValue);
50+
LdapFacade facade = new LdapFacade(config);
51+
assertEquals("{servers=http://foo.foo,http://bar.bar, searchBase=dc=foo,dc=com}",
52+
facade.toString());
53+
}
3954
}

0 commit comments

Comments
 (0)