diff options
author | Robert Schilling <rschilling@student.tugraz.at> | 2017-02-20 13:31:11 +0100 |
---|---|---|
committer | Robert Schilling <rschilling@student.tugraz.at> | 2017-02-20 15:18:40 +0100 |
commit | 8f690604a523115370c011c767dbd76cb85c0f63 (patch) | |
tree | ba5b9a2e5a3c135a33a396cb6f397671d3b937c6 | |
parent | bc0b438d13f6bffd8e837f551a5415173f43f9f3 (diff) | |
download | gitlab-ce-api-post-block.tar.gz |
API: Use POST to (un)block a userapi-post-block
-rw-r--r-- | changelogs/unreleased/api-post-block.yml | 4 | ||||
-rw-r--r-- | doc/api/users.md | 8 | ||||
-rw-r--r-- | doc/api/v3_to_v4.md | 1 | ||||
-rw-r--r-- | lib/api/users.rb | 4 | ||||
-rw-r--r-- | lib/api/v3/users.rb | 32 | ||||
-rw-r--r-- | spec/requests/api/users_spec.rb | 30 | ||||
-rw-r--r-- | spec/requests/api/v3/users_spec.rb | 69 |
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 |