diff --git a/app/models/group.rb b/app/models/group.rb index a84f8650f..a1fd35602 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -41,6 +41,7 @@ class Group < Principal safe_attributes( 'name', + 'twofa_required', 'user_ids', 'custom_field_values', 'custom_fields', diff --git a/app/models/setting.rb b/app/models/setting.rb index e5c547665..dfa054028 100644 --- a/app/models/setting.rb +++ b/app/models/setting.rb @@ -236,6 +236,14 @@ class Setting < ActiveRecord::Base params end + def self.twofa_required? + twofa == '2' + end + + def self.twofa_optional? + twofa == '1' + 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 cc1841a61..2591f56be 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -407,7 +407,10 @@ class User < Principal end def must_activate_twofa? - Setting.twofa == '2' && !twofa_active? + ( + Setting.twofa_required? || + (Setting.twofa_optional? && groups.any?(&:twofa_required?)) + ) && !twofa_active? end def pref diff --git a/app/views/groups/_form.html.erb b/app/views/groups/_form.html.erb index 9d5b087e1..723b9f203 100644 --- a/app/views/groups/_form.html.erb +++ b/app/views/groups/_form.html.erb @@ -3,6 +3,15 @@

<%= f.text_field :name, :required => true, :size => 60, :disabled => !@group.safe_attribute?('name') %>

+ <% unless @group.builtin? %> +

<%= f.check_box :twofa_required, disabled: !Setting.twofa_optional? %> + <% if Setting.twofa_required? %> + <%= l 'twofa_text_group_required' %> + <% elsif !Setting.twofa_optional? %> + <%= l 'twofa_text_group_disabled' %> + <% end %> +

+ <% end %> <% @group.custom_field_values.each do |value| %>

<%= custom_field_tag_with_label :group, value %>

diff --git a/app/views/settings/_authentication.html.erb b/app/views/settings/_authentication.html.erb index 5522ff5cf..9fd0ef646 100644 --- a/app/views/settings/_authentication.html.erb +++ b/app/views/settings/_authentication.html.erb @@ -34,6 +34,7 @@ [l(:label_required_lower), "2"]] -%> <%= t 'twofa_hint_disabled_html', label: t(:label_disabled) -%>
+ <%= t 'twofa_hint_optional_html', label: t(:label_optional) -%>
<%= t 'twofa_hint_required_html', label: t(:label_required_lower) -%>

diff --git a/config/locales/en.yml b/config/locales/en.yml index 0aeafe516..488a9b4ee 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -408,6 +408,7 @@ en: field_history_default_tab: Issue's history default tab field_unique_id: Unique ID field_toolbar_language_options: Code highlighting toolbar languages + field_twofa_required: Require two factor authentication setting_app_title: Application title setting_welcome_text: Welcome text @@ -1335,6 +1336,7 @@ en: twofa_not_active: "Not activated" twofa_label_code: Code twofa_hint_disabled_html: Setting %{label} will deactivate and unpair two-factor authentication devices for all users. + twofa_hint_optional_html: Setting %{label} will let users set up two-factor authentication at will, unless it is required by one of their groups. twofa_hint_required_html: Setting %{label} will require all users 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 @@ -1359,6 +1361,7 @@ en: twofa_text_backup_codes_hint: Use these codes instead of a one-time password should you not have access to your second factor. Each code can only be used once. It is recommended to print and store them in a safe place. twofa_text_backup_codes_created_at: Backup codes generated %{datetime}. twofa_backup_codes_already_shown: Backup codes cannot be shown again, please generate new backup codes if required. - + twofa_text_group_required: "This setting is only effective when the global two factor authentication setting is set to 'optional'. Currently, two factor authentication is required for all users." + twofa_text_group_disabled: "This setting is only effective when the global two factor authentication setting is set to 'optional'. Currently, two factor authentication is disabled." text_user_destroy_confirmation: "Are you sure you want to delete this user and remove all references to them? This cannot be undone. Often, locking a user instead of deleting them is the better solution. To confirm, please enter their login (%{login}) below." text_project_destroy_enter_identifier: "To confirm, please enter the project's identifier (%{identifier}) below." diff --git a/db/migrate/20210704125704_add_twofa_required_to_groups.rb b/db/migrate/20210704125704_add_twofa_required_to_groups.rb new file mode 100644 index 000000000..91361bc46 --- /dev/null +++ b/db/migrate/20210704125704_add_twofa_required_to_groups.rb @@ -0,0 +1,5 @@ +class AddTwofaRequiredToGroups < ActiveRecord::Migration[6.1] + def change + add_column :users, :twofa_required, :boolean, default: false + end +end diff --git a/test/integration/twofa_test.rb b/test/integration/twofa_test.rb index a787e2770..a25fa2895 100644 --- a/test/integration/twofa_test.rb +++ b/test/integration/twofa_test.rb @@ -24,6 +24,32 @@ class TwofaTest < Redmine::IntegrationTest test "should require twofa setup when configured" do with_settings twofa: "2" do + assert Setting.twofa_required? + log_user('jsmith', 'jsmith') + 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? + + group = Group.all.first + group.update_column :twofa_required, true + group.users << user + user.reload + + with_settings twofa: "0" do + assert_not Setting.twofa_optional? + assert_not Setting.twofa_required? + assert_not user.must_activate_twofa? + end + + with_settings twofa: "1" do + assert Setting.twofa_optional? + assert_not Setting.twofa_required? + assert user.must_activate_twofa? log_user('jsmith', 'jsmith') follow_redirect! assert_redirected_to "/my/twofa/totp/activate/confirm"