Commit 2a7e9555 authored by Pavel Shutsin's avatar Pavel Shutsin

Add read-only state for LDAP syncable attributes

Name, email & location should be read-only when LDAP server
is enabled instance-wide
parent e5ecbae9
...@@ -1527,6 +1527,13 @@ class User < ApplicationRecord ...@@ -1527,6 +1527,13 @@ class User < ApplicationRecord
end end
def read_only_attribute?(attribute) 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) user_synced_attributes_metadata&.read_only?(attribute)
end end
......
...@@ -53,7 +53,11 @@ module Users ...@@ -53,7 +53,11 @@ module Users
end end
def discard_read_only_attributes 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 end
def discard_synced_attributes def discard_synced_attributes
......
---
title: Make name, email, and location attributes readonly for LDAP enabled instances
merge_request: 24049
author:
type: changed
...@@ -4084,4 +4084,46 @@ describe User, :do_not_mock_admin_mode do ...@@ -4084,4 +4084,46 @@ describe User, :do_not_mock_admin_mode do
end end
end 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 end
...@@ -55,6 +55,15 @@ describe Users::UpdateService do ...@@ -55,6 +55,15 @@ describe Users::UpdateService do
expect(result[:message]).to eq("Emoji is not included in the list") expect(result[:message]).to eq("Emoji is not included in the list")
end 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) def update_user(user, opts)
described_class.new(user, opts.merge(user: user)).execute described_class.new(user, opts.merge(user: user)).execute
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment