From e313190fdb186e1bc04bdb99eef40d365c23a560 Mon Sep 17 00:00:00 2001 From: Krzysztof Kotlarek Date: Thu, 8 Dec 2022 09:41:22 +1100 Subject: [PATCH] FEATURE: better UI to manage 2fa (#19338) In this PR, we introduced an option, that when all authenticators are disabled, but backup codes still exists, user can authenticate with those backup codes. This was reverted as this is not expected behavior. https://github.com/discourse/discourse/pull/18982 Instead, when the last authenticator is deleted, backup codes should be deleted as well. Because this disables 2fa, user is asked to confirm that action by typing text. In addition, UI for 2fa preferences was refreshed. --- .../addon/components/dialog-holder.hbs | 13 ++- .../dialog-holder/addon/services/dialog.js | 22 ++++ .../app/components/second-factor-form.js | 8 +- .../discourse/app/controllers/login.js | 18 +-- .../controllers/preferences/second-factor.js | 109 ++++++++++++++++++ .../controllers/second-factor-backup-edit.js | 24 ---- .../app/controllers/second-factor-edit.js | 24 ---- .../discourse/app/templates/modal/login.hbs | 2 +- .../modal/second-factor-backup-edit.hbs | 1 - .../templates/modal/second-factor-edit.hbs | 1 - .../templates/preferences-second-factor.hbs | 74 +++++++----- .../acceptance/second-factor-auth-test.js | 14 --- ...r-preferences-second-factor-backup-test.js | 14 +++ .../user-preferences-second-factor-test.js | 32 +++++ .../components/dialog-holder-test.js | 21 +++- app/assets/stylesheets/common/base/user.scss | 13 +++ app/assets/stylesheets/mobile/user.scss | 10 ++ app/models/concerns/second_factor_manager.rb | 2 +- .../admin_detailed_user_serializer.rb | 2 +- app/serializers/current_user_serializer.rb | 2 +- app/serializers/user_serializer.rb | 2 +- config/locales/client.en.yml | 9 +- .../admin_user_list_serializer_spec.rb | 12 -- .../current_user_serializer_spec.rb | 10 -- spec/serializers/user_serializer_spec.rb | 10 -- 25 files changed, 297 insertions(+), 152 deletions(-) diff --git a/app/assets/javascripts/dialog-holder/addon/components/dialog-holder.hbs b/app/assets/javascripts/dialog-holder/addon/components/dialog-holder.hbs index 7a1729f5b1..457b9b2652 100644 --- a/app/assets/javascripts/dialog-holder/addon/components/dialog-holder.hbs +++ b/app/assets/javascripts/dialog-holder/addon/components/dialog-holder.hbs @@ -10,9 +10,14 @@ {{/if}} - {{#if this.dialog.message}} + {{#if (or this.dialog.message this.dialog.confirmPhrase)}}
- {{this.dialog.message}} + {{#if this.dialog.message}} +

{{this.dialog.message}}

+ {{/if}} + {{#if this.dialog.confirmPhrase}} + + {{/if}}
{{/if}} @@ -21,9 +26,9 @@ {{#each this.dialog.buttons as |button|}} {{else}} - + {{#if this.dialog.shouldDisplayCancel}} - + {{/if}} {{/each}} diff --git a/app/assets/javascripts/dialog-holder/addon/services/dialog.js b/app/assets/javascripts/dialog-holder/addon/services/dialog.js index 824e6e02d3..3fed408075 100644 --- a/app/assets/javascripts/dialog-holder/addon/services/dialog.js +++ b/app/assets/javascripts/dialog-holder/addon/services/dialog.js @@ -1,6 +1,7 @@ import Service from "@ember/service"; import A11yDialog from "a11y-dialog"; import { bind } from "discourse-common/utils/decorators"; +import { isBlank } from "@ember/utils"; export default Service.extend({ message: null, @@ -13,7 +14,10 @@ export default Service.extend({ confirmButtonIcon: null, confirmButtonLabel: null, confirmButtonClass: null, + confirmPhrase: null, + confirmPhraseInput: null, cancelButtonLabel: null, + cancelButtonClass: null, shouldDisplayCancel: null, didConfirm: null, @@ -32,6 +36,8 @@ export default Service.extend({ confirmButtonLabel = "ok_value", confirmButtonClass = "btn-primary", cancelButtonLabel = "cancel_value", + cancelButtonClass = "btn-default", + confirmPhrase, shouldDisplayCancel, didConfirm, @@ -39,6 +45,8 @@ export default Service.extend({ buttons, } = params; + let confirmButtonDisabled = !isBlank(confirmPhrase); + const element = document.getElementById("dialog-holder"); this.setProperties({ @@ -49,10 +57,13 @@ export default Service.extend({ title, titleElementId: title !== null ? "dialog-title" : null, + confirmButtonDisabled, confirmButtonClass, confirmButtonLabel, confirmButtonIcon, + confirmPhrase, cancelButtonLabel, + cancelButtonClass, shouldDisplayCancel, didConfirm, @@ -131,7 +142,10 @@ export default Service.extend({ confirmButtonLabel: null, confirmButtonIcon: null, cancelButtonLabel: null, + cancelButtonClass: null, shouldDisplayCancel: null, + confirmPhrase: null, + confirmPhraseInput: null, didConfirm: null, didCancel: null, @@ -160,4 +174,12 @@ export default Service.extend({ cancel() { this.dialogInstance.hide(); }, + + @bind + onConfirmPhraseInput() { + this.set( + "confirmButtonDisabled", + this.confirmPhrase && this.confirmPhraseInput !== this.confirmPhrase + ); + }, }); diff --git a/app/assets/javascripts/discourse/app/components/second-factor-form.js b/app/assets/javascripts/discourse/app/components/second-factor-form.js index 9b23ed64ac..e208a5f314 100644 --- a/app/assets/javascripts/discourse/app/components/second-factor-form.js +++ b/app/assets/javascripts/discourse/app/components/second-factor-form.js @@ -42,12 +42,10 @@ export default Component.extend({ } }, - @discourseComputed("backupEnabled", "totpEnabled", "secondFactorMethod") - showToggleMethodLink(backupEnabled, totpEnabled, secondFactorMethod) { + @discourseComputed("backupEnabled", "secondFactorMethod") + showToggleMethodLink(backupEnabled, secondFactorMethod) { return ( - backupEnabled && - totpEnabled && - secondFactorMethod !== SECOND_FACTOR_METHODS.SECURITY_KEY + backupEnabled && secondFactorMethod !== SECOND_FACTOR_METHODS.SECURITY_KEY ); }, diff --git a/app/assets/javascripts/discourse/app/controllers/login.js b/app/assets/javascripts/discourse/app/controllers/login.js index 482509d68a..de5238a41d 100644 --- a/app/assets/javascripts/discourse/app/controllers/login.js +++ b/app/assets/javascripts/discourse/app/controllers/login.js @@ -223,30 +223,22 @@ export default Controller.extend(ModalFunctionality, { this.clearFlash(); if ( - (result.security_key_enabled || - result.totp_enabled || - result.backup_enabled) && + (result.security_key_enabled || result.totp_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, totpEnabled: result.totp_enabled, - showSecondFactor: result.totp_enabled || result.backup_enabled, + showSecondFactor: result.totp_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 diff --git a/app/assets/javascripts/discourse/app/controllers/preferences/second-factor.js b/app/assets/javascripts/discourse/app/controllers/preferences/second-factor.js index 91f77db8a0..257c99e1c9 100644 --- a/app/assets/javascripts/discourse/app/controllers/preferences/second-factor.js +++ b/app/assets/javascripts/discourse/app/controllers/preferences/second-factor.js @@ -10,6 +10,7 @@ import { findAll } from "discourse/models/login-method"; import { popupAjaxError } from "discourse/lib/ajax-error"; import showModal from "discourse/lib/show-modal"; import { inject as service } from "@ember/service"; +import { htmlSafe } from "@ember/template"; export default Controller.extend(CanCheckEmails, { dialog: service(), @@ -113,6 +114,29 @@ export default Controller.extend(CanCheckEmails, { .finally(() => this.set("resetPasswordLoading", false)); }, + disableAllMessage() { + let templateElements = [I18n.t("user.second_factor.delete_confirm_header")]; + templateElements.push("
    "); + this.totps.forEach((totp) => { + templateElements.push(`
  • ${totp.name}
    • `); + }); + this.security_keys.forEach((key) => { + templateElements.push(`
  • ${key.name}
    • `); + }); + if (this.currentUser.second_factor_backup_enabled) { + templateElements.push( + `
  • ${I18n.t("user.second_factor_backup.title")}
    • ` + ); + } + templateElements.push("
"); + templateElements.push( + I18n.t("user.second_factor.delete_confirm_instruction", { + confirm: I18n.t("user.second_factor.disable"), + }) + ); + return htmlSafe(templateElements.join("")); + }, + actions: { confirmPassword() { if (!this.password) { @@ -130,8 +154,11 @@ export default Controller.extend(CanCheckEmails, { this.dialog.deleteConfirm({ title: I18n.t("user.second_factor.disable_confirm"), + message: this.disableAllMessage(), confirmButtonLabel: "user.second_factor.disable", + confirmPhrase: I18n.t("user.second_factor.disable"), confirmButtonIcon: "ban", + cancelButtonClass: "btn-flat", didConfirm: () => { this.model .disableAllSecondFactors() @@ -144,6 +171,88 @@ export default Controller.extend(CanCheckEmails, { }, }); }, + disableSingleSecondFactor(secondFactorMethod) { + if (this.totps.concat(this.security_keys).length === 1) { + this.send("disableAllSecondFactors"); + return; + } + this.dialog.deleteConfirm({ + title: I18n.t("user.second_factor.delete_single_confirm_title"), + message: I18n.t("user.second_factor.delete_single_confirm_message", { + name: secondFactorMethod.name, + }), + confirmButtonLabel: "user.second_factor.delete", + confirmButtonIcon: "ban", + cancelButtonClass: "btn-flat", + didConfirm: () => { + this.currentUser + .updateSecondFactor( + secondFactorMethod.id, + secondFactorMethod.name, + true, + secondFactorMethod.method + ) + .then((response) => { + if (response.error) { + return; + } + this.markDirty(); + }) + .catch((e) => this.handleError(e)) + .finally(() => { + this.setProperties({ + totps: this.totps.filter( + (totp) => + totp.id !== secondFactorMethod.id || + totp.method !== secondFactorMethod.method + ), + security_keys: this.security_keys.filter( + (key) => + key.id !== secondFactorMethod.id || + key.method !== secondFactorMethod.method + ), + }); + + this.set("loading", false); + }); + }, + }); + }, + + disableSecondFactorBackup() { + this.dialog.deleteConfirm({ + title: I18n.t("user.second_factor.delete_backup_codes_confirm_title"), + message: I18n.t( + "user.second_factor.delete_backup_codes_confirm_message" + ), + confirmButtonLabel: "user.second_factor.delete", + confirmButtonIcon: "ban", + cancelButtonClass: "btn-flat", + didConfirm: () => { + this.set("backupCodes", []); + this.set("loading", true); + + this.model + .updateSecondFactor(0, "", true, SECOND_FACTOR_METHODS.BACKUP_CODE) + .then((response) => { + if (response.error) { + this.set("errorMessage", response.error); + return; + } + + this.set("errorMessage", null); + this.model.set("second_factor_backup_enabled", false); + this.markDirty(); + this.send("closeModal"); + }) + .catch((error) => { + this.send("closeModal"); + this.onError(error); + }) + .finally(() => this.set("loading", false)); + }, + }); + }, createTotp() { const controller = showModal("second-factor-add-totp", { diff --git a/app/assets/javascripts/discourse/app/controllers/second-factor-backup-edit.js b/app/assets/javascripts/discourse/app/controllers/second-factor-backup-edit.js index 7219d39d72..6854102fe0 100644 --- a/app/assets/javascripts/discourse/app/controllers/second-factor-backup-edit.js +++ b/app/assets/javascripts/discourse/app/controllers/second-factor-backup-edit.js @@ -40,30 +40,6 @@ export default Controller.extend(ModalFunctionality, { this._hideCopyMessage(); }, - disableSecondFactorBackup() { - this.set("backupCodes", []); - this.set("loading", true); - - this.model - .updateSecondFactor(0, "", true, SECOND_FACTOR_METHODS.BACKUP_CODE) - .then((response) => { - if (response.error) { - this.set("errorMessage", response.error); - return; - } - - this.set("errorMessage", null); - this.model.set("second_factor_backup_enabled", false); - this.markDirty(); - this.send("closeModal"); - }) - .catch((error) => { - this.send("closeModal"); - this.onError(error); - }) - .finally(() => this.set("loading", false)); - }, - generateSecondFactorCodes() { this.set("loading", true); this.model diff --git a/app/assets/javascripts/discourse/app/controllers/second-factor-edit.js b/app/assets/javascripts/discourse/app/controllers/second-factor-edit.js index 2f0726ad6c..88eaedd645 100644 --- a/app/assets/javascripts/discourse/app/controllers/second-factor-edit.js +++ b/app/assets/javascripts/discourse/app/controllers/second-factor-edit.js @@ -3,30 +3,6 @@ import ModalFunctionality from "discourse/mixins/modal-functionality"; export default Controller.extend(ModalFunctionality, { actions: { - disableSecondFactor() { - this.user - .updateSecondFactor( - this.model.id, - this.model.name, - true, - this.model.method - ) - .then((response) => { - if (response.error) { - return; - } - this.markDirty(); - }) - .catch((error) => { - this.send("closeModal"); - this.onError(error); - }) - .finally(() => { - this.set("loading", false); - this.send("closeModal"); - }); - }, - editSecondFactor() { this.user .updateSecondFactor( diff --git a/app/assets/javascripts/discourse/app/templates/modal/login.hbs b/app/assets/javascripts/discourse/app/templates/modal/login.hbs index 39de9f023e..d7465134af 100644 --- a/app/assets/javascripts/discourse/app/templates/modal/login.hbs +++ b/app/assets/javascripts/discourse/app/templates/modal/login.hbs @@ -26,7 +26,7 @@
{{d-icon "exclamation-triangle"}} {{i18n "login.caps_lock_warning"}}
- + {{#if this.showSecurityKey}} diff --git a/app/assets/javascripts/discourse/app/templates/modal/second-factor-backup-edit.hbs b/app/assets/javascripts/discourse/app/templates/modal/second-factor-backup-edit.hbs index 0316c260f5..e9cbe827a6 100644 --- a/app/assets/javascripts/discourse/app/templates/modal/second-factor-backup-edit.hbs +++ b/app/assets/javascripts/discourse/app/templates/modal/second-factor-backup-edit.hbs @@ -20,7 +20,6 @@
{{#if this.backupEnabled}} - {{else}} {{/if}} diff --git a/app/assets/javascripts/discourse/app/templates/modal/second-factor-edit.hbs b/app/assets/javascripts/discourse/app/templates/modal/second-factor-edit.hbs index 63d3980b1b..e544da78d4 100644 --- a/app/assets/javascripts/discourse/app/templates/modal/second-factor-edit.hbs +++ b/app/assets/javascripts/discourse/app/templates/modal/second-factor-edit.hbs @@ -9,5 +9,4 @@
-
diff --git a/app/assets/javascripts/discourse/app/templates/preferences-second-factor.hbs b/app/assets/javascripts/discourse/app/templates/preferences-second-factor.hbs index 166a2c81d9..4e17f0c92a 100644 --- a/app/assets/javascripts/discourse/app/templates/preferences-second-factor.hbs +++ b/app/assets/javascripts/discourse/app/templates/preferences-second-factor.hbs @@ -30,59 +30,79 @@

{{i18n "user.second_factor.totp.title"}}

- {{#each this.totps as |totp|}} -
- {{#if totp.name}} - {{totp.name}} - {{else}} - {{i18n "user.second_factor.totp.default_name"}} - {{/if}} +
+
+ {{#if totp.name}} + {{totp.name}} + {{else}} + {{i18n "user.second_factor.totp.default_name"}} + {{/if}} +
{{#if this.isCurrentUser}} - +
+ + +
{{/if}}
{{/each}} +

{{i18n "user.second_factor.security_key.title"}}

- {{#each this.security_keys as |security_key|}} -
- {{#if security_key.name}} - {{security_key.name}} - {{else}} - {{i18n "user.second_factor.security_key.default_name"}} - {{/if}} +
+
+ {{#if security_key.name}} + {{security_key.name}} + {{else}} + {{i18n "user.second_factor.security_key.default_name"}} + {{/if}} +
{{#if this.isCurrentUser}} - +
+ + +
{{/if}}
{{/each}} +

{{i18n "user.second_factor_backup.title"}}

- {{#if this.model.second_factor_enabled}} - {{#if this.model.second_factor_backup_enabled}} - {{html-safe (i18n "user.second_factor_backup.manage" count=this.model.second_factor_remaining_backup_codes)}} - {{else}} - {{i18n "user.second_factor_backup.enable_long"}} - {{/if}} +
+ {{#if this.model.second_factor_enabled}} +
+ {{#if this.model.second_factor_backup_enabled}} + {{html-safe (i18n "user.second_factor_backup.manage" count=this.model.second_factor_remaining_backup_codes)}} + {{else}} + {{i18n "user.second_factor_backup.enable_long"}} + {{/if}} +
- {{#if this.isCurrentUser}} - + {{#if this.isCurrentUser}} +
+ + + {{#if this.model.second_factor_backup_enabled}} + + {{/if}} +
+ {{/if}} + {{else}} + {{i18n "user.second_factor_backup.enable_prerequisites"}} {{/if}} - {{else}} - {{i18n "user.second_factor_backup.enable_prerequisites"}} - {{/if}} +
diff --git a/app/assets/javascripts/discourse/tests/acceptance/second-factor-auth-test.js b/app/assets/javascripts/discourse/tests/acceptance/second-factor-auth-test.js index 0d8c4cc0c6..32b57a7e6d 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/second-factor-auth-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/second-factor-auth-test.js @@ -44,12 +44,6 @@ 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) => { @@ -184,14 +178,6 @@ 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) { diff --git a/app/assets/javascripts/discourse/tests/acceptance/user-preferences-second-factor-backup-test.js b/app/assets/javascripts/discourse/tests/acceptance/user-preferences-second-factor-backup-test.js index 218823c676..ebd73c0bc4 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/user-preferences-second-factor-backup-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/user-preferences-second-factor-backup-test.js @@ -3,6 +3,7 @@ import { click, visit } from "@ember/test-helpers"; import { acceptance, exists, + query, updateCurrentUser, } from "discourse/tests/helpers/qunit-helpers"; @@ -42,4 +43,17 @@ acceptance("User Preferences - Second Factor Backup", function (needs) { assert.ok(exists(".backup-codes-area"), "shows backup codes"); }); + + test("delete backup codes", async function (assert) { + updateCurrentUser({ second_factor_enabled: true }); + await visit("/u/eviltrout/preferences/second-factor"); + await click(".edit-2fa-backup"); + await click(".second-factor-backup-preferences .btn-primary"); + await click(".modal-close"); + await click(".pref-second-factor-backup .btn-danger"); + assert.strictEqual( + query("#dialog-title").innerText.trim(), + "Deleting backup codes" + ); + }); }); diff --git a/app/assets/javascripts/discourse/tests/acceptance/user-preferences-second-factor-test.js b/app/assets/javascripts/discourse/tests/acceptance/user-preferences-second-factor-test.js index d5b9a2630e..6f8a1dc810 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/user-preferences-second-factor-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/user-preferences-second-factor-test.js @@ -4,6 +4,7 @@ import { acceptance, exists, query, + updateCurrentUser, } from "discourse/tests/helpers/qunit-helpers"; acceptance("User Preferences - Second Factor", function (needs) { @@ -14,6 +15,8 @@ acceptance("User Preferences - Second Factor", function (needs) { return helper.response({ success: "OK", password_required: "true", + totps: [{ id: 1, name: "one of them" }], + security_keys: [{ id: 2, name: "key" }], }); }); @@ -90,4 +93,33 @@ acceptance("User Preferences - Second Factor", function (needs) { ); } }); + + test("delete second factor security method", async function (assert) { + updateCurrentUser({ moderator: false, admin: false, trust_level: 1 }); + await visit("/u/eviltrout/preferences/second-factor"); + + assert.ok(exists("#password"), "it has a password input"); + + await fillIn("#password", "secrets"); + await click(".user-preferences .btn-primary"); + await click(".totp .btn-danger"); + assert.strictEqual( + query("#dialog-title").innerText.trim(), + "Deleting an authenticator" + ); + await click(".dialog-close"); + + await click(".security-key .btn-danger"); + assert.strictEqual( + query("#dialog-title").innerText.trim(), + "Deleting an authenticator" + ); + await click(".dialog-close"); + + await click(".btn-danger.btn-icon-text"); + assert.strictEqual( + query("#dialog-title").innerText.trim(), + "Are you sure you want to disable two-factor authentication?" + ); + }); }); diff --git a/app/assets/javascripts/discourse/tests/integration/components/dialog-holder-test.js b/app/assets/javascripts/discourse/tests/integration/components/dialog-holder-test.js index 251e8ac4e2..a1ed2606e0 100644 --- a/app/assets/javascripts/discourse/tests/integration/components/dialog-holder-test.js +++ b/app/assets/javascripts/discourse/tests/integration/components/dialog-holder-test.js @@ -1,7 +1,13 @@ import I18n from "I18n"; import { module, test } from "qunit"; import { setupRenderingTest } from "discourse/tests/helpers/component-test"; -import { click, render, settled, triggerKeyEvent } from "@ember/test-helpers"; +import { + click, + fillIn, + render, + settled, + triggerKeyEvent, +} from "@ember/test-helpers"; import { hbs } from "ember-cli-htmlbars"; import { query } from "discourse/tests/helpers/qunit-helpers"; @@ -388,4 +394,17 @@ module("Integration | Component | dialog-holder", function (hooks) { ".btn-primary element is not present in the dialog" ); }); + test("delete confirm with confirmation phase", async function (assert) { + await render(hbs``); + + this.dialog.deleteConfirm({ + message: "A delete confirm message", + confirmPhrase: "test", + }); + await settled(); + + assert.strictEqual(query(".btn-danger").disabled, true); + await fillIn("#confirm-phrase", "test"); + assert.strictEqual(query(".btn-danger").disabled, false); + }); }); diff --git a/app/assets/stylesheets/common/base/user.scss b/app/assets/stylesheets/common/base/user.scss index 2c4399189a..9f0b5fa8fb 100644 --- a/app/assets/stylesheets/common/base/user.scss +++ b/app/assets/stylesheets/common/base/user.scss @@ -758,6 +758,19 @@ } .second-factor-item { margin-top: 0.75em; + width: 500px; + display: flex; + justify-content: space-between; + border-bottom: 1px solid #ddd; + margin: 5px 0px; + align-items: center; + + &:last-child { + border-bottom: 0; + } + .btn-danger .d-icon { + color: var(--danger); + } } .btn.edit { min-height: auto; diff --git a/app/assets/stylesheets/mobile/user.scss b/app/assets/stylesheets/mobile/user.scss index 2665b2eac0..f55a1ade01 100644 --- a/app/assets/stylesheets/mobile/user.scss +++ b/app/assets/stylesheets/mobile/user.scss @@ -433,3 +433,13 @@ margin-top: 1em; } } +.second-factor { + .second-factor-item { + width: auto; + display: flex; + justify-content: space-between; + } + .details { + width: 75%; + } +} diff --git a/app/models/concerns/second_factor_manager.rb b/app/models/concerns/second_factor_manager.rb index 4e212306af..e21957e75f 100644 --- a/app/models/concerns/second_factor_manager.rb +++ b/app/models/concerns/second_factor_manager.rb @@ -79,7 +79,7 @@ module SecondFactorManager end def has_any_second_factor_methods_enabled? - totp_enabled? || security_keys_enabled? || backup_codes_enabled? + totp_enabled? || security_keys_enabled? end def has_multiple_second_factor_methods? diff --git a/app/serializers/admin_detailed_user_serializer.rb b/app/serializers/admin_detailed_user_serializer.rb index 454fcf6cbd..aab939247c 100644 --- a/app/serializers/admin_detailed_user_serializer.rb +++ b/app/serializers/admin_detailed_user_serializer.rb @@ -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.backup_codes_enabled? + object.totp_enabled? || object.security_keys_enabled? end def can_disable_second_factor diff --git a/app/serializers/current_user_serializer.rb b/app/serializers/current_user_serializer.rb index e620cb711d..59c01074d3 100644 --- a/app/serializers/current_user_serializer.rb +++ b/app/serializers/current_user_serializer.rb @@ -238,7 +238,7 @@ class CurrentUserSerializer < BasicUserSerializer end def second_factor_enabled - object.totp_enabled? || object.security_keys_enabled? || object.backup_codes_enabled? + object.totp_enabled? || object.security_keys_enabled? end def featured_topic diff --git a/app/serializers/user_serializer.rb b/app/serializers/user_serializer.rb index eb2db4e56e..0e9f975032 100644 --- a/app/serializers/user_serializer.rb +++ b/app/serializers/user_serializer.rb @@ -105,7 +105,7 @@ class UserSerializer < UserCardSerializer end def second_factor_enabled - object.totp_enabled? || object.security_keys_enabled? || object.backup_codes_enabled? + object.totp_enabled? || object.security_keys_enabled? end def include_second_factor_backup_enabled? diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 4e632641b1..35997ff14a 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -1392,7 +1392,14 @@ en: use: "Use Authenticator app" enforced_notice: "You are required to enable two-factor authentication before accessing this site." disable: "Disable" - disable_confirm: "Are you sure you want to disable all two-factor methods?" + disable_confirm: "Are you sure you want to disable two-factor authentication?" + delete: "Delete" + delete_confirm_header: "These Token-Based Authenticators and Physical Security Keys will be deleted:" + delete_confirm_instruction: 'To confirm, type %{confirm} in the box below.' + delete_single_confirm_title: "Deleting an authenticator" + delete_single_confirm_message: "You are deleting %{name}. You can't undo this action. If you change your mind, you have to register this authenticator again." + delete_backup_codes_confirm_title: "Deleting backup codes" + delete_backup_codes_confirm_message: "You are deleting backup codes. You can't undo this action. If you change your mind, you have to regenerate backup codes." save: "Save" edit: "Edit" edit_title: "Edit Authenticator" diff --git a/spec/serializers/admin_user_list_serializer_spec.rb b/spec/serializers/admin_user_list_serializer_spec.rb index 2c1e3ebadb..955e726719 100644 --- a/spec/serializers/admin_user_list_serializer_spec.rb +++ b/spec/serializers/admin_user_list_serializer_spec.rb @@ -31,18 +31,6 @@ 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") } diff --git a/spec/serializers/current_user_serializer_spec.rb b/spec/serializers/current_user_serializer_spec.rb index cc6a04290d..8f948a1e50 100644 --- a/spec/serializers/current_user_serializer_spec.rb +++ b/spec/serializers/current_user_serializer_spec.rb @@ -102,16 +102,6 @@ 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 diff --git a/spec/serializers/user_serializer_spec.rb b/spec/serializers/user_serializer_spec.rb index beb8637d24..0576545325 100644 --- a/spec/serializers/user_serializer_spec.rb +++ b/spec/serializers/user_serializer_spec.rb @@ -250,16 +250,6 @@ 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