Merge branch 'security_dblessing_access_token_impersonation_leak' into 'master'

Clear session access tokens when starting/stopping impersonation

See merge request gitlab-org/security/gitlab!1712

app/controllers/admin/users_controller.rb

@@ -49,6 +49,7 @@ class Admin::UsersController < Admin::ApplicationController
       session[:impersonator_id] = current_user.id
 
       warden.set_user(user, scope: :user)
+      clear_access_token_session_keys!
 
       log_impersonation_event
 

app/controllers/concerns/impersonation.rb

@@ -3,6 +3,12 @@
 module Impersonation
   include Gitlab::Utils::StrongMemoize
 
+  SESSION_KEYS_TO_DELETE = %w(
+    github_access_token gitea_access_token gitlab_access_token
+    bitbucket_token bitbucket_refresh_token bitbucket_server_personal_access_token
+    bulk_import_gitlab_access_token fogbugz_token
+  ).freeze
+
   def current_user
     user = super
 
@@ -27,6 +33,7 @@ module Impersonation
 
     warden.set_user(impersonator, scope: :user)
     session[:impersonator_id] = nil
+    clear_access_token_session_keys!
 
     current_user
   end
@@ -35,6 +42,12 @@ module Impersonation
     Gitlab::AppLogger.info("User #{impersonator.username} has stopped impersonating #{current_user.username}")
   end
 
+  def clear_access_token_session_keys!
+    access_tokens_keys = session.keys & SESSION_KEYS_TO_DELETE
+
+    access_tokens_keys.each { |key| session.delete(key) }
+  end
+
   def impersonator
     strong_memoize(:impersonator) do
       User.find(session[:impersonator_id]) if session[:impersonator_id]

spec/controllers/admin/impersonations_controller_spec.rb

@@ -92,6 +92,14 @@ RSpec.describe Admin::ImpersonationsController do
 
                 expect(warden.user).to eq(impersonator)
               end
+
+              it 'clears token session keys' do
+                session[:bitbucket_token] = SecureRandom.hex(8)
+
+                delete :destroy
+
+                expect(session[:bitbucket_token]).to be_nil
+              end
             end
 
             # base case

spec/controllers/admin/users_controller_spec.rb

@@ -794,6 +794,14 @@ RSpec.describe Admin::UsersController do
 
         expect(flash[:alert]).to eq("You are now impersonating #{user.username}")
       end
+
+      it 'clears token session keys' do
+        session[:github_access_token] = SecureRandom.hex(8)
+
+        post :impersonate, params: { id: user.username }
+
+        expect(session[:github_access_token]).to be_nil
+      end
     end
 
     context "when impersonation is disabled" do