Merge branch '251-record-user-block-audit-event' into 'master'

Record audit event when user is blocked

See merge request gitlab-org/gitlab!24930

app/controllers/admin/users_controller.rb

@@ -75,7 +75,9 @@ class Admin::UsersController < Admin::ApplicationController
   end
 
   def block
-    if update_user { |user| user.block }
+    result = Users::BlockService.new(current_user).execute(user)
+
+    if result[:status] = :success
       redirect_back_or_admin_user(notice: _("Successfully blocked"))
     else
       redirect_back_or_admin_user(alert: _("Error occurred. User was not blocked"))

app/services/users/block_service.rb

+# frozen_string_literal: true
+
+module Users
+  class BlockService < BaseService
+    def initialize(current_user)
+      @current_user = current_user
+    end
+
+    def execute(user)
+      if user.block
+        after_block_hook(user)
+        success
+      else
+        messages = user.errors.full_messages
+        error(messages.uniq.join('. '))
+      end
+    end
+
+    private
+
+    def after_block_hook(user)
+      # overriden by EE module
+    end
+  end
+end
+
+Users::BlockService.prepend_if_ee('EE::Users::BlockService')

doc/administration/audit_events.md

@@ -107,6 +107,7 @@ recorded:
 - Started/stopped user impersonation
 - Changed username ([introduced](https://gitlab.com/gitlab-org/gitlab/issues/7797) in GitLab 12.8)
 - User was deleted ([introduced](https://gitlab.com/gitlab-org/gitlab/issues/251) in GitLab 12.8)
+- User was blocked via Admin Area ([introduced](https://gitlab.com/gitlab-org/gitlab/issues/251) in GitLab 12.8)
 
 It is possible to filter particular actions by choosing an audit data type from
 the filter dropdown box. You can further filter by specific group, project or user

ee/app/services/ee/users/block_service.rb

+# frozen_string_literal: true
+
+module EE
+  module Users
+    module BlockService
+      extend ::Gitlab::Utils::Override
+
+      override :after_block_hook
+      def after_block_hook(user)
+        super
+
+        log_audit_event(user)
+      end
+
+      private
+
+      def log_audit_event(user)
+        ::AuditEventService.new(
+          current_user,
+          user,
+          action: :custom,
+          custom_message: 'Blocked user'
+        ).for_user.security_event
+      end
+    end
+  end
+end

ee/changelogs/unreleased/251-record-user-block-audit-event.yml

+---
+title: Record audit event when user is blocked
+merge_request: 24930
+author:
+type: added

ee/spec/services/ee/users/block_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Users::BlockService do
+  let(:current_user) { create(:admin) }
+
+  subject(:service) { described_class.new(current_user) }
+
+  describe '#execute' do
+    let(:user) { create(:user) }
+
+    subject(:operation) { service.execute(user) }
+
+    describe 'audit events' do
+      before do
+        stub_licensed_features(admin_audit_log: true)
+      end
+
+      context 'when user block operation succeeds' do
+        it 'logs an audit event' do
+          expect { operation }.to change { AuditEvent.count }.by(1)
+        end
+
+        it 'logs the audit event info' do
+          operation
+
+          expect(AuditEvent.last).to have_attributes(
+            details: hash_including(custom_message: 'Blocked user')
+          )
+        end
+      end
+
+      context 'when user block operation fails' do
+        before do
+          allow(user).to receive(:block).and_return(false)
+        end
+
+        it 'does not log any audit event' do
+          expect { operation }.not_to change { AuditEvent.count }
+        end
+      end
+    end
+  end
+end

spec/services/users/block_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Users::BlockService do
+  let(:current_user) { create(:admin) }
+
+  subject(:service) { described_class.new(current_user) }
+
+  describe '#execute' do
+    subject(:operation) { service.execute(user) }
+
+    context 'when successful' do
+      let(:user) { create(:user) }
+
+      it { is_expected.to eq(status: :success) }
+
+      it "change the user's state" do
+        expect { operation }.to change { user.state }.to('blocked')
+      end
+    end
+
+    context 'when failed' do
+      let(:user) { create(:user, :blocked) }
+
+      it 'returns error result' do
+        aggregate_failures 'error result' do
+          expect(operation[:status]).to eq(:error)
+          expect(operation[:message]).to match(/State cannot transition/)
+        end
+      end
+
+      it "does not change the user's state" do
+        expect { operation }.not_to change { user.state }
+      end
+    end
+  end
+end