Merge branch '338980-fix-oauth-ldap-tests' into 'master'

Refactor tests related to Omniauth users

See merge request gitlab-org/gitlab!81525

ee/spec/lib/gitlab/auth/ldap/access_spec.rb

@@ -5,7 +5,7 @@ require 'spec_helper'
 RSpec.describe Gitlab::Auth::Ldap::Access do
   include LdapHelpers
 
-  let(:user) { create(:omniauth_user) }
+  let(:user) { create(:omniauth_user, :ldap) }
   let(:provider) { user.ldap_identity.provider }
 
   subject(:access) { described_class.new(user) }

lib/gitlab/auth/ldap/user.rb

@@ -11,9 +11,6 @@ module Gitlab
     module Ldap
       class User < Gitlab::Auth::OAuth::User
         extend ::Gitlab::Utils::Override
-        def save
-          super('LDAP')
-        end
 
         # instance methods
         def find_user
@@ -44,6 +41,10 @@ module Gitlab
         def auth_hash=(auth_hash)
           @auth_hash = Gitlab::Auth::Ldap::AuthHash.new(auth_hash)
         end
+
+        def protocol_name
+          'LDAP'
+        end
       end
     end
   end

lib/gitlab/auth/o_auth/user.rb

@@ -46,7 +46,7 @@ module Gitlab
           valid? && persisted?
         end
 
-        def save(provider = 'OAuth')
+        def save(provider = protocol_name)
           raise SigninDisabledForProviderError if oauth_provider_disabled?
           raise SignupDisabledError unless gl_user
 
@@ -96,6 +96,10 @@ module Gitlab
           end
         end
 
+        def protocol_name
+          'OAuth'
+        end
+
         protected
 
         def should_save?

lib/gitlab/auth/saml/user.rb

@@ -11,10 +11,6 @@ module Gitlab
       class User < Gitlab::Auth::OAuth::User
         extend ::Gitlab::Utils::Override
 
-        def save
-          super('SAML')
-        end
-
         def find_user
           user = find_by_uid_and_provider
 
@@ -40,6 +36,10 @@ module Gitlab
           saml_config.upstream_two_factor_authn_contexts&.include?(auth_hash.authn_context)
         end
 
+        def protocol_name
+          'SAML'
+        end
+
         protected
 
         def saml_config

spec/factories/users.rb

@@ -151,7 +151,7 @@ FactoryBot.define do
 
       transient do
         extern_uid { '123456' }
-        provider { 'ldapmain' }
+        provider { 'twitter' }
       end
 
       after(:create) do |user, evaluator|
@@ -166,6 +166,12 @@ FactoryBot.define do
 
         user.identities << create(:identity, identity_attrs)
       end
+
+      trait :ldap do
+        transient do
+          provider { 'ldapmain' }
+        end
+      end
     end
 
     factory :atlassian_user do

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

@@ -5,7 +5,7 @@ require 'spec_helper'
 RSpec.describe Gitlab::Auth::Ldap::Access do
   include LdapHelpers
 
-  let(:user) { create(:omniauth_user) }
+  let(:user) { create(:omniauth_user, :ldap) }
 
   subject(:access) { described_class.new(user) }
 

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

@@ -4,7 +4,7 @@ require 'spec_helper'
 
 RSpec.describe Gitlab::Auth::Ldap::Authentication do
   let(:dn)       { 'uid=John Smith, ou=People, dc=example, dc=com' }
-  let(:user)     { create(:omniauth_user, extern_uid: Gitlab::Auth::Ldap::Person.normalize_dn(dn)) }
+  let(:user)     { create(:omniauth_user, :ldap, extern_uid: Gitlab::Auth::Ldap::Person.normalize_dn(dn)) }
   let(:login)    { 'john' }
   let(:password) { 'password' }
 

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

@@ -577,28 +577,66 @@ RSpec.describe Gitlab::Auth::OAuth::User do
         stub_omniauth_config(allow_single_sign_on: ['twitter'])
       end
 
-      context 'signup with omniauth only' do
-        context 'dont block on create' do
-          before do
-            stub_omniauth_config(block_auto_created_users: false)
+      shared_examples 'being blocked on creation' do
+        context 'when blocking on creation' do
+          it 'creates a blocked user' do
+            oauth_user.save # rubocop:disable Rails/SaveBang
+            expect(gl_user).to be_valid
+            expect(gl_user).to be_blocked
           end
 
-          it do
+          context 'when a sign up user cap has been set up but has not been reached yet' do
+            it 'still creates a blocked user' do
+              stub_application_setting(new_user_signups_cap: 999)
+
+              oauth_user.save # rubocop:disable Rails/SaveBang
+              expect(gl_user).to be_valid
+              expect(gl_user).to be_blocked
+            end
+          end
+        end
+      end
+
+      shared_examples 'not being blocked on creation' do
+        context 'when not blocking on creation' do
+          it 'creates a non-blocked user' do
             oauth_user.save # rubocop:disable Rails/SaveBang
             expect(gl_user).to be_valid
             expect(gl_user).not_to be_blocked
           end
         end
+      end
+
+      context 'signup with SAML' do
+        let(:provider) { 'saml' }
+
+        before do
+          stub_omniauth_config({
+            allow_single_sign_on: ['saml'],
+            auto_link_saml_user: true,
+            block_auto_created_users: block_auto_created_users
+          })
+        end
+
+        it_behaves_like 'being blocked on creation' do
+          let(:block_auto_created_users) { true }
+        end
+
+        it_behaves_like 'not being blocked on creation' do
+          let(:block_auto_created_users) { false }
+        end
+      end
 
-        context 'block on create' do
+      context 'signup with omniauth only' do
+        it_behaves_like 'being blocked on creation' do
           before do
             stub_omniauth_config(block_auto_created_users: true)
           end
+        end
 
-          it do
-            oauth_user.save # rubocop:disable Rails/SaveBang
-            expect(gl_user).to be_valid
-            expect(gl_user).to be_blocked
+        it_behaves_like 'not being blocked on creation' do
+          before do
+            stub_omniauth_config(block_auto_created_users: false)
           end
         end
       end
@@ -614,64 +652,40 @@ RSpec.describe Gitlab::Auth::OAuth::User do
         end
 
         context "and no account for the LDAP user" do
-          context 'dont block on create (LDAP)' do
+          it_behaves_like 'being blocked on creation' do
             before do
               allow_next_instance_of(Gitlab::Auth::Ldap::Config) do |instance|
-                allow(instance).to receive_messages(block_auto_created_users: false)
+                allow(instance).to receive_messages(block_auto_created_users: true)
               end
             end
-
-            it do
-              oauth_user.save # rubocop:disable Rails/SaveBang
-              expect(gl_user).to be_valid
-              expect(gl_user).not_to be_blocked
-            end
           end
 
-          context 'block on create (LDAP)' do
+          it_behaves_like 'not being blocked on creation' do
             before do
               allow_next_instance_of(Gitlab::Auth::Ldap::Config) do |instance|
-                allow(instance).to receive_messages(block_auto_created_users: true)
+                allow(instance).to receive_messages(block_auto_created_users: false)
               end
             end
-
-            it do
-              oauth_user.save # rubocop:disable Rails/SaveBang
-              expect(gl_user).to be_valid
-              expect(gl_user).to be_blocked
-            end
           end
         end
 
         context 'and LDAP user has an account already' do
           let!(:existing_user) { create(:omniauth_user, email: 'john@example.com', extern_uid: dn, provider: 'ldapmain', username: 'john') }
 
-          context 'dont block on create (LDAP)' do
+          it_behaves_like 'not being blocked on creation' do
             before do
               allow_next_instance_of(Gitlab::Auth::Ldap::Config) do |instance|
                 allow(instance).to receive_messages(block_auto_created_users: false)
               end
             end
-
-            it do
-              oauth_user.save # rubocop:disable Rails/SaveBang
-              expect(gl_user).to be_valid
-              expect(gl_user).not_to be_blocked
-            end
           end
 
-          context 'block on create (LDAP)' do
+          it_behaves_like 'not being blocked on creation' do
             before do
               allow_next_instance_of(Gitlab::Auth::Ldap::Config) do |instance|
                 allow(instance).to receive_messages(block_auto_created_users: true)
               end
             end
-
-            it do
-              oauth_user.save # rubocop:disable Rails/SaveBang
-              expect(gl_user).to be_valid
-              expect(gl_user).not_to be_blocked
-            end
           end
         end
       end
@@ -682,56 +696,32 @@ RSpec.describe Gitlab::Auth::OAuth::User do
           oauth_user.gl_user.activate
         end
 
-        context 'dont block on create' do
+        it_behaves_like 'not being blocked on creation' do
           before do
             stub_omniauth_config(block_auto_created_users: false)
           end
-
-          it do
-            oauth_user.save # rubocop:disable Rails/SaveBang
-            expect(gl_user).to be_valid
-            expect(gl_user).not_to be_blocked
-          end
         end
 
-        context 'block on create' do
+        it_behaves_like 'not being blocked on creation' do
           before do
             stub_omniauth_config(block_auto_created_users: true)
           end
-
-          it do
-            oauth_user.save # rubocop:disable Rails/SaveBang
-            expect(gl_user).to be_valid
-            expect(gl_user).not_to be_blocked
-          end
         end
 
-        context 'dont block on create (LDAP)' do
+        it_behaves_like 'not being blocked on creation' do
           before do
             allow_next_instance_of(Gitlab::Auth::Ldap::Config) do |instance|
               allow(instance).to receive_messages(block_auto_created_users: false)
             end
           end
-
-          it do
-            oauth_user.save # rubocop:disable Rails/SaveBang
-            expect(gl_user).to be_valid
-            expect(gl_user).not_to be_blocked
-          end
         end
 
-        context 'block on create (LDAP)' do
+        it_behaves_like 'not being blocked on creation' do
           before do
             allow_next_instance_of(Gitlab::Auth::Ldap::Config) do |instance|
               allow(instance).to receive_messages(block_auto_created_users: true)
             end
           end
-
-          it do
-            oauth_user.save # rubocop:disable Rails/SaveBang
-            expect(gl_user).to be_valid
-            expect(gl_user).not_to be_blocked
-          end
         end
       end
     end
@@ -1057,4 +1047,10 @@ RSpec.describe Gitlab::Auth::OAuth::User do
       expect(oauth_user.bypass_two_factor?).to be_falsey
     end
   end
+
+  describe '#protocol_name' do
+    it 'is OAuth' do
+      expect(oauth_user.protocol_name).to eq('OAuth')
+    end
+  end
 end

spec/models/user_spec.rb

@@ -3090,7 +3090,7 @@ RSpec.describe User do
 
     describe '#ldap_identity' do
       it 'returns ldap identity' do
-        user = create :omniauth_user
+        user = create(:omniauth_user, :ldap)
 
         expect(user.ldap_identity.provider).not_to be_empty
       end

spec/requests/api/users_spec.rb

@@ -11,6 +11,7 @@ RSpec.describe API::Users do
 
   let(:blocked_user) { create(:user, :blocked) }
   let(:omniauth_user) { create(:omniauth_user) }
+  let(:ldap_user) { create(:omniauth_user, provider: 'ldapmain') }
   let(:ldap_blocked_user) { create(:omniauth_user, provider: 'ldapmain', state: 'ldap_blocked') }
   let(:private_user) { create(:user, private_profile: true) }
   let(:deactivated_user) { create(:user, state: 'deactivated') }
@@ -1293,10 +1294,10 @@ RSpec.describe API::Users do
     end
 
     it "updates user's existing identity" do
-      put api("/users/#{omniauth_user.id}", admin), params: { provider: 'ldapmain', extern_uid: '654321' }
+      put api("/users/#{ldap_user.id}", admin), params: { provider: 'ldapmain', extern_uid: '654321' }
 
       expect(response).to have_gitlab_http_status(:ok)
-      expect(omniauth_user.reload.identities.first.extern_uid).to eq('654321')
+      expect(ldap_user.reload.identities.first.extern_uid).to eq('654321')
     end
 
     it 'updates user with new identity' do