From 87bd93a5a4e71c5bd5bafce591df17c473607f04 Mon Sep 17 00:00:00 2001 From: Dombi Attila <83396+dombesz@users.noreply.github.com> Date: Mon, 12 Aug 2024 13:21:26 +0300 Subject: [PATCH 1/3] [#57077] Admin only User Custom field is visible to non-admin users https://community.openproject.org/work_packages/57077 --- app/models/user.rb | 9 +++++++++ spec/models/user_spec.rb | 26 ++++++++++++++++++++++++++ 2 files changed, 35 insertions(+) diff --git a/app/models/user.rb b/app/models/user.rb index df5c4a0f48a2..040f1005f47f 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -187,6 +187,15 @@ def mail=(arg) write_attribute(:mail, arg.to_s.strip) end + def self.available_custom_fields(_user) + user = User.current + RequestStore.fetch(:"#{name.underscore}_custom_fields_#{user.id}_#{user.admin?}") do + scope = CustomField.where(type: "#{name}CustomField").order(:position) + scope = scope.where(admin_only: false) if !user.admin? + scope + end + end + def self.search_in_project(query, options) options.fetch(:project).users.like(query) end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index e0f53d9fdba1..2cb717f4e12c 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -1016,4 +1016,30 @@ def build_user_double_with_expired_password(is_expired) let(:model_instance) { user } let(:custom_field) { create(:user_custom_field, :string) } end + + describe ".available_custom_fields" do + let(:admin) { build_stubbed(:admin) } + let(:user) { build_stubbed(:user) } + + shared_let(:user_cf) { create(:user_custom_field) } + shared_let(:admin_user_cf) { create(:user_custom_field, admin_only: true) } + + context "for an admin" do + current_user { admin } + + it "returns all fields including admin-only" do + expect(user.available_custom_fields) + .to contain_exactly(user_cf, admin_user_cf) + end + end + + context "for a member" do + current_user { user } + + it "does not return admin-only field" do + expect(user.available_custom_fields) + .to contain_exactly(user_cf) + end + end + end end From 3e7db149aa28b5428bb87c71822ca7eb462e5db3 Mon Sep 17 00:00:00 2001 From: Dombi Attila <83396+dombesz@users.noreply.github.com> Date: Mon, 12 Aug 2024 19:38:08 +0300 Subject: [PATCH 2/3] Add dummy available_custom_fields to Deleted users to make migrations runs correctly --- app/models/deleted_user.rb | 24 +++++++++--------------- 1 file changed, 9 insertions(+), 15 deletions(-) diff --git a/app/models/deleted_user.rb b/app/models/deleted_user.rb index 3685dbe3a9f1..ae65cfb4af70 100644 --- a/app/models/deleted_user.rb +++ b/app/models/deleted_user.rb @@ -11,19 +11,13 @@ def self.first end # Overrides a few properties - def logged?; false end - - def builtin?; true end - - def admin; false end - - def name(*_args); I18n.t("user.deleted") end - - def mail; nil end - - def time_zone; nil end - - def rss_key; nil end - - def destroy; false end + def available_custom_fields = [] + def logged? = false + def builtin? = true + def admin = false + def name(*_args) = I18n.t("user.deleted") + def mail = nil + def time_zone = nil + def rss_key = nil + def destroy = false end From 2f7f17379600cda2aead44b68f7ea50ca623f5b6 Mon Sep 17 00:00:00 2001 From: Dombi Attila <83396+dombesz@users.noreply.github.com> Date: Mon, 12 Aug 2024 20:54:51 +0300 Subject: [PATCH 3/3] Fix specs by adding anonymous to the results. The User.anonymous is being created in the tests, because the User.available_custom_fields method references it, thus it gets created any time a user is created. --- .../principals/scopes/ordered_by_name_spec.rb | 13 +++++++------ spec/models/users/scopes/with_time_zone_spec.rb | 8 +++++++- 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/spec/models/principals/scopes/ordered_by_name_spec.rb b/spec/models/principals/scopes/ordered_by_name_spec.rb index c7715b4a696b..65aff95c3e63 100644 --- a/spec/models/principals/scopes/ordered_by_name_spec.rb +++ b/spec/models/principals/scopes/ordered_by_name_spec.rb @@ -30,6 +30,7 @@ RSpec.describe Principals::Scopes::OrderedByName do describe ".ordered_by_name" do + shared_let(:anonymous) { User.anonymous } shared_let(:alice) { create(:user, login: "alice", firstname: "Alice", lastname: "Zetop") } shared_let(:eve) { create(:user, login: "eve", firstname: "Eve", lastname: "Baddie") } @@ -56,37 +57,37 @@ context "with default user sort", with_settings: { user_format: :firstname_lastname } do it_behaves_like "sorted results" do - let(:order) { [alice.id, group.id, placeholder_user.id, eve.id] } + let(:order) { [alice.id, anonymous.id, group.id, placeholder_user.id, eve.id] } end end context "with lastname_firstname user sort", with_settings: { user_format: :lastname_firstname } do it_behaves_like "sorted results" do - let(:order) { [eve.id, group.id, placeholder_user.id, alice.id] } + let(:order) { [anonymous.id, eve.id, group.id, placeholder_user.id, alice.id] } end end context "with lastname_n_firstname user sort", with_settings: { user_format: :lastname_n_firstname } do it_behaves_like "sorted results" do - let(:order) { [eve.id, group.id, placeholder_user.id, alice.id] } + let(:order) { [anonymous.id, eve.id, group.id, placeholder_user.id, alice.id] } end end context "with lastname_coma_firstname user sort", with_settings: { user_format: :lastname_coma_firstname } do it_behaves_like "sorted results" do - let(:order) { [eve.id, group.id, placeholder_user.id, alice.id] } + let(:order) { [anonymous.id, eve.id, group.id, placeholder_user.id, alice.id] } end end context "with firstname user sort", with_settings: { user_format: :firstname } do it_behaves_like "sorted results" do - let(:order) { [alice.id, group.id, placeholder_user.id, eve.id] } + let(:order) { [alice.id, anonymous.id, group.id, placeholder_user.id, eve.id] } end end context "with login user sort", with_settings: { user_format: :username } do it_behaves_like "sorted results" do - let(:order) { [alice.id, group.id, placeholder_user.id, eve.id] } + let(:order) { [alice.id, anonymous.id, group.id, placeholder_user.id, eve.id] } end end end diff --git a/spec/models/users/scopes/with_time_zone_spec.rb b/spec/models/users/scopes/with_time_zone_spec.rb index f6bb45fc52c6..7476c0c28e77 100644 --- a/spec/models/users/scopes/with_time_zone_spec.rb +++ b/spec/models/users/scopes/with_time_zone_spec.rb @@ -78,6 +78,7 @@ preferences: { time_zone: "" } ) end + shared_let(:anonymous) { User.anonymous } describe ".with_time_zone" do it "returns user having set a time zone in their preference matching the specified time zone(s)" do @@ -119,7 +120,12 @@ it "assumes Etc/UTC as default time zone", with_settings: { user_default_timezone: nil } do expect(User.with_time_zone("Etc/UTC")) - .to contain_exactly(user_without_preferences, user_without_time_zone, user_with_empty_time_zone) + .to contain_exactly( + user_without_preferences, + user_without_time_zone, + user_with_empty_time_zone, + anonymous + ) end end end