From 819361ba28f86a1347059af300bb5cca690f9193 Mon Sep 17 00:00:00 2001 From: Daniel Waterworth Date: Tue, 30 Jan 2024 12:27:13 -0600 Subject: [PATCH] SECURITY: Don't disclose the existence of secret subcategories --- app/models/category_list.rb | 9 ++++++++- spec/models/category_list_spec.rb | 23 +++++++++++++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/app/models/category_list.rb b/app/models/category_list.rb index aba935d8fb66d..8004fd257022b 100644 --- a/app/models/category_list.rb +++ b/app/models/category_list.rb @@ -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 diff --git a/spec/models/category_list_spec.rb b/spec/models/category_list_spec.rb index c5495f6c2b8b9..ff08f054b3f3e 100644 --- a/spec/models/category_list_spec.rb +++ b/spec/models/category_list_spec.rb @@ -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)