diff options
author | Byron Jones <glob@mozilla.com> | 2015-09-23 05:54:41 +0200 |
---|---|---|
committer | Byron Jones <glob@mozilla.com> | 2015-09-23 05:54:41 +0200 |
commit | 043c7523acd6af5288191b15f746fc360b73ab40 (patch) | |
tree | 536980970ca7ae13ce29d4cf9e9f69fb0669a972 | |
parent | 2e425408eeb1065eacb4bcded2cc88d05a689e1c (diff) | |
download | bugzilla-043c7523acd6af5288191b15f746fc360b73ab40.tar.gz bugzilla-043c7523acd6af5288191b15f746fc360b73ab40.tar.xz |
Bug 1199087 - extend 2fa protection beyond login
-rw-r--r-- | Bugzilla/Auth.pm | 38 | ||||
-rw-r--r-- | Bugzilla/Constants.pm | 3 | ||||
-rw-r--r-- | Bugzilla/MFA.pm | 39 | ||||
-rw-r--r-- | Bugzilla/MFA/TOTP.pm | 27 | ||||
-rw-r--r-- | Bugzilla/Token.pm | 47 | ||||
-rw-r--r-- | images/mfa.png | bin | 0 -> 1692 bytes | |||
-rw-r--r-- | js/account.js | 106 | ||||
-rw-r--r-- | skins/standard/admin.css | 1 | ||||
-rw-r--r-- | skins/standard/global.css | 7 | ||||
-rw-r--r-- | template/en/default/account/password/set-forgotten-password.html.tmpl | 5 | ||||
-rw-r--r-- | template/en/default/account/prefs/account.html.tmpl | 2 | ||||
-rw-r--r-- | template/en/default/account/prefs/apikey.html.tmpl | 25 | ||||
-rw-r--r-- | template/en/default/account/prefs/mfa.html.tmpl | 112 | ||||
-rw-r--r-- | template/en/default/account/prefs/settings.html.tmpl | 3 | ||||
-rw-r--r-- | template/en/default/global/code-error.html.tmpl | 3 | ||||
-rw-r--r-- | template/en/default/global/user-error.html.tmpl | 9 | ||||
-rw-r--r-- | template/en/default/mfa/protected.html.tmpl | 12 | ||||
-rw-r--r-- | template/en/default/mfa/totp/verify.html.tmpl | 18 | ||||
-rwxr-xr-x | token.cgi | 68 | ||||
-rwxr-xr-x | userprefs.cgi | 261 |
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 Binary files differnew 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"> </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 %] @@ -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; |