github-mirror/discourse
2
0
Fork 0
mirror of https://github.com/discourse/discourse.git synced 2024-12-12 21:16:29 +08:00
Code Issues Actions 1 Packages Projects Releases Wiki Activity

SECURITY: Hide notifications for inaccessible topics (#19209)

Browse Source 
Filter notifications the user cannot see anymore
via guardian.can_see_topic_ids
This commit is contained in:
Martin Brennan 2022-11-28 10:42:05 +10:00 committed by GitHub
parent bf30689059
commit 131c5cff25
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 39 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
app/controllers/notifications_controller.rb
View File

@ -23,6 +23,7 @@ class NotificationsController < ApplicationController
limit = 50 if limit > 50
notifications = Notification.recent_report(current_user, limit)
notifications = filter_inaccessible_notifications(notifications)
changed = false
if notifications.present? && !(params.has_key?(:silent) || @readonly_mode)
@ -50,6 +51,7 @@ class NotificationsController < ApplicationController
total_rows = notifications.dup.count
notifications = notifications.offset(offset).limit(60)
notifications = filter_inaccessible_notifications(notifications)
render_json_dump(notifications: serialize_data(notifications, NotificationSerializer),
total_rows_notifications: total_rows,
seen_notification_id: user.seen_notification_id,
@ -101,4 +103,9 @@ class NotificationsController < ApplicationController
render_json_dump(NotificationSerializer.new(@notification, scope: guardian, root: false))
end
def filter_inaccessible_notifications(notifications)
topic_ids = notifications.map { |n| n.topic_id }.compact.uniq
accessible_topic_ids = guardian.can_see_topic_ids(topic_ids: topic_ids)
notifications.select { |n| n.topic_id.blank? || accessible_topic_ids.include?(n.topic_id) }
end
end

32
spec/requests/notifications_controller_spec.rb
View File

@ -119,6 +119,38 @@ describe NotificationsController do
expect(response.status).to eq(404)
end
end
context "with notifications for inaccessible topics" do
fab!(:sender) { Fabricate.build(:topic_allowed_user, user: Fabricate(:coding_horror)) }
fab!(:allowed_user) { Fabricate.build(:topic_allowed_user, user: user) }
fab!(:another_allowed_user) { Fabricate.build(:topic_allowed_user, user: Fabricate(:user)) }
fab!(:allowed_pm) { Fabricate(:private_message_topic, topic_allowed_users: [sender, allowed_user, another_allowed_user]) }
fab!(:forbidden_pm) { Fabricate(:private_message_topic, topic_allowed_users: [sender, another_allowed_user]) }
fab!(:allowed_pm_notification) { Fabricate(:private_message_notification, user: user, topic: allowed_pm) }
fab!(:forbidden_pm_notification) { Fabricate(:private_message_notification, user: user, topic: forbidden_pm) }
def expect_correct_notifications(response)
notification_ids = response.parsed_body["notifications"].map { |n| n["id"] }
expect(notification_ids).to include(allowed_pm_notification.id)
expect(notification_ids).to_not include(forbidden_pm_notification.id)
end
context "with 'recent' filter" do
it "doesn't include notifications from topics the user isn't allowed to see" do
get "/notifications.json", params: { recent: true }
expect(response.status).to eq(200)
expect_correct_notifications(response)
end
end
context "without 'recent' filter" do
it "doesn't include notifications from topics the user isn't allowed to see" do
get "/notifications.json"
expect(response.status).to eq(200)
expect_correct_notifications(response)
end
end
end
end
it 'should succeed' do