Skip to content

Commit e975089

Browse files
author
Vladimir Kotal
committed
change LdapUserPlugin user lookup to avoid using full username dn,
rather extract just common name and match using filter with objectclass.
1 parent 45ee255 commit e975089

File tree

6 files changed

+214
-15
lines changed

6 files changed

+214
-15
lines changed

‎plugins/LdapPlugin/src/opengrok/auth/plugin/AbstractLdapPlugin.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ abstract public class AbstractLdapPlugin implements IAuthorizationPlugin {
6060
*/
6161
public static long nextId = 1;
6262

63-
private static final String CONFIGURATION_PARAM = "configuration";
63+
protected static final String CONFIGURATION_PARAM = "configuration";
6464
protected static final String FAKE_PARAM = "fake";
6565

6666
protected String SESSION_USERNAME = "opengrok-group-plugin-username";

‎plugins/LdapPlugin/src/opengrok/auth/plugin/LdapUserPlugin.java

Lines changed: 80 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,10 @@
2424

2525
import java.util.Map;
2626
import java.util.Set;
27+
import java.util.logging.Level;
28+
import java.util.logging.Logger;
29+
import java.util.regex.Matcher;
30+
import java.util.regex.Pattern;
2731
import javax.servlet.http.HttpServletRequest;
2832
import opengrok.auth.entity.LdapUser;
2933
import opengrok.auth.plugin.entity.User;
@@ -37,8 +41,46 @@
3741
*/
3842
public class LdapUserPlugin extends AbstractLdapPlugin {
3943

44+
private static final Logger LOGGER = Logger.getLogger(LdapUserPlugin.class.getName());
45+
4046
public static final String SESSION_ATTR = "opengrok-ldap-plugin-user";
47+
protected static final String OBJECT_CLASS = "objectclass";
48+
49+
private String objectClass;
4150

51+
private boolean isAlphanumeric(String str) {
52+
for (int i = 0; i < str.length(); i++) {
53+
char c = str.charAt(i);
54+
if (!Character.isDigit(c) && !Character.isLetter(c)) {
55+
return false;
56+
}
57+
}
58+
59+
return true;
60+
}
61+
62+
private Pattern usernameCnPattern = null;
63+
64+
@Override
65+
public void load(Map<String, Object> parameters) {
66+
super.load(parameters);
67+
68+
if ((objectClass = (String) parameters.get(OBJECT_CLASS)) == null) {
69+
throw new NullPointerException("Missing param [" + OBJECT_CLASS +
70+
"] in the setup");
71+
}
72+
73+
if (!isAlphanumeric(objectClass)) {
74+
throw new NullPointerException("object class '" + objectClass +
75+
"' contains non-alphanumeric characters");
76+
}
77+
78+
usernameCnPattern = Pattern.compile("(cn=[a-zA-Z_]+)");
79+
80+
LOGGER.log(Level.FINE, "LdapUser plugin loaded with objectclass={0}",
81+
objectClass);
82+
}
83+
4284
/**
4385
* Check if the session exists and contains all necessary fields required by
4486
* this plug-in.
@@ -52,26 +94,59 @@ protected boolean sessionExists(HttpServletRequest req) {
5294
&& req.getSession().getAttribute(SESSION_ATTR) != null;
5395
}
5496

97+
protected String getFilter(User user) {
98+
String filter = null;
99+
String commonName;
100+
101+
Matcher matcher = usernameCnPattern.matcher(user.getUsername());
102+
if (matcher.find()) {
103+
commonName = matcher.group(1);
104+
LOGGER.log(Level.FINEST, "extracted common name {0} from {1}",
105+
new Object[]{commonName, user.getUsername()});
106+
} else {
107+
LOGGER.log(Level.WARNING, "cannot get common name out of {0}",
108+
user.getUsername());
109+
return filter;
110+
}
111+
112+
filter = "(&(objectclass=" + this.objectClass + ")(" + commonName + "))";
113+
114+
return filter;
115+
}
116+
55117
@Override
56118
public void fillSession(HttpServletRequest req, User user) {
57119
Map<String, Set<String>> records;
120+
58121
updateSession(req, null);
59122

60123
if (getLdapProvider() == null) {
61124
return;
62125
}
63126

64-
if ((records = getLdapProvider().lookupLdapContent(user, new String[]{"uid", "mail", "ou"})) == null) {
127+
String filter = getFilter(user);
128+
if ((records = getLdapProvider().lookupLdapContent(null, filter,
129+
new String[]{"uid", "mail", "ou"})) == null) {
130+
LOGGER.log(Level.FINER, "failed to get LDAP contents for user '{0}' with filter '{1}'",
131+
new Object[]{user, filter});
132+
return;
133+
}
134+
135+
if (records.isEmpty()) {
136+
LOGGER.log(Level.FINER, "LDAP records for user {0} are empty",
137+
user);
65138
return;
66139
}
67140

68-
if (records.isEmpty()
69-
|| !records.containsKey("uid")
70-
|| !records.containsKey("mail")) {
141+
if (!records.containsKey("uid") || records.get("uid").isEmpty()) {
142+
LOGGER.log(Level.FINER, "uid record for user {0} is not present or empty",
143+
user);
71144
return;
72145
}
73146

74-
if (records.get("uid").isEmpty() || records.get("mail").isEmpty()) {
147+
if (!records.containsKey("mail") || records.get("mail").isEmpty()) {
148+
LOGGER.log(Level.FINER, "mail record for user {0} is not present or empty",
149+
user);
75150
return;
76151
}
77152

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

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ public class LdapFacade extends AbstractLdapProvider {
4848
private static final Logger LOGGER = Logger.getLogger(LdapFacade.class.getName());
4949

5050
/**
51-
* LDAP filter.
51+
* default LDAP filter
5252
*/
5353
private static final String LDAP_FILTER = "objectclass=*";
5454

@@ -237,11 +237,9 @@ public boolean isConfigured() {
237237
/**
238238
* Lookups the authorization values {
239239
*
240-
*
241-
*
242-
* @param user the osso headers
243-
* @param filter LDAP filter to use
244-
* @param values match these LDAP value
240+
* @param user user information. If @{code null} then search base will be used.
241+
* @param filter LDAP filter to use. If @{code null} then @{link LDAP_FILTER} will be used.
242+
* @param values match these LDAP values
245243
*
246244
* @return set of strings describing the user's attributes
247245
* @see #LDAP_VALUES
@@ -289,7 +287,7 @@ private <T> T lookup(String dn, String filter, String[] attributes, AttributeMap
289287
* @param mapper mapper class implementing @code{AttributeMapper} closed
290288
* @param fail current count of failures
291289
*
292-
* @return results transformed with mapper
290+
* @return results transformed with mapper or {@code null} on failure
293291
*/
294292
private <T> T lookup(String dn, String filter, String[] attributes, AttributeMapper<T> mapper, int fail) {
295293

‎plugins/LdapPlugin/test/opengrok/auth/plugin/LdapUserPluginTest.java

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
/*
2+
* CDDL HEADER START
3+
*
4+
* The contents of this file are subject to the terms of the
5+
* Common Development and Distribution License (the "License").
6+
* You may not use this file except in compliance with the License.
7+
*
8+
* See LICENSE.txt included in this distribution for the specific
9+
* language governing permissions and limitations under the License.
10+
*
11+
* When distributing Covered Code, include this CDDL HEADER in each
12+
* file and include the License file at LICENSE.txt.
13+
* If applicable, add the following below this CDDL HEADER, with the
14+
* fields enclosed by brackets "[]" replaced with your own identifying
15+
* information: Portions Copyright [yyyy] [name of copyright owner]
16+
*
17+
* CDDL HEADER END
18+
*/
19+
20+
/*
21+
* Copyright (c) 2017, Oracle and/or its affiliates. All rights reserved.
22+
*/
23+
package opengrok.auth.plugin;
24+
25+
import java.util.Map;
26+
import java.util.TreeMap;
27+
import opengrok.auth.plugin.entity.User;
28+
import org.junit.Assert;
29+
import org.junit.Before;
30+
import org.junit.Test;
31+
32+
/**
33+
*
34+
* @author Vladimir Kotal
35+
*/
36+
public class LdapUserPluginTest {
37+
private LdapUserPlugin plugin;
38+
39+
@Before
40+
public void setUp() {
41+
plugin = new LdapUserPlugin();
42+
}
43+
44+
private Map<String, Object> getParamsMap() {
45+
Map<String, Object> params = new TreeMap<>();
46+
params.put(AbstractLdapPlugin.CONFIGURATION_PARAM,
47+
getClass().getResource("config.xml").getFile());
48+
49+
return params;
50+
}
51+
52+
@Test
53+
public void loadTestNegative1() {
54+
Map<String, Object> params = getParamsMap();
55+
params.put("foo", (Object)"bar");
56+
try {
57+
plugin.load(params);
58+
Assert.fail("should have caused exception");
59+
} catch (NullPointerException e) {
60+
}
61+
}
62+
63+
@Test
64+
public void loadTestNegative2() {
65+
Map<String, Object> params = getParamsMap();
66+
params.put(LdapUserPlugin.OBJECT_CLASS, (Object)"#$@");
67+
try {
68+
plugin.load(params);
69+
Assert.fail("should have caused exception");
70+
} catch (NullPointerException e) {
71+
}
72+
}
73+
74+
@Test
75+
public void loadTestPostitive() {
76+
Map<String, Object> params = getParamsMap();
77+
params.put(LdapUserPlugin.OBJECT_CLASS, (Object)"posixUser");
78+
try {
79+
plugin.load(params);
80+
} catch (NullPointerException e) {
81+
Assert.fail("should not cause exception");
82+
}
83+
}
84+
85+
@Test
86+
public void getFilterTest1() {
87+
Map<String, Object> params = getParamsMap();
88+
String cl = "posixUser";
89+
params.put(LdapUserPlugin.OBJECT_CLASS, (Object) cl);
90+
plugin.load(params);
91+
String cn = "cn=foo";
92+
User user = new User(cn + ",l=EMEA,dc=foobar,dc=com", "id", null, false);
93+
String filter = plugin.getFilter(user);
94+
Assert.assertEquals("(&(" + LdapUserPlugin.OBJECT_CLASS + "=" + cl + ")(" + cn + "))",
95+
filter);
96+
}
97+
}

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

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
<java version="1.8.0_65" class="java.beans.XMLDecoder">
3+
<object class="opengrok.auth.plugin.configuration.Configuration">
4+
<void property="interval">
5+
<int>100</int>
6+
</void>
7+
<void property="searchBase">
8+
<string>dc=foobar,dc=com</string>
9+
</void>
10+
<void property="servers">
11+
<void method="add">
12+
<object class="opengrok.auth.plugin.ldap.LdapServer">
13+
<void property="name">
14+
<string>ldap://ldap.foobar.com</string>
15+
</void>
16+
<void property="timeout">
17+
<int>10</int>
18+
</void>
19+
</object>
20+
</void>
21+
</void>
22+
</object>
23+
</java>

‎plugins/README.md

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,12 @@ indexer via the -R option.
9191
<string>REQUISITE</string>
9292
</void>
9393
</object>
94+
<void property="setup">
95+
<void method="put">
96+
<string>objectclass</string>
97+
<string>posixAccount</string>
98+
</void>
99+
</void>
94100
</void>
95101

96102
<!-- Authorization stacks follow -->
@@ -116,7 +122,7 @@ indexer via the -R option.
116122
<void method="add">
117123
<object class="org.opensolaris.opengrok.authorization.AuthorizationPlugin">
118124
<void property="name">
119-
<string>opengrok.auth.plugin.LdapAttr</string>
125+
<string>opengrok.auth.plugin.LdapAttrPlugin</string>
120126
</void>
121127
<void property="flag">
122128
<string>SUFFICIENT</string>
@@ -136,7 +142,7 @@ indexer via the -R option.
136142
<void method="add">
137143
<object class="org.opensolaris.opengrok.authorization.AuthorizationPlugin">
138144
<void property="name">
139-
<string>opengrok.auth.plugin.LdapFilter</string>
145+
<string>opengrok.auth.plugin.LdapFilterPlugin</string>
140146
</void>
141147
<void property="flag">
142148
<string>REQUIRED</string>

0 commit comments

Comments
 (0)