Feature #34070 » 0001-Allow-setting-a-grace-periond-before-requiring-two-f.patch
| app/controllers/account_controller.rb | ||
|---|---|---|
| 336 | 336 | set_autologin_cookie(user) | 
| 337 | 337 | end | 
| 338 | 338 |     call_hook(:controller_account_success_authentication_after, {:user => user}) | 
| 339 | redirect_back_or_default my_page_path | |
| 339 | ||
| 340 | if Setting.twofa_required? && !user.twofa_grace_period_expired? | |
| 341 | flash[:warning] = l(:twofa_warning_require) + " " + l(:twofa_text_grace_period, :datetime => format_time(user.twofa_grace_period_expiration_date)) | |
| 342 | redirect_to controller: 'twofa', action: 'select_scheme' | |
| 343 | else | |
| 344 | redirect_back_or_default my_page_path | |
| 345 | end | |
| 340 | 346 | end | 
| 341 | 347 | |
| 342 | 348 | def set_autologin_cookie(user) | 
| app/models/setting.rb | ||
|---|---|---|
| 247 | 247 | twofa == '1' | 
| 248 | 248 | end | 
| 249 | 249 | |
| 250 | def self.twofa_grace_period_expiration_date | |
| 251 | Setting.where(name: :twofa).pick(:updated_on) + Setting.twofa_grace_period.to_i * 3600 | |
| 252 | end | |
| 253 | ||
| 250 | 254 | # Helper that returns an array based on per_page_options setting | 
| 251 | 255 | def self.per_page_options_array | 
| 252 | 256 |     per_page_options.split(%r{[\s,]}).collect(&:to_i).select {|n| n > 0}.sort | 
| app/models/user.rb | ||
|---|---|---|
| 385 | 385 | |
| 386 | 386 | def must_activate_twofa? | 
| 387 | 387 | ( | 
| 388 |       Setting.twofa_required? || | |
| 388 |       (Setting.twofa_required? && self.twofa_grace_period_expired?) || | |
| 389 | 389 | ( | 
| 390 | 390 | Setting.twofa_optional? && ( | 
| 391 | 391 | groups.any?(&:twofa_required?) || | 
| ... | ... | |
| 395 | 395 | ) && !twofa_active? | 
| 396 | 396 | end | 
| 397 | 397 | |
| 398 | def twofa_grace_period_expiration_date | |
| 399 | convert_time_to_user_timezone(Setting.twofa_grace_period_expiration_date) | |
| 400 | end | |
| 401 | ||
| 402 | def twofa_grace_period_expired? | |
| 403 | Setting.twofa_required? && (twofa_grace_period_expiration_date < convert_time_to_user_timezone(DateTime.current)) | |
| 404 | end | |
| 405 | ||
| 398 | 406 | def pref | 
| 399 | 407 | self.preference ||= UserPreference.new(:user => self) | 
| 400 | 408 | end | 
| app/views/settings/_authentication.html.erb | ||
|---|---|---|
| 43 | 43 | <%= l(:setting_twofa_required_for_administrators) %> | 
| 44 | 44 | </label> | 
| 45 | 45 | </span> | 
| 46 | ||
| 47 | <span id="twofa_required" class="<%= "hidden" unless Setting.twofa == "2" %>"> | |
| 48 | <label class="block"> | |
| 49 | <%= l(:setting_twofa_grace_period) %> | |
| 50 | <%= setting_text_field :twofa_grace_period, label: false, type: 'number' %> | |
| 51 | <%= l(:field_hours) %> | |
| 52 | </label> | |
| 53 | <em class="info"> | |
| 54 | <%= t(:twofa_hint_grace_period_setting) -%> | |
| 55 | </em> | |
| 56 | </span> | |
| 46 | 57 | </p> | 
| 47 | 58 | |
| 48 | 59 | </div> | 
| ... | ... | |
| 64 | 75 | <%= javascript_tag do %> | 
| 65 | 76 |   $('#settings_twofa').on('change', function(e){ | 
| 66 | 77 | const twofa = e.target.value; | 
| 67 |     const parent_block = document.getElementById("twofa_optional"); | |
| 78 |     const optional = document.getElementById("twofa_optional"); | |
| 79 |     const required = document.getElementById("twofa_required"); | |
| 68 | 80 | |
| 69 | 81 |     if (twofa == "1") { | 
| 70 |       parent_block.classList.remove('hidden'); | |
| 82 |       optional.classList.remove('hidden'); | |
| 83 |       required.classList.add('hidden'); | |
| 84 |     } else if (twofa == "2") { | |
| 85 |       optional.classList.add('hidden'); | |
| 86 |       required.classList.remove('hidden'); | |
| 71 | 87 |     } else { | 
| 72 |       parent_block.classList.add('hidden'); | |
| 88 |       optional.classList.add('hidden'); | |
| 89 |       required.classList.add('hidden'); | |
| 73 | 90 | } | 
| 74 | 91 | }); | 
| 75 | 92 | <% end %> | 
| config/locales/en.yml | ||
|---|---|---|
| 509 | 509 | setting_project_list_defaults: Projects list defaults | 
| 510 | 510 | setting_twofa: Two-factor authentication | 
| 511 | 511 | setting_twofa_required_for_administrators: Require two-factor authentication for administrators | 
| 512 | setting_twofa_grace_period: Grace period | |
| 512 | 513 | |
| 513 | 514 | permission_add_project: Create project | 
| 514 | 515 | permission_add_subprojects: Create subprojects | 
| ... | ... | |
| 1375 | 1376 |   twofa_backup_codes_already_shown: Backup codes cannot be shown again, please <a data-method="post" href="%{bc_path}">generate new backup codes</a> if required. | 
| 1376 | 1377 | 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." | 
| 1377 | 1378 | 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." | 
| 1379 |   twofa_text_grace_period: "You need to do it before <b>%{datetime}</b>." | |
| 1380 | twofa_hint_grace_period_setting: Maximum time (in hours) that users are allowed to skip enablig two-factor authentication. Set to 0 (zero) to require at next sign in. | |
| 1381 | ||
| 1378 | 1382 |   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." | 
| 1379 | 1383 |   text_project_destroy_enter_identifier: "To confirm, please enter the project's identifier (%{identifier}) below." | 
| config/settings.yml | ||
|---|---|---|
| 40 | 40 | twofa_required_for_administrators: | 
| 41 | 41 | default: 0 | 
| 42 | 42 | security_notifications: 1 | 
| 43 | twofa_grace_period: | |
| 44 | format: int | |
| 45 | default: 0 | |
| 46 | security_notifications: 1 | |
| 43 | 47 | unsubscribe: | 
| 44 | 48 | default: 1 | 
| 45 | 49 | password_required_char_classes: | 
| test/integration/twofa_test.rb | ||
|---|---|---|
| 20 | 20 | require File.expand_path('../../test_helper', __FILE__) | 
| 21 | 21 | |
| 22 | 22 | class TwofaTest < Redmine::IntegrationTest | 
| 23 | include Redmine::I18n | |
| 23 | 24 | fixtures :projects, :users, :email_addresses | 
| 24 | 25 | |
| 25 | 26 | test "should require twofa setup when configured" do | 
| ... | ... | |
| 76 | 77 | end | 
| 77 | 78 | end | 
| 78 | 79 | |
| 80 | test "should require twofa when grace period expired" do | |
| 81 | user = User.find_by_login 'jsmith' | |
| 82 | assert_not user.must_activate_twofa? | |
| 83 | ||
| 84 | with_settings twofa: "1", twofa_grace_period: "0" do | |
| 85 | assert Setting.twofa_optional? | |
| 86 | assert_not Setting.twofa_required? | |
| 87 | assert_not user.must_activate_twofa? | |
| 88 | end | |
| 89 | ||
| 90 | with_settings twofa: "2", twofa_grace_period: "1" do | |
| 91 | # Override last updated on value | |
| 92 | datetime = Time.gm(2021, 01, 20, 16, 30).utc # 2021-01-20 16:30 UTC | |
| 93 | Setting.where(name: :twofa).first.update(updated_on: datetime) | |
| 94 | ||
| 95 | assert user.must_activate_twofa? | |
| 96 |       log_user('jsmith', 'jsmith') | |
| 97 | follow_redirect! | |
| 98 | assert_redirected_to "/my/twofa/totp/activate/confirm" | |
| 99 | end | |
| 100 | end | |
| 101 | ||
| 102 | test "should not require twofa when grace period is not expired" do | |
| 103 | user = User.find_by_login 'jsmith' | |
| 104 | assert_not user.must_activate_twofa? | |
| 105 | ||
| 106 | time = Time.current | |
| 107 | with_settings twofa: "2", twofa_grace_period: "1" do | |
| 108 | Setting.where(name: :twofa).first.update(updated_on: time) | |
| 109 | assert_not user.must_activate_twofa? | |
| 110 |       log_user('jsmith', 'jsmith') | |
| 111 | ||
| 112 | # User should be redirected to two-factor authentication setup page | |
| 113 | assert_response :redirect | |
| 114 | assert_redirected_to "/my/twofa/select_scheme" | |
| 115 | ||
| 116 | follow_redirect! | |
| 117 | # Assert flash message that informs the user about the grace period | |
| 118 |       assert_select "div.flash.warning", text: "The administrator requires you to enable two-factor authentication. You need to do it before #{format_time(time + 3600)}." | |
| 119 | ||
| 120 | # Assert user can leave the two-fatctor authentication setup page | |
| 121 | get "/my/page" | |
| 122 | assert_response :success | |
| 123 | assert_select "h2", "My page" | |
| 124 | end | |
| 125 | end | |
| 126 | ||
| 127 | ||
| 79 | 128 | test 'should require to change password first when must_change_passwd is true' do | 
| 80 | 129 | User.find_by(login: 'jsmith').update_attribute(:must_change_passwd, true) | 
| 81 | 130 | with_settings twofa: '2' do | 
| test/unit/setting_test.rb | ||
|---|---|---|
| 145 | 145 | end | 
| 146 | 146 | end | 
| 147 | 147 | end | 
| 148 | ||
| 149 | def test_twofa_grace_period_expiration_date_should_take_into_account_twofa_grace_period_in_hours | |
| 150 | datetime = Time.gm(2021, 01, 20, 16, 30).utc # 2021-01-20 16:30 UTC | |
| 151 | Setting.where(name: :twofa).first.update(updated_on: datetime) | |
| 152 | ||
| 153 | with_settings twofa_grace_period: "4" do | |
| 154 | assert_equal datetime + (4 * 3600) , Setting.twofa_grace_period_expiration_date | |
| 155 | end | |
| 156 | end | |
| 148 | 157 | end | 
| test/unit/user_test.rb | ||
|---|---|---|
| 1348 | 1348 | cv2a.reload | 
| 1349 | 1349 | assert_equal @dlopper.id.to_s, cv2a.value | 
| 1350 | 1350 | end | 
| 1351 | ||
| 1352 | def test_user_twofa_grace_period_expiration_date_should_return_the_expiration_datetime_according_to_user_time_zone | |
| 1353 | datetime = Time.gm(2021, 01, 20, 16, 30).utc # 2021-01-20 16:30 UTC | |
| 1354 | Setting.where(name: :twofa).first.update(updated_on: datetime) | |
| 1355 | ||
| 1356 | preference = User.find(1).pref | |
| 1357 | preference.update_attribute :time_zone, 'Bucharest' # UTC+2 | |
| 1358 | ||
| 1359 | with_settings twofa_grace_period: "4" do | |
| 1360 | # time + 4 hours (grace period) + 2 hours (time zone difference) | |
| 1361 | assert_equal Time.new(2021, 01, 20, 22, 30).to_s, User.find(1).twofa_grace_period_expiration_date.to_s | |
| 1362 | end | |
| 1363 | end | |
| 1364 | ||
| 1365 | def test_twofa_grace_period_expired_should_retun_true | |
| 1366 | datetime = Time.gm(2021, 01, 20, 16, 30).utc # 2021-01-20 16:30 UTC | |
| 1367 | Setting.where(name: :twofa).first.update(updated_on: datetime) | |
| 1368 | ||
| 1369 | with_settings twofa_grace_period: "0", twofa: "2" do | |
| 1370 | assert User.find(1).twofa_grace_period_expired? | |
| 1371 | end | |
| 1372 | end | |
| 1373 | ||
| 1374 | ||
| 1375 | def test_twofa_grace_period_expired_should_retun_false_if_twofa_is_not_required | |
| 1376 | datetime = Time.gm(2021, 01, 20, 16, 30).utc # 2021-01-20 16:30 UTC | |
| 1377 | Setting.where(name: :twofa).first.update(updated_on: datetime) | |
| 1378 | ||
| 1379 | with_settings twofa_grace_period: "0" do | |
| 1380 | assert_not User.find(1).twofa_grace_period_expired? | |
| 1381 | end | |
| 1382 | end | |
| 1351 | 1383 | end |