From c73a53beae2a2c754d027afe058cd6adfd09e361 Mon Sep 17 00:00:00 2001 From: Jens Kraemer Date: Sat, 17 Aug 2019 12:21:34 +0000 Subject: [PATCH 5/5] per group 2fa requirement - when globally set to optional, 2fa can be required for certain user groups --- app/models/group.rb | 1 + app/models/setting.rb | 8 ++++++ app/models/user.rb | 5 +++- app/views/groups/_form.html.erb | 7 ++++++ app/views/settings/_authentication.html.erb | 1 + config/locales/en.yml | 4 +++ .../20190817093525_add_twofa_required_to_groups.rb | 5 ++++ test/integration/twofa_test.rb | 29 ++++++++++++++++++++++ 8 files changed, 59 insertions(+), 1 deletion(-) create mode 100644 db/migrate/20190817093525_add_twofa_required_to_groups.rb diff --git a/app/models/group.rb b/app/models/group.rb index 994ca49a0..724a6ce15 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -40,6 +40,7 @@ class Group < Principal scope :givable, lambda {where(:type => 'Group')} 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 5a6938490..d3ffa4a47 100644 --- a/app/models/setting.rb +++ b/app/models/setting.rb @@ -211,6 +211,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 7cbb70dc7..12d9304f8 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -382,7 +382,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..0dcdfbe50 100644 --- a/app/views/groups/_form.html.erb +++ b/app/views/groups/_form.html.erb @@ -3,6 +3,13 @@

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

+

<%= 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 %> +

<% @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 a4eef6748..48eed1b30 100644 --- a/app/views/settings/_authentication.html.erb +++ b/app/views/settings/_authentication.html.erb @@ -32,6 +32,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 af7635e1c..904532e39 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1279,6 +1279,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 @@ -1303,3 +1304,6 @@ 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. + field_twofa_required: Require two factor authentication + 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." diff --git a/db/migrate/20190817093525_add_twofa_required_to_groups.rb b/db/migrate/20190817093525_add_twofa_required_to_groups.rb new file mode 100644 index 000000000..efd28f404 --- /dev/null +++ b/db/migrate/20190817093525_add_twofa_required_to_groups.rb @@ -0,0 +1,5 @@ +class AddTwofaRequiredToGroups < ActiveRecord::Migration[5.2] + 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 25e47332c..214fd3033 100644 --- a/test/integration/twofa_test.rb +++ b/test/integration/twofa_test.rb @@ -3,8 +3,37 @@ require File.expand_path('../../test_helper', __FILE__) class TwofaTest < Redmine::IntegrationTest fixtures :projects, :users, :email_addresses + setup do + end + 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' + refute user.must_activate_twofa? + + group = Group.all.first + group.update_column :twofa_required, true + group.users << user + user.reload + + with_settings twofa: "0" do + refute Setting.twofa_optional? + refute Setting.twofa_required? + refute user.must_activate_twofa? + end + + with_settings twofa: "1" do + assert Setting.twofa_optional? + refute Setting.twofa_required? + assert user.must_activate_twofa? log_user('jsmith', 'jsmith') follow_redirect! assert_redirected_to "/my/twofa/totp/activate/confirm" -- 2.11.0