Merge branch 'fj-37528-error-after-disabling-ldap' into 'master'

app/models/identity.rb

@@ -9,6 +9,7 @@ class Identity < ActiveRecord::Base
   validates :user_id, uniqueness: { scope: :provider }
 
   before_save :ensure_normalized_extern_uid, if: :extern_uid_changed?
+  after_destroy :clear_user_synced_attributes, if: :user_synced_attributes_metadata_from_provider?
 
   scope :with_provider, ->(provider) { where(provider: provider) }
   scope :with_extern_uid, ->(provider, extern_uid) do
@@ -34,4 +35,12 @@ class Identity < ActiveRecord::Base
 
     self.extern_uid = Identity.normalize_uid(self.provider, self.extern_uid)
   end
+
+  def user_synced_attributes_metadata_from_provider?
+    user.user_synced_attributes_metadata&.provider == provider
+  end
+
+  def clear_user_synced_attributes
+    user.user_synced_attributes_metadata&.destroy
+  end
 end

changelogs/unreleased/fj-37528-error-after-disabling-ldap.yml

+---
+title: Fixed error 500 when removing an identity with synced attributes and visiting
+  the profile page
+merge_request: 17054
+author:
+type: fixed

lib/gitlab/ldap/config.rb

@@ -15,7 +15,7 @@ module Gitlab
       end
 
       def self.servers
-        Gitlab.config.ldap.servers.values
+        Gitlab.config.ldap['servers']&.values || []
       end
 
       def self.available_servers

lib/gitlab/o_auth/user.rb

@@ -198,9 +198,11 @@ module Gitlab
       end
 
       def update_profile
+        clear_user_synced_attributes_metadata
+
         return unless sync_profile_from_provider? || creating_linked_ldap_user?
 
-        metadata = gl_user.user_synced_attributes_metadata || gl_user.build_user_synced_attributes_metadata
+        metadata = gl_user.build_user_synced_attributes_metadata
 
         if sync_profile_from_provider?
           UserSyncedAttributesMetadata::SYNCABLE_ATTRIBUTES.each do |key|
@@ -221,6 +223,10 @@ module Gitlab
         end
       end
 
+      def clear_user_synced_attributes_metadata
+        gl_user.user_synced_attributes_metadata&.destroy
+      end
+
       def log
         Gitlab::AppLogger
       end

spec/lib/gitlab/ldap/config_spec.rb

@@ -5,6 +5,14 @@ describe Gitlab::LDAP::Config do
 
   let(:config) { described_class.new('ldapmain') }
 
+  describe '.servers' do
+    it 'returns empty array if no server information is available' do
+      allow(Gitlab.config).to receive(:ldap).and_return('enabled' => false)
+
+      expect(described_class.servers).to eq []
+    end
+  end
+
   describe '#initialize' do
     it 'requires a provider' do
       expect { described_class.new }.to raise_error ArgumentError

spec/lib/gitlab/o_auth/user_spec.rb

@@ -712,6 +712,10 @@ describe Gitlab::OAuth::User do
       it "does not update the user location" do
         expect(gl_user.location).not_to eq(info_hash[:address][:country])
       end
+
+      it 'does not create associated user synced attributes metadata' do
+        expect(gl_user.user_synced_attributes_metadata).to be_nil
+      end
     end
   end
 

spec/models/identity_spec.rb

@@ -70,5 +70,38 @@ describe Identity do
         end
       end
     end
+
+    context 'after_destroy' do
+      let!(:user) { create(:user) }
+      let(:ldap_identity) { create(:identity, provider: 'ldapmain', extern_uid: 'uid=john smith,ou=people,dc=example,dc=com', user: user) }
+      let(:ldap_user_synced_attributes) { { provider: 'ldapmain', name_synced: true, email_synced: true } }
+      let(:other_provider_user_synced_attributes) { { provider: 'other', name_synced: true, email_synced: true } }
+
+      describe 'if user synced attributes metadada provider' do
+        context 'matches the identity provider ' do
+          it 'removes the user synced attributes' do
+            user.create_user_synced_attributes_metadata(ldap_user_synced_attributes)
+
+            expect(user.user_synced_attributes_metadata.provider).to eq 'ldapmain'
+
+            ldap_identity.destroy
+
+            expect(user.reload.user_synced_attributes_metadata).to be_nil
+          end
+        end
+
+        context 'does not matche the identity provider' do
+          it 'does not remove the user synced attributes' do
+            user.create_user_synced_attributes_metadata(other_provider_user_synced_attributes)
+
+            expect(user.user_synced_attributes_metadata.provider).to eq 'other'
+
+            ldap_identity.destroy
+
+            expect(user.reload.user_synced_attributes_metadata.provider).to eq 'other'
+          end
+        end
+      end
+    end
   end
 end