Merge branch '10003-api-endpoint-to-list-the-packages-of-a-group' into 'master'

API endpoint to list the packages of a group

See merge request gitlab-org/gitlab!18871

app/policies/group_policy.rb

@@ -44,6 +44,7 @@ class GroupPolicy < BasePolicy
 
   rule { public_group }.policy do
     enable :read_group
+    enable :read_package
   end
 
   rule { logged_in_viewable }.enable :read_group
@@ -70,7 +71,10 @@ class GroupPolicy < BasePolicy
 
   rule { has_access }.enable :read_namespace
 
-  rule { developer }.enable :admin_milestone
+  rule { developer }.policy do
+    enable :admin_milestone
+    enable :read_package
+  end
 
   rule { reporter }.policy do
     enable :read_container_image

doc/api/packages.md

@@ -2,7 +2,9 @@
 
 This is the API docs of [GitLab Packages](../administration/packages/index.md).
 
-## List project packages
+## List packages
+
+### Within a project
 
 > [Introduced](https://gitlab.com/gitlab-org/gitlab/merge_requests/9259) in GitLab 11.8.
 
@@ -42,6 +44,47 @@ Example response:
 
 By default, the `GET` request will return 20 results, since the API is [paginated](README.md#pagination).
 
+### Within a group
+
+> [Introduced](https://gitlab.com/gitlab-org/gitlab/merge_requests/18871) in GitLab 12.5.
+
+Get a list of project packages at the group level.
+When accessed without authentication, only packages of public projects are returned.
+
+```
+GET /groups/:id/packages
+```
+
+| Attribute | Type | Required | Description |
+| --------- | ---- | -------- | ----------- |
+| `id`      | integer/string | yes | ID or [URL-encoded path of the group](README.md#namespaced-path-encoding). |
+| `exclude_subgroups` | boolean | false | If the param is included as true, packages from projects from subgroups are not listed. Default is `false`. |
+
+```bash
+curl --header "PRIVATE-TOKEN: <your_access_token>" https://gitlab.example.com/api/v4/group/:id/packages?exclude_subgroups=true
+```
+
+Example response:
+
+```json
+[
+  {
+    "id": 1,
+    "name": "com/mycompany/my-app",
+    "version": "1.0-SNAPSHOT",
+    "package_type": "maven"
+  },
+  {
+    "id": 2,
+    "name": "@foo/bar",
+    "version": "1.0.3",
+    "package_type": "npm"
+  }
+]
+```
+
+By default, the `GET` request will return 20 results, since the API is [paginated](README.md#pagination).
+
 ## Get a project package
 
 > [Introduced](https://gitlab.com/gitlab-org/gitlab/merge_requests/9667) in GitLab 11.9.

ee/app/controllers/concerns/packages_access.rb

@@ -5,7 +5,7 @@ module PackagesAccess
 
   included do
     before_action :verify_packages_enabled!
-    before_action :authorize_read_package!
+    before_action :verify_read_package!
   end
 
   private
@@ -14,4 +14,8 @@ module PackagesAccess
     render_404 unless Gitlab.config.packages.enabled &&
       project.feature_available?(:packages)
   end
+
+  def verify_read_package!
+    authorize_read_package!(project)
+  end
 end

ee/app/finders/packages/group_packages_finder.rb

@@ -2,9 +2,9 @@
 
 module Packages
   class GroupPackagesFinder
-    attr_reader :current_user, :group
+    attr_reader :current_user, :group, :params
 
-    def initialize(current_user, group, params = {})
+    def initialize(current_user, group, params = { exclude_subgroups: false })
       @current_user = current_user
       @group = group
       @params = params
@@ -28,12 +28,22 @@ module Packages
 
     def group_projects_visible_to_current_user
       ::Project
-        .in_namespace(group.self_and_descendants.select(:id))
+        .in_namespace(groups)
         .public_or_visible_to_user(current_user, Gitlab::Access::REPORTER)
     end
 
     def package_type
       @params[:package_type].presence
     end
+
+    def groups
+      return [group] if exclude_subgroups?
+
+      group.self_and_descendants
+    end
+
+    def exclude_subgroups?
+      params[:exclude_subgroups]
+    end
   end
 end

ee/changelogs/unreleased/10003-api-endpoint-to-list-the-packages-of-a-group.yml

+---
+title: API endpoint to list the packages of a group
+merge_request: 18871
+author:
+type: added

ee/lib/api/group_packages.rb

+# frozen_string_literal: true
+
+module API
+  class GroupPackages < Grape::API
+    include PaginationParams
+
+    before do
+      authorize_packages_access!(user_group)
+    end
+
+    helpers ::API::Helpers::PackagesHelpers
+
+    params do
+      requires :id, type: String, desc: "Group's ID or path"
+      optional :exclude_subgroups, type: Boolean, default: false, desc: 'Determines if subgroups should be excluded'
+    end
+    resource :groups, requirements: API::NAMESPACE_OR_PROJECT_REQUIREMENTS do
+      desc 'Get all project packages' do
+        detail 'This feature was introduced in GitLab 12.5'
+        success EE::API::Entities::Package
+      end
+      params do
+        use :pagination
+      end
+      get ':id/packages' do
+        packages = Packages::GroupPackagesFinder.new(
+          current_user,
+          user_group,
+          exclude_subgroups: params[:exclude_subgroups]
+        ).execute
+
+        present paginate(packages), with: EE::API::Entities::Package
+      end
+    end
+  end
+end

ee/lib/api/helpers/packages_helpers.rb

@@ -7,14 +7,19 @@ module API
         not_found! unless ::Gitlab.config.packages.enabled
       end
 
-      def authorize_packages_feature!
-        forbidden! unless user_project.feature_available?(:packages)
+      def authorize_packages_access!(subject)
+        require_packages_enabled!
+        authorize_packages_feature!(subject)
+        authorize_read_package!(subject)
       end
 
-      def authorize_download_package!
-        authorize!(:read_package, user_project)
+      def authorize_packages_feature!(subject)
+        forbidden! unless subject.feature_available?(:packages)
+      end
+
+      def authorize_read_package!(subject)
+        authorize!(:read_package, subject)
       end
-      alias_method :authorize_read_package!, :authorize_download_package!
 
       def authorize_create_package!
         authorize!(:create_package, user_project)

ee/lib/api/maven_packages.rb

@@ -64,12 +64,12 @@ module API
       # the endpoint that includes project id
       project = find_project_by_path(params[:path])
 
-      authorize!(:read_package, project)
+      authorize_read_package!(project)
 
       package = ::Packages::MavenPackageFinder
         .new(params[:path], current_user, project: project).execute!
 
-      forbidden! unless package.project.feature_available?(:packages)
+      authorize_packages_feature!(package.project)
 
       package_file = ::Packages::PackageFileFinder
         .new(package, file_name).execute!
@@ -106,9 +106,8 @@ module API
         package = ::Packages::MavenPackageFinder
           .new(params[:path], current_user, group: group).execute!
 
-        forbidden! unless package.project.feature_available?(:packages)
-
-        authorize!(:read_package, package.project)
+        authorize_packages_feature!(package.project)
+        authorize_read_package!(package.project)
 
         package_file = ::Packages::PackageFileFinder
           .new(package, file_name).execute!
@@ -129,7 +128,7 @@ module API
     end
     resource :projects, requirements: API::NAMESPACE_OR_PROJECT_REQUIREMENTS do
       before do
-        authorize_packages_feature!
+        authorize_packages_feature!(user_project)
       end
 
       desc 'Download the maven package file' do
@@ -141,7 +140,7 @@ module API
       end
       route_setting :authentication, job_token_allowed: true
       get ':id/packages/maven/*path/:file_name', requirements: MAVEN_ENDPOINT_REQUIREMENTS do
-        authorize_download_package!
+        authorize_read_package!(user_project)
 
         file_name, format = extract_format(params[:file_name])
 

ee/lib/api/npm_packages.rb

@@ -33,8 +33,8 @@ module API
 
       project = find_project_by_package_name(package_name)
 
-      authorize!(:read_package, project)
-      forbidden! unless project.feature_available?(:packages)
+      authorize_read_package!(project)
+      authorize_packages_feature!(project)
 
       packages = ::Packages::NpmPackagesFinder
         .new(project, package_name).execute
@@ -48,7 +48,7 @@ module API
     end
     resource :projects, requirements: API::NAMESPACE_OR_PROJECT_REQUIREMENTS do
       before do
-        authorize_packages_feature!
+        authorize_packages_feature!(user_project)
       end
 
       desc 'Download the NPM tarball' do
@@ -59,7 +59,7 @@ module API
         requires :file_name, type: String, desc: 'Package file name'
       end
       get ':id/packages/npm/*package_name/-/*file_name', format: false do
-        authorize_download_package!
+        authorize_read_package!(user_project)
 
         package = user_project.packages.npm
           .by_name_and_file_name(params[:package_name], params[:file_name])

ee/lib/api/package_files.rb

@@ -5,9 +5,7 @@ module API
     include PaginationParams
 
     before do
-      require_packages_enabled!
-      authorize_packages_feature!
-      authorize_read_package!
+      authorize_packages_access!(user_project)
     end
 
     helpers ::API::Helpers::PackagesHelpers

ee/lib/api/packages.rb → ee/lib/api/project_packages.rb

 # frozen_string_literal: true
 
 module API
-  class Packages < Grape::API
+  class ProjectPackages < Grape::API
     include PaginationParams
 
     before do
-      require_packages_enabled!
-      authorize_packages_feature!
-      authorize_read_package!
+      authorize_packages_access!(user_project)
     end
 
     helpers ::API::Helpers::PackagesHelpers

ee/lib/ee/api/api.rb

@@ -32,7 +32,8 @@ module EE
         mount ::API::ConanPackages
         mount ::API::MavenPackages
         mount ::API::NpmPackages
-        mount ::API::Packages
+        mount ::API::ProjectPackages
+        mount ::API::GroupPackages
         mount ::API::PackageFiles
         mount ::API::Scim
         mount ::API::ManagedLicenses

ee/spec/finders/packages/group_packages_finder_spec.rb

@@ -2,19 +2,21 @@
 require 'spec_helper'
 
 describe Packages::GroupPackagesFinder do
-  let_it_be(:user)     { create(:user) }
-  let_it_be(:group)    { create(:group) }
-  let_it_be(:project)  { create(:project, namespace: group) }
+  let(:user)     { create(:user) }
+  let(:group)    { create(:group) }
+  let(:project)  { create(:project, namespace: group) }
+  let(:another_group) { create(:group) }
 
   before do
     group.add_developer(user)
   end
 
   describe '#execute' do
-    subject { described_class.new(user, group).execute }
+    let(:params) { { exclude_subgroups: false } }
+    subject { described_class.new(user, group, params).execute }
 
     shared_examples 'with package type' do |package_type|
-      subject { described_class.new(user, group, package_type: package_type).execute }
+      let(:params) { { exclude_subgroups: false, package_type: package_type } }
 
       it { is_expected.to match_array([send("package_#{package_type}")]) }
     end
@@ -24,11 +26,25 @@ describe Packages::GroupPackagesFinder do
     end
 
     context 'group has packages' do
-      let(:package1) { create(:maven_package, project: project) }
-      let(:package2) { create(:maven_package, project: project) }
-      let_it_be(:package3) { create(:maven_package) }
+      let!(:package1) { create(:maven_package, project: project) }
+      let!(:package2) { create(:maven_package, project: project) }
+      let!(:package3) { create(:maven_package) }
 
       it { is_expected.to match_array([package1, package2]) }
+
+      context 'subgroup has packages' do
+        let(:subgroup) { create(:group, parent: group) }
+        let(:subproject) { create(:project, namespace: subgroup) }
+        let!(:package4) { create(:npm_package, project: subproject) }
+
+        it { is_expected.to match_array([package1, package2, package4]) }
+
+        context 'excluding subgroups' do
+          let(:params) { { exclude_subgroups: true } }
+
+          it { is_expected.to match_array([package1, package2]) }
+        end
+      end
     end
 
     context 'group has package of all types' do
@@ -50,7 +66,7 @@ describe Packages::GroupPackagesFinder do
     end
 
     context 'package type is nil' do
-      let_it_be(:package1) { create(:maven_package, project: project) }
+      let!(:package1) { create(:maven_package, project: project) }
 
       subject { described_class.new(user, group, package_type: nil).execute }
 

ee/spec/requests/api/group_packages_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe API::GroupPackages do
+  let(:group) { create(:group, :public) }
+  let(:project) { create(:project, :public, namespace: group) }
+  let!(:package1) { create(:npm_package, project: project) }
+  let!(:package2) { create(:npm_package, project: project) }
+  let(:user) { create(:user) }
+
+  subject { get api(url) }
+
+  describe 'GET /groups/:id/packages' do
+    let(:url) { "/groups/#{group.id}/packages" }
+
+    context 'with packages feature enabled' do
+      before do
+        stub_licensed_features(packages: true)
+      end
+
+      context 'with private group' do
+        let(:group) { create(:group, :private) }
+        let(:subgroup) { create(:group, :private, parent: group) }
+        let(:project) { create(:project, :private, namespace: group) }
+        let(:subproject) { create(:project, :private, namespace: subgroup) }
+
+        context 'with unauthenticated user' do
+          it_behaves_like 'rejects packages access', :group, :no_type, :not_found
+        end
+
+        context 'with authenticated user' do
+          subject { get api(url, user) }
+
+          it_behaves_like 'returns packages', :group, :owner
+          it_behaves_like 'returns packages', :group, :maintainer
+          it_behaves_like 'returns packages', :group, :developer
+          it_behaves_like 'rejects packages access', :group, :reporter, :forbidden
+          it_behaves_like 'rejects packages access', :group, :guest, :forbidden
+
+          context 'with subgroup' do
+            let(:subgroup) { create(:group, :private, parent: group) }
+            let(:subproject) { create(:project, :private, namespace: subgroup) }
+            let!(:package3) { create(:npm_package, project: subproject) }
+
+            it_behaves_like 'returns packages with subgroups', :group, :owner
+            it_behaves_like 'returns packages with subgroups', :group, :maintainer
+            it_behaves_like 'returns packages with subgroups', :group, :developer
+            it_behaves_like 'rejects packages access', :group, :reporter, :forbidden
+            it_behaves_like 'rejects packages access', :group, :guest, :forbidden
+
+            context 'excluding subgroup' do
+              let(:url) { "/groups/#{group.id}/packages?exclude_subgroups=true" }
+
+              it_behaves_like 'returns packages', :group, :owner
+              it_behaves_like 'returns packages', :group, :maintainer
+              it_behaves_like 'returns packages', :group, :developer
+              it_behaves_like 'rejects packages access', :group, :reporter, :forbidden
+              it_behaves_like 'rejects packages access', :group, :guest, :forbidden
+            end
+          end
+        end
+      end
+
+      context 'with public group' do
+        context 'with unauthenticated user' do
+          it_behaves_like 'returns packages', :group, :no_type
+        end
+
+        context 'with authenticated user' do
+          subject { get api(url, user) }
+
+          it_behaves_like 'returns packages', :group, :owner
+          it_behaves_like 'returns packages', :group, :maintainer
+          it_behaves_like 'returns packages', :group, :developer
+          it_behaves_like 'returns packages', :group, :reporter
+          it_behaves_like 'returns packages', :group, :guest
+        end
+      end
+
+      context 'with pagination params' do
+        let!(:package3) { create(:npm_package, project: project) }
+        let!(:package4) { create(:npm_package, project: project) }
+
+        it_behaves_like 'returns paginated packages'
+      end
+    end
+
+    context 'with packages feature disabled' do
+      before do
+        stub_licensed_features(packages: false)
+      end
+
+      it_behaves_like 'rejects packages access', :group, :no_type, :forbidden
+    end
+  end
+end

ee/spec/requests/api/packages_spec.rb → ee/spec/requests/api/project_packages_spec.rb

@@ -2,81 +2,57 @@
 
 require 'spec_helper'
 
-describe API::Packages do
+describe API::ProjectPackages do
   let(:user) { create(:user) }
   let(:project) { create(:project, :public) }
-  let(:package) { create(:npm_package, project: project) }
-  let(:package_url) { "/projects/#{project.id}/packages/#{package.id}" }
-  let(:another_package) { create(:npm_package) }
+  let!(:package1) { create(:npm_package, project: project) }
+  let(:package_url) { "/projects/#{project.id}/packages/#{package1.id}" }
+  let!(:package2) { create(:npm_package, project: project) }
+  let!(:another_package) { create(:npm_package) }
   let(:no_package_url) { "/projects/#{project.id}/packages/0" }
   let(:wrong_package_url) { "/projects/#{project.id}/packages/#{another_package.id}" }
 
   describe 'GET /projects/:id/packages' do
     let(:url) { "/projects/#{project.id}/packages" }
 
+    subject { get api(url) }
+
     context 'packages feature enabled' do
       before do
         stub_licensed_features(packages: true)
       end
 
       context 'project is public' do
-        it 'returns 200' do
-          get api(url)
-
-          expect(response).to have_gitlab_http_status(200)
-        end
+        it_behaves_like 'returns packages', :project, :no_type
       end
 
       context 'project is private' do
         let(:project) { create(:project, :private) }
 
-        it 'returns 404 for non authenticated user' do
-          get api(url)
-
-          expect(response).to have_gitlab_http_status(404)
-        end
-
-        it 'returns 404 for a user without access to the project' do
-          get api(no_package_url, user)
-
-          expect(response).to have_gitlab_http_status(404)
+        context 'for unauthenticated user' do
+          it_behaves_like 'rejects packages access', :project, :no_type, :not_found
         end
 
-        it 'returns 200 and valid response schema' do
-          project.add_maintainer(user)
-
-          get api(url, user)
+        context 'for authenticated user' do
+          subject { get api(url, user) }
 
-          expect(response).to have_gitlab_http_status(200)
-          expect(response).to match_response_schema('public_api/v4/packages/packages', dir: 'ee')
+          it_behaves_like 'returns packages', :project, :maintainer
+          it_behaves_like 'returns packages', :project, :developer
+          it_behaves_like 'returns packages', :project, :reporter
+          it_behaves_like 'rejects packages access', :project, :no_type, :not_found
+          it_behaves_like 'rejects packages access', :project, :guest, :forbidden
         end
       end
 
       context 'with pagination params' do
-        let(:per_page) { 2 }
-        let!(:package1) { create(:npm_package, project: project) }
-        let!(:package2) { create(:npm_package, project: project) }
         let!(:package3) { create(:maven_package, project: project) }
+        let!(:package4) { create(:maven_package, project: project) }
 
-        before do
-          project.add_maintainer(user)
-          stub_licensed_features(packages: true)
-        end
-
-        context 'when viewing the first page' do
-          it 'returns first 2 packages' do
-            get api(url, user), params: { page: 1, per_page: per_page }
-
-            expect_paginated_array_response([package1.id, package2.id])
-          end
-        end
-
-        context 'viewing the second page' do
-          it 'returns the last package' do
-            get api(url, user), params: { page: 2, per_page: per_page }
+        context 'with pagination params' do
+          let!(:package3) { create(:npm_package, project: project) }
+          let!(:package4) { create(:npm_package, project: project) }
 
-            expect_paginated_array_response([package3.id])
-          end
+          it_behaves_like 'returns paginated packages'
         end
       end
     end

ee/spec/support/shared_examples/packages_shared_examples.rb

+# frozen_string_literal: true
+
+shared_examples 'returns packages' do |container_type, user_type|
+  context "for #{user_type}" do
+    before do
+      send(container_type)&.send("add_#{user_type}", user) unless user_type == :no_type
+    end
+
+    it 'returns success response' do
+      subject
+
+      expect(response).to have_gitlab_http_status(:success)
+    end
+
+    it 'returns a valid response schema' do
+      subject
+
+      expect(response).to match_response_schema('public_api/v4/packages/packages', dir: 'ee')
+    end
+
+    it 'returns two packages' do
+      subject
+
+      expect(json_response.length).to eq(2)
+      expect(json_response.map { |package| package['id'] }).to contain_exactly(package1.id, package2.id)
+    end
+  end
+end
+
+shared_examples 'returns packages with subgroups' do |container_type, user_type|
+  context "with subgroups for #{user_type}" do
+    before do
+      send(container_type)&.send("add_#{user_type}", user) unless user_type == :no_type
+    end
+
+    it 'returns success response' do
+      subject
+
+      expect(response).to have_gitlab_http_status(:success)
+    end
+
+    it 'returns a valid response schema' do
+      subject
+
+      expect(response).to match_response_schema('public_api/v4/packages/packages', dir: 'ee')
+    end
+
+    it 'returns three packages' do
+      subject
+
+      expect(json_response.length).to eq(3)
+      expect(json_response.map { |package| package['id'] }).to contain_exactly(package1.id, package2.id, package3.id)
+    end
+  end
+end
+
+shared_examples 'rejects packages access' do |container_type, user_type, status|
+  context "for #{user_type}" do
+    before do
+      send(container_type)&.send("add_#{user_type}", user) unless user_type == :no_type
+    end
+
+    it "returns #{status}" do
+      subject
+
+      expect(response).to have_gitlab_http_status(status)
+    end
+  end
+end
+
+shared_examples 'returns paginated packages' do
+  let(:per_page) { 2 }
+
+  context 'when viewing the first page' do
+    let(:page) { 1 }
+    it 'returns first 2 packages' do
+      get api(url, user), params: { page: page, per_page: per_page }
+
+      expect_paginated_array_response([package1.id, package2.id])
+    end
+  end
+
+  context 'when viewing the second page' do
+    let(:page) { 2 }
+    it 'returns first 2 packages' do
+      get api(url, user), params: { page: page, per_page: per_page }
+
+      expect_paginated_array_response([package3.id, package4.id])
+    end
+  end
+end