FIX: Ensure disabling 2FA works as expected (#10485)

app/assets/javascripts/discourse/app/controllers/preferences/second-factor.js

@@ -2,6 +2,7 @@ import I18n from "I18n";
 import { alias } from "@ember/object/computed";
 import Controller from "@ember/controller";
 import discourseComputed from "discourse-common/utils/decorators";
+import { iconHTML } from "discourse-common/lib/icon-library";
 import CanCheckEmails from "discourse/mixins/can-check-emails";
 import DiscourseURL, { userPath } from "discourse/lib/url";
 import { popupAjaxError } from "discourse/lib/ajax-error";
@@ -120,12 +121,17 @@ export default Controller.extend(CanCheckEmails, {
       if (this.loading) {
         return;
       }
-      bootbox.confirm(
+      const message = I18n.t("user.second_factor.disable_confirm");
-        I18n.t("user.second_factor.disable_confirm"),
+      const buttons = [
-        I18n.t("cancel"),
+        {
-        I18n.t("user.second_factor.disable"),
+          label: I18n.t("cancel"),
-        result => {
+          class: "d-modal-cancel",
-          if (result) {
+          link: true
+        },
+        {
+          label: `${iconHTML("ban")}${I18n.t("user.second_factor.disable")}`,
+          class: "btn-danger btn-icon-text",
+          callback: () => {
             this.model
               .disableAllSecondFactors()
               .then(() => {
@@ -138,7 +144,11 @@ export default Controller.extend(CanCheckEmails, {
               .finally(() => this.set("loading", false));
           }
         }
-      );
+      ];
+
+      bootbox.dialog(message, buttons, {
+        classes: "disable-second-factor-modal"
+      });
     },
 
     createTotp() {

app/assets/javascripts/discourse/app/templates/preferences-second-factor.hbs

@@ -120,7 +120,7 @@
                   icon="ban"
                   action=(action "disableAllSecondFactors")
                   disabled=loading
-                  label="disable"}}
+                  label="user.second_factor.disable_all"}}
               </div>
             </div>
           {{/unless}}

app/assets/stylesheets/common/base/user.scss

@@ -616,7 +616,7 @@
         height: auto;
         text-align: center;
         width: 100%;
-        background: white;
+        background: var(--secondary);
         border: 0;
         cursor: auto;
         outline: none;

app/controllers/users_controller.rb

@@ -1385,6 +1385,7 @@ class UsersController < ApplicationController
   def disable_second_factor
     # delete all second factors for a user
     current_user.user_second_factors.destroy_all
+    current_user.security_keys.destroy_all
 
     Jobs.enqueue(
       :critical_user_email,

config/locales/client.en.yml

@@ -1086,6 +1086,7 @@ en:
       second_factor:
         title: "Two Factor Authentication"
         enable: "Manage Two Factor Authentication"
+        disable_all: "Disable All"
         forgot_password: "Forgot password?"
         confirm_password_description: "Please confirm your password to continue"
         name: "Name"
@@ -1103,7 +1104,7 @@ 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 second factors?"
+        disable_confirm: "Are you sure you want to disable all second factor methods?"
         save: "Save"
         edit: "Edit"
         edit_title: "Edit Second Factor"

spec/requests/users_controller_spec.rb

@@ -4080,6 +4080,32 @@ describe UsersController do
     end
   end
 
+  describe '#disable_second_factor' do
+    context 'when logged in with secure session' do
+      before do
+        sign_in(user)
+        stub_secure_session_confirmed
+      end
+      context 'when user has a registered totp and security key' do
+        before do
+          totp_second_factor = Fabricate(:user_second_factor_totp, user: user)
+          security_key_second_factor = Fabricate(:user_security_key, user: user, factor_type: UserSecurityKey.factor_types[:second_factor])
+        end
+
+        it 'should disable all totp and security keys' do
+          expect_enqueued_with(job: :critical_user_email, args: { type: :account_second_factor_disabled, user_id: user.id }) do
+            put "/u/disable_second_factor.json"
+
+            expect(response.status).to eq(200)
+
+            expect(user.reload.user_second_factors).to be_empty
+            expect(user.security_keys).to be_empty
+          end
+        end
+      end
+    end
+  end
+
   describe '#revoke_account' do
     fab!(:other_user) { Fabricate(:user) }
     it 'errors for unauthorised users' do