Bug 240325: Keep regexp-based groups up-to-date

5 files changed, 123 insertions, 50 deletions

diff --git a/Bugzilla/Constants.pm b/Bugzilla/Constants.pm
index 1ccde6b99..e3cdf539d 100644
--- a/Bugzilla/Constants.pm
+++ b/Bugzilla/Constants.pm
@@ -50,6 +50,10 @@ use base qw(Exporter);
     LOGOUT_ALL
     LOGOUT_CURRENT
     LOGOUT_KEEP_CURRENT
+
+    GRANT_DIRECT
+    GRANT_DERIVED
+    GRANT_REGEXP
 );
 
 @Bugzilla::Constants::EXPORT_OK = qw(contenttypes);
@@ -114,4 +118,8 @@ use constant contenttypes =>
    "ics" => "text/calendar" ,
   };
 
+use constant GRANT_DIRECT => 0;
+use constant GRANT_DERIVED => 1;
+use constant GRANT_REGEXP => 2;
+
 1;
diff --git a/Bugzilla/User.pm b/Bugzilla/User.pm
index c9535d7bf..b3d953945 100644
--- a/Bugzilla/User.pm
+++ b/Bugzilla/User.pm
@@ -35,6 +35,7 @@ package Bugzilla::User;
 use Bugzilla::Config;
 use Bugzilla::Error;
 use Bugzilla::Util;
+use Bugzilla::Constants;
 
 ################################################################################
 # Functions
@@ -246,9 +247,10 @@ sub derive_groups {
     # first remove any old derived stuff for this user
     $dbh->do(q{DELETE FROM user_group_map
                       WHERE user_id = ?
-                        AND isderived = 1},
+                        AND grant_type != ?},
              undef,
-             $id);
+             $id,
+             GRANT_DIRECT);
 
     my %groupidsadded = ();
     # add derived records for any matching regexps
@@ -260,10 +262,10 @@ sub derive_groups {
     while (my $row = $sth->fetch) {
         if ($self->{login} =~ m/$row->[1]/i) {
             $group_insert ||= $dbh->prepare(q{INSERT INTO user_group_map
-                                              (user_id, group_id, isbless, isderived)
-                                              VALUES (?, ?, 0, 1)});
+                                              (user_id, group_id, isbless, grant_type)
+                                              VALUES (?, ?, 0, ?)});
             $groupidsadded{$row->[0]} = 1;
-            $group_insert->execute($id, $row->[0]);
+            $group_insert->execute($id, $row->[0], GRANT_REGEXP);
         }
     }
 
@@ -294,9 +296,9 @@ sub derive_groups {
                 if (!$groupidsadded{$groupid}) {
                     $groupidsadded{$groupid} = 1;
                     $group_insert ||= $dbh->prepare(q{INSERT INTO user_group_map
-                                                      (user_id, group_id, isbless, isderived)
-                                                      VALUES (?, ?, 0, 1)});
-                    $group_insert->execute($id, $groupid);
+                                                      (user_id, group_id, isbless, grant_type)
+                                                      VALUES (?, ?, 0, ?)});
+                    $group_insert->execute($id, $groupid, GRANT_DERIVED);
                 }
             }
         }
diff --git a/checksetup.pl b/checksetup.pl
index ec7e998c4..f71bae575 100755
--- a/checksetup.pl
+++ b/checksetup.pl
@@ -1894,16 +1894,17 @@ $table{tokens} =
 # directly or due to regexp and which groups can be blessed
 # by a user. 
 #
-# isderived: 
-# if 0 - record was explicitly granted
-# if 1 - record was created by evaluating a regexp or group hierarchy
+# grant_type: 
+# if GRANT_DIRECT - record was explicitly granted
+# if GRANT_DERIVED - record was derived from expanding a group hierarchy
+# if GRANT_REGEXP - record was created by evaluating a regexp
 $table{user_group_map} =
     'user_id mediumint not null,
      group_id mediumint not null,
      isbless tinyint not null default 0,
-     isderived tinyint not null default 0,
+     grant_type tinyint not null default 0,
 
-     unique(user_id, group_id, isderived, isbless)';
+     unique(user_id, group_id, grant_type, isbless)';
 
 $table{group_group_map} =
     'member_id mediumint not null,
@@ -3455,8 +3456,8 @@ if (GetFieldDef("profiles", "groupset")) {
             $sth3->execute();
             if ( !$sth3->fetchrow_array() ) {
                 $dbh->do("INSERT INTO user_group_map
-                       (user_id, group_id, isbless, isderived)
-                       VALUES($uid, $gid, 0, 0)");
+                       (user_id, group_id, isbless, grant_type)
+                       VALUES($uid, $gid, 0, " . GRANT_DIRECT . ")");
             }
         }
         # Create user can bless group grants for old groupsets.
@@ -3466,8 +3467,8 @@ if (GetFieldDef("profiles", "groupset")) {
         $sth2->execute();
         while (my ($uid) = $sth2->fetchrow_array) {
             $dbh->do("INSERT INTO user_group_map
-                   (user_id, group_id, isbless, isderived)
-                   VALUES($uid, $gid, 1, 0)");
+                   (user_id, group_id, isbless, grant_type)
+                   VALUES($uid, $gid, 1, " . GRANT_DIRECT . ")");
         }
         # Create bug_group_map records for old groupsets.
         # Get each bug with the old group bit set.
@@ -3899,6 +3900,36 @@ if (!$series_exists) {
 
 AddFDef("owner_idle_time", "Time Since Owner Touched", 0);
 
+# 2004-04-12 - Keep regexp-based group permissions up-to-date - Bug 240325
+if (GetFieldDef("user_group_map", "isderived")) {
+    AddField('user_group_map', 'grant_type', 'tinyint not null default 0');
+    $dbh->do("UPDATE user_group_map SET grant_type = " .
+                             "IF(isderived, " . GRANT_DERIVED . ", " .
+                             GRANT_DIRECT . ")");
+    $dbh->do("DELETE FROM user_group_map 
+              WHERE isbless = 0 AND grant_type != " . GRANT_DIRECT);
+    DropField("user_group_map", "isderived");
+    DropIndexes("user_group_map");
+    $dbh->do("ALTER TABLE user_group_map 
+              ADD UNIQUE (user_id, group_id, grant_type, isbless)");
+    # Evaluate regexp-based group memberships
+    my $sth = $dbh->prepare("SELECT profiles.userid, profiles.login_name,
+                             groups.id, groups.userregexp 
+                             FROM profiles, groups
+                             WHERE userregexp != ''");
+    $sth->execute();
+    my $sth2 = $dbh->prepare("INSERT IGNORE INTO user_group_map 
+                           (user_id, group_id, isbless, grant_type) 
+                           VALUES(?, ?, 0, " . GRANT_REGEXP . ")");
+    while (my ($uid, $login, $gid, $rexp) = $sth->fetchrow_array()) {
+        if ($login =~ m/$rexp/i) {
+            $sth2->execute($uid, $gid);
+        }
+    }
+}
+
+    
+
 # If you had to change the --TABLE-- definition in any way, then add your
 # differential change code *** A B O V E *** this comment.
 #
@@ -3927,8 +3958,8 @@ if (!GroupDoesExist("editbugs")) {
     $sth->execute();
     while (my ($userid) = $sth->fetchrow_array()) {
         $dbh->do("INSERT INTO user_group_map 
-            (user_id, group_id, isbless, isderived) 
-            VALUES ($userid, $id, 0, 0)");
+            (user_id, group_id, isbless, grant_type) 
+            VALUES ($userid, $id, 0, " . GRANT_DIRECT . ")");
     }
 }
 
@@ -3938,8 +3969,8 @@ if (!GroupDoesExist("canconfirm")) {
     $sth->execute();
     while (my ($userid) = $sth->fetchrow_array()) {
         $dbh->do("INSERT INTO user_group_map 
-            (user_id, group_id, isbless, isderived) 
-            VALUES ($userid, $id, 0, 0)");
+            (user_id, group_id, isbless, grant_type) 
+            VALUES ($userid, $id, 0, " . GRANT_DIRECT . ")");
     }
 
 }
@@ -3965,15 +3996,15 @@ if (@admins) {
     my ($adminid) = $sth->fetchrow_array();
     foreach my $userid (@admins) {
         $dbh->do("INSERT INTO user_group_map 
-            (user_id, group_id, isbless, isderived) 
-            VALUES ($userid, $adminid, 0, 0)");
+            (user_id, group_id, isbless, grant_type) 
+            VALUES ($userid, $adminid, 0, " . GRANT_DIRECT . ")");
         # Existing administrators are made blessers of group "admin"
         # but only explitly defined blessers can bless group admin.
         # Other groups can be blessed by any admin (by default) or additional
         # defined blessers.
         $dbh->do("INSERT INTO user_group_map 
-            (user_id, group_id, isbless, isderived) 
-            VALUES ($userid, $adminid, 1, 0)");
+            (user_id, group_id, isbless, grant_type) 
+            VALUES ($userid, $adminid, 1, " . GRANT_DIRECT . ")");
     }
     $sth = $dbh->prepare("SELECT id FROM groups");
     $sth->execute();
@@ -4167,8 +4198,8 @@ if ($sth->rows == 0) {
         $sth->execute();
         if ( !$sth->fetchrow_array() ) {
             $dbh->do("INSERT INTO user_group_map 
-                (user_id, group_id, isbless, isderived) 
-                VALUES ($userid, $group, 0, 0)");
+                (user_id, group_id, isbless, grant_type) 
+                VALUES ($userid, $group, 0, " . GRANT_DIRECT . ")");
         }
     }
     # the admin also gets an explicit bless capability for the admin group
@@ -4177,8 +4208,8 @@ if ($sth->rows == 0) {
     $sth->execute();
     my ($id) = $sth->fetchrow_array();
     $dbh->do("INSERT INTO user_group_map 
-        (user_id, group_id, isbless, isderived) 
-        VALUES ($userid, $id, 1, 0)");
+        (user_id, group_id, isbless, grant_type) 
+        VALUES ($userid, $id, 1, " . GRANT_DIRECT . ")");
     foreach my $group ( @groups ) {
         $dbh->do("INSERT INTO group_group_map
             (member_id, grantor_id, isbless)
@@ -4262,6 +4293,11 @@ if (GetFieldDef('bugs', 'short_desc')->[2]) { # if it allows nulls
     ChangeFieldType('bugs', 'short_desc', 'mediumtext not null');
 }
 
+# 2004-04-12 - Keep regexp-based group permissions up-to-date - Bug 240325
+# Make sure groups get rederived
+$dbh->do("UPDATE groups SET last_changed = NOW() WHERE name = 'admin'");
+
+
 #
 # Final checks...
 
diff --git a/editgroups.cgi b/editgroups.cgi
index f51dbbab4..a0af0be12 100755
--- a/editgroups.cgi
+++ b/editgroups.cgi
@@ -49,6 +49,30 @@ if (!UserInGroup("creategroups")) {
 
 my $action  = trim($::FORM{action} || '');
 
+# RederiveRegexp: update user_group_map with regexp-based grants
+sub RederiveRegexp ($$)
+{
+    my $regexp = shift;
+    my $gid = shift;
+    my $dbh = Bugzilla->dbh;
+    my $sth = $dbh->prepare("SELECT userid, login_name FROM profiles");
+    my $sthadd = $dbh->prepare("INSERT IGNORE INTO user_group_map
+                               (user_id, group_id, grant_type, isbless)
+                               VALUES (?, ?, ?, 0)");
+    my $sthdel = $dbh->prepare("DELETE FROM user_group_map
+                                WHERE user_id = ? AND group_id = ?
+                                AND grant_type = ? and isbless = 0");
+    $sth->execute();
+    while (my ($uid, $login) = $sth->fetchrow_array()) {
+        if ($login =~ m/$regexp/i)
+        {
+            $sthadd->execute($uid, $gid, GRANT_REGEXP);
+        } else {
+            $sthdel->execute($uid, $gid, GRANT_REGEXP);
+        }
+    }
+}
+
 # TestGroup: check if the group name exists
 sub TestGroup ($)
 {
@@ -384,6 +408,7 @@ if ($action eq 'new') {
                 CONTROLMAPNA . ", 0 " .
                 "FROM products");
     }
+    RederiveRegexp($regexp, $gid);
     print "OK, done.<p>\n";
     PutTrailer("<a href=\"editgroups.cgi?action=add\">add</a> another group",
                "back to the <a href=\"editgroups.cgi\">group list</a>");
@@ -625,9 +650,9 @@ if (($action eq 'remove_all_regexp') || ($action eq 'remove_all')) {
                              FROM user_group_map, profiles
                              WHERE user_group_map.user_id = profiles.userid
                              AND user_group_map.group_id = ?
-                             AND isderived = 0
+                             AND grant_type = ?
                              AND isbless = 0");
-    $sth->execute($gid);
+    $sth->execute($gid, GRANT_DIRECT);
     my $sth2 = $dbh->prepare("DELETE FROM user_group_map
                               WHERE user_id = ?
                               AND isbless = 0
@@ -739,6 +764,7 @@ sub doGroupChanges {
         }
         SendSQL("UPDATE groups SET userregexp = " . 
             SqlQuote($::FORM{"rexp"}) . " WHERE id = $gid");
+        RederiveRegexp($::FORM{"rexp"}, $gid);
     }
     if (($isbuggroup == 1) && ($::FORM{"oldisactive"} ne $::FORM{"isactive"})) {
         $chgs = 1;
diff --git a/editusers.cgi b/editusers.cgi
index f83a64984..f7824e435 100755
--- a/editusers.cgi
+++ b/editusers.cgi
@@ -138,8 +138,9 @@ sub EmitFormElements ($$$$)
     if($user ne "") {
         print "</TR><TR><TH VALIGN=TOP ALIGN=RIGHT>Group Access:</TH><TD><TABLE><TR>";
         SendSQL("SELECT groups.id, groups.name, groups.description, " .
-                "COUNT(user_id), " .
-                "MAX(isderived) " .
+                "MAX(grant_type = " . GRANT_DIRECT . "), " .
+                "MAX(grant_type = " . GRANT_DERIVED . "), " .
+                "MAX(grant_type = " . GRANT_REGEXP . ") " .
                 "FROM groups " .
                 "LEFT JOIN user_group_map " .
                 "ON user_group_map.group_id = groups.id " .
@@ -153,10 +154,8 @@ sub EmitFormElements ($$$$)
             }
             print "<TD COLSPAN=2 ALIGN=LEFT><B>User is a member of these groups</B></TD>\n";
             while (MoreSQLData()) {
-                my ($groupid, $name, $description, $member, $isderived) = FetchSQLData();
+                my ($groupid, $name, $description, $checked, $isderived, $isregexp) = FetchSQLData();
                 next if (!$editall && !UserCanBlessGroup($name));
-                $isderived = $isderived || 0;
-                my $checked = $member - $isderived;
                 PushGlobalSQLState();
                 SendSQL("SELECT user_id " .
                         "FROM user_group_map " .
@@ -174,7 +173,7 @@ sub EmitFormElements ($$$$)
                 my $derivedbless = FetchOneColumn();
                 PopGlobalSQLState();
                 print "</TR><TR";
-                print ' bgcolor=#cccccc' if ($isderived);
+                print ' bgcolor=#cccccc' if ($isderived || $isregexp);
                 print ">\n";
                 print "<INPUT TYPE=HIDDEN NAME=\"oldgroup_$groupid\" VALUE=\"$checked\">\n";
                 print "<INPUT TYPE=HIDDEN NAME=\"oldbless_$groupid\" VALUE=\"$blchecked\">\n";
@@ -189,8 +188,10 @@ sub EmitFormElements ($$$$)
                 $checked = ($checked) ? "CHECKED" : "";
                 print "<TD ALIGN=CENTER>";
                 print '[' if ($isderived);
+                print '*' if ($isregexp);
                 print "<INPUT TYPE=CHECKBOX NAME=\"group_$groupid\" $checked VALUE=\"$groupid\">";
                 print ']' if ($isderived);
+                print '*' if ($isregexp);
                 print "</TD><TD><B>";
                 print ucfirst($name) . "</B>: $description</TD>\n";
             }
@@ -704,13 +705,13 @@ if ($action eq 'edit') {
         value_quote($disabledtext) . "\">\n";
     print "<INPUT TYPE=HIDDEN NAME=\"action\" VALUE=\"update\">\n";
     print "<INPUT TYPE=SUBMIT VALUE=\"Update\">\n";
-    print "<BR>User is a member of any groups shown with grey bars and
-           marked with brackets surrounding the membership checkbox as a 
-           result of a regular expression match 
-           or membership in another group.
-           User can bless any group 
-           marked with brackets surrounding the bless checkbox as a 
-           result of membership in another group.
+    print "<BR>User is a member of any groups shown with a check or grey bar.
+           A grey bar indicates indirect membership, either derived from other
+           groups (marked with square brackets) or via regular expression
+           (marked with '*').<p> 
+           Square brackets around the bless checkbox indicate the ability
+           to bless users (grant them membership in the group) as a result
+           of membership in another group.
        <BR>";
 
     print "</FORM>";
@@ -761,11 +762,11 @@ if ($action eq 'update') {
                      WHERE user_id = $thisuserid
                      AND group_id = $groupid
                      AND isbless = 0
-                     AND isderived = 0");
+                     AND grant_type = " . GRANT_DIRECT);
             if ($::FORM{"group_$groupid"}) {
                 SendSQL("INSERT INTO user_group_map 
-                         (user_id, group_id, isbless, isderived)
-                         VALUES ($thisuserid, $groupid, 0, 0)");
+                         (user_id, group_id, isbless, grant_type)
+                         VALUES ($thisuserid, $groupid, 0," . GRANT_DIRECT . ")");
                 print "Added user to group $name<BR>\n";
                 push(@grpadd, $name);
             } else {
@@ -781,11 +782,11 @@ if ($action eq 'update') {
                      WHERE user_id = $thisuserid
                      AND group_id = $groupid
                      AND isbless = 1
-                     AND isderived = 0");
+                     AND grant_type = " . GRANT_DIRECT);
             if ($::FORM{"bless_$groupid"}) {
                 SendSQL("INSERT INTO user_group_map 
-                         (user_id, group_id, isbless, isderived)
-                         VALUES ($thisuserid, $groupid, 1, 0)");
+                         (user_id, group_id, isbless, grant_type)
+                         VALUES ($thisuserid, $groupid, 1," . GRANT_DIRECT . ")");
                 print "Granted user permission to bless group $name<BR>\n";
             } else {
                 print "Revoked user's permission to bless group $name<BR>\n";