From ab792419a30803c5a49da18c28a9cc37eb212143 Mon Sep 17 00:00:00 2001
From: Marius BALTEANU <marius.balteanu@zitec.com>
Date: Sat, 22 Jan 2022 12:38:11 +0200
Subject: [PATCH] Option to require 2FA authentication only for users with
 administration rights (#35439).

---
 app/models/user.rb                          |  7 ++++++-
 app/views/settings/_authentication.html.erb | 19 +++++++++++++++++++
 config/locales/en.yml                       |  1 +
 config/settings.yml                         |  3 +++
 test/integration/twofa_test.rb              | 20 ++++++++++++++++++++
 5 files changed, 49 insertions(+), 1 deletion(-)

diff --git a/app/models/user.rb b/app/models/user.rb
index 8c190374f..08e949d93 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -386,7 +386,12 @@ class User < Principal
   def must_activate_twofa?
     (
       Setting.twofa_required? ||
-      (Setting.twofa_optional? && groups.any?(&:twofa_required?))
+      (
+        Setting.twofa_optional? && (
+          groups.any?(&:twofa_required?) ||
+          (Setting.twofa_required_for_administrators? && self.admin)
+        )
+      )
     ) && !twofa_active?
   end
 
diff --git a/app/views/settings/_authentication.html.erb b/app/views/settings/_authentication.html.erb
index c861ff50e..3d2a35135 100644
--- a/app/views/settings/_authentication.html.erb
+++ b/app/views/settings/_authentication.html.erb
@@ -37,6 +37,12 @@
     <%= t 'twofa_hint_optional_html', label: t(:label_optional) -%><br/>
     <%= t 'twofa_hint_required_html', label: t(:label_required_lower) -%>
   </em>
+  <span id="twofa_optional" class="<%= "hidden" unless Setting.twofa == "1" %>">
+    <label class="block">
+      <%= setting_check_box :twofa_required_for_administrators, label: false %>
+      <%= l(:setting_twofa_required_for_administrators) %>
+    </label>
+  </span>
 </p>
 
 </div>
@@ -54,3 +60,16 @@
 
 <%= submit_tag l(:button_save) %>
 <% end %>
+
+<%= javascript_tag do %>
+  $('#settings_twofa').on('change', function(e){
+    const twofa = e.target.value;
+    const parent_block = document.getElementById("twofa_optional");
+
+    if (twofa == "1") {
+      parent_block.classList.remove('hidden');
+    } else {
+      parent_block.classList.add('hidden');
+    }
+  });
+<% end %>
diff --git a/config/locales/en.yml b/config/locales/en.yml
index 761e4194c..a925e76a9 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -508,6 +508,7 @@ en:
   setting_show_status_changes_in_mail_subject: Show status changes in issue mail notifications subject
   setting_project_list_defaults: Projects list defaults
   setting_twofa: Two-factor authentication
+  setting_twofa_required_for_administrators: Require two-factor authentication for administrators
 
   permission_add_project: Create project
   permission_add_subprojects: Create subprojects
diff --git a/config/settings.yml b/config/settings.yml
index 0c41b7eda..c76509235 100644
--- a/config/settings.yml
+++ b/config/settings.yml
@@ -37,6 +37,9 @@ lost_password:
 twofa:
   default: 1
   security_notifications: 1
+twofa_required_for_administrators:
+  default: 0
+  security_notifications: 1
 unsubscribe:
   default: 1
 password_required_char_classes:
diff --git a/test/integration/twofa_test.rb b/test/integration/twofa_test.rb
index d23aa5a95..fd284d3a6 100644
--- a/test/integration/twofa_test.rb
+++ b/test/integration/twofa_test.rb
@@ -31,6 +31,26 @@ class TwofaTest < Redmine::IntegrationTest
     end
   end
 
+  test "should require twofa setup when required for administrators" do
+    user = User.find_by_login 'admin'
+    assert_not user.must_activate_twofa?
+
+    with_settings twofa: "0", twofa_required_for_administrators: "1" do
+      assert_not Setting.twofa_optional?
+      assert_not Setting.twofa_required?
+      assert_not user.must_activate_twofa?
+    end
+
+    with_settings twofa: "1", twofa_required_for_administrators: "1" do
+      assert Setting.twofa_optional?
+      assert_not Setting.twofa_required?
+      assert user.must_activate_twofa?
+      log_user('admin', 'admin')
+      follow_redirect!
+      assert_redirected_to "/my/twofa/totp/activate/confirm"
+    end
+  end
+
   test "should require twofa setup when required by group" do
     user = User.find_by_login 'jsmith'
     assert_not user.must_activate_twofa?
-- 
2.32.0 (Apple Git-132)