Bug 1185241: Logging out when or after impersonating a user doesn't delete cookies from the logincookies table correctly

r=dkl

3 files changed, 26 insertions, 62 deletions

diff --git a/Bugzilla/Auth/Persist/Cookie.pm b/Bugzilla/Auth/Persist/Cookie.pm
index 877d1907e..904de3699 100644
--- a/Bugzilla/Auth/Persist/Cookie.pm
+++ b/Bugzilla/Auth/Persist/Cookie.pm
@@ -94,7 +94,7 @@ sub logout {
     my $cgi = Bugzilla->cgi;
     my $input = Bugzilla->input_params;
     $param = {} unless $param;
-    my $user = $param->{user} || Bugzilla->user;
+    my $user = $param->{user} || Bugzilla->sudoer || Bugzilla->user;
     my $type = $param->{type} || LOGOUT_ALL;
 
     if ($type == LOGOUT_ALL) {
diff --git a/relogin.cgi b/relogin.cgi
index c4aae8d0b..286127495 100755
--- a/relogin.cgi
+++ b/relogin.cgi
@@ -54,22 +54,6 @@ elsif ($action eq 'prepare-sudo') {
     # Keep a temporary record of the user visiting this page
     $vars->{'token'} = issue_session_token('sudo_prepared');
 
-    if ($user->authorizer->can_login) {
-        my $value = generate_random_password();
-        my %args;
-        $args{'-secure'} = 1 if Bugzilla->params->{ssl_redirect};
-
-        $cgi->send_cookie(-name => 'Bugzilla_login_request_cookie',
-                          -value => $value,
-                          -httponly => 1,
-                          %args);
-
-        # The user ID must not be set when generating the token, because
-        # that information will not be available when validating it.
-        local Bugzilla->user->{userid} = 0;
-        $vars->{'login_request_token'} = issue_hash_token(['login_request', $value]);
-    }
-
     # Show the sudo page
     $vars->{'target_login_default'} = $cgi->param('target_login');
     $vars->{'reason_default'} = $cgi->param('reason');
@@ -77,32 +61,16 @@ elsif ($action eq 'prepare-sudo') {
 }
 # begin-sudo: Confirm login and start sudo session
 elsif ($action eq 'begin-sudo') {
-    # We must be sure that the user is authenticating by providing a login
-    # and password.
-    # We only need to do this for authentication methods that involve Bugzilla 
-    # directly obtaining a login (i.e. normal CGI login), as opposed to other 
-    # methods (like Environment vars login). 
-
-    # First, record if Bugzilla_login and Bugzilla_password were provided
-    my $credentials_provided;
-    if (defined($cgi->param('Bugzilla_login'))
-        && defined($cgi->param('Bugzilla_password')))
-    {
-        $credentials_provided = 1;
-    }
-
-    # Next, log in the user
     my $user = Bugzilla->login(LOGIN_REQUIRED);
 
     my $target_login = $cgi->param('target_login');
     my $reason = $cgi->param('reason') || '';
 
-    # At this point, the user is logged in.  However, if they used a method
-    # where they could have provided a username/password (i.e. CGI), but they 
-    # did not provide a username/password, then throw an error.
-    if ($user->authorizer->can_login && !$credentials_provided) {
-        ThrowUserError('sudo_password_required',
-                       { target_login => $target_login, reason => $reason });
+    if ($user->authorizer->can_login) {
+        my $password = $cgi->param('password')
+          or ThrowUserError('sudo_password_required',
+                            { target_login => $target_login, reason => $reason });
+        $user->check_current_password($password);
     }
 
     # The user must be in the 'bz_sudoers' group
@@ -112,12 +80,25 @@ elsif ($action eq 'begin-sudo') {
                                          object => 'sudo_session' }
         );
     }
-    
+
     # Do not try to start a new session if one is already in progress!
     if (defined(Bugzilla->sudoer)) {
         ThrowUserError('sudo_in_progress', { target => $user->login });
     }
 
+    # Get & verify the target user (the user who we will be impersonating)
+    my $target_user = new Bugzilla::User({ name => $target_login });
+    unless (defined($target_user)
+            && $target_user->id
+            && $user->can_see_user($target_user))
+    {
+        ThrowUserError('user_match_failed', { name => $target_login });
+    }
+
+    if ($target_user->in_group('bz_sudo_protect')) {
+        ThrowUserError('sudo_protected', { login => $target_user->login });
+    }
+
     # Did the user actually go trough the 'sudo-prepare' action?  Do some 
     # checks on the token the action should have left.
     my ($token_user, $token_timestamp, $token_data) =
@@ -132,18 +113,6 @@ elsif ($action eq 'begin-sudo') {
     }
     delete_token($cgi->param('token'));
 
-    # Get & verify the target user (the user who we will be impersonating)
-    my $target_user = new Bugzilla::User({ name => $target_login });
-    unless (defined($target_user)
-            && $target_user->id
-            && $user->can_see_user($target_user))
-    {
-        ThrowUserError('user_match_failed', { name => $target_login });
-    }
-    if ($target_user->in_group('bz_sudo_protect')) {
-        ThrowUserError('sudo_protected', { login => $target_user->login });
-    }
-
     # Calculate the session expiry time (T + 6 hours)
     my $time_string = time2str('%a, %d-%b-%Y %T %Z', time + MAX_SUDO_TOKEN_AGE, 'GMT');
 
diff --git a/template/en/default/admin/sudo.html.tmpl b/template/en/default/admin/sudo.html.tmpl
index 8cdfb5204..8f7e3fcea 100644
--- a/template/en/default/admin/sudo.html.tmpl
+++ b/template/en/default/admin/sudo.html.tmpl
@@ -17,9 +17,9 @@
 
 <p>
   The <b>sudo</b> feature of Bugzilla allows you to impersonate a
-  user for a short time  While an sudo session is in progress, every action you 
-  perform will be taking place as if you had logged in as the user whom will be 
-  impersonating.
+  user for a short time. While a sudo session is in progress, every action you
+  perform will be taking place as if you had logged in as the user who will be
+  impersonated.
 </p>
 
 <p class="areyoureallyreallysure">
@@ -67,12 +67,8 @@
   
   [% IF user.authorizer.can_login %]
     <p>
-      Finally, enter <label for="Bugzilla_password">your [% terms.Bugzilla %]
-      password</label>:
-      <input type="hidden" name="Bugzilla_login" value="[% user.login FILTER html %]">
-      <input type="password" id="Bugzilla_password" name="Bugzilla_password" size="20" required>
-      <input type="hidden" name="Bugzilla_login_token"
-             value="[% login_request_token FILTER html %]">
+      Finally, enter <label for="password">your [% terms.Bugzilla %] password</label>:
+      <input type="password" id="password" name="password" size="20" required>
       <br>
       This is done for two reasons.  First of all, it is done to reduce 
       the chances of someone doing large amounts of damage using your 
@@ -80,9 +76,8 @@
       time to consider if you really need to use this feature.
     </p>
   [% END %]
-  
+
   <p>
-    Click the button to begin the session:
     <input type="submit" id="begin_sudo" value="Begin Session">
     <input type="hidden" name="action" value="begin-sudo">
     <input type="hidden" name="token" value="[% token FILTER html %]">