UX: Add description to the 2FA page when adding new admins (#16098)

This PR adds an extra description to the 2FA page when granting a user admin access. It also introduces a general system for adding customized descriptions that can be used by future actions.

(Follow-up to dd6ec65061)

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

@@ -27,6 +27,7 @@ export default Controller.extend({
   backupCodesEnabled: readOnly("model.backup_enabled"),
   securityKeysEnabled: readOnly("model.security_keys_enabled"),
   allowedMethods: readOnly("model.allowed_methods"),
+  customDescription: readOnly("model.description"),
 
   showTotpForm: equal("shownSecondFactorMethod", TOTP),
   showSecurityKeyForm: equal("shownSecondFactorMethod", SECURITY_KEY),

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

@@ -3,6 +3,9 @@
 {{/if}}
 {{#unless loadError}}
   <h3>{{secondFactorTitle}}</h3>
+  {{#if customDescription}}
+    <p class="action-description">{{customDescription}}</p>
+  {{/if}}
   <p>{{secondFactorDescription}}</p>
   {{#if showSecurityKeyForm}}
     <div id="security-key">

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

@@ -46,6 +46,15 @@ const RESPONSES = {
   },
 };
 
+Object.keys(RESPONSES).forEach((k) => {
+  if (k.startsWith("ok")) {
+    const response = RESPONSES[k];
+    if (!response.description) {
+      response.description =
+        "This is an additional description that can be customized per action";
+    }
+  }
+});
 const WRONG_TOTP = "124323";
 let callbackCount = 0;
 
@@ -254,6 +263,16 @@ acceptance("Second Factor Auth Page", function (needs) {
     );
   });
 
+  test("2FA action description", async function (assert) {
+    await visit("/session/2fa?nonce=ok111111");
+
+    assert.equal(
+      query(".action-description").textContent.trim(),
+      "This is an additional description that can be customized per action",
+      "action description is rendered on the page"
+    );
+  });
+
   test("error when submitting 2FA form", async function (assert) {
     await visit("/session/2fa?nonce=ok110111");
     await fillIn("form.totp-token .second-factor-token-input", WRONG_TOTP);

app/controllers/session_controller.rb

@@ -456,6 +456,9 @@ class SessionController < ApplicationController
       else
         json[:security_keys_enabled] = false
       end
+      if challenge[:description]
+        json[:description] = challenge[:description]
+      end
     else
       json[:error] = I18n.t(error_key)
     end

config/locales/server.en.yml

@@ -2583,6 +2583,9 @@ en:
     challenge_not_found: "Couldn't find a 2FA challenge in your current session."
     challenge_expired: "Too much time has passed since the 2FA challenge was staged and it's no longer valid. Please try again."
     challenge_not_completed: "You've not completed the 2FA challenge to perform this action. Please complete the 2FA challenge and try again."
+    actions:
+      grant_admin:
+        description: "For additional security measures, you need to confirm your 2FA before %{username} is granted admin access."
   admin:
     email:
       sent_test: "sent!"

lib/second_factor/actions/grant_admin.rb

@@ -9,9 +9,14 @@ module SecondFactor::Actions
 
     def second_factor_auth_required!(params)
       user = find_user(params[:user_id])
+      description = I18n.t(
+        "second_factor_auth.actions.grant_admin.description",
+        username: "@#{user.username}"
+      )
       {
         callback_params: { user_id: user.id },
-        redirect_path: admin_user_show_path(id: user.id, username: user.username)
+        redirect_path: admin_user_show_path(id: user.id, username: user.username),
+        description: description
       }
     end
 

lib/second_factor/auth_manager.rb

@@ -52,6 +52,9 @@ the following methods:
   redirected to after the action is finished. When this key is omitted, the
   redirect path is set to the homepage (/).
 
+  :description => optional action-specific description message that's shown on
+  the 2FA page.
+
   After this method is called, the auth manager will send a 403 response with a
   JSON body. It does that by raising an exception that's then rescued by a
   `rescue_from` handler. The JSON response contains a challenge nonce which the
@@ -169,6 +172,9 @@ class SecondFactor::AuthManager
       allowed_methods: allowed_methods.to_a,
       generated_at: Time.zone.now.to_i
     }
+    if config[:description]
+      challenge[:description] = config[:description]
+    end
     secure_session["current_second_factor_auth_challenge"] = challenge.to_json
     nonce
   end

spec/lib/second_factor/actions/grant_admin_spec.rb

@@ -40,11 +40,17 @@ describe SecondFactor::Actions::GrantAdmin do
   end
 
   describe "#second_factor_auth_required!" do
-    it "returns a hash with callback_params and redirect_path" do
+    it "returns a hash with callback_params, redirect_path and a description" do
       instance = create_instance(admin)
       hash = instance.second_factor_auth_required!(params({ user_id: user.id }))
       expect(hash[:callback_params]).to eq({ user_id: user.id })
       expect(hash[:redirect_path]).to eq("/admin/users/#{user.id}/#{user.username}")
+      expect(hash[:description]).to eq(
+        I18n.t(
+          "second_factor_auth.actions.grant_admin.description",
+          username: "@#{user.username}"
+        )
+      )
     end
 
     it "ensures the acting user is admin" do

spec/lib/second_factor/auth_manager_spec.rb

@@ -94,7 +94,7 @@ describe SecondFactor::AuthManager do
       action
         .expects(:second_factor_auth_required!)
         .with({ expect_me: 131 })
-        .returns({ callback_params: { call_me_back: 4314 }, redirect_path: "/gg" })
+        .returns({ callback_params: { call_me_back: 4314 }, redirect_path: "/gg", description: "hello world!" })
         .once
       action.expects(:second_factor_auth_completed!).never
       manager = create_manager(action)
@@ -114,6 +114,7 @@ describe SecondFactor::AuthManager do
       expect(challenge[:redirect_path]).to eq("/gg")
       expect(challenge[:allowed_methods]).to eq(manager.allowed_methods.to_a)
       expect(challenge[:callback_params]).to eq({ call_me_back: 4314 })
+      expect(challenge[:description]).to eq("hello world!")
     end
 
     it "sets the redirect_path to the root path if second_factor_auth_required! doesn't specify a redirect_path" do

spec/requests/session_controller_spec.rb

@@ -2305,6 +2305,7 @@ describe SessionController do
         UserSecondFactor.methods[:totp],
         UserSecondFactor.methods[:security_key],
       )
+      expect(challenge_data["description"]).to eq("this is description for test action")
 
       Fabricate(
         :user_security_key_with_random_credential,

spec/support/test_second_factor_action.rb

@@ -10,7 +10,8 @@ class TestSecondFactorAction < SecondFactor::Actions::Base
       callback_params: {
         saved_param_1: params[:saved_param_1],
         saved_param_2: params[:saved_param_2]
-      }
+      },
+      description: "this is description for test action"
     }
   end