Merge branch '257878-allow-registration-in-blocked-pending-approval-state' into 'master'

Allow new user registrations in `blocked pending approval` state

See merge request gitlab-org/gitlab!44398

app/controllers/registrations_controller.rb

@@ -6,6 +6,8 @@ class RegistrationsController < Devise::RegistrationsController
   include RecaptchaExperimentHelper
   include InvisibleCaptchaOnSignup
 
+  BLOCKED_PENDING_APPROVAL_STATE = 'blocked_pending_approval'.freeze
+
   layout :choose_layout
 
   skip_before_action :required_signup_info, :check_two_factor_requirement, only: [:welcome, :update_registration]
@@ -28,6 +30,7 @@ class RegistrationsController < Devise::RegistrationsController
   end
 
   def create
+    set_user_state
     accept_pending_invitations
 
     super do |new_user|
@@ -37,9 +40,9 @@ class RegistrationsController < Devise::RegistrationsController
       yield new_user if block_given?
     end
 
-    # Devise sets a flash message on `create` for a successful signup,
-    # which we don't want to show.
-    flash[:notice] = nil
+    # Devise sets a flash message on both successful & failed signups,
+    # but we only want to show a message if the resource is blocked by a pending approval.
+    flash[:notice] = nil unless resource.blocked_pending_approval?
   rescue Gitlab::Access::AccessDeniedError
     redirect_to(new_user_session_path)
   end
@@ -121,6 +124,8 @@ class RegistrationsController < Devise::RegistrationsController
 
   def after_inactive_sign_up_path_for(resource)
     Gitlab::AppLogger.info(user_created_message)
+    return new_user_session_path(anchor: 'login-pane') if resource.blocked_pending_approval?
+
     Feature.enabled?(:soft_email_confirmation) ? dashboard_projects_path : users_almost_there_path
   end
 
@@ -235,6 +240,13 @@ class RegistrationsController < Devise::RegistrationsController
       !helpers.in_oauth_flow? &&
       !helpers.in_trial_flow?
   end
+
+  def set_user_state
+    return unless Feature.enabled?(:admin_approval_for_new_user_signups)
+    return unless Gitlab::CurrentSettings.require_admin_approval_after_user_signup
+
+    resource.state = BLOCKED_PENDING_APPROVAL_STATE
+  end
 end
 
 RegistrationsController.prepend_if_ee('EE::RegistrationsController')

config/locales/devise.en.yml

@@ -45,6 +45,7 @@ en:
       signed_up_but_inactive: "You have signed up successfully. However, we could not sign you in because your account is not yet activated."
       signed_up_but_locked: "You have signed up successfully. However, we could not sign you in because your account is locked."
       signed_up_but_unconfirmed: "A message with a confirmation link has been sent to your email address. Please follow the link to activate your account."
+      signed_up_but_blocked_pending_approval: "You have signed up successfully. However, we could not sign you in because your account is awaiting approval from your administrator."
       update_needs_confirmation: "You updated your account successfully, but we need to verify your new email address. Please check your email and follow the confirm link to confirm your new email address."
       updated: "Your account has been updated successfully."
     sessions:

spec/controllers/registrations_controller_spec.rb

@@ -83,19 +83,117 @@ RSpec.describe RegistrationsController do
     let(:base_user_params) { { first_name: 'first', last_name: 'last', username: 'new_username', email: 'new@user.com', password: 'Any_password' } }
     let(:user_params) { { user: base_user_params } }
 
+    subject { post(:create, params: user_params) }
+
+    context '`blocked_pending_approval` state' do
+      context 'when the feature is enabled' do
+        before do
+          stub_feature_flags(admin_approval_for_new_user_signups: true)
+        end
+
+        context 'when the `require_admin_approval_after_user_signup` setting is turned on' do
+          before do
+            stub_application_setting(require_admin_approval_after_user_signup: true)
+          end
+
+          it 'signs up the user in `blocked_pending_approval` state' do
+            subject
+            created_user = User.find_by(email: 'new@user.com')
+
+            expect(created_user).to be_present
+            expect(created_user.blocked_pending_approval?).to eq(true)
+          end
+
+          it 'does not log in the user after sign up' do
+            subject
+
+            expect(controller.current_user).to be_nil
+          end
+
+          it 'shows flash message after signing up' do
+            subject
+
+            expect(response).to redirect_to(new_user_session_path(anchor: 'login-pane'))
+            expect(flash[:notice])
+              .to eq('You have signed up successfully. However, we could not sign you in because your account is awaiting approval from your administrator.')
+          end
+
+          context 'email confirmation' do
+            context 'when `send_user_confirmation_email` is true' do
+              before do
+                stub_application_setting(send_user_confirmation_email: true)
+              end
+
+              it 'does not send a confirmation email' do
+                expect { subject }
+                  .not_to have_enqueued_mail(DeviseMailer, :confirmation_instructions)
+              end
+            end
+          end
+        end
+
+        context 'when the `require_admin_approval_after_user_signup` setting is turned off' do
+          before do
+            stub_application_setting(require_admin_approval_after_user_signup: false)
+          end
+
+          it 'signs up the user in `active` state' do
+            subject
+            created_user = User.find_by(email: 'new@user.com')
+
+            expect(created_user).to be_present
+            expect(created_user.active?).to eq(true)
+          end
+
+          it 'does not show any flash message after signing up' do
+            subject
+
+            expect(flash[:notice]).to be_nil
+          end
+
           context 'email confirmation' do
-      around do |example|
-        perform_enqueued_jobs do
-          example.run
+            context 'when `send_user_confirmation_email` is true' do
+              before do
+                stub_application_setting(send_user_confirmation_email: true)
+              end
+
+              it 'sends a confirmation email' do
+                expect { subject }
+                  .to have_enqueued_mail(DeviseMailer, :confirmation_instructions)
               end
             end
+          end
+        end
+      end
+
+      context 'when the feature is disabled' do
+        before do
+          stub_feature_flags(admin_approval_for_new_user_signups: false)
+        end
+
+        context 'when the `require_admin_approval_after_user_signup` setting is turned on' do
+          before do
+            stub_application_setting(require_admin_approval_after_user_signup: true)
+          end
 
+          it 'signs up the user in `active` state' do
+            subject
+
+            created_user = User.find_by(email: 'new@user.com')
+            expect(created_user).to be_present
+            expect(created_user.active?).to eq(true)
+          end
+        end
+      end
+    end
+
+    context 'email confirmation' do
       context 'when send_user_confirmation_email is false' do
         it 'signs the user in' do
           stub_application_setting(send_user_confirmation_email: false)
 
-          expect { post(:create, params: user_params) }.not_to change { ActionMailer::Base.deliveries.size }
-          expect(subject.current_user).not_to be_nil
+          expect { subject }.not_to have_enqueued_mail(DeviseMailer, :confirmation_instructions)
+          expect(controller.current_user).not_to be_nil
         end
       end
 
@@ -111,10 +209,8 @@ RSpec.describe RegistrationsController do
           end
 
           it 'does not authenticate the user and sends a confirmation email' do
-            post(:create, params: user_params)
-
-            expect(ActionMailer::Base.deliveries.last.to.first).to eq(user_params[:user][:email])
-            expect(subject.current_user).to be_nil
+            expect { subject }.to have_enqueued_mail(DeviseMailer, :confirmation_instructions)
+            expect(controller.current_user).to be_nil
           end
         end
 
@@ -125,9 +221,8 @@ RSpec.describe RegistrationsController do
           end
 
           it 'authenticates the user and sends a confirmation email' do
-            post(:create, params: user_params)
-
-            expect(ActionMailer::Base.deliveries.last.to.first).to eq(user_params[:user][:email])
+            expect { subject }.to have_enqueued_mail(DeviseMailer, :confirmation_instructions)
+            expect(controller.current_user).to be_present
             expect(response).to redirect_to(users_sign_up_welcome_path)
           end
         end
@@ -137,7 +232,7 @@ RSpec.describe RegistrationsController do
         it 'redirects to sign_in' do
           stub_application_setting(signup_enabled: false)
 
-          expect { post(:create, params: user_params) }.not_to change(User, :count)
+          expect { subject }.not_to change(User, :count)
           expect(response).to redirect_to(new_user_session_path)
         end
       end
@@ -158,14 +253,14 @@ RSpec.describe RegistrationsController do
       it 'displays an error when the reCAPTCHA is not solved' do
         allow_any_instance_of(described_class).to receive(:verify_recaptcha).and_return(false)
 
-        post(:create, params: user_params)
+        subject
 
         expect(response).to render_template(:new)
         expect(flash[:alert]).to eq(_('There was an error with the reCAPTCHA. Please solve the reCAPTCHA again.'))
       end
 
       it 'redirects to the welcome page when the reCAPTCHA is solved' do
-        post(:create, params: user_params)
+        subject
 
         expect(response).to redirect_to(users_sign_up_welcome_path)
       end
@@ -264,7 +359,7 @@ RSpec.describe RegistrationsController do
       end
 
       it 'redirects back with a notice when the checkbox was not checked' do
-        post :create, params: user_params
+        subject
 
         expect(flash[:alert]).to eq(_('You must accept our Terms of Service and privacy policy in order to register an account'))
       end
@@ -272,8 +367,8 @@ RSpec.describe RegistrationsController do
       it 'creates the user with agreement when terms are accepted' do
         post :create, params: user_params.merge(terms_opt_in: '1')
 
-        expect(subject.current_user).to be_present
-        expect(subject.current_user.terms_accepted?).to be(true)
+        expect(controller.current_user).to be_present
+        expect(controller.current_user.terms_accepted?).to be(true)
       end
 
       context 'when experiment terms_opt_in is enabled' do
@@ -287,10 +382,10 @@ RSpec.describe RegistrationsController do
           end
 
           it 'creates the user with accepted terms' do
-            post :create, params: user_params
+            subject
 
-            expect(subject.current_user).to be_present
-            expect(subject.current_user.terms_accepted?).to be(true)
+            expect(controller.current_user).to be_present
+            expect(controller.current_user.terms_accepted?).to be(true)
           end
         end
 
@@ -300,7 +395,7 @@ RSpec.describe RegistrationsController do
           end
 
           it 'creates the user without accepted terms' do
-            post :create, params: user_params
+            subject
 
             expect(flash[:alert]).to eq(_('You must accept our Terms of Service and privacy policy in order to register an account'))
           end
@@ -310,8 +405,6 @@ RSpec.describe RegistrationsController do
 
     describe 'tracking data' do
       context 'with sign up flow and terms_opt_in experiment being enabled' do
-        subject { post :create, params: user_params }
-
         before do
           stub_experiment(signup_flow: true, terms_opt_in: true)
         end
@@ -361,13 +454,13 @@ RSpec.describe RegistrationsController do
     it "logs a 'User Created' message" do
       expect(Gitlab::AppLogger).to receive(:info).with(/\AUser Created: username=new_username email=new@user.com.+\z/).and_call_original
 
-      post(:create, params: user_params)
+      subject
     end
 
     it 'handles when params are new_user' do
       post(:create, params: { new_user: base_user_params })
 
-      expect(subject.current_user).not_to be_nil
+      expect(controller.current_user).not_to be_nil
     end
 
     it 'sets name from first and last name' do