Merge branch '34262-readonly-ldap-attributes' into 'master'

Add read-only state for LDAP syncable attributes

Closes #34262

See merge request gitlab-org/gitlab!24049

app/models/user.rb

@@ -1527,6 +1527,13 @@ class User < ApplicationRecord
   end
 
   def read_only_attribute?(attribute)
+    if Feature.enabled?(:ldap_readonly_attributes, default_enabled: true)
+      enabled = Gitlab::Auth::LDAP::Config.enabled?
+      read_only = attribute.to_sym.in?(UserSyncedAttributesMetadata::SYNCABLE_ATTRIBUTES)
+
+      return true if enabled && read_only
+    end
+
     user_synced_attributes_metadata&.read_only?(attribute)
   end
 

app/services/users/update_service.rb

@@ -53,7 +53,11 @@ module Users
     end
 
     def discard_read_only_attributes
-      discard_synced_attributes
+      if Feature.enabled?(:ldap_readonly_attributes, default_enabled: true)
+        params.reject! { |key, _| @user.read_only_attribute?(key.to_sym) }
+      else
+        discard_synced_attributes
+      end
     end
 
     def discard_synced_attributes

changelogs/unreleased/34262-readonly-ldap-attributes.yml

+---
+title: Make name, email, and location attributes readonly for LDAP enabled instances
+merge_request: 24049
+author:
+type: changed

spec/models/user_spec.rb

@@ -4084,4 +4084,46 @@ describe User, :do_not_mock_admin_mode do
       end
     end
   end
+
+  describe '#read_only_attribute?' do
+    context 'when LDAP server is enabled' do
+      before do
+        allow(Gitlab::Auth::LDAP::Config).to receive(:enabled?).and_return(true)
+      end
+
+      %i[name email location].each do |attribute|
+        it "is true for #{attribute}" do
+          expect(subject.read_only_attribute?(attribute)).to be_truthy
+        end
+      end
+
+      context 'and ldap_readonly_attributes feature is disabled' do
+        before do
+          stub_feature_flags(ldap_readonly_attributes: false)
+        end
+
+        %i[name email location].each do |attribute|
+          it "is false" do
+            expect(subject.read_only_attribute?(attribute)).to be_falsey
+          end
+        end
+      end
+    end
+
+    context 'when synced attributes metadata is present' do
+      it 'delegates to synced_attributes_metadata' do
+        subject.build_user_synced_attributes_metadata
+
+        expect(subject.build_user_synced_attributes_metadata)
+          .to receive(:read_only?).with(:email).and_return('return-value')
+        expect(subject.read_only_attribute?(:email)).to eq('return-value')
+      end
+    end
+
+    context 'when synced attributes metadata is present' do
+      it 'is false for any attribute' do
+        expect(subject.read_only_attribute?(:email)).to be_falsey
+      end
+    end
+  end
 end

spec/services/users/update_service_spec.rb

@@ -55,6 +55,15 @@ describe Users::UpdateService do
       expect(result[:message]).to eq("Emoji is not included in the list")
     end
 
+    it 'ignores read-only attributes' do
+      allow(user).to receive(:read_only_attribute?).with(:name).and_return(true)
+
+      expect do
+        update_user(user, name: 'changed' + user.name)
+        user.reload
+      end.not_to change { user.name }
+    end
+
     def update_user(user, opts)
       described_class.new(user, opts.merge(user: user)).execute
     end