Skip to content

Commit

Permalink
GRP-1588: Ldap Groups - "Entry already exists" errors when groups fou…
Browse files Browse the repository at this point in the history 
…nd by DN/EntryDn/DistinguishedName

Added automatic recovery to PSPNG when bulk searching for groups fails. This can particularly happen
when singleGroupSearchFilter is configured to look for groups by DN because DNs can be esacped
and the necessary in-memory group search does not work with escaped DNs.

To avoid the warnings (see below) and duplicate ldap searches, this commit adds an enableBulkGroupSearching config
property that disables the bulk searching and always does slower-but-simpler, single-group searches.

WARN  Bulk fetch failed (returned unmatchable group data). This can be caused by searching for a DN with escaping or by singleGroupSearchFilter ... that are not included in groupSearchAttributes
WARN  Slower fetching will be attempted
  • Loading branch information
@bafbl
bafbl committed Aug 9, 2017
1 parent 21d4f3a commit 3caabb1
Show file tree
Hide file tree
Showing 3 changed files with 121 additions and 62 deletions.
165 changes: 106 additions & 59 deletions ...uper-pspng/src/main/java/edu/internet2/middleware/grouper/pspng/LdapGroupProvisioner.java
View file
Expand Up @@ -363,73 +363,120 @@ protected Map<GrouperGroupInfo, LdapGroup> fetchTargetSystemGroups(
// If this is a full-sync provisioner, then we want to make sure we get the member attribute of the
// group so we see all members.
String[] returnAttributes = getLdapAttributesToFetch();

StringBuilder combinedLdapFilter = new StringBuilder();

// Start the combined ldap filter as an OR-query
combinedLdapFilter.append("(|");

for ( GrouperGroupInfo grouperGroup : grouperGroupsToFetch ) {
SearchFilter f = getGroupLdapFilter(grouperGroup);
String groupFilterString = f.format();

// Wrap the subject's filter in (...) if it doesn't start with (
if ( groupFilterString.startsWith("(") )
combinedLdapFilter.append(groupFilterString);
else
combinedLdapFilter.append('(').append(groupFilterString).append(')');
}
combinedLdapFilter.append(')');

// Actually do the search
List<LdapObject> searchResult;

LOG.debug("{}: Searching for {} groups with:: {}",
new Object[]{getName(), grouperGroupsToFetch.size(), combinedLdapFilter});

try {
searchResult = getLdapSystem().performLdapSearchRequest(
new SearchRequest(config.getGroupSearchBaseDn(),
combinedLdapFilter.toString(),
returnAttributes));
}
catch (PspException e) {
LOG.error("Problem fetching groups with filter '{}' on base '{}'",
new Object[]{combinedLdapFilter, config.getGroupSearchBaseDn(), e});
throw e;
if ( config.isBulkGroupSearchingEnabled() ) {
StringBuilder combinedLdapFilter = new StringBuilder();

// Start the combined ldap filter as an OR-query
combinedLdapFilter.append("(|");

for (GrouperGroupInfo grouperGroup : grouperGroupsToFetch) {
SearchFilter f = getGroupLdapFilter(grouperGroup);
String groupFilterString = f.format();

// Wrap the subject's filter in (...) if it doesn't start with (
if (groupFilterString.startsWith("("))
combinedLdapFilter.append(groupFilterString);
else
combinedLdapFilter.append('(').append(groupFilterString).append(')');
}
combinedLdapFilter.append(')');

// Actually do the search
List<LdapObject> searchResult;

LOG.debug("{}: Searching for {} groups with:: {}",
new Object[]{getName(), grouperGroupsToFetch.size(), combinedLdapFilter});

try {
searchResult = getLdapSystem().performLdapSearchRequest(
new SearchRequest(config.getGroupSearchBaseDn(),
combinedLdapFilter.toString(),
returnAttributes));
} catch (PspException e) {
LOG.error("Problem fetching groups with filter '{}' on base '{}'",
new Object[]{combinedLdapFilter, config.getGroupSearchBaseDn(), e});
throw e;
}

LOG.debug("{}: Group search returned {} groups", getName(), searchResult.size());

// Now we have a bag of LdapObjects, but we don't know which goes with which grouperGroup.
// We're going to go through the Grouper Groups and their filters and compare
// them to the Ldap data we've fetched into memory.
Map<GrouperGroupInfo, LdapGroup> result = new HashMap<GrouperGroupInfo, LdapGroup>();

Set<LdapObject> matchedFetchResults = new HashSet<LdapObject>();

// For every group we tried to bulk fetch, find the matching LdapObject that came back
for (GrouperGroupInfo groupToFetch : grouperGroupsToFetch) {
SearchFilter f = getGroupLdapFilter(groupToFetch);

for (LdapObject aFetchedLdapObject : searchResult) {
if (aFetchedLdapObject.matchesLdapFilter(f)) {
result.put(groupToFetch, new LdapGroup(aFetchedLdapObject));
matchedFetchResults.add(aFetchedLdapObject);
break;
}
}
}

Set<LdapObject> unmatchedFetchResults = new HashSet<LdapObject>(searchResult);
unmatchedFetchResults.removeAll(matchedFetchResults);

for (LdapObject unmatchedFetchResult : unmatchedFetchResults) {
LOG.warn("{}: Bulk fetch failed (returned unmatchable group data). "
+ "This can be caused by searching for a DN with escaping or by singleGroupSearchFilter ({}) that are not included "
+ "in groupSearchAttributes ({})?): {}",
new Object[]{getName(), config.getSingleGroupSearchFilter(), config.getGroupSearchAttributes(), unmatchedFetchResult.getDn()});
LOG.warn("{}: Slower fetching will be attempted", getName());
}

// We're done if everything matched up
if ( unmatchedFetchResults.size() == 0 ) {
return result;
}
else {
// Fall through to the one-by-one group searching below. This is slower, but doesn't require the
// result-matching step that just failed
}
}

LOG.debug("{}: Group search returned {} groups", getName(), searchResult.size());

// Now we have a bag of LdapObjects, but we don't know which goes with which grouperGroup.
// We're going to go through the Grouper Groups and their filters and compare
// them to the Ldap data we've fetched into memory.
// Do simple ldap searching
Map<GrouperGroupInfo, LdapGroup> result = new HashMap<GrouperGroupInfo, LdapGroup>();

Set<LdapObject> matchedFetchResults = new HashSet<LdapObject>();

// For every group we tried to bulk fetch, find the matching LdapObject that came back
for ( GrouperGroupInfo groupToFetch : grouperGroupsToFetch ) {
SearchFilter f = getGroupLdapFilter(groupToFetch);

for ( LdapObject aFetchedLdapObject : searchResult ) {
if ( aFetchedLdapObject.matchesLdapFilter(f) ) {
result.put(groupToFetch, new LdapGroup(aFetchedLdapObject));
matchedFetchResults.add(aFetchedLdapObject);
break;

for (GrouperGroupInfo grouperGroup : grouperGroupsToFetch) {
SearchFilter groupLdapFilter = getGroupLdapFilter(grouperGroup);
try {
LOG.debug("{}: Searching for group {} with:: {}",
new Object[]{getName(), grouperGroup, groupLdapFilter});

// Actually do the search
List<LdapObject> searchResult = getLdapSystem().performLdapSearchRequest(
new SearchRequest(config.getGroupSearchBaseDn(),
groupLdapFilter,
returnAttributes));

if (searchResult.size() == 1) {
LdapObject ldapObject = searchResult.iterator().next();
LOG.debug("{}: Group search returned {}", getName(), ldapObject.getDn());
result.put(grouperGroup, new LdapGroup(ldapObject));
}
else if ( searchResult.size() > 1 ){
LOG.error("{}: Search for group {} with '{}' returned multiple matches: {}",
new Object[]{getName(), grouperGroup, groupLdapFilter, searchResult});
throw new PspException("Search for ldap group returned multiple matches");
}
else if ( searchResult.size() == 0 ) {
// No match found ==> result will not include an entry for this grouperGroup
}
} catch (PspException e) {
LOG.error("{}: Problem fetching group with filter '{}' on base '{}'",
new Object[]{getName(), groupLdapFilter, config.getGroupSearchBaseDn(), e});
throw e;
}
}

Set<LdapObject> unmatchedFetchResults = new HashSet<LdapObject>(searchResult);
unmatchedFetchResults.removeAll(matchedFetchResults);

for ( LdapObject unmatchedFetchResult : unmatchedFetchResults )
LOG.error("{}: Group data from ldap server was not matched with a grouper group "
+ "(perhaps attributes are used in singleGroupSearchFilter ({}) that are not included "
+ "in groupSearchAttributes ({})?): {}",
new Object[] {getName(), config.getSingleGroupSearchFilter(), config.getGroupSearchAttributes(), unmatchedFetchResult.getDn()});

return result;
}

Expand Down
16 changes: 14 additions & 2 deletions ...c/main/java/edu/internet2/middleware/grouper/pspng/LdapGroupProvisionerConfiguration.java
View file
Expand Up @@ -110,6 +110,12 @@ public class LdapGroupProvisionerConfiguration extends LdapProvisionerConfigurat
// Should comparisons of the memberAttributeName be case sensitive?
private boolean memberAttributeIsCaseSensitive;
protected boolean memberAttributeIsCaseSensitive_defaultValue = false;

// Should bulk searching for groups be enabled (where ldap searches are ORed together
// and then rematched in memory)? This speeds up several operations, but sometimes fails
// when in-memory matching is done differently than on the actual ldap server
private boolean enableBulkGroupSearching;
protected boolean enableBulkGroupSearching_defaultValue = true;

public LdapGroupProvisionerConfiguration(String provisionerName) {
super(provisionerName);
Expand Down Expand Up @@ -185,7 +191,11 @@ public void readConfiguration() {
memberAttributeIsCaseSensitive =
GrouperLoaderConfig.retrieveConfig().propertyValueBoolean(qualifiedParameterNamespace + "memberAttributeIsCaseSensitive", memberAttributeIsCaseSensitive_defaultValue);
LOG.debug("Ldap Group Provisioner {} - Setting memberAttributeIsCaseSensitive to {}", provisionerName, memberAttributeIsCaseSensitive);
}

enableBulkGroupSearching =
GrouperLoaderConfig.retrieveConfig().propertyValueBoolean(qualifiedParameterNamespace + "enableBulkGroupSearching", enableBulkGroupSearching_defaultValue);
LOG.debug("Ldap Group Provisioner {} - Setting enableBulkGroupSearching to {}", provisionerName, enableBulkGroupSearching);
}


public String getMemberAttributeName() {
Expand Down Expand Up @@ -252,7 +262,9 @@ public int getLdapGroupCacheSize() {
public boolean isMemberAttributeCaseSensitive() {
return memberAttributeIsCaseSensitive;
}


public boolean isBulkGroupSearchingEnabled() { return enableBulkGroupSearching; }

public void populateElMap(Map<String, Object> variableMap) {
super.populateElMap(variableMap);
variableMap.put("groupSearchBaseDn", getGroupSearchBaseDn());
Expand Down
2 changes: 1 addition & 1 deletion ...-misc/grouper-pspng/src/main/java/edu/internet2/middleware/grouper/pspng/Provisioner.java
View file
Expand Up @@ -1359,7 +1359,7 @@ public void provisionBatchOfItems(List<ProvisioningWorkItem> allWorkItems) {
MDC.put("step", "filter/");
try {
filteredWorkItems = filterWorkItems(allWorkItems);
LOG.info("{}: {} work items need to be processed futher", getName(), filteredWorkItems.size());
LOG.info("{}: {} work items need to be processed further", getName(), filteredWorkItems.size());
}
catch (PspException e) {
LOG.error("Unable to filter the provisioning batch", e);
Expand Down

0 comments on commit 3caabb1

Please sign in to comment.