Merge branch 'reject-pending-user-api' into 'master'

Reject pending approval users via API

See merge request gitlab-org/gitlab!69420

app/services/users/reject_service.rb

@@ -7,8 +7,8 @@ module Users
     end
 
     def execute(user)
-      return error(_('You are not allowed to reject a user')) unless allowed?
-      return error(_('This user does not have a pending request')) unless user.blocked_pending_approval?
+      return error(_('You are not allowed to reject a user'), :forbidden) unless allowed?
+      return error(_('User does not have a pending request'), :conflict) unless user.blocked_pending_approval?
 
       user.delete_async(deleted_by: current_user, params: { hard_delete: true })
 
@@ -18,7 +18,7 @@ module Users
 
       log_event(user)
 
-      success
+      success(message: 'Success', http_status: :ok)
     end
 
     private

doc/api/users.md

@@ -1598,6 +1598,45 @@ Example Responses:
 { "message": "The user you are trying to approve is not pending approval" }
 ```
 
+## Reject user
+
+> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/339925) in GitLab 14.3.
+
+Rejects specified user that is [pending approval](../user/admin_area/moderate_users.md#users-pending-approval). Available only for administrators.
+
+```plaintext
+POST /users/:id/reject
+```
+
+Parameters:
+
+- `id` (required) - ID of specified user
+
+```shell
+curl --request POST --header "PRIVATE-TOKEN: <your_access_token>" "https://gitlab.example.com/api/v4/users/42/reject"
+```
+
+Returns:
+
+- `200 OK` on success.
+- `403 Forbidden` if not authenticated as an administrator.
+- `404 User Not Found` if user cannot be found.
+- `409 Conflict` if user is not pending approval.
+
+Example Responses:
+
+```json
+{ "message": "Success" }
+```
+
+```json
+{ "message": "404 User Not Found" }
+```
+
+```json
+{ "message": "User does not have a pending request" }
+```
+
 ## Get an impersonation token of a user
 
 > Requires admin permissions.

lib/api/users.rb

@@ -615,6 +615,22 @@ module API
         end
       end
 
+      desc 'Reject a pending user. Available only for admins.'
+      params do
+        requires :id, type: Integer, desc: 'The ID of the user'
+      end
+      post ':id/reject', feature_category: :authentication_and_authorization do
+        user = find_user_by_id(params)
+
+        result = ::Users::RejectService.new(current_user).execute(user)
+
+        if result[:success]
+          present user
+        else
+          render_api_error!(result[:message], result[:http_status])
+        end
+      end
+
       # rubocop: enable CodeReuse/ActiveRecord
       desc 'Deactivate an active user. Available only for admins.'
       params do

locale/gitlab.pot

@@ -34572,9 +34572,6 @@ msgstr ""
 msgid "This user cannot be unlocked manually from GitLab"
 msgstr ""
 
-msgid "This user does not have a pending request"
-msgstr ""
-
 msgid "This user has an unconfirmed email address (%{email}). You may force a confirmation."
 msgstr ""
 
@@ -36507,6 +36504,9 @@ msgstr ""
 msgid "User and IP Rate Limits"
 msgstr ""
 
+msgid "User does not have a pending request"
+msgstr ""
+
 msgid "User identity was successfully created."
 msgstr ""
 

spec/controllers/admin/users_controller_spec.rb

@@ -165,7 +165,7 @@ RSpec.describe Admin::UsersController do
       it 'displays the error' do
         subject
 
-        expect(flash[:alert]).to eq('This user does not have a pending request')
+        expect(flash[:alert]).to eq('User does not have a pending request')
       end
 
       it 'does not email the user' do

spec/requests/api/users_spec.rb

@@ -2778,7 +2778,9 @@ RSpec.describe API::Users do
     end
   end
 
-  context 'approve pending user' do
+  context 'approve and reject pending user' do
+    let(:pending_user) { create(:user, :blocked_pending_approval) }
+
     shared_examples '404' do
       it 'returns 404' do
         expect(response).to have_gitlab_http_status(:not_found)
@@ -2789,7 +2791,6 @@ RSpec.describe API::Users do
     describe 'POST /users/:id/approve' do
       subject(:approve) { post api("/users/#{user_id}/approve", api_user) }
 
-      let_it_be(:pending_user) { create(:user, :blocked_pending_approval) }
       let_it_be(:deactivated_user) { create(:user, :deactivated) }
       let_it_be(:blocked_user) { create(:user, :blocked) }
 
@@ -2868,6 +2869,96 @@ RSpec.describe API::Users do
         end
       end
     end
+
+    describe 'POST /users/:id/reject', :aggregate_failures do
+      subject(:reject) { post api("/users/#{user_id}/reject", api_user) }
+
+      shared_examples 'returns 409' do
+        it 'returns 409' do
+          reject
+
+          expect(response).to have_gitlab_http_status(:conflict)
+          expect(json_response['message']).to eq('User does not have a pending request')
+        end
+      end
+
+      context 'performed by a non-admin user' do
+        let(:api_user) { user }
+        let(:user_id) { pending_user.id }
+
+        it 'returns 403' do
+          expect { reject }.not_to change { pending_user.reload.state }
+          expect(response).to have_gitlab_http_status(:forbidden)
+          expect(json_response['message']).to eq('You are not allowed to reject a user')
+        end
+      end
+
+      context 'performed by an admin user' do
+        let(:api_user) { admin }
+
+        context 'for an pending approval user' do
+          let(:user_id) { pending_user.id }
+
+          it 'returns 200' do
+            reject
+
+            expect(response).to have_gitlab_http_status(:ok)
+            expect(json_response['message']).to eq('Success')
+          end
+        end
+
+        context 'for a deactivated user' do
+          let(:user_id) { deactivated_user.id }
+
+          it 'does not reject a deactivated user' do
+            expect { reject }.not_to change { deactivated_user.reload.state }
+          end
+
+          it_behaves_like 'returns 409'
+        end
+
+        context 'for an active user' do
+          let(:user_id) { user.id }
+
+          it 'does not reject an active user' do
+            expect { reject }.not_to change { user.reload.state }
+          end
+
+          it_behaves_like 'returns 409'
+        end
+
+        context 'for a blocked user' do
+          let(:blocked_user) { create(:user, :blocked) }
+          let(:user_id) { blocked_user.id }
+
+          it 'does not reject a blocked user' do
+            expect { reject }.not_to change { blocked_user.reload.state }
+          end
+
+          it_behaves_like 'returns 409'
+        end
+
+        context 'for a ldap blocked user' do
+          let(:user_id) { ldap_blocked_user.id }
+
+          it 'does not reject a ldap blocked user' do
+            expect { reject }.not_to change { ldap_blocked_user.reload.state }
+          end
+
+          it_behaves_like 'returns 409'
+        end
+
+        context 'for a user that does not exist' do
+          let(:user_id) { non_existing_record_id }
+
+          before do
+            reject
+          end
+
+          it_behaves_like '404'
+        end
+      end
+    end
   end
 
   describe 'POST /users/:id/block', :aggregate_failures do

spec/services/users/reject_service_spec.rb

@@ -27,7 +27,7 @@ RSpec.describe Users::RejectService do
           it 'returns error result' do
             expect(subject[:status]).to eq(:error)
             expect(subject[:message])
-              .to match(/This user does not have a pending request/)
+              .to match(/User does not have a pending request/)
           end
         end
       end