diff --git a/Bugzilla/MFA.pm b/Bugzilla/MFA.pm index 2de52428ce..b3c76c3be9 100644 --- a/Bugzilla/MFA.pm +++ b/Bugzilla/MFA.pm @@ -108,7 +108,7 @@ sub verify_check { foreach my $i (1 .. 10) { my $key = "recovery.$i"; if (($self->property_get($key) // '') eq $code) { - $self->property_delete($key); + $self->property_delete($key) if !$params->{no_delete}; return; } } diff --git a/Bugzilla/WebService/User.pm b/Bugzilla/WebService/User.pm index ed48f19e6b..6e4186bc81 100644 --- a/Bugzilla/WebService/User.pm +++ b/Bugzilla/WebService/User.pm @@ -538,7 +538,7 @@ sub mfa_enroll { return $provider->enroll_api(); } -# Pre-validate the users TOTP code using the mfa_verification_token +# Pre-validate the users TOTP code using the mfa_verification_token # obtained from the cookie set when loading the verfication form. sub mfa_verify_totp_code { my ($self, $params) = @_; @@ -559,7 +559,11 @@ sub mfa_verify_totp_code { # verify try { - $user->mfa_provider->verify_check({code => $params->{mfa_code}}); + # Pass no_delete=1 just in case user is using a recovery code + # Normally a recovery code is deleted once it is used and we + # do no want that to happen yet here. + $user->mfa_provider->verify_check( + {code => $params->{mfa_code}, no_delete => 1}); } catch { return {error => 1, message => 'invalid verification code'}; diff --git a/qa/t/2_test_login_mfa.t b/qa/t/2_test_login_mfa.t index 4e0ffd3f22..4af3a681b9 100644 --- a/qa/t/2_test_login_mfa.t +++ b/qa/t/2_test_login_mfa.t @@ -51,6 +51,25 @@ $sel->type_ok('mfa-totp-enable-code', $auth->code); $sel->click_ok('update'); $sel->title_is('User Preferences'); +# Generate recovery codes to use in testing +$sel->click_ok('mfa-recovery'); +$sel->type_ok('mfa-password', $config->{admin_user_passwd}); +$sel->type_ok('code', $auth->code); +$sel->click_ok('update'); + +# Hack needed view contents of TOTP iframe and return the recovery codes +my $recovery_string = $sel->driver->execute_script(' + var iframe = document.getElementById("mfa-recovery-frame").contentWindow; + var codes = iframe.document.getElementById("codes"); + return codes.innerText; +'); +my @recovery_codes; +foreach my $code (split /\n/, $recovery_string) { + $code =~ s/^\s+//; + $code =~ s/\s+$//; + push @recovery_codes, $code; +} +ok(scalar @recovery_codes == 10, 'Ten recovery codes generated properly'); logout($sel); # Log back in but this time we are asked for a TOTP code @@ -79,6 +98,52 @@ ok($error eq 'Invalid verification code.', 'Correct error generated for invalid $sel->type_ok('code', $auth->code); $sel->click_ok('//input[@value="Submit"]'); $sel->title_is('Bugzilla Main Page'); +logout($sel); + +# Try to use one of the recovery codes as a MFA code +$sel->open_ok('/login', undef, 'Go to the home page'); +$sel->title_is('Log in to Bugzilla'); +$sel->type_ok( + 'Bugzilla_login', + $config->{admin_user_login}, + 'Enter admin login name' +); +$sel->type_ok( + 'Bugzilla_password', + $config->{admin_user_passwd}, + 'Enter admin password' +); +$sel->click_ok('log_in', undef, 'Submit credentials'); +$sel->title_is('Account Verification'); +$sel->type_ok('code', $recovery_codes[0]); +$sel->click_ok('//input[@value="Submit"]'); +$sel->title_is('Bugzilla Main Page'); +logout($sel); + +# Reusing the same recovery code a second time should fail +$sel->open_ok('/login', undef, 'Go to the home page'); +$sel->title_is('Log in to Bugzilla'); +$sel->type_ok( + 'Bugzilla_login', + $config->{admin_user_login}, + 'Enter admin login name' +); +$sel->type_ok( + 'Bugzilla_password', + $config->{admin_user_passwd}, + 'Enter admin password' +); +$sel->click_ok('log_in', undef, 'Submit credentials'); +$sel->title_is('Account Verification'); +$sel->type_ok('code', $recovery_codes[0]); +$sel->click_ok('//input[@value="Submit"]'); +$error = $sel->get_text('verify-totp-error'); +ok($error eq 'Invalid verification code.', 'Correct error generated for invalid code'); + +# Now enter a different valid recovery code +$sel->type_ok('code', $recovery_codes[1]); +$sel->click_ok('//input[@value="Submit"]'); +$sel->title_is('Bugzilla Main Page'); # Disable TOTP for the admin account $sel->open_ok('/userprefs.cgi?tab=mfa');