From 5a23a74bbc07565dfedf6198c8e5460f8a09fa15 Mon Sep 17 00:00:00 2001 From: Keegan George Date: Fri, 8 Nov 2024 09:39:42 +0900 Subject: [PATCH] DEV: Show confirmation dialog when admins disable 2FA (#29652) This PR ensures that admins are shown a confirmation dialog when clicking to disable 2FA for a user. The 2FA button is right below the "Grant Badge" button and as such it can easily be clicked accidentally. It's also good practice to ask for confirmation before removing important functionality. --- .../addon/controllers/admin-user-index.js | 7 +++- .../admin/addon/templates/user-index.hbs | 4 +-- .../tests/acceptance/admin-user-index-test.js | 34 +++++++++++++++++++ config/locales/client.en.yml | 7 ++-- 4 files changed, 46 insertions(+), 6 deletions(-) diff --git a/app/assets/javascripts/admin/addon/controllers/admin-user-index.js b/app/assets/javascripts/admin/addon/controllers/admin-user-index.js index d1fac2a80d3..0eb43bd5bd8 100644 --- a/app/assets/javascripts/admin/addon/controllers/admin-user-index.js +++ b/app/assets/javascripts/admin/addon/controllers/admin-user-index.js @@ -374,7 +374,12 @@ export default class AdminUserIndexController extends Controller.extend( @action disableSecondFactor() { - return this.model.disableSecondFactor(); + this.dialog.yesNoConfirm({ + message: I18n.t("admin.user.disable_second_factor_confirm"), + didConfirm: () => { + return this.model.disableSecondFactor(); + }, + }); } @action diff --git a/app/assets/javascripts/admin/addon/templates/user-index.hbs b/app/assets/javascripts/admin/addon/templates/user-index.hbs index 6244c0f7022..b1a5ac21a9b 100644 --- a/app/assets/javascripts/admin/addon/templates/user-index.hbs +++ b/app/assets/javascripts/admin/addon/templates/user-index.hbs @@ -230,7 +230,7 @@ {{/if}} -
+
{{i18n "user.second_factor.title"}}
{{#if this.model.second_factor_enabled}} @@ -245,7 +245,7 @@ @action={{this.disableSecondFactor}} @icon="unlock-keyhole" @label="user.second_factor.disable" - class="btn-default" + class="btn-default disable-second-factor" /> {{/if}}
diff --git a/app/assets/javascripts/discourse/tests/acceptance/admin-user-index-test.js b/app/assets/javascripts/discourse/tests/acceptance/admin-user-index-test.js index 89f8fbd6ff9..20e77426647 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/admin-user-index-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/admin-user-index-test.js @@ -81,6 +81,24 @@ acceptance("Admin - User Index", function (needs) { }); }); + server.get("/admin/users/7.json", () => { + return helper.response({ + id: 7, + username: "jimmy", + name: "Jimmy Johnson", + avatar_template: "/letter_avatar_proxy/v4/letter/b/f0a364/{size}.png", + active: true, + admin: false, + moderator: false, + can_grant_admin: true, + can_revoke_admin: false, + can_grant_moderation: true, + can_revoke_moderation: false, + second_factor_enabled: true, + can_disable_second_factor: true, + }); + }); + server.put("/admin/users/4/grant_admin", () => { return helper.response(403, { second_factor_challenge_nonce: "some-nonce", @@ -140,6 +158,10 @@ acceptance("Admin - User Index", function (needs) { html_message: true, }); }); + + server.put("/admin/users/7/disable_second_factor", () => { + return helper.response({ success: "OK" }); + }); }); needs.hooks.beforeEach(() => { @@ -255,6 +277,18 @@ acceptance("Admin - User Index", function (needs) { ); }); + test("disable 2fa - shows the confirmation dialog", async function (assert) { + await visit("/admin/users/7/jimmy"); + await click(".disable-second-factor"); + assert.dom(".dialog-content").exists(); + assert.strictEqual( + I18n.t("admin.user.disable_second_factor_confirm"), + query(".dialog-body").textContent.trim() + ); + + await click(".dialog-footer .btn-primary"); + }); + test("delete user - delete without blocking works as expected", async function (assert) { await visit("/admin/users/5/user5"); await click(".btn-user-delete"); diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 6d2df1e5cb6..34e7fde9e39 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -6770,8 +6770,9 @@ en: time_read: "Read Time" post_edits_count: "Post Edits" anonymize: "Anonymize User" - anonymize_confirm: "Are you SURE you want to anonymize this account? This will change the username and email, and reset all profile information." - delete_associated_accounts_confirm: "Are you SURE you want to delete all associated accounts from this account? The user may not be able to log in if their email has changed." + anonymize_confirm: "Are you sure you want to anonymize this account? This will change the username and email, and reset all profile information." + delete_associated_accounts_confirm: "Are you sure you want to delete all associated accounts from this account? The user may not be able to log in if their email has changed." + disable_second_factor_confirm: "Are you sure you want to disable Two-Factor Authentication for this account?" anonymize_yes: "Yes, anonymize this account" anonymize_failed: "There was a problem anonymizing the account." delete: "Delete User" @@ -6832,7 +6833,7 @@ en: cant_delete_all_too_many_posts: one: "Can't delete all posts because the user has more than %{count} post. (delete_all_posts_max)" other: "Can't delete all posts because the user has more than %{count} posts. (delete_all_posts_max)" - delete_confirm_title: "Are you SURE you want to delete this user? This is permanent!" + delete_confirm_title: "Are you sure you want to delete this user? This is permanent!" delete_confirm: "It is generally preferable to anonymize users rather than deleting them, to avoid removing content from existing discussions." delete_and_block: "Delete and block this email and IP address" delete_dont_block: "Delete only"