From 7ce76143ac3ac0432059d5f81183537ca046f016 Mon Sep 17 00:00:00 2001 From: Martin Brennan Date: Thu, 8 Feb 2024 13:10:26 +1000 Subject: [PATCH] FIX: Always trust admin and moderators with post edits (#25602) Removes duplication from LimitedEdit to see who can edit posts, and also removes the old trust level setting check since it's no longer necessary. Also make it so staff can always edit since can_edit_post? already has a staff escape hatch. --- app/models/concerns/limited_edit.rb | 11 +---------- lib/guardian/post_guardian.rb | 11 +++++------ spec/lib/guardian_spec.rb | 28 +++++++++++++++++----------- 3 files changed, 23 insertions(+), 27 deletions(-) diff --git a/app/models/concerns/limited_edit.rb b/app/models/concerns/limited_edit.rb index d2bbe65743d..405b2778a1f 100644 --- a/app/models/concerns/limited_edit.rb +++ b/app/models/concerns/limited_edit.rb @@ -4,22 +4,13 @@ module LimitedEdit extend ActiveSupport::Concern def edit_time_limit_expired?(user) - return true if !trusted_with_edits?(user) + return true if !user.guardian.trusted_with_post_edits? time_limit = user_time_limit(user) created_at && (time_limit > 0) && (created_at < time_limit.minutes.ago) end private - # TODO: This duplicates some functionality from PostGuardian. This should - # probably be checked there instead, because this has nothing to do - # with time limits. - # - def trusted_with_edits?(user) - user.trust_level >= SiteSetting.min_trust_to_edit_post || - user.in_any_groups?(SiteSetting.edit_post_allowed_groups_map) - end - def user_time_limit(user) if user.trust_level < 2 SiteSetting.post_edit_time_limit.to_i diff --git a/lib/guardian/post_guardian.rb b/lib/guardian/post_guardian.rb index 3eb9555fbd2..ee4be1c0d8d 100644 --- a/lib/guardian/post_guardian.rb +++ b/lib/guardian/post_guardian.rb @@ -163,7 +163,7 @@ module PostGuardian return can_create_post?(post.topic) end - return false if !trusted_with_edits? + return false if !trusted_with_post_edits? if is_my_own?(post) return false if @user.silenced? @@ -369,13 +369,12 @@ module PostGuardian is_staff? || @user.has_trust_level?(TrustLevel[4]) end - private - - def trusted_with_edits? - @user.trust_level >= SiteSetting.min_trust_to_edit_post || - @user.in_any_groups?(SiteSetting.edit_post_allowed_groups_map) + def trusted_with_post_edits? + is_staff? || @user.in_any_groups?(SiteSetting.edit_post_allowed_groups_map) end + private + def can_create_post_in_topic?(topic) if !SiteSetting.enable_system_message_replies? && topic.try(:subtype) == "system_message" return false diff --git a/spec/lib/guardian_spec.rb b/spec/lib/guardian_spec.rb index a7d1242dcfb..38a639f304e 100644 --- a/spec/lib/guardian_spec.rb +++ b/spec/lib/guardian_spec.rb @@ -1677,25 +1677,31 @@ RSpec.describe Guardian do expect(Guardian.new(trust_level_4).can_edit?(post)).to be_truthy end - it "returns false when trying to edit a topic with no trust" do - SiteSetting.min_trust_to_edit_post = 2 - SiteSetting.edit_post_allowed_groups = 12 - post.user.trust_level = 1 + it "returns false when trying to edit a topic when the user is not in the allowed groups" do + SiteSetting.edit_post_allowed_groups = Group::AUTO_GROUPS[:trust_level_2] + post.user.change_trust_level!(TrustLevel[1]) expect(Guardian.new(topic.user).can_edit?(topic)).to be_falsey end - it "returns false when trying to edit a post with no trust" do - SiteSetting.min_trust_to_edit_post = 2 - SiteSetting.edit_post_allowed_groups = 12 - post.user.trust_level = 1 + it "returns false when trying to edit a post when the user is not in the allowed groups" do + SiteSetting.edit_post_allowed_groups = Group::AUTO_GROUPS[:trust_level_2] + post.user.change_trust_level!(TrustLevel[1]) expect(Guardian.new(post.user).can_edit?(post)).to be_falsey end - it "returns true when trying to edit a post with trust" do - SiteSetting.min_trust_to_edit_post = 1 - post.user.trust_level = 1 + it "returns true when editing a post when the user is in the allowed groups" do + SiteSetting.edit_post_allowed_groups = Group::AUTO_GROUPS[:trust_level_1] + post.user.change_trust_level!(TrustLevel[1]) + + expect(Guardian.new(post.user).can_edit?(post)).to be_truthy + end + + it "returns true when editing a post when the user is admin regardless of groups" do + SiteSetting.edit_post_allowed_groups = Group::AUTO_GROUPS[:trust_level_2] + post.user.update!(admin: true) + post.user.change_trust_level!(TrustLevel[1]) expect(Guardian.new(post.user).can_edit?(post)).to be_truthy end