Fix host name in email

Use notification service instead of devise

Devise uses the resource/user to know who to send the email to, but if
the user is deleted, fails because can't find user with id

Add email method to profile mailer

Change back to hard delete

Email user when request rejected

Run gettext regenerate

Update user admin specs

Add admin user controller spec

Add notification service spec

Don't email non pending user

Add reject to dropdown

Fix static analysis errors

Reject pending user file

Actually push reject user file

Change reject user UI text

Split out reject service

Make new file for reject service and add reject route

Percent interpolation instead of hash

Update dropdown reject action

Add reject user policy

Add EE audit event if reject user

Update locale translations

Refactor reject user service

And add reject user spec

Disable rubocop code reuse

Rename file

Do audit event in follow up MR

Remove references to EE file until that gets done

Run gettext regenerate

Address MR review comments

Remove extra new line

Remove uneeded disable Active Record

Change back to destroy service for now

Change back to find user exists

Delete async instead of sync

Delete users with delete_async instead of destroy service

Change links to root url

Rerun gettext regenerate

Remove extra test from rebase

app/controllers/admin/users_controller.rb

@@ -72,6 +72,16 @@ class Admin::UsersController < Admin::ApplicationController
     end
   end
 
+  def reject
+    result = Users::RejectService.new(current_user).execute(user)
+
+    if result[:status] == :success
+      redirect_to admin_users_path, status: :found, notice: _("You've rejected %{user}" % { user: user.name })
+    else
+      redirect_back_or_admin_user(alert: result[:message])
+    end
+  end
+
   def activate
     return redirect_back_or_admin_user(notice: _("Error occurred. A blocked user must be unblocked to be activated")) if user.blocked?
 

app/mailers/devise_mailer.rb

@@ -17,10 +17,6 @@ class DeviseMailer < Devise::Mailer
     devise_mail(record, :user_admin_approval, opts)
   end
 
-  def user_admin_rejection(record, opts = {})
-    devise_mail(record, :user_admin_rejection, opts)
-  end
-
   protected
 
   def subject_for(key)

app/mailers/emails/profile.rb

@@ -18,6 +18,14 @@ module Emails
         subject: subject(_("GitLab Account Request")))
     end
 
+    def user_admin_rejection_email(name, email)
+      @name = name
+
+      profile_email_with_layout(
+        to: email,
+        subject: subject(_("GitLab account request rejected")))
+    end
+
     # rubocop: disable CodeReuse/ActiveRecord
     def new_ssh_key_email(key_id)
       @key = Key.find_by(id: key_id)

app/mailers/previews/devise_mailer_preview.rb

@@ -28,10 +28,6 @@ class DeviseMailerPreview < ActionMailer::Preview
     DeviseMailer.user_admin_approval(unsaved_user, {})
   end
 
-  def user_admin_rejection
-    DeviseMailer.user_admin_rejection(unsaved_user, {})
-  end
-
   private
 
   def unsaved_user

app/policies/global_policy.rb

@@ -99,6 +99,7 @@ class GlobalPolicy < BasePolicy
     enable :read_custom_attribute
     enable :update_custom_attribute
     enable :approve_user
+    enable :reject_user
   end
 
   # We can't use `read_statistics` because the user may have different permissions for different projects

app/services/notification_service.rb

@@ -380,6 +380,10 @@ class NotificationService
     end
   end
 
+  def user_admin_rejection(name, email)
+    mailer.user_admin_rejection_email(name, email).deliver_later
+  end
+
   # Members
   def new_access_request(member)
     return true unless member.notifiable?(:subscription)

app/services/users/reject_service.rb

+# frozen_string_literal: true
+
+module Users
+  class RejectService < BaseService
+    def initialize(current_user)
+      @current_user = current_user
+    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?
+
+      user.delete_async(deleted_by: current_user, params: { hard_delete: true })
+
+      NotificationService.new.user_admin_rejection(user.name, user.email)
+
+      success
+    end
+
+    private
+
+    attr_reader :current_user
+
+    def allowed?
+      can?(current_user, :reject_user)
+    end
+  end
+end

app/views/admin/users/_reject_pending_user.html.haml

+.card.border-danger
+  .card-header.bg-danger.gl-text-white
+    = s_('AdminUsers|This user has requested access')
+  .card-body
+    = render partial: 'admin/users/user_reject_effects'
+    %br
+    = link_to s_('AdminUsers|Reject request'), reject_admin_user_path(user), method: :delete, class: "btn gl-button btn-danger", data: { confirm: s_('AdminUsers|Are you sure?') }

app/views/admin/users/_user.html.haml

@@ -37,8 +37,7 @@
                 - elsif user.blocked?
                   - if user.blocked_pending_approval?
                     = link_to s_('AdminUsers|Approve'), approve_admin_user_path(user), method: :put
-                    %button.btn.btn-default-tertiary.js-confirm-modal-button{ data: user_block_data(user, user_block_effects) }
-                      = s_('AdminUsers|Block')
+                    = link_to s_('AdminUsers|Reject'), reject_admin_user_path(user), method: :delete
                   - else
                     %button.btn.btn-default-tertiary.js-confirm-modal-button{ data: user_unblock_data(user) }
                       = s_('AdminUsers|Unblock')
@@ -56,7 +55,7 @@
               - if user.access_locked?
                 %li
                   = link_to _('Unlock'), unlock_admin_user_path(user), method: :put, data: { confirm: _('Are you sure?') }
-            - if can?(current_user, :destroy_user, user)
+            - if can?(current_user, :destroy_user, user) && !user.blocked_pending_approval?
               %li.divider
               - if user.can_be_removed?
                 %li

app/views/admin/users/_user_reject_effects.html.haml

 %p
-  = s_('AdminUsers|If you reject the user:')
+  = s_('AdminUsers|Rejected users:')
 %ul
   %li
-    = s_('AdminUsers|The user can not sign in or access instance information.')
+    = s_('AdminUsers|Cannot sign in or access instance information')
   %li
-    = s_('AdminUsers|The user will be deleted.')
+    = s_('AdminUsers|Will be deleted')
 %p
   - link_start = '<a href="%{url}">'.html_safe % { url: help_page_path("user/profile/account/delete_account", anchor: "associated-records") }
-  = _('For more information, please refer to the %{link_start}user account deletion documentation.%{link_end}').html_safe % { link_start: link_start, link_end: '</a>'.html_safe }
+  = s_('AdminUsers|For more information, please refer to the %{link_start}user account deletion documentation.%{link_end}').html_safe % { link_start: link_start, link_end: '</a>'.html_safe }

app/views/admin/users/show.html.haml

@@ -172,13 +172,7 @@
       - if @user.blocked?
         - if @user.blocked_pending_approval?
           = render 'admin/users/approve_user', user: @user
-          .card.border-danger
-            .card-header.bg-danger.gl-text-white
-              = s_('AdminUsers|This user has requested access')
-            .card-body
-              = render partial: 'admin/users/user_reject_effects'
-              %br
-              = link_to 'Reject request', admin_user_path(@user, hard_delete: true), method: :delete, class: "btn gl-button delete-user-button btn-danger", data: { confirm: s_('AdminUsers|Are you sure?') }
+          = render 'admin/users/reject_pending_user', user: @user
         - else
           .card.border-info
             .card-header.gl-bg-blue-500.gl-text-white

app/views/devise/mailer/user_admin_rejection.html.haml → app/views/notify/user_admin_rejection_email.html.haml

-= email_default_heading(say_hi(@resource))
-
+= email_default_heading(_('Hello %{name},') % { name: @name })
 %p
-  = _('Your request to join host fix this has been rejected.')
+  = _('Your request to join %{host} has been rejected.').html_safe % { host: link_to(root_url, root_url) }
 %p
   = _('Please contact your GitLab administrator if you think this is an error.')

app/views/devise/mailer/user_admin_rejection.text.erb → app/views/notify/user_admin_rejection_email.text.erb

-<%= say_hi(@resource) %>
+<%= _('Hello %{name},') % { name: @name } %>
 
-<%= _('Your request to join host fix this has been rejected.') %>
+<%= _('Your request to join %{host} has been rejected.') % { host: root_url } %>
 
 <%= _('Please contact your GitLab administrator if you think this is an error.') %>
 

config/locales/devise.en.yml

@@ -32,8 +32,6 @@ en:
         subject: "Password changed by administrator"
       user_admin_approval:
         subject: "Welcome to GitLab!"
-      user_admin_rejection:
-        subject: "GitLab account request rejected"
     omniauth_callbacks:
       failure: "Could not authenticate you from %{kind} because \"%{reason}\"."
       success: "Successfully authenticated from %{kind} account."

config/routes/admin.rb

@@ -18,6 +18,7 @@ namespace :admin do
       put :unlock
       put :confirm
       put :approve
+      delete :reject
       post :impersonate
       patch :disable_two_factor
       delete 'remove/:email_id', action: 'remove_email', as: 'remove_email'

locale/gitlab.pot

@@ -2124,6 +2124,9 @@ msgstr ""
 msgid "AdminUsers|Blocking user has the following effects:"
 msgstr ""
 
+msgid "AdminUsers|Cannot sign in or access instance information"
+msgstr ""
+
 msgid "AdminUsers|Cannot unblock LDAP blocked users"
 msgstr ""
 
@@ -2160,7 +2163,7 @@ msgstr ""
 msgid "AdminUsers|External users cannot see internal or private projects unless access is explicitly granted. Also, external users cannot create projects, groups, or personal snippets."
 msgstr ""
 
-msgid "AdminUsers|If you reject the user:"
+msgid "AdminUsers|For more information, please refer to the %{link_start}user account deletion documentation.%{link_end}"
 msgstr ""
 
 msgid "AdminUsers|Is using seat"
@@ -2199,9 +2202,15 @@ msgstr ""
 msgid "AdminUsers|Regular users have access to their groups and projects"
 msgstr ""
 
+msgid "AdminUsers|Reject"
+msgstr ""
+
 msgid "AdminUsers|Reject request"
 msgstr ""
 
+msgid "AdminUsers|Rejected users:"
+msgstr ""
+
 msgid "AdminUsers|Restore user access to the account, including web, Git and API."
 msgstr ""
 
@@ -2217,12 +2226,6 @@ msgstr ""
 msgid "AdminUsers|Sort by"
 msgstr ""
 
-msgid "AdminUsers|The user can not sign in or access instance information."
-msgstr ""
-
-msgid "AdminUsers|The user will be deleted."
-msgstr ""
-
 msgid "AdminUsers|The user will be logged out"
 msgstr ""
 
@@ -2265,6 +2268,9 @@ msgstr ""
 msgid "AdminUsers|When the user logs back in, their account will reactivate as a fully active account"
 msgstr ""
 
+msgid "AdminUsers|Will be deleted"
+msgstr ""
+
 msgid "AdminUsers|Without projects"
 msgstr ""
 
@@ -12181,9 +12187,6 @@ msgstr ""
 msgid "For more information, go to the "
 msgstr ""
 
-msgid "For more information, please refer to the %{link_start}user account deletion documentation.%{link_end}"
-msgstr ""
-
 msgid "For more information, please review %{link_start_tag}Jaeger's configuration doc%{link_end_tag}"
 msgstr ""
 
@@ -12796,6 +12799,9 @@ msgstr ""
 msgid "GitLab Workhorse"
 msgstr ""
 
+msgid "GitLab account request rejected"
+msgstr ""
+
 msgid "GitLab commit"
 msgstr ""
 
@@ -13834,6 +13840,9 @@ msgstr ""
 msgid "HealthCheck|Unhealthy"
 msgstr ""
 
+msgid "Hello %{name},"
+msgstr ""
+
 msgid "Hello there"
 msgstr ""
 
@@ -28130,6 +28139,9 @@ 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 no active %{type}."
 msgstr ""
 
@@ -31065,6 +31077,9 @@ msgstr ""
 msgid "You are not allowed to push into this branch. Create another branch or open a merge request."
 msgstr ""
 
+msgid "You are not allowed to reject a user"
+msgstr ""
+
 msgid "You are not allowed to unlink your primary login account"
 msgstr ""
 
@@ -31533,6 +31548,9 @@ msgstr ""
 msgid "You've already enabled two-factor authentication using one time password authenticators. In order to register a different device, you must first disable two-factor authentication."
 msgstr ""
 
+msgid "You've rejected %{user}"
+msgstr ""
+
 msgid "YouTube"
 msgstr ""
 
@@ -31767,6 +31785,9 @@ msgstr ""
 msgid "Your request for access has been queued for review."
 msgstr ""
 
+msgid "Your request to join %{host} has been rejected."
+msgstr ""
+
 msgid "Your requirements are being imported. Once finished, you'll receive a confirmation email."
 msgstr ""
 

spec/controllers/admin/users_controller_spec.rb

@@ -102,6 +102,57 @@ RSpec.describe Admin::UsersController do
     end
   end
 
+  describe 'DELETE #reject' do
+    subject { put :reject, params: { id: user.username } }
+
+    context 'when rejecting a pending user' do
+      let(:user) { create(:user, :blocked_pending_approval) }
+
+      it 'hard deletes the user', :sidekiq_inline do
+        subject
+
+        expect(User.exists?(user.id)).to be_falsy
+      end
+
+      it 'displays the rejection message' do
+        subject
+
+        expect(response).to redirect_to(admin_users_path)
+        expect(flash[:notice]).to eq("You've rejected #{user.name}")
+      end
+
+      it 'sends the user a rejection email' do
+        expect_next_instance_of(NotificationService) do |notification|
+          allow(notification).to receive(:user_admin_rejection).with(user.name, user.notification_email)
+        end
+
+        subject
+      end
+    end
+
+    context 'when user is not pending' do
+      let(:user) { create(:user, state: 'active') }
+
+      it 'does not reject and delete the user' do
+        subject
+
+        expect(User.exists?(user.id)).to be_truthy
+      end
+
+      it 'displays the error' do
+        subject
+
+        expect(flash[:alert]).to eq('This user does not have a pending request')
+      end
+
+      it 'does not email the user' do
+        expect(NotificationService).not_to receive(:new)
+
+        subject
+      end
+    end
+  end
+
   describe 'PUT #approve' do
     let(:user) { create(:user, :blocked_pending_approval) }
 

spec/features/admin/users/user_spec.rb

@@ -37,9 +37,7 @@ RSpec.describe 'Admin::Users::User' do
         expect(page).to have_content(user.name)
         expect(page).to have_content('Pending approval')
         expect(page).to have_link('Approve user')
-        expect(page).to have_button('Block user')
-        expect(page).to have_button('Delete user')
-        expect(page).to have_button('Delete user and contributions')
+        expect(page).to have_link('Reject request')
       end
     end
 

spec/policies/global_policy_spec.rb

@@ -150,6 +150,24 @@ RSpec.describe GlobalPolicy do
     end
   end
 
+  describe 'rejecting users' do
+    context 'regular user' do
+      it { is_expected.not_to be_allowed(:reject_user) }
+    end
+
+    context 'admin' do
+      let(:current_user) { create(:admin) }
+
+      context 'when admin mode is enabled', :enable_admin_mode do
+        it { is_expected.to be_allowed(:reject_user) }
+      end
+
+      context 'when admin mode is disabled' do
+        it { is_expected.to be_disallowed(:reject_user) }
+      end
+    end
+  end
+
   describe 'using project statistics filters' do
     context 'regular user' do
       it { is_expected.not_to be_allowed(:use_project_statistics_filters) }

spec/services/notification_service_spec.rb

@@ -2326,6 +2326,20 @@ RSpec.describe NotificationService, :mailer do
     end
   end
 
+  describe '#user_admin_rejection', :deliver_mails_inline do
+    let_it_be(:user) { create(:user, :blocked_pending_approval) }
+
+    before do
+      reset_delivered_emails!
+    end
+
+    it 'sends the user a rejection email' do
+      notification.user_admin_rejection(user.name, user.email)
+
+      should_only_email(user)
+    end
+  end
+
   describe 'GroupMember', :deliver_mails_inline do
     let(:added_user) { create(:user) }
 

spec/services/users/reject_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Users::RejectService do
+  let_it_be(:current_user) { create(:admin) }
+  let(:user) { create(:user, :blocked_pending_approval) }
+
+  subject(:execute) { described_class.new(current_user).execute(user) }
+
+  describe '#execute' do
+    context 'failures' do
+      context 'when the executor user is not allowed to reject users' do
+        let(:current_user) { create(:user) }
+
+        it 'returns error result' do
+          expect(subject[:status]).to eq(:error)
+          expect(subject[:message]).to match(/You are not allowed to reject a user/)
+        end
+      end
+
+      context 'when the executor user is an admin in admin mode', :enable_admin_mode do
+        context 'when user is not in pending approval state' do
+          let(:user) { create(:user, state: 'active') }
+
+          it 'returns error result' do
+            expect(subject[:status]).to eq(:error)
+            expect(subject[:message])
+              .to match(/This user does not have a pending request/)
+          end
+        end
+      end
+    end
+
+    context 'success' do
+      context 'when the executor user is an admin in admin mode', :enable_admin_mode do
+        it 'deletes the user', :sidekiq_inline do
+          subject
+
+          expect(subject[:status]).to eq(:success)
+          expect { User.find(user.id) }.to raise_error(ActiveRecord::RecordNotFound)
+        end
+
+        it 'emails the user on rejection' do
+          expect_next_instance_of(NotificationService) do |notification|
+            allow(notification).to receive(:user_admin_rejection).with(user.name, user.notification_email)
+          end
+
+          subject
+        end
+      end
+    end
+  end
+end