Skip to content

Commit ac14522

Browse files
Daan HooglandDaanHoogland
Daan Hoogland
authored and
DaanHoogland
committed
created an ad specific usertype filter
1 parent 613beaf commit ac14522

File tree

3 files changed

+77
-85
lines changed

3 files changed

+77
-85
lines changed

‎plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/ldap/ADLdapUserManagerImpl.java‎

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ public List<LdapUser> getUsersInGroup(String groupName, LdapContext context, Lon
4949
searchControls.setReturningAttributes(_ldapConfiguration.getReturnAttributes(domainId));
5050

5151
NamingEnumeration<SearchResult> results = context.search(basedn, generateADGroupSearchFilter(groupName, domainId), searchControls);
52-
final List<LdapUser> users = new ArrayList<LdapUser>();
52+
final List<LdapUser> users = new ArrayList<>();
5353
while (results.hasMoreElements()) {
5454
final SearchResult result = results.nextElement();
5555
users.add(createUser(result, domainId));
@@ -58,12 +58,8 @@ public List<LdapUser> getUsersInGroup(String groupName, LdapContext context, Lon
5858
}
5959

6060
String generateADGroupSearchFilter(String groupName, Long domainId) {
61-
final String isPersonFilter = "(objectCategory=person)";
6261

63-
final StringBuilder userObjectFilter = new StringBuilder();
64-
userObjectFilter.append("(objectClass=");
65-
userObjectFilter.append(_ldapConfiguration.getUserObject(domainId));
66-
userObjectFilter.append(")");
62+
final StringBuilder userObjectFilter = getUserObjectFilter(domainId);
6763

6864
final StringBuilder memberOfFilter = new StringBuilder();
6965
String groupCnName = _ldapConfiguration.getCommonNameAttribute() + "=" +groupName + "," + _ldapConfiguration.getBaseDn(domainId);
@@ -73,15 +69,22 @@ String generateADGroupSearchFilter(String groupName, Long domainId) {
7369

7470
final StringBuilder result = new StringBuilder();
7571
result.append("(&");
76-
result.append(isPersonFilter);
7772
result.append(userObjectFilter);
7873
result.append(memberOfFilter);
7974
result.append(")");
8075

81-
logger.debug("group search filter = " + result);
76+
logger.debug("group search filter = {}", result);
8277
return result.toString();
8378
}
8479

80+
StringBuilder getUserObjectFilter(Long domainId) {
81+
final StringBuilder userObjectFilter = new StringBuilder();
82+
userObjectFilter.append("(&(objectCategory=person)");
83+
userObjectFilter.append(super.getUserObjectFilter(domainId));
84+
userObjectFilter.append(")");
85+
return userObjectFilter;
86+
}
87+
8588
protected boolean isUserDisabled(SearchResult result) throws NamingException {
8689
boolean isDisabledUser = false;
8790
String userAccountControl = LdapUtils.getAttributeValue(result.getAttributes(), _ldapConfiguration.getUserAccountControlAttribute());

‎plugins/user-authenticators/ldap/src/main/java/org/apache/cloudstack/ldap/OpenLdapUserManagerImpl.java‎

Lines changed: 64 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -75,23 +75,15 @@ protected LdapUser createUser(final SearchResult result, Long domainId) throws N
7575
}
7676

7777
private String generateSearchFilter(final String username, Long domainId) {
78-
final StringBuilder userObjectFilter = new StringBuilder();
79-
userObjectFilter.append("(objectClass=");
80-
userObjectFilter.append(_ldapConfiguration.getUserObject(domainId));
81-
userObjectFilter.append(")");
78+
final StringBuilder userObjectFilter = getUserObjectFilter(domainId);
8279

83-
final StringBuilder usernameFilter = new StringBuilder();
84-
usernameFilter.append("(");
85-
usernameFilter.append(_ldapConfiguration.getUsernameAttribute(domainId));
86-
usernameFilter.append("=");
87-
usernameFilter.append((username == null ? "*" : LdapUtils.escapeLDAPSearchFilter(username)));
88-
usernameFilter.append(")");
80+
final StringBuilder usernameFilter = getUsernameFilter(username, domainId);
8981

9082
String memberOfAttribute = getMemberOfAttribute(domainId);
9183
StringBuilder ldapGroupsFilter = new StringBuilder();
9284
// this should get the trustmaps for this domain
9385
List<String> ldapGroups = getMappedLdapGroups(domainId);
94-
if (null != ldapGroups && ldapGroups.size() > 0) {
86+
if (!ldapGroups.isEmpty()) {
9587
ldapGroupsFilter.append("(|");
9688
for (String ldapGroup : ldapGroups) {
9789
ldapGroupsFilter.append(getMemberOfGroupString(ldapGroup, memberOfAttribute));
@@ -104,21 +96,35 @@ private String generateSearchFilter(final String username, Long domainId) {
10496
if (null != pricipleGroup) {
10597
principleGroupFilter.append(getMemberOfGroupString(pricipleGroup, memberOfAttribute));
10698
}
107-
final StringBuilder result = new StringBuilder();
108-
result.append("(&");
109-
result.append(userObjectFilter);
110-
result.append(usernameFilter);
111-
result.append(ldapGroupsFilter);
112-
result.append(principleGroupFilter);
113-
result.append(")");
114-
115-
String returnString = result.toString();
116-
if (logger.isTraceEnabled()) {
117-
logger.trace("constructed ldap query: " + returnString);
118-
}
99+
100+
String returnString = "(&" +
101+
userObjectFilter +
102+
usernameFilter +
103+
ldapGroupsFilter +
104+
principleGroupFilter +
105+
")";
106+
logger.trace("constructed ldap query: {}", returnString);
119107
return returnString;
120108
}
121109

110+
private StringBuilder getUsernameFilter(String username, Long domainId) {
111+
final StringBuilder usernameFilter = new StringBuilder();
112+
usernameFilter.append("(");
113+
usernameFilter.append(_ldapConfiguration.getUsernameAttribute(domainId));
114+
usernameFilter.append("=");
115+
usernameFilter.append((username == null ? "*" : LdapUtils.escapeLDAPSearchFilter(username)));
116+
usernameFilter.append(")");
117+
return usernameFilter;
118+
}
119+
120+
StringBuilder getUserObjectFilter(Long domainId) {
121+
final StringBuilder userObjectFilter = new StringBuilder();
122+
userObjectFilter.append("(objectClass=");
123+
userObjectFilter.append(_ldapConfiguration.getUserObject(domainId));
124+
userObjectFilter.append(")");
125+
return userObjectFilter;
126+
}
127+
122128
private List<String> getMappedLdapGroups(Long domainId) {
123129
List <String> ldapGroups = new ArrayList<>();
124130
// first get the trustmaps
@@ -134,37 +140,31 @@ private List<String> getMappedLdapGroups(Long domainId) {
134140
private String getMemberOfGroupString(String group, String memberOfAttribute) {
135141
final StringBuilder memberOfFilter = new StringBuilder();
136142
if (null != group) {
137-
if(logger.isDebugEnabled()) {
138-
logger.debug("adding search filter for '" + group +
139-
"', using '" + memberOfAttribute + "'");
140-
}
141-
memberOfFilter.append("(" + memberOfAttribute + "=");
142-
memberOfFilter.append(group);
143-
memberOfFilter.append(")");
143+
logger.debug("adding search filter for '{}', using '{}'", group, memberOfAttribute);
144+
memberOfFilter.append("(")
145+
.append(memberOfAttribute)
146+
.append("=")
147+
.append(group)
148+
.append(")");
144149
}
145150
return memberOfFilter.toString();
146151
}
147152

148153
private String generateGroupSearchFilter(final String groupName, Long domainId) {
149-
final StringBuilder groupObjectFilter = new StringBuilder();
150-
groupObjectFilter.append("(objectClass=");
151-
groupObjectFilter.append(_ldapConfiguration.getGroupObject(domainId));
152-
groupObjectFilter.append(")");
153-
154-
final StringBuilder groupNameFilter = new StringBuilder();
155-
groupNameFilter.append("(");
156-
groupNameFilter.append(_ldapConfiguration.getCommonNameAttribute());
157-
groupNameFilter.append("=");
158-
groupNameFilter.append((groupName == null ? "*" : LdapUtils.escapeLDAPSearchFilter(groupName)));
159-
groupNameFilter.append(")");
160-
161-
final StringBuilder result = new StringBuilder();
162-
result.append("(&");
163-
result.append(groupObjectFilter);
164-
result.append(groupNameFilter);
165-
result.append(")");
166-
167-
return result.toString();
154+
String groupObjectFilter = "(objectClass=" +
155+
_ldapConfiguration.getGroupObject(domainId) +
156+
")";
157+
158+
String groupNameFilter = "(" +
159+
_ldapConfiguration.getCommonNameAttribute() +
160+
"=" +
161+
(groupName == null ? "*" : LdapUtils.escapeLDAPSearchFilter(groupName)) +
162+
")";
163+
164+
return "(&" +
165+
groupObjectFilter +
166+
groupNameFilter +
167+
")";
168168
}
169169

170170
@Override
@@ -186,17 +186,9 @@ public LdapUser getUser(final String username, final String type, final String n
186186
basedn = _ldapConfiguration.getBaseDn(domainId);
187187
}
188188

189-
final StringBuilder userObjectFilter = new StringBuilder();
190-
userObjectFilter.append("(objectClass=");
191-
userObjectFilter.append(_ldapConfiguration.getUserObject(domainId));
192-
userObjectFilter.append(")");
189+
final StringBuilder userObjectFilter = getUserObjectFilter(domainId);
193190

194-
final StringBuilder usernameFilter = new StringBuilder();
195-
usernameFilter.append("(");
196-
usernameFilter.append(_ldapConfiguration.getUsernameAttribute(domainId));
197-
usernameFilter.append("=");
198-
usernameFilter.append((username == null ? "*" : LdapUtils.escapeLDAPSearchFilter(username)));
199-
usernameFilter.append(")");
191+
final StringBuilder usernameFilter = getUsernameFilter(username, domainId);
200192

201193
final StringBuilder memberOfFilter = new StringBuilder();
202194
if ("GROUP".equals(type)) {
@@ -205,18 +197,17 @@ public LdapUser getUser(final String username, final String type, final String n
205197
memberOfFilter.append(")");
206198
}
207199

208-
final StringBuilder searchQuery = new StringBuilder();
209-
searchQuery.append("(&");
210-
searchQuery.append(userObjectFilter);
211-
searchQuery.append(usernameFilter);
212-
searchQuery.append(memberOfFilter);
213-
searchQuery.append(")");
200+
String searchQuery = "(&" +
201+
userObjectFilter +
202+
usernameFilter +
203+
memberOfFilter +
204+
")";
214205

215-
return searchUser(basedn, searchQuery.toString(), context, domainId);
206+
return searchUser(basedn, searchQuery, context, domainId);
216207
}
217208

218209
protected String getMemberOfAttribute(final Long domainId) {
219-
return _ldapConfiguration.getUserMemberOfAttribute(domainId);
210+
return LdapConfiguration.getUserMemberOfAttribute(domainId);
220211
}
221212

222213
@Override
@@ -243,7 +234,7 @@ public List<LdapUser> getUsersInGroup(String groupName, LdapContext context, Lon
243234

244235
NamingEnumeration<SearchResult> result = context.search(_ldapConfiguration.getBaseDn(domainId), generateGroupSearchFilter(groupName, domainId), controls);
245236

246-
final List<LdapUser> users = new ArrayList<LdapUser>();
237+
final List<LdapUser> users = new ArrayList<>();
247238
//Expecting only one result which has all the users
248239
if (result.hasMoreElements()) {
249240
Attribute attribute = result.nextElement().getAttributes().get(attributeName);
@@ -254,7 +245,7 @@ public List<LdapUser> getUsersInGroup(String groupName, LdapContext context, Lon
254245
try{
255246
users.add(getUserForDn(userdn, context, domainId));
256247
} catch (NamingException e){
257-
logger.info("Userdn: " + userdn + " Not Found:: Exception message: " + e.getMessage());
248+
logger.info("Userdn: {} Not Found:: Exception message: {}", userdn, e.getMessage());
258249
}
259250
}
260251
}
@@ -286,17 +277,15 @@ protected boolean isUserDisabled(SearchResult result) throws NamingException {
286277
return false;
287278
}
288279

289-
public LdapUser searchUser(final String basedn, final String searchString, final LdapContext context, Long domainId) throws NamingException, IOException {
280+
public LdapUser searchUser(final String basedn, final String searchString, final LdapContext context, Long domainId) throws NamingException {
290281
final SearchControls searchControls = new SearchControls();
291282

292283
searchControls.setSearchScope(_ldapConfiguration.getScope());
293284
searchControls.setReturningAttributes(_ldapConfiguration.getReturnAttributes(domainId));
294285

295286
NamingEnumeration<SearchResult> results = context.search(basedn, searchString, searchControls);
296-
if(logger.isDebugEnabled()) {
297-
logger.debug("searching user(s) with filter: \"" + searchString + "\"");
298-
}
299-
final List<LdapUser> users = new ArrayList<LdapUser>();
287+
logger.debug("searching user(s) with filter: \"{}\"", searchString);
288+
final List<LdapUser> users = new ArrayList<>();
300289
while (results.hasMoreElements()) {
301290
final SearchResult result = results.nextElement();
302291
users.add(createUser(result, domainId));
@@ -324,7 +313,7 @@ public List<LdapUser> searchUsers(final String username, final LdapContext conte
324313
byte[] cookie = null;
325314
int pageSize = _ldapConfiguration.getLdapPageSize(domainId);
326315
context.setRequestControls(new Control[]{new PagedResultsControl(pageSize, Control.NONCRITICAL)});
327-
final List<LdapUser> users = new ArrayList<LdapUser>();
316+
final List<LdapUser> users = new ArrayList<>();
328317
NamingEnumeration<SearchResult> results;
329318
do {
330319
results = context.search(basedn, generateSearchFilter(username, domainId), searchControls);

‎plugins/user-authenticators/ldap/src/test/java/org/apache/cloudstack/ldap/ADLdapUserManagerImplTest.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ public void testGenerateADSearchFilterWithNestedGroupsEnabled() {
5454
String [] groups = {"dev", "dev-hyd"};
5555
for (String group: groups) {
5656
String result = adLdapUserManager.generateADGroupSearchFilter(group, 1L);
57-
assertTrue(("(&(objectCategory=person)(objectClass=user)(memberOf:1.2.840.113556.1.4.1941:=CN=" + group + ",DC=cloud,DC=citrix,DC=com))").equals(result));
57+
assertTrue(("(&(&(objectCategory=person)(objectClass=user))(memberOf:1.2.840.113556.1.4.1941:=CN=" + group + ",DC=cloud,DC=citrix,DC=com))").equals(result));
5858
}
5959
}
6060

@@ -68,7 +68,7 @@ public void testGenerateADSearchFilterWithNestedGroupsDisabled() {
6868
String [] groups = {"dev", "dev-hyd"};
6969
for (String group: groups) {
7070
String result = adLdapUserManager.generateADGroupSearchFilter(group, 1L);
71-
assertTrue(("(&(objectCategory=person)(objectClass=user)(memberOf=CN=" + group + ",DC=cloud,DC=citrix,DC=com))").equals(result));
71+
assertTrue(("(&(&(objectCategory=person)(objectClass=user))(memberOf=CN=" + group + ",DC=cloud,DC=citrix,DC=com))").equals(result));
7272
}
7373
}
7474

0 commit comments

Comments
 (0)