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 |