Skip to content
Permalink
Browse files Browse the repository at this point in the history
SECURITY: Publish reactions based on topic permissions (#218)
Only show post reactions to users that have permission to see the
post.
  • Loading branch information
oblakeerickson committed Apr 3, 2023
1 parent 2ca28ad commit 01aca15
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 11 deletions.
17 changes: 10 additions & 7 deletions app/controllers/discourse_reactions/custom_reactions_controller.rb
Expand Up @@ -138,7 +138,7 @@ def reactions_received
def post_reactions_users
id = params.require(:id).to_i
reaction_value = params[:reaction_value]
post = Post.find_by(id: id)
post = fetch_post_from_params

raise Discourse::InvalidParameters if !post

Expand Down Expand Up @@ -237,17 +237,20 @@ def format_likes_users(likes)
end

def fetch_post_from_params
post = Post.find(params[:post_id])
post_id = params[:post_id] || params[:id]
post = Post.find(post_id)
guardian.ensure_can_see!(post)
post
end

def publish_change_to_clients!(post, reaction: nil, previous_reaction: nil)
MessageBus.publish(
"/topic/#{post.topic.id}/reactions",
post_id: post.id,
reactions: [reaction, previous_reaction].compact.uniq,
)
message = { post_id: post.id, reactions: [reaction, previous_reaction].compact.uniq }

opts = {}
secure_audience = post.topic.secure_audience_publish_messages
opts = secure_audience if secure_audience[:user_ids] != [] && secure_audience[:group_ids] != []

MessageBus.publish("/topic/#{post.topic.id}/reactions", message, opts)
end

def secure_reaction_users!(reaction_users)
Expand Down
2 changes: 1 addition & 1 deletion plugin.rb
Expand Up @@ -2,7 +2,7 @@

# name: discourse-reactions
# about: Allows users to react with emojis to a post
# version: 0.2
# version: 0.3
# author: Ahmed Gagan, Rafael dos Santos Silva, Kris Aubuchon, Joffrey Jaffeux, Kris Kotlarek, Jordan Vidrine
# url: https://github.com/discourse/discourse-reactions
# transpile_js: true
Expand Down
40 changes: 37 additions & 3 deletions spec/requests/custom_reactions_controller_spec.rb
Expand Up @@ -9,10 +9,14 @@
fab!(:user_3) { Fabricate(:user) }
fab!(:user_4) { Fabricate(:user) }
fab!(:user_5) { Fabricate(:user) }
fab!(:admin) { Fabricate(:admin) }
fab!(:post_2) { Fabricate(:post, user: user_1) }
fab!(:private_topic) { Fabricate(:private_message_topic, user: user_2, recipient: admin) }
fab!(:private_post) { Fabricate(:post, topic: private_topic) }
fab!(:reaction_1) { Fabricate(:reaction, post: post_2, reaction_value: "laughing") }
fab!(:reaction_2) { Fabricate(:reaction, post: post_2, reaction_value: "open_mouth") }
fab!(:reaction_3) { Fabricate(:reaction, post: post_2, reaction_value: "hugs") }
fab!(:reaction_4) { Fabricate(:reaction, post: private_post, reaction_value: "hugs") }
fab!(:like) do
Fabricate(
:post_action,
Expand All @@ -33,6 +37,9 @@
fab!(:reaction_user_4) do
Fabricate(:reaction_user, reaction: reaction_2, user: user_3, post: post_2)
end
fab!(:reaction_user_5) do
Fabricate(:reaction_user, reaction: reaction_4, user: admin, post: private_post)
end

before do
SiteSetting.discourse_reactions_enabled = true
Expand All @@ -43,6 +50,7 @@

describe "#toggle" do
let(:payload_with_user) { [{ "id" => "hugs", "type" => "emoji", "count" => 1 }] }
let(:api_key) { Fabricate(:api_key, user: admin, created_by: admin) }

it "toggles reaction" do
sign_in(user_1)
Expand Down Expand Up @@ -118,6 +126,21 @@
expect(messages[1].data[:reactions]).to contain_exactly("cry", "angry")
end

it "publishes MessageBus messages securely" do
sign_in(user_1)
messages =
MessageBus.track_publish("/topic/#{private_post.topic.id}/reactions") do
put "/discourse-reactions/posts/#{private_post.id}/custom-reactions/cry/toggle.json",
headers: {
"HTTP_API_KEY" => api_key.key,
"HTTP_API_USERNAME" => api_key.user.username,
}
end
user_1_messages = messages.find { |m| m.user_ids.include?(user_1.id) }
expect(messages.count).to eq(1)
expect(user_1_messages).to eq(nil)
end

it "errors when reaction is invalid" do
sign_in(user_1)
expect do
Expand Down Expand Up @@ -388,12 +411,12 @@
)
end

it "gives 400 ERROR when the post_id OR reaction_value is invalid" do
it "gives 404 ERROR when the post_id OR reaction_value is invalid" do
get "/discourse-reactions/posts/1000000/reactions-users.json"
expect(response.status).to eq(400)
expect(response.status).to eq(404)

get "/discourse-reactions/posts/1000000/reactions-users.json?reaction_value=test"
expect(response.status).to eq(400)
expect(response.status).to eq(404)
end

it "merges identic custom reaction into likes" do
Expand All @@ -413,6 +436,17 @@
parsed = response.parsed_body
expect(parsed["reaction_users"][0]["count"]).to eq(like_count + reaction_count)
end

it "does not show reaction_users on PMs without permission" do
get "/discourse-reactions/posts/#{private_post.id}/reactions-users.json"
expect(response.status).to eq(403)
end

it "shows reaction_users on PMs with permission" do
sign_in(user_2)
get "/discourse-reactions/posts/#{private_post.id}/reactions-users.json"
expect(response.status).to eq(200)
end
end

describe "positive notifications" do
Expand Down

0 comments on commit 01aca15

Please sign in to comment.