From 8f002c297e54e8e23d810a911505d0184c8d4bcd Mon Sep 17 00:00:00 2001 From: Marius Balteanu Date: Wed, 13 Aug 2025 05:57:21 +0000 Subject: [PATCH] Deny 2fa setup when 2fa already present (#43083). MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Patch by Felix Schäfer (user:felix). git-svn-id: https://svn.redmine.org/redmine/trunk@23916 e93f8b46-1217-0410-a6f0-8f06a7374b81 --- app/controllers/twofa_controller.rb | 11 +++++++++++ config/locales/en.yml | 1 + test/integration/twofa_test.rb | 20 ++++++++++++++++++++ 3 files changed, 32 insertions(+) diff --git a/app/controllers/twofa_controller.rb b/app/controllers/twofa_controller.rb index 446d2f105..347c16d6e 100644 --- a/app/controllers/twofa_controller.rb +++ b/app/controllers/twofa_controller.rb @@ -31,6 +31,8 @@ class TwofaController < ApplicationController skip_before_action :check_twofa_activation, only: [:select_scheme, :activate_init, :activate_confirm, :activate] + before_action :ensure_user_has_no_twofa, only: [:select_scheme, :activate_init, :activate_confirm, :activate] + def select_scheme @user = User.current end @@ -114,4 +116,13 @@ class TwofaController < ApplicationController redirect_to my_account_path end end + + def ensure_user_has_no_twofa + # Allow activating a new 2FA scheme / showing twofa secret only if no other + # is already configured + return true if User.current.twofa_scheme.blank? + + flash[:warning] = l('twofa_already_setup') + redirect_to controller: 'my', action: 'account' + end end diff --git a/config/locales/en.yml b/config/locales/en.yml index ef7f17e2f..c114634a3 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1446,6 +1446,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_already_setup: Two-factor authentication already set up 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." diff --git a/test/integration/twofa_test.rb b/test/integration/twofa_test.rb index 7aa8ec4ac..3cdc355ff 100644 --- a/test/integration/twofa_test.rb +++ b/test/integration/twofa_test.rb @@ -213,6 +213,26 @@ class TwofaTest < Redmine::IntegrationTest end end + test "should deny showing twofa information again" do + log_user('jsmith', 'jsmith') + get "/my/account" + assert_response :success + post "/my/twofa/totp/activate/init" + assert_redirected_to "/my/twofa/totp/activate/confirm" + follow_redirect! + assert_response :success + + totp = ROTP::TOTP.new User.find_by_login('jsmith').twofa_totp_key + post "/my/twofa/totp/activate", params: {twofa_code: totp.now} + assert_redirected_to "/my/account" + follow_redirect! + assert_response :success + assert_select '.flash', /Two-factor authentication successfully enabled/i + + get "/my/twofa/totp/activate/confirm" + assert_redirected_to "/my/account" + end + def test_enable_twofa_should_destroy_tokens recovery_token = Token.create!(:user_id => 2, :action => 'recovery') autologin_token = Token.create!(:user_id => 2, :action => 'autologin')