summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorSean McGivern <sean@mcgivern.me.uk>2017-02-20 15:16:23 +0000
committerSean McGivern <sean@mcgivern.me.uk>2017-02-20 15:16:23 +0000
commitfbbbf1e4e77768a40b835455f17749384f7c4984 (patch)
tree641ea53edc2509be47280a03ec675c2dd97cc94f
parent173dbeb972d0da365ac77129d0e12727ae571e91 (diff)
parent8f690604a523115370c011c767dbd76cb85c0f63 (diff)
downloadgitlab-ce-fbbbf1e4e77768a40b835455f17749384f7c4984.tar.gz
Merge branch 'api-post-block' into 'master'
API: Use POST to (un)block a user Closes #14596 See merge request !9371
-rw-r--r--changelogs/unreleased/api-post-block.yml4
-rw-r--r--doc/api/users.md8
-rw-r--r--doc/api/v3_to_v4.md1
-rw-r--r--lib/api/users.rb4
-rw-r--r--lib/api/v3/users.rb32
-rw-r--r--spec/requests/api/users_spec.rb30
-rw-r--r--spec/requests/api/v3/users_spec.rb69
7 files changed, 127 insertions, 21 deletions
diff --git a/changelogs/unreleased/api-post-block.yml b/changelogs/unreleased/api-post-block.yml
new file mode 100644
index 00000000000..dfc61ffa9e3
--- /dev/null
+++ b/changelogs/unreleased/api-post-block.yml
@@ -0,0 +1,4 @@
+---
+title: 'API: Use POST to (un)block a user'
+merge_request: 9371
+author: Robert Schilling
diff --git a/doc/api/users.md b/doc/api/users.md
index 626f7e63e3e..852c7ac8ec2 100644
--- a/doc/api/users.md
+++ b/doc/api/users.md
@@ -659,14 +659,14 @@ Will return `200 OK` on success, or `404 Not found` if either user or email cann
Blocks the specified user. Available only for admin.
```
-PUT /users/:id/block
+POST /users/:id/block
```
Parameters:
- `id` (required) - id of specified user
-Will return `200 OK` on success, `404 User Not Found` is user cannot be found or
+Will return `201 OK` on success, `404 User Not Found` is user cannot be found or
`403 Forbidden` when trying to block an already blocked user by LDAP synchronization.
## Unblock user
@@ -674,14 +674,14 @@ Will return `200 OK` on success, `404 User Not Found` is user cannot be found or
Unblocks the specified user. Available only for admin.
```
-PUT /users/:id/unblock
+POST /users/:id/unblock
```
Parameters:
- `id` (required) - id of specified user
-Will return `200 OK` on success, `404 User Not Found` is user cannot be found or
+Will return `201 OK` on success, `404 User Not Found` is user cannot be found or
`403 Forbidden` when trying to unblock a user blocked by LDAP synchronization.
### Get user contribution events
diff --git a/doc/api/v3_to_v4.md b/doc/api/v3_to_v4.md
index 0e7c5251329..2f82b6e97cc 100644
--- a/doc/api/v3_to_v4.md
+++ b/doc/api/v3_to_v4.md
@@ -26,3 +26,4 @@ changes are in V4:
- Endpoints `/projects/owned`, `/projects/visible`, `/projects/starred` & `/projects/all` are consolidated into `/projects` using query parameters
- Return pagination headers for all endpoints that return an array
- Removed `DELETE projects/:id/deploy_keys/:key_id/disable`. Use `DELETE projects/:id/deploy_keys/:key_id` instead
+- Moved `PUT /users/:id/(block|unblock)` to `POST /users/:id/(block|unblock)`
diff --git a/lib/api/users.rb b/lib/api/users.rb
index 05538f5a42f..fbc17953691 100644
--- a/lib/api/users.rb
+++ b/lib/api/users.rb
@@ -314,7 +314,7 @@ module API
params do
requires :id, type: Integer, desc: 'The ID of the user'
end
- put ':id/block' do
+ post ':id/block' do
authenticated_as_admin!
user = User.find_by(id: params[:id])
not_found!('User') unless user
@@ -330,7 +330,7 @@ module API
params do
requires :id, type: Integer, desc: 'The ID of the user'
end
- put ':id/unblock' do
+ post ':id/unblock' do
authenticated_as_admin!
user = User.find_by(id: params[:id])
not_found!('User') unless user
diff --git a/lib/api/v3/users.rb b/lib/api/v3/users.rb
index ceb139d11b8..e05e457a5df 100644
--- a/lib/api/v3/users.rb
+++ b/lib/api/v3/users.rb
@@ -39,6 +39,38 @@ module API
present user.emails, with: ::API::Entities::Email
end
+
+ desc 'Block a user. Available only for admins.'
+ params do
+ requires :id, type: Integer, desc: 'The ID of the user'
+ end
+ put ':id/block' do
+ authenticated_as_admin!
+ user = User.find_by(id: params[:id])
+ not_found!('User') unless user
+
+ if !user.ldap_blocked?
+ user.block
+ else
+ forbidden!('LDAP blocked users cannot be modified by the API')
+ end
+ end
+
+ desc 'Unblock a user. Available only for admins.'
+ params do
+ requires :id, type: Integer, desc: 'The ID of the user'
+ end
+ put ':id/unblock' do
+ authenticated_as_admin!
+ user = User.find_by(id: params[:id])
+ not_found!('User') unless user
+
+ if user.ldap_blocked?
+ forbidden!('LDAP blocked users cannot be unblocked by the API')
+ else
+ user.activate
+ end
+ end
end
resource :user do
diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb
index 7ece22f1934..9484d82a11b 100644
--- a/spec/requests/api/users_spec.rb
+++ b/spec/requests/api/users_spec.rb
@@ -1003,69 +1003,69 @@ describe API::Users, api: true do
end
end
- describe 'PUT /users/:id/block' do
+ describe 'POST /users/:id/block' do
before { admin }
it 'blocks existing user' do
- put api("/users/#{user.id}/block", admin)
- expect(response).to have_http_status(200)
+ post api("/users/#{user.id}/block", admin)
+ expect(response).to have_http_status(201)
expect(user.reload.state).to eq('blocked')
end
it 'does not re-block ldap blocked users' do
- put api("/users/#{ldap_blocked_user.id}/block", admin)
+ post api("/users/#{ldap_blocked_user.id}/block", admin)
expect(response).to have_http_status(403)
expect(ldap_blocked_user.reload.state).to eq('ldap_blocked')
end
it 'does not be available for non admin users' do
- put api("/users/#{user.id}/block", user)
+ post api("/users/#{user.id}/block", user)
expect(response).to have_http_status(403)
expect(user.reload.state).to eq('active')
end
it 'returns a 404 error if user id not found' do
- put api('/users/9999/block', admin)
+ post api('/users/9999/block', admin)
expect(response).to have_http_status(404)
expect(json_response['message']).to eq('404 User Not Found')
end
end
- describe 'PUT /users/:id/unblock' do
+ describe 'POST /users/:id/unblock' do
let(:blocked_user) { create(:user, state: 'blocked') }
before { admin }
it 'unblocks existing user' do
- put api("/users/#{user.id}/unblock", admin)
- expect(response).to have_http_status(200)
+ post api("/users/#{user.id}/unblock", admin)
+ expect(response).to have_http_status(201)
expect(user.reload.state).to eq('active')
end
it 'unblocks a blocked user' do
- put api("/users/#{blocked_user.id}/unblock", admin)
- expect(response).to have_http_status(200)
+ post api("/users/#{blocked_user.id}/unblock", admin)
+ expect(response).to have_http_status(201)
expect(blocked_user.reload.state).to eq('active')
end
it 'does not unblock ldap blocked users' do
- put api("/users/#{ldap_blocked_user.id}/unblock", admin)
+ post api("/users/#{ldap_blocked_user.id}/unblock", admin)
expect(response).to have_http_status(403)
expect(ldap_blocked_user.reload.state).to eq('ldap_blocked')
end
it 'does not be available for non admin users' do
- put api("/users/#{user.id}/unblock", user)
+ post api("/users/#{user.id}/unblock", user)
expect(response).to have_http_status(403)
expect(user.reload.state).to eq('active')
end
it 'returns a 404 error if user id not found' do
- put api('/users/9999/block', admin)
+ post api('/users/9999/block', admin)
expect(response).to have_http_status(404)
expect(json_response['message']).to eq('404 User Not Found')
end
it "returns a 404 for invalid ID" do
- put api("/users/ASDF/block", admin)
+ post api("/users/ASDF/block", admin)
expect(response).to have_http_status(404)
end
diff --git a/spec/requests/api/v3/users_spec.rb b/spec/requests/api/v3/users_spec.rb
index 7022f87bc51..5020ef18a3a 100644
--- a/spec/requests/api/v3/users_spec.rb
+++ b/spec/requests/api/v3/users_spec.rb
@@ -7,6 +7,7 @@ describe API::V3::Users, api: true do
let(:admin) { create(:admin) }
let(:key) { create(:key, user: user) }
let(:email) { create(:email, user: user) }
+ let(:ldap_blocked_user) { create(:omniauth_user, provider: 'ldapmain', state: 'ldap_blocked') }
describe 'GET /user/:id/keys' do
before { admin }
@@ -117,4 +118,72 @@ describe API::V3::Users, api: true do
end
end
end
+
+ describe 'PUT /users/:id/block' do
+ before { admin }
+ it 'blocks existing user' do
+ put v3_api("/users/#{user.id}/block", admin)
+ expect(response).to have_http_status(200)
+ expect(user.reload.state).to eq('blocked')
+ end
+
+ it 'does not re-block ldap blocked users' do
+ put v3_api("/users/#{ldap_blocked_user.id}/block", admin)
+ expect(response).to have_http_status(403)
+ expect(ldap_blocked_user.reload.state).to eq('ldap_blocked')
+ end
+
+ it 'does not be available for non admin users' do
+ put v3_api("/users/#{user.id}/block", user)
+ expect(response).to have_http_status(403)
+ expect(user.reload.state).to eq('active')
+ end
+
+ it 'returns a 404 error if user id not found' do
+ put v3_api('/users/9999/block', admin)
+ expect(response).to have_http_status(404)
+ expect(json_response['message']).to eq('404 User Not Found')
+ end
+ end
+
+ describe 'PUT /users/:id/unblock' do
+ let(:blocked_user) { create(:user, state: 'blocked') }
+ before { admin }
+
+ it 'unblocks existing user' do
+ put v3_api("/users/#{user.id}/unblock", admin)
+ expect(response).to have_http_status(200)
+ expect(user.reload.state).to eq('active')
+ end
+
+ it 'unblocks a blocked user' do
+ put v3_api("/users/#{blocked_user.id}/unblock", admin)
+ expect(response).to have_http_status(200)
+ expect(blocked_user.reload.state).to eq('active')
+ end
+
+ it 'does not unblock ldap blocked users' do
+ put v3_api("/users/#{ldap_blocked_user.id}/unblock", admin)
+ expect(response).to have_http_status(403)
+ expect(ldap_blocked_user.reload.state).to eq('ldap_blocked')
+ end
+
+ it 'does not be available for non admin users' do
+ put v3_api("/users/#{user.id}/unblock", user)
+ expect(response).to have_http_status(403)
+ expect(user.reload.state).to eq('active')
+ end
+
+ it 'returns a 404 error if user id not found' do
+ put v3_api('/users/9999/block', admin)
+ expect(response).to have_http_status(404)
+ expect(json_response['message']).to eq('404 User Not Found')
+ end
+
+ it "returns a 404 for invalid ID" do
+ put v3_api("/users/ASDF/block", admin)
+
+ expect(response).to have_http_status(404)
+ end
+ end
end