From 680024f9071b7696e5a444a58791016c6dc1f1e5 Mon Sep 17 00:00:00 2001 From: Alan Guo Xiang Tan Date: Fri, 23 Jul 2021 11:35:01 +0800 Subject: [PATCH] SECURITY: Do not reveal post whisperer in personal messages. Prior to this fix, post whisperer in personal messages are revealed in the topic's participants list even though non-staff users are unable to see the whisper. --- lib/post_creator.rb | 18 +++++++++++------- spec/components/post_creator_spec.rb | 28 ++++++++++++++++++++++------ 2 files changed, 33 insertions(+), 13 deletions(-) diff --git a/lib/post_creator.rb b/lib/post_creator.rb index bb28f3cdd24..27158ccbf4c 100644 --- a/lib/post_creator.rb +++ b/lib/post_creator.rb @@ -458,14 +458,18 @@ class PostCreator def ensure_in_allowed_users return unless @topic.private_message? && @topic.id + return if @post.whisper? + return if @topic.topic_allowed_users.exists?(user_id: @user.id) - unless @topic.topic_allowed_users.where(user_id: @user.id).exists? - unless @topic.topic_allowed_groups.where('group_id IN ( - SELECT group_id FROM group_users where user_id = ? - )', @user.id).exists? - @topic.topic_allowed_users.create!(user_id: @user.id) - end - end + return if @topic + .topic_allowed_groups + .where( + "group_id IN (SELECT group_id FROM group_users where user_id = ?)", + @user.id + ) + .exists? + + @topic.topic_allowed_users.create!(user_id: @user.id) end def unarchive_message diff --git a/spec/components/post_creator_spec.rb b/spec/components/post_creator_spec.rb index 3f4262a65ae..fb6a1391aec 100644 --- a/spec/components/post_creator_spec.rb +++ b/spec/components/post_creator_spec.rb @@ -901,10 +901,10 @@ describe PostCreator do context 'private message' do let(:target_user1) { Fabricate(:coding_horror) } fab!(:target_user2) { Fabricate(:moderator) } - fab!(:unrelated) { Fabricate(:user) } + fab!(:unrelated_user) { Fabricate(:user) } let(:post) do - PostCreator.create(user, title: 'hi there welcome to my topic', - raw: "this is my awesome message @#{unrelated.username_lower}", + PostCreator.create!(user, title: 'hi there welcome to my topic', + raw: "this is my awesome message @#{unrelated_user.username_lower}", archetype: Archetype.private_message, target_usernames: [target_user1.username, target_user2.username].join(','), category: 1) @@ -926,7 +926,7 @@ describe PostCreator do expect(post.topic.category).to eq(nil) # does not notify an unrelated user - expect(unrelated.notifications.count).to eq(0) + expect(unrelated_user.notifications.count).to eq(0) expect(post.topic.subtype).to eq(TopicSubtype.user_to_user) # PMs do not increase post count or topic count @@ -941,7 +941,7 @@ describe PostCreator do # if an admin replies they should be added to the allowed user list admin = Fabricate(:admin) - PostCreator.create(admin, raw: 'hi there welcome topic, I am a mod', + PostCreator.create!(admin, raw: 'hi there welcome topic, I am a mod', topic_id: post.topic_id) post.topic.reload @@ -955,11 +955,27 @@ describe PostCreator do admin2 = Fabricate(:admin) group.add(admin2) - PostCreator.create(admin2, raw: 'I am also an admin, and a mod', topic_id: post.topic_id) + PostCreator.create!(admin2, raw: 'I am also an admin, and a mod', topic_id: post.topic_id) expect(post.topic.topic_allowed_users.where(user_id: admin2.id).count).to eq(0) end + it 'does not add whisperers to allowed users of the topic' do + SiteSetting.enable_whispers = true + unrelated_user.update!(admin: true) + + PostCreator.create!( + unrelated_user, + raw: "This is a whisper that I am testing", + topic_id: post.topic_id, + post_type: Post.types[:whisper] + ) + + expect(post.topic.topic_allowed_users.map(&:user_id)).to contain_exactly( + target_user1.id, target_user2.id, user.id + ) + end + it 'does not increase posts count for small actions' do topic = Fabricate(:private_message_topic, user: Fabricate(:user))