Bug 1199087 - extend 2fa protection beyond login

20 files changed, 576 insertions, 210 deletions

diff --git a/Bugzilla/Auth.pm b/Bugzilla/Auth.pm
index a4f2dd9a9..b39bb827b 100644
--- a/Bugzilla/Auth.pm
+++ b/Bugzilla/Auth.pm
@@ -39,6 +39,8 @@ use Bugzilla::Auth::Login::Stack;
 use Bugzilla::Auth::Verify::Stack;
 use Bugzilla::Auth::Persist::Cookie;
 use Socket;
+use URI;
+use URI::QueryParam;
 
 sub new {
     my ($class, $params) = @_;
@@ -93,26 +95,48 @@ sub login {
     }
     $user->set_authorizer($self);
 
-    # trigger multi-factor auth.  once verified the provider calls mfa_verified()
+    # trigger multi-factor auth
     if ($self->{_info_getter}->{successful}->requires_verification
         && $user->mfa
         && !Bugzilla->sudoer
         && !i_am_webservice()
     ) {
-        $user->mfa_provider->prompt({ user => $user, type => $type });
-        exit;
+        my $params = Bugzilla->input_params;
+        my $cgi = Bugzilla->cgi;
+        my $uri = URI->new($cgi->self_url);
+        foreach my $param (qw( Bugzilla_remember Bugzilla_restrictlogin GoAheadAndLogIn )) {
+            $uri->query_param_delete($param);
+        }
+        $user->mfa_provider->verify_prompt({
+            user          => $user,
+            type          => $type,
+            reason        => 'Logging in as ' . $user->identity,
+            restrictlogin => $params->{Bugzilla_restrictlogin},
+            remember      => $params->{Bugzilla_remember},
+            url           => $uri->as_string,
+            postback      => {
+                action      => 'token.cgi',
+                token_field => 't',
+                fields      => {
+                    a => 'mfa_l',
+                },
+            }
+        });
     }
 
     return $self->_handle_login_result($login_info, $type);
 }
 
 sub mfa_verified {
-    my ($self, $user, $type) = @_;
+    my ($self, $user, $event) = @_;
     require Bugzilla::Auth::Login::CGI;
+
+    my $params = Bugzilla->input_params;
     $self->{_info_getter}->{successful} = Bugzilla::Auth::Login::CGI->new();
-    $self->_handle_login_result({ user => $user }, $type);
-    print Bugzilla->cgi->redirect('index.cgi');
-    exit;
+    $params->{Bugzilla_restrictlogin}   = $event->{restrictlogin};
+    $params->{Bugzilla_remember}        = $event->{remember};
+
+    $self->_handle_login_result({ user => $user }, $event->{type});
 }
 
 sub successful_info_getter {
diff --git a/Bugzilla/Constants.pm b/Bugzilla/Constants.pm
index 59796a076..2fd6a23b1 100644
--- a/Bugzilla/Constants.pm
+++ b/Bugzilla/Constants.pm
@@ -173,6 +173,7 @@ use Memoize;
     ON_ACTIVESTATE
 
     MAX_TOKEN_AGE
+    MAX_SHORT_TOKEN_HOURS
     MAX_LOGINCOOKIE_AGE
     MAX_SUDO_TOKEN_AGE
     MAX_LOGIN_ATTEMPTS
@@ -469,6 +470,8 @@ use constant TIMETRACKING_FIELDS =>
 
 # The maximum number of days a token will remain valid.
 use constant MAX_TOKEN_AGE => 3;
+# The maximum number of hours a short-lived token will remain valid.
+use constant MAX_SHORT_TOKEN_HOURS => 1;
 # How many days a logincookie will remain valid if not used.
 use constant MAX_LOGINCOOKIE_AGE => 7;
 # How many seconds (default is 6 hours) a sudo cookie remains valid.
diff --git a/Bugzilla/MFA.pm b/Bugzilla/MFA.pm
index 5db38e55e..21f42fbfb 100644
--- a/Bugzilla/MFA.pm
+++ b/Bugzilla/MFA.pm
@@ -8,6 +8,8 @@
 package Bugzilla::MFA;
 use strict;
 
+use Bugzilla::Token qw( issue_short_lived_session_token set_token_extra_data get_token_extra_data delete_token );
+
 sub new {
     my ($class, $user) = @_;
     return bless({ user => $user }, $class);
@@ -27,9 +29,42 @@ sub prompt {}
 # throws errors if code is invalid
 sub check {}
 
-# during-login verification
-sub check_login {}
+# verification
+
+sub verify_prompt {
+    my ($self, $event) = @_;
+    my $user = delete $event->{user} // Bugzilla->user;
+
+    # generate token and attach mfa data
+    my $token = issue_short_lived_session_token('mfa', $user);
+    set_token_extra_data($token, $event);
+
+    # trigger provider verification
+    my $token_field = $event->{postback}->{token_field} // 'mfa_token';
+    $event->{postback}->{fields}->{$token_field} = $token;
+    $self->prompt($event);
+    exit;
+}
 
+sub verify_check {
+    my ($self, $token) = @_;
+
+    # check token
+    my ($user_id) = Bugzilla::Token::GetTokenData($token);
+    my $user = Bugzilla::User->check({ id => $user_id, cache => 1 });
+
+    # mfa verification
+    $self->check(Bugzilla->input_params);
+
+    # return event data
+    my $event = get_token_extra_data($token);
+    delete_token($token);
+    if (!$event) {
+        print Bugzilla->cgi->redirect('index.cgi');
+        exit;
+    }
+    return $event;
+}
 
 # helpers
 
diff --git a/Bugzilla/MFA/TOTP.pm b/Bugzilla/MFA/TOTP.pm
index 05e4e4e3b..859ca4b8d 100644
--- a/Bugzilla/MFA/TOTP.pm
+++ b/Bugzilla/MFA/TOTP.pm
@@ -48,32 +48,25 @@ sub enrolled {
 }
 
 sub prompt {
-    my ($self, $params) = @_;
+    my ($self, $vars) = @_;
     my $template = Bugzilla->template;
 
-    my $vars = {
-        user  => $params->{user},
-        token => scalar issue_session_token('mfa', $params->{user}),
-        type  => $params->{type},
-    };
-
     print Bugzilla->cgi->header();
     $template->process('mfa/totp/verify.html.tmpl', $vars)
         || ThrowTemplateError($template->error());
 }
 
 sub check {
-    my ($self, $code) = @_;
-    $self->_auth()->verify($code, 1)
-        || ThrowUserError('mfa_totp_bad_code');
-}
-
-sub check_login {
-    my ($self, $user) = @_;
-    my $cgi = Bugzilla->cgi;
+    my ($self, $params) = @_;
+    my $code = $params->{code} // '';
+    return if $self->_auth()->verify($code, 1);
 
-    $self->check($cgi->param('code') // '');
-    $user->authorizer->mfa_verified($user, $cgi->param('type'));
+    if (exists $params->{mfa_action}) {
+        ThrowUserError('mfa_totp_bad_enrolment_code');
+    }
+    else {
+        ThrowUserError('mfa_totp_bad_code');
+    }
 }
 
 1;
diff --git a/Bugzilla/Token.pm b/Bugzilla/Token.pm
index 6e1414db6..3c5261821 100644
--- a/Bugzilla/Token.pm
+++ b/Bugzilla/Token.pm
@@ -41,13 +41,17 @@ use Date::Parse;
 use File::Basename;
 use Digest::MD5 qw(md5_hex);
 use Digest::SHA qw(hmac_sha256_base64);
+use Encode;
+use JSON qw(encode_json decode_json);
 
 use base qw(Exporter);
 
 @Bugzilla::Token::EXPORT = qw(issue_api_token issue_session_token
+                              issue_short_lived_session_token
                               issue_auth_delegation_token check_auth_delegation_token
                               check_token_data delete_token
-                              issue_hash_token check_hash_token);
+                              issue_hash_token check_hash_token
+                              set_token_extra_data get_token_extra_data);
 
 # 128 bits password:
 # 128 * log10(2) / log10(62) = 21.49, round up to 22.
@@ -227,6 +231,15 @@ sub issue_session_token {
     return _create_token($user->id, 'session', $data);
 }
 
+sub issue_short_lived_session_token {
+    my ($data, $user) = @_;
+    # Generates a random token, adds it to the tokens table, and returns
+    # the token to the caller.
+
+    $user //= Bugzilla->user;
+    return _create_token($user->id, 'session.short', $data);
+}
+
 sub issue_hash_token {
     my ($data, $time) = @_;
     $data ||= [];
@@ -290,6 +303,9 @@ sub CleanTokenTable {
     $dbh->do("DELETE FROM tokens WHERE " .
              $dbh->sql_date_math('issuedate', '+', '?', 'HOUR') . " <= NOW()",
              undef, MAX_TOKEN_AGE * 24);
+    $dbh->do("DELETE FROM tokens WHERE tokentype = ? AND " .
+             $dbh->sql_date_math('issuedate', '+', '?', 'HOUR') . " <= NOW()",
+             undef, 'session.short', MAX_SHORT_TOKEN_HOURS);
 }
 
 sub GenerateUniqueToken {
@@ -475,6 +491,35 @@ sub check_token_data {
     return 1;
 }
 
+sub set_token_extra_data {
+    my ($token, $data) = @_;
+
+    $data = encode_json($data) if ref($data);
+
+    # extra_data is MEDIUMTEXT, max 16M
+    if (length($data) > 16_777_215) {
+        ThrowCodeError('token_data_too_big');
+    }
+
+    Bugzilla->dbh->do(
+        "INSERT INTO token_data (token, extra_data) VALUES (?, ?) ON DUPLICATE KEY UPDATE extra_data = ?",
+        undef, $token, $data, $data);
+}
+
+sub get_token_extra_data {
+    my ($token) = @_;
+    trick_taint($token);
+    my ($data) = Bugzilla->dbh->selectrow_array(
+        "SELECT extra_data FROM token_data WHERE token = ?",
+        undef, $token);
+    return undef unless defined $data;
+    $data = encode('UTF-8', $data);
+    eval {
+        $data = decode_json($data);
+    };
+    return $data;
+}
+
 ################################################################################
 # Internal Functions
 ################################################################################
diff --git a/images/mfa.png b/images/mfa.png
new file mode 100644
index 000000000..781c4837f
--- /dev/null
+++ b/images/mfa.png
diff --git a/js/account.js b/js/account.js
index 0525045b1..12a6c7a10 100644
--- a/js/account.js
+++ b/js/account.js
@@ -52,56 +52,43 @@ $(function() {
 
     // mfa
 
-    $('#mfa-enable')
+    $('#mfa-select-totp')
         .click(function(event) {
             event.preventDefault();
-            $('#mfa-enable-container').show();
-            $(this).hide();
-        });
-
-    $('#mfa')
-        .change(function(event) {
-            var mfa = $(this).val();
+            $('#mfa').val('TOTP');
 
-            $('.mfa-provider').hide();
+            $('#mfa-select').hide();
             $('#update').attr('disabled', true);
-            if (mfa === '') {
-                $('#mfa-confirm').hide();
-            }
-            else {
-                $('#mfa-confirm').show();
-                $('.mfa-api-blurb').show();
-                if (mfa === 'TOTP') {
-                    $('#mfa-enable-totp').show();
-                    $('#mfa-totp-throbber').show();
-                    $('#mfa-totp-issued').hide();
-                    var url = 'rest/user/mfa/totp/enroll' +
-                        '?Bugzilla_api_token=' + encodeURIComponent(BUGZILLA.api_token);
-                    $.ajax({
-                        "url": url,
-                        "contentType": "application/json",
-                        "processData": false
-                    })
-                    .done(function(data) {
-                        $('#mfa-totp-throbber').hide();
-                        var iframe = $('#mfa-enable-totp-frame').contents();
-                        iframe.find('#qr').attr('src', 'data:image/png;base64,' + data.png);
-                        iframe.find('#secret').text(data.secret32);
-                        $('#mfa-totp-issued').show();
-                        $('#mfa-password').focus();
-                        $('#update').attr('disabled', false);
-                    })
-                    .error(function(data) {
-                        $('#mfa-totp-throbber').hide();
-                        if (data.statusText === 'abort')
-                            return;
-                        var message = data.responseJSON ? data.responseJSON.message : 'Unexpected Error';
-                        console.log(message);
-                    });
-                }
-            }
-        })
-        .change();
+            $('#mfa-confirm').show();
+            $('.mfa-api-blurb').show();
+            $('#mfa-enable-totp').show();
+            $('#mfa-totp-throbber').show();
+            $('#mfa-totp-issued').hide();
+
+            var url = 'rest/user/mfa/totp/enroll' +
+                '?Bugzilla_api_token=' + encodeURIComponent(BUGZILLA.api_token);
+            $.ajax({
+                "url": url,
+                "contentType": "application/json",
+                "processData": false
+            })
+            .done(function(data) {
+                $('#mfa-totp-throbber').hide();
+                var iframe = $('#mfa-enable-totp-frame').contents();
+                iframe.find('#qr').attr('src', 'data:image/png;base64,' + data.png);
+                iframe.find('#secret').text(data.secret32);
+                $('#mfa-totp-issued').show();
+                $('#mfa-password').focus();
+                $('#update').attr('disabled', false);
+            })
+            .error(function(data) {
+                $('#mfa-totp-throbber').hide();
+                if (data.statusText === 'abort')
+                    return;
+                var message = data.responseJSON ? data.responseJSON.message : 'Unexpected Error';
+                console.log(message);
+            });
+        });
 
     $('#mfa-disable')
         .click(function(event) {
@@ -111,6 +98,7 @@ $(function() {
             $('.mfa-api-blurb').hide();
             $('#mfa-password').focus();
             $('#update').attr('disabled', false);
+            $('.mfa-protected').hide();
             $(this).hide();
         });
 
@@ -132,8 +120,28 @@ $(function() {
 
     if ($('#mfa-action').length) {
         $('#update').attr('disabled', true);
-        $(window).on('pageshow', function() {
-            $('#mfa').val('').change();
-        });
     }
+
+    // api-key
+
+    $('#apikey-toggle-revoked')
+        .click(function(event) {
+            event.preventDefault();
+            $('.apikey_revoked.bz_tui_hidden').removeClass('bz_tui_hidden');
+            if ($('.apikey_revoked').is(':visible')) {
+                $('.apikey_revoked').hide();
+                $(this).text('Show Revoked Keys');
+            }
+            else {
+                $('.apikey_revoked').show();
+                $(this).text('Hide Revoked Keys');
+            }
+        });
+
+    $('#new_key')
+        .change(function(event) {
+            if ($(this).is(':checked')) {
+                $('#new_description').focus();
+            }
+        });
 });
diff --git a/skins/standard/admin.css b/skins/standard/admin.css
index 44b474b8f..160d51858 100644
--- a/skins/standard/admin.css
+++ b/skins/standard/admin.css
@@ -199,6 +199,7 @@ input[disabled] {
     padding: .2em 1.5em 1.5em 1.5em;
     box-shadow: 1px 1px 5px rgba(0, 0, 0, 0.1);
     background: #FFF none repeat scroll 0% 0%;
+    min-height: 30em;
 }
 
 #prefnav, #prefcontent {
diff --git a/skins/standard/global.css b/skins/standard/global.css
index 2db958458..3f0ec55a7 100644
--- a/skins/standard/global.css
+++ b/skins/standard/global.css
@@ -255,6 +255,11 @@ div#docslinks {
     margin: 0;
 }
 
+.mfa-protected {
+    vertical-align: middle;
+    margin: 0 4px;
+}
+
 /**************************/
 /* Bug links and statuses */
 /**************************/
@@ -406,7 +411,7 @@ table#flags td {
 }
 
 .column_header {
-    background-color: #66f;
+    background-color: #ddeef9;
 }
 
 .column_header th {
diff --git a/template/en/default/account/password/set-forgotten-password.html.tmpl b/template/en/default/account/password/set-forgotten-password.html.tmpl
index a2ae517c8..cfeacbb93 100644
--- a/template/en/default/account/password/set-forgotten-password.html.tmpl
+++ b/template/en/default/account/password/set-forgotten-password.html.tmpl
@@ -36,18 +36,19 @@
         (minimum [% constants.USER_PASSWORD_MIN_LENGTH FILTER none %] characters)
       </td>
     </tr>
-    
+
     <tr>
       <th align="right">New Password Again:</th>
       <td>
         <input type="password" name="matchpassword">
       </td>
     </tr>
-    
+
     <tr>
       <th align="right">&nbsp;</th>
       <td>
         <input type="submit" id="update" value="Submit">
+        [% INCLUDE mfa/protected.html.tmpl user=token_user %]
       </td>
     </tr>
   </table>
diff --git a/template/en/default/account/prefs/account.html.tmpl b/template/en/default/account/prefs/account.html.tmpl
index bfae7f071..3f838691b 100644
--- a/template/en/default/account/prefs/account.html.tmpl
+++ b/template/en/default/account/prefs/account.html.tmpl
@@ -72,6 +72,7 @@
         <th align="right">New password:</th>
         <td>
           <input type="password" name="new_password1">
+          [% INCLUDE "mfa/protected.html.tmpl" %]
         </td>
       </tr>
 
@@ -109,6 +110,7 @@
           <th align="right">New email address:</th>
           <td>
             <input size="35" name="new_login_name">
+            [% INCLUDE "mfa/protected.html.tmpl" %]
           </td>
         </tr>
       [% END %]
diff --git a/template/en/default/account/prefs/apikey.html.tmpl b/template/en/default/account/prefs/apikey.html.tmpl
index 8b740cf1e..926f3838b 100644
--- a/template/en/default/account/prefs/apikey.html.tmpl
+++ b/template/en/default/account/prefs/apikey.html.tmpl
@@ -14,8 +14,10 @@
 <p>
   API keys are used to authenticate WebService API calls. You can create more than
   one API key if required. Each API key has an optional description which can help
-  you record what each key is used for. Documentation on how to log in is available from
-  <a href="https://bugzilla.readthedocs.org/en/latest/api/core/v1/general.html#authentication">
+  you record what each key is used for.<br>
+  <br>
+  Documentation on how to log in is available
+  <a href="https://bmo.readthedocs.org/en/latest/api/core/v1/general.html#authentication">
     here</a>.
 </p>
 
@@ -33,7 +35,7 @@ here.</p>
   </tr>
 
   [% FOREACH api_key IN api_keys %]
-    <tr[% IF api_key.revoked %] class="apikey_revoked"[% END %]>
+    <tr[% IF api_key.revoked %] class="apikey_revoked bz_tui_hidden" style="display:none"[% END %]>
       <td>[% api_key.api_key FILTER html %]</td>
       <td>
         <input name="description_[% api_key.id FILTER html %]"
@@ -52,6 +54,9 @@ here.</p>
           name="revoked_[% api_key.id FILTER html %]"
           id="revoked_[% api_key.id FILTER html %]"
           [% IF api_key.revoked %] checked="checked" [% END %]>
+        [% IF api_key.revoked %]
+          [% INCLUDE "mfa/protected.html.tmpl" %]
+        [% END %]
       </td>
     </tr>
   [% END %]
@@ -61,15 +66,7 @@ here.</p>
 </table>
 
 [% IF any_revoked %]
-  <a id="apikey_revoked_controller" class="bz_default_hidden"
-     href="javascript:TUI_toggle_class('apikey_revoked')">Hide Revoked Keys</a>
-  [%# Show the link if the browser supports JS %]
-  <script type="text/javascript">
-    TUI_hide_default('apikey_revoked');
-    TUI_alternates['apikey_revoked'] = 'Show Revoked Keys';
-    YAHOO.util.Dom.removeClass('apikey_revoked_controller',
-                               'bz_default_hidden');
-  </script>
+  <a href="#" id="apikey-toggle-revoked">Show Revoked Keys</a>
 [% END %]
 
 <h3>New API key</h3>
@@ -79,10 +76,10 @@ providing a description for the API key. The API key will be randomly
 generated for you.</p>
 
 <p>
-  <input type="checkbox" name="new_key" id="new_key"
-         onchange="if (this.checked) YAHOO.util.Dom.get('new_description').focus();">
+  <input type="checkbox" name="new_key" id="new_key">
   <label for="new_key">
     Generate a new API key with optional description</label>
   <input name="new_description" id="new_description">
+  [% INCLUDE "mfa/protected.html.tmpl" %]
 </p>
 
diff --git a/template/en/default/account/prefs/mfa.html.tmpl b/template/en/default/account/prefs/mfa.html.tmpl
index e3751a5b7..5aed954f9 100644
--- a/template/en/default/account/prefs/mfa.html.tmpl
+++ b/template/en/default/account/prefs/mfa.html.tmpl
@@ -33,6 +33,7 @@
     <input type="hidden" name="mfa_action" id="mfa-action" value="disable">
 
     <button type="button" id="mfa-disable">Disable Two-factor Authentication</button>
+    [% INCLUDE "mfa/protected.html.tmpl" %]
 
     <div id="mfa-disable-container" style="display:none">
 
@@ -50,7 +51,7 @@
 
       [% IF user.mfa == "TOTP" %]
         <label class="mfa-totp">Code:</label>
-        <input type="text" name="mfa_disable_code" id="mfa-totp-disable-code"
+        <input type="text" name="code" id="mfa-totp-disable-code"
                placeholder="123456" maxlength="6" pattern="\d{6}" size="10"
                autocomplete="off" required autofocus>
       [% END %]
@@ -79,70 +80,67 @@
       Two-factor authentication is currently <b>disabled</b>.
     </p>
     <input type="hidden" name="mfa_action" id="mfa-action" value="enable">
+    <input type="hidden" name="mfa" id="mfa">
 
-    <button type="button" id="mfa-enable">Enable Two-factor Authentication</button>
-
-    <div id="mfa-enable-container" style="display:none">
-      <b>System:</b>
-      <select name="mfa" id="mfa">
-        <option value="" selected></option>
-        <option value="TOTP">Time-based One-Time Password (TOTP)</option>
-      </select>
+    <div id="mfa-select">
+      <p>
+        Select the two-factor system you want to use:
+      </p>
+      <button type="button" id="mfa-select-totp">Time-based One-Time Password (TOTP)</button>
+    </div>
 
-      [%# TOTP %]
-      <div id="mfa-enable-totp" class="mfa-provider" style="display:none">
+    [%# TOTP %]
+    <div id="mfa-enable-totp" class="mfa-provider" style="display:none">
 
-        <p>
-          Your current password is required to enable two-factor authentication.
-        </p>
-        <p>
-          <label class="mfa-totp">Current Password:</label>
-          <input type="password" name="password" id="mfa-password" required>
-        </p>
+      <p>
+        Your current password is required to enable two-factor authentication.
+      </p>
+      <p>
+        <label class="mfa-totp">Current Password:</label>
+        <input type="password" name="password" id="mfa-password" required>
+      </p>
 
-        <div id="mfa-totp-throbber">
-          Generating new QR code.. <img src="skins/standard/throbber.gif" width="16" height="11">
-        </div>
+      <div id="mfa-totp-throbber">
+        Generating new QR code.. <img src="skins/standard/throbber.gif" width="16" height="11">
+      </div>
 
-        <div id="mfa-totp-issued" style="display:none">
-          <iframe id="mfa-enable-totp-frame" src="userprefs.cgi?tab=mfa&frame=totp" tabindex="-1"></iframe>
-          <div id="mfa-totp-blurb">
-            Scan this QR code with your <a href="#" id="mfa-totp-apps">TOTP App</a>,
-            then enter the six digit code the app generates.<br>
-            <br>
-            <label class="mfa-totp">Code:</label>
-            <input type="text" name="mfa_enable_code" id="mfa-totp-enable-code"
-                   placeholder="123456" maxlength="6" pattern="\d{6}" size="10"
-                   autocomplete="off" required autofocus>
-          </div>
+      <div id="mfa-totp-issued" style="display:none">
+        <iframe id="mfa-enable-totp-frame" src="userprefs.cgi?tab=mfa&frame=totp" tabindex="-1"></iframe>
+        <div id="mfa-totp-blurb">
+          Scan this QR code with your <a href="#" id="mfa-totp-apps">TOTP App</a>,
+          then enter the six digit code the app generates.<br>
+          <br>
+          <label class="mfa-totp">Code:</label>
+          <input type="text" name="code" id="mfa-totp-enable-code"
+                  placeholder="123456" maxlength="6" pattern="\d{6}" size="10"
+                  autocomplete="off" required autofocus>
         </div>
+      </div>
 
-        <p>
-          If you have problems enrolling, this may be due to an inaccurate time on your device.<br>
-          Please check that the time on your device is accurate by visiting <b>http://time.is/</b>.
-        </p>
-
-        <div id="mfa-totp-apps-popup" class="mfa-totp-popup" style="display:none">
-          Example TOTP Applications:<br>
-          <ul>
-            <li>Android and iOS:
-              <a href="https://support.google.com/accounts/answer/1066447" target="_blank">Google Authenticator</a>,
-              <a href="https://fedorahosted.org/freeotp/" target="_blank">Red Hat FreeOTP</a>
-            </li>
-            <li>Firefox OS:
-              <a href="https://marketplace.firefox.com/app/firekey/" target="_blank">Firekey</a>
-            </li>
-            <li>Windows Phone:
-              <a href="http://www.windowsphone.com/en-us/store/app/authenticator/021dd79f-0598-e011-986b-78e7d1fa76f8"
-                 target="_blank">Authenticator</a>
-            </li>
-          </ul>
-          <a href="https://en.wikipedia.org/wiki/Time-based_One-time_Password_Algorithm#Client_implementations" target="_blank">
-            Other clients
-          </a>
-          <button type="button" class="mfa-totp-popup-close">Close</button>
-        </div>
+      <p>
+        If you have problems enrolling, this may be due to an inaccurate time on your device.<br>
+        Please check that the time on your device is accurate by visiting <b>http://time.is/</b>.
+      </p>
 
+      <div id="mfa-totp-apps-popup" class="mfa-totp-popup" style="display:none">
+        Example TOTP Applications:<br>
+        <ul>
+          <li>Android and iOS:
+            <a href="https://support.google.com/accounts/answer/1066447" target="_blank">Google Authenticator</a>,
+            <a href="https://fedorahosted.org/freeotp/" target="_blank">Red Hat FreeOTP</a>
+          </li>
+          <li>Firefox OS:
+            <a href="https://marketplace.firefox.com/app/firekey/" target="_blank">Firekey</a>
+          </li>
+          <li>Windows Phone:
+            <a href="http://www.windowsphone.com/en-us/store/app/authenticator/021dd79f-0598-e011-986b-78e7d1fa76f8"
+                target="_blank">Authenticator</a>
+          </li>
+        </ul>
+        <a href="https://en.wikipedia.org/wiki/Time-based_One-time_Password_Algorithm#Client_implementations" target="_blank">
+          Other clients
+        </a>
+        <button type="button" class="mfa-totp-popup-close">Close</button>
       </div>
 
     </div>
diff --git a/template/en/default/account/prefs/settings.html.tmpl b/template/en/default/account/prefs/settings.html.tmpl
index 65e31359b..0147f95ef 100644
--- a/template/en/default/account/prefs/settings.html.tmpl
+++ b/template/en/default/account/prefs/settings.html.tmpl
@@ -62,6 +62,9 @@
                 </option>
               [% END %]
             </select>
+            [% IF name == "api_key_only" %]
+              [% INCLUDE "mfa/protected.html.tmpl" %]
+            [% END %]
           [% ELSE %]
             <select name="[% name FILTER html %]" id="[% name FILTER html %]" disabled="disabled">
               <option value="[% default_name FILTER html %]">
diff --git a/template/en/default/global/code-error.html.tmpl b/template/en/default/global/code-error.html.tmpl
index 316e98450..c4ff7e73a 100644
--- a/template/en/default/global/code-error.html.tmpl
+++ b/template/en/default/global/code-error.html.tmpl
@@ -453,6 +453,9 @@
   [% ELSIF error == "token_generation_error" %]
     Something is seriously wrong with the token generation system.
 
+  [% ELSIF error == "token_data_too_big" %]
+    The data is too large to store in a token.
+
   [% ELSIF error == "cancel_token_does_not_exist" %]
     The token to be cancelled does not exist.
 
diff --git a/template/en/default/global/user-error.html.tmpl b/template/en/default/global/user-error.html.tmpl
index 1ec3202bb..7a3a536cd 100644
--- a/template/en/default/global/user-error.html.tmpl
+++ b/template/en/default/global/user-error.html.tmpl
@@ -1215,6 +1215,11 @@
   [% ELSIF error == "mfa_totp_bad_code" %]
     Invalid verification code.
 
+  [% ELSIF error == "mfa_totp_bad_enrolment_code" %]
+    Invalid verification code.<br>
+    <br>
+    The QR code has been deleted - please generate and scan a new code.
+
   [% ELSIF error == "migrate_config_created" %]
     The file <kbd>[% file FILTER html %]</kbd> contains configuration
     variables that must be set before continuing with the migration.
@@ -1468,8 +1473,8 @@
     You did not enter your old password correctly.
 
   [% ELSIF error == "old_password_required" %]
-    [% title = "Old Password Required" %]
-    You must enter your old password to change your email address.
+    [% title = "Password Required" %]
+    You must enter your current password to change your email address.
 
   [% ELSIF error == "password_change_requests_not_allowed" %]
     [% title = "Password Change Requests Not Allowed" %]
diff --git a/template/en/default/mfa/protected.html.tmpl b/template/en/default/mfa/protected.html.tmpl
new file mode 100644
index 000000000..da945244d
--- /dev/null
+++ b/template/en/default/mfa/protected.html.tmpl
@@ -0,0 +1,12 @@
+[%# This Source Code Form is subject to the terms of the Mozilla Public
+  # License, v. 2.0. If a copy of the MPL was not distributed with this
+  # file, You can obtain one at http://mozilla.org/MPL/2.0/.
+  #
+  # This Source Code Form is "Incompatible With Secondary Licenses", as
+  # defined by the Mozilla Public License, v. 2.0.
+  #%]
+
+[% RETURN UNLESS user.mfa %]
+
+<img src="images/mfa.png" class="mfa-protected" width="16" height="16"
+     alt="2FA" title="Protected by Two-factor Authentication">
diff --git a/template/en/default/mfa/totp/verify.html.tmpl b/template/en/default/mfa/totp/verify.html.tmpl
index 3ff720d62..e61ee3866 100644
--- a/template/en/default/mfa/totp/verify.html.tmpl
+++ b/template/en/default/mfa/totp/verify.html.tmpl
@@ -13,17 +13,19 @@
 <h1>Account Verification</h1>
 
 <p>
+  <b>[% reason FILTER html %]</b> requires verification.<br>
   Please enter your verification code from your TOTP application:
 </p>
 
-<form method="POST" action="token.cgi">
-<input type="hidden" name="a" value="mfa">
-<input type="hidden" name="t" value="[% token FILTER html %]">
-<input type="text" name="code" id="code"
-       placeholder="123456" maxlength="6" pattern="\d{6}" size="10"
-       autocomplete="off" required autofocus><br>
-<br>
-<input type="submit" value="Submit">
+<form method="POST" action="[% postback.action FILTER none %]">
+  [% FOREACH field IN postback.fields.keys %]
+    <input type="hidden" name="[% field FILTER html %]" value="[% postback.fields.item(field) FILTER html %]">
+  [% END %]
+  <input type="text" name="code" id="code"
+        placeholder="123456" maxlength="6" pattern="\d{6}" size="10"
+        autocomplete="off" required autofocus><br>
+  <br>
+  <input type="submit" value="Submit">
 </form>
 
 [% INCLUDE global/footer.html.tmpl %]
diff --git a/token.cgi b/token.cgi
index 9ae307215..26b402159 100755
--- a/token.cgi
+++ b/token.cgi
@@ -39,6 +39,7 @@ use Bugzilla::User;
 
 use Date::Format;
 use Date::Parse;
+use JSON qw( decode_json );
 
 my $dbh = Bugzilla->dbh;
 local our $cgi = Bugzilla->cgi;
@@ -93,7 +94,7 @@ if ($token) {
       Bugzilla::Token::Cancel($token, 'wrong_token_for_creating_account');
       ThrowUserError('wrong_token_for_creating_account');
   }
-  if ($action eq 'mfa' && $tokentype ne 'session') {
+  if (substr($action, 0, 4) eq 'mfa_' && $tokentype ne 'session.short') {
       Bugzilla::Token::Cancel($token, 'wrong_token_for_mfa');
       ThrowUserError('wrong_token_for_mfa');
   }
@@ -172,8 +173,10 @@ if ($action eq 'reqpw') {
     confirm_create_account($token);
 } elsif ($action eq 'cancel_new_account') {
     cancel_create_account($token);
-} elsif ($action eq 'mfa') {
-    verify_mfa($token);
+} elsif ($action eq 'mfa_l') {
+    verify_mfa_login($token);
+} elsif ($action eq 'mfa_p') {
+    verify_mfa_password($token);
 } else { 
     ThrowUserError('unknown_action', {action => $action});
 }
@@ -199,6 +202,9 @@ sub confirmChangePassword {
     my $token = shift;
     $vars->{'token'} = $token;
 
+    my ($user_id) = Bugzilla::Token::GetTokenData($token);
+    $vars->{token_user} = Bugzilla::User->check({ id => $user_id, cache => 1 });
+
     print $cgi->header();
     $template->process("account/password/set-forgotten-password.html.tmpl", $vars)
       || ThrowTemplateError($template->error());
@@ -218,14 +224,44 @@ sub changePassword {
     my ($token, $password) = @_;
     my $dbh = Bugzilla->dbh;
 
-    my ($user_id) = $dbh->selectrow_array('SELECT userid FROM tokens WHERE token = ?', undef, $token);
+    my ($user_id) = Bugzilla::Token::GetTokenData($token);
     my $user = Bugzilla::User->check({ id => $user_id });
+
+    if ($user->mfa) {
+        $user->mfa_provider->verify_prompt({
+            user     => $user,
+            reason   => 'Setting your password',
+            password => $password,
+            token    => $token,
+            postback => {
+                action      => 'token.cgi',
+                token_field => 't',
+                fields      => {
+                    a => 'mfa_p',
+                },
+            },
+        });
+    }
+    else {
+        set_user_password($token, $user, $password);
+    }
+}
+
+sub verify_mfa_password {
+    my $token = shift;
+    my ($user, $event) = mfa_event_from_token($token);
+    set_user_password($event->{token}, $user, $event->{password});
+}
+
+sub set_user_password {
+    my ($token, $user, $password) = @_;
+
     $user->set_password($password);
     $user->update();
     delete_token($token);
-    $dbh->do("DELETE FROM tokens WHERE userid = ? AND tokentype = 'password'", undef, $user_id);
+    $dbh->do("DELETE FROM tokens WHERE userid = ? AND tokentype = 'password'", undef, $user->id);
 
-    Bugzilla->logout_user_by_id($user_id);
+    Bugzilla->logout_user_by_id($user->id);
 
     $vars->{'message'} = "password_changed";
 
@@ -415,15 +451,29 @@ sub cancel_create_account {
       || ThrowTemplateError($template->error());
 }
 
-sub verify_mfa {
+sub verify_mfa_login {
+    my $token = shift;
+    my ($user, $event) = mfa_event_from_token($token);
+    $user->authorizer->mfa_verified($user, $event);
+    print Bugzilla->cgi->redirect($event->{url} // 'index.cgi');
+    exit;
+}
+
+sub mfa_event_from_token {
     my $token = shift;
+
+    # create user from token
     my ($user_id) = Bugzilla::Token::GetTokenData($token);
     my $user = Bugzilla::User->check({ id => $user_id, cache => 1 });
+
+    # sanity check
     if (!$user->mfa) {
         delete_token($token);
         print Bugzilla->cgi->redirect('index.cgi');
         exit;
     }
-    $user->mfa_provider->check_login($user);
-    delete_token($token);
+
+    # verify
+    my $event = $user->mfa_provider->verify_check($token);
+    return ($user, $event);
 }
diff --git a/userprefs.cgi b/userprefs.cgi
index 9b86f010c..dcb518b80 100755
--- a/userprefs.cgi
+++ b/userprefs.cgi
@@ -91,10 +91,11 @@ sub SaveAccount {
 
     my $user = Bugzilla->user;
 
-    my $oldpassword = $cgi->param('old_password');
-    my $pwd1 = $cgi->param('new_password1');
-    my $pwd2 = $cgi->param('new_password2');
+    my $oldpassword    = $cgi->param('old_password');
+    my $pwd1           = $cgi->param('new_password1');
+    my $pwd2           = $cgi->param('new_password2');
     my $new_login_name = trim($cgi->param('new_login_name'));
+    my @mfa_events;
 
     if ($user->authorizer->can_change_password
         && ($oldpassword ne "" || $pwd1 ne "" || $pwd2 ne ""))
@@ -111,9 +112,18 @@ sub SaveAccount {
             validate_password($pwd1, $pwd2);
 
             if ($oldpassword ne $pwd1) {
-                $user->set_password($pwd1);
-                # Invalidate all logins except for the current one
-                Bugzilla->logout(LOGOUT_KEEP_CURRENT);
+                if ($user->mfa) {
+                    push @mfa_events, {
+                        type     => 'set_password',
+                        reason   => 'changing your password',
+                        password => $pwd1,
+                    };
+                }
+                else {
+                    $user->set_password($pwd1);
+                    # Invalidate all logins except for the current one
+                    Bugzilla->logout(LOGOUT_KEEP_CURRENT);
+                }
             }
         }
     }
@@ -136,15 +146,63 @@ sub SaveAccount {
             is_available_username($new_login_name)
               || ThrowUserError("account_exists", {email => $new_login_name});
 
-            Bugzilla::Token::IssueEmailChangeToken($user, $new_login_name);
-
-            $vars->{'email_changes_saved'} = 1;
+            if ($user->mfa) {
+                push @mfa_events, {
+                    type   => 'set_login',
+                    reason => 'changing your email address',
+                    login  => $new_login_name,
+                };
+            }
+            else {
+                Bugzilla::Token::IssueEmailChangeToken($user, $new_login_name);
+                $vars->{email_changes_saved} = 1;
+            }
         }
     }
 
     $user->set_name($cgi->param('realname'));
     $user->update({ keep_session => 1, keep_tokens => 1 });
     $dbh->bz_commit_transaction;
+
+    if (@mfa_events) {
+        # build the fields for the postback
+        my $mfa_event = {
+            postback => {
+                action => 'userprefs.cgi',
+                fields => {
+                    tab => 'account',
+                },
+            },
+            reason => ucfirst(join(' and ', map { $_->{reason} } @mfa_events)),
+            actions => \@mfa_events,
+        };
+        # display 2fa verification
+        $user->mfa_provider->verify_prompt($mfa_event);
+    }
+}
+
+sub MfaAccount {
+    my $cgi = Bugzilla->cgi;
+    my $user = Bugzilla->user;
+    my $dbh = Bugzilla->dbh;
+    return unless $user->mfa;
+
+    my $event = $user->mfa_provider->verify_check($cgi->param('mfa_token'));
+
+    foreach my $action (@{ $event->{actions} }) {
+        if ($action->{type} eq 'set_login') {
+            Bugzilla::Token::IssueEmailChangeToken($user, $action->{login});
+            $vars->{email_changes_saved} = 1;
+        }
+
+        elsif ($action->{type} eq 'set_password') {
+            $dbh->bz_begin_transaction;
+            $user->set_password($action->{password});
+            Bugzilla->logout(LOGOUT_KEEP_CURRENT);
+            $user->update({ keep_session => 1, keep_tokens => 1 });
+            $dbh->bz_commit_transaction;
+        }
+    }
 }
 
 sub DisableAccount {
@@ -190,8 +248,9 @@ sub SaveSettings {
     my $cgi = Bugzilla->cgi;
     my $user = Bugzilla->user;
 
-    my $settings = $user->settings;
+    my $settings     = $user->settings;
     my @setting_list = keys %$settings;
+    my $mfa_event    = undef;
 
     foreach my $name (@setting_list) {
         next if ! ($settings->{$name}->{'is_enabled'});
@@ -199,14 +258,31 @@ sub SaveSettings {
         next unless defined $value;
         my $setting = new Bugzilla::User::Setting($name);
 
+        if ($name eq 'api_key_only' && $user->mfa
+            && ($value eq 'off'
+                || ($value eq 'api_key_only-isdefault' && $setting->{default_value} eq 'off'))
+        ) {
+            $mfa_event = {};
+        }
+
         if ($value eq "${name}-isdefault" ) {
             if (! $settings->{$name}->{'is_default'}) {
-                $settings->{$name}->reset_to_default;
+                if ($mfa_event) {
+                    $mfa_event->{reset} = 1;
+                }
+                else {
+                    $settings->{$name}->reset_to_default;
+                }
             }
         }
         else {
             $setting->validate_value($value);
-            $settings->{$name}->set($value);
+            if ($mfa_event) {
+                $mfa_event->{set} = $value;
+            }
+            else {
+                $settings->{$name}->set($value);
+            }
         }
     }
 
@@ -214,6 +290,36 @@ sub SaveSettings {
 
     $vars->{'settings'} = $user->settings(1);
     clear_settings_cache($user->id);
+
+    if ($mfa_event) {
+        $mfa_event->{reason}   = 'Disabling API key authentication requirements';
+        $mfa_event->{postback} = {
+            action => 'userprefs.cgi',
+            fields => {
+                tab => 'settings',
+            },
+        };
+        $user->mfa_provider->verify_prompt($mfa_event);
+    }
+}
+
+sub MfaSettings {
+    my $cgi = Bugzilla->cgi;
+    my $user = Bugzilla->user;
+    return unless $user->mfa;
+
+    my $event = $user->mfa_provider->verify_check($cgi->param('mfa_token'));
+
+    my $settings = $user->settings;
+    if ($event->{reset}) {
+        $settings->{api_key_only}->reset_to_default();
+    }
+    elsif (my $value = $event->{set}) {
+        $settings->{api_key_only}->set($value);
+    }
+
+    $vars->{settings} = $user->settings(1);
+    clear_settings_cache($user->id);
 }
 
 sub DoEmail {
@@ -561,20 +667,19 @@ sub SaveMFA {
     $dbh->bz_start_transaction;
     if ($action eq 'enable') {
         $user->set_mfa($cgi->param('mfa'));
-        $user->mfa_provider->check($cgi->param('mfa_enable_code') // '');
+        $user->mfa_provider->check(Bugzilla->input_params);
         $user->mfa_provider->enrolled();
+
+        my $settings = Bugzilla->user->settings;
+        $settings->{api_key_only}->set('on');
+        clear_settings_cache(Bugzilla->user->id);
     }
     else {
-        $user->mfa_provider->check($cgi->param('mfa_disable_code') // '');
+        $user->mfa_provider->check(Bugzilla->input_params);
         $user->set_mfa('');
     }
 
     $user->update({ keep_session => 1, keep_tokens => 1 });
-
-    my $settings = Bugzilla->user->settings;
-    $settings->{api_key_only}->set('on');
-    clear_settings_cache(Bugzilla->user->id);
-
     $dbh->bz_commit_transaction;
 }
 
@@ -653,6 +758,7 @@ sub SaveApiKey {
     my $cgi = Bugzilla->cgi;
     my $dbh = Bugzilla->dbh;
     my $user = Bugzilla->user;
+    my @mfa_events;
 
     # Do it in a transaction.
     $dbh->bz_start_transaction;
@@ -661,37 +767,106 @@ sub SaveApiKey {
     my $api_keys = Bugzilla::User::APIKey->match({ user_id => $user->id });
     foreach my $api_key (@$api_keys) {
         my $description = $cgi->param('description_' . $api_key->id);
-        my $revoked = $cgi->param('revoked_' . $api_key->id);
+        my $revoked = !!$cgi->param('revoked_' . $api_key->id);
+
+        if ($description ne $api_key->description || $revoked != $api_key->revoked) {
+            if ($user->mfa && !$revoked && $api_key->revoked) {
+                push @mfa_events, {
+                    type        => 'update',
+                    reason      => 'enabling an API key',
+                    id          => $api_key->id,
+                    description => $description,
+                };
+            }
+            else {
+                $api_key->set_all({
+                    description => $description,
+                    revoked     => $revoked,
+                });
+                $api_key->update();
+            }
+        }
+    }
 
-        if ($description ne $api_key->description
-            || $revoked != $api_key->revoked)
-        {
-            $api_key->set_all({
+    # Create a new API key if requested.
+    if ($cgi->param('new_key')) {
+        my $description = $cgi->param('new_description');
+        if ($user->mfa) {
+            push @mfa_events, {
+                type        => 'create',
+                reason      => 'creating an API key',
                 description => $description,
-                revoked     => $revoked,
+            };
+        }
+        else {
+            $vars->{new_key} = _create_api_key($description);
+        }
+    }
+
+    $dbh->bz_commit_transaction;
+
+    if (@mfa_events) {
+        # build the fields for the postback
+        my $mfa_event = {
+            postback => {
+                action => 'userprefs.cgi',
+                fields => {
+                    tab => 'apikey',
+                },
+            },
+            reason => ucfirst(join(' and ', map { $_->{reason} } @mfa_events)),
+            actions => \@mfa_events,
+        };
+        # display 2fa verification
+        $user->mfa_provider->verify_prompt($mfa_event);
+    }
+}
+
+sub MfaApiKey {
+    my $cgi = Bugzilla->cgi;
+    my $user = Bugzilla->user;
+    my $dbh = Bugzilla->dbh;
+    return unless $user->mfa;
+
+    my $event = $user->mfa_provider->verify_check($cgi->param('mfa_token'));
+
+    foreach my $action (@{ $event->{actions} }) {
+        if ($action->{type} eq 'create') {
+            $vars->{new_key} = _create_api_key($action->{description});
+        }
+
+        elsif ($action->{type} eq 'update') {
+            $dbh->bz_start_transaction;
+            my $api_key = Bugzilla::User::APIKey->check({ id => $action->{id} });
+            $api_key->set_all({
+                description => $action->{description},
+                revoked     => 0,
             });
             $api_key->update();
+            $dbh->bz_commit_transaction;
         }
     }
+}
 
-    # Create a new API key if requested.
-    if ($cgi->param('new_key')) {
-        $vars->{new_key} = Bugzilla::User::APIKey->create({
-            user_id     => $user->id,
-            description => scalar $cgi->param('new_description'),
-        });
+sub _create_api_key {
+    my ($description) = @_;
+    my $user = Bugzilla->user;
 
-        # As a security precaution, we always sent out an e-mail when
-        # an API key is created
-        my $template = Bugzilla->template_inner($user->setting('lang'));
-        my $message;
-        $template->process('email/new-api-key.txt.tmpl', $vars, \$message)
-          || ThrowTemplateError($template->error());
+    my $key = Bugzilla::User::APIKey->create({
+        user_id     => $user->id,
+        description => $description,
+    });
 
-        MessageToMTA($message);
-    }
+    # As a security precaution, we always sent out an e-mail when
+    # an API key is created
+    my $template = Bugzilla->template_inner($user->setting('lang'));
+    my $message;
+    $template->process('email/new-api-key.txt.tmpl', $vars, \$message)
+        || ThrowTemplateError($template->error());
 
-    $dbh->bz_commit_transaction;
+    MessageToMTA($message);
+
+    return $key;
 }
 
 ###############################################################################
@@ -715,8 +890,9 @@ if (!Bugzilla->user->id) {
 Bugzilla->login(LOGIN_REQUIRED);
 
 my $save_changes = $cgi->param('dosave');
-$vars->{'changes_saved'} = $save_changes;
 my $disable_account = $cgi->param('account_disable');
+my $mfa_token = $cgi->param('mfa_token');
+$vars->{'changes_saved'} = $save_changes || $mfa_token;
 
 my $current_tab_name = $cgi->param('tab') || "account";
 
@@ -741,12 +917,14 @@ SWITCH: for ($current_tab_name) {
     last SWITCH if $handled;
 
     /^account$/ && do {
+        MfaAccount() if $mfa_token;
         DisableAccount() if $disable_account;
         SaveAccount() if $save_changes;
         DoAccount();
         last SWITCH;
     };
     /^settings$/ && do {
+        MfaSettings() if $mfa_token;
         SaveSettings() if $save_changes;
         DoSettings();
         last SWITCH;
@@ -766,6 +944,7 @@ SWITCH: for ($current_tab_name) {
         last SWITCH;
     };
     /^apikey$/ && do {
+        MfaApiKey() if $mfa_token;
         SaveApiKey() if $save_changes;
         DoApiKey();
         last SWITCH;