Commit 6ff7788d authored by Eugenia Grieff's avatar Eugenia Grieff

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 6653aab9
...@@ -51,7 +51,7 @@ class LabelsFinder < UnionFinder ...@@ -51,7 +51,7 @@ class LabelsFinder < UnionFinder
end end
label_ids << Label.where(group_id: projects.group_ids) 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 end
label_ids label_ids
...@@ -188,4 +188,10 @@ class LabelsFinder < UnionFinder ...@@ -188,4 +188,10 @@ class LabelsFinder < UnionFinder
groups.select { |group| authorized_to_read_labels?(group) } groups.select { |group| authorized_to_read_labels?(group) }
end end
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 end
...@@ -609,11 +609,11 @@ class Project < ApplicationRecord ...@@ -609,11 +609,11 @@ class Project < ApplicationRecord
joins(:namespace).where(namespaces: { type: 'Group' }).select(:namespace_id) joins(:namespace).where(namespaces: { type: 'Group' }).select(:namespace_id)
end 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 # Used on queries to find milestones or labels which user can see
# For example: Milestone.where(project_id: ids_with_milestone_available_for(user)) # For example: Milestone.where(project_id: ids_with_issuables_available_for(user))
def ids_with_milestone_available_for(user) def ids_with_issuables_available_for(user)
with_issues_enabled = with_issues_available_for_user(user).select(:id) with_issues_enabled = with_issues_available_for_user(user).select(:id)
with_merge_requests_enabled = with_merge_requests_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 ...@@ -163,7 +163,7 @@ module Gitlab
return Milestone.none if project_ids.nil? return Milestone.none if project_ids.nil?
authorized_project_ids_relation = 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) milestones.where(project_id: authorized_project_ids_relation)
end end
......
...@@ -128,6 +128,89 @@ describe LabelsFinder do ...@@ -128,6 +128,89 @@ describe LabelsFinder do
expect(finder.execute).to eq [private_subgroup_label_1] expect(finder.execute).to eq [private_subgroup_label_1]
end end
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 end
context 'filtering by project_id' do context 'filtering by project_id' do
......
...@@ -3416,7 +3416,7 @@ describe Project do ...@@ -3416,7 +3416,7 @@ describe Project do
end end
end end
describe '.ids_with_milestone_available_for' do describe '.ids_with_issuables_available_for' do
let!(:user) { create(:user) } let!(:user) { create(:user) }
it 'returns project ids with milestones available for user' do it 'returns project ids with milestones available for user' do
...@@ -3426,7 +3426,7 @@ describe Project do ...@@ -3426,7 +3426,7 @@ describe Project do
project_4 = create(:project, :public) project_4 = create(:project, :public)
project_4.project_feature.update(issues_access_level: ProjectFeature::PRIVATE, merge_requests_access_level: ProjectFeature::PRIVATE ) 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).to include(project_2.id, project_3.id)
expect(project_ids).not_to include(project_1.id, project_4.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