From 6e382fc2ee518c8bbd3878fc5937a4313967f725 Mon Sep 17 00:00:00 2001
From: "marius.balteanu" <marius.balteanu@zitec.com>
Date: Sun, 30 Jan 2022 12:04:47 +0200
Subject: [PATCH] Option to require 2FA authentication only for users with
 administration rights (#35439).

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

diff --git a/app/models/setting.rb b/app/models/setting.rb
index f4bdbaadf..a7b763503 100644
--- a/app/models/setting.rb
+++ b/app/models/setting.rb
@@ -244,7 +244,11 @@ class Setting < ActiveRecord::Base
   end
 
   def self.twofa_optional?
-    twofa == '1'
+    %w[1 3].include? twofa
+  end
+
+  def self.twofa_required_for_administrators?
+    twofa == '3'
   end
 
   # Helper that returns an array based on per_page_options setting
diff --git a/app/models/user.rb b/app/models/user.rb
index 7cdfa1dbd..eac3d82ae 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -387,6 +387,7 @@ class User < Principal
     return false if twofa_active?
 
     return true if Setting.twofa_required?
+    return true if Setting.twofa_required_for_administrators? && admin?
     return true if Setting.twofa_optional? && groups.any?(&:twofa_required?)
   end
 
diff --git a/app/views/settings/_authentication.html.erb b/app/views/settings/_authentication.html.erb
index c861ff50e..fc20dd03d 100644
--- a/app/views/settings/_authentication.html.erb
+++ b/app/views/settings/_authentication.html.erb
@@ -31,10 +31,12 @@
 <p>
   <%= setting_select :twofa, [[l(:label_disabled), "0"],
                               [l(:label_optional), "1"],
+                              [l(:label_required_administrators), "3"],
                               [l(:label_required_lower), "2"]] -%>
   <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_administrators_html', label: t(:label_required_administrators) -%><br/>
     <%= t 'twofa_hint_required_html', label: t(:label_required_lower) -%>
   </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/locales/en.yml b/config/locales/en.yml
index 761e4194c..2378e56d5 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> behaves like optional, but 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..dd94c83a1 100644
--- a/test/integration/twofa_test.rb
+++ b/test/integration/twofa_test.rb
@@ -31,6 +31,27 @@ class TwofaTest < Redmine::IntegrationTest
     end
   end
 
+  test "should require twofa setup when required for administrators" do
+    admin = User.find_by_login 'admin'
+    user = User.find_by_login 'jsmith'
+
+    assert_not admin.must_activate_twofa?
+    assert_not user.must_activate_twofa?
+
+    with_settings twofa: "3" do
+      assert_not Setting.twofa_required?
+
+      assert Setting.twofa_optional?
+      assert Setting.twofa_required_for_administrators?
+      assert admin.must_activate_twofa?
+      assert_not 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.35.1