Merge branch 'tancnle/impersonated-audit-events' into 'master'

Record impersonator information on audit event

See merge request gitlab-org/gitlab!30970

app/controllers/application_controller.rb

@@ -18,6 +18,7 @@ class ApplicationController < ActionController::Base
   include Gitlab::Tracking::ControllerConcern
   include Gitlab::Experimentation::ControllerConcern
   include InitializesCurrentUserMode
+  include Impersonation
   include Gitlab::Logging::CloudflareHelper
 
   before_action :authenticate_user!, except: [:route_not_found]
@@ -528,36 +529,6 @@ class ApplicationController < ActionController::Base
       .execute
   end
 
-  def check_impersonation_availability
-    return unless session[:impersonator_id]
-
-    unless Gitlab.config.gitlab.impersonation_enabled
-      stop_impersonation
-      access_denied! _('Impersonation has been disabled')
-    end
-  end
-
-  def stop_impersonation
-    log_impersonation_event
-
-    warden.set_user(impersonator, scope: :user)
-    session[:impersonator_id] = nil
-
-    impersonated_user
-  end
-
-  def impersonated_user
-    current_user
-  end
-
-  def log_impersonation_event
-    Gitlab::AppLogger.info("User #{impersonator.username} has stopped impersonating #{impersonated_user.username}")
-  end
-
-  def impersonator
-    @impersonator ||= User.find(session[:impersonator_id]) if session[:impersonator_id]
-  end
-
   def sentry_context(&block)
     Gitlab::ErrorTracking.with_context(current_user, &block)
   end

app/controllers/concerns/impersonation.rb

+# frozen_string_literal: true
+
+module Impersonation
+  include Gitlab::Utils::StrongMemoize
+
+  def current_user
+    user = super
+
+    user.impersonator = impersonator if impersonator
+
+    user
+  end
+
+  protected
+
+  def check_impersonation_availability
+    return unless session[:impersonator_id]
+
+    unless Gitlab.config.gitlab.impersonation_enabled
+      stop_impersonation
+      access_denied! _('Impersonation has been disabled')
+    end
+  end
+
+  def stop_impersonation
+    log_impersonation_event
+
+    warden.set_user(impersonator, scope: :user)
+    session[:impersonator_id] = nil
+
+    current_user
+  end
+
+  def log_impersonation_event
+    Gitlab::AppLogger.info("User #{impersonator.username} has stopped impersonating #{current_user.username}")
+  end
+
+  def impersonator
+    strong_memoize(:impersonator) do
+      User.find(session[:impersonator_id]) if session[:impersonator_id]
+    end
+  end
+end

app/models/user.rb

@@ -88,6 +88,9 @@ class User < ApplicationRecord
   # Virtual attribute for authenticating by either username or email
   attr_accessor :login
 
+  # Virtual attribute for impersonator
+  attr_accessor :impersonator
+
   #
   # Relations
   #
@@ -1683,6 +1686,10 @@ class User < ApplicationRecord
     !confirmed? && !confirmation_period_valid?
   end
 
+  def impersonated?
+    impersonator.present?
+  end
+
   protected
 
   # override, from Devise::Validatable

app/services/audit_event_service.rb

@@ -50,8 +50,9 @@ class AuditEventService
   private
 
   def build_author(author)
-    if author.is_a?(User)
-      author
+    case author
+    when User
+      author.impersonated? ? Gitlab::Audit::ImpersonatedAuthor.new(author) : author
     else
       Gitlab::Audit::UnauthenticatedAuthor.new(name: author)
     end

doc/administration/audit_events.md

@@ -34,6 +34,12 @@ There are two kinds of events logged:
 - Instance events scoped to the whole GitLab instance, used by your Compliance team to
   perform formal audits.
 
+### Impersonation data **(PREMIUM)**
+
+> [Introduced](https://gitlab.com/gitlab-org/gitlab/issues/536) in [GitLab Premium](https://about.gitlab.com/pricing/) 13.0.
+
+Impersonation is where an administrator uses credentials to perform an action as a different user.
+
 ### Group events **(STARTER)**
 
 NOTE: **Note:**

ee/app/controllers/ee/admin/users_controller.rb

@@ -27,7 +27,8 @@ module EE
       end
 
       def log_audit_event
-        AuditEvents::ImpersonationAuditEventService.new(current_user, request.remote_ip, 'Started Impersonation').for_user(user.username).security_event
+        EE::AuditEvents::ImpersonationAuditEventService.new(current_user, request.remote_ip, 'Started Impersonation')
+          .for_user(user.username).security_event
       end
 
       def allowed_user_params

ee/app/controllers/ee/application_controller.rb

@@ -36,7 +36,8 @@ module EE
     end
 
     def log_audit_event
-      AuditEvents::ImpersonationAuditEventService.new(impersonator, request.remote_ip, 'Stopped Impersonation').for_user(impersonated_user.username).security_event
+      EE::AuditEvents::ImpersonationAuditEventService.new(impersonator, request.remote_ip, 'Stopped Impersonation')
+        .for_user(current_user.username).security_event
     end
 
     def set_current_ip_address(&block)

ee/app/services/ee/audit_event_service.rb

@@ -127,6 +127,7 @@ module EE
     def prepare_security_event
       if admin_audit_log_enabled?
         add_security_event_admin_details!
+        add_impersonation_details!
       end
     end
 
@@ -264,8 +265,16 @@ module EE
     end
 
     def add_security_event_admin_details!
-      @details.merge!(ip_address: ip_address,
-                      entity_path: @entity.full_path)
+      @details.merge!(
+        ip_address: ip_address,
+        entity_path: @entity.full_path
+      )
+    end
+
+    def add_impersonation_details!
+      if @author.is_a?(::Gitlab::Audit::ImpersonatedAuthor)
+        @details.merge!(impersonated_by: @author.impersonated_by)
+      end
     end
 
     def custom_project_link_group_attributes(group_link)

ee/changelogs/unreleased/536-impersonated-audit-events.yml

+---
+title: Record impersonator information on audit event
+merge_request: 30970
+author:
+type: added

ee/lib/audit/details.rb

@@ -17,6 +17,8 @@ module Audit
         "Signed in with #{@details[:with].upcase} authentication"
       elsif event_created_by_system?
         "#{action_text} via system job. Reason: #{@details[:reason]}"
+      elsif impersonated_event?
+        "#{action_text} (by #{@details[:impersonated_by]})"
       else
         action_text
       end
@@ -28,6 +30,10 @@ module Audit
       @details[:system_event]
     end
 
+    def impersonated_event?
+      @details[:impersonated_by].present?
+    end
+
     def action_text
       action_name, action_info = @details.slice(*ACTIONS).first
 

ee/lib/gitlab/audit/impersonated_author.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module Audit
+    class ImpersonatedAuthor
+      def initialize(user)
+        @user = user
+      end
+
+      def id
+        @user.id
+      end
+
+      def name
+        @user.name
+      end
+
+      def current_sign_in_ip
+        impersonator.current_sign_in_ip
+      end
+
+      def impersonated_by
+        impersonator.name
+      end
+
+      private
+
+      def impersonator
+        @user.impersonator
+      end
+    end
+  end
+end

ee/spec/features/admin/admin_audit_logs_spec.rb

@@ -6,9 +6,10 @@ describe 'Admin::AuditLogs', :js do
   include Select2Helper
 
   let(:user) { create(:user) }
+  let(:admin) { create(:admin, name: 'Bruce Wayne') }
 
   before do
-    sign_in(create(:admin))
+    sign_in(admin)
   end
 
   context 'unlicensed' do
@@ -132,6 +133,32 @@ describe 'Admin::AuditLogs', :js do
         expect(page).to have_content('Invalid date format. Please use UTC format as YYYY-MM-DD')
       end
     end
+
+    describe 'impersonated events' do
+      it 'show impersonation details' do
+        visit admin_user_path(user)
+
+        click_link 'Impersonate'
+
+        visit(new_project_path)
+
+        fill_in(:project_name, with: 'Gotham City')
+
+        page.within('#content-body') do
+          click_button('Create project')
+        end
+
+        wait_for('Creation to complete') do
+          page.has_content?('was successfully created', wait: 0)
+        end
+
+        click_link 'Stop impersonation'
+
+        visit admin_audit_logs_path
+
+        expect(page).to have_content('by Bruce Wayne')
+      end
+    end
   end
 
   def filter_for(type, name)

ee/spec/lib/audit/details_spec.rb

@@ -6,6 +6,24 @@ describe Audit::Details do
   let(:user) { create(:user) }
 
   describe '.humanize' do
+    context 'an impersonated event' do
+      let(:impersonated_action) do
+        {
+          add: 'user_access',
+          author_name: user.name,
+          target_id: user.id,
+          target_type: 'User',
+          impersonated_by: 'Agent 47'
+        }
+      end
+
+      it 'includes impersonation details' do
+        string = described_class.humanize(impersonated_action)
+
+        expect(string).to end_with('(by Agent 47)')
+      end
+    end
+
     context 'user' do
       let(:login_action) do
         {

ee/spec/services/audit_event_service_spec.rb

@@ -142,6 +142,23 @@ describe AuditEventService do
           expect(event[:details][:ip_address]).to eq(user.current_sign_in_ip)
         end
       end
+
+      context 'for an impersonated user' do
+        let(:impersonator) { build(:user, name: 'Donald Duck', current_sign_in_ip: '192.168.88.88') }
+        let(:user) { build(:user, impersonator: impersonator) }
+
+        it 'has the impersonator IP address' do
+          event = service.security_event
+
+          expect(event[:details][:ip_address]).to eq('192.168.88.88')
+        end
+
+        it 'has the impersonator name' do
+          event = service.security_event
+
+          expect(event[:details][:impersonated_by]).to eq('Donald Duck')
+        end
+      end
     end
   end
 

spec/controllers/application_controller_spec.rb

@@ -914,4 +914,43 @@ describe ApplicationController do
       expect(json_response['meta.caller_id']).to eq('AnonymousController#index')
     end
   end
+
+  describe '#current_user' do
+    controller(described_class) do
+      def index; end
+    end
+
+    let_it_be(:impersonator) { create(:user) }
+    let_it_be(:user) { create(:user) }
+
+    before do
+      sign_in(user)
+    end
+
+    context 'when being impersonated' do
+      before do
+        allow(controller).to receive(:session).and_return({ impersonator_id: impersonator.id })
+      end
+
+      it 'returns a User with impersonator', :aggregate_failures do
+        get :index
+
+        expect(controller.current_user).to be_a(User)
+        expect(controller.current_user.impersonator).to eq(impersonator)
+      end
+    end
+
+    context 'when not being impersonated' do
+      before do
+        allow(controller).to receive(:session).and_return({})
+      end
+
+      it 'returns a User', :aggregate_failures do
+        get :index
+
+        expect(controller.current_user).to be_a(User)
+        expect(controller.current_user.impersonator).to be_nil
+      end
+    end
+  end
 end