handavid commented on code in PR #1479:
URL: https://github.com/apache/knox/pull/1479#discussion_r4458170297
##########
gateway-server/src/main/java/org/apache/knox/gateway/services/ldap/interceptor/LDAPRolesLookupInterceptor.java:
##########
@@ -257,7 +266,7 @@ private EntryFilteringCursor
handleSearch(SearchOperationContext ctx) throws Lda
if (LdapUtils.isGroupEntry(entry)) {
roleEntries.addAll(translateGroupEntry(entry));
} else {
- final String username =
LdapUtils.extractUsernameFromEntry(entry, "uid", "cn");
+ final String username =
toRolesLookupUserId(LdapUtils.extractUsernameFromEntry(entry, "uid", "cn"));
final Set<String> groups = fetchGroups(entry);
Review Comment:
we should lowercase the group names as well
##########
gateway-server/src/main/java/org/apache/knox/gateway/services/ldap/KnoxLDAPServerManager.java:
##########
@@ -211,6 +211,11 @@ private void createInterceptors(GatewayConfig config)
throws Exception {
}
}
+ // Add common LDAP Proxy configurations to the roles lookup
interceptor
Review Comment:
we won't need to copy the config if we move the config under the interceptor
instead of on the roles lookup service.
##########
.github/workflows/tests/test_knox_ldap_proxy_search_with_role_lookup.py:
##########
@@ -163,6 +169,11 @@ def test_list_all_roles_by_objectclass_presence(self) ->
None:
groups = self.rdn_values(self.search(PROXY_GROUPS_BASE,
"(objectClass=*)"))
self.assertEqual(ALL_ROLES, set(groups))
+ # ApacheDS hands the interceptors
(|(objectClass=groupOfNames)(objectClass=referral)), which
+ # the roles lookup does not take for a list search from an ancestor of
ou=groups. So the
+ # backend groups are translated instead and roles assigned only to users
(platform:deployer)
+ # are missing.
+ @unittest.skip("root-level role listing misses roles assigned only to
users")
Review Comment:
I wasn't expecting this. this may be a bug in the way we determine whether a
filter is an all groups filter. I'll have to take a closer look
##########
gateway-spi/src/main/java/org/apache/knox/gateway/config/GatewayConfig.java:
##########
@@ -153,11 +153,13 @@ public interface GatewayConfig {
String LDAP_BACKEND_DATA_FILE = "gateway.ldap.backend.data.file";
String LDAP_RECURSIVE_GROUP_RESOLUTION =
"gateway.ldap.recursive.group.resolution";
String LDAP_RECURSIVE_GROUP_RESOLUTION_MAX_DEPTH =
"gateway.ldap.recursive.group.resolution.max.depth";
- String LDAP_ROLES_LOOKUP_STRATEGY = "gateway.ldap.roles.lookup.strategy";
- String LDAP_ROLES_LOOKUP_REST_API_ENDPOINT =
"gateway.ldap.roles.lookup.rest.api.endpoint";
- String LDAP_ROLES_LOOKUP_FILE_PATH = "gateway.ldap.roles.lookup.file.path";
- String LDAP_ROLES_LIST_REST_API_ENDPOINT =
"gateway.ldap.roles.list.rest.api.endpoint";
- String LDAP_ROLES_GET_REST_API_ENDPOINT =
"gateway.ldap.roles.get.rest.api.endpoint";
+ String LDAP_ROLES_PREFIX = "gateway.ldap.roles.";
+ String LDAP_ROLES_LOOKUP_STRATEGY = LDAP_ROLES_PREFIX + "lookup.strategy";
+ String LDAP_ROLES_LOOKUP_REST_API_ENDPOINT = LDAP_ROLES_PREFIX +
"lookup.rest.api.endpoint";
+ String LDAP_ROLES_LOOKUP_FILE_PATH = LDAP_ROLES_PREFIX + "lookup.file.path";
+ String LDAP_ROLES_LIST_REST_API_ENDPOINT = LDAP_ROLES_PREFIX +
"list.rest.api.endpoint";
+ String LDAP_ROLES_GET_REST_API_ENDPOINT = LDAP_ROLES_PREFIX +
"get.rest.api.endpoint";
+ String LDAP_ROLES_LOOKUP_LOWERCASE_USER_ID = LDAP_ROLES_PREFIX +
"lookup.lowercase.user.id";
Review Comment:
I think we can push this under the roles lookup interceptor config instead
of the roles lookup config. This config isn't used by the roles lookup service
itself.
--
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]
