FEATURE: Move security related user preferences to different tab (#12264)

app/assets/javascripts/discourse/app/controllers/preferences/account.js

@@ -4,19 +4,12 @@ import CanCheckEmails from "discourse/mixins/can-check-emails";
 import Controller from "@ember/controller";
 import EmberObject from "@ember/object";
 import I18n from "I18n";
-import { ajax } from "discourse/lib/ajax";
 import bootbox from "bootbox";
 import discourseComputed from "discourse-common/utils/decorators";
 import { findAll } from "discourse/models/login-method";
 import getURL from "discourse-common/lib/get-url";
 import { iconHTML } from "discourse-common/lib/icon-library";
-import logout from "discourse/lib/logout";
 import { popupAjaxError } from "discourse/lib/ajax-error";
-import showModal from "discourse/lib/show-modal";
-import { userPath } from "discourse/lib/url";
-
-// Number of tokens shown by default.
-const DEFAULT_AUTH_TOKENS_COUNT = 2;
 
 export default Controller.extend(CanCheckEmails, {
   init() {
@@ -33,10 +26,6 @@ export default Controller.extend(CanCheckEmails, {
   newTitleInput: null,
   newPrimaryGroupInput: null,
 
-  passwordProgress: null,
-
-  showAllAuthTokens: false,
-
   revoking: null,
 
   cannotDeleteAccount: not("currentUser.can_delete_account"),
@@ -65,18 +54,6 @@ export default Controller.extend(CanCheckEmails, {
     );
   },
 
-  @discourseComputed("model.is_anonymous")
-  canChangePassword(isAnonymous) {
-    if (isAnonymous) {
-      return false;
-    } else {
-      return (
-        !this.siteSettings.enable_discourse_connect &&
-        this.siteSettings.enable_local_logins
-      );
-    }
-  },
-
   @discourseComputed("model.associated_accounts")
   associatedAccountsLoaded(associatedAccounts) {
     return typeof associatedAccounts !== "undefined";
@@ -147,28 +124,6 @@ export default Controller.extend(CanCheckEmails, {
     return findAll().length > 0;
   },
 
-  @discourseComputed("showAllAuthTokens", "model.user_auth_tokens")
-  authTokens(showAllAuthTokens, tokens) {
-    tokens.sort((a, b) => {
-      if (a.is_active) {
-        return -1;
-      } else if (b.is_active) {
-        return 1;
-      } else {
-        return b.seen_at.localeCompare(a.seen_at);
-      }
-    });
-
-    return showAllAuthTokens
-      ? tokens
-      : tokens.slice(0, DEFAULT_AUTH_TOKENS_COUNT);
-  },
-
-  canShowAllAuthTokens: gt(
-    "model.user_auth_tokens.length",
-    DEFAULT_AUTH_TOKENS_COUNT
-  ),
-
   actions: {
     save() {
       this.set("saved", false);
@@ -205,31 +160,6 @@ export default Controller.extend(CanCheckEmails, {
         });
     },
 
-    changePassword() {
-      if (!this.passwordProgress) {
-        this.set(
-          "passwordProgress",
-          I18n.t("user.change_password.in_progress")
-        );
-        return this.model
-          .changePassword()
-          .then(() => {
-            // password changed
-            this.setProperties({
-              changePasswordProgress: false,
-              passwordProgress: I18n.t("user.change_password.success"),
-            });
-          })
-          .catch(() => {
-            // password failed to change
-            this.setProperties({
-              changePasswordProgress: false,
-              passwordProgress: I18n.t("user.change_password.error"),
-            });
-          });
-      }
-    },
-
     delete() {
       this.set("deleting", true);
       const message = I18n.t("user.delete_account_confirm"),
@@ -282,32 +212,6 @@ export default Controller.extend(CanCheckEmails, {
         .finally(() => this.set(`revoking.${account.name}`, false));
     },
 
-    toggleShowAllAuthTokens() {
-      this.toggleProperty("showAllAuthTokens");
-    },
-
-    revokeAuthToken(token) {
-      ajax(
-        userPath(
-          `${this.get("model.username_lower")}/preferences/revoke-auth-token`
-        ),
-        {
-          type: "POST",
-          data: token ? { token_id: token.id } : {},
-        }
-      )
-        .then(() => {
-          if (!token) {
-            logout();
-          } // All sessions revoked
-        })
-        .catch(popupAjaxError);
-    },
-
-    showToken(token) {
-      showModal("auth-token", { model: token });
-    },
-
     connectAccount(method) {
       method.doLogin({ reconnect: true });
     },

app/assets/javascripts/discourse/app/controllers/preferences/security.js

@@ -0,0 +1,114 @@
+import Controller from "@ember/controller";
+import { gt } from "@ember/object/computed";
+import discourseComputed from "discourse-common/utils/decorators";
+import { ajax } from "discourse/lib/ajax";
+import { popupAjaxError } from "discourse/lib/ajax-error";
+import logout from "discourse/lib/logout";
+import showModal from "discourse/lib/show-modal";
+import { userPath } from "discourse/lib/url";
+import CanCheckEmails from "discourse/mixins/can-check-emails";
+import I18n from "I18n";
+
+// Number of tokens shown by default.
+const DEFAULT_AUTH_TOKENS_COUNT = 2;
+
+export default Controller.extend(CanCheckEmails, {
+  passwordProgress: null,
+
+  showAllAuthTokens: false,
+
+  @discourseComputed("model.is_anonymous")
+  canChangePassword(isAnonymous) {
+    if (isAnonymous) {
+      return false;
+    } else {
+      return (
+        !this.siteSettings.enable_discourse_connect &&
+        this.siteSettings.enable_local_logins
+      );
+    }
+  },
+
+  @discourseComputed("showAllAuthTokens", "model.user_auth_tokens")
+  authTokens(showAllAuthTokens, tokens) {
+    tokens.sort((a, b) => {
+      if (a.is_active) {
+        return -1;
+      } else if (b.is_active) {
+        return 1;
+      } else {
+        return b.seen_at.localeCompare(a.seen_at);
+      }
+    });
+
+    return showAllAuthTokens
+      ? tokens
+      : tokens.slice(0, DEFAULT_AUTH_TOKENS_COUNT);
+  },
+
+  canShowAllAuthTokens: gt(
+    "model.user_auth_tokens.length",
+    DEFAULT_AUTH_TOKENS_COUNT
+  ),
+
+  actions: {
+    save() {
+      this.set("saved", false);
+
+      return this.model
+        .then(() => this.set("saved", true))
+        .catch(popupAjaxError);
+    },
+
+    changePassword() {
+      if (!this.passwordProgress) {
+        this.set(
+          "passwordProgress",
+          I18n.t("user.change_password.in_progress")
+        );
+        return this.model
+          .changePassword()
+          .then(() => {
+            // password changed
+            this.setProperties({
+              changePasswordProgress: false,
+              passwordProgress: I18n.t("user.change_password.success"),
+            });
+          })
+          .catch(() => {
+            // password failed to change
+            this.setProperties({
+              changePasswordProgress: false,
+              passwordProgress: I18n.t("user.change_password.error"),
+            });
+          });
+      }
+    },
+
+    toggleShowAllAuthTokens() {
+      this.toggleProperty("showAllAuthTokens");
+    },
+
+    revokeAuthToken(token) {
+      ajax(
+        userPath(
+          `${this.get("model.username_lower")}/preferences/revoke-auth-token`
+        ),
+        {
+          type: "POST",
+          data: token ? { token_id: token.id } : {},
+        }
+      )
+        .then(() => {
+          if (!token) {
+            logout();
+          } // All sessions revoked
+        })
+        .catch(popupAjaxError);
+    },
+
+    showToken(token) {
+      showModal("auth-token", { model: token });
+    },
+  },
+});

app/assets/javascripts/discourse/app/routes/app-route-map.js

@@ -158,6 +158,7 @@ export default function () {
 
       this.route("preferences", { resetNamespace: true }, function () {
         this.route("account");
+        this.route("security");
         this.route("profile");
         this.route("emails");
         this.route("notifications");

app/assets/javascripts/discourse/app/routes/preferences-security.js

@@ -0,0 +1,5 @@
+import RestrictedUserRoute from "discourse/routes/restricted-user";
+
+export default RestrictedUserRoute.extend({
+  showFooter: true,
+});

app/assets/javascripts/discourse/app/templates/preferences.hbs

@@ -5,6 +5,11 @@
         {{i18n "user.preferences_nav.account"}}
       {{/link-to}}
     </li>
+    <li class="nav-security">
+      {{#link-to "preferences.security"}}
+        {{i18n "user.preferences_nav.security"}}
+      {{/link-to}}
+    </li>
     <li class="nav-profile">
       {{#link-to "preferences.profile"}}
         {{i18n "user.preferences_nav.profile"}}

app/assets/javascripts/discourse/app/templates/preferences/account.hbs

@@ -26,22 +26,6 @@
   </div>
 {{/unless}}
 
-{{#if canEditName}}
-  <div class="control-group pref-name">
-    <label class="control-label">{{i18n "user.name.title"}}</label>
-    <div class="controls">
-      {{#if model.can_edit_name}}
-        {{text-field value=newNameInput classNames="input-xxlarge" maxlength="255"}}
-      {{else}}
-        <span class="static">{{model.name}}</span>
-      {{/if}}
-    </div>
-    <div class="instructions">
-      {{nameInstructions}}
-    </div>
-  </div>
-{{/if}}
-
 {{#if canCheckEmails}}
   <div class="control-group pref-email">
     <label class="control-label">{{i18n "user.email.title"}}</label>
@@ -107,40 +91,6 @@
   </div>
 {{/if}}
 
-{{#if canChangePassword}}
-  <div class="control-group pref-password">
-    <label class="control-label">{{i18n "user.password.title"}}</label>
-    <div class="controls">
-      <a href {{action "changePassword"}} class="btn btn-default">
-        {{d-icon "envelope"}}
-        {{#if model.no_password}}
-          {{i18n "user.change_password.set_password"}}
-        {{else}}
-          {{i18n "user.change_password.action"}}
-        {{/if}}
-      </a>
-
-      {{passwordProgress}}
-    </div>
-  </div>
-
-  <div class="control-group pref-second-factor">
-    <label class="control-label">{{i18n "user.second_factor.title"}}</label>
-    {{#unless model.second_factor_enabled}}
-      <label>
-        {{i18n "user.second_factor.short_description"}}
-      </label>
-    {{/unless}}
-    <div class="controls pref-second-factor">
-      {{#if isCurrentUser}}
-        {{#link-to "preferences.second-factor" class="btn btn-default"}}
-          {{d-icon "lock"}} <span>{{i18n "user.second_factor.enable"}}</span>
-        {{/link-to}}
-      {{/if}}
-    </div>
-  </div>
-{{/if}}
-
 {{#if canUpdateAssociatedAccounts}}
   <div class="control-group pref-associated-accounts">
     <label class="control-label">{{i18n "user.associated_accounts.title"}}</label>
@@ -181,6 +131,22 @@
   </div>
 {{/if}}
 
+{{#if canEditName}}
+  <div class="control-group pref-name">
+    <label class="control-label">{{i18n "user.name.title"}}</label>
+    <div class="controls">
+      {{#if model.can_edit_name}}
+        {{text-field value=newNameInput classNames="input-xxlarge" maxlength="255"}}
+      {{else}}
+        <span class="static">{{model.name}}</span>
+      {{/if}}
+    </div>
+    <div class="instructions">
+      {{nameInstructions}}
+    </div>
+  </div>
+{{/if}}
+
 {{#if canSelectTitle}}
   <div class="control-group pref-title">
     <label class="control-label">{{i18n "user.title.title"}}</label>
@@ -207,47 +173,6 @@
   </div>
 {{/if}}
 
-{{#if canCheckEmails}}
-  <div class="control-group pref-auth-tokens">
-    <label class="control-label">{{i18n "user.auth_tokens.title"}}</label>
-
-    <div class="auth-tokens">
-      {{#each authTokens as |token|}}
-        <div class="row auth-token">
-          <div class="auth-token-icon">{{d-icon token.icon}}</div>
-          {{#unless token.is_active}}
-            {{auth-token-dropdown token=token
-                                  revokeAuthToken=(action "revokeAuthToken")
-                                  showToken=(action "showToken")}}
-          {{/unless}}
-          <div class="auth-token-first">
-            {{html-safe (i18n "user.auth_tokens.device_location" device=token.device ip=token.client_ip location=token.location)}}
-          </div>
-          <div class="auth-token-second">
-            {{#if token.is_active}}
-              {{html-safe (i18n "user.auth_tokens.browser_active" browser=token.browser)}}
-            {{else}}
-              {{html-safe (i18n "user.auth_tokens.browser_last_seen" browser=token.browser date=(format-date token.seen_at))}}
-            {{/if}}
-          </div>
-        </div>
-      {{/each}}
-    </div>
-
-    {{#if canShowAllAuthTokens}}
-      <a href {{action "toggleShowAllAuthTokens"}}>
-        {{#if showAllAuthTokens}}
-          {{d-icon "caret-up"}} {{i18n "user.auth_tokens.show_few"}}
-        {{else}}
-          {{d-icon "caret-down"}} {{i18n "user.auth_tokens.show_all" count=model.user_auth_tokens.length}}
-        {{/if}}
-      </a>
-    {{/if}}
-
-    <a href {{action "revokeAuthToken"}} class="pull-right text-danger">{{d-icon "sign-out-alt"}} {{i18n "user.auth_tokens.log_out_all"}}</a>
-  </div>
-{{/if}}
-
 {{plugin-outlet name="user-preferences-account" args=(hash model=model save=(action "save"))}}
 
 <br>

app/assets/javascripts/discourse/app/templates/preferences/security.hbs

@@ -0,0 +1,80 @@
+{{#if canChangePassword}}
+  <div class="control-group pref-password">
+    <label class="control-label">{{i18n "user.password.title"}}</label>
+    <div class="controls">
+      <a href {{action "changePassword"}} class="btn btn-default">
+        {{d-icon "envelope"}}
+        {{#if model.no_password}}
+          {{i18n "user.change_password.set_password"}}
+        {{else}}
+          {{i18n "user.change_password.action"}}
+        {{/if}}
+      </a>
+
+      {{passwordProgress}}
+    </div>
+  </div>
+
+  <div class="control-group pref-second-factor">
+    <label class="control-label">{{i18n "user.second_factor.title"}}</label>
+    {{#unless model.second_factor_enabled}}
+      <label>
+        {{i18n "user.second_factor.short_description"}}
+      </label>
+    {{/unless}}
+    <div class="controls pref-second-factor">
+      {{#if isCurrentUser}}
+        {{#link-to "preferences.second-factor" class="btn btn-default"}}
+          {{d-icon "lock"}} <span>{{i18n "user.second_factor.enable"}}</span>
+        {{/link-to}}
+      {{/if}}
+    </div>
+  </div>
+{{/if}}
+
+{{#if canCheckEmails}}
+  <div class="control-group pref-auth-tokens">
+    <label class="control-label">{{i18n "user.auth_tokens.title"}}</label>
+
+    <div class="auth-tokens">
+      {{#each authTokens as |token|}}
+        <div class="row auth-token">
+          <div class="auth-token-icon">{{d-icon token.icon}}</div>
+          {{#unless token.is_active}}
+            {{auth-token-dropdown token=token
+                                  revokeAuthToken=(action "revokeAuthToken")
+                                  showToken=(action "showToken")}}
+          {{/unless}}
+          <div class="auth-token-first">
+            {{html-safe (i18n "user.auth_tokens.device_location" device=token.device ip=token.client_ip location=token.location)}}
+          </div>
+          <div class="auth-token-second">
+            {{#if token.is_active}}
+              {{html-safe (i18n "user.auth_tokens.browser_active" browser=token.browser)}}
+            {{else}}
+              {{html-safe (i18n "user.auth_tokens.browser_last_seen" browser=token.browser date=(format-date token.seen_at))}}
+            {{/if}}
+          </div>
+        </div>
+      {{/each}}
+    </div>
+
+    {{#if canShowAllAuthTokens}}
+      <a href {{action "toggleShowAllAuthTokens"}}>
+        {{#if showAllAuthTokens}}
+          {{d-icon "caret-up"}} {{i18n "user.auth_tokens.show_few"}}
+        {{else}}
+          {{d-icon "caret-down"}} {{i18n "user.auth_tokens.show_all" count=model.user_auth_tokens.length}}
+        {{/if}}
+      </a>
+    {{/if}}
+
+    <a href {{action "revokeAuthToken"}} class="pull-right text-danger">{{d-icon "sign-out-alt"}} {{i18n "user.auth_tokens.log_out_all"}}</a>
+  </div>
+{{/if}}
+
+{{plugin-outlet name="user-preferences-security" args=(hash model=model save=(action "save"))}}
+
+<br>
+
+{{plugin-outlet name="user-custom-controls" args=(hash model=model)}}

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

@@ -329,47 +329,6 @@ acceptance("User Preferences when badges are disabled", function (needs) {
     );
     assert.ok(exists(".user-preferences"), "it shows the preferences");
   });
-
-  test("recently connected devices", async function (assert) {
-    await visit("/u/eviltrout/preferences");
-
-    assert.equal(
-      queryAll(".auth-tokens > .auth-token:nth-of-type(1) .auth-token-device")
-        .text()
-        .trim(),
-      "Linux Computer",
-      "it should display active token first"
-    );
-
-    assert.equal(
-      queryAll(".pref-auth-tokens > a:nth-of-type(1)").text().trim(),
-      I18n.t("user.auth_tokens.show_all", { count: 3 }),
-      "it should display two tokens"
-    );
-    assert.ok(
-      queryAll(".pref-auth-tokens .auth-token").length === 2,
-      "it should display two tokens"
-    );
-
-    await click(".pref-auth-tokens > a:nth-of-type(1)");
-
-    assert.ok(
-      queryAll(".pref-auth-tokens .auth-token").length === 3,
-      "it should display three tokens"
-    );
-
-    await click(".auth-token-dropdown button:nth-of-type(1)");
-    await click("li[data-value='notYou']");
-
-    assert.ok(queryAll(".d-modal:visible").length === 1, "modal should appear");
-
-    await click(".modal-footer .btn-primary");
-
-    assert.ok(
-      queryAll(".pref-password.highlighted").length === 1,
-      "it should highlight password preferences"
-    );
-  });
 });
 
 acceptance(
@@ -515,3 +474,49 @@ acceptance("Ignored users", function (needs) {
     assert.ok(exists(".user-ignore"), "it shows the list of ignored users");
   });
 });
+
+acceptance("Security", function (needs) {
+  needs.user();
+  needs.pretender(preferencesPretender);
+
+  test("recently connected devices", async function (assert) {
+    await visit("/u/eviltrout/preferences/security");
+
+    assert.equal(
+      queryAll(".auth-tokens > .auth-token:nth-of-type(1) .auth-token-device")
+        .text()
+        .trim(),
+      "Linux Computer",
+      "it should display active token first"
+    );
+
+    assert.equal(
+      queryAll(".pref-auth-tokens > a:nth-of-type(1)").text().trim(),
+      I18n.t("user.auth_tokens.show_all", { count: 3 }),
+      "it should display two tokens"
+    );
+    assert.ok(
+      queryAll(".pref-auth-tokens .auth-token").length === 2,
+      "it should display two tokens"
+    );
+
+    await click(".pref-auth-tokens > a:nth-of-type(1)");
+
+    assert.ok(
+      queryAll(".pref-auth-tokens .auth-token").length === 3,
+      "it should display three tokens"
+    );
+
+    await click(".auth-token-dropdown button:nth-of-type(1)");
+    await click("li[data-value='notYou']");
+
+    assert.ok(queryAll(".d-modal:visible").length === 1, "modal should appear");
+
+    await click(".modal-footer .btn-primary");
+
+    assert.ok(
+      queryAll(".pref-password.highlighted").length === 1,
+      "it should highlight password preferences"
+    );
+  });
+});

config/locales/client.en.yml

@@ -1139,6 +1139,7 @@ en:
 
       preferences_nav:
         account: "Account"
+        security: "Security"
         profile: "Profile"
         emails: "Emails"
         notifications: "Notifications"

config/routes.rb

@@ -450,6 +450,7 @@ Discourse::Application.routes.draw do
       get "#{root_path}/:username/preferences" => "users#preferences", constraints: { username: RouteFormat.username }
       get "#{root_path}/:username/preferences/email" => "users_email#index", constraints: { username: RouteFormat.username }
       get "#{root_path}/:username/preferences/account" => "users#preferences", constraints: { username: RouteFormat.username }
+      get "#{root_path}/:username/preferences/security" => "users#preferences", constraints: { username: RouteFormat.username }
       get "#{root_path}/:username/preferences/profile" => "users#preferences", constraints: { username: RouteFormat.username }
       get "#{root_path}/:username/preferences/emails" => "users#preferences", constraints: { username: RouteFormat.username }
       put "#{root_path}/:username/preferences/primary-email" => "users#update_primary_email", format: :json, constraints: { username: RouteFormat.username }