Add API method listing packages within a group

This API method is similar to the one listing packages within a project
but for the group level

Rename the existing packages API class to make it more explicit that it
  is for the project level
Add the packages API class for the group level
Add the related access checks in `GroupPolicy`
Refactor packages API specs so that both levels share a common examples
Add the docs for the new group level API method

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