diff --git a/app/models/user.rb b/app/models/user.rb index 4045830007..8400c30a4e 100644 --- a/app/models/user.rb +++ b/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 diff --git a/config/site_settings.yml b/config/site_settings.yml index bd52c57722..33e32eb071 100644 --- a/config/site_settings.yml +++ b/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: diff --git a/lib/guardian/topic_guardian.rb b/lib/guardian/topic_guardian.rb index 04feb20b24..66df380baf 100644 --- a/lib/guardian/topic_guardian.rb +++ b/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) diff --git a/spec/lib/guardian/topic_guardian_spec.rb b/spec/lib/guardian/topic_guardian_spec.rb index c6da52b9ee..bfe003457c 100644 --- a/spec/lib/guardian/topic_guardian_spec.rb +++ b/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 diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index b77e39ff74..26af6b6228 100644 --- a/spec/models/user_spec.rb +++ b/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