Merge branch 'fix-impersonation-button-availability' into 'master'

Fix impersonation button availability in admin area

See merge request https://gitlab.com/gitlab-org/gitlab/-/merge_requests/126616



Merged-by: Jessie Young <jessieyoung@gitlab.com>
Approved-by: Ravi Kumar <rkumar@gitlab.com>
Approved-by: Terri Chu <tchu@gitlab.com>
Approved-by: Jessie Young <jessieyoung@gitlab.com>
Reviewed-by: Niklas <mc.taucher2003@gmail.com>
Co-authored-by: Ravi Kumar <rkumar@gitlab.com>
Co-authored-by: Niklas <mc.taucher2003@gmail.com>

app/controllers/admin/identities_controller.rb

@@ -23,6 +23,8 @@ def create
 
   def index
     @identities = @user.identities
+    @can_impersonate = helpers.can_impersonate_user(user, impersonation_in_progress?)
+    @impersonation_error_text = @can_impersonate ? nil : helpers.impersonation_error_text(user, impersonation_in_progress?)
   end
 
   def edit

app/controllers/admin/impersonation_tokens_controller.rb

@@ -8,6 +8,8 @@ class Admin::ImpersonationTokensController < Admin::ApplicationController
 
   def index
     set_index_vars
+    @can_impersonate = helpers.can_impersonate_user(user, impersonation_in_progress?)
+    @impersonation_error_text = @can_impersonate ? nil : helpers.impersonation_error_text(user, impersonation_in_progress?)
   end
 
   def create

app/controllers/admin/users_controller.rb

@@ -7,6 +7,7 @@ class Admin::UsersController < Admin::ApplicationController
   before_action :user, except: [:index, :new, :create]
   before_action :check_impersonation_availability, only: :impersonate
   before_action :ensure_destroy_prerequisites_met, only: [:destroy]
+  before_action :set_shared_view_parameters, only: [:show, :projects, :keys]
 
   feature_category :user_management
 
@@ -24,10 +25,7 @@ def index
     @users = @users.without_count if paginate_without_count?
   end
 
-  def show
-    @can_impersonate = can_impersonate_user
-    @impersonation_error_text = @can_impersonate ? nil : impersonation_error_text
-  end
+  def show; end
 
   # rubocop: disable CodeReuse/ActiveRecord
   def projects
@@ -48,7 +46,7 @@ def edit
   end
 
   def impersonate
-    if can_impersonate_user
+    if helpers.can_impersonate_user(user, impersonation_in_progress?)
       session[:impersonator_id] = current_user.id
 
       warden.set_user(user, scope: :user)
@@ -60,7 +58,7 @@ def impersonate
 
       redirect_to root_path
     else
-      flash[:alert] = impersonation_error_text
+      flash[:alert] = helpers.impersonation_error_text(user, impersonation_in_progress?)
 
       redirect_to admin_user_path(user)
     end
@@ -384,28 +382,17 @@ def log_impersonation_event
     Gitlab::AppLogger.info(format(_("User %{current_user_username} has started impersonating %{username}"), current_user_username: current_user.username, username: user.username))
   end
 
-  def can_impersonate_user
-    can?(user, :log_in) && !user.password_expired? && !impersonation_in_progress?
-  end
-
-  def impersonation_error_text
-    if impersonation_in_progress?
-      _("You are already impersonating another user")
-    elsif user.blocked?
-      _("You cannot impersonate a blocked user")
-    elsif user.password_expired?
-      _("You cannot impersonate a user with an expired password")
-    elsif user.internal?
-      _("You cannot impersonate an internal user")
-    else
-      _("You cannot impersonate a user who cannot log in")
-    end
-  end
-
   # method overriden in EE
   def unlock_user
     update_user(&:unlock_access!)
   end
+
+  private
+
+  def set_shared_view_parameters
+    @can_impersonate = helpers.can_impersonate_user(user, impersonation_in_progress?)
+    @impersonation_error_text = @can_impersonate ? nil : helpers.impersonation_error_text(user, impersonation_in_progress?)
+  end
 end
 
 Admin::UsersController.prepend_mod_with('Admin::UsersController')

app/helpers/users_helper.rb

@@ -104,6 +104,24 @@ def impersonation_enabled?
     Gitlab.config.gitlab.impersonation_enabled
   end
 
+  def can_impersonate_user(user, impersonation_in_progress)
+    can?(user, :log_in) && !user.password_expired? && !impersonation_in_progress
+  end
+
+  def impersonation_error_text(user, impersonation_in_progress)
+    if impersonation_in_progress
+      _("You are already impersonating another user")
+    elsif user.blocked?
+      _("You cannot impersonate a blocked user")
+    elsif user.password_expired?
+      _("You cannot impersonate a user with an expired password")
+    elsif user.internal?
+      _("You cannot impersonate an internal user")
+    else
+      _("You cannot impersonate a user who cannot log in")
+    end
+  end
+
   def user_badges_in_admin_section(user)
     [].tap do |badges|
       badges << blocked_user_badge(user) if user.blocked?

spec/helpers/users_helper_spec.rb

@@ -150,6 +150,76 @@ def filter_ee_badges(badges)
     end
   end
 
+  describe '#can_impersonate_user' do
+    let(:user) { create(:user) }
+    let(:impersonation_in_progress) { false }
+
+    subject { helper.can_impersonate_user(user, impersonation_in_progress) }
+
+    context 'when password is expired' do
+      let(:user) { create(:user, password_expires_at: 1.minute.ago) }
+
+      it { is_expected.to be false }
+    end
+
+    context 'when impersonation is in progress' do
+      let(:impersonation_in_progress) { true }
+
+      it { is_expected.to be false }
+    end
+
+    context 'when user is blocked' do
+      let(:user) { create(:user, :blocked) }
+
+      it { is_expected.to be false }
+    end
+
+    context 'when user is internal' do
+      let(:user) { create(:user, :bot) }
+
+      it { is_expected.to be false }
+    end
+
+    it { is_expected.to be true }
+  end
+
+  describe '#impersonation_error_text' do
+    let(:user) { create(:user) }
+    let(:impersonation_in_progress) { false }
+
+    subject { helper.impersonation_error_text(user, impersonation_in_progress) }
+
+    context 'when password is expired' do
+      let(:user) { create(:user, password_expires_at: 1.minute.ago) }
+
+      it { is_expected.to eq(_("You cannot impersonate a user with an expired password")) }
+    end
+
+    context 'when impersonation is in progress' do
+      let(:impersonation_in_progress) { true }
+
+      it { is_expected.to eq(_("You are already impersonating another user")) }
+    end
+
+    context 'when user is blocked' do
+      let(:user) { create(:user, :blocked) }
+
+      it { is_expected.to eq(_("You cannot impersonate a blocked user")) }
+    end
+
+    context 'when user is internal' do
+      let(:user) { create(:user, :bot) }
+
+      it { is_expected.to eq(_("You cannot impersonate an internal user")) }
+    end
+
+    context 'when user is inactive' do
+      let(:user) { create(:user, :deactivated) }
+
+      it { is_expected.to eq(_("You cannot impersonate a user who cannot log in")) }
+    end
+  end
+
   describe '#user_badges_in_admin_section' do
     before do
       allow(helper).to receive(:current_user).and_return(user)