FIX: Deleting security keys was not working (#20427)

Bug was introduced in e313190fdb. There is
a workaround, using the trash icon in the edit modal, but that UI is
quite confusing to users.

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

@@ -166,6 +166,7 @@ export default Controller.extend(CanCheckEmails, {
         confirmButtonIcon: "ban",
         cancelButtonClass: "btn-flat",
         didConfirm: () => {
+          if (this.totps.includes(secondFactorMethod)) {
             this.currentUser
               .updateSecondFactor(
                 secondFactorMethod.id,
@@ -178,28 +179,48 @@ export default Controller.extend(CanCheckEmails, {
                   return;
                 }
                 this.markDirty();
-            })
+                this.set(
-            .catch((e) => this.handleError(e))
+                  "totps",
-            .finally(() => {
+                  this.totps.filter(
-              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 ||
+              .catch((e) => this.handleError(e))
-                    key.method !== secondFactorMethod.method
+              .finally(() => {
-                ),
-              });
-
                 this.set("loading", false);
               });
+          }
+
+          if (this.security_keys.includes(secondFactorMethod)) {
+            this.currentUser
+              .updateSecurityKey(
+                secondFactorMethod.id,
+                secondFactorMethod.name,
+                true
+              )
+              .then((response) => {
+                if (response.error) {
+                  return;
+                }
+                this.markDirty();
+                this.set(
+                  "security_keys",
+                  this.security_keys.filter(
+                    (securityKey) => securityKey.id !== secondFactorMethod.id
+                  )
+                );
+              })
+              .catch((e) => this.handleError(e))
+              .finally(() => {
+                this.set("loading", false);
+              });
+          }
         },
       });
     },
-
     disableSecondFactorBackup() {
       this.dialog.deleteConfirm({
         title: I18n.t("user.second_factor.delete_backup_codes_confirm_title"),

app/assets/javascripts/discourse/app/controllers/second-factor-edit-security-key.js

@@ -3,25 +3,6 @@ import ModalFunctionality from "discourse/mixins/modal-functionality";
 
 export default Controller.extend(ModalFunctionality, {
   actions: {
-    disableSecurityKey() {
-      this.user
-        .updateSecurityKey(this.model.id, this.model.name, true)
-        .then((response) => {
-          if (response.error) {
-            return;
-          }
-          this.markDirty();
-        })
-        .catch((error) => {
-          this.send("closeModal");
-          this.onError(error);
-        })
-        .finally(() => {
-          this.set("loading", false);
-          this.send("closeModal");
-        });
-    },
-
     editSecurityKey() {
       this.user
         .updateSecurityKey(this.model.id, this.model.name, false)

app/assets/javascripts/discourse/app/templates/modal/second-factor-edit-security-key.hbs

@@ -15,11 +15,4 @@
     @class="btn-primary"
     @label="user.second_factor.security_key.save"
   />
-  <DButton
-    @action={{action "disableSecurityKey"}}
-    @class="btn-danger no-text"
-    @icon="trash-alt"
-    @aria-label="user.second_factor.disable"
-    @title="user.second_factor.disable"
-  />
 </div>

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

@@ -169,7 +169,7 @@
 
           {{#if this.model.second_factor_enabled}}
             {{#unless this.showEnforcedNotice}}
-              <div class="control-group">
+              <div class="control-group pref-second-factor-disable-all">
                 <div class="controls">
                   <DButton
                     @class="btn-danger"

app/assets/javascripts/discourse/tests/acceptance/user-preferences-second-factor-test.js

@@ -41,6 +41,12 @@ acceptance("User Preferences - Second Factor", function (needs) {
       });
     });
 
+    server.put("/u/security_key.json", () => {
+      return helper.response({
+        success: "OK",
+      });
+    });
+
     server.put("/u/second_factors_backup.json", () => {
       return helper.response({
         backup_codes: ["dsffdsd", "fdfdfdsf", "fddsds"],
@@ -109,14 +115,23 @@ acceptance("User Preferences - Second Factor", function (needs) {
     );
     await click(".dialog-close");
 
+    assert.ok(
+      exists(".security-key .second-factor-item"),
+      "User has a physical security key"
+    );
+
     await click(".security-key .btn-danger");
     assert.strictEqual(
       query("#dialog-title").innerText.trim(),
       "Deleting an authenticator"
     );
-    await click(".dialog-close");
+    await click(".dialog-footer .btn-danger");
+    assert.notOk(
+      exists(".security-key .second-factor-item"),
+      "security key row is removed after a successful delete"
+    );
 
-    await click(".btn-danger.btn-icon-text");
+    await click(".pref-second-factor-disable-all .btn-danger");
     assert.strictEqual(
       query("#dialog-title").innerText.trim(),
       "Are you sure you want to disable two-factor authentication?"