Merge branch 'ban-user-api' into 'master'

Ban and unban user via API

See merge request gitlab-org/gitlab!68332

app/models/user.rb

@@ -352,6 +352,10 @@ class User < ApplicationRecord
       transition active: :banned
     end
 
+    event :unban do
+      transition banned: :active
+    end
+
     event :deactivate do
       # Any additional changes to this event should be also
       # reflected in app/workers/users/deactivate_dormant_users_worker.rb

app/services/users/ban_service.rb

@@ -8,6 +8,10 @@ module Users
       user.ban
     end
 
+    def valid_state?(user)
+      user.active?
+    end
+
     def action
       :ban
     end

app/services/users/banned_user_base_service.rb

@@ -8,6 +8,7 @@ module Users
 
     def execute(user)
       return permission_error unless allowed?
+      return state_error(user) unless valid_state?(user)
 
       if update_user(user)
         log_event(user)
@@ -22,6 +23,10 @@ module Users
 
     attr_reader :current_user
 
+    def state_error(user)
+      error(_("You cannot %{action} %{state} users." % { action: action.to_s, state: user.state }), :forbidden)
+    end
+
     def allowed?
       can?(current_user, :admin_all_resources)
     end

app/services/users/unban_service.rb

@@ -5,7 +5,11 @@ module Users
     private
 
     def update_user(user)
-      user.activate
+      user.unban
+    end
+
+    def valid_state?(user)
+      user.banned?
     end
 
     def action

doc/api/users.md

@@ -1467,6 +1467,46 @@ Returns:
 - `404 User Not Found` if the user cannot be found.
 - `403 Forbidden` if the user cannot be activated because they are blocked by an administrator or by LDAP synchronization.
 
+## Ban user
+
+> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/327354) in GitLab 14.3.
+
+Bans the specified user. Available only for admin.
+
+```plaintext
+POST /users/:id/ban
+```
+
+Parameters:
+
+- `id` (required) - ID of specified user
+
+Returns:
+
+- `201 OK` on success.
+- `404 User Not Found` if user cannot be found.
+- `403 Forbidden` when trying to ban a user that is not active.
+
+## Unban user
+
+> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/327354) in GitLab 14.3.
+
+Unbans the specified user. Available only for admin.
+
+```plaintext
+POST /users/:id/unban
+```
+
+Parameters:
+
+- `id` (required) - ID of specified user
+
+Returns:
+
+- `201 OK` on success.
+- `404 User Not Found` if the user cannot be found.
+- `403 Forbidden` when trying to unban a user that is not banned.
+
 ### Get user contribution events
 
 Please refer to the [Events API documentation](events.md#get-user-contribution-events)

lib/api/users.rb

@@ -687,6 +687,38 @@ module API
       end
       # rubocop: enable CodeReuse/ActiveRecord
 
+      desc 'Ban a user. Available only for admins.'
+      params do
+        requires :id, type: Integer, desc: 'The ID of the user'
+      end
+      post ':id/ban', feature_category: :authentication_and_authorization do
+        authenticated_as_admin!
+        user = find_user_by_id(params)
+
+        result = ::Users::BanService.new(current_user).execute(user)
+        if result[:status] == :success
+          true
+        else
+          render_api_error!(result[:message], result[:http_status])
+        end
+      end
+
+      desc 'Unban a user. Available only for admins.'
+      params do
+        requires :id, type: Integer, desc: 'The ID of the user'
+      end
+      post ':id/unban', feature_category: :authentication_and_authorization do
+        authenticated_as_admin!
+        user = find_user_by_id(params)
+
+        result = ::Users::UnbanService.new(current_user).execute(user)
+        if result[:status] == :success
+          true
+        else
+          render_api_error!(result[:message], result[:http_status])
+        end
+      end
+
       desc 'Get memberships' do
         success Entities::Membership
       end

locale/gitlab.pot

@@ -38218,6 +38218,9 @@ msgstr ""
 msgid "You can view the source or %{linkStart}%{cloneIcon} clone the repository%{linkEnd}"
 msgstr ""
 
+msgid "You cannot %{action} %{state} users."
+msgstr ""
+
 msgid "You cannot access the raw file. Please wait a minute."
 msgstr ""
 

spec/models/user_spec.rb

@@ -1919,15 +1919,15 @@ RSpec.describe User do
         user.ban!
       end
 
-      it 'activates the user' do
-        user.activate
+      it 'unbans the user' do
+        user.unban
 
         expect(user.banned?).to eq(false)
         expect(user.active?).to eq(true)
       end
 
       it 'deletes the BannedUser record' do
-        expect { user.activate }.to change { Users::BannedUser.count }.by(-1)
+        expect { user.unban }.to change { Users::BannedUser.count }.by(-1)
         expect(Users::BannedUser.where(user_id: user.id)).not_to exist
       end
     end

spec/requests/api/users_spec.rb

@@ -12,6 +12,8 @@ RSpec.describe API::Users do
   let(:omniauth_user) { create(:omniauth_user) }
   let(:ldap_blocked_user) { create(:omniauth_user, provider: 'ldapmain', state: 'ldap_blocked') }
   let(:private_user) { create(:user, private_profile: true) }
+  let(:deactivated_user) { create(:user, state: 'deactivated') }
+  let(:banned_user) { create(:user, :banned) }
 
   context 'admin notes' do
     let_it_be(:admin) { create(:admin, note: '2019-10-06 | 2FA added | user requested | www.gitlab.com') }
@@ -2964,6 +2966,169 @@ RSpec.describe API::Users do
     end
   end
 
+  describe 'POST /users/:id/ban', :aggregate_failures do
+    context 'when admin' do
+      subject(:ban_user) { post api("/users/#{user_id}/ban", admin) }
+
+      context 'with an active user' do
+        let(:user_id) { user.id }
+
+        it 'bans an active user' do
+          ban_user
+
+          expect(response).to have_gitlab_http_status(:created)
+          expect(response.body).to eq('true')
+          expect(user.reload.state).to eq('banned')
+        end
+      end
+
+      context 'with an ldap blocked user' do
+        let(:user_id) { ldap_blocked_user.id }
+
+        it 'does not ban ldap blocked users' do
+          ban_user
+
+          expect(response).to have_gitlab_http_status(:forbidden)
+          expect(json_response['message']).to eq('You cannot ban ldap_blocked users.')
+          expect(ldap_blocked_user.reload.state).to eq('ldap_blocked')
+        end
+      end
+
+      context 'with a deactivated user' do
+        let(:user_id) { deactivated_user.id }
+
+        it 'does not ban deactivated users' do
+          ban_user
+
+          expect(response).to have_gitlab_http_status(:forbidden)
+          expect(json_response['message']).to eq('You cannot ban deactivated users.')
+          expect(deactivated_user.reload.state).to eq('deactivated')
+        end
+      end
+
+      context 'with a banned user' do
+        let(:user_id) { banned_user.id }
+
+        it 'does not ban banned users' do
+          ban_user
+
+          expect(response).to have_gitlab_http_status(:forbidden)
+          expect(json_response['message']).to eq('You cannot ban banned users.')
+          expect(banned_user.reload.state).to eq('banned')
+        end
+      end
+
+      context 'with a non existent user' do
+        let(:user_id) { non_existing_record_id }
+
+        it 'does not ban non existent users' do
+          ban_user
+
+          expect(response).to have_gitlab_http_status(:not_found)
+          expect(json_response['message']).to eq('404 User Not Found')
+        end
+      end
+
+      context 'with an invalid id' do
+        let(:user_id) { 'ASDF' }
+
+        it 'does not ban invalid id users' do
+          ban_user
+
+          expect(response).to have_gitlab_http_status(:not_found)
+        end
+      end
+    end
+
+    it 'is not available for non-admin users' do
+      post api("/users/#{user.id}/ban", user)
+
+      expect(response).to have_gitlab_http_status(:forbidden)
+      expect(user.reload.state).to eq('active')
+    end
+  end
+
+  describe 'POST /users/:id/unban', :aggregate_failures do
+    context 'when admin' do
+      subject(:unban_user) { post api("/users/#{user_id}/unban", admin) }
+
+      context 'with a banned user' do
+        let(:user_id) { banned_user.id }
+
+        it 'activates a banned user' do
+          unban_user
+
+          expect(response).to have_gitlab_http_status(:created)
+          expect(banned_user.reload.state).to eq('active')
+        end
+      end
+
+      context 'with an ldap_blocked user' do
+        let(:user_id) { ldap_blocked_user.id }
+
+        it 'does not unban ldap_blocked users' do
+          unban_user
+
+          expect(response).to have_gitlab_http_status(:forbidden)
+          expect(json_response['message']).to eq('You cannot unban ldap_blocked users.')
+          expect(ldap_blocked_user.reload.state).to eq('ldap_blocked')
+        end
+      end
+
+      context 'with a deactivated user' do
+        let(:user_id) { deactivated_user.id }
+
+        it 'does not unban deactivated users' do
+          unban_user
+
+          expect(response).to have_gitlab_http_status(:forbidden)
+          expect(json_response['message']).to eq('You cannot unban deactivated users.')
+          expect(deactivated_user.reload.state).to eq('deactivated')
+        end
+      end
+
+      context 'with an active user' do
+        let(:user_id) { user.id }
+
+        it 'does not unban active users' do
+          unban_user
+
+          expect(response).to have_gitlab_http_status(:forbidden)
+          expect(json_response['message']).to eq('You cannot unban active users.')
+          expect(user.reload.state).to eq('active')
+        end
+      end
+
+      context 'with a non existent user' do
+        let(:user_id) { non_existing_record_id }
+
+        it 'does not unban non existent users' do
+          unban_user
+
+          expect(response).to have_gitlab_http_status(:not_found)
+          expect(json_response['message']).to eq('404 User Not Found')
+        end
+      end
+
+      context 'with an invalid id user' do
+        let(:user_id) { 'ASDF' }
+
+        it 'does not unban invalid id users' do
+          unban_user
+
+          expect(response).to have_gitlab_http_status(:not_found)
+        end
+      end
+    end
+
+    it 'is not available for non admin users' do
+      post api("/users/#{banned_user.id}/unban", user)
+
+      expect(response).to have_gitlab_http_status(:forbidden)
+      expect(user.reload.state).to eq('active')
+    end
+  end
+
   describe "GET /users/:id/memberships" do
     let_it_be(:user) { create(:user) }
     let_it_be(:project) { create(:project) }

spec/services/users/ban_service_spec.rb

@@ -50,7 +50,7 @@ RSpec.describe Users::BanService do
           response = ban_user
 
           expect(response[:status]).to eq(:error)
-          expect(response[:message]).to match(/State cannot transition/)
+          expect(response[:message]).to match('You cannot ban blocked users.')
         end
 
         it_behaves_like 'does not modify the BannedUser record or user state'

spec/services/users/unban_service_spec.rb

@@ -50,7 +50,7 @@ RSpec.describe Users::UnbanService do
           response = unban_user
 
           expect(response[:status]).to eq(:error)
-          expect(response[:message]).to match(/State cannot transition/)
+          expect(response[:message]).to match('You cannot unban active users.')
         end
 
         it_behaves_like 'does not modify the BannedUser record or user state'