Commit b58dd075 authored by Eugenia Grieff's avatar Eugenia Grieff

Fix labels finder to filter issuables

Use project scopes to filter project labels that are visible for user
parent 1425a56c
......@@ -51,7 +51,7 @@ class LabelsFinder < UnionFinder
end
label_ids << Label.where(group_id: projects.group_ids)
label_ids << Label.where(project_id: projects.select(:id)) unless only_group_labels?
label_ids << Label.where(project_id: ids_user_can_read_labels(projects)) unless only_group_labels?
end
label_ids
......@@ -188,4 +188,10 @@ class LabelsFinder < UnionFinder
groups.select { |group| authorized_to_read_labels?(group) }
end
end
# rubocop: disable CodeReuse/ActiveRecord
def ids_user_can_read_labels(projects)
Project.where(id: projects.select(:id)).ids_with_issuables_available_for(current_user)
end
# rubocop: enable CodeReuse/ActiveRecord
end
......@@ -609,11 +609,11 @@ class Project < ApplicationRecord
joins(:namespace).where(namespaces: { type: 'Group' }).select(:namespace_id)
end
# Returns ids of projects with milestones available for given user
# Returns ids of projects with issuables available for given user
#
# Used on queries to find milestones which user can see
# For example: Milestone.where(project_id: ids_with_milestone_available_for(user))
def ids_with_milestone_available_for(user)
# Used on queries to find milestones or labels which user can see
# For example: Milestone.where(project_id: ids_with_issuables_available_for(user))
def ids_with_issuables_available_for(user)
with_issues_enabled = with_issues_available_for_user(user).select(:id)
with_merge_requests_enabled = with_merge_requests_available_for_user(user).select(:id)
......
---
title: Do not display project labels that are not visible for user accessing group labels
merge_request:
author:
type: security
......@@ -163,7 +163,7 @@ module Gitlab
return Milestone.none if project_ids.nil?
authorized_project_ids_relation =
Project.where(id: project_ids).ids_with_milestone_available_for(current_user)
Project.where(id: project_ids).ids_with_issuables_available_for(current_user)
milestones.where(project_id: authorized_project_ids_relation)
end
......
......@@ -128,6 +128,89 @@ describe LabelsFinder do
expect(finder.execute).to eq [private_subgroup_label_1]
end
end
context 'when including labels from group projects with limited visibility' do
let(:finder) { described_class.new(user, group_id: group_4.id) }
let(:group_4) { create(:group) }
let(:limited_visibility_project) { create(:project, :public, group: group_4) }
let(:visible_project) { create(:project, :public, group: group_4) }
let!(:group_label_1) { create(:group_label, group: group_4) }
let!(:limited_visibility_label) { create(:label, project: limited_visibility_project) }
let!(:visible_label) { create(:label, project: visible_project) }
shared_examples 'with full visibility' do
it 'returns all projects labels' do
expect(finder.execute).to eq [group_label_1, limited_visibility_label, visible_label]
end
end
shared_examples 'with limited visibility' do
it 'returns only authorized projects labels' do
expect(finder.execute).to eq [group_label_1, visible_label]
end
end
context 'when merge requests and issues are not visible for non members' do
before do
limited_visibility_project.project_feature.update!(
merge_requests_access_level: ProjectFeature::PRIVATE,
issues_access_level: ProjectFeature::PRIVATE
)
end
context 'when user is not a group member' do
it_behaves_like 'with limited visibility'
end
context 'when user is a group member' do
before do
group_4.add_developer(user)
end
it_behaves_like 'with full visibility'
end
end
context 'when merge requests are not visible for non members' do
before do
limited_visibility_project.project_feature.update!(
merge_requests_access_level: ProjectFeature::PRIVATE
)
end
context 'when user is not a group member' do
it_behaves_like 'with full visibility'
end
context 'when user is a group member' do
before do
group_4.add_developer(user)
end
it_behaves_like 'with full visibility'
end
end
context 'when issues are not visible for non members' do
before do
limited_visibility_project.project_feature.update!(
issues_access_level: ProjectFeature::PRIVATE
)
end
context 'when user is not a group member' do
it_behaves_like 'with full visibility'
end
context 'when user is a group member' do
before do
group_4.add_developer(user)
end
it_behaves_like 'with full visibility'
end
end
end
end
context 'filtering by project_id' do
......
......@@ -3416,7 +3416,7 @@ describe Project do
end
end
describe '.ids_with_milestone_available_for' do
describe '.ids_with_issuables_available_for' do
let!(:user) { create(:user) }
it 'returns project ids with milestones available for user' do
......@@ -3426,7 +3426,7 @@ describe Project do
project_4 = create(:project, :public)
project_4.project_feature.update(issues_access_level: ProjectFeature::PRIVATE, merge_requests_access_level: ProjectFeature::PRIVATE )
project_ids = described_class.ids_with_milestone_available_for(user).pluck(:id)
project_ids = described_class.ids_with_issuables_available_for(user).pluck(:id)
expect(project_ids).to include(project_2.id, project_3.id)
expect(project_ids).not_to include(project_1.id, project_4.id)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment