Commit 33040676 authored by Tan Le's avatar Tan Le Committed by Imre Farkas

Ignore blocking already blocked user

This maintains the idempotent nature of this API and consistent with
other state change method (e.g. `deactivated`).
parent 7985d778
...@@ -107,6 +107,7 @@ recorded: ...@@ -107,6 +107,7 @@ recorded:
- 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 added ([introduced](https://gitlab.com/gitlab-org/gitlab/issues/251) in GitLab 12.8) - User was added ([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) - User was blocked via Admin Area ([introduced](https://gitlab.com/gitlab-org/gitlab/issues/251) in GitLab 12.8)
- User was blocked via API ([introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/25872) in GitLab 12.9)
It's possible to filter particular actions by choosing an audit data type from It's 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
......
...@@ -1168,8 +1168,11 @@ Parameters: ...@@ -1168,8 +1168,11 @@ Parameters:
- `id` (required) - id of specified user - `id` (required) - id of specified user
Will return `201 OK` on success, `404 User Not Found` is user cannot be found or Returns:
`403 Forbidden` when trying to block an already blocked user by LDAP synchronization.
- `201 OK` on success.
- `404 User Not Found` if user cannot be found.
- `403 Forbidden` when trying to block an already blocked user by LDAP synchronization.
## Unblock user ## Unblock user
......
---
title: Audit user blocked via API
merge_request: 25872
author:
type: added
...@@ -57,6 +57,16 @@ describe API::Users do ...@@ -57,6 +57,16 @@ describe API::Users do
expect(AuditEvent.count).to eq(1) expect(AuditEvent.count).to eq(1)
end end
end end
describe 'POST /users/:id/block' do
it 'creates audit event when blocking user' do
stub_licensed_features(extended_audit_events: true)
expect do
post api("/users/#{user.id}/block", admin)
end.to change { AuditEvent.count }.by(1)
end
end
end end
context 'shared_runners_minutes_limit' do context 'shared_runners_minutes_limit' do
......
...@@ -528,11 +528,18 @@ module API ...@@ -528,11 +528,18 @@ module API
user = User.find_by(id: params[:id]) user = User.find_by(id: params[:id])
not_found!('User') unless user not_found!('User') unless user
if !user.ldap_blocked? if user.ldap_blocked?
user.block
else
forbidden!('LDAP blocked users cannot be modified by the API') forbidden!('LDAP blocked users cannot be modified by the API')
end end
break if user.blocked?
result = ::Users::BlockService.new(current_user).execute(user)
if result[:status] == :success
true
else
render_api_error!(result[:message], result[:http_status])
end
end end
# rubocop: enable CodeReuse/ActiveRecord # rubocop: enable CodeReuse/ActiveRecord
......
...@@ -2165,14 +2165,20 @@ describe API::Users, :do_not_mock_admin_mode do ...@@ -2165,14 +2165,20 @@ describe API::Users, :do_not_mock_admin_mode do
end end
describe 'POST /users/:id/block' do describe 'POST /users/:id/block' do
let(:blocked_user) { create(:user, state: 'blocked') }
before do before do
admin admin
end end
it 'blocks existing user' do it 'blocks existing user' do
post api("/users/#{user.id}/block", admin) post api("/users/#{user.id}/block", admin)
expect(response).to have_gitlab_http_status(:created)
expect(user.reload.state).to eq('blocked') aggregate_failures do
expect(response).to have_gitlab_http_status(:created)
expect(response.body).to eq('true')
expect(user.reload.state).to eq('blocked')
end
end end
it 'does not re-block ldap blocked users' do it 'does not re-block ldap blocked users' do
...@@ -2192,6 +2198,15 @@ describe API::Users, :do_not_mock_admin_mode do ...@@ -2192,6 +2198,15 @@ describe API::Users, :do_not_mock_admin_mode do
expect(response).to have_gitlab_http_status(:not_found) expect(response).to have_gitlab_http_status(:not_found)
expect(json_response['message']).to eq('404 User Not Found') expect(json_response['message']).to eq('404 User Not Found')
end end
it 'returns a 201 if user is already blocked' do
post api("/users/#{blocked_user.id}/block", admin)
aggregate_failures do
expect(response).to have_gitlab_http_status(:created)
expect(response.body).to eq('null')
end
end
end end
describe 'POST /users/:id/unblock' do describe 'POST /users/:id/unblock' do
......
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