Commit bf4884d0 authored by David Fernandez's avatar David Fernandez Committed by Alex Kalderimis

Optimize the package finder helper when dealing with deploy tokens [RUN ALL RSPEC] [RUN AS-IF-FOSS]

parent d2be5844
...@@ -13,7 +13,7 @@ module Packages ...@@ -13,7 +13,7 @@ module Packages
return ::Packages::Package.none unless within_group return ::Packages::Package.none unless within_group
return ::Packages::Package.none unless Ability.allowed?(user, :read_group, within_group) return ::Packages::Package.none unless Ability.allowed?(user, :read_group, within_group)
projects = projects_visible_to_reporters(user, within_group.self_and_descendants.select(:id)) projects = projects_visible_to_reporters(user, within_group: within_group)
::Packages::Package.for_projects(projects.select(:id)) ::Packages::Package.for_projects(projects.select(:id))
end end
...@@ -21,13 +21,17 @@ module Packages ...@@ -21,13 +21,17 @@ module Packages
return ::Project.none unless within_group return ::Project.none unless within_group
return ::Project.none unless Ability.allowed?(user, :read_group, within_group) return ::Project.none unless Ability.allowed?(user, :read_group, within_group)
projects_visible_to_reporters(user, within_group.self_and_descendants.select(:id)) projects_visible_to_reporters(user, within_group: within_group)
end end
def projects_visible_to_reporters(user, namespace_ids) def projects_visible_to_reporters(user, within_group:)
::Project.in_namespace(namespace_ids) if user.is_a?(DeployToken) && Feature.enabled?(:packages_finder_helper_deploy_token)
user.accessible_projects
else
within_group.all_projects
.public_or_visible_to_user(user, ::Gitlab::Access::REPORTER) .public_or_visible_to_user(user, ::Gitlab::Access::REPORTER)
end end
end
def package_type def package_type
params[:package_type].presence params[:package_type].presence
......
---
name: packages_finder_helper_deploy_token
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/58497
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/326808
milestone: '13.11'
type: development
group: group::package
default_enabled: false
...@@ -6,7 +6,6 @@ RSpec.describe ::Packages::FinderHelper do ...@@ -6,7 +6,6 @@ RSpec.describe ::Packages::FinderHelper do
describe '#packages_visible_to_user' do describe '#packages_visible_to_user' do
using RSpec::Parameterized::TableSyntax using RSpec::Parameterized::TableSyntax
let_it_be(:user) { create(:user) }
let_it_be_with_reload(:group) { create(:group) } let_it_be_with_reload(:group) { create(:group) }
let_it_be_with_reload(:project1) { create(:project, namespace: group) } let_it_be_with_reload(:project1) { create(:project, namespace: group) }
let_it_be(:package1) { create(:package, project: project1) } let_it_be(:package1) { create(:package, project: project1) }
...@@ -44,6 +43,9 @@ RSpec.describe ::Packages::FinderHelper do ...@@ -44,6 +43,9 @@ RSpec.describe ::Packages::FinderHelper do
it { is_expected.to be_empty } it { is_expected.to be_empty }
end end
context 'with a user' do
let_it_be(:user) { create(:user) }
where(:group_visibility, :subgroup_visibility, :project2_visibility, :user_role, :shared_example_name) do where(:group_visibility, :subgroup_visibility, :project2_visibility, :user_role, :shared_example_name) do
'PUBLIC' | 'PUBLIC' | 'PUBLIC' | :maintainer | 'returning both packages' 'PUBLIC' | 'PUBLIC' | 'PUBLIC' | :maintainer | 'returning both packages'
'PUBLIC' | 'PUBLIC' | 'PUBLIC' | :developer | 'returning both packages' 'PUBLIC' | 'PUBLIC' | 'PUBLIC' | :developer | 'returning both packages'
...@@ -82,6 +84,49 @@ RSpec.describe ::Packages::FinderHelper do ...@@ -82,6 +84,49 @@ RSpec.describe ::Packages::FinderHelper do
end end
end end
context 'with a group deploy token' do
let_it_be(:user) { create(:deploy_token, :group, read_package_registry: true) }
let_it_be(:group_deploy_token) { create(:group_deploy_token, deploy_token: user, group: group) }
shared_examples 'handling all conditions' do
where(:group_visibility, :subgroup_visibility, :project2_visibility, :shared_example_name) do
'PUBLIC' | 'PUBLIC' | 'PUBLIC' | 'returning both packages'
'PUBLIC' | 'PUBLIC' | 'PRIVATE' | 'returning both packages'
'PUBLIC' | 'PRIVATE' | 'PRIVATE' | 'returning both packages'
'PRIVATE' | 'PRIVATE' | 'PRIVATE' | 'returning both packages'
end
with_them do
before do
project2.update!(visibility_level: Gitlab::VisibilityLevel.const_get(project2_visibility, false))
subgroup.update!(visibility_level: Gitlab::VisibilityLevel.const_get(subgroup_visibility, false))
project1.update!(visibility_level: Gitlab::VisibilityLevel.const_get(group_visibility, false))
group.update!(visibility_level: Gitlab::VisibilityLevel.const_get(group_visibility, false))
end
it_behaves_like params[:shared_example_name]
end
end
context 'with packages_finder_helper_deploy_token enabled' do
before do
expect(group).not_to receive(:all_projects)
end
it_behaves_like 'handling all conditions'
end
context 'with packages_finder_helper_deploy_token disabled' do
before do
stub_feature_flags(packages_finder_helper_deploy_token: false)
expect(group).to receive(:all_projects).and_call_original
end
it_behaves_like 'handling all conditions'
end
end
end
describe '#projects_visible_to_user' do describe '#projects_visible_to_user' do
using RSpec::Parameterized::TableSyntax using RSpec::Parameterized::TableSyntax
...@@ -121,6 +166,9 @@ RSpec.describe ::Packages::FinderHelper do ...@@ -121,6 +166,9 @@ RSpec.describe ::Packages::FinderHelper do
it { is_expected.to be_empty } it { is_expected.to be_empty }
end end
context 'with a user' do
let_it_be(:user) { create(:user) }
where(:group_visibility, :subgroup_visibility, :project2_visibility, :user_role, :shared_example_name) do where(:group_visibility, :subgroup_visibility, :project2_visibility, :user_role, :shared_example_name) do
'PUBLIC' | 'PUBLIC' | 'PUBLIC' | :maintainer | 'returning both projects' 'PUBLIC' | 'PUBLIC' | 'PUBLIC' | :maintainer | 'returning both projects'
'PUBLIC' | 'PUBLIC' | 'PUBLIC' | :developer | 'returning both projects' 'PUBLIC' | 'PUBLIC' | 'PUBLIC' | :developer | 'returning both projects'
...@@ -158,4 +206,47 @@ RSpec.describe ::Packages::FinderHelper do ...@@ -158,4 +206,47 @@ RSpec.describe ::Packages::FinderHelper do
it_behaves_like params[:shared_example_name] it_behaves_like params[:shared_example_name]
end end
end end
context 'with a group deploy token' do
let_it_be(:user) { create(:deploy_token, :group, read_package_registry: true) }
let_it_be(:group_deploy_token) { create(:group_deploy_token, deploy_token: user, group: group) }
shared_examples 'handling all conditions' do
where(:group_visibility, :subgroup_visibility, :project2_visibility, :shared_example_name) do
'PUBLIC' | 'PUBLIC' | 'PUBLIC' | 'returning both projects'
'PUBLIC' | 'PUBLIC' | 'PRIVATE' | 'returning both projects'
'PUBLIC' | 'PRIVATE' | 'PRIVATE' | 'returning both projects'
'PRIVATE' | 'PRIVATE' | 'PRIVATE' | 'returning both projects'
end
with_them do
before do
project2.update!(visibility_level: Gitlab::VisibilityLevel.const_get(project2_visibility, false))
subgroup.update!(visibility_level: Gitlab::VisibilityLevel.const_get(subgroup_visibility, false))
project1.update!(visibility_level: Gitlab::VisibilityLevel.const_get(group_visibility, false))
group.update!(visibility_level: Gitlab::VisibilityLevel.const_get(group_visibility, false))
end
it_behaves_like params[:shared_example_name]
end
end
context 'with packages_finder_helper_deploy_token enabled' do
before do
expect(group).not_to receive(:all_projects)
end
it_behaves_like 'handling all conditions'
end
context 'with packages_finder_helper_deploy_token disabled' do
before do
stub_feature_flags(packages_finder_helper_deploy_token: false)
expect(group).to receive(:all_projects).and_call_original
end
it_behaves_like 'handling all conditions'
end
end
end
end end
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