Merge branch '257878-introduce-blocked-pending-approval-user-status' into 'master'

Introduce `blocked_pending_approval` user state

See merge request gitlab-org/gitlab!44260

app/models/user.rb

@@ -304,13 +304,18 @@ class User < ApplicationRecord
       transition deactivated: :active
       transition blocked: :active
       transition ldap_blocked: :active
+      transition blocked_pending_approval: :active
+    end
+
+    event :block_pending_approval do
+      transition active: :blocked_pending_approval
     end
 
     event :deactivate do
       transition active: :deactivated
     end
 
-    state :blocked, :ldap_blocked do
+    state :blocked, :ldap_blocked, :blocked_pending_approval do
       def blocked?
         true
       end
@@ -333,7 +338,7 @@ class User < ApplicationRecord
 
   # Scopes
   scope :admins, -> { where(admin: true) }
-  scope :blocked, -> { with_states(:blocked, :ldap_blocked) }
+  scope :blocked, -> { with_states(:blocked, :ldap_blocked, :blocked_pending_approval) }
   scope :external, -> { where(external: true) }
   scope :confirmed, -> { where.not(confirmed_at: nil) }
   scope :active, -> { with_state(:active).non_internal }
@@ -378,7 +383,9 @@ class User < ApplicationRecord
 
   # The messages for these keys are defined in `devise.en.yml`
   def inactive_message
-    if blocked?
+    if blocked_pending_approval?
+      :blocked_pending_approval
+    elsif blocked?
       :blocked
     elsif internal?
       :forbidden

config/locales/devise.en.yml

@@ -18,6 +18,7 @@ en:
       unconfirmed: "You have to confirm your email address before continuing. Please check your email for the link we sent you, or click 'Resend confirmation email'."
       blocked: "Your account has been blocked. Please contact your GitLab administrator if you think this is an error."
       forbidden: "Your account does not have the required permission to login. Please contact your GitLab administrator if you think this is an error."
+      blocked_pending_approval: "Your account is pending approval from your GitLab administrator and hence blocked. Please contact your GitLab administrator if you think this is an error."
     mailer:
       confirmation_instructions:
         subject: "Confirmation instructions"

lib/gitlab/auth/user_access_denied_reason.rb

@@ -11,6 +11,8 @@ module Gitlab
         case rejection_type
         when :internal
           "This action cannot be performed by internal users"
+        when :blocked_pending_approval
+          "Your account is pending approval from your administrator and hence blocked."
         when :terms_not_accepted
           "You (#{@user.to_reference}) must accept the Terms of Service in order to perform this action. "\
           "Please access GitLab from a web browser to accept these terms."
@@ -31,6 +33,8 @@ module Gitlab
       def rejection_type
         if @user.internal?
           :internal
+        elsif @user.blocked_pending_approval?
+          :blocked_pending_approval
         elsif @user.required_terms_not_accepted?
           :terms_not_accepted
         elsif @user.deactivated?

spec/controllers/sessions_controller_spec.rb

@@ -100,6 +100,16 @@ RSpec.describe SessionsController do
         end
       end
 
+      context 'a `blocked pending approval` user' do
+        it 'does not authenticate the user' do
+          user.block_pending_approval!
+          post_action
+
+          expect(@request.env['warden']).not_to be_authenticated
+          expect(flash[:alert]).to include('Your account is pending approval from your GitLab administrator and hence blocked')
+        end
+      end
+
       context 'an internal user' do
         it 'does not authenticate the user' do
           user.ghost!

spec/factories/users.rb

@@ -23,6 +23,14 @@ FactoryBot.define do
       after(:build) { |user, _| user.block! }
     end
 
+    trait :blocked_pending_approval do
+      after(:build) { |user, _| user.block_pending_approval! }
+    end
+
+    trait :ldap_blocked do
+      after(:build) { |user, _| user.ldap_block! }
+    end
+
     trait :bot do
       user_type { :alert_bot }
     end

spec/lib/gitlab/auth/user_access_denied_reason_spec.rb

@@ -49,5 +49,13 @@ RSpec.describe Gitlab::Auth::UserAccessDeniedReason do
 
       it { is_expected.to match /Your primary email address is not confirmed/ }
     end
+
+    context 'when the user is blocked pending approval' do
+      before do
+        user.block_pending_approval!
+      end
+
+      it { is_expected.to eq('Your account is pending approval from your administrator and hence blocked.') }
+    end
   end
 end

spec/lib/gitlab/auth_spec.rb

@@ -726,6 +726,12 @@ RSpec.describe Gitlab::Auth, :use_clean_rails_memory_store_caching do
       expect( gl_auth.find_with_user_password(username, password) ).not_to eql user
     end
 
+    it 'does not find user in blocked_pending_approval state' do
+      user.block_pending_approval
+
+      expect( gl_auth.find_with_user_password(username, password) ).not_to eql user
+    end
+
     context 'with increment_failed_attempts' do
       wrong_password = 'incorrect_password'
 

spec/lib/gitlab/git_access_spec.rb

@@ -420,6 +420,13 @@ RSpec.describe Gitlab::GitAccess do
       expect { pull_access_check }.to raise_forbidden('Your account has been blocked.')
     end
 
+    it 'disallows users that are blocked pending approval to pull' do
+      project.add_maintainer(user)
+      user.block_pending_approval
+
+      expect { pull_access_check }.to raise_forbidden('Your account is pending approval from your administrator and hence blocked.')
+    end
+
     it 'disallows deactivated users to pull' do
       project.add_maintainer(user)
       user.deactivate!
@@ -915,6 +922,12 @@ RSpec.describe Gitlab::GitAccess do
         project.add_developer(user)
       end
 
+      it 'disallows users that are blocked pending approval to push' do
+        user.block_pending_approval
+
+        expect { push_access_check }.to raise_forbidden('Your account is pending approval from your administrator and hence blocked.')
+      end
+
       it 'does not allow deactivated users to push' do
         user.deactivate!
 

spec/models/user_spec.rb

@@ -705,6 +705,25 @@ RSpec.describe User do
   end
 
   describe "scopes" do
+    describe '.blocked' do
+      subject { described_class.blocked }
+
+      it 'returns only blocked users' do
+        active_user = create(:user)
+        blocked_user = create(:user, :blocked)
+        blocked_pending_approval_user = create(:user, :blocked_pending_approval)
+        ldap_blocked_user = create(:omniauth_user, :ldap_blocked)
+
+        expect(subject).to include(
+          blocked_user,
+          blocked_pending_approval_user,
+          ldap_blocked_user
+        )
+
+        expect(subject).not_to include(active_user)
+      end
+    end
+
     describe ".with_two_factor" do
       it "returns users with 2fa enabled via OTP" do
         user_with_2fa = create(:user, :two_factor_via_otp)
@@ -1694,6 +1713,24 @@ RSpec.describe User do
     end
   end
 
+  describe 'blocking a user pending approval' do
+    let(:user) { create(:user) }
+
+    before do
+      user.block_pending_approval
+    end
+
+    context 'an active user' do
+      it 'can be blocked pending approval' do
+        expect(user.blocked_pending_approval?).to eq(true)
+      end
+
+      it 'behaves like a blocked user' do
+        expect(user.blocked?).to eq(true)
+      end
+    end
+  end
+
   describe '.filter_items' do
     let(:user) { double }
 
@@ -4917,6 +4954,14 @@ RSpec.describe User do
 
       it { is_expected.to be :locked }
     end
+
+    context 'when user is blocked pending approval' do
+      before do
+        user.block_pending_approval!
+      end
+
+      it { is_expected.to be :blocked_pending_approval }
+    end
   end
 
   describe '#password_required?' do

spec/policies/global_policy_spec.rb

@@ -187,6 +187,14 @@ RSpec.describe GlobalPolicy do
       it { is_expected.not_to be_allowed(:access_api) }
     end
 
+    context 'user blocked pending approval' do
+      before do
+        current_user.block_pending_approval
+      end
+
+      it { is_expected.not_to be_allowed(:access_api) }
+    end
+
     context 'when terms are enforced' do
       before do
         enforce_terms
@@ -276,6 +284,14 @@ RSpec.describe GlobalPolicy do
 
       it { is_expected.not_to be_allowed(:receive_notifications) }
     end
+
+    context 'user blocked pending approval' do
+      before do
+        current_user.block_pending_approval
+      end
+
+      it { is_expected.not_to be_allowed(:receive_notifications) }
+    end
   end
 
   describe 'git access' do
@@ -344,6 +360,14 @@ RSpec.describe GlobalPolicy do
 
       it { is_expected.to be_allowed(:access_git) }
     end
+
+    context 'user blocked pending approval' do
+      before do
+        current_user.block_pending_approval
+      end
+
+      it { is_expected.not_to be_allowed(:access_git) }
+    end
   end
 
   describe 'read instance metadata' do
@@ -412,6 +436,14 @@ RSpec.describe GlobalPolicy do
 
       it { is_expected.not_to be_allowed(:use_slash_commands) }
     end
+
+    context 'user blocked pending approval' do
+      before do
+        current_user.block_pending_approval
+      end
+
+      it { is_expected.not_to be_allowed(:use_slash_commands) }
+    end
   end
 
   describe 'create_snippet' do
@@ -444,5 +476,13 @@ RSpec.describe GlobalPolicy do
 
       it { is_expected.not_to be_allowed(:log_in) }
     end
+
+    context 'user blocked pending approval' do
+      before do
+        current_user.block_pending_approval
+      end
+
+      it { is_expected.not_to be_allowed(:log_in) }
+    end
   end
 end

spec/requests/api/doorkeeper_access_spec.rb

@@ -71,4 +71,12 @@ RSpec.describe 'doorkeeper access' do
 
     it_behaves_like 'forbidden request'
   end
+
+  context 'when user is blocked pending approval' do
+    before do
+      user.block_pending_approval
+    end
+
+    it_behaves_like 'forbidden request'
+  end
 end