Merge branch '31294-fix-oauth-users-do-not-need-to-be-confirmed' into '9-1-stable'

Ensures that OAuth/LDAP/SAML users don't need to be confirmed

See merge request !10925

app/services/users/create_service.rb

@@ -9,15 +9,13 @@ module Users
     def build(skip_authorization: false)
       raise Gitlab::Access::AccessDeniedError unless skip_authorization || can_create_user?
-      user = User.new(build_user_params)
+      user_params = build_user_params(skip_authorization: skip_authorization)
+      user = User.new(user_params)
       if current_user&.admin?
-        if params[:reset_password]
-          @reset_token = user.generate_reset_token
-          params[:force_random_password] = true
-        end
-        if params[:force_random_password]
+        @reset_token = user.generate_reset_token if params[:reset_password]
+        if user_params[:force_random_password]
           random_password = Devise.friendly_token.first(Devise.password_length.min)
           user.password = user.password_confirmation = random_password
         end
@@ -93,7 +91,7 @@ module Users
       ]
     end
-    def build_user_params
+    def build_user_params(skip_authorization:)
       if current_user&.admin?
         user_params = params.slice(*admin_create_params)
         user_params[:created_by_id] = current_user&.id
@@ -102,11 +100,20 @@ module Users
           user_params.merge!(force_random_password: true, password_expires_at: nil)
         end
       else
-        user_params = params.slice(*signup_params)
-        user_params[:skip_confirmation] = !current_application_settings.send_user_confirmation_email
+        allowed_signup_params = signup_params
+        allowed_signup_params << :skip_confirmation if skip_authorization
+
+        user_params = params.slice(*allowed_signup_params)
+        if user_params[:skip_confirmation].nil?
+          user_params[:skip_confirmation] = skip_user_confirmation_email_from_setting
+        end
       end
       user_params
     end
+
+    def skip_user_confirmation_email_from_setting
+      !current_application_settings.send_user_confirmation_email
+    end
   end
 end

changelogs/unreleased/31294-fix-ldap-user-not-confirmed.yml

+---
+title: Ensures that OAuth/LDAP/SAML users don't need to be confirmed
+merge_request:
+author:

spec/lib/gitlab/ldap/user_spec.rb

@@ -120,6 +120,19 @@ describe Gitlab::LDAP::User, lib: true do
         expect(gl_user).to be_persisted
       end
     end
+
+    context 'when user confirmation email is enabled' do
+      before do
+        stub_application_setting send_user_confirmation_email: true
+      end
+
+      it 'creates and confirms the user anyway' do
+        ldap_user.save
+
+        expect(gl_user).to be_persisted
+        expect(gl_user).to be_confirmed
+      end
+    end
   end
   describe 'updating email' do

spec/lib/gitlab/o_auth/user_spec.rb

@@ -54,6 +54,21 @@ describe Gitlab::OAuth::User, lib: true do
         end
       end
+      context 'when user confirmation email is enabled' do
+        before do
+          stub_application_setting send_user_confirmation_email: true
+        end
+
+        it 'creates and confirms the user anyway' do
+          stub_omniauth_config(allow_single_sign_on: ['twitter'])
+
+          oauth_user.save
+
+          expect(gl_user).to be_persisted
+          expect(gl_user).to be_confirmed
+        end
+      end
+
       it 'marks user as having password_automatically_set' do
         stub_omniauth_config(allow_single_sign_on: ['twitter'], external_providers: ['twitter'])

spec/lib/gitlab/saml/user_spec.rb

@@ -223,6 +223,19 @@ describe Gitlab::Saml::User, lib: true do
           expect(gl_user).to be_persisted
         end
       end
+
+      context 'when user confirmation email is enabled' do
+        before do
+          stub_application_setting send_user_confirmation_email: true
+        end
+
+        it 'creates and confirms the user anyway' do
+          saml_user.save
+
+          expect(gl_user).to be_persisted
+          expect(gl_user).to be_confirmed
+        end
+      end
     end
     describe 'blocking' do