diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 06919ff3f34165..df0c093fe209d0 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -1921,7 +1921,8 @@ en: tl3_requires_likes_received: "The minimum number of likes that must be received in the last (tl3 time period) days to qualify for promotion to trust level 3." tl3_links_no_follow: "Do not remove rel=nofollow from links posted by trust level 3 users." tl4_delete_posts_and_topics: "Allow TL4 users to delete posts and topics created by other users. TL4 users will also be able to see deleted topics and posts." - trusted_users_can_edit_others: "Allow users with high trust levels to edit content from other users" + edit_all_topic_groups: "Allow users in this group to edit other users' topic titles, tags, and categories" + edit_all_post_groups: "Allow users in this group to edit other users' posts" min_trust_to_create_topic: "The minimum trust level required to create a new topic." allow_flagging_staff: "If enabled, users can flag posts from staff accounts." diff --git a/config/site_settings.yml b/config/site_settings.yml index 55f8be4bf8af15..a6d7046eb4e3ad 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -1684,8 +1684,12 @@ trust: tl4_delete_posts_and_topics: default: false client: true - trusted_users_can_edit_others: - default: true + edit_all_topic_groups: + default: "13" + type: group_list + edit_all_post_groups: + default: "14" + type: group_list security: detailed_404: false diff --git a/db/migrate/20230509214723_separate_trusted_users_can_edit_others_site_setting.rb b/db/migrate/20230509214723_separate_trusted_users_can_edit_others_site_setting.rb new file mode 100644 index 00000000000000..91d2bd7d985d94 --- /dev/null +++ b/db/migrate/20230509214723_separate_trusted_users_can_edit_others_site_setting.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +class SeparateTrustedUsersCanEditOthersSiteSetting < ActiveRecord::Migration[7.0] + def up + if select_value( + "SELECT 1 FROM site_settings WHERE name = 'trusted_users_can_edit_others' AND value = 'f'", + ) + execute <<~SQL + INSERT INTO site_settings (name, data_type, value, created_at, updated_at) VALUES ('edit_all_topic_groups', 20, '', now(), now()); + INSERT INTO site_settings (name, data_type, value, created_at, updated_at) VALUES ('edit_all_post_groups', 20, '', now(), now()); + SQL + end + end + + def down + execute <<~SQL + DELETE FROM site_settings WHERE name = 'edit_all_topic_groups'; + DELETE FROM site_settings WHERE name = 'edit_all_post_groups'; + SQL + end +end diff --git a/lib/guardian/post_guardian.rb b/lib/guardian/post_guardian.rb index 6b485569571df4..daa36992caa8ce 100644 --- a/lib/guardian/post_guardian.rb +++ b/lib/guardian/post_guardian.rb @@ -129,11 +129,7 @@ def can_edit_post?(post) # Must be staff to edit a locked post return false if post.locked? && !is_staff? - if ( - is_staff? || - (SiteSetting.trusted_users_can_edit_others? && @user.has_trust_level?(TrustLevel[4])) || - is_category_group_moderator?(post.topic&.category) - ) + if (is_staff? || is_in_edit_post_groups? || is_category_group_moderator?(post.topic&.category)) return can_create_post?(post.topic) end @@ -173,6 +169,11 @@ def can_edit_post?(post) false end + def is_in_edit_post_groups? + SiteSetting.edit_all_post_groups.present? && + user.in_any_groups?(SiteSetting.edit_all_post_groups.to_s.split("|").map(&:to_i)) + end + def can_edit_hidden_post?(post) return false if post.nil? post.hidden_at.nil? || diff --git a/lib/guardian/topic_guardian.rb b/lib/guardian/topic_guardian.rb index 9ba3a248c5d6cd..0f8782f3bbf8ba 100644 --- a/lib/guardian/topic_guardian.rb +++ b/lib/guardian/topic_guardian.rb @@ -119,18 +119,16 @@ def can_edit_topic?(topic) return true end - # TL4 users can edit archived topics, but can not edit private messages if ( - SiteSetting.trusted_users_can_edit_others? && topic.archived && !topic.private_message? && - user.has_trust_level?(TrustLevel[4]) && can_create_post?(topic) + is_in_edit_post_groups? && topic.archived && !topic.private_message? && + can_create_post?(topic) ) return true end - # TL3 users can not edit archived topics and private messages if ( - SiteSetting.trusted_users_can_edit_others? && !topic.archived && !topic.private_message? && - user.has_trust_level?(TrustLevel[3]) && can_create_post?(topic) + is_in_edit_topic_groups? && !topic.archived && !topic.private_message? && + can_create_post?(topic) ) return true end @@ -141,6 +139,11 @@ def can_edit_topic?(topic) (!first_post&.hidden? || can_edit_hidden_post?(first_post)) end + def is_in_edit_topic_groups? + SiteSetting.edit_all_topic_groups.present? && + user.in_any_groups?(SiteSetting.edit_all_topic_groups.to_s.split("|").map(&:to_i)) + end + def can_recover_topic?(topic) if is_staff? || (topic&.category && is_category_group_moderator?(topic.category)) || (SiteSetting.tl4_delete_posts_and_topics && user&.has_trust_level?(TrustLevel[4])) diff --git a/plugins/discourse-presence/plugin.rb b/plugins/discourse-presence/plugin.rb index ff8af8d663f8c7..42919c143a6294 100644 --- a/plugins/discourse-presence/plugin.rb +++ b/plugins/discourse-presence/plugin.rb @@ -57,8 +57,8 @@ ] end - if !topic.private_message? && SiteSetting.trusted_users_can_edit_others? - config.allowed_group_ids << Group::AUTO_GROUPS[:trust_level_4] + if !topic.private_message? && SiteSetting.edit_all_post_groups.present? + config.allowed_group_ids += SiteSetting.edit_all_post_groups.split("|").map(&:to_i) end if SiteSetting.enable_category_group_moderation? && diff --git a/plugins/discourse-presence/spec/integration/presence_spec.rb b/plugins/discourse-presence/spec/integration/presence_spec.rb index c3a024c45f68ef..5a5acc1477560d 100644 --- a/plugins/discourse-presence/spec/integration/presence_spec.rb +++ b/plugins/discourse-presence/spec/integration/presence_spec.rb @@ -66,7 +66,7 @@ end it "handles category moderators for edit" do - SiteSetting.trusted_users_can_edit_others = false + SiteSetting.edit_all_post_groups = nil p = Fabricate(:post, topic: private_topic, user: private_topic.user) c = PresenceChannel.new("/discourse-presence/edit/#{p.id}") @@ -79,6 +79,20 @@ expect(c.config.allowed_group_ids).to contain_exactly(Group::AUTO_GROUPS[:staff], group.id) end + it "adds edit_all_post_groups to the presence channel" do + p = Fabricate(:post, topic: public_topic, user: user) + g = Fabricate(:group) + + SiteSetting.edit_all_post_groups = "#{Group::AUTO_GROUPS[:trust_level_1]}|#{g.id}" + + c = PresenceChannel.new("/discourse-presence/edit/#{p.id}") + expect(c.config.allowed_group_ids).to contain_exactly( + Group::AUTO_GROUPS[:staff], + Group::AUTO_GROUPS[:trust_level_1], + g.id, + ) + end + it "handles permissions for a public topic" do c = PresenceChannel.new("/discourse-presence/reply/#{public_topic.id}") expect(c.config.public).to eq(false) @@ -135,27 +149,13 @@ expect(c.config.allowed_user_ids).to contain_exactly(user.id) end - it "allows only author and staff when editing a public post with tl4 editing disabled" do - SiteSetting.trusted_users_can_edit_others = false - - p = Fabricate(:post, topic: public_topic, user: user) - c = PresenceChannel.new("/discourse-presence/edit/#{p.id}") - expect(c.config.public).to eq(false) - expect(c.config.allowed_group_ids).to contain_exactly(Group::AUTO_GROUPS[:staff]) - expect(c.config.allowed_user_ids).to contain_exactly(user.id) - end - it "follows the wiki edit trust level site setting" do p = Fabricate(:post, topic: public_topic, user: user, wiki: true) SiteSetting.min_trust_to_edit_wiki_post = TrustLevel.levels[:basic] - SiteSetting.trusted_users_can_edit_others = false c = PresenceChannel.new("/discourse-presence/edit/#{p.id}") expect(c.config.public).to eq(false) - expect(c.config.allowed_group_ids).to contain_exactly( - Group::AUTO_GROUPS[:staff], - Group::AUTO_GROUPS[:trust_level_1], - ) + expect(c.config.allowed_group_ids).to include(Group::AUTO_GROUPS[:trust_level_1]) expect(c.config.allowed_user_ids).to contain_exactly(user.id) end diff --git a/spec/lib/guardian/post_guardian_spec.rb b/spec/lib/guardian/post_guardian_spec.rb index c67579bd328067..da2e385fd9d5b8 100644 --- a/spec/lib/guardian/post_guardian_spec.rb +++ b/spec/lib/guardian/post_guardian_spec.rb @@ -56,4 +56,24 @@ end end end + + describe "#is_in_edit_post_groups?" do + it "returns true if the user is in edit_all_post_groups" do + SiteSetting.edit_all_post_groups = group.id.to_s + + expect(Guardian.new(user).is_in_edit_post_groups?).to eq(true) + end + + it "returns false if the user is not in edit_all_post_groups" do + SiteSetting.edit_all_post_groups = Group::AUTO_GROUPS[:trust_level_4] + + expect(Guardian.new(Fabricate(:trust_level_3)).is_in_edit_post_groups?).to eq(false) + end + + it "returns false if the edit_all_post_groups is empty" do + SiteSetting.edit_all_post_groups = nil + + expect(Guardian.new(user).is_in_edit_post_groups?).to eq(false) + end + end end diff --git a/spec/lib/guardian/topic_guardian_spec.rb b/spec/lib/guardian/topic_guardian_spec.rb index 7bddc0e9e469b3..5b070b167c70de 100644 --- a/spec/lib/guardian/topic_guardian_spec.rb +++ b/spec/lib/guardian/topic_guardian_spec.rb @@ -173,6 +173,27 @@ end end + describe "#is_in_edit_topic_groups?" do + it "returns true if the user is in edit_all_topic_groups" do + group.add(user) + SiteSetting.edit_all_topic_groups = group.id.to_s + + expect(Guardian.new(user).is_in_edit_topic_groups?).to eq(true) + end + + it "returns false if the user is not in edit_all_topic_groups" do + SiteSetting.edit_all_topic_groups = Group::AUTO_GROUPS[:trust_level_4] + + expect(Guardian.new(tl3_user).is_in_edit_topic_groups?).to eq(false) + end + + it "returns false if the edit_all_topic_groups is empty" do + SiteSetting.edit_all_topic_groups = nil + + expect(Guardian.new(user).is_in_edit_topic_groups?).to eq(false) + end + end + describe "#can_review_topic?" do it "returns false for TL4 users" do topic = Fabricate(:topic) diff --git a/spec/lib/guardian_spec.rb b/spec/lib/guardian_spec.rb index 580db7c5dd5fe6..d0bf00ed97ffc0 100644 --- a/spec/lib/guardian_spec.rb +++ b/spec/lib/guardian_spec.rb @@ -1626,11 +1626,6 @@ expect(Guardian.new(trust_level_4).can_edit?(post)).to be_truthy end - it "returns false as a TL4 user if trusted_users_can_edit_others is false" do - SiteSetting.trusted_users_can_edit_others = false - expect(Guardian.new(trust_level_4).can_edit?(post)).to eq(false) - end - it "returns false when trying to edit a topic with no trust" do SiteSetting.min_trust_to_edit_post = 2 post.user.trust_level = 1 @@ -1876,11 +1871,6 @@ expect(Guardian.new(trust_level_3).can_edit?(topic)).to eq(true) end - it "is false at TL3, if `trusted_users_can_edit_others` is false" do - SiteSetting.trusted_users_can_edit_others = false - expect(Guardian.new(trust_level_3).can_edit?(topic)).to eq(false) - end - it "returns false when the category is read only" do topic.category.set_permissions(everyone: :readonly) topic.category.save @@ -1930,9 +1920,14 @@ expect(Guardian.new(trust_level_4).can_edit?(archived_topic)).to be_truthy end - it "is false at TL4, if `trusted_users_can_edit_others` is false" do - SiteSetting.trusted_users_can_edit_others = false - expect(Guardian.new(trust_level_4).can_edit?(archived_topic)).to eq(false) + it "returns true if the user is in edit_all_post_groups" do + SiteSetting.edit_all_post_groups = "14" + expect(Guardian.new(trust_level_4).can_edit?(archived_topic)).to eq(true) + end + + it "returns false if the user is not in edit_all_post_groups" do + SiteSetting.edit_all_post_groups = "14" + expect(Guardian.new(trust_level_3).can_edit?(archived_topic)).to eq(false) end it "returns false at trust level 3" do diff --git a/spec/requests/topics_controller_spec.rb b/spec/requests/topics_controller_spec.rb index 653d2c2a8a0e54..d7f90ba6399a3c 100644 --- a/spec/requests/topics_controller_spec.rb +++ b/spec/requests/topics_controller_spec.rb @@ -1819,8 +1819,9 @@ def topic_user_post_timings_count(user, topic) end describe "when first post is locked" do - it "blocks non-staff from editing even if 'trusted_users_can_edit_others' is true" do - SiteSetting.trusted_users_can_edit_others = true + it "blocks user from editing even if they are in 'edit_all_topic_groups' and 'edit_all_post_groups'" do + SiteSetting.edit_all_topic_groups = Group::AUTO_GROUPS[:trust_level_3] + SiteSetting.edit_all_post_groups = Group::AUTO_GROUPS[:trust_level_4] user.update!(trust_level: 3) topic.first_post.update!(locked_by_id: admin.id)