Prevent double-impersonation and impersonation breakout

When an admin impersonated another admin, it was possible to
impersonate multiple levels deep. The side-effect is when
stopping impersonation at a deeper level the actual user
would then assume the session of the last impersonating user
rather than their own session.

Changelog: security

app/controllers/admin/users_controller.rb

@@ -45,7 +45,7 @@ class Admin::UsersController < Admin::ApplicationController
   end
 
   def impersonate
-    if can?(user, :log_in)
+    if can?(user, :log_in) && !impersonation_in_progress?
       session[:impersonator_id] = current_user.id
 
       warden.set_user(user, scope: :user)
@@ -57,7 +57,9 @@ class Admin::UsersController < Admin::ApplicationController
       redirect_to root_path
     else
       flash[:alert] =
-        if user.blocked?
+        if impersonation_in_progress?
+          _("You are already impersonating another user")
+        elsif user.blocked?
           _("You cannot impersonate a blocked user")
         elsif user.internal?
           _("You cannot impersonate an internal user")

app/controllers/concerns/impersonation.rb

@@ -14,7 +14,7 @@ module Impersonation
   protected
 
   def check_impersonation_availability
-    return unless session[:impersonator_id]
+    return unless impersonation_in_progress?
 
     unless Gitlab.config.gitlab.impersonation_enabled
       stop_impersonation
@@ -31,6 +31,10 @@ module Impersonation
     current_user
   end
 
+  def impersonation_in_progress?
+    session[:impersonator_id].present?
+  end
+
   def log_impersonation_event
     Gitlab::AppLogger.info("User #{impersonator.username} has stopped impersonating #{current_user.username}")
   end

locale/gitlab.pot

@@ -38452,6 +38452,9 @@ msgstr ""
 msgid "You are already a member of this %{member_source}."
 msgstr ""
 
+msgid "You are already impersonating another user"
+msgstr ""
+
 msgid "You are an admin, which means granting access to %{client_name} will allow them to interact with GitLab as an admin as well. Proceed with caution."
 msgstr ""
 

spec/controllers/admin/users_controller_spec.rb

@@ -807,5 +807,20 @@ RSpec.describe Admin::UsersController do
         expect(response).to have_gitlab_http_status(:not_found)
       end
     end
+
+    context 'when impersonating an admin and attempting to impersonate again' do
+      let(:admin2) { create(:admin) }
+
+      before do
+        post :impersonate, params: { id: admin2.username }
+      end
+
+      it 'does not allow double impersonation', :aggregate_failures do
+        post :impersonate, params: { id: user.username }
+
+        expect(flash[:alert]).to eq(_('You are already impersonating another user'))
+        expect(warden.user).to eq(admin2)
+      end
+    end
   end
 end