FIX: properly secure poll message bus

Co-authored-by: Sam <sam.saffron@gmail.com>

app/models/post.rb

@@ -152,13 +152,12 @@ class Post < ActiveRecord::Base
     end
   end
 
-  def publish_change_to_clients!(type, options = {})
-    # special failsafe for posts missing topics consistency checks should fix, but message
-    # is safe to skip
+  def publish_change_to_clients!(type, opts = {})
+    # special failsafe for posts missing topics consistency checks should fix,
+    # but message is safe to skip
     return unless topic
 
-    channel = "/topic/#{topic_id}"
-    msg = {
+    message = {
       id: id,
       post_number: post_number,
       updated_at: Time.now,
@@ -166,20 +165,26 @@ class Post < ActiveRecord::Base
       last_editor_id: last_editor_id,
       type: type,
       version: version
-    }.merge(options)
+    }.merge(opts)
+
+    publish_message!("/topic/#{topic_id}", message)
+  end
+
+  def publish_message!(channel, message, opts = {})
+    return unless topic
 
     if Topic.visible_post_types.include?(post_type)
       if topic.private_message?
-        user_ids = User.where('admin or moderator').pluck(:id)
-        user_ids |= topic.allowed_users.pluck(:id)
-        MessageBus.publish(channel, msg, user_ids: user_ids)
+        opts[:user_ids] = User.where("admin OR moderator").pluck(:id)
+        opts[:user_ids] |= topic.allowed_users.pluck(:id)
       else
-        MessageBus.publish(channel, msg, group_ids: topic.secure_group_ids)
+        opts[:group_ids] = topic.secure_group_ids
       end
     else
-      user_ids = User.where('admin or moderator or id = ?', user_id).pluck(:id)
-      MessageBus.publish(channel, msg, user_ids: user_ids)
+      opts[:user_ids] = User.where("admin OR moderator OR id = ?", user_id).pluck(:id)
     end
+
+    MessageBus.publish(channel, message, opts)
   end
 
   def trash!(trashed_by = nil)

plugins/poll/lib/polls_updater.rb

@@ -91,7 +91,7 @@ module DiscoursePoll
         if has_changed
           polls = ::Poll.includes(poll_options: :poll_votes).where(post: post)
           polls = ActiveModel::ArraySerializer.new(polls, each_serializer: PollSerializer, root: false).as_json
-          MessageBus.publish("/polls/#{post.topic_id}", post_id: post.id, polls: polls)
+          post.publish_message!("/polls/#{post.topic_id}", post_id: post.id, polls: polls)
         end
       end
     end

plugins/poll/plugin.rb

@@ -102,7 +102,7 @@ after_initialize do
           serialized_poll = PollSerializer.new(poll, root: false).as_json
           payload = { post_id: post_id, polls: [serialized_poll] }
 
-          MessageBus.publish("/polls/#{post.topic_id}", payload)
+          post.publish_message!("/polls/#{post.topic_id}", payload)
 
           [serialized_poll, options]
         end
@@ -137,7 +137,7 @@ after_initialize do
           serialized_poll = PollSerializer.new(poll, root: false).as_json
           payload = { post_id: post_id, polls: [serialized_poll] }
 
-          MessageBus.publish("/polls/#{post.topic_id}", payload)
+          post.publish_message!("/polls/#{post.topic_id}", payload)
 
           serialized_poll
         end
@@ -425,7 +425,7 @@ after_initialize do
 
     unless post.is_first_post?
       polls = ActiveModel::ArraySerializer.new(post.polls, each_serializer: PollSerializer, root: false).as_json
-      MessageBus.publish("/polls/#{post.topic_id}", post_id: post.id, polls: polls)
+      post.publish_message!("/polls/#{post.topic_id}", post_id: post.id, polls: polls)
     end
   end
 

plugins/poll/spec/controllers/polls_controller_spec.rb

@@ -17,16 +17,69 @@ describe ::DiscoursePoll::PollsController do
         put :vote, params: {
           post_id: poll.id, poll_name: "poll", options: ["5c24fc1df56d764b550ceae1b9319125"]
         }, format: :json
-
-        expect(response.status).to eq(200)
       end.first
 
+      expect(response.status).to eq(200)
+
       json = ::JSON.parse(response.body)
       expect(json["poll"]["name"]).to eq("poll")
       expect(json["poll"]["voters"]).to eq(1)
       expect(json["vote"]).to eq(["5c24fc1df56d764b550ceae1b9319125"])
 
       expect(message.channel).to eq("/polls/#{poll.topic_id}")
+      expect(message.user_ids).to eq(nil)
+      expect(message.group_ids).to eq(nil)
+    end
+
+    it "works in PM" do
+      user2 = Fabricate(:user)
+      topic = Fabricate(:private_message_topic, topic_allowed_users: [
+        Fabricate.build(:topic_allowed_user, user: user),
+        Fabricate.build(:topic_allowed_user, user: user2)
+      ])
+      poll = Fabricate(:post, topic: topic, user: user, raw: "[poll]\n- A\n- B\n[/poll]")
+
+      message = MessageBus.track_publish do
+        put :vote, params: {
+          post_id: poll.id, poll_name: "poll", options: ["5c24fc1df56d764b550ceae1b9319125"]
+        }, format: :json
+      end.first
+
+      expect(response.status).to eq(200)
+
+      json = ::JSON.parse(response.body)
+      expect(json["poll"]["name"]).to eq("poll")
+      expect(json["poll"]["voters"]).to eq(1)
+      expect(json["vote"]).to eq(["5c24fc1df56d764b550ceae1b9319125"])
+
+      expect(message.channel).to eq("/polls/#{poll.topic_id}")
+      expect(message.user_ids).to contain_exactly(-2, -1, user.id, user2.id)
+      expect(message.group_ids).to eq(nil)
+    end
+
+    it "works in secure categories" do
+      group = Fabricate(:group)
+      group.add_owner(user)
+      category = Fabricate(:private_category, group: group)
+      topic = Fabricate(:topic, category: category)
+      poll = Fabricate(:post, topic: topic, user: user, raw: "[poll]\n- A\n- B\n[/poll]")
+
+      message = MessageBus.track_publish do
+        put :vote, params: {
+          post_id: poll.id, poll_name: "poll", options: ["5c24fc1df56d764b550ceae1b9319125"]
+        }, format: :json
+      end.first
+
+      expect(response.status).to eq(200)
+
+      json = ::JSON.parse(response.body)
+      expect(json["poll"]["name"]).to eq("poll")
+      expect(json["poll"]["voters"]).to eq(1)
+      expect(json["vote"]).to eq(["5c24fc1df56d764b550ceae1b9319125"])
+
+      expect(message.channel).to eq("/polls/#{poll.topic_id}")
+      expect(message.user_ids).to eq(nil)
+      expect(message.group_ids).to contain_exactly(group.id)
     end
 
     it "requires at least 1 valid option" do