Merge branch '326653-fix-package-finder-helpers' into 'master'

Update the group permission check in packages finder helper

See merge request gitlab-org/gitlab!58329

app/finders/concerns/packages/finder_helper.rb

@@ -11,7 +11,7 @@ module Packages
 
     def packages_visible_to_user(user, within_group:)
       return ::Packages::Package.none unless within_group
-      return ::Packages::Package.none unless Ability.allowed?(user, :read_package, 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))
       ::Packages::Package.for_projects(projects.select(:id))
@@ -19,7 +19,7 @@ module Packages
 
     def projects_visible_to_user(user, within_group:)
       return ::Project.none unless within_group
-      return ::Project.none unless Ability.allowed?(user, :read_package, 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))
     end

changelogs/unreleased/326653-fix-package-finder-helpers.yml

+---
+title: Update the group permission check in packages finder helper
+merge_request: 58329
+author:
+type: fixed

spec/requests/api/maven_packages_spec.rb

@@ -420,6 +420,25 @@ RSpec.describe API::MavenPackages do
             expect(response.media_type).to eq('application/octet-stream')
           end
         end
+
+        context 'with a reporter from a subgroup accessing the root group' do
+          let_it_be(:root_group) { create(:group, :private) }
+          let_it_be(:group) { create(:group, :private, parent: root_group) }
+
+          subject { download_file_with_token(package_file.file_name, {}, headers_with_token, root_group.id) }
+
+          before do
+            project.update!(namespace: group)
+            group.add_reporter(user)
+          end
+
+          it 'returns the file' do
+            subject
+
+            expect(response).to have_gitlab_http_status(:ok)
+            expect(response.media_type).to eq('application/octet-stream')
+          end
+        end
       end
 
       context 'maven metadata file' do
@@ -492,12 +511,12 @@ RSpec.describe API::MavenPackages do
       it_behaves_like 'handling all conditions'
     end
 
-    def download_file(file_name, params = {}, request_headers = headers)
-      get api("/groups/#{group.id}/-/packages/maven/#{maven_metadatum.path}/#{file_name}"), params: params, headers: request_headers
+    def download_file(file_name, params = {}, request_headers = headers, group_id = group.id)
+      get api("/groups/#{group_id}/-/packages/maven/#{maven_metadatum.path}/#{file_name}"), params: params, headers: request_headers
     end
 
-    def download_file_with_token(file_name, params = {}, request_headers = headers_with_token)
-      download_file(file_name, params, request_headers)
+    def download_file_with_token(file_name, params = {}, request_headers = headers_with_token, group_id = group.id)
+      download_file(file_name, params, request_headers, group_id)
     end
   end
 

spec/requests/api/nuget_group_packages_spec.rb

@@ -113,6 +113,45 @@ RSpec.describe API::NugetGroupPackages do
       end
     end
 
+    context 'with a reporter of subgroup' do
+      let_it_be(:package_name) { 'Dummy.Package' }
+      let_it_be(:package) { create(:nuget_package, :with_metadatum, name: package_name, project: project) }
+
+      let(:headers) { basic_auth_header(user.username, personal_access_token.token) }
+
+      subject { get api(url), headers: headers }
+
+      before do
+        subgroup.add_reporter(user)
+        project.update_column(:visibility_level, Gitlab::VisibilityLevel.level_value('private'))
+        subgroup.update_column(:visibility_level, Gitlab::VisibilityLevel.level_value('private'))
+        group.update_column(:visibility_level, Gitlab::VisibilityLevel.level_value('private'))
+      end
+
+      describe 'GET /api/v4/groups/:id/-/packages/nuget/metadata/*package_name/index' do
+        let(:url) { "/groups/#{group.id}/-/packages/nuget/metadata/#{package_name}/index.json" }
+
+        it_behaves_like 'returning response status', :forbidden
+      end
+
+      describe 'GET /api/v4/groups/:id/-/packages/nuget/metadata/*package_name/*package_version' do
+        let(:url) { "/groups/#{group.id}/-/packages/nuget/metadata/#{package_name}/#{package.version}.json" }
+
+        it_behaves_like 'returning response status', :forbidden
+      end
+
+      describe 'GET /api/v4/groups/:id/-/packages/nuget/query' do
+        let(:search_term) { 'uMmy' }
+        let(:take) { 26 }
+        let(:skip) { 0 }
+        let(:include_prereleases) { false }
+        let(:query_parameters) { { q: search_term, take: take, skip: skip, prerelease: include_prereleases } }
+        let(:url) { "/groups/#{group.id}/-/packages/nuget/query?#{query_parameters.to_query}" }
+
+        it_behaves_like 'returning response status', :forbidden
+      end
+    end
+
     def update_visibility_to(visibility)
       project.update!(visibility_level: visibility)
       subgroup.update!(visibility_level: visibility)

spec/support/shared_examples/requests/api/nuget_endpoints_shared_examples.rb

@@ -156,7 +156,7 @@ RSpec.shared_examples 'handling nuget metadata requests with package name and pa
   include_context 'with expected presenters dependency groups'
 
   let_it_be(:package_name) { 'Dummy.Package' }
-  let_it_be(:package) { create(:nuget_package, :with_metadatum, name: 'Dummy.Package', project: project) }
+  let_it_be(:package) { create(:nuget_package, :with_metadatum, name: package_name, project: project) }
   let_it_be(:tag) { create(:packages_tag, package: package, name: 'test') }
 
   subject { get api(url) }