Bug 314039: editusers.cgi edits user 0 if you don't pass a userid - Patch by Frédéric...

diff --git a/editusers.cgi b/editusers.cgi
index 6d46163aa2a1753d25b561ec642c570eb480648c..cc83fdb083ce86f99c73c7eeac114f750cc556fc 100755 (executable)
--- a/editusers.cgi
+++ b/editusers.cgi
@@ -14,6 +14,7 @@
 # The Original Code is the Bugzilla Bug Tracking System.
 #
 # Contributor(s): Marc Schumann <wurblzap@gmail.com>
+#                 Frédéric Buclin <LpSolit@gmail.com>
 
 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 b5d7987317207b0ea2781431ab9340490bc3f265..cd8d88fe132ed5c368806a621dbabbca21ce4b15 100644 (file)
--- 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 <em>[% id FILTER html %]</em> couldn't be
     found.
 
-  [% ELSIF error == "invalid_user_id" %]
-    [% title = "Invalid User ID" %]
-    There is no user account with ID <em>[% userid FILTER html %]</em>.
+  [% ELSIF error == "invalid_user" %]
+    [% title = "Invalid User" %]
+    There is no user account
+    [% IF user_id %]
+      with ID <em>[% user_id FILTER html %]</em>.
+    [% ELSIF user_login %]
+      with login name <em>[% user_login FILTER html %]</em>.
+    [% ELSE %]
+      given.
+    [% END %]
 
   [% ELSIF error == "missing_bug_id" %]
     No [% terms.bug %] ID was given.