From 17b3cb6c42f11ee3ef911fac3474987a236d7614 Mon Sep 17 00:00:00 2001
From: manojmj <mmj@gitlab.com>
Date: Thu, 18 Jul 2019 11:56:43 +0530
Subject: [PATCH] Log impersonation actions in audit log

This change adds audit logs for user impersonation
when an admin starts/stops impersonating
another user.
---
 app/controllers/admin/users_controller.rb     |  6 ++-
 app/controllers/application_controller.rb     | 12 ++++--
 doc/administration/audit_events.md            |  1 +
 .../controllers/ee/admin/users_controller.rb  | 13 +++++++
 .../controllers/ee/application_controller.rb  | 11 ++++++
 .../impersonation_audit_event_service.rb      | 15 ++++++++
 ...log-impersonation-actions-in-audit-log.yml |  5 +++
 .../admin/impersonations_controller_spec.rb   | 27 ++++++++++++++
 .../admin/users_controller_spec.rb            | 10 +++++
 .../impersonation_audit_event_service_spec.rb | 37 +++++++++++++++++++
 .../admin/users_controller_spec.rb            |  6 +++
 11 files changed, 139 insertions(+), 4 deletions(-)
 create mode 100644 ee/app/services/ee/audit_events/impersonation_audit_event_service.rb
 create mode 100644 ee/changelogs/unreleased/315-log-impersonation-actions-in-audit-log.yml
 create mode 100644 ee/spec/controllers/admin/impersonations_controller_spec.rb
 create mode 100644 ee/spec/services/ee/audit_events/impersonation_audit_event_service_spec.rb

diff --git a/app/controllers/admin/users_controller.rb b/app/controllers/admin/users_controller.rb
index ad7f614ba10..b261fb50a06 100644
--- a/app/controllers/admin/users_controller.rb
+++ b/app/controllers/admin/users_controller.rb
@@ -39,7 +39,7 @@ class Admin::UsersController < Admin::ApplicationController
 
       warden.set_user(user, scope: :user)
 
-      Gitlab::AppLogger.info(_("User %{current_user_username} has started impersonating %{username}") % { current_user_username: current_user.username, username: user.username })
+      log_impersonation_event
 
       flash[:alert] = _("You are now impersonating %{username}") % { username: user.username }
 
@@ -236,6 +236,10 @@ class Admin::UsersController < Admin::ApplicationController
   def check_impersonation_availability
     access_denied! unless Gitlab.config.gitlab.impersonation_enabled
   end
+
+  def log_impersonation_event
+    Gitlab::AppLogger.info(_("User %{current_user_username} has started impersonating %{username}") % { current_user_username: current_user.username, username: user.username })
+  end
 end
 
 Admin::UsersController.prepend(EE::Admin::UsersController)
diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index 33e087b8439..9534ad56861 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -499,9 +499,7 @@ class ApplicationController < ActionController::Base
   end
 
   def stop_impersonation
-    impersonated_user = current_user
-
-    Gitlab::AppLogger.info("User #{impersonator.username} has stopped impersonating #{impersonated_user.username}")
+    log_impersonation_event
 
     warden.set_user(impersonator, scope: :user)
     session[:impersonator_id] = nil
@@ -509,6 +507,14 @@ class ApplicationController < ActionController::Base
     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
diff --git a/doc/administration/audit_events.md b/doc/administration/audit_events.md
index a80ff330e03..aaa43f67760 100644
--- a/doc/administration/audit_events.md
+++ b/doc/administration/audit_events.md
@@ -94,6 +94,7 @@ recorded:
 - Changed password
 - Ask for password reset
 - Grant OAuth access
+- Started/stopped user impersonation
 
 It is possible to filter particular actions by choosing an audit data type from
 the filter drop-down. You can further filter by specific group, project or user
diff --git a/ee/app/controllers/ee/admin/users_controller.rb b/ee/app/controllers/ee/admin/users_controller.rb
index 61420d7103a..f7ff1e43d6b 100644
--- a/ee/app/controllers/ee/admin/users_controller.rb
+++ b/ee/app/controllers/ee/admin/users_controller.rb
@@ -4,6 +4,8 @@
 module EE
   module Admin
     module UsersController
+      extend ::Gitlab::Utils::Override
+
       def reset_runners_minutes
         user
 
@@ -17,6 +19,17 @@ module EE
 
       private
 
+      override :log_impersonation_event
+      def log_impersonation_event
+        super
+
+        log_audit_event
+      end
+
+      def log_audit_event
+        AuditEvents::ImpersonationAuditEventService.new(current_user, request.remote_ip, 'Started Impersonation').for_user(user.username).security_event
+      end
+
       def allowed_user_params
         super + [
           :note,
diff --git a/ee/app/controllers/ee/application_controller.rb b/ee/app/controllers/ee/application_controller.rb
index b996a5c5870..b875be78759 100644
--- a/ee/app/controllers/ee/application_controller.rb
+++ b/ee/app/controllers/ee/application_controller.rb
@@ -24,6 +24,17 @@ module EE
 
     private
 
+    override :log_impersonation_event
+    def log_impersonation_event
+      super
+
+      log_audit_event
+    end
+
+    def log_audit_event
+      AuditEvents::ImpersonationAuditEventService.new(impersonator, request.remote_ip, 'Stopped Impersonation').for_user(impersonated_user.username).security_event
+    end
+
     def set_current_ip_address(&block)
       ::Gitlab::IpAddressState.with(request.ip, &block) # rubocop: disable CodeReuse/ActiveRecord
     end
diff --git a/ee/app/services/ee/audit_events/impersonation_audit_event_service.rb b/ee/app/services/ee/audit_events/impersonation_audit_event_service.rb
new file mode 100644
index 00000000000..c59391f110a
--- /dev/null
+++ b/ee/app/services/ee/audit_events/impersonation_audit_event_service.rb
@@ -0,0 +1,15 @@
+# frozen_string_literal: true
+
+module EE
+  module AuditEvents
+    class ImpersonationAuditEventService < ::AuditEventService
+      def initialize(author, ip_address, message)
+        super(author, author, {
+          action: :custom,
+          custom_message: message,
+          ip_address: ip_address
+        })
+      end
+    end
+  end
+end
diff --git a/ee/changelogs/unreleased/315-log-impersonation-actions-in-audit-log.yml b/ee/changelogs/unreleased/315-log-impersonation-actions-in-audit-log.yml
new file mode 100644
index 00000000000..502bdee705b
--- /dev/null
+++ b/ee/changelogs/unreleased/315-log-impersonation-actions-in-audit-log.yml
@@ -0,0 +1,5 @@
+---
+title: Log impersonation actions in audit log
+merge_request: 14740
+author:
+type: added
diff --git a/ee/spec/controllers/admin/impersonations_controller_spec.rb b/ee/spec/controllers/admin/impersonations_controller_spec.rb
new file mode 100644
index 00000000000..2fea0aa3c78
--- /dev/null
+++ b/ee/spec/controllers/admin/impersonations_controller_spec.rb
@@ -0,0 +1,27 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Admin::ImpersonationsController do
+  let(:impersonator) { create(:admin) }
+  let(:user) { create(:user) }
+
+  describe "DELETE destroy" do
+    context "when signed in" do
+      before do
+        sign_in(user)
+      end
+
+      context "when impersonating" do
+        before do
+          session[:impersonator_id] = impersonator.id
+          stub_licensed_features(extended_audit_events: true)
+        end
+
+        it 'creates an audit log record' do
+          expect { delete :destroy }.to change { SecurityEvent.count }.by(1)
+        end
+      end
+    end
+  end
+end
diff --git a/ee/spec/controllers/admin/users_controller_spec.rb b/ee/spec/controllers/admin/users_controller_spec.rb
index 88cc8af977b..fa934f81eb4 100644
--- a/ee/spec/controllers/admin/users_controller_spec.rb
+++ b/ee/spec/controllers/admin/users_controller_spec.rb
@@ -38,4 +38,14 @@ describe Admin::UsersController do
       end
     end
   end
+
+  describe "POST #impersonate" do
+    before do
+      stub_licensed_features(extended_audit_events: true)
+    end
+
+    it 'creates an audit log record' do
+      expect { post :impersonate, params: { id: user.username } }.to change { SecurityEvent.count }.by(1)
+    end
+  end
 end
diff --git a/ee/spec/services/ee/audit_events/impersonation_audit_event_service_spec.rb b/ee/spec/services/ee/audit_events/impersonation_audit_event_service_spec.rb
new file mode 100644
index 00000000000..65ed5ffad33
--- /dev/null
+++ b/ee/spec/services/ee/audit_events/impersonation_audit_event_service_spec.rb
@@ -0,0 +1,37 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe EE::AuditEvents::ImpersonationAuditEventService do
+  let(:impersonator) { create(:user) }
+  let(:ip_address) { '127.0.0.1' }
+  let(:message) { 'Impersonation Started' }
+  let(:logger) { instance_double(Gitlab::AuditJsonLogger) }
+  let(:service) { described_class.new(impersonator, ip_address, message) }
+
+  describe '#security_event' do
+    before do
+      stub_licensed_features(extended_audit_events: true)
+    end
+
+    it 'creates an event and logs to a file with the provided details' do
+      expect(service).to receive(:file_logger).and_return(logger)
+      expect(logger).to receive(:info).with(author_id: impersonator.id,
+                                            entity_id: impersonator.id,
+                                            entity_type: "User",
+                                            action: :custom,
+                                            ip_address: ip_address,
+                                            custom_message: message)
+
+      expect { service.security_event }.to change(SecurityEvent, :count).by(1)
+      security_event = SecurityEvent.last
+
+      expect(security_event.details).to eq(custom_message: message,
+                                               ip_address: ip_address,
+                                               action: :custom)
+      expect(security_event.author_id).to eq(impersonator.id)
+      expect(security_event.entity_id).to eq(impersonator.id)
+      expect(security_event.entity_type).to eq('User')
+    end
+  end
+end
diff --git a/spec/controllers/admin/users_controller_spec.rb b/spec/controllers/admin/users_controller_spec.rb
index 89a0eba66f7..d7428f8b52c 100644
--- a/spec/controllers/admin/users_controller_spec.rb
+++ b/spec/controllers/admin/users_controller_spec.rb
@@ -279,6 +279,12 @@ describe Admin::UsersController do
         expect(warden.user).to eq(user)
       end
 
+      it 'logs the beginning of the impersonation event' do
+        expect(Gitlab::AppLogger).to receive(:info).with("User #{admin.username} has started impersonating #{user.username}").and_call_original
+
+        post :impersonate, params: { id: user.username }
+      end
+
       it "redirects to root" do
         post :impersonate, params: { id: user.username }
 
-- 
2.30.9