Merge branch '197925-setting-minimum-password-length-breaks-sign-up-with-saml' into 'master'

Resolve "Setting minimum password length breaks sign-up with SAML"

Closes #197925

See merge request gitlab-org/gitlab!23387

app/models/user.rb

@@ -394,6 +394,11 @@ class User < ApplicationRecord
       Gitlab::CurrentSettings.minimum_password_length..Devise.password_length.max
     end
 
+    # Generate a random password that conforms to the current password length settings
+    def random_password
+      Devise.friendly_token(password_length.max)
+    end
+
     # Devise method overridden to allow sign in with email or username
     def find_for_database_authentication(warden_conditions)
       conditions = warden_conditions.dup

app/services/users/build_service.rb

@@ -23,7 +23,7 @@ module Users
         @reset_token = user.generate_reset_token if params[:reset_password]
 
         if user_params[:force_random_password]
-          random_password = Devise.friendly_token.first(User.password_length.min)
+          random_password = User.random_password
           user.password = user.password_confirmation = random_password
         end
       end

changelogs/unreleased/197925-setting-minimum-password-length-breaks-sign-up-with-saml.yml

+---
+title: Fixes random passwords generated not conforming to minimum_password_length setting
+merge_request: 23387
+author:
+type: fixed

ee/lib/ee/gitlab/scim/provisioning_service.rb

@@ -77,7 +77,7 @@ module EE
         end
 
         def random_password
-          Devise.friendly_token.first(::User.password_length.min)
+          ::User.random_password
         end
 
         def valid_username

ee/lib/gitlab/auth/smartcard/base.rb

@@ -67,6 +67,10 @@ module Gitlab
         def valid?
           self.class.store.verify(@certificate) if @certificate
         end
+
+        def password
+          @password ||= ::User.random_password
+        end
       end
     end
   end

ee/lib/gitlab/auth/smartcard/certificate.rb

@@ -75,10 +75,6 @@ module Gitlab
         def username
           ::Namespace.clean_path(common_name)
         end
-
-        def password
-          @password ||= Devise.friendly_token(8)
-        end
       end
     end
   end

ee/lib/gitlab/auth/smartcard/ldap_certificate.rb

@@ -64,10 +64,6 @@ module Gitlab
         def username
           ::Namespace.clean_path(ldap_user.username)
         end
-
-        def password
-          @password ||= Devise.friendly_token(8)
-        end
       end
     end
   end

ee/spec/lib/ee/gitlab/scim/provisioning_service_spec.rb

@@ -53,6 +53,16 @@ describe ::EE::Gitlab::Scim::ProvisioningService do
         expect(user).not_to be_confirmed
       end
 
+      context 'when the current minimum password length is different from the default minimum password length' do
+        before do
+          stub_application_setting minimum_password_length: 21
+        end
+
+        it 'creates the user' do
+          expect { service.execute }.to change { User.count }.by(1)
+        end
+      end
+
       context 'existing user' do
         before do
           create(:user, email: 'work@example.com')

ee/spec/lib/gitlab/auth/smartcard/certificate_spec.rb

@@ -91,10 +91,22 @@ describe Gitlab::Auth::Smartcard::Certificate do
         allow(Gitlab::Auth::Smartcard).to receive(:enabled?).and_return(true)
       end
 
-      it 'creates user' do
-        expect { subject }.to change { User.count }.from(0).to(1)
-        expect(User.first.username).to eql('gitlab-user')
-        expect(User.first.email).to eql('gitlab-user@random-corp.org')
+      shared_examples_for 'creates user' do
+        it do
+          expect { subject }.to change { User.count }.from(0).to(1)
+          expect(User.first.username).to eql('gitlab-user')
+          expect(User.first.email).to eql('gitlab-user@random-corp.org')
+        end
+      end
+
+      it_behaves_like 'creates user'
+
+      context 'when the current minimum password length is different from the default minimum password length' do
+        before do
+          stub_application_setting minimum_password_length: 21
+        end
+
+        it_behaves_like 'creates user'
       end
 
       it_behaves_like 'a new smartcard identity'

ee/spec/lib/gitlab/auth/smartcard/ldap_certificate_spec.rb

@@ -91,8 +91,20 @@ describe Gitlab::Auth::Smartcard::LDAPCertificate do
     context 'user does not exist' do
       let(:user) { create(:user) }
 
-      it 'creates user' do
-        expect { subject }.to change { User.count }.from(0).to(1)
+      shared_examples_for 'creates user' do
+        it do
+          expect { subject }.to change { User.count }.from(0).to(1)
+        end
+      end
+
+      it_behaves_like 'creates user'
+
+      context 'when the current minimum password length is different from the default minimum password length' do
+        before do
+          stub_application_setting minimum_password_length: 21
+        end
+
+        it_behaves_like 'creates user'
       end
 
       it 'creates user with correct attributes' do

lib/gitlab/auth/o_auth/auth_hash.rb

@@ -34,7 +34,7 @@ module Gitlab
         end
 
         def password
-          @password ||= Gitlab::Utils.force_utf8(Devise.friendly_token[0, 8].downcase)
+          @password ||= Gitlab::Utils.force_utf8(::User.random_password.downcase)
         end
 
         def location

spec/lib/gitlab/auth/ldap/user_spec.rb

@@ -139,6 +139,18 @@ describe Gitlab::Auth::LDAP::User do
         expect(gl_user).to be_confirmed
       end
     end
+
+    context 'when the current minimum password length is different from the default minimum password length' do
+      before do
+        stub_application_setting minimum_password_length: 21
+      end
+
+      it 'creates the user' do
+        ldap_user.save
+
+        expect(gl_user).to be_persisted
+      end
+    end
   end
 
   describe 'updating email' do

spec/lib/gitlab/auth/o_auth/user_spec.rb

@@ -86,6 +86,20 @@ describe Gitlab::Auth::OAuth::User do
         end
       end
 
+      context 'when the current minimum password length is different from the default minimum password length' do
+        before do
+          stub_application_setting minimum_password_length: 21
+        end
+
+        it 'creates the user' do
+          stub_omniauth_config(allow_single_sign_on: [provider])
+
+          oauth_user.save
+
+          expect(gl_user).to be_persisted
+        end
+      end
+
       it 'marks user as having password_automatically_set' do
         stub_omniauth_config(allow_single_sign_on: [provider], external_providers: [provider])
 

spec/lib/gitlab/auth/saml/user_spec.rb

@@ -325,6 +325,18 @@ describe Gitlab::Auth::Saml::User do
           expect(gl_user).to be_confirmed
         end
       end
+
+      context 'when the current minimum password length is different from the default minimum password length' do
+        before do
+          stub_application_setting minimum_password_length: 21
+        end
+
+        it 'creates the user' do
+          saml_user.save
+
+          expect(gl_user).to be_persisted
+        end
+      end
     end
 
     describe 'blocking' do

spec/models/user_spec.rb

@@ -508,6 +508,20 @@ describe User, :do_not_mock_admin_mode do
       end
     end
 
+    describe '.random_password' do
+      let(:random_password) { described_class.random_password }
+
+      before do
+        expect(User).to receive(:password_length).and_return(88..128)
+      end
+
+      context 'length' do
+        it 'conforms to the current password length settings' do
+          expect(random_password.length).to eq(128)
+        end
+      end
+    end
+
     describe '.password_length' do
       let(:password_length) { described_class.password_length }