Merge pull request #14941 from virgo47/feature/53330_disable_profile_…

…fields_for_ldap_user

Feature/53330 disable profile fields for ldap user

app/models/permitted_params.rb

@@ -187,8 +187,13 @@ def settings
   end
 
   def user(additional_params = [])
-    permitted_params = params.require(:user).permit(*self.class.permitted_attributes[:user] + additional_params)
-    permitted_params.merge(custom_field_values(:user))
+    if params[:user].present?
+      permitted_params = params.require(:user).permit(*self.class.permitted_attributes[:user] + additional_params)
+      permitted_params.merge(custom_field_values(:user))
+    else
+      # This happens on the Profile page for LDAP user, no "user" hash is sent.
+      {}.merge(custom_field_values(:user, required: false))
+    end
   end
 
   def placeholder_user

app/views/my/account.html.erb

@@ -49,19 +49,31 @@ See COPYRIGHT and LICENSE files for more details.
       </div>
     </div>
     <% login_via_provider = !!@user.identity_url %>
+    <% login_via_ldap = !!@user.ldap_auth_source_id %>
     <div class="form--field -required">
-      <%= f.text_field :firstname, required: true, container_class: '-middle', disabled: login_via_provider %>
+      <%= f.text_field :firstname, required: true, container_class: '-middle', disabled: login_via_provider || login_via_ldap %>
       <% if login_via_provider %>
         <span class="form--field-instructions"><%= t('user.text_change_disabled_for_provider_login') %></span>
       <% end %>
+      <% if login_via_ldap %>
+        <span class="form--field-instructions"><%= t('user.text_change_disabled_for_ldap_login') %></span>
+      <% end %>
     </div>
     <div class="form--field -required">
-      <%= f.text_field :lastname, required: true, container_class: '-middle', disabled: login_via_provider %>
+      <%= f.text_field :lastname, required: true, container_class: '-middle', disabled: login_via_provider || login_via_ldap %>
       <% if login_via_provider %>
         <span class="form--field-instructions"><%= t('user.text_change_disabled_for_provider_login') %></span>
       <% end %>
+      <% if login_via_ldap %>
+        <span class="form--field-instructions"><%= t('user.text_change_disabled_for_ldap_login') %></span>
+      <% end %>
+    </div>
+    <div class="form--field -required">
+      <%= f.text_field :mail, required: true, container_class: '-middle', disabled: login_via_ldap %>
+      <% if login_via_ldap %>
+        <span class="form--field-instructions"><%= t('user.text_change_disabled_for_ldap_login') %></span>
+      <% end %>
     </div>
-    <div class="form--field -required"><%= f.text_field :mail, required: true, container_class: '-middle' %></div>
 
     <%= fields_for :pref, @user.pref, builder: TabularFormBuilder, lang: current_language do |pref_fields| %>
       <div class="form--field"><%= pref_fields.check_box :hide_mail %></div>

config/locales/en.yml

@@ -3338,6 +3338,7 @@ en:
     status_user_and_brute_force: "%{user} and %{brute_force}"
     status_change: "Status change"
     text_change_disabled_for_provider_login: "The name is set by your login provider and can thus not be changed."
+    text_change_disabled_for_ldap_login: "The name and email is set by LDAP and can thus not be changed."
     unlock: "Unlock"
     unlock_and_reset_failed_logins: "Unlock and reset failed logins"
 

spec/features/users/my_spec.rb

@@ -30,6 +30,7 @@
 
 RSpec.describe 'my', :js, :with_cuprite do
   let(:user_password) { 'bob' * 4 }
+  let!(:string_cf) { create(:user_custom_field, :string, name: 'Hobbies', is_required: false) }
   let(:user) do
     create(:user,
            mail: '[email protected]',
@@ -39,7 +40,7 @@
   end
 
   ##
-  # Expecations for a successful account change
+  # Expectations for a successful account change
   def expect_changed!
     expect(page).to have_content I18n.t(:notice_account_updated)
     expect(page).to have_content I18n.t(:notice_account_other_session_expired)
@@ -62,6 +63,24 @@ def expect_changed!
     expect(Sessions::UserSession.for_user(user).where(session_id: 'other').count).to eq 1
   end
 
+  shared_examples 'common tests for normal and LDAP user' do
+    describe 'settings' do
+      context 'with a default time zone', with_settings: { user_default_timezone: 'Asia/Tokyo' } do
+        it 'can override a time zone' do
+          expect(user.pref.time_zone).to eq 'Asia/Tokyo'
+          visit my_settings_path
+
+          expect(page).to have_select 'pref_time_zone', selected: '(UTC+09:00) Tokyo'
+          select '(UTC+01:00) Paris', from: 'pref_time_zone'
+          click_on 'Save'
+
+          expect(page).to have_select 'pref_time_zone', selected: '(UTC+01:00) Paris'
+          expect(user.pref.time_zone).to eq 'Europe/Paris'
+        end
+      end
+    end
+  end
+
   context 'user' do
     describe '#account' do
       let(:dialog) { Components::PasswordConfirmationDialog.new }
@@ -108,21 +127,7 @@ def expect_changed!
       end
     end
 
-    describe 'settings' do
-      context 'with a default time zone', with_settings: { user_default_timezone: 'Asia/Tokyo' } do
-        it 'can override a time zone' do
-          expect(user.pref.time_zone).to eq 'Asia/Tokyo'
-          visit my_settings_path
-
-          expect(page).to have_select 'pref_time_zone', selected: '(UTC+09:00) Tokyo'
-          select '(UTC+01:00) Paris', from: 'pref_time_zone'
-          click_on 'Save'
-
-          expect(page).to have_select 'pref_time_zone', selected: '(UTC+01:00) Paris'
-          expect(user.pref.time_zone).to eq 'Europe/Paris'
-        end
-      end
-    end
+    include_examples 'common tests for normal and LDAP user'
 
     describe "API tokens" do
       context 'when API access is disabled via global settings', with_settings: { rest_api_enabled: false } do
@@ -415,4 +420,45 @@ def expect_changed!
       end
     end
   end
+
+  # Without password confirmation the test doesn't try to connect to the LDAP:
+  context 'LDAP user', with_config: { internal_password_confirmation: false } do
+    let(:ldap_auth_source) { create(:ldap_auth_source) }
+    let(:user) do
+      create(:user,
+             mail: '[email protected]',
+             login: 'bob',
+             ldap_auth_source:)
+    end
+
+    describe '#account' do
+      before do
+        visit my_account_path
+      end
+
+      it 'does not allow change of name and email but other fields can be changed' do
+        email_field = find_field('user[mail]', disabled: true)
+        firstname_field = find_field('user[firstname]', disabled: true)
+        lastname_field = find_field('user[lastname]', disabled: true)
+
+        expect(email_field).to be_disabled
+        expect(firstname_field).to be_disabled
+        expect(lastname_field).to be_disabled
+
+        expect(page).to have_text(I18n.t('user.text_change_disabled_for_ldap_login'), count: 3)
+
+        fill_in 'Hobbies', with: 'Ruby, DCS'
+        uncheck 'pref[hide_mail]'
+        click_on 'Save'
+
+        expect(page).to have_content I18n.t(:notice_account_updated)
+
+        user.reload
+        expect(user.custom_values.find_by(custom_field_id: string_cf).value).to eql 'Ruby, DCS'
+        expect(user.pref.hide_mail).to be false
+      end
+    end
+
+    include_examples 'common tests for normal and LDAP user'
+  end
 end