SECURITY: User's read state for topic is leaked to unauthorized clients.

SECURITY: User’s read state for topic is leaked to unauthorized clients.

A user’s read state for a topic such as the last read post number and the notification level is exposed.

diff --git a/app/models/topic_tracking_state.rb b/app/models/topic_tracking_state.rb
index e7db1b8..3e44f0c 100644
--- a/app/models/topic_tracking_state.rb
+++ b/app/models/topic_tracking_state.rb
@@ -145,6 +145,15 @@ class TopicTrackingState
     return unless post.topic.regular?
     # TODO at high scale we are going to have to defer this,
     #   perhaps cut down to users that are around in the last 7 days as well
+    tags = nil
+    tag_ids = nil
+    if include_tags_in_report?
+      tag_ids, tags = post.topic.tags.pluck(:id, :name).transpose
+    end
+
+    scope = TopicUser
+      .tracking(post.topic_id)
+      .includes(user: :user_stat)
 
     group_ids =
       if post.post_type == Post.types[:whisper]
@@ -153,15 +162,13 @@ class TopicTrackingState
         post.topic.category && post.topic.category.secure_group_ids
       end
 
-    tags = nil
-    tag_ids = nil
-    if include_tags_in_report?
-      tag_ids, tags = post.topic.tags.pluck(:id, :name).transpose
+    if group_ids.present?
+      scope = scope
+        .joins("INNER JOIN group_users gu ON gu.user_id = topic_users.user_id")
+        .where("gu.group_id IN (?)", group_ids)
     end
 
-    TopicUser
-      .tracking(post.topic_id)
-      .includes(user: :user_stat)
+    scope
       .select([:user_id, :last_read_post_number, :notification_level])
       .each do |tu|
 
@@ -188,7 +195,9 @@ class TopicTrackingState
         payload: payload
       }
 
-      MessageBus.publish(self.unread_channel_key(tu.user_id), message.as_json, group_ids: group_ids)
+      MessageBus.publish(self.unread_channel_key(tu.user_id), message.as_json,
+        user_ids: [tu.user_id]
+      )
     end
 
   end
diff --git a/spec/components/post_creator_spec.rb b/spec/components/post_creator_spec.rb
index f993f5a..1651629 100644
--- a/spec/components/post_creator_spec.rb
+++ b/spec/components/post_creator_spec.rb
@@ -137,7 +137,8 @@ describe PostCreator do
         Jobs.run_immediately!
         UserActionManager.enable
 
-        admin = Fabricate(:admin)
+        admin = Fabricate(:user)
+        admin.grant_admin!
 
         cat = Fabricate(:category)
         cat.set_permissions(admins: :full)
diff --git a/spec/models/topic_tracking_state_spec.rb b/spec/models/topic_tracking_state_spec.rb
index e25df41..ddcfc37 100644
--- a/spec/models/topic_tracking_state_spec.rb
+++ b/spec/models/topic_tracking_state_spec.rb
@@ -48,11 +48,54 @@ describe TopicTrackingState do
 
       data = message.data
 
+      expect(message.user_ids).to contain_exactly(post.user.id)
+      expect(message.group_ids).to eq(nil)
       expect(data["topic_id"]).to eq(topic.id)
       expect(data["message_type"]).to eq(described_class::UNREAD_MESSAGE_TYPE)
       expect(data["payload"]["archetype"]).to eq(Archetype.default)
     end
 
+    it "does not publish whisper post to non-staff users" do
+      post.update!(post_type: Post.types[:whisper])
+
+      messages = MessageBus.track_publish(described_class.unread_channel_key(post.user_id)) do
+        TopicTrackingState.publish_unread(post)
+      end
+
+      expect(messages).to eq([])
+
+      post.user.grant_admin!
+
+      message = MessageBus.track_publish(described_class.unread_channel_key(post.user_id)) do
+        TopicTrackingState.publish_unread(post)
+      end.first
+
+      expect(message.user_ids).to contain_exactly(post.user_id)
+      expect(message.group_ids).to eq(nil)
+    end
+
+    it "correctly publishes unread for a post in a restricted category" do
+      group = Fabricate(:group)
+      category = Fabricate(:private_category, group: group)
+
+      post.topic.update!(category: category)
+
+      messages = MessageBus.track_publish(described_class.unread_channel_key(post.user_id)) do
+        TopicTrackingState.publish_unread(post)
+      end
+
+      expect(messages).to eq([])
+
+      group.add(post.user)
+
+      message = MessageBus.track_publish(described_class.unread_channel_key(post.user_id)) do
+        TopicTrackingState.publish_unread(post)
+      end.first
+
+      expect(message.user_ids).to contain_exactly(post.user_id)
+      expect(message.group_ids).to eq(nil)
+    end
+
     describe 'for a private message' do
       before do
         TopicUser.change(

GitHub sha: aed65ec16d38886d7be7209d8c02df4ffd4937a4

This commit appears in #14021 which was approved by lis2. It was merged by tgxworld.