smolnar82 commented on code in PR #1247: URL: https://github.com/apache/knox/pull/1247#discussion_r3379764485
########## gateway-server/src/main/java/org/apache/knox/gateway/services/ldap/LDAPRolesLookupInterceptor.java: ########## @@ -0,0 +1,147 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.knox.gateway.services.ldap; + +import org.apache.directory.api.ldap.model.cursor.ListCursor; +import org.apache.directory.api.ldap.model.entry.Attribute; +import org.apache.directory.api.ldap.model.entry.Entry; +import org.apache.directory.api.ldap.model.entry.Value; +import org.apache.directory.api.ldap.model.exception.LdapException; +import org.apache.directory.api.ldap.model.exception.LdapInvalidDnException; +import org.apache.directory.api.ldap.model.name.Dn; +import org.apache.directory.api.ldap.model.name.Rdn; +import org.apache.directory.server.core.api.filtering.EntryFilteringCursor; +import org.apache.directory.server.core.api.filtering.EntryFilteringCursorImpl; +import org.apache.directory.server.core.api.interceptor.BaseInterceptor; +import org.apache.directory.server.core.api.interceptor.context.SearchOperationContext; +import org.apache.knox.gateway.i18n.messages.MessagesFactory; + +import java.util.ArrayList; +import java.util.Collection; +import java.util.List; + +/** + * Interceptor that replaces group names in memberOf attributes with role names + * if LDAP roles lookup is enabled. + */ +public class LDAPRolesLookupInterceptor extends BaseInterceptor { + private static final LdapMessages LOG = MessagesFactory.get(LdapMessages.class); + private final LDAPRolesLookupService rolesLookupService; + + public LDAPRolesLookupInterceptor(LDAPRolesLookupService rolesLookupService) { + this.rolesLookupService = rolesLookupService; + } + + @Override + public EntryFilteringCursor search(SearchOperationContext ctx) throws LdapException { + final List<Entry> entries = new ArrayList<>(); + try (EntryFilteringCursor cursor = next(ctx)) { + while (cursor.next()) { + entries.add(modifyEntry(cursor.get())); Review Comment: Thank you for the feedback regarding the batching strategy! I looked into implementing the 3-step approach (Collect -> Batch Map -> Merge), but I found that because our current REST API endpoint is designed for single-context lookups (one user + their associated groups), batching actually introduces more overhead rather than reducing it. Specifically, since the API doesn't support a 'Bulk' request (e.g., fetching roles for multiple users at once), we must still make $N$ requests for $N$ users. Using a separate 3-step process actually increases the total number of HTTP round-trips. Here is a comparison for a search result returning 100 users sharing the same 5 groups: | Metric | Per-Entry Approach (Proposed) | 3-Step Strategy (Batching) | |---------------------|-------------------------------|----------------------------------| | Total HTTP Requests | 100 | 105 | | Backend DB Lookups | 600 | 105 | | Network Bottleneck | 100 round-trips | 105 round-trips | | Implementation | Simple & Stateless | Complex (manual result merging) | _Conclusion_: While the 3-step strategy reduces the number of lookups on the backend (database) side, it increases the number of HTTP round-trips, which is the primary latency bottleneck in this architecture. Furthermore, the REST API backend was specifically built to perform the User/Group 'OR' logic in a single query. By using the per-entry approach, we leverage the backend's design to get the final role set in exactly one trip per user. If we ever introduce a 'Bulk' endpoint to the REST API, the 3-step strategy would absolutely be the way to go. But for the current API, the per-entry approach is both faster and simpler. -- 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]
