Bug 285153: editusers.cgi cleanliness re-do - Patch by Marc Schumann…

editusers.cgi

@@ -81,24 +81,33 @@ if ($action eq 'search') {
                 'FROM profiles';
     my @bindValues;
     my $nextCondition;
+    my $visibleGroups;
 
     if (Param('usevisibilitygroups')) {
         # Show only users in visible groups.
-        my $visibleGroups = visibleGroupsAsString();
+        $visibleGroups = visibleGroupsAsString();
+
+        if ($visibleGroups) {
             $query .= qq{, user_group_map AS ugm
                          WHERE ugm.user_id = profiles.userid
                            AND ugm.isbless = 0
                            AND ugm.group_id IN ($visibleGroups)
                         };
             $nextCondition = 'AND';
+        }
     } else {
+        $visibleGroups = 1;
         if ($grouprestrict eq '1') {
             $query .= ', user_group_map AS ugm';
         }
         $nextCondition = 'WHERE';
     }
 
-    # Selection by user name.
+    if (!$visibleGroups) {
+        $vars->{'users'} = {};
+    }
+    else {
+        # Handle selection by user name.
         if (defined($matchtype)) {
             $query .= " $nextCondition profiles.login_name ";
             if ($matchtype eq 'regexp') {
@@ -112,18 +121,18 @@ if ($action eq 'search') {
                 $matchstr = "%$matchstr%";
             }
             $nextCondition = 'AND';
-        # We can trick_taint because we use the value in a SELECT only, using
-        # a placeholder.
+            # We can trick_taint because we use the value in a SELECT only,
+            # using a placeholder.
             trick_taint($matchstr);
             push(@bindValues, $matchstr);
         }
 
-    # Selection by group.
+        # Handle selection by group.
         if ($grouprestrict eq '1') {
             $query .= " $nextCondition profiles.userid = ugm.user_id " .
                       'AND ugm.group_id = ?';
-        # We can trick_taint because we use the value in a SELECT only, using
-        # a placeholder.
+            # We can trick_taint because we use the value in a SELECT only,
+            # using a placeholder.
             trick_taint($groupid);
             push(@bindValues, $groupid);
         }
@@ -132,6 +141,8 @@ if ($action eq 'search') {
         $vars->{'users'} = $dbh->selectall_arrayref($query,
                                                     {'Slice' => {}},
                                                     @bindValues);
+    }
+
     $template->process('admin/users/list.html.tmpl', $vars)
        || ThrowTemplateError($template->error());
 
@@ -591,7 +602,7 @@ sub mirrorListSelectionValues {
 
 # Give a list of IDs of groups the user can see.
 sub visibleGroupsAsString {
-    return join(', ', -1, @{$user->visible_groups_direct()});
+    return join(', ', @{$user->visible_groups_direct()});
 }
 
 # Give a list of IDs of groups the user may bless.
@@ -623,7 +634,8 @@ sub groupsUserMayBless {
 
     # If visibilitygroups are used, restrict the set of groups.
     if (Param('usevisibilitygroups')) {
-        my $visibleGroups = visibleGroupsAsString();
+        # Users need to see a group in order to bless it.
+        my $visibleGroups = visibleGroupsAsString() || return {};
         $query .= " $connector id in ($visibleGroups)";
     }
 
@@ -638,7 +650,9 @@ sub canSeeUser {
     my $query;
 
     if (Param('usevisibilitygroups')) {
-        my $visibleGroups = visibleGroupsAsString();
+        # If the user can see no groups, then no users are visible either.
+        my $visibleGroups = visibleGroupsAsString() || return 0;
+
         $query = qq{SELECT COUNT(DISTINCT userid)
                     FROM profiles, user_group_map
                     WHERE userid = ?