You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by GitBox <gi...@apache.org> on 2020/11/22 14:49:48 UTC

[GitHub] [nifi] CefBoud opened a new pull request #4681: added userGroupsFilterExpression to allow per user LDAP filters

CefBoud opened a new pull request #4681:
URL: https://github.com/apache/nifi/pull/4681


   Added a 'User Groups Filter Expression' property to the LdapUserGroupProvider's configuration in authorizers.xml. When set, this property retrieves each user's groups according to the filter expressed in it and by replacing '{}' with the user's DN.
   
   This introduces the possibility of filters such as : "member:1.2.840.113556.1.4.1941:={}". For each user, a request will be made and will populate all his direct and indirect groups.
   This property can also be used to map groups to users regardless of the presence of an attribute that links the user to the group or vice versa. As long as a group satisfies the filter, the user will be assigned to it. The presence of '{}' (which is replaced by a user's DN) is optionnal.
   
   The 'Group Search Base' property is used as the search base for each request but the 'User Group Name Attribute' property is not allowed when the new property is defined as it conflicts with it since they both rely on a user entry for group inference.
   
   As mentionned in the JIRA issue, this property will offer more flexibility and will allow handling of nested groups which are necessary in Microsoft's AGDLP and other organizational security requirements.
   
   Thank you for submitting a contribution to Apache NiFi.
   
   Please provide a short description of the PR here:
   
   #### Description of PR
   
   _Enables X functionality; fixes bug NIFI-YYYY._
   
   In order to streamline the review of the contribution we ask you
   to ensure the following steps have been taken:
   
   ### For all changes:
   - [ ] Is there a JIRA ticket associated with this PR? Is it referenced 
        in the commit message?
   
   - [ ] Does your PR title start with **NIFI-XXXX** where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.
   
   - [ ] Has your PR been rebased against the latest commit within the target branch (typically `main`)?
   
   - [ ] Is your initial contribution a single, squashed commit? _Additional commits in response to PR reviewer feedback should be made on this branch and pushed to allow change tracking. Do not `squash` or use `--force` when pushing to allow for clean monitoring of changes._
   
   ### For code changes:
   - [ ] Have you ensured that the full suite of tests is executed via `mvn -Pcontrib-check clean install` at the root `nifi` folder?
   - [ ] Have you written or updated unit tests to verify your changes?
   - [ ] Have you verified that the full build is successful on JDK 8?
   - [ ] Have you verified that the full build is successful on JDK 11?
   - [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)? 
   - [ ] If applicable, have you updated the `LICENSE` file, including the main `LICENSE` file under `nifi-assembly`?
   - [ ] If applicable, have you updated the `NOTICE` file, including the main `NOTICE` file found under `nifi-assembly`?
   - [ ] If adding new Properties, have you added `.displayName` in addition to .name (programmatic access) for each of the new properties?
   
   ### For documentation related changes:
   - [ ] Have you ensured that format looks appropriate for the output in which it is rendered?
   
   ### Note:
   Please ensure that once the PR is submitted, you check GitHub Actions CI for build issues and submit an update to your PR as soon as possible.
   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [nifi] CefBoud edited a comment on pull request #4681: NIFI-8035: added userGroupsFilterExpression to allow per user LDAP filters and LDAP nested groups

Posted by GitBox <gi...@apache.org>.
CefBoud edited a comment on pull request #4681:
URL: https://github.com/apache/nifi/pull/4681#issuecomment-733080961


   > This appears to be a straightforward improvement, see code comments for a few small improvements. Please update the PR label to include NIFI-8035 as described in the check list.
   
   Thank you for your diligent remarks. I adjusted everything as per your comments.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [nifi] exceptionfactory commented on pull request #4681: added userGroupsFilterExpression to allow per user LDAP filters

Posted by GitBox <gi...@apache.org>.
exceptionfactory commented on pull request #4681:
URL: https://github.com/apache/nifi/pull/4681#issuecomment-734045123


   @CefBoud Thanks for making the changes, project maintainers will need to review and approve, but the changes look good.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [nifi] github-actions[bot] commented on pull request #4681: NIFI-8035: added userGroupsFilterExpression to allow per user LDAP filters and LDAP nested groups

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #4681:
URL: https://github.com/apache/nifi/pull/4681#issuecomment-819929608


   We're marking this PR as stale due to lack of updates in the past few months. If after another couple of weeks the stale label has not been removed this PR will be closed. This stale marker and eventual auto close does not indicate a judgement of the PR just lack of reviewer bandwidth and helps us keep the PR queue more manageable.  If you would like this PR re-opened you can do so and a committer can remove the stale tag.  Or you can open a new PR.  Try to help review other PRs to increase PR review bandwidth which in turn helps yours.


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [nifi] CefBoud commented on pull request #4681: added userGroupsFilterExpression to allow per user LDAP filters

Posted by GitBox <gi...@apache.org>.
CefBoud commented on pull request #4681:
URL: https://github.com/apache/nifi/pull/4681#issuecomment-733080961


   > This appears to be a straightforward improvement, see code comments for a few small improvements. Please update the PR label to include NIFI-8035 as described in the check list.
   
   Thank you for your diligent remarks. I adjusted everything as per your comments but I am unable to add a label to this PR (permissions or unfamiliarity with the UI problem).


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [nifi] exceptionfactory commented on a change in pull request #4681: added userGroupsFilterExpression to allow per user LDAP filters

Posted by GitBox <gi...@apache.org>.
exceptionfactory commented on a change in pull request #4681:
URL: https://github.com/apache/nifi/pull/4681#discussion_r528769423



##########
File path: nifi-nar-bundles/nifi-ldap-iaa-providers-bundle/nifi-ldap-iaa-providers/src/main/java/org/apache/nifi/ldap/tenants/LdapUserGroupProvider.java
##########
@@ -343,7 +347,14 @@ public void onConfigured(final AuthorizerConfigurationContext configurationConte
         if (StringUtils.isNotBlank(userGroupReferencedGroupAttribute) && !performGroupSearch) {
             throw new AuthorizerCreationException("'Group Search Base' must be set when specifying 'User Group Name Attribute - Referenced Group Attribute'.");
         }
-
+        //ensure that groupSearchBase is set when userGroupsFilterExpression is specified.
+        if (StringUtils.isNotBlank(userGroupsFilterExpression) && StringUtils.isBlank(groupSearchBase)){
+         throw new AuthorizerCreationException("Group Search Base' must be set when specifying 'User Groups Filter Expression'.");

Review comment:
       The message appears to be missing an opening single quote to call out 'Group Search Base'

##########
File path: nifi-nar-bundles/nifi-ldap-iaa-providers-bundle/nifi-ldap-iaa-providers/src/main/java/org/apache/nifi/ldap/tenants/LdapUserGroupProvider.java
##########
@@ -512,38 +523,66 @@ protected User doMapFromContext(DirContextOperations ctx) {
                             // store the user for group member later
                             userLookup.put(getReferencedUserValue(ctx), user);
 
-                            if (StringUtils.isNotBlank(userGroupNameAttribute)) {
-                                final Attribute attributeGroups = ctx.getAttributes().get(userGroupNameAttribute);
 
+                            // we can compute group values from user in two ways :
+                            //  1 - either directly from user entry using userGroupNameAttribute
+                            //  2 - search for each user's groups using userGroupsFilterExpression
+                            //if either userGroupNameAttribute or userGroupsFilterExpression is not blank, we try to populate the groupValues list, otherwise no groups are inferred from users.
+                            List<String> groupValues = null;
+                            if (StringUtils.isNotBlank(userGroupNameAttribute)) {
+                                Attribute attributeGroups = attributeGroups = ctx.getAttributes().get(userGroupNameAttribute);
                                 if (attributeGroups == null) {
                                     logger.debug(String.format("User group name attribute [%s] does not exist for %s. This may be due to "
                                             + "misconfiguration or the user may just not belong to any groups. Ignoring group membership.", userGroupNameAttribute, identity));
                                 } else {
                                     try {
-                                        final NamingEnumeration<String> groupValues = (NamingEnumeration<String>) attributeGroups.getAll();
-                                        while (groupValues.hasMoreElements()) {
-                                            final String groupValue = groupValues.next();
-
-                                            // if we are performing a group search, then we need to normalize the group value so that each
-                                            // user associating with it can be matched. if we are not performing a group search then these
-                                            // values will be used to actually build the group itself. case sensitivity is for group
-                                            // membership, not group identification.
-                                            final String groupValueNormalized;
-                                            if (performGroupSearch) {
-                                                groupValueNormalized = groupMembershipEnforceCaseSensitivity ? groupValue : groupValue.toLowerCase();
-                                            } else {
-                                                groupValueNormalized = groupValue;
-                                            }
-
-                                            // store the group -> user identifier mapping... if case sensitivity is disabled, the group reference value will
-                                            // be lowercased when adding to groupToUserIdentifierMappings
-                                            groupToUserIdentifierMappings.computeIfAbsent(groupValueNormalized, g -> new HashSet<>()).add(user.getIdentifier());
-                                        }
+                                        NamingEnumeration<String> groupValuesFromAttribute = (NamingEnumeration<String>) attributeGroups.getAll();
+                                        groupValues = new LinkedList<String>();
+                                        while (groupValuesFromAttribute.hasMoreElements())

Review comment:
       Recommend adding brackets to this while loop block instead of using the shorthand syntax.

##########
File path: nifi-nar-bundles/nifi-ldap-iaa-providers-bundle/nifi-ldap-iaa-providers/src/main/java/org/apache/nifi/ldap/tenants/LdapUserGroupProvider.java
##########
@@ -512,38 +523,66 @@ protected User doMapFromContext(DirContextOperations ctx) {
                             // store the user for group member later
                             userLookup.put(getReferencedUserValue(ctx), user);
 
-                            if (StringUtils.isNotBlank(userGroupNameAttribute)) {
-                                final Attribute attributeGroups = ctx.getAttributes().get(userGroupNameAttribute);
 
+                            // we can compute group values from user in two ways :
+                            //  1 - either directly from user entry using userGroupNameAttribute
+                            //  2 - search for each user's groups using userGroupsFilterExpression
+                            //if either userGroupNameAttribute or userGroupsFilterExpression is not blank, we try to populate the groupValues list, otherwise no groups are inferred from users.
+                            List<String> groupValues = null;
+                            if (StringUtils.isNotBlank(userGroupNameAttribute)) {
+                                Attribute attributeGroups = attributeGroups = ctx.getAttributes().get(userGroupNameAttribute);
                                 if (attributeGroups == null) {
                                     logger.debug(String.format("User group name attribute [%s] does not exist for %s. This may be due to "
                                             + "misconfiguration or the user may just not belong to any groups. Ignoring group membership.", userGroupNameAttribute, identity));
                                 } else {
                                     try {
-                                        final NamingEnumeration<String> groupValues = (NamingEnumeration<String>) attributeGroups.getAll();
-                                        while (groupValues.hasMoreElements()) {
-                                            final String groupValue = groupValues.next();
-
-                                            // if we are performing a group search, then we need to normalize the group value so that each
-                                            // user associating with it can be matched. if we are not performing a group search then these
-                                            // values will be used to actually build the group itself. case sensitivity is for group
-                                            // membership, not group identification.
-                                            final String groupValueNormalized;
-                                            if (performGroupSearch) {
-                                                groupValueNormalized = groupMembershipEnforceCaseSensitivity ? groupValue : groupValue.toLowerCase();
-                                            } else {
-                                                groupValueNormalized = groupValue;
-                                            }
-
-                                            // store the group -> user identifier mapping... if case sensitivity is disabled, the group reference value will
-                                            // be lowercased when adding to groupToUserIdentifierMappings
-                                            groupToUserIdentifierMappings.computeIfAbsent(groupValueNormalized, g -> new HashSet<>()).add(user.getIdentifier());
-                                        }
+                                        NamingEnumeration<String> groupValuesFromAttribute = (NamingEnumeration<String>) attributeGroups.getAll();
+                                        groupValues = new LinkedList<String>();
+                                        while (groupValuesFromAttribute.hasMoreElements())
+                                            groupValues.add(groupValuesFromAttribute.next());
                                     } catch (NamingException e) {
                                         throw new AuthorizationAccessException("Error while retrieving user group name attribute [" + userIdentityAttribute + "].");
                                     }
                                 }
                             }
+                            if (StringUtils.isNotBlank(userGroupsFilterExpression)) {
+                                // for each user, we search for groups according to userGroupsFilterExpression. Nested Ldap filter such as LDAP_MATCHING_RULE_IN_CHAIN can be used.
+                                AndFilter filter = new AndFilter();
+                                if (StringUtils.isNotBlank(userGroupsFilterExpression)) {
+                                    String currentUserGroupsFilterExpression = userGroupsFilterExpression.replaceAll("\\{\\}", ctx.getDn().toString());

Review comment:
       Recommend promoting the distinguished name placeholder string to a static class variable for improved code clarity.

##########
File path: nifi-nar-bundles/nifi-ldap-iaa-providers-bundle/nifi-ldap-iaa-providers/src/main/java/org/apache/nifi/ldap/tenants/LdapUserGroupProvider.java
##########
@@ -343,7 +347,14 @@ public void onConfigured(final AuthorizerConfigurationContext configurationConte
         if (StringUtils.isNotBlank(userGroupReferencedGroupAttribute) && !performGroupSearch) {
             throw new AuthorizerCreationException("'Group Search Base' must be set when specifying 'User Group Name Attribute - Referenced Group Attribute'.");
         }
-
+        //ensure that groupSearchBase is set when userGroupsFilterExpression is specified.
+        if (StringUtils.isNotBlank(userGroupsFilterExpression) && StringUtils.isBlank(groupSearchBase)){
+         throw new AuthorizerCreationException("Group Search Base' must be set when specifying 'User Groups Filter Expression'.");
+             }
+         //ensure we are not simultaneously searching groups inside user entry through userGroupNameAttribute and using userGroupsFilterExpression.
+           if (StringUtils.isNotBlank(userGroupNameAttribute) && StringUtils.isNotBlank(userGroupsFilterExpression)){
+                       throw new AuthorizerCreationException("'User Group Name Attribute' and 'User Groups Filter Expression' are both set. Only one may be used.");
+           }

Review comment:
       Recommend running automatic formatting on these lines to normalize indentation following standard conventions.

##########
File path: nifi-nar-bundles/nifi-ldap-iaa-providers-bundle/nifi-ldap-iaa-providers/src/test/java/org/apache/nifi/ldap/tenants/LdapUserGroupProviderTest.java
##########
@@ -745,6 +746,44 @@ public void testSearchUsersAndGroupsMembershipThroughUsersCaseInsensitive() thro
                 user -> "user6".equals(user.getIdentity()) || "user7".equals(user.getIdentity()) || "user8".equals(user.getIdentity())).count());
     }
 
+    @Test
+    public void testSearchUsersAndGroupsMembershipThroughUserGroupsFilterExpression() throws Exception {
+        final AuthorizerConfigurationContext configurationContext = getBaseConfiguration(USER_SEARCH_BASE, GROUP_SEARCH_BASE);
+        when(configurationContext.getProperty(PROP_USER_IDENTITY_ATTRIBUTE)).thenReturn(new StandardPropertyValue("uid", null, ParameterLookup.EMPTY));
+        // using PROP_USER_GROUPS_FILTER_EXPRESSION to search for each user groups based on its DN. Allows special filters to retrieve nested groups.
+        when(configurationContext.getProperty(PROP_USER_GROUPS_FILTER_EXPRESSION)).thenReturn(new StandardPropertyValue("member={}", null, ParameterLookup.EMPTY));
+        when(configurationContext.getProperty(PROP_GROUP_NAME_ATTRIBUTE)).thenReturn(new StandardPropertyValue("cn", null, ParameterLookup.EMPTY));
+        when(configurationContext.getProperty(PROP_GROUP_MEMBERSHIP_ENFORCE_CASE_SENSITIVITY)).thenReturn(new StandardPropertyValue("false", null, ParameterLookup.EMPTY));
+        ldapUserGroupProvider.onConfigured(configurationContext);
+
+        assertEquals(8, ldapUserGroupProvider.getUsers().size());
+
+        final Set<Group> groups = ldapUserGroupProvider.getGroups();
+        assertEquals(5, groups.size());
+
+        final Group team1 = groups.stream().filter(group -> "team1".equals(group.getName())).findFirst().orElse(null);
+        assertNotNull(team1);
+        assertEquals(1, team1.getUsers().size()); // team1 members are inferred from 'member' attribute. Only user1 is there.
+        assertEquals(1, team1.getUsers().stream().map(
+                userIdentifier -> ldapUserGroupProvider.getUser(userIdentifier)).filter(
+                user -> "user1".equals(user.getIdentity())).count());
+
+        final Group team4 = groups.stream().filter(group -> "team4".equals(group.getName())).findFirst().orElse(null);
+        assertNotNull(team4);
+        assertEquals(2, team4.getUsers().size()); // team4 members are inferred from 'member' attribute. Only user1 and user2 are there.
+        assertEquals(2, team4.getUsers().stream().map(
+                userIdentifier -> ldapUserGroupProvider.getUser(userIdentifier)).filter(
+                user -> "user1".equals(user.getIdentity()) || "user2".equals(user.getIdentity()) ).count());
+    }
+
+    @Test(expected = AuthorizerCreationException.class)
+    public void tesUserGroupsFilterExpressionAndUserGroupNameAttributeBothSpecified() throws Exception {

Review comment:
       When running the unit test in debug mode, this actually appears to hit the exception on 352 for an empty Group Search Base as opposed to the check for having both group filter expression and group attribute specified.  Recommend revisiting this unit test and adding another to exercise the other condition.

##########
File path: nifi-nar-bundles/nifi-ldap-iaa-providers-bundle/nifi-ldap-iaa-providers/src/main/java/org/apache/nifi/ldap/tenants/LdapUserGroupProvider.java
##########
@@ -512,38 +523,66 @@ protected User doMapFromContext(DirContextOperations ctx) {
                             // store the user for group member later
                             userLookup.put(getReferencedUserValue(ctx), user);
 
-                            if (StringUtils.isNotBlank(userGroupNameAttribute)) {
-                                final Attribute attributeGroups = ctx.getAttributes().get(userGroupNameAttribute);
 
+                            // we can compute group values from user in two ways :
+                            //  1 - either directly from user entry using userGroupNameAttribute
+                            //  2 - search for each user's groups using userGroupsFilterExpression
+                            //if either userGroupNameAttribute or userGroupsFilterExpression is not blank, we try to populate the groupValues list, otherwise no groups are inferred from users.
+                            List<String> groupValues = null;
+                            if (StringUtils.isNotBlank(userGroupNameAttribute)) {
+                                Attribute attributeGroups = attributeGroups = ctx.getAttributes().get(userGroupNameAttribute);
                                 if (attributeGroups == null) {
                                     logger.debug(String.format("User group name attribute [%s] does not exist for %s. This may be due to "
                                             + "misconfiguration or the user may just not belong to any groups. Ignoring group membership.", userGroupNameAttribute, identity));
                                 } else {
                                     try {
-                                        final NamingEnumeration<String> groupValues = (NamingEnumeration<String>) attributeGroups.getAll();
-                                        while (groupValues.hasMoreElements()) {
-                                            final String groupValue = groupValues.next();
-
-                                            // if we are performing a group search, then we need to normalize the group value so that each
-                                            // user associating with it can be matched. if we are not performing a group search then these
-                                            // values will be used to actually build the group itself. case sensitivity is for group
-                                            // membership, not group identification.
-                                            final String groupValueNormalized;
-                                            if (performGroupSearch) {
-                                                groupValueNormalized = groupMembershipEnforceCaseSensitivity ? groupValue : groupValue.toLowerCase();
-                                            } else {
-                                                groupValueNormalized = groupValue;
-                                            }
-
-                                            // store the group -> user identifier mapping... if case sensitivity is disabled, the group reference value will
-                                            // be lowercased when adding to groupToUserIdentifierMappings
-                                            groupToUserIdentifierMappings.computeIfAbsent(groupValueNormalized, g -> new HashSet<>()).add(user.getIdentifier());
-                                        }
+                                        NamingEnumeration<String> groupValuesFromAttribute = (NamingEnumeration<String>) attributeGroups.getAll();
+                                        groupValues = new LinkedList<String>();
+                                        while (groupValuesFromAttribute.hasMoreElements())
+                                            groupValues.add(groupValuesFromAttribute.next());
                                     } catch (NamingException e) {
                                         throw new AuthorizationAccessException("Error while retrieving user group name attribute [" + userIdentityAttribute + "].");
                                     }
                                 }
                             }
+                            if (StringUtils.isNotBlank(userGroupsFilterExpression)) {
+                                // for each user, we search for groups according to userGroupsFilterExpression. Nested Ldap filter such as LDAP_MATCHING_RULE_IN_CHAIN can be used.
+                                AndFilter filter = new AndFilter();
+                                if (StringUtils.isNotBlank(userGroupsFilterExpression)) {

Review comment:
       Is this check necessary?  It seems to be duplicative of line 548.

##########
File path: nifi-nar-bundles/nifi-ldap-iaa-providers-bundle/nifi-ldap-iaa-providers/src/main/java/org/apache/nifi/ldap/tenants/LdapUserGroupProvider.java
##########
@@ -512,38 +523,66 @@ protected User doMapFromContext(DirContextOperations ctx) {
                             // store the user for group member later
                             userLookup.put(getReferencedUserValue(ctx), user);
 
-                            if (StringUtils.isNotBlank(userGroupNameAttribute)) {
-                                final Attribute attributeGroups = ctx.getAttributes().get(userGroupNameAttribute);
 
+                            // we can compute group values from user in two ways :
+                            //  1 - either directly from user entry using userGroupNameAttribute
+                            //  2 - search for each user's groups using userGroupsFilterExpression
+                            //if either userGroupNameAttribute or userGroupsFilterExpression is not blank, we try to populate the groupValues list, otherwise no groups are inferred from users.
+                            List<String> groupValues = null;
+                            if (StringUtils.isNotBlank(userGroupNameAttribute)) {
+                                Attribute attributeGroups = attributeGroups = ctx.getAttributes().get(userGroupNameAttribute);
                                 if (attributeGroups == null) {
                                     logger.debug(String.format("User group name attribute [%s] does not exist for %s. This may be due to "
                                             + "misconfiguration or the user may just not belong to any groups. Ignoring group membership.", userGroupNameAttribute, identity));
                                 } else {
                                     try {
-                                        final NamingEnumeration<String> groupValues = (NamingEnumeration<String>) attributeGroups.getAll();
-                                        while (groupValues.hasMoreElements()) {
-                                            final String groupValue = groupValues.next();
-
-                                            // if we are performing a group search, then we need to normalize the group value so that each
-                                            // user associating with it can be matched. if we are not performing a group search then these
-                                            // values will be used to actually build the group itself. case sensitivity is for group
-                                            // membership, not group identification.
-                                            final String groupValueNormalized;
-                                            if (performGroupSearch) {
-                                                groupValueNormalized = groupMembershipEnforceCaseSensitivity ? groupValue : groupValue.toLowerCase();
-                                            } else {
-                                                groupValueNormalized = groupValue;
-                                            }
-
-                                            // store the group -> user identifier mapping... if case sensitivity is disabled, the group reference value will
-                                            // be lowercased when adding to groupToUserIdentifierMappings
-                                            groupToUserIdentifierMappings.computeIfAbsent(groupValueNormalized, g -> new HashSet<>()).add(user.getIdentifier());
-                                        }
+                                        NamingEnumeration<String> groupValuesFromAttribute = (NamingEnumeration<String>) attributeGroups.getAll();
+                                        groupValues = new LinkedList<String>();
+                                        while (groupValuesFromAttribute.hasMoreElements())
+                                            groupValues.add(groupValuesFromAttribute.next());
                                     } catch (NamingException e) {
                                         throw new AuthorizationAccessException("Error while retrieving user group name attribute [" + userIdentityAttribute + "].");
                                     }
                                 }
                             }
+                            if (StringUtils.isNotBlank(userGroupsFilterExpression)) {

Review comment:
       The property check on line 351 ensures that only one of the two group value determination paths is followed, but the logic here makes it appear that both could be followed.  Recommend changing this an "else if" for clarity.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [nifi] github-actions[bot] closed pull request #4681: NIFI-8035: added userGroupsFilterExpression to allow per user LDAP filters and LDAP nested groups

Posted by GitBox <gi...@apache.org>.
github-actions[bot] closed pull request #4681:
URL: https://github.com/apache/nifi/pull/4681


   


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org