Commit 6cb9bca8 authored by Tan Le's avatar Tan Le Committed by Nick Thomas

Introduce new service to block user

The existing `Users::UpdateService` is getting more complex and does not
allow capturing state changes that caused by `state_machines`. This
replacement will help to simplify logic around blocking user and make
way for new additional features (e.g. audit event).
parent 5f964fcf
...@@ -75,7 +75,9 @@ class Admin::UsersController < Admin::ApplicationController ...@@ -75,7 +75,9 @@ class Admin::UsersController < Admin::ApplicationController
end end
def block 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")) redirect_back_or_admin_user(notice: _("Successfully blocked"))
else else
redirect_back_or_admin_user(alert: _("Error occurred. User was not blocked")) redirect_back_or_admin_user(alert: _("Error occurred. User was not blocked"))
......
# 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')
...@@ -107,6 +107,7 @@ recorded: ...@@ -107,6 +107,7 @@ recorded:
- Started/stopped user impersonation - Started/stopped user impersonation
- Changed username ([introduced](https://gitlab.com/gitlab-org/gitlab/issues/7797) in GitLab 12.8) - 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 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 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 the filter dropdown box. You can further filter by specific group, project or user
......
# 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
---
title: Record audit event when user is blocked
merge_request: 24930
author:
type: added
# 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
# 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
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment