From 6abbe9871df75a41ddadff43597735d7efea3245 Mon Sep 17 00:00:00 2001
From: sameer shaik <sshaik@gitlab.com>
Date: Wed, 24 May 2023 14:18:49 +0000
Subject: [PATCH 1/6] Add activate service

---
 app/controllers/admin/users_controller.rb     | 12 ++--
 app/services/users/activate_service.rb        | 50 ++++++++++++++
 ee/app/services/ee/users/activate_service.rb  | 32 +++++++++
 .../audit_events/types/user_activate.yml      |  8 +++
 .../ee/users/activate_service_spec.rb         | 41 ++++++++++++
 lib/api/users.rb                              | 14 ++--
 .../admin/users_controller_spec.rb            |  4 +-
 spec/requests/api/users_spec.rb               |  4 +-
 spec/services/users/activate_service_spec.rb  | 67 +++++++++++++++++++
 9 files changed, 218 insertions(+), 14 deletions(-)
 create mode 100644 app/services/users/activate_service.rb
 create mode 100644 ee/app/services/ee/users/activate_service.rb
 create mode 100644 ee/config/audit_events/types/user_activate.yml
 create mode 100644 ee/spec/services/ee/users/activate_service_spec.rb
 create mode 100644 spec/services/users/activate_service_spec.rb

diff --git a/app/controllers/admin/users_controller.rb b/app/controllers/admin/users_controller.rb
index 8d2146cddc5bfd..bd6e0800fe45f9 100644
--- a/app/controllers/admin/users_controller.rb
+++ b/app/controllers/admin/users_controller.rb
@@ -87,12 +87,14 @@ def reject
   end
 
   def activate
-    if user.blocked?
-      return redirect_back_or_admin_user(notice: _("Error occurred. A blocked user must be unblocked to be activated"))
+    activate_service = Users::ActivateService.new(current_user, skip_authorization: true)
+    result = activate_service.execute(user)
+  
+    if result.success?
+      redirect_back_or_admin_user(notice: _("Successfully activated"))
+    else
+      redirect_back_or_admin_user(alert: result.message)
     end
-
-    user.activate
-    redirect_back_or_admin_user(notice: _("Successfully activated"))
   end
 
   def deactivate
diff --git a/app/services/users/activate_service.rb b/app/services/users/activate_service.rb
new file mode 100644
index 00000000000000..ced072a214139c
--- /dev/null
+++ b/app/services/users/activate_service.rb
@@ -0,0 +1,50 @@
+# frozen_string_literal: true
+
+module Users
+  class ActivateService < BaseService
+    def initialize(current_user, skip_authorization: false)
+      @current_user = current_user
+      @skip_authorization = skip_authorization
+    end
+
+    def execute(user)
+      unless allowed?
+        return ::ServiceResponse.error(message: _('You are not authorized to perform this action'),
+          reason: :forbidden)
+      end
+
+      if user.blocked?
+        return ::ServiceResponse.error(message: _('Error occurred. A blocked user must be unblocked to be activated'),
+          reason: :forbidden)
+      end
+
+      if user.active?
+        return ::ServiceResponse.success(message: _('Successfully activated'))
+      end
+
+      if user.activate
+        log_event(user)
+        ::ServiceResponse.success(message: _('Successfully activated'))
+      else
+        ::ServiceResponse.error(message: user.errors.full_messages.to_sentence,
+          reason: :bad_request)
+      end
+    end
+
+    private
+
+    attr_reader :current_user
+
+    def allowed?
+      return true if @skip_authorization
+      can?(current_user, :admin_all_resources)
+    end
+
+    def log_event(user)
+      Gitlab::AppLogger.info(message: 'User activated', user: user.username.to_s, email: user.email.to_s,
+        activated_by: current_user.username.to_s, ip_address: current_user.current_sign_in_ip.to_s)
+    end
+  end
+end
+
+Users::ActivateService.prepend_mod_with('Users::ActivateService')
diff --git a/ee/app/services/ee/users/activate_service.rb b/ee/app/services/ee/users/activate_service.rb
new file mode 100644
index 00000000000000..8847f5a93f7430
--- /dev/null
+++ b/ee/app/services/ee/users/activate_service.rb
@@ -0,0 +1,32 @@
+# frozen_string_literal: true
+
+module EE
+    module Users
+      module ActivateService
+        extend ::Gitlab::Utils::Override
+  
+        override :execute
+        def execute(user)
+          super.tap do |result|
+            log_audit_event(user) if result[:status] == :success
+          end
+        end
+  
+        private
+  
+        def log_audit_event(user)
+          audit_context = {
+            name: 'user_activate',
+            author: current_user,
+            scope: user,
+            target: user,
+            message: "Activated user",
+            target_details: user.username
+          }
+  
+          ::Gitlab::Audit::Auditor.audit(audit_context)
+        end
+      end
+    end
+  end
+  
\ No newline at end of file
diff --git a/ee/config/audit_events/types/user_activate.yml b/ee/config/audit_events/types/user_activate.yml
new file mode 100644
index 00000000000000..93d9cfaab20317
--- /dev/null
+++ b/ee/config/audit_events/types/user_activate.yml
@@ -0,0 +1,8 @@
+name: user_activate
+description: Event triggered on user activate action
+introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/13473
+introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/121708
+feature_category: "user_management"
+milestone: "16.1"
+saved_to_database: true
+streamed: true
diff --git a/ee/spec/services/ee/users/activate_service_spec.rb b/ee/spec/services/ee/users/activate_service_spec.rb
new file mode 100644
index 00000000000000..97a97e5e88b350
--- /dev/null
+++ b/ee/spec/services/ee/users/activate_service_spec.rb
@@ -0,0 +1,41 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Users::ActivateService, feature_category: :user_management do
+  let_it_be(:current_user) { create(:admin) }
+
+  subject(:service) { described_class.new(current_user, skip_authorization: true) }
+
+  describe '#execute' do
+    let!(:user) { create(:user, :deactivated) }
+
+    subject(:operation) { service.execute(user) }
+
+    context 'for audit events', :enable_admin_mode do
+      include_examples 'audit event logging' do
+        let(:operation) { service.execute(user) }
+
+        let(:fail_condition!) do
+          allow(user).to receive(:activate).and_return(false)
+        end
+
+        let(:attributes) do
+          {
+            author_id: current_user.id,
+            entity_id: user.id,
+            entity_type: 'User',
+            details: {
+              author_class: 'User',
+              author_name: current_user.name,
+              custom_message: 'Activated user',
+              target_details: user.username,
+              target_id: user.id,
+              target_type: 'User'
+            }
+          }
+        end
+      end
+    end
+  end
+end
diff --git a/lib/api/users.rb b/lib/api/users.rb
index dc77dc5c157df3..0b1fa493cebef3 100644
--- a/lib/api/users.rb
+++ b/lib/api/users.rb
@@ -704,13 +704,17 @@ def reorder_users(users)
       # rubocop: disable CodeReuse/ActiveRecord
       post ':id/activate', feature_category: :system_access do
         authenticated_as_admin!
-
+      
         user = User.find_by(id: params[:id])
         not_found!('User') unless user
-        forbidden!('A blocked user must be unblocked to be activated') if user.blocked?
-
-        user.activate
-      end
+      
+        result = ::Users::ActivateService.new(current_user, skip_authorization: true).execute(user)
+        if result[:status] == :success
+          true
+        else
+          render_api_error!(result[:message], result[:reason] || :bad_request)
+        end
+      end      
 
       desc 'Approve a pending user. Available only for admins.'
       params do
diff --git a/spec/controllers/admin/users_controller_spec.rb b/spec/controllers/admin/users_controller_spec.rb
index a53d200b1e1290..c57ffdd4b05a76 100644
--- a/spec/controllers/admin/users_controller_spec.rb
+++ b/spec/controllers/admin/users_controller_spec.rb
@@ -356,7 +356,7 @@
         put :activate, params: { id: user.username }
         user.reload
         expect(user.active?).to be_falsey
-        expect(flash[:notice]).to eq('Error occurred. A blocked user must be unblocked to be activated')
+        expect(flash[:alert]).to eq('Error occurred. A blocked user must be unblocked to be activated')
       end
     end
   end
@@ -904,7 +904,7 @@ def update_password(user, password = User.random_password, password_confirmation
       it "shows a notice" do
         post :impersonate, params: { id: user.username }
 
-        expect(flash[:notice]).to eq("You are now impersonating #{user.username}")
+        expect(flash[:alert]).to eq("You are now impersonating #{user.username}")
       end
 
       it 'clears token session keys' do
diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb
index cc8be312c71a43..c1aeb9991cd94f 100644
--- a/spec/requests/api/users_spec.rb
+++ b/spec/requests/api/users_spec.rb
@@ -3480,7 +3480,7 @@ def update_password(user, admin, password = User.random_password)
             activate
 
             expect(response).to have_gitlab_http_status(:forbidden)
-            expect(json_response['message']).to eq('403 Forbidden - A blocked user must be unblocked to be activated')
+            expect(json_response['message']).to eq('Error occurred. A blocked user must be unblocked to be activated')
             expect(blocked_user.reload.state).to eq('blocked')
           end
         end
@@ -3494,7 +3494,7 @@ def update_password(user, admin, password = User.random_password)
             activate
 
             expect(response).to have_gitlab_http_status(:forbidden)
-            expect(json_response['message']).to eq('403 Forbidden - A blocked user must be unblocked to be activated')
+            expect(json_response['message']).to eq('Error occurred. A blocked user must be unblocked to be activated')
             expect(user.reload.state).to eq('ldap_blocked')
           end
         end
diff --git a/spec/services/users/activate_service_spec.rb b/spec/services/users/activate_service_spec.rb
new file mode 100644
index 00000000000000..145d1c6aff80c5
--- /dev/null
+++ b/spec/services/users/activate_service_spec.rb
@@ -0,0 +1,67 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Users::ActivateService, feature_category: :user_management do
+  let_it_be(:current_user) { build(:admin) }
+
+  subject(:service) { described_class.new(current_user) }
+
+  describe '#execute' do
+    let!(:user) { create(:user, :deactivated) }
+    
+    subject(:operation) { service.execute(user) }
+
+    context 'when successful', :enable_admin_mode do
+      it 'returns success status' do
+        expect(operation[:status]).to eq(:success)
+      end
+
+      it "changes the user's state" do
+        expect { operation }.to change { user.state }.to('active')
+      end
+
+      it 'creates a log entry' do
+        expect(Gitlab::AppLogger).to receive(:info).with(message: "User activated", user: user.username,
+          email: user.email, activated_by: current_user.username, ip_address: current_user.current_sign_in_ip.to_s)
+
+        operation
+      end
+    end
+
+    context 'when the user is already active', :enable_admin_mode do
+      let(:user) { create(:user) }
+
+      it 'returns success result' do
+        aggregate_failures 'success result' do
+          expect(operation[:status]).to eq(:success)
+          expect(operation[:message]).to eq('Successfully activated')
+        end
+      end
+
+      it "does not change the user's state" do
+        expect { operation }.not_to change { user.state }
+      end
+    end
+
+    context 'when user is not an admin' do
+      let(:non_admin_user) { build(:user) }
+      let(:service) { described_class.new(non_admin_user) }
+
+      it 'returns permissions error message' do
+        expect(operation[:status]).to eq(:error)
+        expect(operation[:message]).to eq("You are not authorized to perform this action")
+        expect(operation.reason).to eq :forbidden
+      end
+    end
+
+    context 'when skip_authorization is true', :enable_admin_mode do
+      let(:non_admin_user) { create(:user) }
+      let(:service) { described_class.new(non_admin_user, skip_authorization: true) }
+
+      it 'activates the user even if the current user is not an admin' do
+        expect(operation[:status]).to eq(:success)
+      end
+    end
+  end
+end
-- 
GitLab


From a4716a8af58d5e24cb513cf498670a734bbb35e3 Mon Sep 17 00:00:00 2001
From: sameer shaik <sshaik@gitlab.com>
Date: Wed, 24 May 2023 14:40:42 +0000
Subject: [PATCH 2/6] Fix rubocop failures

---
 app/controllers/admin/users_controller.rb    |  2 +-
 app/services/users/activate_service.rb       |  5 +-
 ee/app/services/ee/users/activate_service.rb | 49 ++++++++++----------
 lib/api/users.rb                             |  6 +--
 spec/services/users/activate_service_spec.rb |  2 +-
 5 files changed, 31 insertions(+), 33 deletions(-)

diff --git a/app/controllers/admin/users_controller.rb b/app/controllers/admin/users_controller.rb
index bd6e0800fe45f9..a3a1970f54fb77 100644
--- a/app/controllers/admin/users_controller.rb
+++ b/app/controllers/admin/users_controller.rb
@@ -89,7 +89,7 @@ def reject
   def activate
     activate_service = Users::ActivateService.new(current_user, skip_authorization: true)
     result = activate_service.execute(user)
-  
+
     if result.success?
       redirect_back_or_admin_user(notice: _("Successfully activated"))
     else
diff --git a/app/services/users/activate_service.rb b/app/services/users/activate_service.rb
index ced072a214139c..465715f13335ec 100644
--- a/app/services/users/activate_service.rb
+++ b/app/services/users/activate_service.rb
@@ -18,9 +18,7 @@ def execute(user)
           reason: :forbidden)
       end
 
-      if user.active?
-        return ::ServiceResponse.success(message: _('Successfully activated'))
-      end
+      return ::ServiceResponse.success(message: _('Successfully activated')) if user.active?
 
       if user.activate
         log_event(user)
@@ -37,6 +35,7 @@ def execute(user)
 
     def allowed?
       return true if @skip_authorization
+
       can?(current_user, :admin_all_resources)
     end
 
diff --git a/ee/app/services/ee/users/activate_service.rb b/ee/app/services/ee/users/activate_service.rb
index 8847f5a93f7430..638e9d15d2ff7c 100644
--- a/ee/app/services/ee/users/activate_service.rb
+++ b/ee/app/services/ee/users/activate_service.rb
@@ -1,32 +1,31 @@
 # frozen_string_literal: true
 
 module EE
-    module Users
-      module ActivateService
-        extend ::Gitlab::Utils::Override
-  
-        override :execute
-        def execute(user)
-          super.tap do |result|
-            log_audit_event(user) if result[:status] == :success
-          end
-        end
-  
-        private
-  
-        def log_audit_event(user)
-          audit_context = {
-            name: 'user_activate',
-            author: current_user,
-            scope: user,
-            target: user,
-            message: "Activated user",
-            target_details: user.username
-          }
-  
-          ::Gitlab::Audit::Auditor.audit(audit_context)
+  module Users
+    module ActivateService
+      extend ::Gitlab::Utils::Override
+
+      override :execute
+      def execute(user)
+        super.tap do |result|
+          log_audit_event(user) if result[:status] == :success
         end
       end
+
+      private
+
+      def log_audit_event(user)
+        audit_context = {
+          name: 'user_activate',
+          author: current_user,
+          scope: user,
+          target: user,
+          message: "Activated user",
+          target_details: user.username
+        }
+
+        ::Gitlab::Audit::Auditor.audit(audit_context)
+      end
     end
   end
-  
\ No newline at end of file
+end
diff --git a/lib/api/users.rb b/lib/api/users.rb
index 0b1fa493cebef3..561dbeb3157651 100644
--- a/lib/api/users.rb
+++ b/lib/api/users.rb
@@ -704,17 +704,17 @@ def reorder_users(users)
       # rubocop: disable CodeReuse/ActiveRecord
       post ':id/activate', feature_category: :system_access do
         authenticated_as_admin!
-      
+
         user = User.find_by(id: params[:id])
         not_found!('User') unless user
-      
+
         result = ::Users::ActivateService.new(current_user, skip_authorization: true).execute(user)
         if result[:status] == :success
           true
         else
           render_api_error!(result[:message], result[:reason] || :bad_request)
         end
-      end      
+      end
 
       desc 'Approve a pending user. Available only for admins.'
       params do
diff --git a/spec/services/users/activate_service_spec.rb b/spec/services/users/activate_service_spec.rb
index 145d1c6aff80c5..0ec5c1b8b77a94 100644
--- a/spec/services/users/activate_service_spec.rb
+++ b/spec/services/users/activate_service_spec.rb
@@ -9,7 +9,7 @@
 
   describe '#execute' do
     let!(:user) { create(:user, :deactivated) }
-    
+
     subject(:operation) { service.execute(user) }
 
     context 'when successful', :enable_admin_mode do
-- 
GitLab


From 35f720c28153cccf3030591704fe89b386f88eff Mon Sep 17 00:00:00 2001
From: sameer shaik <sshaik@gitlab.com>
Date: Wed, 24 May 2023 15:19:56 +0000
Subject: [PATCH 3/6] Refactor activate service

Add user deactivate service and log audit event at the instance level

Changelog: added
MR: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/121708
EE: true
---
 doc/administration/audit_events.md              | 2 ++
 spec/controllers/admin/users_controller_spec.rb | 2 +-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/doc/administration/audit_events.md b/doc/administration/audit_events.md
index 50bd943b8e4ff6..8207918dbbc622 100644
--- a/doc/administration/audit_events.md
+++ b/doc/administration/audit_events.md
@@ -393,6 +393,8 @@ The following user actions on a GitLab instance generate instance audit events:
 - User was unblocked using the Admin Area or API. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/115727) in GitLab 15.11.
 - User was banned using the Admin Area or API. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/116103) in GitLab 15.11.
 - User was unbanned using the Admin Area or API. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/116221) in GitLab 15.11.
+- User was deactivated using the Admin Area or API. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/117776) in GitLab 16.0.
+- User was activated using the Admin Area or API. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/121708) in GitLab 16.1.
 
 Instance events can also be accessed using the [Instance Audit Events API](../api/audit_events.md#instance-audit-events).
 
diff --git a/spec/controllers/admin/users_controller_spec.rb b/spec/controllers/admin/users_controller_spec.rb
index c57ffdd4b05a76..399b7c02c52434 100644
--- a/spec/controllers/admin/users_controller_spec.rb
+++ b/spec/controllers/admin/users_controller_spec.rb
@@ -904,7 +904,7 @@ def update_password(user, password = User.random_password, password_confirmation
       it "shows a notice" do
         post :impersonate, params: { id: user.username }
 
-        expect(flash[:alert]).to eq("You are now impersonating #{user.username}")
+        expect(flash[:notice]).to eq("You are now impersonating #{user.username}")
       end
 
       it 'clears token session keys' do
-- 
GitLab


From 72b9500217bfcc75c2b7c4b23bda3098ed4a120b Mon Sep 17 00:00:00 2001
From: sameer shaik <sshaik@gitlab.com>
Date: Mon, 5 Jun 2023 14:02:58 +0000
Subject: [PATCH 4/6] Refactor activate service

---
 app/services/users/activate_service.rb | 25 +++++++++++++------------
 1 file changed, 13 insertions(+), 12 deletions(-)

diff --git a/app/services/users/activate_service.rb b/app/services/users/activate_service.rb
index 465715f13335ec..e8d4dd5897cbb3 100644
--- a/app/services/users/activate_service.rb
+++ b/app/services/users/activate_service.rb
@@ -8,24 +8,17 @@ def initialize(current_user, skip_authorization: false)
     end
 
     def execute(user)
-      unless allowed?
-        return ::ServiceResponse.error(message: _('You are not authorized to perform this action'),
-          reason: :forbidden)
-      end
+      return error(_('You are not authorized to perform this action'), :forbidden) unless allowed?
 
-      if user.blocked?
-        return ::ServiceResponse.error(message: _('Error occurred. A blocked user must be unblocked to be activated'),
-          reason: :forbidden)
-      end
+      return error(_('Error occurred. A blocked user must be unblocked to be activated'), :forbidden) if user.blocked?
 
-      return ::ServiceResponse.success(message: _('Successfully activated')) if user.active?
+      return success(_('Successfully activated')) if user.active?
 
       if user.activate
         log_event(user)
-        ::ServiceResponse.success(message: _('Successfully activated'))
+        success(_('Successfully activated'))
       else
-        ::ServiceResponse.error(message: user.errors.full_messages.to_sentence,
-          reason: :bad_request)
+        error(user.errors.full_messages.to_sentence, :bad_request)
       end
     end
 
@@ -43,6 +36,14 @@ def log_event(user)
       Gitlab::AppLogger.info(message: 'User activated', user: user.username.to_s, email: user.email.to_s,
         activated_by: current_user.username.to_s, ip_address: current_user.current_sign_in_ip.to_s)
     end
+
+    def success(message)
+      ::ServiceResponse.success(message: message)
+    end
+
+    def error(message, reason)
+      ::ServiceResponse.error(message: message, reason: reason)
+    end
   end
 end
 
-- 
GitLab


From a79996e91106e1a544193541c941f74d27b272ed Mon Sep 17 00:00:00 2001
From: sameer shaik <sshaik@gitlab.com>
Date: Tue, 6 Jun 2023 14:52:19 +0000
Subject: [PATCH 5/6] Modify activate service

---
 app/controllers/admin/users_controller.rb     |  2 +-
 app/services/users/activate_service.rb        | 14 +++++++-----
 ee/app/services/ee/users/activate_service.rb  |  8 +++----
 .../ee/users/activate_service_spec.rb         |  2 +-
 lib/api/users.rb                              |  2 +-
 spec/services/users/activate_service_spec.rb  | 22 +++++++++++--------
 6 files changed, 27 insertions(+), 23 deletions(-)

diff --git a/app/controllers/admin/users_controller.rb b/app/controllers/admin/users_controller.rb
index a3a1970f54fb77..3c96e49499f06d 100644
--- a/app/controllers/admin/users_controller.rb
+++ b/app/controllers/admin/users_controller.rb
@@ -87,7 +87,7 @@ def reject
   end
 
   def activate
-    activate_service = Users::ActivateService.new(current_user, skip_authorization: true)
+    activate_service = Users::ActivateService.new(current_user)
     result = activate_service.execute(user)
 
     if result.success?
diff --git a/app/services/users/activate_service.rb b/app/services/users/activate_service.rb
index e8d4dd5897cbb3..dfc2996bcce9d1 100644
--- a/app/services/users/activate_service.rb
+++ b/app/services/users/activate_service.rb
@@ -2,9 +2,8 @@
 
 module Users
   class ActivateService < BaseService
-    def initialize(current_user, skip_authorization: false)
+    def initialize(current_user)
       @current_user = current_user
-      @skip_authorization = skip_authorization
     end
 
     def execute(user)
@@ -15,10 +14,11 @@ def execute(user)
       return success(_('Successfully activated')) if user.active?
 
       if user.activate
+        after_activate_hook(user)
         log_event(user)
         success(_('Successfully activated'))
       else
-        error(user.errors.full_messages.to_sentence, :bad_request)
+        error(user.errors.full_messages.to_sentence, :unprocessable_entity)
       end
     end
 
@@ -27,11 +27,13 @@ def execute(user)
     attr_reader :current_user
 
     def allowed?
-      return true if @skip_authorization
-
       can?(current_user, :admin_all_resources)
     end
 
+    def after_activate_hook(user)
+      # overridden by EE module
+    end
+
     def log_event(user)
       Gitlab::AppLogger.info(message: 'User activated', user: user.username.to_s, email: user.email.to_s,
         activated_by: current_user.username.to_s, ip_address: current_user.current_sign_in_ip.to_s)
@@ -47,4 +49,4 @@ def error(message, reason)
   end
 end
 
-Users::ActivateService.prepend_mod_with('Users::ActivateService')
+Users::ActivateService.prepend_mod_with('Users::ActivateService') # rubocop: disable Cop/InjectEnterpriseEditionModule
diff --git a/ee/app/services/ee/users/activate_service.rb b/ee/app/services/ee/users/activate_service.rb
index 638e9d15d2ff7c..cc18045ffdcef3 100644
--- a/ee/app/services/ee/users/activate_service.rb
+++ b/ee/app/services/ee/users/activate_service.rb
@@ -5,11 +5,9 @@ module Users
     module ActivateService
       extend ::Gitlab::Utils::Override
 
-      override :execute
-      def execute(user)
-        super.tap do |result|
-          log_audit_event(user) if result[:status] == :success
-        end
+      override :after_activate_hook
+      def after_activate_hook(user)
+        log_audit_event(user)
       end
 
       private
diff --git a/ee/spec/services/ee/users/activate_service_spec.rb b/ee/spec/services/ee/users/activate_service_spec.rb
index 97a97e5e88b350..7cd90adb257c53 100644
--- a/ee/spec/services/ee/users/activate_service_spec.rb
+++ b/ee/spec/services/ee/users/activate_service_spec.rb
@@ -5,7 +5,7 @@
 RSpec.describe Users::ActivateService, feature_category: :user_management do
   let_it_be(:current_user) { create(:admin) }
 
-  subject(:service) { described_class.new(current_user, skip_authorization: true) }
+  subject(:service) { described_class.new(current_user) }
 
   describe '#execute' do
     let!(:user) { create(:user, :deactivated) }
diff --git a/lib/api/users.rb b/lib/api/users.rb
index 561dbeb3157651..dbf8f7711ea236 100644
--- a/lib/api/users.rb
+++ b/lib/api/users.rb
@@ -708,7 +708,7 @@ def reorder_users(users)
         user = User.find_by(id: params[:id])
         not_found!('User') unless user
 
-        result = ::Users::ActivateService.new(current_user, skip_authorization: true).execute(user)
+        result = ::Users::ActivateService.new(current_user).execute(user)
         if result[:status] == :success
           true
         else
diff --git a/spec/services/users/activate_service_spec.rb b/spec/services/users/activate_service_spec.rb
index 0ec5c1b8b77a94..8b8c0dbdd3e797 100644
--- a/spec/services/users/activate_service_spec.rb
+++ b/spec/services/users/activate_service_spec.rb
@@ -44,6 +44,19 @@
       end
     end
 
+    context 'when user activation fails', :enable_admin_mode do
+      before do
+        allow(user).to receive(:activate).and_return(false)
+      end
+
+      it 'returns an unprocessable entity error' do
+        result = service.execute(user)
+
+        expect(result[:status]).to eq(:error)
+        expect(result[:reason]).to eq(:unprocessable_entity)
+      end
+    end
+
     context 'when user is not an admin' do
       let(:non_admin_user) { build(:user) }
       let(:service) { described_class.new(non_admin_user) }
@@ -54,14 +67,5 @@
         expect(operation.reason).to eq :forbidden
       end
     end
-
-    context 'when skip_authorization is true', :enable_admin_mode do
-      let(:non_admin_user) { create(:user) }
-      let(:service) { described_class.new(non_admin_user, skip_authorization: true) }
-
-      it 'activates the user even if the current user is not an admin' do
-        expect(operation[:status]).to eq(:success)
-      end
-    end
   end
 end
-- 
GitLab


From fd8ce69c8cbb2c2df30830f15dd8c81f3f923ba2 Mon Sep 17 00:00:00 2001
From: Vitali Tatarintev <vtatarintev@gitlab.com>
Date: Wed, 7 Jun 2023 10:53:59 +0000
Subject: [PATCH 6/6] Change after activate hook scope

---
 ee/app/services/ee/users/activate_service.rb | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ee/app/services/ee/users/activate_service.rb b/ee/app/services/ee/users/activate_service.rb
index cc18045ffdcef3..79fce38f24711d 100644
--- a/ee/app/services/ee/users/activate_service.rb
+++ b/ee/app/services/ee/users/activate_service.rb
@@ -5,13 +5,13 @@ module Users
     module ActivateService
       extend ::Gitlab::Utils::Override
 
+      private
+
       override :after_activate_hook
       def after_activate_hook(user)
         log_audit_event(user)
       end
 
-      private
-
       def log_audit_event(user)
         audit_context = {
           name: 'user_activate',
-- 
GitLab