Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FEATURE: Separated 'trusted users can edit others' setting for trust level 3 & 4 #21493

Merged
merged 12 commits into from Jul 7, 2023
Merged
3 changes: 2 additions & 1 deletion config/locales/server.en.yml
Expand Up @@ -1910,7 +1910,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."
Expand Down
8 changes: 6 additions & 2 deletions config/site_settings.yml
Expand Up @@ -1683,8 +1683,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
Expand Down
@@ -0,0 +1,25 @@
# 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'")
execute <<~SQL
DELETE FROM site_settings WHERE name = 'trusted_users_can_edit_others';
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());
markvanlan marked this conversation as resolved.
Show resolved Hide resolved
SQL
end
end

def down
if select_value(
"SELECT 1 FROM site_settings WHERE name IN ('edit_all_topic_groups', 'edit_all_post_groups')",
)
execute <<~SQL
DELETE FROM site_settings WHERE name = 'edit_all_topic_groups';
DELETE FROM site_settings WHERE name = 'edit_all_post_groups';
INSERT INTO site_settings (name, data_type, value, created_at, updated_at) VALUES ('trusted_users_can_edit_others', 5, 'f', now(), now());
SQL
end
end
end
11 changes: 6 additions & 5 deletions lib/guardian/post_guardian.rb
Expand Up @@ -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

Expand Down Expand Up @@ -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.split("|").map(&:to_i))
end

def can_edit_hidden_post?(post)
return false if post.nil?
post.hidden_at.nil? ||
Expand Down
15 changes: 9 additions & 6 deletions lib/guardian/topic_guardian.rb
Expand Up @@ -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
Expand All @@ -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.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]))
Expand Down
4 changes: 2 additions & 2 deletions plugins/discourse-presence/plugin.rb
Expand Up @@ -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? &&
Expand Down
32 changes: 16 additions & 16 deletions plugins/discourse-presence/spec/integration/presence_spec.rb
Expand Up @@ -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}")
Expand All @@ -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)
Expand Down Expand Up @@ -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
nattsw marked this conversation as resolved.
Show resolved Hide resolved
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 contain(Group::AUTO_GROUPS[:trust_level_1])
expect(c.config.allowed_user_ids).to contain_exactly(user.id)
end

Expand Down
21 changes: 8 additions & 13 deletions spec/lib/guardian_spec.rb
Expand Up @@ -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
nattsw marked this conversation as resolved.
Show resolved Hide resolved
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
Expand Down Expand Up @@ -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
nattsw marked this conversation as resolved.
Show resolved Hide resolved
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
Expand Down Expand Up @@ -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
nattsw marked this conversation as resolved.
Show resolved Hide resolved
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
Expand Down
5 changes: 3 additions & 2 deletions spec/requests/topics_controller_spec.rb
Expand Up @@ -1611,8 +1611,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)

Expand Down