diff options
31 files changed, 647 insertions, 437 deletions
diff --git a/Bugzilla.pm b/Bugzilla.pm index cded650d7..0ce6a5e72 100644 --- a/Bugzilla.pm +++ b/Bugzilla.pm @@ -77,17 +77,15 @@ sub login { # Compat stuff $::userid = $userid; - &::ConfirmGroup($userid); # Evil compat hack. The cookie stores the id now, not the name, but # old code still looks at this to get the current user's email # so it needs to be set. - $::COOKIE{'Bugzilla_login'} = $_user->{email}; - - $::vars->{'user'} = &::GetUserInfo($userid); + $::COOKIE{'Bugzilla_login'} = $_user->login; } else { # Old compat stuff + undef $_user; $::userid = 0; delete $::COOKIE{'Bugzilla_login'}; delete $::COOKIE{'Bugzilla_logincookie'}; @@ -97,7 +95,12 @@ sub login { # - use Bugzilla->user instead! } - return $userid || 0; + return $_user; +} + +sub logout { + undef $_user; + $::userid = 0; } my $_dbh; @@ -257,8 +260,16 @@ or if the login code has not yet been run. =item C<login> -Logs in a user, returning the userid, or C<0> if there is no logged in user. -See L<Bugzilla::Auth>. +Logs in a user, returning a C<Bugzilla::User> object, or C<undef> if there is +no logged in user. See L<Bugzilla::Auth|Bugzilla::Auth> and +L<Bugzilla::User|Bugzilla::User>. + +=item C<logout> + +Logs out the current user. For the moment, this will just cause calls to +C<user> to return C<undef>. Eventually this will handle deleting cookies from +the browser and values from the database, which is currently all handled +by C<relogin.cgi>. =item C<dbh> diff --git a/Bugzilla/BugMail.pm b/Bugzilla/BugMail.pm index da25b6a70..16b44d6ff 100644 --- a/Bugzilla/BugMail.pm +++ b/Bugzilla/BugMail.pm @@ -111,14 +111,6 @@ sub Send($;$) { # require abuse we do. GetVersionTable(); - # Since any email recipients must be rederived if the user has not - # been rederived since the most recent group change, figure out when that - # is once and determine the need to rederive users using the same DB - # access that gets the user's email address each time a person is - # processed. - SendSQL("SELECT MAX(last_changed) FROM groups"); - ($last_changed) = FetchSQLData(); - # Make sure to clean up _all_ package vars here. Yuck... $nametoexclude = $recipients->{'changer'} || ""; @{$force{'CClist'}} = (exists $recipients->{'cc'} && @@ -710,19 +702,13 @@ sub NewProcessOnePerson ($$$$$$$$$$$$$) { return; } - - SendSQL("SELECT userid, (refreshed_when > " . SqlQuote($last_changed) . - ") FROM profiles WHERE login_name = " . SqlQuote($person)); - my ($userid, $current) = (FetchSQLData()); + # This routine should really get passed a userid + # This rederives groups as a side effect + my $user = Bugzilla::User->new_from_login($person); + my $userid = $user->id; $seen{$person} = 1; - detaint_natural($userid); - - if (!$current) { - DeriveGroup($userid); - } - # if this person doesn't have permission to see info on this bug, # return. # @@ -732,12 +718,11 @@ sub NewProcessOnePerson ($$$$$$$$$$$$$) { # quietly disappear from their radar. # return unless CanSeeBug($id, $userid); - # Drop any non-insiders if the comment is private - return if (Param("insidergroup") && + return if (Param("insidergroup") && ($anyprivate != 0) && - (!UserInGroup(Param("insidergroup"), $userid))); + (!$user->groups->{Param("insidergroup")})); # We shouldn't send changedmail if this is a dependency mail, and any of # the depending bugs is not visible to the user. @@ -761,8 +746,8 @@ sub NewProcessOnePerson ($$$$$$$$$$$$$) { } # Don't send estimated_time if user not in the group, or not enabled if ($f ne 'estimated_time' || - UserInGroup(Param('timetrackinggroup'), $userid)) { - + $user->groups->{Param('timetrackinggroup')}) { + my $desc = $fielddescription{$f}; $head .= FormatDouble($desc, $value); } @@ -781,7 +766,7 @@ sub NewProcessOnePerson ($$$$$$$$$$$$$) { ($diff->{'fieldname'} eq 'estimated_time' || $diff->{'fieldname'} eq 'remaining_time' || $diff->{'fieldname'} eq 'work_time')) { - if (UserInGroup(Param("timetrackinggroup"), $userid)) { + if ($user->groups->{Param("timetrackinggroup")}) { $add_diff = 1; } } else { diff --git a/Bugzilla/Error.pm b/Bugzilla/Error.pm index 485646274..a23d6db15 100644 --- a/Bugzilla/Error.pm +++ b/Bugzilla/Error.pm @@ -34,9 +34,6 @@ sub ThrowUserError { $vars->{error} = $error; - # Need to do this until User.pm goes in, so that the footer is correct - $vars->{user} = $::vars->{user}; - Bugzilla->dbh->do("UNLOCK TABLES") if $unlock_tables; print Bugzilla->cgi->header(); diff --git a/Bugzilla/Flag.pm b/Bugzilla/Flag.pm index f8eb8a4a4..4a1752cd0 100644 --- a/Bugzilla/Flag.pm +++ b/Bugzilla/Flag.pm @@ -177,18 +177,14 @@ sub validate { if ($requestee_email ne $flag->{'requestee'}->{'email'}) { # We know the requestee exists because we ran # Bugzilla::User::match_field before getting here. - # ConfirmGroup makes sure their group settings - # are up-to-date or calls DeriveGroups to update them. - my $requestee_id = &::DBname_to_id($requestee_email); - &::ConfirmGroup($requestee_id); + my $requestee = Bugzilla::User->new_from_login($requestee_email); # Throw an error if the user can't see the bug. - if (!&::CanSeeBug($bug_id, $requestee_id)) + if (!&::CanSeeBug($bug_id, $requestee->id)) { ThrowUserError("flag_requestee_unauthorized", { flag_type => $flag->{'type'}, - requestee => - new Bugzilla::User($requestee_id), + requestee => $requestee, bug_id => $bug_id, attach_id => $flag->{target}->{attachment}->{id} }); @@ -198,13 +194,12 @@ sub validate { # the requestee isn't in the group of insiders who can see it. if ($flag->{target}->{attachment}->{exists} && $data->{'isprivate'} - && &::Param("insidergroup") - && !&::UserInGroup(&::Param("insidergroup"), $requestee_id)) + && Param("insidergroup") + && !$requestee->in_group(Param("insidergroup"))) { ThrowUserError("flag_requestee_unauthorized_attachment", { flag_type => $flag->{'type'}, - requestee => - new Bugzilla::User($requestee_id), + requestee => $requestee, bug_id => $bug_id, attach_id => $flag->{target}->{attachment}->{id} }); @@ -236,7 +231,7 @@ sub process { my @old_summaries; foreach my $flag (@$flags) { my $summary = $flag->{'type'}->{'name'} . $flag->{'status'}; - $summary .= "($flag->{'requestee'}->{'email'})" if $flag->{'requestee'}; + $summary .= "(" . $flag->{'requestee'}->login . ")" if $flag->{'requestee'}; push(@old_summaries, $summary); } @@ -275,7 +270,7 @@ sub process { my @new_summaries; foreach my $flag (@$flags) { my $summary = $flag->{'type'}->{'name'} . $flag->{'status'}; - $summary .= "($flag->{'requestee'}->{'email'})" if $flag->{'requestee'}; + $summary .= "(" . $flag->{'requestee'}->login . ")" if $flag->{'requestee'}; push(@new_summaries, $summary); } @@ -307,7 +302,7 @@ sub create { # Insert a record for the flag into the flags table. my $attach_id = $flag->{'target'}->{'attachment'}->{'id'} || "NULL"; - my $requestee_id = $flag->{'requestee'} ? $flag->{'requestee'}->{'id'} : "NULL"; + my $requestee_id = $flag->{'requestee'} ? $flag->{'requestee'}->id : "NULL"; &::SendSQL("INSERT INTO flags (id, type_id, bug_id, attach_id, requestee_id, setter_id, status, @@ -317,7 +312,7 @@ sub create { $flag->{'target'}->{'bug'}->{'id'}, $attach_id, $requestee_id, - $flag->{'setter'}->{'id'}, + " . $flag->{'setter'}->id . ", '$flag->{'status'}', $timestamp, $timestamp)"); @@ -380,7 +375,7 @@ sub modify { # the flag isn't specifically requestable || $status ne "?" # or the flag isn't being requested || ($flag->{'requestee'} # or the requestee hasn't changed - && ($requestee_email eq $flag->{'requestee'}->{'email'}))); + && ($requestee_email eq $flag->{'requestee'}->login))); # Since the status is validated, we know it's safe, but it's still # tainted, so we have to detaint it before using it in a query. @@ -568,14 +563,15 @@ sub notify { { my @new_cc_list; foreach my $cc (split(/[, ]+/, $flag->{'type'}->{'cc_list'})) { - my $user_id = &::DBname_to_id($cc) || next; - # re-derive permissions if necessary - &::ConfirmGroup($user_id, TABLES_ALREADY_LOCKED); + my $ccuser = Bugzilla::User->new_from_login($cc, + TABLES_ALREADY_LOCKED) + || next; + next if $flag->{'target'}->{'bug'}->{'restricted'} - && !&::CanSeeBug($flag->{'target'}->{'bug'}->{'id'}, $user_id); + && !&::CanSeeBug($flag->{'target'}->{'bug'}->{'id'}, $ccuser->id); next if $flag->{'target'}->{'attachment'}->{'isprivate'} && Param("insidergroup") - && !&::UserInGroup(Param("insidergroup"), $user_id); + && !$ccuser->in_group(Param("insidergroup")); push(@new_cc_list, $cc); } $flag->{'type'}->{'cc_list'} = join(", ", @new_cc_list); @@ -646,7 +642,7 @@ sub perlify_record { id => $id , type => Bugzilla::FlagType::get($type_id) , target => GetTarget($bug_id, $attach_id) , - requestee => new Bugzilla::User($requestee_id) , + requestee => $requestee_id ? new Bugzilla::User($requestee_id) : undef, setter => new Bugzilla::User($setter_id) , status => $status , }; diff --git a/Bugzilla/FlagType.pm b/Bugzilla/FlagType.pm index 523f60190..7fbe1f142 100644 --- a/Bugzilla/FlagType.pm +++ b/Bugzilla/FlagType.pm @@ -219,20 +219,17 @@ sub validate { && trim($data->{"requestee_type-$id"})) { my $requestee_email = trim($data->{"requestee_type-$id"}); - my $requestee_id = &::DBname_to_id($requestee_email); # We know the requestee exists because we ran # Bugzilla::User::match_field before getting here. - # ConfirmGroup makes sure their group settings - # are up-to-date or calls DeriveGroups to update them. - &::ConfirmGroup($requestee_id); + my $requestee = Bugzilla::User->new_from_login($requestee_email); # Throw an error if the user can't see the bug. - if (!&::CanSeeBug($bug_id, $requestee_id)) + if (!&::CanSeeBug($bug_id, $requestee->id)) { ThrowUserError("flag_requestee_unauthorized", { flag_type => $flag_type, - requestee => new Bugzilla::User($requestee_id), + requestee => $requestee, bug_id => $bug_id, attach_id => $attach_id }); } @@ -240,13 +237,13 @@ sub validate { # Throw an error if the target is a private attachment and # the requestee isn't in the group of insiders who can see it. if ($attach_id - && &::Param("insidergroup") + && Param("insidergroup") && $data->{'isprivate'} - && !&::UserInGroup(&::Param("insidergroup"), $requestee_id)) + && !$requestee->in_group(Param("insidergroup"))) { ThrowUserError("flag_requestee_unauthorized_attachment", { flag_type => $flag_type, - requestee => new Bugzilla::User($requestee_id), + requestee => $requestee, bug_id => $bug_id, attach_id => $attach_id }); } diff --git a/Bugzilla/Search.pm b/Bugzilla/Search.pm index 45c26fdf2..df2ab58e3 100644 --- a/Bugzilla/Search.pm +++ b/Bugzilla/Search.pm @@ -926,28 +926,31 @@ sub init { # Make sure we create a legal SQL query. @andlist = ("1 = 1") if !@andlist; + my $user = Bugzilla->user; + my $query = "SELECT " . join(', ', @fields) . " FROM $suppstring" . " LEFT JOIN bug_group_map " . " ON bug_group_map.bug_id = bugs.bug_id "; - if (defined @{$::vars->{user}{groupids}} && @{$::vars->{user}{groupids}} > 0) { - $query .= " AND bug_group_map.group_id NOT IN (" . join(',', @{$::vars->{user}{groupids}}) . ") "; - } + if ($user) { + if (%{$user->groups}) { + $query .= " AND bug_group_map.group_id NOT IN (" . join(',', values(%{$user->groups})) . ") "; + } - if ($::vars->{user}{userid}) { - $query .= " LEFT JOIN cc ON cc.bug_id = bugs.bug_id AND cc.who = $::userid "; + $query .= " LEFT JOIN cc ON cc.bug_id = bugs.bug_id AND cc.who = " . $user->id; } $query .= " WHERE " . join(' AND ', (@wherepart, @andlist)) . " AND ((bug_group_map.group_id IS NULL)"; - if ($::vars->{user}{userid}) { - $query .= " OR (bugs.reporter_accessible = 1 AND bugs.reporter = $::userid) " . + if ($user) { + my $userid = $user->id; + $query .= " OR (bugs.reporter_accessible = 1 AND bugs.reporter = $userid) " . " OR (bugs.cclist_accessible = 1 AND cc.who IS NOT NULL) " . - " OR (bugs.assigned_to = $::userid) "; + " OR (bugs.assigned_to = $userid) "; if (Param('useqacontact')) { - $query .= "OR (bugs.qa_contact = $::userid) "; + $query .= "OR (bugs.qa_contact = $userid) "; } } diff --git a/Bugzilla/Template.pm b/Bugzilla/Template.pm index e596af226..7c084ecb9 100644 --- a/Bugzilla/Template.pm +++ b/Bugzilla/Template.pm @@ -256,7 +256,10 @@ sub create { # Generic linear search function 'lsearch' => \&Bugzilla::Util::lsearch, - # UserInGroup - you probably want to cache this + # Currently logged in user, if any + 'user' => sub { return Bugzilla->user; }, + + # UserInGroup. Deprecated - use the user.* functions instead 'UserInGroup' => \&::UserInGroup, # SendBugMail - sends mail about a bug, using Bugzilla::BugMail.pm diff --git a/Bugzilla/User.pm b/Bugzilla/User.pm index fde9d336b..f5df92063 100644 --- a/Bugzilla/User.pm +++ b/Bugzilla/User.pm @@ -19,6 +19,8 @@ # # Contributor(s): Myk Melez <myk@mozilla.org> # Erik Stambaugh <not_erik@dasbistro.com> +# Bradley Baetz <bbaetz@acm.org> +# Joel Peshkin <bugreport@peshkin.net> ################################################################################ # Module Initialization @@ -30,57 +32,311 @@ use strict; # This module implements utilities for dealing with Bugzilla users. package Bugzilla::User; +use Bugzilla::Config; +use Bugzilla::Util; + ################################################################################ # Functions ################################################################################ -my $user_cache = {}; sub new { - # Returns a hash of information about a particular user. + my $invocant = shift; + return $invocant->_create("userid=?", @_); +} +# This routine is sort of evil. Nothing except the login stuff should +# be dealing with addresses as an input, and they can get the id as a +# side effect of the other sql they have to do anyway. +# Bugzilla::BugMail still does this, probably as a left over from the +# pre-id days. Provide this as a helper, but don't document it, and hope +# that it can go away. +# The request flag stuff also does this, but it really should be passing +# in the id its already had to validate (or the User.pm object, of course) +sub new_from_login { + my $invocant = shift; + return $invocant->_create("login_name=?", @_); +} + +# Internal helper for the above |new| methods +# $cond is a string (including a placeholder ?) for the search +# requirement for the profiles table +sub _create { my $invocant = shift; my $class = ref($invocant) || $invocant; - - my $exists = 1; - my ($id, $name, $email) = @_; - - return undef if !$id; - return $user_cache->{$id} if exists($user_cache->{$id}); - - my $self = { 'id' => $id }; - - bless($self, $class); - - if (!$name && !$email) { - &::PushGlobalSQLState(); - &::SendSQL("SELECT 1, realname, login_name FROM profiles WHERE userid = $id"); - ($exists, $name, $email) = &::FetchSQLData(); - &::PopGlobalSQLState(); + + my $cond = shift; + my $val = shift; + + # 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, + $login, + $name, + $mybugslink) = $dbh->selectrow_array(qq{SELECT userid, + login_name, + realname, + mybugslink + FROM profiles + WHERE $cond}, + undef, + $val); + + return undef unless defined $id; + + my $self = { id => $id, + name => $name, + login => $login, + showmybugslink => $mybugslink, + }; + + bless ($self, $class); + + # 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) { + $self->derive_groups($tables_locked_for_derive_groups); } - - $self->{'name'} = $name; - $self->{'email'} = $email || "__UNKNOWN__"; - $self->{'exists'} = $exists; - - # Generate a string to identify the user by name + email if the user - # has a name or by email only if she doesn't. - $self->{'identity'} = $name ? "$name <$email>" : $email; - - # Generate a user "nickname" -- i.e. a shorter, not-necessarily-unique name - # by which to identify the user. Currently the part of the user's email - # address before the at sign (@), but that could change, especially if we - # implement usernames not dependent on email address. - my @email_components = split("@", $email); - $self->{'nick'} = $email_components[0]; - - $user_cache->{$id} = $self; - + return $self; } +# Accessors for user attributes +sub id { $_[0]->{id}; } +sub login { $_[0]->{login}; } +sub email { $_[0]->{login}; } +sub name { $_[0]->{name}; } +sub showmybugslink { $_[0]->{showmybugslink}; } + +# Generate a string to identify the user by name + email if the user +# has a name or by email only if she doesn't. +sub identity { + my $self = shift; + + if (!defined $self->{identity}) { + $self->{identity} = + $self->{name} ? "$self->{name} <$self->{login}>" : $self->{login}; + } + + return $self->{identity}; +} + +sub nick { + my $self = shift; + + if (!defined $self->{nick}) { + $self->{nick} = (split(/@/, $self->{login}, 2))[0]; + } + + return $self->{nick}; +} + +sub queries { + my $self = shift; + + return $self->{queries} if defined $self->{queries}; + + my $dbh = Bugzilla->dbh; + my $sth = $dbh->prepare(q{ SELECT name, query, linkinfooter + FROM namedqueries + WHERE userid=? + ORDER BY UPPER(name)}); + $sth->execute($self->{id}); + + my @queries; + while (my $row = $sth->fetch) { + push (@queries, { + name => $row->[0], + query => $row->[1], + linkinfooter => $row->[2], + }); + } + $self->{queries} = \@queries; + + return $self->{queries}; +} + +sub flush_queries_cache { + my $self = shift; + + delete $self->{queries}; +} + +sub groups { + my $self = shift; + + return $self->{groups} if defined $self->{groups}; + + my $dbh = Bugzilla->dbh; + my $groups = $dbh->selectcol_arrayref(q{SELECT DISTINCT groups.name, group_id + FROM groups, user_group_map + WHERE groups.id=user_group_map.group_id + AND user_id=? + AND isbless=0}, + { Columns=>[1,2] }, + $self->{id}); + + # The above gives us an arrayref [name, id, name, id, ...] + # Convert that into a hashref + my %groups = @$groups; + $self->{groups} = \%groups; + + return $self->{groups}; +} + +sub in_group { + my ($self, $group) = @_; + + # If we already have the info, just return it. + return defined($self->{groups}->{$group}) if defined $self->{groups}; + + # Otherwise, go check for it + + my $dbh = Bugzilla->dbh; + + my $res = $dbh->selectrow(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 derive_groups { + my ($self, $already_locked) = @_; + + my $id = $self->id; + + my $dbh = Bugzilla->dbh; + + my $sth; + + $dbh->do(q{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 isderived = 1}, + undef, + $id); + + 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, isderived) + VALUES (?, ?, 0, 1)}); + $groupidsadded{$row->[0]} = 1; + $group_insert->execute($id, $row->[0]); + } + } + + # 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 isbless=0}); + $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, isderived) + VALUES (?, ?, 0, 1)}); + $group_insert->execute($id, $groupid); + } + } + } + } + + $dbh->do(q{UPDATE profiles + SET refreshed_when = ? + WHERE userid=?}, + undef, + $time, + $id); + $dbh->do("UNLOCK TABLES") unless $already_locked; +} + +sub can_bless { + my $self = shift; + + return $self->{can_bless} if defined $self->{can_bless}; + + my $dbh = Bugzilla->dbh; + # First check if the user can explicitly bless a group + my $res = $dbh->selectrow_arrayref(q{SELECT 1 + FROM user_group_map + WHERE user_id=? + AND isbless=1}, + undef, + $self->{id}); + if (!$res) { + # Now check if user is a member of a group that can bless a group + $res = $dbh->selectrow_arrayref(q{SELECT 1 + FROM user_group_map, group_group_map + WHERE user_group_map.user_id=? + AND user_group_map.group_id=member_id + AND group_group_map.isbless=1}, + undef, + $self->{id}); + } + + $self->{can_bless} = $res ? 1 : 0; + + return $self->{can_bless}; +} + sub match { # Generates a list of users whose login name (email address) or real name # matches a substring or wildcard. + # This is also called if matches are disabled (for error checking), but + # in this case only the exact match code will end up running. # $str contains the string to match, while $limit contains the # maximum number of records to retrieve. @@ -99,7 +355,8 @@ sub match { my $wildstr = $str; - if ($wildstr =~ s/\*/\%/g) { # don't do wildcards if no '*' in the string + if ($wildstr =~ s/\*/\%/g && # don't do wildcards if no '*' in the string + Param('usermatchmode') ne 'off') { # or if we only want exact matches # Build the query. my $sqlstr = &::SqlQuote($wildstr); @@ -159,7 +416,7 @@ sub match { # order @users by alpha - @users = sort { uc($a->{'email'}) cmp uc($b->{'email'}) } @users; + @users = sort { uc($a->login) cmp uc($b->login) } @users; return \@users; } @@ -251,9 +508,6 @@ sub match_field { } $fields = $expanded_fields; - # Skip all of this if the option has been turned off - return 1 if (&::Param('usermatchmode') eq 'off'); - for my $field (keys %{$fields}) { # Tolerate fields that do not exist. @@ -312,14 +566,14 @@ sub match_field { # skip confirmation for exact matches if ((scalar(@{$users}) == 1) - && (@{$users}[0]->{'email'} eq $query)) + && (@{$users}[0]->{'login'} eq $query)) { # delimit with spaces if necessary if ($vars->{'form'}->{$field}) { $vars->{'form'}->{$field} .= " "; } - $vars->{'form'}->{$field} .= @{$users}[0]->{'email'}; - push @{$vars->{'mform'}->{$field}}, @{$users}[0]->{'email'}; + $vars->{'form'}->{$field} .= @{$users}[0]->{'login'}; + push @{$vars->{'mform'}->{$field}}, @{$users}[0]->{'login'}; next; } @@ -333,8 +587,8 @@ sub match_field { if ($vars->{'form'}->{$field}) { $vars->{'form'}->{$field} .= " "; } - $vars->{'form'}->{$field} .= @{$users}[0]->{'email'}; - push @{$vars->{'mform'}->{$field}}, @{$users}[0]->{'email'}; + $vars->{'form'}->{$field} .= @{$users}[0]->{'login'}; + push @{$vars->{'mform'}->{$field}}, @{$users}[0]->{'login'}; $need_confirm = 1 if &::Param('confirmuniqueusermatch'); } @@ -443,3 +697,159 @@ sub email_prefs { } 1; + +__END__ + +=head1 NAME + +Bugzilla::User - Object for a Bugzilla user + +=head1 SYNOPSIS + + use Bugzilla::User; + + my $user = new Bugzilla::User($id); + +=head1 DESCRIPTION + +This package handles Bugzilla users. Data obtained from here is read-only; +there is currently no way to modify a user from this package. + +Note that the currently logged in user (if any) is available via +L<Bugzilla-E<gt>user|Bugzilla/"user">. + +=head1 METHODS + +=over 4 + +=item C<new($userid)> + +Creates a new C<Bugzilla::User> object for the given user id. Returns +C<undef> if no matching user is found. + +=begin undocumented + +=item C<new_from_login($login)> + +Creates a new C<Bugzilla::User> object given the provided login. Returns +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> + +Returns the userid for this user. + +=item C<login> + +Returns the login name for this user. + +=item C<email> + +Returns the user's email address. Currently this is the same value as the +login. + +=item C<name> + +Returns the 'real' name for this user, if any. + +=item C<showmybugslink> + +Returns C<1> if the user has set his preference to show the 'My Bugs' link in +the page footer, and C<0> otherwise. + +=item C<identity> + +Retruns a string for the identity of the user. This will be of the form +C<name E<lt>emailE<gt>> if the user has specified a name, and C<email> +otherwise. + +=item C<nick> + +Returns a user "nickname" -- i.e. a shorter, not-necessarily-unique name by +which to identify the user. Currently the part of the user's email address +before the at sign (@), but that could change, especially if we implement +usernames not dependent on email address. + +=item C<queries> + +Returns an array of the user's named queries, sorted in a case-insensitive +order by name. Each entry is a hash with three keys: + +=over + +=item * + +name - The name of the query + +=item * + +query - The text for the query + +=item * + +linkinfooter - Whether or not the query should be displayed in the footer. + +=back + +=item C<flush_queries_cache> + +Some code modifies the set of stored queries. Because C<Bugzilla::User> does +not handle these modifications, but does cache the result of calling C<queries> +internally, such code must call this method to flush the cached result. + +=item C<groups> + +Returns a hashref of group names for groups the user is a member of. 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 is in can be +obtained by C<values(%{$user->groups})>.) + +=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. + +=item C<derive_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 +care of by the constructor. However, when updating the email address, the +user may be placed into different groups, based on a new email regexp. This +method should be called in such a case to force reresolution of these groups. + +=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 ahve 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<can_bless> + +Returns C<1> if the user can bless at least one group. Otherwise returns C<0>. + +=back + +=head1 SEE ALSO + +L<Bugzilla|Bugzilla> @@ -202,53 +202,6 @@ sub quietly_check_login { return Bugzilla->login($_[0] ? LOGIN_OPTIONAL : LOGIN_NORMAL); } -# Populate a hash with information about this user. -sub GetUserInfo { - my ($userid) = (@_); - my %user; - my @queries; - my %groups; - my @groupids; - - # No info if not logged in - return \%user if ($userid == 0); - - $user{'login'} = $::COOKIE{"Bugzilla_login"}; - $user{'userid'} = $userid; - - SendSQL("SELECT mybugslink, realname " . - "FROM profiles WHERE userid = $userid"); - ($user{'showmybugslink'}, $user{'realname'}) = FetchSQLData(); - - SendSQL("SELECT name, query, linkinfooter FROM namedqueries " . - "WHERE userid = $userid"); - while (MoreSQLData()) { - my %query; - ($query{'name'}, $query{'query'}, $query{'linkinfooter'}) = - FetchSQLData(); - push(@queries, \%query); - } - - $user{'queries'} = \@queries; - - $user{'canblessany'} = UserCanBlessAnything(); - - SendSQL("SELECT DISTINCT id, name FROM groups, user_group_map " . - "WHERE groups.id = user_group_map.group_id " . - "AND user_id = $userid " . - "AND NOT isbless"); - while (MoreSQLData()) { - my ($id, $name) = FetchSQLData(); - push(@groupids,$id); - $groups{$name} = 1; - } - - $user{'groups'} = \%groups; - $user{'groupids'} = \@groupids; - - return \%user; -} - sub CheckEmailSyntax { my ($addr) = (@_); my $match = Param('emailregexp'); diff --git a/attachment.cgi b/attachment.cgi index ece6522d9..e70fb88f4 100755 --- a/attachment.cgi +++ b/attachment.cgi @@ -693,11 +693,11 @@ sub update "flaginclusions AS i READ, flagexclusions AS e READ, " . # cc, bug_group_map, user_group_map, and groups are in here so we # can check the permissions of flag requestees and email addresses - # on the flag type cc: lists via the ConfirmGroup and CanSeeBug - # function calls in Flag::notify. group_group_map is in here in case - # ConfirmGroup needs to call DeriveGroup. profiles and user_group_map - # would be READ locks instead of WRITE locks if it weren't for - # DeriveGroup, which needs to write to those tables. + # on the flag type cc: lists via the CanSeeBug + # function call in Flag::notify. group_group_map is in here in case + # Bugzilla::User needs to rederive groups. profiles and + # user_group_map would be READ locks instead of WRITE locks if it + # weren't for derive_groups, which needs to write to those tables. "bugs READ, profiles WRITE, " . "cc READ, bug_group_map READ, user_group_map WRITE, " . "group_group_map READ, groups READ"); diff --git a/buglist.cgi b/buglist.cgi index 088f69546..0f7dda0ac 100755 --- a/buglist.cgi +++ b/buglist.cgi @@ -272,15 +272,9 @@ if ($::FORM{'cmdtype'} eq "dorem") { my $userid = DBNameToIdAndCheck($::COOKIE{"Bugzilla_login"}); my $qname = SqlQuote($::FORM{'namedcmd'}); SendSQL("DELETE FROM namedqueries WHERE userid = $userid AND name = $qname"); - # Now remove this query from the footer - my $count = 0; - foreach my $q (@{$::vars->{'user'}{'queries'}}) { - if ($q->{'name'} eq $::FORM{'namedcmd'}) { - splice(@{$::vars->{'user'}{'queries'}}, $count, 1); - last; - } - $count++; - } + + # Now reset the cached queries + Bugzilla->user->flush_queries_cache(); print $cgi->header(); # Generate and return the UI (HTML page) from the appropriate template. @@ -317,6 +311,14 @@ elsif ($::FORM{'cmdtype'} eq "doit" && $::FORM{'remember'}) { my $tofooter = $::FORM{'tofooter'} ? 1 : 0; + $vars->{'message'} = "buglist_new_named_query"; + + # We want to display the correct message. Check if it existed before + # we insert, because ->queries may fetch from the db anyway + if (grep { $_->{name} eq $name } @{Bugzilla->user->queries()}) { + $vars->{'message'} = "buglist_updated_named_query"; + } + SendSQL("SELECT query FROM namedqueries WHERE userid = $userid AND name = $qname"); if (FetchOneColumn()) { SendSQL("UPDATE namedqueries @@ -327,28 +329,11 @@ elsif ($::FORM{'cmdtype'} eq "doit" && $::FORM{'remember'}) { SendSQL("REPLACE INTO namedqueries (userid, name, query, linkinfooter) VALUES ($userid, $qname, $qbuffer, $tofooter)"); } - - my $new_in_footer = $tofooter; - $vars->{'message'} = "buglist_new_named_query"; - - # Don't add it to the list if they are reusing an existing query name. - foreach my $query (@{$vars->{'user'}{'queries'}}) { - if ($query->{'name'} eq $name) { - $vars->{'message'} = "buglist_updated_named_query"; - if ($query->{'linkinfooter'} == 1) { - $new_in_footer = 0; - } - last; - } - } - - if ($new_in_footer) { - my %query = (name => $name, - query => $::buffer, - linkinfooter => $tofooter); - push(@{$vars->{'user'}{'queries'}}, \%query); - } - + + # Make sure to invalidate any cached query data, so that the footer is + # correctly displayed + Bugzilla->user->flush_queries_cache(); + $vars->{'queryname'} = $name; } } diff --git a/docs/xml/administration.xml b/docs/xml/administration.xml index 2382fca82..ecf465fdc 100644 --- a/docs/xml/administration.xml +++ b/docs/xml/administration.xml @@ -1385,7 +1385,7 @@ skip-networking positive check, which returns 1 (allow) if certain conditions are true, or a negative check, which returns 0 (deny.) E.g.: <programlisting> if ($field eq "qacontact") { - if (UserInGroup("quality_assurance")) { + if (Bugzilla->user->groups("quality_assurance")) { return 1; } else { @@ -1395,7 +1395,7 @@ skip-networking This says that only users in the group "quality_assurance" can change the QA Contact field of a bug. Getting more weird: <programlisting> if (($field eq "priority") && - ($vars->{'user'}{'login'} =~ /.*\@example\.com$/)) + (Bugzilla->user->email =~ /.*\@example\.com$/)) { if ($oldvalue eq "P1") { return 1; diff --git a/editusers.cgi b/editusers.cgi index b0e6d621c..b4ed7c2d0 100755 --- a/editusers.cgi +++ b/editusers.cgi @@ -34,6 +34,8 @@ use lib "."; require "CGI.pl"; require "globals.pl"; +use Bugzilla::User; + # Shut up misguided -w warnings about "used only once". "use vars" just # doesn't work for me. @@ -241,7 +243,7 @@ print Bugzilla->cgi->header(); $editall = UserInGroup("editusers"); if (!$editall) { - if (!UserCanBlessAnything()) { + if (!Bugzilla->user->can_bless) { PutHeader("Not allowed"); print "Sorry, you aren't a member of the 'editusers' group, and you\n"; print "don't have permissions to put people in or out of any group.\n"; @@ -483,7 +485,7 @@ if ($action eq 'new') { print "OK, done.<br>\n"; SendSQL("SELECT last_insert_id()"); my ($newuserid) = FetchSQLData(); - DeriveGroup($newuserid); + print "To change ${user}'s permissions, go back and <a href=\"editusers.cgi?action=edit&user=" . url_quote($user)."\">edit this user</A>"; print "<p>\n"; PutTrailer($localtrailer, @@ -682,7 +684,9 @@ if ($action eq 'edit') { my ($thisuserid, $realname, $disabledtext) = FetchSQLData(); if ($thisuserid > 0) { - DeriveGroup($thisuserid); + # Force groups to be up to date + my $changeduser = new Bugzilla::User($thisuserid); + $changeduser->derive_groups(); } print "<FORM METHOD=POST ACTION=editusers.cgi>\n"; print "<TABLE BORDER=0 CELLPADDING=4 CELLSPACING=0><TR>\n"; @@ -844,7 +848,8 @@ if ($action eq 'update') { print "Updated user's name.<BR>\n"; } - DeriveGroup($thisuserid); + my $changeduser = new Bugzilla::User($thisuserid); + $changeduser->derive_groups(); PutTrailer($localtrailer); exit; diff --git a/globals.pl b/globals.pl index 1c1ee075a..9c36e9003 100644 --- a/globals.pl +++ b/globals.pl @@ -510,10 +510,9 @@ sub CanEditProductId { my $query = "SELECT group_id FROM group_control_map " . "WHERE product_id = $productid " . "AND canedit != 0 "; - if ((defined @{$::vars->{user}{groupids}}) - && (@{$::vars->{user}{groupids}} > 0)) { + if (defined Bugzilla->user && %{Bugzilla->user->groups}) { $query .= "AND group_id NOT IN(" . - join(',',@{$::vars->{user}{groupids}}) . ") "; + join(',', values(%{Bugzilla->user->groups})) . ") "; } $query .= "LIMIT 1"; PushGlobalSQLState(); @@ -533,10 +532,9 @@ sub CanEnterProduct { "LEFT JOIN group_control_map " . "ON group_control_map.product_id = products.id " . "AND group_control_map.entry != 0 "; - if ((defined @{$::vars->{user}{groupids}}) - && (@{$::vars->{user}{groupids}} > 0)) { + if (defined Bugzilla->user && %{Bugzilla->user->groups}) { $query .= "AND group_id NOT IN(" . - join(',',@{$::vars->{user}{groupids}}) . ") "; + join(',', values(%{Bugzilla->user->groups})) . ") "; } $query .= "WHERE products.name = " . SqlQuote($productname) . " LIMIT 1"; PushGlobalSQLState(); @@ -566,10 +564,9 @@ sub GetSelectableProducts { $query .= "AND group_control_map.membercontrol = " . CONTROLMAPMANDATORY . " "; } - if ((defined @{$::vars->{user}{groupids}}) - && (@{$::vars->{user}{groupids}} > 0)) { + if (defined Bugzilla->user && %{Bugzilla->user->groups}) { $query .= "AND group_id NOT IN(" . - join(',',@{$::vars->{user}{groupids}}) . ") "; + join(',', values(%{Bugzilla->user->groups})) . ") "; } $query .= "WHERE group_id IS NULL ORDER BY name"; PushGlobalSQLState(); @@ -722,99 +719,6 @@ sub Crypt { return $cryptedpassword; } -# ConfirmGroup(userid) is called prior to any activity that relies -# on user_group_map to ensure that derived group permissions are up-to-date. -# Permissions must be rederived if ANY groups have a last_changed newer -# than the profiles.refreshed_when value. -sub ConfirmGroup { - my ($user, $locked) = (@_); - PushGlobalSQLState(); - SendSQL("SELECT userid FROM profiles, groups WHERE userid = $user " . - "AND profiles.refreshed_when <= groups.last_changed "); - my $ret = FetchSQLData(); - PopGlobalSQLState(); - if ($ret) { - DeriveGroup($user, $locked); - } -} - -# DeriveGroup removes and rederives all derived group permissions for -# the specified user. If $locked is true, Bugzilla has already locked -# the necessary tables as part of a larger transaction, so this function -# shouldn't lock them again (since then tables not part of this function's -# lock will get unlocked). -sub DeriveGroup { - my ($user, $locked) = (@_); - PushGlobalSQLState(); - - SendSQL("LOCK TABLES profiles WRITE, user_group_map WRITE, group_group_map READ, groups READ") - unless $locked; - - # avoid races, we are only as up to date as the BEGINNING of this process - SendSQL("SELECT login_name, NOW() FROM profiles WHERE userid = $user"); - my ($login, $starttime) = FetchSQLData(); - - # first remove any old derived stuff for this user - SendSQL("DELETE FROM user_group_map WHERE user_id = $user " . - "AND isderived = 1"); - - my %groupidsadded = (); - # add derived records for any matching regexps - SendSQL("SELECT id, userregexp FROM groups WHERE userregexp != ''"); - while (MoreSQLData()) { - my ($groupid, $rexp) = FetchSQLData(); - if ($login =~ m/$rexp/i) { - PushGlobalSQLState(); - $groupidsadded{$groupid} = 1; - SendSQL("INSERT INTO user_group_map " . - "(user_id, group_id, isbless, isderived) " . - "VALUES ($user, $groupid, 0, 1)"); - PopGlobalSQLState(); - - } - } - - # Get a list of the groups of which the user is a member. - my %groupidschecked = (); - my @groupidstocheck = (); - SendSQL("SELECT group_id FROM user_group_map WHERE user_id = $user - AND NOT isbless"); - while (MoreSQLData()) { - my ($groupid) = FetchSQLData(); - push(@groupidstocheck,$groupid); - } - - # Each group needs to be checked for inherited memberships once. - while (@groupidstocheck) { - my $group = shift @groupidstocheck; - if (!defined($groupidschecked{"$group"})) { - $groupidschecked{"$group"} = 1; - SendSQL("SELECT grantor_id FROM group_group_map WHERE" - . " member_id = $group AND NOT isbless"); - while (MoreSQLData()) { - my ($groupid) = FetchSQLData(); - if (!defined($groupidschecked{"$groupid"})) { - push(@groupidstocheck,$groupid); - } - if (!$groupidsadded{$groupid}) { - $groupidsadded{$groupid} = 1; - PushGlobalSQLState(); - SendSQL("INSERT INTO user_group_map" - . " (user_id, group_id, isbless, isderived)" - . " VALUES ($user, $groupid, 0, 1)"); - PopGlobalSQLState(); - } - } - } - } - - SendSQL("UPDATE profiles SET refreshed_when = " . - SqlQuote($starttime) . "WHERE userid = $user"); - SendSQL("UNLOCK TABLES"); - PopGlobalSQLState(); -}; - - sub DBID_to_real_or_loginname { my ($id) = (@_); PushGlobalSQLState(); @@ -1189,23 +1093,8 @@ sub SplitEnumType { return @result; } -# UserInGroup returns information aboout the current user if no second -# parameter is specified sub UserInGroup { - my ($groupname, $userid) = (@_); - if (!$userid) { - return $::vars->{'user'}{'groups'}{$_[0]}; - } - PushGlobalSQLState(); - $userid ||= $::userid; - SendSQL("SELECT groups.id FROM groups, user_group_map - WHERE groups.id = user_group_map.group_id - AND user_group_map.user_id = $userid - AND isbless = 0 - AND groups.name = " . SqlQuote($groupname)); - my $result = FetchOneColumn(); - PopGlobalSQLState(); - return defined($result); + return defined Bugzilla->user && defined Bugzilla->user->groups->{$_[0]}; } sub UserCanBlessGroup { @@ -1238,32 +1127,6 @@ sub UserCanBlessGroup { return $result; } -sub UserCanBlessAnything { - PushGlobalSQLState(); - # check if user explicitly can bless a group - SendSQL("SELECT group_id FROM user_group_map - WHERE user_id = $::userid AND isbless = 1"); - my $result = FetchOneColumn(); - PopGlobalSQLState(); - if ($result) { - return 1; - } - PushGlobalSQLState(); - # check if user is a member of a group that can bless this group - SendSQL("SELECT groups.id FROM groups, user_group_map, - group_group_map - WHERE groups.id = grantor_id - AND user_group_map.user_id = $::userid - AND group_group_map.isbless = 1 - AND user_group_map.group_id = member_id"); - $result = FetchOneColumn(); - PopGlobalSQLState(); - if ($result) { - return 1; - } - return 0; -} - sub BugInGroup { my ($bugid, $groupname) = (@_); PushGlobalSQLState(); diff --git a/post_bug.cgi b/post_bug.cgi index 76d86fe58..f0d4f0816 100755 --- a/post_bug.cgi +++ b/post_bug.cgi @@ -54,7 +54,7 @@ sub sillyness { use vars qw($vars $template); ConnectToDatabase(); -my $whoid = confirm_login(); +my $user = confirm_login(); my $cgi = Bugzilla->cgi; @@ -454,7 +454,7 @@ if (UserInGroup("editbugs")) { "($id, $i)"); push(@all_deps, $i); # list for mailing dependent bugs # Log the activity for the other bug: - LogActivityEntry($i, $me, "", $id, $whoid, $timestamp); + LogActivityEntry($i, $me, "", $id, $user->id, $timestamp); } my $tmp = $me; $me = $target; diff --git a/process_bug.cgi b/process_bug.cgi index b9414d534..45974d465 100755 --- a/process_bug.cgi +++ b/process_bug.cgi @@ -57,7 +57,8 @@ use vars qw(%versions ); ConnectToDatabase(); -my $whoid = confirm_login(); +my $user = confirm_login(); +my $whoid = $user->id; my $cgi = Bugzilla->cgi; @@ -1093,9 +1094,10 @@ foreach my $id (@idlist) { "keywords $write, longdescs $write, fielddefs $write, " . "bug_group_map $write, flags $write, duplicates $write," . # user_group_map would be a READ lock except that Flag::process - # may call Flag::notify, which calls ConfirmGroup, which might - # call DeriveGroup, which wants a WRITE lock on that table. - # group_group_map is in here at all because DeriveGroups needs it. + # may call Flag::notify, which creates a new user object, + # which might call derive_groups, which wants a WRITE lock on that + # table. group_group_map is in here at all because derive_groups + # needs it. "user_group_map $write, group_group_map READ, flagtypes READ, " . "flaginclusions AS i READ, flagexclusions AS e READ, " . "keyworddefs READ, groups READ, attachments READ, " . @@ -53,19 +53,21 @@ ConnectToDatabase(); my $cgi = Bugzilla->cgi; -my $userid = 0; if (defined $::FORM{"GoAheadAndLogIn"}) { # We got here from a login page, probably from relogin.cgi. We better # make sure the password is legit. - $userid = confirm_login(); + confirm_login(); } else { - $userid = quietly_check_login(); + quietly_check_login(); } +my $user = Bugzilla->user; +my $userid = $user ? $user->id : 0; + # Backwards compatibility hack -- if there are any of the old QUERY_* # cookies around, and we are logged in, then move them into the database # and nuke the cookie. This is required for Bugzilla 2.8 and earlier. -if ($userid) { +if ($user) { my @oldquerycookies; foreach my $i (keys %::COOKIE) { if ($i =~ /^QUERY_(.*)$/) { @@ -97,7 +99,7 @@ if ($userid) { } if ($::FORM{'nukedefaultquery'}) { - if ($userid) { + if ($user) { SendSQL("DELETE FROM namedqueries " . "WHERE userid = $userid AND name = '$::defaultqueryname'"); } @@ -105,7 +107,7 @@ if ($::FORM{'nukedefaultquery'}) { } my $userdefaultquery; -if ($userid) { +if ($user) { SendSQL("SELECT query FROM namedqueries " . "WHERE userid = $userid AND name = '$::defaultqueryname'"); $userdefaultquery = FetchOneColumn(); @@ -308,7 +310,6 @@ $vars->{'rep_platform'} = \@::legal_platform; $vars->{'op_sys'} = \@::legal_opsys; $vars->{'priority'} = \@::legal_priority; $vars->{'bug_severity'} = \@::legal_severity; -$vars->{'userid'} = $userid; # Boolean charts my @fields; @@ -362,7 +363,7 @@ for (my $chart = 0; $::FORM{"field$chart-0-0"}; $chart++) { $default{'charts'} = \@charts; # Named queries -if ($userid) { +if ($user) { my @namedqueries; SendSQL("SELECT name FROM namedqueries " . "WHERE userid = $userid AND name != '$::defaultqueryname' " . diff --git a/relogin.cgi b/relogin.cgi index d2ce053a5..65cb07b25 100755 --- a/relogin.cgi +++ b/relogin.cgi @@ -59,7 +59,9 @@ $cgi->send_cookie(-name => "Bugzilla_logincookie", delete $::COOKIE{"Bugzilla_login"}; $vars->{'message'} = "logged_out"; -$vars->{'user'} = {}; + +# This entire script should eventually just become a call to Bugzilla->logout +Bugzilla->logout; print $cgi->header(); $template->process("global/message.html.tmpl", $vars) diff --git a/sanitycheck.cgi b/sanitycheck.cgi index 98734e5b0..aab9c8d38 100755 --- a/sanitycheck.cgi +++ b/sanitycheck.cgi @@ -106,12 +106,16 @@ if (exists $::FORM{'rederivegroups'}) { } # 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 (exists $::FORM{'rederivegroupsnow'}) { + require Bugzilla::User; Status("OK, now rederiving groups."); SendSQL("SELECT userid FROM profiles"); while ((my $id) = FetchSQLData()) { - DeriveGroup($id); - Status("Group $id"); + my $user = new Bugzilla::User($id); + $user->derive_groups(); + Status("User $id"); } } diff --git a/sidebar.cgi b/sidebar.cgi index 83c89c29c..cf801eba3 100755 --- a/sidebar.cgi +++ b/sidebar.cgi @@ -35,26 +35,6 @@ my $cgi = Bugzilla->cgi; # Main Body Execution ############################################################################### -$vars->{'username'} = $::COOKIE{'Bugzilla_login'} || ''; - -if (defined $::COOKIE{'Bugzilla_login'}) { - SendSQL("SELECT mybugslink, userid FROM profiles " . - "WHERE login_name = " . SqlQuote($::COOKIE{'Bugzilla_login'})); - my ($mybugslink, $userid) = (FetchSQLData()); - $vars->{'userid'} = $userid; - $vars->{'canblessanything'} = UserCanBlessAnything(); - if ($mybugslink) { - my $mybugstemplate = Param("mybugstemplate"); - my %substs = ( 'userid' => url_quote($::COOKIE{'Bugzilla_login'}) ); - $vars->{'mybugsurl'} = PerformSubsts($mybugstemplate, \%substs); - } - SendSQL("SELECT name FROM namedqueries WHERE userid = $userid AND linkinfooter"); - while (MoreSQLData()) { - my ($name) = FetchSQLData(); - push(@{$vars->{'namedqueries'}}, $name); - } -} - # This sidebar is currently for use with Mozilla based web browsers. # Internet Explorer 6 is supposed to have a similar feature, but it # most likely won't support XUL ;) When that does come out, this diff --git a/template/en/default/attachment/create.html.tmpl b/template/en/default/attachment/create.html.tmpl index a298df5a9..56bad7fae 100644 --- a/template/en/default/attachment/create.html.tmpl +++ b/template/en/default/attachment/create.html.tmpl @@ -123,7 +123,8 @@ [% END %] </td> </tr> - [% IF (user.userid != bugassignee_id) AND UserInGroup("editbugs") %] + + [% IF (user.id != bugassignee_id) AND user.groups.editbugs %] <tr> <th>Reassignment:</th> <td> diff --git a/template/en/default/bug/show.xml.tmpl b/template/en/default/bug/show.xml.tmpl index 618745902..45ef1712a 100644 --- a/template/en/default/bug/show.xml.tmpl +++ b/template/en/default/bug/show.xml.tmpl @@ -25,7 +25,7 @@ <bugzilla version="[% VERSION %]" urlbase="[% Param('urlbase') %]" maintainer="[% Param('maintainer') FILTER xml %]" -[% IF user.login %] +[% IF user %] exporter="[% user.login FILTER xml %]" [% END %] > diff --git a/template/en/default/flag/list.html.tmpl b/template/en/default/flag/list.html.tmpl index 9ca1c25c7..50752a0df 100644 --- a/template/en/default/flag/list.html.tmpl +++ b/template/en/default/flag/list.html.tmpl @@ -116,7 +116,7 @@ id="requestee-[% flag.id %]" name="requestee-[% flag.id %]" [% IF flag.status == "?" && flag.requestee %] - value="[% flag.requestee.email FILTER html %]" + value="[% flag.requestee.login FILTER html %]" [% END %] >) </span> diff --git a/template/en/default/global/useful-links.html.tmpl b/template/en/default/global/useful-links.html.tmpl index fac714b0e..e7588db7d 100644 --- a/template/en/default/global/useful-links.html.tmpl +++ b/template/en/default/global/useful-links.html.tmpl @@ -19,19 +19,6 @@ # Contributor(s): Gervase Markham <gerv@gerv.net> #%] -[%# INTERFACE: - # user: hash. Information about the user. If the user is not logged in, - # user.login is undefined. - # login: string. The user's Bugzilla login email address. - # showmybugslink: boolean. True if user wants My Bugs in the footer. - # queries: list of strings. The names of those of the user's named - # queries which should be displayed in the footer. - # groups: hash. Keys are group names, values are true if user in that group. - # The keys used in this template are - # tweakparams, editcomponents, creategroups, editkeywords, confirm, - # editbugs, editusers. - #%] - [%# Migration note: this whole file corresponds to the old %commandmenu% substitution param in 'footerhtml' %] @@ -51,14 +38,14 @@ <a href="report.cgi">Reports</a> - [% IF user.login %] + [% IF user %] [% email = user.login FILTER url_quote %] | <a href="request.cgi?requester=[% email %]&requestee=[% email %]&do_union=1&group=type">My Requests</a> [% ELSE %] | <a href="request.cgi">Requests</a> [% END %] - [% IF user.login && Param('usevotes') %] + [% IF user && Param('usevotes') %] | <a href="votes.cgi?action=show_user">My Votes</a> [% END %] </td> @@ -72,7 +59,7 @@ [% ', <a href="editparams.cgi">parameters</a>' IF user.groups.tweakparams %] [% ', <a href="editusers.cgi">users</a>' IF user.groups.editusers - || user.canblessany %] + || user.can_bless %] [% ', <a href="editproducts.cgi">products</a>' IF user.groups.editcomponents %] [% ', <a href="editflagtypes.cgi">flags</a>' @@ -91,9 +78,14 @@ [%# Preset queries %] [% preset_queries = user.showmybugslink %] - [% FOREACH q = user.queries %] - [% SET preset_queries = 1 IF q.linkinfooter %] - [% END %] + [% IF NOT preset_queries %] + [% FOREACH q = user.queries %] + [% IF q.linkinfooter %] + [% preset_queries = 1 %] + [% LAST %] + [% END %] + [% END %] + [% END %] <tr> [% IF preset_queries %] diff --git a/template/en/default/list/quips.html.tmpl b/template/en/default/list/quips.html.tmpl index 4a6ef1ad5..d9f092db2 100644 --- a/template/en/default/list/quips.html.tmpl +++ b/template/en/default/list/quips.html.tmpl @@ -35,7 +35,7 @@ <p> <font color="red"> Your quip '<tt>[% added_quip FILTER html %]</tt>' has been added. - [% IF Param("enablequips") == "approved" AND !UserInGroup('admin') %] + [% IF Param("enablequips") == "approved" AND !user.groups.admin %] It will be used as soon as it gets approved. [% END %] </font> @@ -58,7 +58,7 @@ Bugzilla will pick a random quip for the headline on each bug list, and you can extend the quip list. Type in something clever or funny or boring (but not obscene or offensive, please) and bonk on the button. - [% IF Param("enablequips") == "approved" AND !UserInGroup('admin') %] + [% IF Param("enablequips") == "approved" AND !user.groups.admin %] Note that your quip has to be approved before it is used. [% END %] </p> diff --git a/template/en/default/request/email.txt.tmpl b/template/en/default/request/email.txt.tmpl index e35132181..cd7e43358 100644 --- a/template/en/default/request/email.txt.tmpl +++ b/template/en/default/request/email.txt.tmpl @@ -43,7 +43,7 @@ Subject: [% flag.type.name %] [%+ subject_status %]: [Bug [% flag.target.bug.id [%+ USE wrap -%] [%- FILTER bullet = wrap(80) -%] -[% user.realname %] <[% user.login %]> has [% statuses.${flag.status} %] [%+ to_identity %] for [% flag.type.name %]: +[% user.identity %] has [% statuses.${flag.status} %] [%+ to_identity %] for [% flag.type.name %]: Bug [% bugidsummary %] [% END %] @@ -58,7 +58,7 @@ Attachment [% attidsummary %] [%- FILTER bullet = wrap(80) %] [% IF form.comment.length > 0 %] -------- Additional Comments from [% user.realname %] <[% user.login %]> +------- Additional Comments from [% user.identity %] [%+ form.comment %] [% END %] diff --git a/template/en/default/search/knob.html.tmpl b/template/en/default/search/knob.html.tmpl index 50e8fc646..2c99c3f82 100644 --- a/template/en/default/search/knob.html.tmpl +++ b/template/en/default/search/knob.html.tmpl @@ -30,7 +30,7 @@ "Last Changed" => "Last Changed" } %] <br> -[% IF NOT userid %] +[% IF NOT user %] <input type="hidden" name="cmdtype" value="doit"> [% ELSE %] <script type="text/javascript"> <!-- diff --git a/template/en/default/sidebar.xul.tmpl b/template/en/default/sidebar.xul.tmpl index a065d447a..97b3fe4c0 100644 --- a/template/en/default/sidebar.xul.tmpl +++ b/template/en/default/sidebar.xul.tmpl @@ -72,37 +72,38 @@ function normal_keypress_handler( aEvent ) { <text class="text-link" onclick="load_relative_url('enter_bug.cgi')" value="new bug"/> <separator class="thin"/> -[% IF username %] +[% IF user %] <text class="text-link" onclick="load_relative_url('userprefs.cgi')" value="edit prefs"/> - [%- IF UserInGroup('tweakparams') %] + [%- IF user.groups.tweakparams %] <text class="text-link" onclick="load_relative_url('editparams.cgi')" value="edit params"/> [%- END %] - [%- IF UserInGroup('editusers') || canblessany %] + [%- IF user.groups.editusers || user.can_bless %] <text class="text-link" onclick="load_relative_url('editusers.cgi')" value="edit users"/> [%- END %] - [%- IF UserInGroup('editcomponents') %] + [%- IF user.groups.editcomponents %] <text class="text-link" onclick="load_relative_url('editcomponents.cgi')" value="edit components"/> [%- END %] - [%- IF UserInGroup('creategroups') %] + [%- IF user.groups.creategroups %] <text class="text-link" onclick="load_relative_url('editgroups.cgi')" value="edit groups"/> [%- END %] - [%- IF UserInGroup('editkeywords') %] + [%- IF user.groups.editkeywords %] <text class="text-link" onclick="load_relative_url('editkeywords.cgi')" value="edit keywords"/> [%- END %] - [%- IF UserInGroup('tweakparams') %] + [%- IF user.groups.tweakparams %] <text class="text-link" onclick="load_relative_url('sanitycheck.cgi')" value="sanity check"/> [%- END %] - <text class="text-link" onclick="load_relative_url('relogin.cgi')" value="logout [% username FILTER html %]"/> + <text class="text-link" onclick="load_relative_url('relogin.cgi')" value="logout [% user.login FILTER html %]"/> <separator class="thin"/> - [%- IF mybugsurl %] - <text class="text-link" onclick="load_relative_url('[% mybugsurl FILTER html %]')" value="my bugs"/> + [%- IF user.showmybugslink %] + [% filtered_username = user.login FILTER url_quote %] + <text class="text-link" onclick="load_relative_url('[% Param('mybugstemplate').replace('%userid%', filtered_username) FILTER js FILTER html %]')" value="my bugs"/> [%- END %] [%- IF Param('usevotes') %] <text class="text-link" onclick="load_relative_url('votes.cgi?action=show_user')" value="my votes"/> [%- END %] - [%- FOREACH name = namedqueries %] - <text class="text-link" onclick="load_relative_url('buglist.cgi?cmdtype=runnamed&namedcmd=[% name FILTER url_quote %]')" value="[% name FILTER html %]"/> + [%- FOREACH q = user.queries %] + <text class="text-link" onclick="load_relative_url('buglist.cgi?cmdtype=runnamed&namedcmd=[% q.name FILTER url_quote %]')" value="[% q.name FILTER html %]"/> [% END %] [% ELSE %] @@ -44,6 +44,8 @@ quietly_check_login('permit_anonymous'); # token-related tasks. use Token; +use Bugzilla::User; + ################################################################################ # Data Validation / Security Authorization ################################################################################ @@ -248,7 +250,10 @@ sub changeEmail { SendSQL("DELETE FROM tokens WHERE userid = $userid AND tokentype = 'emailnew'"); SendSQL("UNLOCK TABLES"); - DeriveGroup($userid); + + # The email address has been changed, so we need to rederive the groups + my $user = new Bugzilla::User($userid); + $user->derive_groups; # Return HTTP response headers. print Bugzilla->cgi->header(); @@ -283,7 +288,16 @@ sub cancelChangeEmail { SET login_name = $quotedoldemail WHERE userid = $userid"); SendSQL("UNLOCK TABLES"); - DeriveGroup($userid); + + # email has changed, so rederive groups + # Note that this is done _after_ the tables are unlocked + # This is sort of a race condition (given the lack of transactions) + # but the user had access to it just now, so it's not a security + # issue + + my $user = new Bugzilla::User($userid); + $user->derive_groups; + $vars->{'message'} = "email_change_cancelled_reinstated"; } } diff --git a/userprefs.cgi b/userprefs.cgi index 206a115a9..857665f7c 100755 --- a/userprefs.cgi +++ b/userprefs.cgi @@ -314,8 +314,13 @@ sub SaveFooter { SendSQL("UPDATE profiles SET mybugslink = " . SqlQuote($::FORM{'mybugslink'}) . " WHERE userid = $userid"); - # Regenerate cached info about queries in footer. - $vars->{'user'} = GetUserInfo($::userid); + # Make sure that cached queries in the user object are invalidated + # so that the footer is correct + my $user = Bugzilla->user; + $user->flush_queries_cache(); + + # Also need to update showmybugslink + $user->{showmybugslink} = $::FORM{'mybugslink'} ? 1 : 0; } @@ -127,7 +127,7 @@ sub show_user { # If a bug_id is given, and we're editing, we'll add it to the votes list. my $bug_id = $::FORM{'bug_id'} || ""; - my $name = $::FORM{'user'} || $::COOKIE{'Bugzilla_login'}; + my $name = $::FORM{'user'} || Bugzilla->user->login; my $who = DBname_to_id($name); # After DBNameToIdAndCheck is templatised and prints a Content-Type, @@ -135,7 +135,7 @@ sub show_user { # special error handling should go away. $who || ThrowUserError("invalid_username", {name => $name}); - my $canedit = 1 if ($name eq $::COOKIE{'Bugzilla_login'}); + my $canedit = 1 if ($name eq Bugzilla->user->login); SendSQL("LOCK TABLES bugs READ, products READ, votes WRITE, cc READ, bug_group_map READ, user_group_map READ, @@ -270,7 +270,7 @@ sub record_votes { GetVersionTable(); - my $who = DBNameToIdAndCheck($::COOKIE{'Bugzilla_login'}); + my $who = Bugzilla->user->id; # If the user is voting for bugs, make sure they aren't overstuffing # the ballot box. |