diff options
| -rw-r--r-- | Bugzilla/Auth/Login/WWW/Env.pm | 9 | ||||
| -rwxr-xr-x | Bugzilla/Bug.pm | 9 | ||||
| -rw-r--r-- | Bugzilla/Constants.pm | 8 | ||||
| -rw-r--r-- | Bugzilla/Flag.pm | 4 | ||||
| -rw-r--r-- | Bugzilla/User.pm | 219 | ||||
| -rwxr-xr-x | checksetup.pl | 59 | ||||
| -rwxr-xr-x | editgroups.cgi | 17 | ||||
| -rwxr-xr-x | editusers.cgi | 28 | ||||
| -rwxr-xr-x | post_bug.cgi | 6 | ||||
| -rwxr-xr-x | process_bug.cgi | 14 | ||||
| -rwxr-xr-x | sanitycheck.cgi | 60 | ||||
| -rwxr-xr-x | token.cgi | 4 | ||||
| -rwxr-xr-x | userprefs.cgi | 8 | ||||
| -rwxr-xr-x | whine.pl | 10 |
14 files changed, 168 insertions, 287 deletions
diff --git a/Bugzilla/Auth/Login/WWW/Env.pm b/Bugzilla/Auth/Login/WWW/Env.pm index 39bea28df..64487884c 100644 --- a/Bugzilla/Auth/Login/WWW/Env.pm +++ b/Bugzilla/Auth/Login/WWW/Env.pm @@ -38,6 +38,7 @@ sub login { my $matched_userid = ''; my $matched_extern_id = ''; my $disabledtext = ''; + my $new_login_name = 0; my $dbh = Bugzilla->dbh; my $sth; @@ -122,6 +123,7 @@ sub login { ") VALUES ( ?, ?, ?, '' )"); $sth->execute($env_email, '*', $env_realname); $matched_userid = $dbh->bz_last_key('profiles', 'userid'); + $new_login_name = $matched_userid; } } } @@ -147,9 +149,16 @@ sub login { ($env_realname || $this_realname), $matched_userid); $sth->execute; + $new_login_name = $matched_userid; } } + # If the login name may be new, make sure the regexp groups are current + if ($new_login_name) { + my $userprofile = new Bugzilla::User($matched_userid); + $userprofile->derive_regexp_groups; + } + # Now we throw an error if the user has been disabled if ($disabledtext) { ThrowUserError("account_disabled", diff --git a/Bugzilla/Bug.pm b/Bugzilla/Bug.pm index f281dbda8..ec1927355 100755 --- a/Bugzilla/Bug.pm +++ b/Bugzilla/Bug.pm @@ -541,25 +541,22 @@ sub groups { # user_group_map record putting the user in that group. # The LEFT JOINs are checking for record existence. # + my $grouplist = Bugzilla->user->groups_as_string; my $sth = $dbh->prepare( "SELECT DISTINCT groups.id, name, description," . " bug_group_map.group_id IS NOT NULL," . - " user_group_map.group_id IS NOT NULL," . + " CASE WHEN groups.id IN($grouplist) THEN 1 ELSE 0 END," . " isactive, membercontrol, othercontrol" . " FROM groups" . " LEFT JOIN bug_group_map" . " ON bug_group_map.group_id = groups.id" . " AND bug_id = ?" . - " LEFT JOIN user_group_map" . - " ON user_group_map.group_id = groups.id" . - " AND user_id = ?" . - " AND isbless = 0" . " LEFT JOIN group_control_map" . " ON group_control_map.group_id = groups.id" . " AND group_control_map.product_id = ? " . " WHERE isbuggroup = 1" . " ORDER BY description"); - $sth->execute($self->{'bug_id'}, Bugzilla->user->id, + $sth->execute($self->{'bug_id'}, $self->{'product_id'}); while (my ($groupid, $name, $description, $ison, $ingroup, $isactive, diff --git a/Bugzilla/Constants.pm b/Bugzilla/Constants.pm index 2ebb7f10c..078c988b6 100644 --- a/Bugzilla/Constants.pm +++ b/Bugzilla/Constants.pm @@ -53,7 +53,6 @@ use base qw(Exporter); LOGOUT_KEEP_CURRENT GRANT_DIRECT - GRANT_DERIVED GRANT_REGEXP GROUP_MEMBERSHIP @@ -68,8 +67,6 @@ use base qw(Exporter); COMMENT_COLS - DERIVE_GROUPS_TABLES_ALREADY_LOCKED - UNLOCK_ABORT RELATIONSHIPS @@ -157,7 +154,6 @@ use constant contenttypes => }; use constant GRANT_DIRECT => 0; -use constant GRANT_DERIVED => 1; use constant GRANT_REGEXP => 2; use constant GROUP_MEMBERSHIP => 0; @@ -180,10 +176,6 @@ use constant DEFAULT_QUERY_NAME => '(Default query)'; # The column length for displayed (and wrapped) bug comments. use constant COMMENT_COLS => 80; -# Used to indicate to User::new and User::new_from_login calls -# that the derive_groups tables are already locked -use constant DERIVE_GROUPS_TABLES_ALREADY_LOCKED => 1; - # used by Bugzilla::DB to indicate that tables are being unlocked # because of error use constant UNLOCK_ABORT => 1; diff --git a/Bugzilla/Flag.pm b/Bugzilla/Flag.pm index 4c4864ea7..02ddb65ba 100644 --- a/Bugzilla/Flag.pm +++ b/Bugzilla/Flag.pm @@ -906,9 +906,7 @@ sub notify { { my @new_cc_list; foreach my $cc (split(/[, ]+/, $flag->{'type'}->{'cc_list'})) { - my $ccuser = Bugzilla::User->new_from_login($cc, - DERIVE_GROUPS_TABLES_ALREADY_LOCKED) - || next; + my $ccuser = Bugzilla::User->new_from_login($cc) || next; next if $flag->{'target'}->{'bug'}->{'restricted'} && !$ccuser->can_see_bug($flag->{'target'}->{'bug'}->{'id'}); diff --git a/Bugzilla/User.pm b/Bugzilla/User.pm index ab70f06fa..0ad345979 100644 --- a/Bugzilla/User.pm +++ b/Bugzilla/User.pm @@ -111,8 +111,6 @@ sub _create { # We're checking for validity here, so any value is OK trick_taint($val); - my $tables_locked_for_derive_groups = shift; - my $dbh = Bugzilla->dbh; my ($id, @@ -137,26 +135,6 @@ sub _create { $self->{'disabledtext'} = $disabledtext; $self->{'showmybugslink'} = $mybugslink; - # Now update any old group information if needed - my $result = $dbh->selectrow_array(q{SELECT 1 - FROM profiles, groups - WHERE userid=? - AND profiles.refreshed_when <= - groups.last_changed}, - undef, - $id); - - if ($result) { - my $is_main_db; - unless ($is_main_db = Bugzilla->dbwritesallowed()) { - Bugzilla->switch_to_main_db(); - } - $self->derive_groups($tables_locked_for_derive_groups); - unless ($is_main_db) { - Bugzilla->switch_to_shadow_db(); - } - } - return $self; } @@ -283,11 +261,36 @@ sub groups { # The above gives us an arrayref [name, id, name, id, ...] # Convert that into a hashref my %groups = @$groups; + my $sth; + my @groupidstocheck = values(%groups); + my %groupidschecked = (); + $sth = $dbh->prepare("SELECT groups.name, groups.id + FROM group_group_map + INNER JOIN groups + ON groups.id = grantor_id + WHERE member_id = ? + AND grant_type = " . GROUP_MEMBERSHIP); + while (my $node = shift @groupidstocheck) { + $sth->execute($node); + my ($member_name, $member_id); + while (($member_name, $member_id) = $sth->fetchrow_array) { + if (!$groupidschecked{$member_id}) { + $groupidschecked{$member_id} = 1; + push @groupidstocheck, $member_id; + $groups{$member_name} = $member_id; + } + } + } $self->{groups} = \%groups; return $self->{groups}; } +sub groups_as_string { + my $self = shift; + return (join(',',values(%{$self->groups})) || '-1'); +} + sub bless_groups { my $self = shift; @@ -308,8 +311,6 @@ sub bless_groups { # 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 @@ -318,8 +319,9 @@ sub bless_groups { 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))}; + AND ggm.member_id IN(} . + $self->groups_as_string . + q{)))}; $connector = 'AND'; @bindValues = ($self->id, GROUP_BLESS); } @@ -340,26 +342,13 @@ sub bless_groups { sub in_group { my ($self, $group) = @_; + return exists $self->groups->{$group} ? 1 : 0; +} - # If we already have the info, just return it. - return defined($self->{groups}->{$group}) if defined $self->{groups}; - return 0 unless $self->id; - - # Otherwise, go check for it - - my $dbh = Bugzilla->dbh; - - my ($res) = $dbh->selectrow_array(q{SELECT 1 - FROM groups, user_group_map - WHERE groups.id=user_group_map.group_id - AND user_group_map.user_id=? - AND isbless=0 - AND groups.name=?}, - undef, - $self->id, - $group); - - return defined($res); +sub in_group_id { + my ($self, $id) = @_; + my %j = reverse(%{$self->groups}); + return exists $j{$id} ? 1 : 0; } sub can_see_user { @@ -405,7 +394,7 @@ sub can_see_bug { LEFT JOIN bug_group_map ON bugs.bug_id = bug_group_map.bug_id AND bug_group_map.group_ID NOT IN(" . - join(',',(-1, values(%{$self->groups}))) . + $self->groups_as_string . ") WHERE bugs.bug_id = ? AND creation_ts IS NOT NULL " . $dbh->sql_group_by('bugs.bug_id', 'reporter, ' . @@ -445,7 +434,7 @@ sub get_selectable_products { CONTROLMAPMANDATORY . " "; } $query .= "AND group_id NOT IN(" . - join(',', (-1,values(%{Bugzilla->user->groups}))) . ") " . + $self->groups_as_string . ") " . "WHERE group_id IS NULL ORDER BY name"; my $dbh = Bugzilla->dbh; my $sth = $dbh->prepare($query); @@ -501,8 +490,8 @@ sub visible_groups_as_string { return join(', ', @{$self->visible_groups_inherited()}); } -sub derive_groups { - my ($self, $already_locked) = @_; +sub derive_regexp_groups { + my ($self) = @_; my $id = $self->id; return unless $id; @@ -511,71 +500,32 @@ sub derive_groups { my $sth; - $dbh->bz_lock_tables('profiles WRITE', 'user_group_map WRITE', - 'group_group_map READ', - 'groups READ') unless $already_locked; - # avoid races, we are only up to date as of the BEGINNING of this process my $time = $dbh->selectrow_array("SELECT NOW()"); - # first remove any old derived stuff for this user - $dbh->do(q{DELETE FROM user_group_map - WHERE user_id = ? - AND grant_type != ?}, - undef, - $id, - GRANT_DIRECT); - - my %groupidsadded = (); # add derived records for any matching regexps - $sth = $dbh->prepare("SELECT id, userregexp FROM groups WHERE userregexp != ''"); - $sth->execute; - - my $group_insert; - 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, grant_type) - VALUES (?, ?, 0, ?)}); - $groupidsadded{$row->[0]} = 1; - $group_insert->execute($id, $row->[0], GRANT_REGEXP); - } - } - - # Get a list of the groups of which the user is a member. - my %groupidschecked = (); - - my @groupidstocheck = @{$dbh->selectcol_arrayref(q{SELECT group_id - FROM user_group_map - WHERE user_id=?}, - undef, - $id)}; - - # Each group needs to be checked for inherited memberships once. - my $group_sth; - while (@groupidstocheck) { - my $group = shift @groupidstocheck; - if (!defined($groupidschecked{"$group"})) { - $groupidschecked{"$group"} = 1; - $group_sth ||= $dbh->prepare(q{SELECT grantor_id - FROM group_group_map - WHERE member_id=? - AND grant_type = } . - GROUP_MEMBERSHIP); - $group_sth->execute($group); - while (my ($groupid) = $group_sth->fetchrow_array) { - if (!defined($groupidschecked{"$groupid"})) { - push(@groupidstocheck,$groupid); - } - if (!$groupidsadded{$groupid}) { - $groupidsadded{$groupid} = 1; - $group_insert ||= $dbh->prepare(q{INSERT INTO user_group_map - (user_id, group_id, isbless, grant_type) - VALUES (?, ?, 0, ?)}); - $group_insert->execute($id, $groupid, GRANT_DERIVED); - } - } + $sth = $dbh->prepare("SELECT id, userregexp, user_group_map.group_id + FROM groups + LEFT JOIN user_group_map + ON groups.id = user_group_map.group_id + AND user_group_map.user_id = ? + AND user_group_map.grant_type = ?"); + $sth->execute($id, GRANT_REGEXP); + + my $group_insert = $dbh->prepare(q{INSERT INTO user_group_map + (user_id, group_id, isbless, grant_type) + VALUES (?, ?, 0, ?)}); + my $group_delete = $dbh->prepare(q{DELETE FROM user_group_map + WHERE user_id = ? + AND group_id = ? + AND isbless = 0 + AND grant_type = ?}); + while (my ($group, $regexp, $present) = $sth->fetchrow_array()) { + if (($regexp ne '') && ($self->{login} =~ m/$regexp/i)) { + $group_insert->execute($id, $group, GRANT_REGEXP) unless $present; + } else { + $group_delete->execute($id, $group, GRANT_REGEXP) if $present; } } @@ -585,7 +535,6 @@ sub derive_groups { undef, $time, $id); - $dbh->bz_unlock_tables() unless $already_locked; } sub product_responsibilities { @@ -690,8 +639,8 @@ sub match { $query .= "AND user_group_map.user_id = userid " . "AND isbless = 0 " . "AND group_id IN(" . - join(', ', (-1, @{$user->visible_groups_inherited})) . ") " . - "AND grant_type <> " . GRANT_DERIVED; + join(', ', (-1, @{$user->visible_groups_inherited})) . + ")"; } $query .= " AND disabledtext = '' " if $exclude_disabled; $query .= "ORDER BY namelength "; @@ -743,8 +692,7 @@ sub match { $query .= " AND user_group_map.user_id = userid" . " AND isbless = 0" . " AND group_id IN(" . - join(', ', (-1, @{$user->visible_groups_inherited})) . ")" . - " AND grant_type <> " . GRANT_DERIVED; + join(', ', (-1, @{$user->visible_groups_inherited})) . ")"; } $query .= " AND disabledtext = ''" if $exclude_disabled; $query .= " ORDER BY namelength"; @@ -1151,8 +1099,7 @@ sub get_userlist { $query .= "LEFT JOIN user_group_map " . "ON user_group_map.user_id = userid AND isbless = 0 " . "AND group_id IN(" . - join(', ', (-1, @{$self->visible_groups_inherited})) . ") " . - "AND grant_type <> " . GRANT_DERIVED; + join(', ', (-1, @{$self->visible_groups_inherited})) . ")"; } $query .= " WHERE disabledtext = '' "; $query .= $dbh->sql_group_by('userid', 'login_name, realname'); @@ -1271,7 +1218,7 @@ sub login_to_id { } sub UserInGroup { - return defined Bugzilla->user->groups->{$_[0]} ? 1 : 0; + return exists Bugzilla->user->groups->{$_[0]} ? 1 : 0; } 1; @@ -1346,10 +1293,6 @@ C<undef> if no matching user is found. This routine should not be required in general; most scripts should be using userids instead. -This routine and C<new> both take an extra optional argument, which is -passed as the argument to C<derive_groups> to avoid locking. See that -routine's documentation for details. - =end undocumented =item C<id> @@ -1439,12 +1382,19 @@ 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 is in can be obtained by C<values(%{$user-E<gt>groups})>.) +=item C<groups_as_string> + +Returns a string containing a comma-seperated list of numeric group ids. If +the user is not a member of any groups, returns "-1". This is most often used +within an SQL IN() function. + =item C<in_group> -Determines whether or not a user is in the given group. This method is mainly -intended for cases where we are not looking at the currently logged in user, -and only need to make a quick check for the group, where calling C<groups> -and getting all of the groups would be overkill. +Determines whether or not a user is in the given group by name. + +=item C<in_group_id> + +Determines whether or not a user is in the given group by id. =item C<bless_groups> @@ -1464,7 +1414,7 @@ Returns 1 if the specified user account exists and is visible to the user, Determines if the user can see the specified bug. -=item C<derive_groups> +=item C<derive_regexp_groups> Bugzilla allows for group inheritance. When data about the user (or any of the groups) changes, the database must be updated. Handling updated groups is taken @@ -1508,23 +1458,6 @@ Returns a list of groups that the user is aware of. Returns the result of C<visible_groups_direct> as a string (a comma-separated list). -=begin undocumented - -This routine takes an optional argument. If true, then this routine will not -lock the tables, but will rely on the caller to have done so itsself. - -This is required because mysql will only execute a query if all of the tables -are locked, or if none of them are, not a mixture. If the caller has already -done some locking, then this routine would fail. Thus the caller needs to lock -all the tables required by this method, and then C<derive_groups> won't do -any locking. - -This is a really ugly solution, and when Bugzilla supports transactions -instead of using the explicit table locking we were forced to do when thats -all MySQL supported, this will go away. - -=end undocumented - =item C<product_responsibilities> Retrieve user's product responsibilities as a list of hashes. diff --git a/checksetup.pl b/checksetup.pl index 1b3c5d094..f20d1dcb2 100755 --- a/checksetup.pl +++ b/checksetup.pl @@ -31,6 +31,7 @@ # Erik Stambaugh <erik@dasbistro.com> # Dave Lawrence <dkl@redhat.com> # Max Kanat-Alexander <mkanat@bugzilla.org> +# Joel Peshkin <bugreport@peshkin.net> # # # @@ -3557,11 +3558,8 @@ AddFDef("owner_idle_time", "Time Since Assignee Touched", 0); if ($dbh->bz_column_info("user_group_map", "isderived")) { $dbh->bz_add_column('user_group_map', 'grant_type', {TYPE => 'INT1', NOTNULL => 1, 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); + $dbh->do("DELETE FROM user_group_map WHERE isderived != 0"); + $dbh->do("UPDATE user_group_map SET grant_type = " . GRANT_DIRECT); $dbh->bz_drop_column("user_group_map", "isderived"); $dbh->bz_drop_index('user_group_map', 'user_group_map_user_id_idx'); @@ -3569,21 +3567,6 @@ if ($dbh->bz_column_info("user_group_map", "isderived")) { {TYPE => 'UNIQUE', FIELDS => [qw(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); - } - } - # Make sure groups get rederived $dbh->do("UPDATE groups SET last_changed = NOW() WHERE name = 'admin'"); } @@ -4090,6 +4073,42 @@ if (!GroupDoesExist('bz_canusewhines')) { GROUP_MEMBERSHIP . ")") unless $group_exists; } +# 2005-08-14 bugreport@peshkin.net -- Bug 304583 +use constant GRANT_DERIVED => 1; +# Get rid of leftover DERIVED group permissions +$dbh->do("DELETE FROM user_group_map WHERE grant_type = " . GRANT_DERIVED); +# Evaluate regexp-based group memberships +$sth = $dbh->prepare("SELECT profiles.userid, profiles.login_name, + groups.id, groups.userregexp, + user_group_map.group_id + FROM profiles + CROSS JOIN groups + LEFT JOIN user_group_map + ON user_group_map.user_id = profiles.userid + AND user_group_map.group_id = groups.id + AND user_group_map.grant_type = ? + WHERE (userregexp != '' + OR user_group_map.group_id IS NOT NULL)"); + +my $sth_add = $dbh->prepare("INSERT INTO user_group_map + (user_id, group_id, isbless, grant_type) + VALUES(?, ?, 0, " . GRANT_REGEXP . ")"); + +my $sth_del = $dbh->prepare("DELETE FROM user_group_map + WHERE user_id = ? + AND group_id = ? + AND isbless = 0 + AND grant_type = " . GRANT_REGEXP); + +$sth->execute(GRANT_REGEXP); +while (my ($uid, $login, $gid, $rexp, $present) = $sth->fetchrow_array()) { + if ($login =~ m/$rexp/i) { + $sth_add->execute($uid, $gid) unless $present; + } else { + $sth_del->execute($uid, $gid) if $present; + } +} + ########################################################################### # Create --SETTINGS-- users can adjust ########################################################################### diff --git a/editgroups.cgi b/editgroups.cgi index b5ecd1e98..b9759910b 100755 --- a/editgroups.cgi +++ b/editgroups.cgi @@ -56,20 +56,21 @@ sub RederiveRegexp my $regexp = shift; my $gid = shift; my $dbh = Bugzilla->dbh; - my $sth = $dbh->prepare("SELECT userid, login_name FROM profiles"); - my $sthqry = $dbh->prepare("SELECT 1 FROM user_group_map - WHERE user_id = ? AND group_id = ? - AND grant_type = ? and isbless = 0"); + my $sth = $dbh->prepare("SELECT userid, login_name, group_id + FROM profiles + LEFT JOIN user_group_map + ON user_group_map.user_id = profiles.userid + AND group_id = ? + AND grant_type = ? + AND isbless = 0"); my $sthadd = $dbh->prepare("INSERT 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()) { - my $present = $dbh->selectrow_array($sthqry, undef, - $uid, $gid, GRANT_REGEXP); + $sth->execute($gid, GRANT_REGEXP); + while (my ($uid, $login, $present) = $sth->fetchrow_array()) { if (($regexp =~ /\S+/) && ($login =~ m/$regexp/i)) { $sthadd->execute($uid, $gid, GRANT_REGEXP) unless $present; diff --git a/editusers.cgi b/editusers.cgi index 56c0a7635..6c9fceefe 100755 --- a/editusers.cgi +++ b/editusers.cgi @@ -185,6 +185,8 @@ if ($action eq 'search') { insert_new_user($login, $realname, $password, $disabledtext); $otherUserID = $dbh->bz_last_key('profiles', 'userid'); $dbh->bz_unlock_tables(); + my $newprofile = new Bugzilla::User($otherUserID); + $newprofile->derive_regexp_groups(); userDataToVars($otherUserID); $vars->{'message'} = 'account_created'; @@ -290,6 +292,12 @@ if ($action eq 'search') { 'WHERE userid = ?', undef, @values); # XXX: should create profiles_activity entries. + # + # We create a new user object here because it needs to + # read information that may have changed since this + # script started. + my $newprofile = new Bugzilla::User($otherUserID); + $newprofile->derive_regexp_groups(); } } @@ -639,7 +647,7 @@ sub userDataToVars { my $query; my $dbh = Bugzilla->dbh; - $otheruser->derive_groups(); + my $grouplist = $otheruser->groups_as_string; $vars->{'otheruser'} = $otheruser; $vars->{'groups'} = $user->bless_groups(); @@ -648,7 +656,7 @@ sub userDataToVars { qq{SELECT id, COUNT(directmember.group_id) AS directmember, COUNT(regexpmember.group_id) AS regexpmember, - COUNT(derivedmember.group_id) AS derivedmember, + CASE WHEN groups.id IN ($grouplist) THEN 1 ELSE 0 END, COUNT(directbless.group_id) AS directbless FROM groups LEFT JOIN user_group_map AS directmember @@ -661,11 +669,6 @@ sub userDataToVars { AND regexpmember.user_id = ? AND regexpmember.isbless = 0 AND regexpmember.grant_type = ? - LEFT JOIN user_group_map AS derivedmember - ON derivedmember.group_id = id - AND derivedmember.user_id = ? - AND derivedmember.isbless = 0 - AND derivedmember.grant_type = ? LEFT JOIN user_group_map AS directbless ON directbless.group_id = id AND directbless.user_id = ? @@ -675,20 +678,17 @@ sub userDataToVars { 'id', undef, ($otheruserid, GRANT_DIRECT, $otheruserid, GRANT_REGEXP, - $otheruserid, GRANT_DERIVED, $otheruserid, GRANT_DIRECT)); # Find indirect bless permission. $query = qq{SELECT groups.id - FROM groups, user_group_map AS ugm, group_group_map AS ggm - WHERE ugm.user_id = ? - AND groups.id = ggm.grantor_id - AND ggm.member_id = ugm.group_id - AND ugm.isbless = 0 + FROM groups, group_group_map AS ggm + WHERE groups.id = ggm.grantor_id + AND ggm.member_id IN ($grouplist) AND ggm.grant_type = ? } . $dbh->sql_group_by('id'); foreach (@{$dbh->selectall_arrayref($query, undef, - ($otheruserid, GROUP_BLESS))}) { + (GROUP_BLESS))}) { # Merge indirect bless permissions into permission variable. $vars->{'permissions'}{${$_}[0]}{'indirectbless'} = 1; } diff --git a/post_bug.cgi b/post_bug.cgi index db95cbc5e..f0c2de65a 100755 --- a/post_bug.cgi +++ b/post_bug.cgi @@ -335,11 +335,7 @@ foreach my $b (grep(/^bit-\d*$/, $cgi->param())) { $vars->{'bit'} = $v; ThrowCodeError("inactive_group"); } - SendSQL("SELECT user_id FROM user_group_map - WHERE user_id = $::userid - AND group_id = $v - AND isbless = 0"); - my ($permit) = FetchSQLData(); + my ($permit) = $user->in_group_id($v); if (!$permit) { SendSQL("SELECT othercontrol FROM group_control_map WHERE group_id = $v AND product_id = $product_id"); diff --git a/process_bug.cgi b/process_bug.cgi index b0decb861..543c9bf0e 100755 --- a/process_bug.cgi +++ b/process_bug.cgi @@ -73,6 +73,7 @@ use vars qw(@legal_product my $user = Bugzilla->login(LOGIN_REQUIRED); my $whoid = $user->id; +my $grouplist = $user->groups_as_string; my $cgi = Bugzilla->cgi; my $dbh = Bugzilla->dbh; @@ -704,10 +705,9 @@ sub ChangeResolution { my @groupAdd = (); my @groupDel = (); -SendSQL("SELECT groups.id, isactive FROM groups INNER JOIN user_group_map " . - "ON groups.id = user_group_map.group_id " . - "WHERE user_group_map.user_id = $whoid " . - "AND isbless = 0 AND isbuggroup = 1"); +SendSQL("SELECT groups.id, isactive FROM groups " . + "WHERE id IN($grouplist) " . + "AND isbuggroup = 1"); while (my ($b, $isactive) = FetchSQLData()) { # The multiple change page may not show all groups a bug is in # (eg product groups when listing more than one product) @@ -1560,7 +1560,7 @@ foreach my $id (@idlist) { # - Is the bug in this group? SendSQL("SELECT DISTINCT groups.id, isactive, " . "oldcontrolmap.membercontrol, newcontrolmap.membercontrol, " . - "user_group_map.user_id IS NOT NULL, " . + "CASE WHEN groups_id IN ($grouplist) THEN 1 ELSE 0 END, " . "bug_group_map.group_id IS NOT NULL " . "FROM groups " . "LEFT JOIN group_control_map AS oldcontrolmap " . @@ -1569,10 +1569,6 @@ foreach my $id (@idlist) { " LEFT JOIN group_control_map AS newcontrolmap " . "ON newcontrolmap.group_id = groups.id " . "AND newcontrolmap.product_id = $newproduct_id " . - "LEFT JOIN user_group_map " . - "ON user_group_map.group_id = groups.id " . - "AND user_group_map.user_id = $whoid " . - "AND user_group_map.isbless = 0 " . "LEFT JOIN bug_group_map " . "ON bug_group_map.group_id = groups.id " . "AND bug_group_map.bug_id = $id " diff --git a/sanitycheck.cgi b/sanitycheck.cgi index b5272d205..0ae58591c 100755 --- a/sanitycheck.cgi +++ b/sanitycheck.cgi @@ -118,66 +118,6 @@ if (defined $cgi->param('rebuildvotecache')) { } ########################################################################### -# Fix group derivations -########################################################################### - -if (defined $cgi->param('rederivegroups')) { - Status("OK, All users' inherited permissions will be rechecked when " . - "they next access Bugzilla."); - SendSQL("UPDATE groups SET last_changed = NOW() " . $dbh->sql_limit(1)); -} - -# rederivegroupsnow is REALLY only for testing. -# If it wasn't, then we'd do this the faster way as a per-group -# thing rather than per-user for group inheritance -if (defined $cgi->param('rederivegroupsnow')) { - require Bugzilla::User; - Status("OK, now rederiving groups."); - SendSQL("SELECT userid FROM profiles"); - while ((my $id) = FetchSQLData()) { - my $user = new Bugzilla::User($id); - $user->derive_groups(); - Status("User $id"); - } -} - -if (defined $cgi->param('cleangroupsnow')) { - Status("OK, now cleaning stale groups."); - # Only users that were out of date already long ago should be cleaned - # and the cleaning is done with tables locked. This is require in order - # to keep another session from proceeding with permission checks - # after the groups have been cleaned unless it first had an opportunity - # to get the groups up to date. - # If any page starts taking longer than one hour to load, this interval - # should be revised. - SendSQL("SELECT MAX(last_changed) FROM groups WHERE last_changed < NOW() - " . - $dbh->sql_interval('1 HOUR')); - (my $cutoff) = FetchSQLData(); - Status("Cutoff is $cutoff"); - SendSQL("SELECT COUNT(*) FROM user_group_map"); - (my $before) = FetchSQLData(); - $dbh->bz_lock_tables('user_group_map WRITE', 'profiles WRITE'); - SendSQL("SELECT userid FROM profiles " . - "WHERE refreshed_when > 0 " . - "AND refreshed_when < " . SqlQuote($cutoff) . " " . - $dbh->sql_limit(1000)); - my $count = 0; - while ((my $id) = FetchSQLData()) { - $count++; - PushGlobalSQLState(); - SendSQL("DELETE FROM user_group_map WHERE " . - "user_id = $id AND isderived = 1 AND isbless = 0"); - SendSQL("UPDATE profiles SET refreshed_when = 0 WHERE userid = $id"); - PopGlobalSQLState(); - } - $dbh->bz_unlock_tables(); - SendSQL("SELECT COUNT(*) FROM user_group_map"); - (my $after) = FetchSQLData(); - Status("Cleaned table for $count users " . - "- reduced from $before records to $after records"); -} - -########################################################################### # Create missing group_control_map entries ########################################################################### @@ -268,7 +268,7 @@ sub changeEmail { # The email address has been changed, so we need to rederive the groups my $user = new Bugzilla::User($userid); - $user->derive_groups; + $user->derive_regexp_groups; # Return HTTP response headers. print $cgi->header(); @@ -313,7 +313,7 @@ sub cancelChangeEmail { # issue my $user = new Bugzilla::User($userid); - $user->derive_groups; + $user->derive_regexp_groups; $vars->{'message'} = "email_change_cancelled_reinstated"; } diff --git a/userprefs.cgi b/userprefs.cgi index 065dcb472..14b28f6bb 100755 --- a/userprefs.cgi +++ b/userprefs.cgi @@ -311,11 +311,9 @@ sub DoPermissions { my (@has_bits, @set_bits); SendSQL("SELECT DISTINCT name, description FROM groups " . - "INNER JOIN user_group_map " . - "ON user_group_map.group_id = groups.id " . - "WHERE user_id = $::userid " . - "AND isbless = 0 " . - "ORDER BY name"); + "WHERE id IN (" . + Bugzilla->user->groups_as_string . + ") ORDER BY name"); while (MoreSQLData()) { my ($nam, $desc) = FetchSQLData(); push(@has_bits, {"desc" => $desc, "name" => $nam}); @@ -240,8 +240,7 @@ sub get_next_event { return undef unless $fetched; my ($eventid, $owner_id, $subject, $body) = @{$fetched}; - my $owner = Bugzilla::User->new($owner_id, - DERIVE_GROUPS_TABLES_ALREADY_LOCKED); + my $owner = Bugzilla::User->new($owner_id); my $whineatothers = $owner->in_group('bz_canusewhineatothers'); @@ -275,10 +274,13 @@ sub get_next_event { my $group_id = Bugzilla::Group::ValidateGroupName( $groupname, $owner); if ($group_id) { + my $glist = join(',', + Bugzilla::User->flatten_group_membership( + $group_id)); $sth = $dbh->prepare("SELECT user_id FROM " . "user_group_map " . - "WHERE group_id=?"); - $sth->execute($group_id); + "WHERE group_id IN ($glist)"); + $sth->execute(); for my $row (@{$sth->fetchall_arrayref}) { if (not defined $user_objects{$row->[0]}) { $user_objects{$row->[0]} = |