NIFI-7051 Protect against empty group membership in ShellUserGroupProvider, and add differentiator to id seeding

NIFI-7051 Fixing issue where identity was being used instead of identifier, making a flag to control legacy id behavior, increasing timeout of shell command runner, and changing the NSS system check command to return less info

NIFI-7051 Updating command for getSystemCheck in NSS impl to use getent --version to improve performance

This closes #4003.

nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-resources/src/main/resources/conf/authorizers.xml

@@ -178,6 +178,9 @@
         'Refresh Delay' - duration to wait between subsequent refreshes.  Default is '5 mins'.
         'Exclude Groups' - regular expression used to exclude groups.  Default is '', which means no groups are excluded.
         'Exclude Users' - regular expression used to exclude users.  Default is '', which means no users are excluded.
+        'Legacy Identifier Mode' - preserves the legacy behavior for id generation. Disabling this will ensure that
+                                    user and group ids are differentiated to handle the case where a user and group have
+                                    the same identity. Default is 'true', which means users and groups are not differentiated.
     -->
     <!-- To enable the shell-user-group-provider remove 2 lines. This is 1 of 2.
     <userGroupProvider>
@@ -186,6 +189,7 @@
         <property name="Refresh Delay">5 mins</property>
         <property name="Exclude Groups"></property>
         <property name="Exclude Users"></property>
+        <property name="Legacy Identifier Mode">true</property>
     </userGroupProvider>
     To enable the shell-user-group-provider remove 2 lines. This is 2 of 2. -->
 

nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-shell-authorizer/src/main/java/org/apache/nifi/authorization/NssShellCommands.java

@@ -85,6 +85,6 @@ class NssShellCommands implements ShellCommandsProvider {
      * @return Shell command string that will exit normally (0) on a suitable system.
      */
     public String getSystemCheck() {
-        return "getent passwd";
+        return "getent --version";
     }
 }

nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-shell-authorizer/src/main/java/org/apache/nifi/authorization/ShellUserGroupProvider.java

@@ -22,16 +22,18 @@ import org.apache.nifi.authorization.exception.AuthorizerDestructionException;
 import org.apache.nifi.authorization.util.ShellRunner;
 import org.apache.nifi.components.PropertyValue;
 import org.apache.nifi.util.FormatUtils;
+import org.apache.nifi.util.StringUtils;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import java.io.IOException;
-import java.util.Arrays;
+import java.util.Comparator;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.TreeSet;
 import java.util.concurrent.Executors;
 import java.util.concurrent.ScheduledExecutorService;
 import java.util.concurrent.TimeUnit;
@@ -56,10 +58,12 @@ public class ShellUserGroupProvider implements UserGroupProvider {
 
     public static final String EXCLUDE_USER_PROPERTY = "Exclude Users";
     public static final String EXCLUDE_GROUP_PROPERTY = "Exclude Groups";
+    public static final String LEGACY_IDENTIFIER_MODE = "Legacy Identifier Mode";
 
     private long fixedDelay;
     private Pattern excludeUsers;
     private Pattern excludeGroups;
+    private boolean legacyIdentifierMode;
 
     // Our scheduler has one thread for users, one for groups:
     private final ScheduledExecutorService scheduler = Executors.newScheduledThreadPool(2);
@@ -134,7 +138,7 @@ public class ShellUserGroupProvider implements UserGroupProvider {
         if (user == null) {
             logger.debug("getUser (by name) user not found: " + identity);
         } else {
-            logger.debug("getUser (by name) found user: " + user + " for name: " + identity);
+            logger.debug("getUser (by name) found user: " + user.getIdentity() + " for name: " + identity);
         }
         return user;
     }
@@ -176,7 +180,7 @@ public class ShellUserGroupProvider implements UserGroupProvider {
         if (group == null) {
             logger.debug("getGroup (by id) group not found: " + identifier);
         } else {
-            logger.debug("getGroup (by id) found group: " + group + " for id: " + identifier);
+            logger.debug("getGroup (by id) found group: " + group.getName() + " for id: " + identifier);
         }
         return group;
 
@@ -194,12 +198,14 @@ public class ShellUserGroupProvider implements UserGroupProvider {
         logger.debug("Retrieved user {} for identity {}", new Object[]{user, identity});
 
         Set<Group> groups = new HashSet<>();
-
+        if (user != null) {
             for (Group g : getGroups()) {
-            if (user != null && g.getUsers().contains(user.getIdentity())) {
+                if (g.getUsers().contains(user.getIdentifier())) {
+                    logger.debug("User {} belongs to group {}", new Object[]{user.getIdentity(), g.getName()});
                     groups.add(g);
                 }
             }
+        }
 
         if (groups.isEmpty()) {
             logger.debug("User {} belongs to no groups", user);
@@ -249,9 +255,9 @@ public class ShellUserGroupProvider implements UserGroupProvider {
         // will work on this host or not.
         try {
             ShellRunner.runShell(commands.getSystemCheck());
-        } catch (final IOException ioexc) {
+        } catch (final Exception e) {
-            logger.error("initialize exception: " + ioexc + " system check command: " + commands.getSystemCheck());
+            logger.error("initialize exception: " + e + " system check command: " + commands.getSystemCheck());
-            throw new AuthorizerCreationException(SYS_CHECK_ERROR, ioexc.getCause());
+            throw new AuthorizerCreationException(SYS_CHECK_ERROR, e);
         }
 
         // The next step is to add the user and group exclude regexes:
@@ -262,6 +268,9 @@ public class ShellUserGroupProvider implements UserGroupProvider {
             throw new AuthorizerCreationException(e);
         }
 
+        // Get the value for Legacy Identifier Mo
+        legacyIdentifierMode = Boolean.parseBoolean(getProperty(configurationContext, LEGACY_IDENTIFIER_MODE, "true"));
+
         // With our command set selected, and our system check passed, we can pull in the users and groups:
         refreshUsersAndGroups();
 
@@ -272,7 +281,7 @@ public class ShellUserGroupProvider implements UserGroupProvider {
             }catch (final Throwable t) {
                 logger.error("", t);
             }
-        }, fixedDelay, fixedDelay, TimeUnit.SECONDS);
+        }, fixedDelay, fixedDelay, TimeUnit.MILLISECONDS);
 
     }
 
@@ -444,6 +453,13 @@ public class ShellUserGroupProvider implements UserGroupProvider {
         synchronized (usersById) {
             usersById.clear();
             usersById.putAll(uidToUser);
+
+            if (logger.isTraceEnabled()) {
+                logger.trace("=== Users by id...");
+                Set<User> sortedUsers = new TreeSet<>(Comparator.comparing(User::getIdentity));
+                sortedUsers.addAll(usersById.values());
+                sortedUsers.forEach(u -> logger.trace("=== " + u.toString()));
+            }
         }
 
         synchronized (usersByName) {
@@ -456,6 +472,13 @@ public class ShellUserGroupProvider implements UserGroupProvider {
             groupsById.clear();
             groupsById.putAll(gidToGroup);
             logger.debug("groups now size: " + groupsById.size());
+
+            if (logger.isTraceEnabled()) {
+                logger.trace("=== Groups by id...");
+                Set<Group> sortedGroups = new TreeSet<>(Comparator.comparing(Group::getName));
+                sortedGroups.addAll(groupsById.values());
+                sortedGroups.forEach(g -> logger.trace("=== " + g.toString()));
+            }
         }
     }
 
@@ -468,25 +491,35 @@ public class ShellUserGroupProvider implements UserGroupProvider {
      */
     private void rebuildUsers(List<String> userLines, Map<String, User> idToUser, Map<String, User> usernameToUser, Map<String, User> gidToUser) {
         userLines.forEach(line -> {
+            logger.trace("Processing user: {}", new Object[]{line});
+
             String[] record = line.split(":");
             if (record.length > 2) {
-                String name = record[0], id = record[1], gid = record[2];
+                String userIdentity = record[0], userIdentifier = record[1], primaryGroupIdentifier = record[2];
 
-                if (name != null && id != null && !name.equals("") && !id.equals("") && !excludeUsers.matcher(name).matches()) {
+                if (!StringUtils.isBlank(userIdentifier) && !StringUtils.isBlank(userIdentity) && !excludeUsers.matcher(userIdentity).matches()) {
-                    User user = new User.Builder().identity(name).identifierGenerateFromSeed(id).build();
+                    User user = new User.Builder()
+                            .identity(userIdentity)
+                            .identifierGenerateFromSeed(getUserIdentifierSeed(userIdentity))
+                            .build();
                     idToUser.put(user.getIdentifier(), user);
-                    usernameToUser.put(name, user);
+                    usernameToUser.put(userIdentity, user);
+                    logger.debug("Refreshed user {}", new Object[]{user});
 
-                    if (gid != null && !gid.equals("")) {
+                    if (!StringUtils.isBlank(primaryGroupIdentifier)) {
                         // create a temporary group to deterministically generate the group id and associate this user
-                        Group group = new Group.Builder().name(gid).identifierGenerateFromSeed(gid).build();
+                        Group group = new Group.Builder()
+                                .name(primaryGroupIdentifier)
+                                .identifierGenerateFromSeed(getGroupIdentifierSeed(primaryGroupIdentifier))
+                                .build();
                         gidToUser.put(group.getIdentifier(), user);
+                        logger.debug("Associated primary group {} with user {}", new Object[]{group.getIdentifier(), userIdentity});
                     } else {
-                        logger.warn("Null or empty primary group id for: " + name);
+                        logger.warn("Null or empty primary group id for: " + userIdentity);
                     }
 
                 } else {
-                    logger.warn("Null, empty, or skipped user name: " + name + " or id: " + id);
+                    logger.warn("Null, empty, or skipped user name: " + userIdentity + " or id: " + userIdentifier);
                 }
             } else {
                 logger.warn("Unexpected record format.  Expected 3 or more colon separated values per line.");
@@ -506,16 +539,34 @@ public class ShellUserGroupProvider implements UserGroupProvider {
      */
     private void rebuildGroups(List<String> groupLines, Map<String, Group> groupsById) {
         groupLines.forEach(line -> {
+            logger.trace("Processing group: {}", new Object[]{line});
+
             String[] record = line.split(":");
             if (record.length > 1) {
                 Set<String> users = new HashSet<>();
-                String name = record[0], id = record[1];
+                String groupName = record[0], groupIdentifier = record[1];
 
                 try {
-                    List<String> memberLines = ShellRunner.runShell(selectedShellCommands.getGroupMembers(name));
+                    String groupMembersCommand = selectedShellCommands.getGroupMembers(groupName);
+                    List<String> memberLines = ShellRunner.runShell(groupMembersCommand);
                     // Use the first line only, and log if the line count isn't exactly one:
                     if (!memberLines.isEmpty()) {
-                        users.addAll(Arrays.asList(memberLines.get(0).split(",")));
+                        String memberLine = memberLines.get(0);
+                        if (!StringUtils.isBlank(memberLine)) {
+                            String[] members = memberLine.split(",");
+                            for (String userIdentity : members) {
+                                if (!StringUtils.isBlank(userIdentity)) {
+                                    User tempUser = new User.Builder()
+                                            .identity(userIdentity)
+                                            .identifierGenerateFromSeed(getUserIdentifierSeed(userIdentity))
+                                            .build();
+                                    users.add(tempUser.getIdentifier());
+                                    logger.debug("Added temp user {} for group {}", new Object[]{tempUser, groupName});
+                                }
+                            }
+                        } else {
+                            logger.debug("list membership returned no members");
+                        }
                     } else {
                         logger.debug("list membership returned zero lines.");
                     }
@@ -527,12 +578,16 @@ public class ShellUserGroupProvider implements UserGroupProvider {
                     logger.error("list membership shell exception: " + ioexc);
                 }
 
-                if (name != null && id != null && !name.equals("") && !id.equals("") && !excludeGroups.matcher(name).matches()) {
+                if (!StringUtils.isBlank(groupIdentifier) && !StringUtils.isBlank(groupName) && !excludeGroups.matcher(groupName).matches()) {
-                    Group group = new Group.Builder().name(name).identifierGenerateFromSeed(id).addUsers(users).build();
+                    Group group = new Group.Builder()
+                            .name(groupName)
+                            .identifierGenerateFromSeed(getGroupIdentifierSeed(groupIdentifier))
+                            .addUsers(users)
+                            .build();
                     groupsById.put(group.getIdentifier(), group);
-                    logger.debug("Refreshed group: " + group);
+                    logger.debug("Refreshed group {}", new Object[] {group});
                 } else {
-                    logger.warn("Null, empty, or skipped group name: " + name + " or id: " + id);
+                    logger.warn("Null, empty, or skipped group name: " + groupName + " or id: " + groupIdentifier);
                 }
             } else {
                 logger.warn("Unexpected record format.  Expected 1 or more comma separated values.");
@@ -552,19 +607,38 @@ public class ShellUserGroupProvider implements UserGroupProvider {
             Group primaryGroup = gidToGroup.get(primaryGid);
 
             if (primaryGroup == null) {
-                logger.warn("user: " + primaryUser + " primary group not found");
+                logger.warn("Primary group {} not found for {}", new Object[]{primaryGid, primaryUser.getIdentity()});
             } else if (!excludeGroups.matcher(primaryGroup.getName()).matches()) {
                 Set<String> groupUsers = primaryGroup.getUsers();
-                if (!groupUsers.contains(primaryUser.getIdentity())) {
+                if (!groupUsers.contains(primaryUser.getIdentifier())) {
-                    Set<String> secondSet = new HashSet<>(groupUsers);
+                    Set<String> updatedUserIdentifiers = new HashSet<>(groupUsers);
-                    secondSet.add(primaryUser.getIdentity());
+                    updatedUserIdentifiers.add(primaryUser.getIdentifier());
-                    Group group = new Group.Builder().name(primaryGroup.getName()).identifierGenerateFromSeed(primaryGid).addUsers(secondSet).build();
+
-                    gidToGroup.put(group.getIdentifier(), group);
+                    Group updatedGroup = new Group.Builder()
+                            .identifier(primaryGroup.getIdentifier())
+                            .name(primaryGroup.getName())
+                            .addUsers(updatedUserIdentifiers)
+                            .build();
+                    gidToGroup.put(updatedGroup.getIdentifier(), updatedGroup);
+                    logger.debug("Added user {} to primary group {}", new Object[]{primaryUser, updatedGroup});
+                } else {
+                    logger.debug("Primary group {} already contains user {}", new Object[]{primaryGroup, primaryUser});
                 }
+            } else {
+                logger.debug("Primary group {} excluded from matcher for {}", new Object[]{primaryGroup.getName(), primaryUser.getIdentity()});
             }
         });
     }
 
+    private String getUserIdentifierSeed(final String userIdentifier) {
+        return legacyIdentifierMode ? userIdentifier : userIdentifier + "-user";
+    }
+
+    private String getGroupIdentifierSeed(final String groupIdentifier) {
+        return legacyIdentifierMode ? groupIdentifier : groupIdentifier + "-group";
+    }
+
+
     /**
      * @return The fixed refresh delay.
      */

nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-shell-authorizer/src/main/java/org/apache/nifi/authorization/util/ShellRunner.java

@@ -33,7 +33,7 @@ public class ShellRunner {
 
     static String SHELL = "sh";
     static String OPTS = "-c";
-    static Integer TIMEOUT = 30;
+    static Integer TIMEOUT = 60;
 
     public static List<String> runShell(String command) throws IOException {
         return runShell(command, "<unknown>");
@@ -46,12 +46,17 @@ public class ShellRunner {
         logger.debug("Run Command '" + description + "': " + builderCommand);
         final Process proc = builder.start();
 
+        boolean completed;
         try {
-            proc.waitFor(TIMEOUT, TimeUnit.SECONDS);
+            completed = proc.waitFor(TIMEOUT, TimeUnit.SECONDS);
         } catch (InterruptedException irexc) {
             throw new IOException(irexc.getMessage(), irexc.getCause());
         }
 
+        if (!completed) {
+            throw new IllegalStateException("Shell command '" + command + "' did not complete during the allotted time period");
+        }
+
         if (proc.exitValue() != 0) {
             try (final Reader stderr = new InputStreamReader(proc.getErrorStream());
                  final BufferedReader reader = new BufferedReader(stderr)) {