FIX: do not lock account if backup codes are available (#18982)

Currently, we have available three 2fa methods:
- Token-Based Authenticators
- Physical Security Keys
- Two-Factor Backup Codes

If the first two are deleted, user lose visibility of their backup codes, which suggests that 2fa is disabled.

However, when they try to authenticate, the account is locked, and they have to ask admin to fix that problem.

This PR is fixing the issue. User still sees backup codes in their panel and can use them to authenticate.

In next PR, I will improve UI to clearly notify the user when 2fa is fully disabled and when it is still active.
This commit is contained in:
Krzysztof Kotlarek 2022-11-11 13:00:06 +11:00 committed by GitHub
parent 4692f4ee7c
commit 4db5525d25
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
11 changed files with 70 additions and 13 deletions

View File

@ -42,10 +42,12 @@ export default Component.extend({
}
},
@discourseComputed("backupEnabled", "secondFactorMethod")
showToggleMethodLink(backupEnabled, secondFactorMethod) {
@discourseComputed("backupEnabled", "totpEnabled", "secondFactorMethod")
showToggleMethodLink(backupEnabled, totpEnabled, secondFactorMethod) {
return (
backupEnabled && secondFactorMethod !== SECOND_FACTOR_METHODS.SECURITY_KEY
backupEnabled &&
totpEnabled &&
secondFactorMethod !== SECOND_FACTOR_METHODS.SECURITY_KEY
);
},

View File

@ -223,21 +223,30 @@ export default Controller.extend(ModalFunctionality, {
this.clearFlash();
if (
(result.security_key_enabled || result.totp_enabled) &&
(result.security_key_enabled ||
result.totp_enabled ||
result.backup_enabled) &&
!this.secondFactorRequired
) {
let secondFactorMethod;
if (result.security_key_enabled) {
secondFactorMethod = SECOND_FACTOR_METHODS.SECURITY_KEY;
} else if (result.totp_enabled) {
secondFactorMethod = SECOND_FACTOR_METHODS.TOTP;
} else {
secondFactorMethod = SECOND_FACTOR_METHODS.BACKUP_CODE;
}
this.setProperties({
otherMethodAllowed: result.multiple_second_factor_methods,
secondFactorRequired: true,
showLoginButtons: false,
backupEnabled: result.backup_enabled,
showSecondFactor: result.totp_enabled,
totpEnabled: result.totp_enabled,
showSecondFactor: result.totp_enabled || result.backup_enabled,
showSecurityKey: result.security_key_enabled,
secondFactorMethod: result.security_key_enabled
? SECOND_FACTOR_METHODS.SECURITY_KEY
: SECOND_FACTOR_METHODS.TOTP,
securityKeyChallenge: result.challenge,
securityKeyAllowedCredentialIds: result.allowed_credential_ids,
secondFactorMethod,
});
// only need to focus the 2FA input for TOTP

View File

@ -26,7 +26,7 @@
<div class="caps-lock-warning {{unless this.capsLockOn "hidden"}}">{{d-icon "exclamation-triangle"}} {{i18n "login.caps_lock_warning"}}</div>
</div>
</div>
<SecondFactorForm @secondFactorMethod={{this.secondFactorMethod}} @secondFactorToken={{this.secondFactorToken}} @class={{this.secondFactorClass}} @backupEnabled={{this.backupEnabled}} @isLogin={{true}}>
<SecondFactorForm @secondFactorMethod={{this.secondFactorMethod}} @secondFactorToken={{this.secondFactorToken}} @class={{this.secondFactorClass}} @backupEnabled={{this.backupEnabled}} @totpEnabled={{this.totpEnabled}} @isLogin={{true}}>
{{#if this.showSecurityKey}}
<SecurityKeyForm @allowedCredentialIds={{this.securityKeyAllowedCredentialIds}} @challenge={{this.securityKeyChallenge}} @showSecurityKey={{this.showSecurityKey}} @showSecondFactor={{this.showSecondFactor}} @secondFactorMethod={{this.secondFactorMethod}} @otherMethodAllowed={{this.otherMethodAllowed}} @action={{action "authenticateSecurityKey"}}>
</SecurityKeyForm>

View File

@ -44,6 +44,12 @@ const RESPONSES = {
security_keys_enabled: true,
allowed_methods: [BACKUP_CODE],
},
ok010010: {
totp_enabled: false,
backup_enabled: true,
security_keys_enabled: false,
allowed_methods: [BACKUP_CODE],
},
};
Object.keys(RESPONSES).forEach((k) => {
@ -178,6 +184,14 @@ acceptance("Second Factor Auth Page", function (needs) {
!exists(".toggle-second-factor-method"),
"no alternative methods are shown if only 1 method is allowed"
);
// only backup codes
await visit("/session/2fa?nonce=ok010010");
assert.ok(exists("form.backup-code-token"), "backup code form is shown");
assert.ok(
!exists(".toggle-second-factor-method"),
"no alternative methods are shown if only 1 method is allowed"
);
});
test("switching 2FA methods", async function (assert) {

View File

@ -79,7 +79,7 @@ module SecondFactorManager
end
def has_any_second_factor_methods_enabled?
totp_enabled? || security_keys_enabled?
totp_enabled? || security_keys_enabled? || backup_codes_enabled?
end
def has_multiple_second_factor_methods?

View File

@ -45,7 +45,7 @@ class AdminDetailedUserSerializer < AdminUserSerializer
has_many :groups, embed: :object, serializer: BasicGroupSerializer
def second_factor_enabled
object.totp_enabled? || object.security_keys_enabled?
object.totp_enabled? || object.security_keys_enabled? || object.backup_codes_enabled?
end
def can_disable_second_factor

View File

@ -323,7 +323,7 @@ class CurrentUserSerializer < BasicUserSerializer
end
def second_factor_enabled
object.totp_enabled? || object.security_keys_enabled?
object.totp_enabled? || object.security_keys_enabled? || object.backup_codes_enabled?
end
def featured_topic

View File

@ -105,7 +105,7 @@ class UserSerializer < UserCardSerializer
end
def second_factor_enabled
object.totp_enabled? || object.security_keys_enabled?
object.totp_enabled? || object.security_keys_enabled? || object.backup_codes_enabled?
end
def include_second_factor_backup_enabled?

View File

@ -31,6 +31,18 @@ RSpec.describe AdminUserListSerializer do
end
end
context "when backup codes enabled" do
before do
Fabricate(:user_second_factor_backup, user: user)
end
it "is true" do
json = serializer.as_json
expect(json[:second_factor_enabled]).to eq(true)
end
end
describe "emails" do
fab!(:admin) { Fabricate(:user, admin: true, email: "admin@email.com") }
fab!(:moderator) { Fabricate(:user, moderator: true, email: "moderator@email.com") }

View File

@ -102,6 +102,16 @@ RSpec.describe CurrentUserSerializer do
expect(json[:second_factor_enabled]).to eq(true)
end
end
context "when backup codes enabled" do
before do
User.any_instance.stubs(:backup_codes_enabled?).returns(true)
end
it "is true" do
expect(json[:second_factor_enabled]).to eq(true)
end
end
end
describe "#groups" do

View File

@ -250,6 +250,16 @@ RSpec.describe UserSerializer do
expect(json[:second_factor_enabled]).to eq(true)
end
end
context "when backup codes enabled" do
before do
User.any_instance.stubs(:backup_codes_enabled?).returns(true)
end
it "is true" do
expect(json[:second_factor_enabled]).to eq(true)
end
end
end
describe "ignored and muted" do