FEATURE: hidden site setting to suppress unsecured categories from admins (#19098)

The hidden site setting `suppress_secured_categories_from_admin` will
suppress visibility of categories without explicit access from admins
in a few key areas (category drop downs and topic lists)

It is not intended to be a security wall since admins can amend any site
setting. Instead it is feature that allows hiding the categories from the
UI.

Admins will still be able to see topics in categories without explicit
access using direct URLs or flags.

Co-authored-by: Alan Guo Xiang Tan <gxtan1990@gmail.com>

app/models/user.rb

@@ -1283,7 +1283,13 @@ class User < ActiveRecord::Base
   end
 
   def secure_category_ids
-    cats = self.admin? ? Category.unscoped.where(read_restricted: true) : secure_categories.references(:categories)
+    cats =
+      if self.admin? && !SiteSetting.suppress_secured_categories_from_admin
+        Category.unscoped.where(read_restricted: true)
+      else
+        secure_categories.references(:categories)
+      end
+
     cats.pluck('categories.id').sort
   end
 

config/site_settings.yml

@@ -1771,6 +1771,9 @@ security:
     default: false
     client: true
     hidden: true
+  suppress_secured_categories_from_admin:
+    default: false
+    hidden: true
 
 onebox:
   post_onebox_maxlength:

lib/guardian/topic_guardian.rb

@@ -262,13 +262,13 @@ module TopicGuardian
   end
 
   def filter_allowed_categories(records)
-    unless is_admin?
-      records = allowed_category_ids.size == 0 ?
-        records.where('topics.category_id IS NULL') :
-        records.where('topics.category_id IS NULL or topics.category_id IN (?)', allowed_category_ids)
-      records = records.references(:categories)
-    end
-    records
+    return records if is_admin? && !SiteSetting.suppress_secured_categories_from_admin
+
+    records = allowed_category_ids.size == 0 ?
+      records.where('topics.category_id IS NULL') :
+      records.where('topics.category_id IS NULL or topics.category_id IN (?)', allowed_category_ids)
+
+    records.references(:categories)
   end
 
   def can_edit_featured_link?(category_id)

spec/lib/guardian/topic_guardian_spec.rb

@@ -6,9 +6,11 @@ RSpec.describe TopicGuardian do
   fab!(:tl3_user) { Fabricate(:leader) }
   fab!(:moderator) { Fabricate(:moderator) }
   fab!(:category) { Fabricate(:category) }
-  fab!(:topic) { Fabricate(:topic, category: category) }
-  fab!(:private_message_topic) { Fabricate(:private_message_topic) }
   fab!(:group) { Fabricate(:group) }
+  fab!(:private_category) { Fabricate(:private_category, group: group) }
+  fab!(:topic) { Fabricate(:topic, category: category) }
+  fab!(:private_topic) { Fabricate(:topic, category: private_category) }
+  fab!(:private_message_topic) { Fabricate(:private_message_topic) }
 
   before do
     Guardian.enable_topic_can_see_consistency_check
@@ -174,4 +176,30 @@ RSpec.describe TopicGuardian do
       )
     end
   end
+
+  describe '#filter_allowed_categories' do
+
+    it 'allows admin access to categories without explicit access' do
+      guardian = Guardian.new(admin)
+      list = Topic.where(id: private_topic.id)
+      list = guardian.filter_allowed_categories(list)
+
+      expect(list.count).to eq(1)
+    end
+
+    context 'when SiteSetting.suppress_secured_categories_from_admin is true' do
+      before do
+        SiteSetting.suppress_secured_categories_from_admin = true
+      end
+
+      it 'does not allow admin access to categories without explicit access' do
+        guardian = Guardian.new(admin)
+        list = Topic.where(id: private_topic.id)
+        list = guardian.filter_allowed_categories(list)
+
+        expect(list.count).to eq(0)
+      end
+    end
+
+  end
 end

spec/models/user_spec.rb

@@ -3100,4 +3100,22 @@ RSpec.describe User do
       expect(user.visible_sidebar_tags).to contain_exactly(tag, hidden_tag)
     end
   end
+
+  describe '#secure_category_ids' do
+    fab!(:admin) { Fabricate(:admin) }
+    fab!(:group) { Fabricate(:group) }
+    fab!(:private_category) { Fabricate(:private_category, group: group) }
+
+    it 'allows admin to see all secure categories' do
+      expect(admin.secure_category_ids).to include(private_category.id)
+    end
+
+    context 'when SiteSetting.suppress_secured_categories_from_admin is true' do
+      it 'hides secure categories from admins' do
+        SiteSetting.suppress_secured_categories_from_admin = true
+        expect(admin.secure_category_ids).not_to include(private_category.id)
+      end
+    end
+
+  end
 end