SECURITY: Avoid leaking private group name when viewing category. (#16337)

In certain instances when viewing a category, the name of a group with
restricted visilbity may be revealed to users which do not have the
required permission.
This commit is contained in:
Alan Guo Xiang Tan 2022-03-31 14:39:01 +08:00 committed by GitHub
parent e431000b23
commit dfaf9831f7
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 75 additions and 11 deletions

View File

@ -33,15 +33,22 @@ class CategorySerializer < SiteCategorySerializer
def group_permissions
@group_permissions ||= begin
perms = object.category_groups.joins(:group).includes(:group).order("groups.name").map do |cg|
{
permission_type: cg.permission_type,
group_name: cg.group.name
}
end
perms = object
.category_groups
.joins(:group)
.includes(:group)
.merge(Group.visible_groups(scope&.user))
.order("groups.name").map do |cg|
{
permission_type: cg.permission_type,
group_name: cg.group.name
}
end
if perms.length == 0 && !object.read_restricted
perms << { permission_type: CategoryGroup.permission_types[:full], group_name: Group[:everyone]&.name.presence || :everyone }
end
perms
end
end

View File

@ -1,6 +1,8 @@
# frozen_string_literal: true
describe CategorySerializer do
fab!(:user) { Fabricate(:user) }
fab!(:admin) { Fabricate(:admin) }
fab!(:group) { Fabricate(:group) }
fab!(:category) { Fabricate(:category, reviewable_by_group_id: group.id) }
@ -33,8 +35,6 @@ describe CategorySerializer do
end
describe "user notification level" do
fab!(:user) { Fabricate(:user) }
it "includes the user's notification level" do
CategoryUser.set_notification_level_for_category(user, NotificationLevels.all[:watching], category.id)
json = described_class.new(category, scope: Guardian.new(user), root: false).as_json
@ -42,10 +42,67 @@ describe CategorySerializer do
end
end
describe "available groups" do
fab!(:user) { Fabricate(:user) }
fab!(:admin) { Fabricate(:admin) }
describe '#group_permissions' do
context "category without group permissions configured" do
it "returns the right category group permissions for an anon user" do
json = described_class.new(category, scope: Guardian.new, root: false).as_json
expect(json[:group_permissions]).to eq([
{ permission_type: CategoryGroup.permission_types[:full], group_name: Group[:everyone]&.name }
])
end
end
context "category with group permissions configured" do
fab!(:private_group) { Fabricate(:group, visibility_level: Group.visibility_levels[:staff]) }
fab!(:user_group) do
Fabricate(:group, visibility_level: Group.visibility_levels[:members]).tap do |g|
g.add(user)
end
end
before do
category.set_permissions(
:everyone => :readonly,
group.name => :readonly,
user_group.name => :full,
private_group.name => :full,
)
category.save!
end
it "returns the right category group permissions for an anon user" do
json = described_class.new(category, scope: Guardian.new, root: false).as_json
expect(json[:group_permissions]).to eq([
{ permission_type: CategoryGroup.permission_types[:readonly], group_name: group.name },
])
end
it "returns the right category group permissions for a regular user" do
json = described_class.new(category, scope: Guardian.new(user), root: false).as_json
expect(json[:group_permissions]).to eq([
{ permission_type: CategoryGroup.permission_types[:readonly], group_name: group.name },
{ permission_type: CategoryGroup.permission_types[:full], group_name: user_group.name },
])
end
it "returns the right category group permission for a staff user" do
json = described_class.new(category, scope: Guardian.new(admin), root: false).as_json
expect(json[:group_permissions]).to eq([
{ permission_type: CategoryGroup.permission_types[:readonly], group_name: group.name },
{ permission_type: CategoryGroup.permission_types[:full], group_name: private_group.name },
{ permission_type: CategoryGroup.permission_types[:full], group_name: user_group.name }
])
end
end
end
describe "available groups" do
it "not included for a regular user" do
json = described_class.new(category, scope: Guardian.new(user), root: false).as_json
expect(json[:available_groups]).to eq(nil)