Skip to content

Commit

Permalink
SECURITY: Don't disclose the existence of secret subcategories
Browse files Browse the repository at this point in the history
  • Loading branch information
danielwaterworth authored and nattsw committed Mar 15, 2024
1 parent 085edb1 commit 819361b
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 1 deletion.
9 changes: 8 additions & 1 deletion app/models/category_list.rb
Original file line number Diff line number Diff line change
Expand Up @@ -209,12 +209,19 @@ def find_categories

allowed_topic_create = Set.new(Category.topic_create_allowed(@guardian).pluck(:id))

parent_ids =
Category
.secured(@guardian)
.where(parent_category_id: categories_with_descendants.map(&:id))
.pluck("DISTINCT parent_category_id")
.to_set

categories_with_descendants.each do |category|
category.notification_level = notification_levels[category.id] || default_notification_level
category.permission = CategoryGroup.permission_types[:full] if allowed_topic_create.include?(
category.id,
)
category.has_children = category.subcategories.present?
category.has_children = parent_ids.include?(category.id)
end
end

Expand Down
23 changes: 23 additions & 0 deletions spec/models/category_list_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,29 @@
fab!(:admin)
let(:category_list) { CategoryList.new(Guardian.new(user), include_topics: true) }

context "when a category has a secret subcategory" do
fab!(:category)

fab!(:secret_subcategory) do
cat = Fabricate(:category, parent_category: category)
cat.set_permissions(admins: :full)
cat.save!
cat
end

let(:admin_category_list) { CategoryList.new(Guardian.new(admin), include_topics: true) }

it "doesn't set has_children when an unpriveleged user is querying" do
found = category_list.categories.find { |c| c.id == category.id }
expect(found.has_children).to eq(false)
end

it "sets has_children when an admin is querying" do
found = admin_category_list.categories.find { |c| c.id == category.id }
expect(found.has_children).to eq(true)
end
end

describe "security" do
it "properly hide secure categories" do
cat = Fabricate(:category_with_definition)
Expand Down

0 comments on commit 819361b

Please sign in to comment.