Skip to content
Permalink
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
tgxworld committed Aug 12, 2021
1 parent 9a60c83 commit aed65ec
Show file tree
Hide file tree
Showing 3 changed files with 62 additions and 9 deletions.
25 changes: 17 additions & 8 deletions app/models/topic_tracking_state.rb
Expand Up @@ -145,6 +145,15 @@ def self.publish_unread(post)
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]
Expand All @@ -153,15 +162,13 @@ def self.publish_unread(post)
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|

Expand All @@ -188,7 +195,9 @@ def self.publish_unread(post)
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
Expand Down
3 changes: 2 additions & 1 deletion spec/components/post_creator_spec.rb
Expand Up @@ -137,7 +137,8 @@
Jobs.run_immediately!
UserActionManager.enable

admin = Fabricate(:admin)
admin = Fabricate(:user)
admin.grant_admin!

cat = Fabricate(:category)
cat.set_permissions(admins: :full)
Expand Down
43 changes: 43 additions & 0 deletions spec/models/topic_tracking_state_spec.rb
Expand Up @@ -48,11 +48,54 @@

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(
Expand Down

0 comments on commit aed65ec

Please sign in to comment.