Bug 1221423 - unable to impersonate users when 2fa is enabled

2 files changed, 42 insertions, 30 deletions

diff --git a/relogin.cgi b/relogin.cgi
index cfbb52b34..85ba70534 100755
--- a/relogin.cgi
+++ b/relogin.cgi
@@ -90,34 +90,50 @@ 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
+    # log in the user
     my $user = Bugzilla->login(LOGIN_REQUIRED);
 
-    my $target_login = $cgi->param('target_login');
-    my $reason = $cgi->param('reason') || '';
+    my $target_login     = $cgi->param('target_login');
+    my $reason           = $cgi->param('reason') || '';
+    my $token            = $cgi->param('token');
+    my $current_password = $cgi->param('current_password');
+    my $mfa_token        = $cgi->param('mfa_token');
 
-    # 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) {
+    # validate entered password
+    my $crypt_password = $user->cryptpassword;
+    unless (($user->mfa && $mfa_token)
+            || bz_crypt($current_password, $crypt_password) eq $crypt_password)
+    {
         ThrowUserError('sudo_password_required',
                        { target_login => $target_login, reason => $reason });
     }
 
+    # Check for MFA
+    if ($user->mfa) {
+        if (!$mfa_token) {
+            # display 2fa verification
+            $user->mfa_provider->verify_prompt({
+                postback => {
+                    action => 'relogin.cgi',
+                    fields => {
+                        token        => $token,
+                        action       => $action,
+                        reason       => $reason,
+                        target_login => $target_login,
+                    },
+                },
+                reason => 'Impersonating another user',
+            });
+        }
+        else {
+            # verify mfa token and override with values stored in token data
+            my $event = $user->mfa_provider->verify_token($mfa_token);
+            $target_login = $event->{postback}->{fields}->{target_login};
+            $reason       = $event->{postback}->{fields}->{reason};
+            $token        = $event->{postback}->{fields}->{token};
+        }
+    }
+
     # The user must be in the 'bz_sudoers' group
     unless ($user->in_group('bz_sudoers')) {
         ThrowUserError('auth_failure', {  group => 'bz_sudoers',
@@ -125,7 +141,7 @@ 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 });
@@ -133,8 +149,7 @@ elsif ($action eq 'begin-sudo') {
 
     # 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) =
-        Bugzilla::Token::GetTokenData($cgi->param('token'));
+    my ($token_user, $token_timestamp, $token_data) = Bugzilla::Token::GetTokenData($token);
     unless (defined($token_user)
             && defined($token_data)
             && ($token_user == $user->id)
@@ -161,7 +176,7 @@ elsif ($action eq 'begin-sudo') {
     my $time_string = time2str('%a, %d-%b-%Y %T %Z', time + MAX_SUDO_TOKEN_AGE, 'GMT');
 
     # For future sessions, store the unique ID of the target user
-    my $token = Bugzilla::Token::_create_token($user->id, 'sudo', $target_user->id);
+    $token = Bugzilla::Token::_create_token($user->id, 'sudo', $target_user->id);
 
     my %args;
     if (Bugzilla->params->{ssl_redirect}) {
diff --git a/template/en/default/admin/sudo.html.tmpl b/template/en/default/admin/sudo.html.tmpl
index 9c0605567..e88ae2617 100644
--- a/template/en/default/admin/sudo.html.tmpl
+++ b/template/en/default/admin/sudo.html.tmpl
@@ -80,12 +80,9 @@
   
   [% IF user.authorizer.can_login %]
     <p>
-      Finally, enter <label for="Bugzilla_password">your [% terms.Bugzilla %]
+      Finally, enter <label for="current_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">
-      <input type="hidden" name="Bugzilla_login_token"
-             value="[% login_request_token FILTER html %]">
+      <input type="password" id="current_password" name="current_password" size="20">
       <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