From: lpsolit%gmail.com <> Date: Wed, 30 Nov 2005 20:19:00 +0000 (+0000) Subject: Bug 314039: editusers.cgi edits user 0 if you don't pass a userid - Patch by Frédéric... X-Git-Tag: bugzilla-2.20.1~81 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=2115ec9a8f1a0d014a8236210df06440737bb3b0;p=thirdparty%2Fbugzilla.git Bug 314039: editusers.cgi edits user 0 if you don't pass a userid - Patch by Frédéric Buclin r=wurblzap a=justdave --- diff --git a/editusers.cgi b/editusers.cgi index 6d46163aa2..cc83fdb083 100755 --- a/editusers.cgi +++ b/editusers.cgi @@ -14,6 +14,7 @@ # The Original Code is the Bugzilla Bug Tracking System. # # Contributor(s): Marc Schumann +# Frédéric Buclin use strict; use lib "."; @@ -30,36 +31,28 @@ use Bugzilla::Config; use Bugzilla::Constants; use Bugzilla::Util; -Bugzilla->login(LOGIN_REQUIRED); +my $user = Bugzilla->login(LOGIN_REQUIRED); my $cgi = Bugzilla->cgi; my $template = Bugzilla->template; my $dbh = Bugzilla->dbh; -my $user = Bugzilla->user; my $userid = $user->id; my $editusers = $user->in_group('editusers'); # Reject access if there is no sense in continuing. $editusers - || Bugzilla->user->can_bless() + || $user->can_bless() || ThrowUserError("auth_failure", {group => "editusers", reason => "cant_bless", action => "edit", object => "users"}); -print Bugzilla->cgi->header(); +print $cgi->header(); # Common CGI params -my $action = $cgi->param('action') || 'search'; -my $login = $cgi->param('login'); -my $password = $cgi->param('password'); -my $groupid = $cgi->param('groupid'); -my $otherUser = new Bugzilla::User($cgi->param('userid')); -my $realname = trim($cgi->param('name') || ''); -my $disabledtext = trim($cgi->param('disabledtext') || ''); - -# Directly from common CGI params derived values -my $otherUserID = $otherUser->id(); +my $action = $cgi->param('action') || 'search'; +my $otherUserID = $cgi->param('userid'); +my $otherUserLogin = $cgi->param('user'); # Prefill template vars with data used in all or nearly all templates $vars->{'editusers'} = $editusers; @@ -77,12 +70,19 @@ if ($action eq 'search') { my $matchstr = $cgi->param('matchstr'); my $matchtype = $cgi->param('matchtype'); my $grouprestrict = $cgi->param('grouprestrict') || '0'; + my $groupid = $cgi->param('groupid'); my $query = 'SELECT DISTINCT userid, login_name, realname, disabledtext ' . 'FROM profiles'; my @bindValues; my $nextCondition; my $visibleGroups; + # If a group ID is given, make sure it is a valid one. + if ($grouprestrict) { + (detaint_natural($groupid) && GroupIdToName($groupid)) + || ThrowUserError('invalid_group_ID'); + } + if (!$editusers && Param('usevisibilitygroups')) { # Show only users in visible groups. $visibleGroups = visibleGroupsAsString(); @@ -133,7 +133,6 @@ if ($action eq 'search') { 'AND ugm.group_id = ?'; # We can trick_taint because we use the value in a SELECT only, # using a placeholder. - trick_taint($groupid); push(@bindValues, $groupid); } $query .= ' ORDER BY profiles.login_name'; @@ -161,6 +160,11 @@ if ($action eq 'search') { action => "add", object => "users"}); + my $login = $cgi->param('login'); + my $password = $cgi->param('password'); + my $realname = trim($cgi->param('name') || ''); + my $disabledtext = trim($cgi->param('disabledtext') || ''); + # Lock tables during the check+creation session. $dbh->bz_lock_tables('profiles WRITE', 'profiles_activity WRITE', @@ -194,8 +198,8 @@ if ($action eq 'search') { ########################################################################### } elsif ($action eq 'edit') { - $otherUser - || ThrowCodeError('invalid_user_id', {'userid' => $cgi->param('userid')}); + my $otherUser = check_user($otherUserID, $otherUserLogin); + $otherUserID = $otherUser->id; $editusers || canSeeUser($otherUserID) || ThrowUserError('auth_failure', {reason => "not_visible", @@ -209,8 +213,9 @@ if ($action eq 'search') { ########################################################################### } elsif ($action eq 'update') { - $otherUser - || ThrowCodeError('invalid_user_id', {'userid' => $cgi->param('userid')}); + my $otherUser = check_user($otherUserID, $otherUserLogin); + $otherUserID = $otherUser->id; + my $logoutNeeded = 0; my @changedFields; @@ -236,9 +241,13 @@ if ($action eq 'search') { # Cleanups my $loginold = $cgi->param('loginold') || ''; my $realnameold = $cgi->param('nameold') || ''; - my $password = $cgi->param('password') || ''; my $disabledtextold = $cgi->param('disabledtextold') || ''; + my $login = $cgi->param('login'); + my $password = $cgi->param('password'); + my $realname = trim($cgi->param('name') || ''); + my $disabledtext = trim($cgi->param('disabledtext') || ''); + # Update profiles table entry; silently skip doing this if the user # is not authorized. if ($editusers) { @@ -283,7 +292,7 @@ if ($action eq 'search') { } if (@changedFields) { push (@values, $otherUserID); - $logoutNeeded && Bugzilla->logout_user_by_id($otherUserID); + $logoutNeeded && Bugzilla->logout_user($otherUser); $dbh->do('UPDATE profiles SET ' . join(' = ?,', @changedFields).' = ? ' . 'WHERE userid = ?', @@ -389,8 +398,8 @@ if ($action eq 'search') { ########################################################################### } elsif ($action eq 'del') { - $otherUser - || ThrowCodeError('invalid_user_id', {'userid' => $cgi->param('userid')}); + my $otherUser = check_user($otherUserID, $otherUserLogin); + $otherUserID = $otherUser->id; Param('allowuserdeletion') || ThrowUserError('users_deletion_disabled'); $editusers || ThrowUserError('auth_failure', {group => "editusers", @@ -457,9 +466,8 @@ if ($action eq 'search') { ########################################################################### } elsif ($action eq 'delete') { - $otherUser - || ThrowCodeError('invalid_user_id', {'userid' => $cgi->param('userid')}); - my $otherUserLogin = $otherUser->login(); + my $otherUser = check_user($otherUserID, $otherUserLogin); + $otherUserID = $otherUser->id; # Cache for user accounts. my %usercache = (0 => new Bugzilla::User()); @@ -504,7 +512,7 @@ if ($action eq 'search') { @{$otherUser->product_responsibilities()} && ThrowUserError('user_has_responsibility'); - Bugzilla->logout_user_by_id($otherUserID); + Bugzilla->logout_user($otherUser); # Get the timestamp for LogActivityEntry. my $timestamp = $dbh->selectrow_array('SELECT NOW()'); @@ -667,7 +675,7 @@ if ($action eq 'search') { $dbh->bz_unlock_tables(); $vars->{'message'} = 'account_deleted'; - $vars->{'otheruser'}{'login'} = $otherUserLogin; + $vars->{'otheruser'}{'login'} = $otherUser->login; $vars->{'restrictablegroups'} = $user->bless_groups(); $template->process('admin/users/search.html.tmpl', $vars) || ThrowTemplateError($template->error()); @@ -690,6 +698,27 @@ exit; # Helpers ########################################################################### +# Try to build a user object using its ID, else its login name, and throw +# an error if the user does not exist. +sub check_user { + my ($otherUserID, $otherUserLogin) = @_; + + my $otherUser; + my $vars = {}; + + if ($otherUserID) { + $otherUser = Bugzilla::User->new($otherUserID); + $vars->{'user_id'} = $otherUserID; + } + elsif ($otherUserLogin) { + $otherUser = Bugzilla::User->new_from_login($otherUserLogin); + $vars->{'user_login'} = $otherUserLogin; + } + ($otherUser && $otherUser->id) || ThrowCodeError('invalid_user', $vars); + + return $otherUser; +} + # Copy incoming list selection values from CGI params to template variables. sub mirrorListSelectionValues { if (defined($cgi->param('matchtype'))) { diff --git a/template/en/default/global/code-error.html.tmpl b/template/en/default/global/code-error.html.tmpl index b5d7987317..cd8d88fe13 100644 --- a/template/en/default/global/code-error.html.tmpl +++ b/template/en/default/global/code-error.html.tmpl @@ -211,9 +211,16 @@ The keyword ID [% id FILTER html %] couldn't be found. - [% ELSIF error == "invalid_user_id" %] - [% title = "Invalid User ID" %] - There is no user account with ID [% userid FILTER html %]. + [% ELSIF error == "invalid_user" %] + [% title = "Invalid User" %] + There is no user account + [% IF user_id %] + with ID [% user_id FILTER html %]. + [% ELSIF user_login %] + with login name [% user_login FILTER html %]. + [% ELSE %] + given. + [% END %] [% ELSIF error == "missing_bug_id" %] No [% terms.bug %] ID was given.