Deny 2fa setup when 2fa already present (#43083).

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

@@ -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

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 <a data-method="post" href="%{bc_path}">generate new backup codes</a> 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."

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')