Yair Zaslavsky has uploaded a new change for review. Change subject: core: pre processing for search-generated ldap filters ......................................................................
core: pre processing for search-generated ldap filters The search generates a uniform ldap uery in format that contains upper case keys with $ sign before them - for example ($PRINCIPAL_NAME=a...@ovirt.org) In case of open ldap and RHDS, principal name (i.e - user@domain) should not be passed to the ldap server as the ldap query that is being generated uses the "uid" attribute which is based on user name and not UPN. Furthermore, at least some of the versions of RHDS, and open ldap do not contain support for UPN upon out of the box installation. Instead of passing the UPN to the query, it should be split, and the ldap query should be run with a filter that contains only the user part. Change-Id: I35744eb423ca7f553bf7e9b20c72179bca4e6827 Bug-Url: https://bugzilla.redhat.com/991800 Signed-off-by: Yair Zaslavsky <yzasl...@redhat.com> --- A backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/adbroker/LdapFilterSearchEnginePreProcessor.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/adbroker/LdapQueryMetadataFactoryImpl.java A backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/adbroker/NoOpLdapFilterSearchEnginePreProcessor.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/adbroker/SearchQueryFotmatter.java A backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/adbroker/UpnSplitterLdapFilterSearchEnginePreProcessor.java A backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/adbroker/LdapFilterSearchEnginePreProcessorTest.java 6 files changed, 96 insertions(+), 11 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/94/17794/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/adbroker/LdapFilterSearchEnginePreProcessor.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/adbroker/LdapFilterSearchEnginePreProcessor.java new file mode 100644 index 0000000..6622095 --- /dev/null +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/adbroker/LdapFilterSearchEnginePreProcessor.java @@ -0,0 +1,9 @@ +package org.ovirt.engine.core.bll.adbroker; + +/** + * This defines the pre processing operation to be carried out on the filter of an LDAP query issued by the search + * mechanism + */ +public interface LdapFilterSearchEnginePreProcessor { + String preProcess(String filter); +} diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/adbroker/LdapQueryMetadataFactoryImpl.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/adbroker/LdapQueryMetadataFactoryImpl.java index 0409bda..fd5d00c 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/adbroker/LdapQueryMetadataFactoryImpl.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/adbroker/LdapQueryMetadataFactoryImpl.java @@ -17,6 +17,10 @@ private static EnumMap<SearchLangageLDAPTokens, String> dsSearchSyntaxMap; private static EnumMap<SearchLangageLDAPTokens, String> itdsSearchSyntaxMap; private static EnumMap<SearchLangageLDAPTokens, String> openLdapSearchSyntaxMap; + private static LdapFilterSearchEnginePreProcessor noOpFilterSearchEnginePreProcessor = + new NoOpLdapFilterSearchEnginePreProcessor(); + private static LdapFilterSearchEnginePreProcessor upnSplitterLdapFilterSearchEnginePreProcessor = + new UpnSplitterLdapFilterSearchEnginePreProcessor(); @Override public LdapQueryMetadata getLdapQueryMetadata(LdapProviderType providerType, LdapQueryData queryData) { @@ -157,7 +161,7 @@ new ADUserContextMapper(), SearchControls.SUBTREE_SCOPE, ADUserContextMapper.USERS_ATTRIBUTE_FILTER, - new SearchQueryFotmatter(activeDirectorySearchSyntaxMap), + new SearchQueryFotmatter(activeDirectorySearchSyntaxMap, noOpFilterSearchEnginePreProcessor), ADLdapGuidEncoder.getInstance()); adHashMap.put(LdapQueryType.searchUsers, searchUsersMetadata); @@ -167,7 +171,7 @@ new ADGroupContextMapper(), SearchControls.SUBTREE_SCOPE, ADGroupContextMapper.GROUP_ATTRIBUTE_FILTER, - new SearchQueryFotmatter(activeDirectorySearchSyntaxMap), + new SearchQueryFotmatter(activeDirectorySearchSyntaxMap, noOpFilterSearchEnginePreProcessor), ADLdapGuidEncoder.getInstance()); adHashMap.put(LdapQueryType.searchGroups, searchGroupsMetadata); return adHashMap; @@ -253,7 +257,7 @@ new IPAUserContextMapper(), SearchControls.SUBTREE_SCOPE, IPAUserContextMapper.USERS_ATTRIBUTE_FILTER, - new SearchQueryFotmatter(ipaSearchSyntaxMap), + new SearchQueryFotmatter(ipaSearchSyntaxMap, noOpFilterSearchEnginePreProcessor), DefaultLdapGuidEncoder.getInstance()); ipaHashMap.put(LdapQueryType.searchUsers, ipaSearchUsersMetadata); @@ -263,7 +267,7 @@ new IPAGroupContextMapper(), SearchControls.SUBTREE_SCOPE, IPAGroupContextMapper.GROUP_ATTRIBUTE_FILTER, - new SearchQueryFotmatter(ipaSearchSyntaxMap), + new SearchQueryFotmatter(ipaSearchSyntaxMap, noOpFilterSearchEnginePreProcessor), DefaultLdapGuidEncoder.getInstance()); ipaHashMap.put(LdapQueryType.searchGroups, ipaSearchGroupsMetadata); @@ -353,7 +357,7 @@ new RHDSUserContextMapper(), SearchControls.SUBTREE_SCOPE, RHDSUserContextMapper.USERS_ATTRIBUTE_FILTER, - new SearchQueryFotmatter(dsSearchSyntaxMap), + new SearchQueryFotmatter(dsSearchSyntaxMap, upnSplitterLdapFilterSearchEnginePreProcessor), RHDSLdapGuidEncoder.getInstance()); dsHashMap.put(LdapQueryType.searchUsers, rhdsSearchUsersMetadata); @@ -363,7 +367,7 @@ new RHDSGroupContextMapper(), SearchControls.SUBTREE_SCOPE, RHDSGroupContextMapper.GROUP_ATTRIBUTE_FILTER, - new SearchQueryFotmatter(dsSearchSyntaxMap), + new SearchQueryFotmatter(dsSearchSyntaxMap, noOpFilterSearchEnginePreProcessor), RHDSLdapGuidEncoder.getInstance()); dsHashMap.put(LdapQueryType.searchGroups, rhdsSearchGroupsMetadata); @@ -451,7 +455,7 @@ new ITDSUserContextMapper(), SearchControls.SUBTREE_SCOPE, ITDSUserContextMapper.USERS_ATTRIBUTE_FILTER, - new SearchQueryFotmatter(itdsSearchSyntaxMap), + new SearchQueryFotmatter(itdsSearchSyntaxMap, noOpFilterSearchEnginePreProcessor), DefaultLdapGuidEncoder.getInstance()); itdsHashMap.put(LdapQueryType.searchUsers, itdsSearchUsersMetadata); @@ -461,7 +465,7 @@ new ITDSGroupContextMapper(), SearchControls.SUBTREE_SCOPE, ITDSGroupContextMapper.GROUP_ATTRIBUTE_FILTER, - new SearchQueryFotmatter(itdsSearchSyntaxMap), + new SearchQueryFotmatter(itdsSearchSyntaxMap, noOpFilterSearchEnginePreProcessor), DefaultLdapGuidEncoder.getInstance()); itdsHashMap.put(LdapQueryType.searchGroups, itdsSearchGroupsMetadata); @@ -549,7 +553,7 @@ new OpenLdapUserContextMapper(), SearchControls.SUBTREE_SCOPE, OpenLdapUserContextMapper.USERS_ATTRIBUTE_FILTER, - new SearchQueryFotmatter(openLdapSearchSyntaxMap), + new SearchQueryFotmatter(openLdapSearchSyntaxMap, upnSplitterLdapFilterSearchEnginePreProcessor), new DefaultGuidEncoder()); openLdapHashMap.put(LdapQueryType.searchUsers, OpenLdapSearchUsersMetadata); @@ -559,7 +563,7 @@ new OpenLdapGroupContextMapper(), SearchControls.SUBTREE_SCOPE, OpenLdapGroupContextMapper.GROUP_ATTRIBUTE_FILTER, - new SearchQueryFotmatter(openLdapSearchSyntaxMap), + new SearchQueryFotmatter(openLdapSearchSyntaxMap, noOpFilterSearchEnginePreProcessor), new DefaultGuidEncoder()); openLdapHashMap.put(LdapQueryType.searchGroups, OpenLdapSearchGroupsMetadata); diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/adbroker/NoOpLdapFilterSearchEnginePreProcessor.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/adbroker/NoOpLdapFilterSearchEnginePreProcessor.java new file mode 100644 index 0000000..cc89adf --- /dev/null +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/adbroker/NoOpLdapFilterSearchEnginePreProcessor.java @@ -0,0 +1,10 @@ +package org.ovirt.engine.core.bll.adbroker; + +public class NoOpLdapFilterSearchEnginePreProcessor implements LdapFilterSearchEnginePreProcessor { + + @Override + public String preProcess(String filter) { + return filter; + } + +} diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/adbroker/SearchQueryFotmatter.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/adbroker/SearchQueryFotmatter.java index 7a3a356..87b74ea 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/adbroker/SearchQueryFotmatter.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/adbroker/SearchQueryFotmatter.java @@ -8,9 +8,12 @@ public class SearchQueryFotmatter implements LdapQueryFormatter<LdapQueryExecution> { private final EnumMap<SearchLangageLDAPTokens, String> tokensToLDAPKeys; + private LdapFilterSearchEnginePreProcessor preProcessor; - public SearchQueryFotmatter(EnumMap<SearchLangageLDAPTokens, String> tokensToLDAPKeys) { + public SearchQueryFotmatter(EnumMap<SearchLangageLDAPTokens, String> tokensToLDAPKeys, + LdapFilterSearchEnginePreProcessor p) { this.tokensToLDAPKeys = tokensToLDAPKeys; + this.preProcessor = p; } /** @@ -23,6 +26,7 @@ public LdapQueryExecution format(LdapQueryMetadata queryMetadata) { String filter = (String) queryMetadata.getQueryData().getFilterParameters()[0]; + filter = preProcessor.preProcess(filter); for (Entry<SearchLangageLDAPTokens, String> tokenEntry : tokensToLDAPKeys.entrySet()) { filter = StringUtils.replace(filter, tokenEntry.getKey().name(), tokenEntry.getValue()); } diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/adbroker/UpnSplitterLdapFilterSearchEnginePreProcessor.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/adbroker/UpnSplitterLdapFilterSearchEnginePreProcessor.java new file mode 100644 index 0000000..7d7a4db --- /dev/null +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/adbroker/UpnSplitterLdapFilterSearchEnginePreProcessor.java @@ -0,0 +1,32 @@ +package org.ovirt.engine.core.bll.adbroker; + +/** + * The filter generated by the search mechanism contains the $PRINCIPAL_NAME entry + * to perform search by UPN. + * Thie filter takes the part after the equation sign (as the $PRINCIPAL_NAME part of + * the filter looks like ($PRINCIPAL_NAME=user@domain) + * and splits to user and domain. + * It composes back the filter, so the part of $PRINCIPAL_NAME will not look like + * ($PRINCIPAL_NAME=user) + * This is relevant for RHDS and OpenLdap + */ +public class UpnSplitterLdapFilterSearchEnginePreProcessor implements LdapFilterSearchEnginePreProcessor { + + @Override + public String preProcess(String filter) { + int principalNameIdx = filter.indexOf(SearchLangageLDAPTokens.$PRINCIPAL_NAME.toString()); + if (principalNameIdx != -1) { + int equalsIdx = filter.indexOf('=', principalNameIdx); + int closingParen = filter.indexOf(')', equalsIdx); + String user = filter.substring(equalsIdx + 1, closingParen); + if (user.indexOf('@') != -1) { // Indeed a principal name format + String[] parts = user.split("@"); + user = parts[0]; + } + filter = filter.substring(0, equalsIdx + 1) + user + filter.substring(closingParen); + + } + return filter; + } + +} diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/adbroker/LdapFilterSearchEnginePreProcessorTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/adbroker/LdapFilterSearchEnginePreProcessorTest.java new file mode 100644 index 0000000..348517d --- /dev/null +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/adbroker/LdapFilterSearchEnginePreProcessorTest.java @@ -0,0 +1,26 @@ +package org.ovirt.engine.core.bll.adbroker; + +import static org.junit.Assert.assertEquals; + +import org.junit.Test; + +public class LdapFilterSearchEnginePreProcessorTest { + + @Test + public void testNoOp() { + String filter = "(|($SN=oVirt)($DEPARTMENT=acme))"; + NoOpLdapFilterSearchEnginePreProcessor preProcessor = new NoOpLdapFilterSearchEnginePreProcessor(); + String result = preProcessor.preProcess(filter); + assertEquals(filter, result); + } + + @Test + public void testUpnSplit() { + String filter = "(|($EMAIL=a...@aovirt.org)($PRINCIPAL_NAME=test...@ovirt.org)($DUMMY=dummy)"; + UpnSplitterLdapFilterSearchEnginePreProcessor preProcessor = + new UpnSplitterLdapFilterSearchEnginePreProcessor(); + String result = preProcessor.preProcess(filter); + assertEquals("(|($EMAIL=a...@aovirt.org)($PRINCIPAL_NAME=testing)($DUMMY=dummy)", result); + } + +} -- To view, visit http://gerrit.ovirt.org/17794 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I35744eb423ca7f553bf7e9b20c72179bca4e6827 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Yair Zaslavsky <yzasl...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
