Commit 5f24f383 authored by Eugenia Grieff's avatar Eugenia Grieff
Browse files

Fix labels finder to filter visible issuables

Use project scopes to filter project labels

Add changelog file

Check  issuables visibility in  LabelsFinder

Add specs for issuables visibility cases

Rename Project method to reuse in LabelsFinder

Remove commented code

Improve changelog title
parent 2e139997f05d
......@@ -51,7 +51,7 @@ def label_ids
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 @@ def groups_user_can_read_labels(groups)
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 @@ def group_ids
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 @@ def filter_milestones_by_project(milestones)
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 @@
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 @@ def enable_lfs
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 @@ def enable_lfs
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