From: lpsolit%gmail.com <>
Date: Sun, 14 Aug 2005 03:27:54 +0000 (+0000)
Subject: Bug 284263: Inherited bless permissions without inherited membership do not work... 
X-Git-Tag: bugzilla-2.20~49
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=3d7722812ae24bf10feb6c5d949d32525625ffdc;p=thirdparty%2Fbugzilla.git

Bug 284263: Inherited bless permissions without inherited membership do not work - Patch by Marc Schumann <wurblzap@gmail.com> r=LpSolit a=justdave
---

diff --git a/Bugzilla/User.pm b/Bugzilla/User.pm
index 1964eed4b5..0e34957521 100644
--- a/Bugzilla/User.pm
+++ b/Bugzilla/User.pm
@@ -291,33 +291,51 @@ sub groups {
 sub bless_groups {
     my $self = shift;
 
-    return $self->{bless_groups} if defined $self->{bless_groups};
+    return $self->{'bless_groups'} if defined $self->{'bless_groups'};
     return {} unless $self->id;
 
     my $dbh = Bugzilla->dbh;
-    # Get all groups for the user where:
-    #    + They have direct bless privileges
-    #    + They are a member of a group that inherits bless privs.
-    # Because of the second requirement, derive_groups must be up-to-date
-    # for this to function properly in all circumstances.
-    my $bless_groups = $dbh->selectcol_arrayref(
-        q{SELECT DISTINCT groups.name, groups.id
-            FROM groups, user_group_map, group_group_map AS ggm
-           WHERE user_group_map.user_id = ?
-             AND ((user_group_map.isbless = 1
-                   AND groups.id=user_group_map.group_id)
-                  OR (groups.id = ggm.grantor_id
-                      AND ggm.grant_type = } . GROUP_BLESS .
-                   q{ AND user_group_map.group_id = ggm.member_id
-                      AND user_group_map.isbless = 0))},
-         { Columns=>[1,2] }, $self->{id});
+    my $query;
+    my $connector;
+    my @bindValues;
+
+    if ($self->in_group('editusers')) {
+        # Users having editusers permissions may bless all groups.
+        $query = 'SELECT DISTINCT id, name, description FROM groups';
+        $connector = 'WHERE';
+    }
+    else {
+        # Get all groups for the user where:
+        #    + They have direct bless privileges
+        #    + They are a member of a group that inherits bless privs.
+        # Because of the second requirement, derive_groups must be up-to-date
+        # for this to function properly in all circumstances.
+        $query = q{
+            SELECT DISTINCT groups.id, groups.name, groups.description
+                       FROM groups, user_group_map, group_group_map AS ggm
+                      WHERE user_group_map.user_id = ?
+                        AND ((user_group_map.isbless = 1
+                              AND groups.id=user_group_map.group_id)
+                             OR (groups.id = ggm.grantor_id
+                                 AND ggm.grant_type = ?
+                                 AND user_group_map.group_id = ggm.member_id
+                                 AND user_group_map.isbless = 0))};
+        $connector = 'AND';
+        @bindValues = ($self->id, GROUP_BLESS);
+    }
 
-    # The above gives us an arrayref [name, id, name, id, ...]
-    # Convert that into a hashref
-    my %bless_groups_hashref = @$bless_groups;
-    $self->{bless_groups} = \%bless_groups_hashref;
+    # If visibilitygroups are used, restrict the set of groups.
+    if (Param('usevisibilitygroups')) {
+        # Users need to see a group in order to bless it.
+        my $visibleGroups = join(', ', @{$self->visible_groups_direct()})
+            || return $self->{'bless_groups'} = [];
+        $query .= " $connector id in ($visibleGroups)";
+    }
 
-    return $self->{bless_groups};
+    $query .= ' ORDER BY name';
+
+    return $self->{'bless_groups'} =
+        $dbh->selectall_arrayref($query, {'Slice' => {}}, @bindValues);
 }
 
 sub in_group {
@@ -569,12 +587,12 @@ sub can_bless {
     if (!scalar(@_)) {
         # If we're called without an argument, just return 
         # whether or not we can bless at all.
-        return scalar(keys %{$self->bless_groups}) ? 1 : 0;
+        return scalar(@{$self->bless_groups}) ? 1 : 0;
     }
 
     # Otherwise, we're checking a specific group
     my $group_name = shift;
-    return exists($self->bless_groups->{$group_name});
+    return (grep {$$_{'name'} eq $group_name} (@{$self->bless_groups})) ? 1 : 0;
 }
 
 sub flatten_group_membership {
@@ -1402,10 +1420,12 @@ and getting all of the groups would be overkill.
 
 =item C<bless_groups>
 
-Returns a hashref of group names for groups that the user can bless. The keys
-are the names of the groups, whilst the values are the respective group ids.
-(This is so that a set of all groupids for groups the user can bless can be
-obtained by C<values(%{$user-E<gt>bless_groups})>.)
+Returns an arrayref of hashes of C<groups> entries, where the keys of each hash
+are the names of C<id>, C<name> and C<description> columns of the C<groups>
+table.
+The arrayref consists of the groups the user can bless, taking into account
+that having editusers permissions means that you can bless all groups, and
+that you need to be aware of a group in order to bless a group.
 
 =item C<can_see_bug(bug_id)>
 
diff --git a/editusers.cgi b/editusers.cgi
index 5ccd1d06f2..90980c6101 100755
--- a/editusers.cgi
+++ b/editusers.cgi
@@ -67,7 +67,7 @@ mirrorListSelectionValues();
 ###########################################################################
 if ($action eq 'search') {
     # Allow to restrict the search to any group the user is allowed to bless.
-    $vars->{'restrictablegroups'} = groupsUserMayBless($user, 'id', 'name');
+    $vars->{'restrictablegroups'} = $user->bless_groups();
     $template->process('admin/users/search.html.tmpl', $vars)
        || ThrowTemplateError($template->error());
 
@@ -316,7 +316,7 @@ if ($action eq 'search') {
     # silently.
     # XXX: checking for existence of each user_group_map entry
     #      would allow to display a friendlier error message on page reloads.
-    foreach (@{groupsUserMayBless($user, 'id', 'name')}) {
+    foreach (@{$user->bless_groups()}) {
         my $id = $$_{'id'};
         my $name = $$_{'name'};
 
@@ -604,7 +604,7 @@ if ($action eq 'search') {
 
     $vars->{'message'} = 'account_deleted';
     $vars->{'otheruser'}{'login'} = $otherUserLogin;
-    $vars->{'restrictablegroups'} = groupsUserMayBless($user, 'id', 'name');
+    $vars->{'restrictablegroups'} = $user->bless_groups();
     $template->process('admin/users/search.html.tmpl', $vars)
        || ThrowTemplateError($template->error());
 
@@ -634,46 +634,6 @@ sub visibleGroupsAsString {
     return join(', ', @{$user->visible_groups_direct()});
 }
 
-# Give a list of IDs of groups the user may bless.
-sub groupsUserMayBless {
-    my $user = shift;
-    my $fieldList = join(', ', @_);
-    my $query;
-    my $connector;
-    my @bindValues;
-
-    $user->derive_groups(1);
-
-    if ($editusers) {
-        $query = "SELECT DISTINCT $fieldList FROM groups";
-        $connector = 'WHERE';
-    } else {
-        $query = qq{SELECT DISTINCT $fieldList
-                    FROM groups
-                    LEFT JOIN user_group_map AS ugm
-                           ON groups.id = ugm.group_id
-                    LEFT JOIN group_group_map AS ggm
-                           ON ggm.member_id = ugm.group_id
-                          AND ggm.grant_type = ?
-                    WHERE user_id = ?
-                      AND (ugm.isbless = 1 OR groups.id = ggm.grantor_id)
-                   };
-        @bindValues = (GROUP_BLESS, $userid);
-        $connector = 'AND';
-    }
-
-    # If visibilitygroups are used, restrict the set of groups.
-    if (Param('usevisibilitygroups')) {
-        # Users need to see a group in order to bless it.
-        my $visibleGroups = visibleGroupsAsString() || return {};
-        $query .= " $connector id in ($visibleGroups)";
-    }
-
-    $query .= ' ORDER BY name';
-
-    return $dbh->selectall_arrayref($query, {'Slice' => {}}, @bindValues);
-}
-
 # Determine whether the user can see a user. (Checks for existence, too.)
 sub canSeeUser {
     my $otherUserID = shift;
@@ -702,17 +662,15 @@ sub canSeeUser {
 # Retrieve user data for the user editing form. User creation and user
 # editing code rely on this to call derive_groups().
 sub userDataToVars {
-    my $userid = shift;
-    my $user = new Bugzilla::User($userid);
+    my $otheruserid = shift;
+    my $otheruser = new Bugzilla::User($otheruserid);
     my $query;
     my $dbh = Bugzilla->dbh;
 
-    $user->derive_groups();
+    $otheruser->derive_groups();
 
-    $vars->{'otheruser'} = $user;
-    $vars->{'groups'} = groupsUserMayBless($user, 'id', 'name', 'description');
-    $vars->{'disabledtext'} = $dbh->selectrow_array(
-        'SELECT disabledtext FROM profiles WHERE userid = ?', undef, $userid);
+    $vars->{'otheruser'} = $otheruser;
+    $vars->{'groups'} = $user->bless_groups();
 
     $vars->{'permissions'} = $dbh->selectall_hashref(
         qq{SELECT id,
@@ -743,10 +701,10 @@ sub userDataToVars {
                  AND directbless.grant_type = ?
           } . $dbh->sql_group_by('id'),
         'id', undef,
-        ($userid, GRANT_DIRECT,
-         $userid, GRANT_REGEXP,
-         $userid, GRANT_DERIVED,
-         $userid, GRANT_DIRECT));
+        ($otheruserid, GRANT_DIRECT,
+         $otheruserid, GRANT_REGEXP,
+         $otheruserid, GRANT_DERIVED,
+         $otheruserid, GRANT_DIRECT));
 
     # Find indirect bless permission.
     $query = qq{SELECT groups.id
@@ -757,7 +715,8 @@ sub userDataToVars {
                   AND ugm.isbless = 0
                   AND ggm.grant_type = ?
                } . $dbh->sql_group_by('id');
-    foreach (@{$dbh->selectall_arrayref($query, undef, ($userid, GROUP_BLESS))}) {
+    foreach (@{$dbh->selectall_arrayref($query, undef,
+                                        ($otheruserid, GROUP_BLESS))}) {
         # Merge indirect bless permissions into permission variable.
         $vars->{'permissions'}{${$_}[0]}{'indirectbless'} = 1;
     }