handavid commented on code in PR #1454:
URL: https://github.com/apache/knox/pull/1454#discussion_r4322445212
##########
gateway-server/src/main/java/org/apache/knox/gateway/services/ldap/interceptor/LDAPRolesLookupInterceptor.java:
##########
@@ -86,6 +96,90 @@ public EntryFilteringCursor search(SearchOperationContext
ctx) throws LdapExcept
}
}
+ if (isAllGroupsSearch(ctx)) {
+ return handleAllGroupsSearch(ctx);
+ } else {
+ return handleSearch(ctx);
+ }
+ }
+
+ private boolean isAllGroupsSearch(SearchOperationContext ctx) {
+ Dn searchDn = ctx.getDn();
+ if (searchDn == null) {
+ return false;
+ }
+
+ if (ctx.getScope() == SearchScope.OBJECT ||
!isAllGroupsFilter(ctx.getFilter())) {
+ return false;
+ }
+ // Either the proxy groups base, or the root with an explicit group
filter
+ return searchDn.toString().equalsIgnoreCase(proxyGroupSearchBase) ||
searchDn.size() <= 2;
Review Comment:
fixed to use separate criteria for the group search base and ancestor DNs.
##########
gateway-server/src/main/java/org/apache/knox/gateway/services/ldap/interceptor/LDAPRolesLookupInterceptor.java:
##########
@@ -86,6 +96,90 @@ public EntryFilteringCursor search(SearchOperationContext
ctx) throws LdapExcept
}
}
+ if (isAllGroupsSearch(ctx)) {
+ return handleAllGroupsSearch(ctx);
+ } else {
+ return handleSearch(ctx);
+ }
+ }
+
+ private boolean isAllGroupsSearch(SearchOperationContext ctx) {
+ Dn searchDn = ctx.getDn();
+ if (searchDn == null) {
+ return false;
+ }
+
+ if (ctx.getScope() == SearchScope.OBJECT ||
!isAllGroupsFilter(ctx.getFilter())) {
+ return false;
+ }
+ // Either the proxy groups base, or the root with an explicit group
filter
+ return searchDn.toString().equalsIgnoreCase(proxyGroupSearchBase) ||
searchDn.size() <= 2;
+ }
+
+ private boolean isAllGroupsFilter(ExprNode filter) {
+ // Case 1: Simple objectClass evaluation (e.g.,
objectClass=groupOfNames)
+ if (filter instanceof EqualityNode) {
+ EqualityNode<?> eqNode = (EqualityNode<?>) filter;
+ if ("objectClass".equalsIgnoreCase(eqNode.getAttribute())) {
+ String value = eqNode.getValue().toString();
+ return "groupOfNames".equalsIgnoreCase(value) ||
+ "groupOfUniqueNames".equalsIgnoreCase(value);
+ }
+ }
+
+ // Case 2a: Wildcard match on all common names and object classes
(e.g., cn=*, objectClass=*)
+ if (filter instanceof PresenceNode) {
+ PresenceNode presNode = (PresenceNode) filter;
+ return "cn".equalsIgnoreCase(presNode.getAttribute()) ||
+ "objectClass".equalsIgnoreCase(presNode.getAttribute());
+ }
+ // Case 2b: Wildcard match on object class (e.g., objectClass=*)
+ if (filter instanceof ObjectClassNode) {
+ return true;
+ }
+
+ // Case 3: Compound filters. An AND is an all groups filter only if
every child is, an OR
+ // if any child is, e.g. (&(objectClass=groupOfNames)(cn=*)) or
(|(uid=alice)(cn=*)).
+ // A NOT is never an all groups filter: negating a condition never
positively selects groups.
+ if (filter instanceof AndNode) {
+ List<ExprNode> children = ((AndNode) filter).getChildren();
+ return !children.isEmpty() &&
children.stream().allMatch(this::isAllGroupsFilter);
+ }
+ if (filter instanceof OrNode) {
+ return ((OrNode)
filter).getChildren().stream().anyMatch(this::isAllGroupsFilter);
Review Comment:
`anyMatch` is correct for searches on the `proxyGroupSearchBase`. This
criteria is too loose for searches on the ancestors or the root. This is solved
by splitting the filter examination depending on whether the search base is the
complete `proxyGroupSearchBase` or an ancestor.
##########
gateway-server/src/main/java/org/apache/knox/gateway/services/ldap/backend/LdapProxyBackend.java:
##########
@@ -576,6 +586,54 @@ public List<Entry> search(String searchBase, SearchScope
searchScope, String fil
}
}
+ private String mapMemberOfExpression(String filter, SchemaManager
schemaManager) throws Exception {
+ if (useMemberOf || !filter.toLowerCase(ROOT).contains("memberof")) {
+ return filter;
+ }
+ // Convert filter if it contains "memberOf" but the backend doesn't
support "memberOf"
+ MemberOfMappingVisitor visitor = new
MemberOfMappingVisitor(schemaManager, this::lookupGroup, this::isUser);
+ ExprNode node = FilterParser.parse(schemaManager, filter);
+ // Baseline in the same form as the visitor output so an unchanged
visit exits on the first pass
+ String current = node.toString();
+ for (int i = 0; i < recursiveGroupResolutionMaxDepth; ++i) {
+ Object visitedNode = node.accept(visitor);
+ if (!(visitedNode instanceof ExprNode)) {
+ throw new Exception("Unable to transform memberOf expressions
in filter: " + filter);
+ }
+ node = (ExprNode) visitedNode;
+ String updated = node.toString();
+ if (updated.equals(current)) {
+ break;
+ }
+ current = updated;
+ }
+ return current;
+ }
+
+ private Entry lookupGroup(String dn, SchemaManager schemaManager) throws
Exception {
+ String remoteDn = remoteSchemaConverter.convertProxyDnToRemoteDn(dn);
+ LdapConnection connection = null;
+ try {
+ connection = getConnection();
+ Entry entry = connection.lookup(remoteDn);
+ if (entry != null) {
+ return
remoteSchemaConverter.convertRemoteEntryToProxyEntry(entry, schemaManager);
+ } else {
+ return null;
+ }
+ } finally {
+ releaseConnection(connection);
+ }
+ }
+
+ private boolean isUser(String userDn) {
+ if (userDn == null) {
+ return false;
+ }
+ String lowerCaseUserDn = userDn.toLowerCase(ROOT);
+ return lowerCaseUserDn.contains(proxyUserSearchBase) ||
lowerCaseUserDn.contains(remoteUserObjectClass);
Review Comment:
switched string handling to DN handling and check against both proxy and
backend user search base.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]