From 9606d04e525554ede65d1ca9ff95d67dfe13bfac 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 1/2] Option to require 2FA authentication only for users with
 administration rights (#35439).

---
 app/models/setting.rb                       |  4 ++++
 app/models/user.rb                          |  1 +
 app/views/settings/_authentication.html.erb |  8 +++++---
 config/additional_environment.rb.example    |  1 -
 config/locales/en.yml                       |  2 ++
 test/integration/twofa_test.rb              | 15 +++++++++++++++
 6 files changed, 27 insertions(+), 4 deletions(-)

diff --git a/app/models/setting.rb b/app/models/setting.rb
index f4bdbaadf..50cd8eaab 100644
--- a/app/models/setting.rb
+++ b/app/models/setting.rb
@@ -247,6 +247,10 @@ class Setting < ActiveRecord::Base
     twofa == '1'
   end
 
+  def self.twofa_required_for_administrators?
+    twofa == '3'
+  end
+
   # Helper that returns an array based on per_page_options setting
   def self.per_page_options_array
     per_page_options.split(%r{[\s,]}).collect(&:to_i).select {|n| n > 0}.sort
diff --git a/app/models/user.rb b/app/models/user.rb
index 8c190374f..2570c801a 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -386,6 +386,7 @@ class User < Principal
   def must_activate_twofa?
     (
       Setting.twofa_required? ||
+      (Setting.twofa_required_for_administrators? && admin?) ||
       (Setting.twofa_optional? && groups.any?(&:twofa_required?))
     ) && !twofa_active?
   end
diff --git a/app/views/settings/_authentication.html.erb b/app/views/settings/_authentication.html.erb
index c861ff50e..5da36abdb 100644
--- a/app/views/settings/_authentication.html.erb
+++ b/app/views/settings/_authentication.html.erb
@@ -31,11 +31,13 @@
 <p>
   <%= setting_select :twofa, [[l(:label_disabled), "0"],
                               [l(:label_optional), "1"],
-                              [l(:label_required_lower), "2"]] -%>
+                              [l(:label_required_lower), "2"],
+                              [l(:label_required_administrators), "3"]] -%>
   <em class="info">
     <%= t 'twofa_hint_disabled_html', label: t(:label_disabled) -%><br/>
     <%= t 'twofa_hint_optional_html', label: t(:label_optional) -%><br/>
-    <%= t 'twofa_hint_required_html', label: t(:label_required_lower) -%>
+    <%= t 'twofa_hint_required_html', label: t(:label_required_lower) -%><br/>
+    <%= t 'twofa_hint_required_administrators_html', label: t(:label_required_administrators) -%>
   </em>
 </p>
 
@@ -48,7 +50,7 @@
     <p><%= setting_select :session_lifetime, session_lifetime_options %></p>
     <p><%= setting_select :session_timeout, session_timeout_options %></p>
   </div>
-  
+
   <p><em class="info"><%= l(:text_session_expiration_settings) %></em></p>
 </fieldset>
 
diff --git a/config/additional_environment.rb.example b/config/additional_environment.rb.example
index 2a317a396..13db3a0ef 100644
--- a/config/additional_environment.rb.example
+++ b/config/additional_environment.rb.example
@@ -7,4 +7,3 @@
 #   config.log_level = :debug
 #   ...
 #
-
diff --git a/config/locales/en.yml b/config/locales/en.yml
index 761e4194c..1537a3fc6 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -1019,6 +1019,7 @@ en:
   label_readonly: Read-only
   label_required: Required
   label_required_lower: required
+  label_required_administrators: required for administrators
   label_hidden: Hidden
   label_attribute_of_project: "Project's %{name}"
   label_attribute_of_issue: "Issue's %{name}"
@@ -1349,6 +1350,7 @@ en:
   twofa_hint_disabled_html: Setting <strong>%{label}</strong> will deactivate and unpair two-factor authentication devices for all users.
   twofa_hint_optional_html: Setting <strong>%{label}</strong> will let users set up two-factor authentication at will, unless it is required by one of their groups.
   twofa_hint_required_html: Setting <strong>%{label}</strong> will require all users to set up two-factor authentication at their next login.
+  twofa_hint_required_administrators_html: Setting <strong>%{label}</strong> will require all users with administration rights to set up two-factor authentication at their next login.
   twofa_label_setup: Enable two-factor authentication
   twofa_label_deactivation_confirmation: Disable two-factor authentication
   twofa_notice_select: "Please select the two-factor scheme you would like to use:"
diff --git a/test/integration/twofa_test.rb b/test/integration/twofa_test.rb
index d23aa5a95..1d5624a16 100644
--- a/test/integration/twofa_test.rb
+++ b/test/integration/twofa_test.rb
@@ -31,6 +31,21 @@ 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: "3" do
+      assert_not Setting.twofa_optional?
+      assert_not Setting.twofa_required?
+      assert Setting.twofa_required_for_administrators?
+      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.30.1 (Apple Git-130)