From 0880fd8d1ef634bf425c553713ca4124046138a0 Mon Sep 17 00:00:00 2001
From: Zamir Martins Filho <zfilho@gitlab.com>
Date: Thu, 11 May 2023 16:39:59 +0200
Subject: [PATCH 1/8] Add routes, controller and view to

group level dependencies.

EE: true
Changelog: added
---
 .../groups/dependencies_controller.rb         |  58 ++++++
 ee/app/finders/sbom/dependencies_finder.rb    |   4 +-
 ee/app/policies/ee/group_policy.rb            |   1 +
 ee/app/serializers/dependency_entity.rb       |   8 +-
 .../views/groups/dependencies/index.html.haml |  11 ++
 .../development/group_level_dependencies.yml  |   8 +
 ee/config/routes/group.rb                     |   2 +
 .../groups/dependencies_controller_spec.rb    | 187 ++++++++++++++++++
 ee/spec/policies/group_policy_spec.rb         |   2 +-
 ee/spec/serializers/dependency_entity_spec.rb |  10 +
 10 files changed, 287 insertions(+), 4 deletions(-)
 create mode 100644 ee/app/controllers/groups/dependencies_controller.rb
 create mode 100644 ee/app/views/groups/dependencies/index.html.haml
 create mode 100644 ee/config/feature_flags/development/group_level_dependencies.yml
 create mode 100644 ee/spec/controllers/groups/dependencies_controller_spec.rb

diff --git a/ee/app/controllers/groups/dependencies_controller.rb b/ee/app/controllers/groups/dependencies_controller.rb
new file mode 100644
index 0000000000000000..f618df28be805c43
--- /dev/null
+++ b/ee/app/controllers/groups/dependencies_controller.rb
@@ -0,0 +1,58 @@
+# frozen_string_literal: true
+
+module Groups
+  class DependenciesController < Groups::ApplicationController
+    before_action :authorize_read_dependency_list!
+
+    feature_category :dependency_management
+    urgency :low
+
+    def index
+      respond_to do |format|
+        format.html do
+          render status: :ok
+        end
+        format.json do
+          render json: serializer.represent(dependencies)
+        end
+      end
+    end
+
+    private
+
+    def authorize_read_dependency_list!
+      return if can?(current_user, :read_dependencies, group) && Feature.enabled?(:group_level_dependencies, group)
+
+      render_not_authorized
+    end
+
+    def dependency_list_params
+      params.permit(:sort_by, :sort, package_managers: [])
+    end
+
+    def collect_dependencies
+      return [] unless can?(current_user, :read_security_resource, group)
+
+      ::Sbom::DependenciesFinder.new(group, params: dependency_list_params).execute
+    end
+
+    def dependencies
+      @dependencies ||= ::Gitlab::ItemsCollection.new(collect_dependencies)
+    end
+
+    def serializer
+      ::DependencyListSerializer.new(group: group, user: current_user).with_pagination(request, response)
+    end
+
+    def render_not_authorized
+      respond_to do |format|
+        format.html do
+          render_404
+        end
+        format.json do
+          render_403
+        end
+      end
+    end
+  end
+end
diff --git a/ee/app/finders/sbom/dependencies_finder.rb b/ee/app/finders/sbom/dependencies_finder.rb
index 03b1a8df72186737..2ae922fe1b20b993 100644
--- a/ee/app/finders/sbom/dependencies_finder.rb
+++ b/ee/app/finders/sbom/dependencies_finder.rb
@@ -2,6 +2,8 @@
 
 module Sbom
   class DependenciesFinder
+    SBOM_OCCURRENCES_LIMIT = 100
+
     def initialize(project_or_group, params: {})
       @project_or_group = project_or_group
       @params = params
@@ -17,7 +19,7 @@ def execute
         sbom_occurrences.order_by_package_name(params[:sort])
       else
         sbom_occurrences.order_by_id
-      end
+      end.limit(SBOM_OCCURRENCES_LIMIT)
     end
 
     private
diff --git a/ee/app/policies/ee/group_policy.rb b/ee/app/policies/ee/group_policy.rb
index e034f86e8694d111..3f05868a6df10a23 100644
--- a/ee/app/policies/ee/group_policy.rb
+++ b/ee/app/policies/ee/group_policy.rb
@@ -441,6 +441,7 @@ module GroupPolicy
       rule { can?(:read_group_security_dashboard) }.policy do
         enable :create_vulnerability_export
         enable :read_security_resource
+        enable :read_dependencies
       end
 
       rule { admin | owner }.policy do
diff --git a/ee/app/serializers/dependency_entity.rb b/ee/app/serializers/dependency_entity.rb
index c9c5bec238b6ec20..d84914e048b1812a 100644
--- a/ee/app/serializers/dependency_entity.rb
+++ b/ee/app/serializers/dependency_entity.rb
@@ -28,10 +28,14 @@ class LicenseEntity < Grape::Entity
   private
 
   def can_read_vulnerabilities?
-    can?(request.user, :read_security_resource, request.project)
+    return can?(request.user, :read_security_resource, request.project) if request.respond_to?(:project)
+
+    false
   end
 
   def can_read_licenses?
-    can?(request.user, :read_licenses, request.project)
+    return can?(request.user, :read_licenses, request.project) if request.respond_to?(:project)
+
+    false
   end
 end
diff --git a/ee/app/views/groups/dependencies/index.html.haml b/ee/app/views/groups/dependencies/index.html.haml
new file mode 100644
index 0000000000000000..213a99a4460012ae
--- /dev/null
+++ b/ee/app/views/groups/dependencies/index.html.haml
@@ -0,0 +1,11 @@
+- breadcrumb_title _('Dependency list')
+- page_title _('Dependency list')
+
+#js-dependencies-app{
+  data: {
+    endpoint: group_dependencies_path(@group, format: :json),
+    documentation_path: help_page_path('user/application_security/dependency_list/index'),
+    support_documentation_path: help_page_path('user/application_security/dependency_scanning/index', anchor: 'supported-languages-and-package-managers'),
+    empty_state_svg_path: image_path('illustrations/Dependency-list-empty-state.svg')
+  }
+}
diff --git a/ee/config/feature_flags/development/group_level_dependencies.yml b/ee/config/feature_flags/development/group_level_dependencies.yml
new file mode 100644
index 0000000000000000..7e5b32d06a08ddff
--- /dev/null
+++ b/ee/config/feature_flags/development/group_level_dependencies.yml
@@ -0,0 +1,8 @@
+---
+name: group_level_dependencies
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/120489
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/411257
+milestone: '16.0'
+type: development
+group: group::threat insights
+default_enabled: false
diff --git a/ee/config/routes/group.rb b/ee/config/routes/group.rb
index 0db875d0ccf4a88b..6efe588fa02fdbc9 100644
--- a/ee/config/routes/group.rb
+++ b/ee/config/routes/group.rb
@@ -195,6 +195,8 @@
       resources :compliance_framework_reports, only: [:index], constraints: { format: :csv }
     end
 
+    resources :dependencies, only: [:index]
+
     resource :push_rules, only: [:update]
 
     resources :protected_branches, only: [:create, :update, :destroy]
diff --git a/ee/spec/controllers/groups/dependencies_controller_spec.rb b/ee/spec/controllers/groups/dependencies_controller_spec.rb
new file mode 100644
index 0000000000000000..eccc646b2eeaf627
--- /dev/null
+++ b/ee/spec/controllers/groups/dependencies_controller_spec.rb
@@ -0,0 +1,187 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Groups::DependenciesController, feature_category: :dependency_management do
+  let_it_be(:user) { create(:user) }
+  let_it_be(:group) { create(:group) }
+
+  before do
+    sign_in(user)
+  end
+
+  describe 'GET index' do
+    context 'with HTML format' do
+      subject { get :index, params: { group_id: group.to_param } }
+
+      context 'when security dashboard feature is enabled' do
+        before do
+          stub_licensed_features(security_dashboard: true)
+          stub_feature_flags(group_level_dependencies: true)
+        end
+
+        context 'and user is allowed to access group level dependencies' do
+          render_views
+
+          before do
+            group.add_developer(user)
+          end
+
+          it { is_expected.to have_gitlab_http_status(:ok) }
+
+          it 'returns the correct template' do
+            subject
+
+            expect(assigns(:group)).to eq(group)
+            expect(response).to render_template(:index)
+            expect(response.body).to include('data-documentation-path')
+            expect(response.body).to include('data-empty-state-svg-path')
+            expect(response.body).to include('data-endpoint')
+            expect(response.body).to include('data-support-documentation-path')
+          end
+
+          context 'when feature flag group_level_dependencies is disabled' do
+            before do
+              stub_feature_flags(group_level_dependencies: false)
+            end
+
+            it { is_expected.to have_gitlab_http_status(:not_found) }
+          end
+        end
+
+        context 'when user is not allowed to access group level dependencies' do
+          it { is_expected.to have_gitlab_http_status(:not_found) }
+        end
+      end
+
+      context 'when security dashboard feature is disabled' do
+        it { is_expected.to have_gitlab_http_status(:not_found) }
+      end
+    end
+
+    context 'with JSON format' do
+      subject { get :index, params: params, format: :json }
+
+      let(:params) { { group_id: group.to_param } }
+
+      context 'when security dashboard feature is enabled' do
+        before do
+          stub_licensed_features(security_dashboard: true)
+          stub_feature_flags(group_level_dependencies: true)
+        end
+
+        context 'and user is allowed to access group level dependencies' do
+          render_views
+
+          let(:expected_response) do
+            {
+              'report' => {
+                'status' => 'job_not_set_up'
+              },
+              'dependencies' => []
+            }
+          end
+
+          before do
+            group.add_developer(user)
+          end
+
+          it { is_expected.to have_gitlab_http_status(:ok) }
+
+          it 'returns the expected data' do
+            subject
+
+            expect(json_response).to eq(expected_response)
+          end
+
+          context 'with existing dependencies' do
+            let_it_be(:project) { create(:project, group: group) }
+            let_it_be(:sbom_occurrence_npm) { create(:sbom_occurrence, project: project, packager_name: 'npm') }
+            let_it_be(:sbom_occurrence_bundler) { create(:sbom_occurrence, project: project, packager_name: 'bundler') }
+
+            let(:expected_response) do
+              {
+                'report' => {
+                  'status' => 'job_not_set_up'
+                },
+                'dependencies' => [
+                  {
+                    'location' => sbom_occurrence_npm.location.as_json,
+                    'name' => sbom_occurrence_npm.name,
+                    'packager' => sbom_occurrence_npm.packager,
+                    'version' => sbom_occurrence_npm.version
+                  },
+                  {
+                    'location' => sbom_occurrence_bundler.location.as_json,
+                    'name' => sbom_occurrence_bundler.name,
+                    'packager' => sbom_occurrence_bundler.packager,
+                    'version' => sbom_occurrence_bundler.version
+                  }
+                ]
+              }
+            end
+
+            it 'returns the paginated data' do
+              subject
+
+              expect(json_response).to eq(expected_response)
+              expect(response).to include_pagination_headers
+            end
+
+            context 'with sorting params' do
+              context 'when sorted by packager' do
+                let(:params) { { group_id: group.to_param, sort_by: 'packager', sort: 'desc' } }
+
+                it 'returns sorted list' do
+                  subject
+
+                  expect(json_response['dependencies'].first['packager']).to eq('npm')
+                  expect(json_response['dependencies'].last['packager']).to eq('bundler')
+                end
+              end
+
+              context 'when sorted by name' do
+                let(:params) { { group_id: group.to_param, sort_by: 'name', sort: 'asc' } }
+
+                it 'returns sorted list' do
+                  subject
+
+                  expect(json_response['dependencies'].first['name']).to eq('component-1')
+                  expect(json_response['dependencies'].last['name']).to eq('component-2')
+                end
+              end
+            end
+
+            context 'with filtering params' do
+              context 'when filtered by package managers' do
+                let(:params) { { group_id: group.to_param, package_managers: ['npm'] } }
+
+                it 'returns filtered list' do
+                  subject
+
+                  expect(json_response['dependencies'].pluck('packager')).to eq(['npm'])
+                end
+              end
+            end
+          end
+
+          context 'when feature flag group_level_dependencies is disabled' do
+            before do
+              stub_feature_flags(group_level_dependencies: false)
+            end
+
+            it { is_expected.to have_gitlab_http_status(:forbidden) }
+          end
+        end
+
+        context 'when user is not allowed to access group level dependencies' do
+          it { is_expected.to have_gitlab_http_status(:forbidden) }
+        end
+      end
+
+      context 'when security dashboard feature is disabled' do
+        it { is_expected.to have_gitlab_http_status(:forbidden) }
+      end
+    end
+  end
+end
diff --git a/ee/spec/policies/group_policy_spec.rb b/ee/spec/policies/group_policy_spec.rb
index a8ee33afb6c40262..a384d0fef1f3c397 100644
--- a/ee/spec/policies/group_policy_spec.rb
+++ b/ee/spec/policies/group_policy_spec.rb
@@ -1569,7 +1569,7 @@ def stub_group_saml_config(enabled)
 
   describe 'read_group_security_dashboard & create_vulnerability_export' do
     let(:abilities) do
-      %i[read_group_security_dashboard create_vulnerability_export read_security_resource]
+      %i[read_group_security_dashboard create_vulnerability_export read_security_resource read_dependencies]
     end
 
     before do
diff --git a/ee/spec/serializers/dependency_entity_spec.rb b/ee/spec/serializers/dependency_entity_spec.rb
index 63930f9bf992aca4..a79c0d29633ce6dd 100644
--- a/ee/spec/serializers/dependency_entity_spec.rb
+++ b/ee/spec/serializers/dependency_entity_spec.rb
@@ -32,6 +32,16 @@
         it 'includes license info and vulnerabilities' do
           is_expected.to eq(dependency.except(:package_manager, :iid))
         end
+
+        context 'with relation to group instead project' do
+          before do
+            allow(request).to receive(:project).and_return(nil)
+          end
+
+          it 'does not include license and vulnerabilities info' do
+            is_expected.to eq(dependency.except(:licenses, :vulnerabilities, :package_manager, :iid))
+          end
+        end
       end
 
       context 'with reporter' do
-- 
GitLab


From 6ef490d6a377b473366b40998b371673907c1b9e Mon Sep 17 00:00:00 2001
From: Zamir Martins Filho <zfilho@gitlab.com>
Date: Wed, 17 May 2023 17:23:58 +0200
Subject: [PATCH 2/8] address backend and database reviews

---
 .../groups/dependencies_controller.rb            | 16 +++++++++-------
 ee/app/finders/sbom/dependencies_finder.rb       |  4 +---
 ee/app/models/sbom/occurrence.rb                 | 13 ++++++++++++-
 ee/app/models/sbom/source.rb                     |  4 ++++
 ee/app/serializers/dependency_entity.rb          |  8 ++------
 .../groups/dependencies_controller_spec.rb       |  7 -------
 ee/spec/serializers/dependency_entity_spec.rb    | 10 ----------
 7 files changed, 28 insertions(+), 34 deletions(-)

diff --git a/ee/app/controllers/groups/dependencies_controller.rb b/ee/app/controllers/groups/dependencies_controller.rb
index f618df28be805c43..222957e7d7a8e92b 100644
--- a/ee/app/controllers/groups/dependencies_controller.rb
+++ b/ee/app/controllers/groups/dependencies_controller.rb
@@ -13,7 +13,7 @@ def index
           render status: :ok
         end
         format.json do
-          render json: serializer.represent(dependencies)
+          render json: serialized_dependencies
         end
       end
     end
@@ -27,21 +27,23 @@ def authorize_read_dependency_list!
     end
 
     def dependency_list_params
-      params.permit(:sort_by, :sort, package_managers: [])
+      params.permit(:sort_by, :sort, :page, :per_page, package_managers: [])
     end
 
     def collect_dependencies
       return [] unless can?(current_user, :read_security_resource, group)
 
-      ::Sbom::DependenciesFinder.new(group, params: dependency_list_params).execute
+      @collect_dependencies ||= ::Sbom::DependenciesFinder.new(group, params: dependency_list_params).execute
     end
 
-    def dependencies
-      @dependencies ||= ::Gitlab::ItemsCollection.new(collect_dependencies)
+    def serialized_dependencies
+      DependencyListEntity.represent(collect_dependencies, entity_request)
     end
 
-    def serializer
-      ::DependencyListSerializer.new(group: group, user: current_user).with_pagination(request, response)
+    def entity_request
+      {
+        request: EntityRequest.new(project: nil, user: current_user)
+      }
     end
 
     def render_not_authorized
diff --git a/ee/app/finders/sbom/dependencies_finder.rb b/ee/app/finders/sbom/dependencies_finder.rb
index 2ae922fe1b20b993..6f90a41af731afcc 100644
--- a/ee/app/finders/sbom/dependencies_finder.rb
+++ b/ee/app/finders/sbom/dependencies_finder.rb
@@ -2,8 +2,6 @@
 
 module Sbom
   class DependenciesFinder
-    SBOM_OCCURRENCES_LIMIT = 100
-
     def initialize(project_or_group, params: {})
       @project_or_group = project_or_group
       @params = params
@@ -19,7 +17,7 @@ def execute
         sbom_occurrences.order_by_package_name(params[:sort])
       else
         sbom_occurrences.order_by_id
-      end.limit(SBOM_OCCURRENCES_LIMIT)
+      end.paginate(params[:per_page].to_i, params[:page].to_i)
     end
 
     private
diff --git a/ee/app/models/sbom/occurrence.rb b/ee/app/models/sbom/occurrence.rb
index e32591aec607a609..aafc901dcc9727ac 100644
--- a/ee/app/models/sbom/occurrence.rb
+++ b/ee/app/models/sbom/occurrence.rb
@@ -4,6 +4,9 @@ module Sbom
   class Occurrence < ApplicationRecord
     include EachBatch
 
+    SBOM_OCCURRENCES_MAX_LIMIT = 100
+    SBOM_OCCURRENCES_DEFAULT_LIMIT = 25
+
     belongs_to :component, optional: false
     belongs_to :component_version
     belongs_to :project, optional: false
@@ -30,7 +33,7 @@ class Occurrence < ApplicationRecord
     end
 
     scope :filter_by_package_managers, ->(package_managers) do
-      joins(:source).where("sbom_sources.source->'package_manager'->>'name' IN (?)", package_managers)
+      where(source_id: Sbom::Source.filter_by_package_managers(package_managers))
     end
 
     scope :filter_by_component_names, ->(component_names) do
@@ -46,6 +49,14 @@ def location
       }
     end
 
+    def self.paginate(per_page, page)
+      limit = [[per_page, SBOM_OCCURRENCES_DEFAULT_LIMIT].max, SBOM_OCCURRENCES_MAX_LIMIT].min
+      offset_multiplier = [page, 1].max - 1
+      offset = offset_multiplier * limit
+
+      limit(limit).offset(offset)
+    end
+
     private
 
     def input_file_blob_path
diff --git a/ee/app/models/sbom/source.rb b/ee/app/models/sbom/source.rb
index 8fdea47cc0b74377..37d357f209e4546b 100644
--- a/ee/app/models/sbom/source.rb
+++ b/ee/app/models/sbom/source.rb
@@ -10,6 +10,10 @@ class Source < ApplicationRecord
     validates :source_type, presence: true
     validates :source, presence: true, json_schema: { filename: 'sbom_source' }
 
+    scope :filter_by_package_managers, ->(package_managers) do
+      where("source->'package_manager'->>'name' IN (?)", package_managers)
+    end
+
     def packager
       source.dig('package_manager', 'name')
     end
diff --git a/ee/app/serializers/dependency_entity.rb b/ee/app/serializers/dependency_entity.rb
index d84914e048b1812a..c9c5bec238b6ec20 100644
--- a/ee/app/serializers/dependency_entity.rb
+++ b/ee/app/serializers/dependency_entity.rb
@@ -28,14 +28,10 @@ class LicenseEntity < Grape::Entity
   private
 
   def can_read_vulnerabilities?
-    return can?(request.user, :read_security_resource, request.project) if request.respond_to?(:project)
-
-    false
+    can?(request.user, :read_security_resource, request.project)
   end
 
   def can_read_licenses?
-    return can?(request.user, :read_licenses, request.project) if request.respond_to?(:project)
-
-    false
+    can?(request.user, :read_licenses, request.project)
   end
 end
diff --git a/ee/spec/controllers/groups/dependencies_controller_spec.rb b/ee/spec/controllers/groups/dependencies_controller_spec.rb
index eccc646b2eeaf627..658d59ecbb960e5b 100644
--- a/ee/spec/controllers/groups/dependencies_controller_spec.rb
+++ b/ee/spec/controllers/groups/dependencies_controller_spec.rb
@@ -121,13 +121,6 @@
               }
             end
 
-            it 'returns the paginated data' do
-              subject
-
-              expect(json_response).to eq(expected_response)
-              expect(response).to include_pagination_headers
-            end
-
             context 'with sorting params' do
               context 'when sorted by packager' do
                 let(:params) { { group_id: group.to_param, sort_by: 'packager', sort: 'desc' } }
diff --git a/ee/spec/serializers/dependency_entity_spec.rb b/ee/spec/serializers/dependency_entity_spec.rb
index a79c0d29633ce6dd..63930f9bf992aca4 100644
--- a/ee/spec/serializers/dependency_entity_spec.rb
+++ b/ee/spec/serializers/dependency_entity_spec.rb
@@ -32,16 +32,6 @@
         it 'includes license info and vulnerabilities' do
           is_expected.to eq(dependency.except(:package_manager, :iid))
         end
-
-        context 'with relation to group instead project' do
-          before do
-            allow(request).to receive(:project).and_return(nil)
-          end
-
-          it 'does not include license and vulnerabilities info' do
-            is_expected.to eq(dependency.except(:licenses, :vulnerabilities, :package_manager, :iid))
-          end
-        end
       end
 
       context 'with reporter' do
-- 
GitLab


From 8529343d9686fa136f178db2e6f4ac72923db88f Mon Sep 17 00:00:00 2001
From: Zamir Martins Filho <zfilho@gitlab.com>
Date: Mon, 22 May 2023 11:15:55 +0200
Subject: [PATCH 3/8] Move specs into requests

---
 .../groups/dependencies_controller_spec.rb    | 56 ++++++++++++++-----
 1 file changed, 42 insertions(+), 14 deletions(-)
 rename ee/spec/{controllers => requests}/groups/dependencies_controller_spec.rb (79%)

diff --git a/ee/spec/controllers/groups/dependencies_controller_spec.rb b/ee/spec/requests/groups/dependencies_controller_spec.rb
similarity index 79%
rename from ee/spec/controllers/groups/dependencies_controller_spec.rb
rename to ee/spec/requests/groups/dependencies_controller_spec.rb
index 658d59ecbb960e5b..9cb33977c914e5d0 100644
--- a/ee/spec/controllers/groups/dependencies_controller_spec.rb
+++ b/ee/spec/requests/groups/dependencies_controller_spec.rb
@@ -12,7 +12,7 @@
 
   describe 'GET index' do
     context 'with HTML format' do
-      subject { get :index, params: { group_id: group.to_param } }
+      subject { get group_dependencies_path(group_id: group.full_path) }
 
       context 'when security dashboard feature is enabled' do
         before do
@@ -21,13 +21,15 @@
         end
 
         context 'and user is allowed to access group level dependencies' do
-          render_views
-
           before do
             group.add_developer(user)
           end
 
-          it { is_expected.to have_gitlab_http_status(:ok) }
+          it 'returns http status :ok' do
+            subject
+
+            expect(response).to have_gitlab_http_status(:ok)
+          end
 
           it 'returns the correct template' do
             subject
@@ -45,22 +47,34 @@
               stub_feature_flags(group_level_dependencies: false)
             end
 
-            it { is_expected.to have_gitlab_http_status(:not_found) }
+            it 'return http status :not_found' do
+              subject
+
+              expect(response).to have_gitlab_http_status(:not_found)
+            end
           end
         end
 
         context 'when user is not allowed to access group level dependencies' do
-          it { is_expected.to have_gitlab_http_status(:not_found) }
+          it 'return http status :not_found' do
+            subject
+
+            expect(response).to have_gitlab_http_status(:not_found)
+          end
         end
       end
 
       context 'when security dashboard feature is disabled' do
-        it { is_expected.to have_gitlab_http_status(:not_found) }
+        it 'return http status :not_found' do
+          subject
+
+          expect(response).to have_gitlab_http_status(:not_found)
+        end
       end
     end
 
     context 'with JSON format' do
-      subject { get :index, params: params, format: :json }
+      subject { get group_dependencies_path(group_id: group.full_path), params: params, as: :json }
 
       let(:params) { { group_id: group.to_param } }
 
@@ -71,8 +85,6 @@
         end
 
         context 'and user is allowed to access group level dependencies' do
-          render_views
-
           let(:expected_response) do
             {
               'report' => {
@@ -86,7 +98,11 @@
             group.add_developer(user)
           end
 
-          it { is_expected.to have_gitlab_http_status(:ok) }
+          it 'returns http status :ok' do
+            subject
+
+            expect(response).to have_gitlab_http_status(:ok)
+          end
 
           it 'returns the expected data' do
             subject
@@ -163,17 +179,29 @@
               stub_feature_flags(group_level_dependencies: false)
             end
 
-            it { is_expected.to have_gitlab_http_status(:forbidden) }
+            it 'returns http status :forbidden' do
+              subject
+
+              expect(response).to have_gitlab_http_status(:forbidden)
+            end
           end
         end
 
         context 'when user is not allowed to access group level dependencies' do
-          it { is_expected.to have_gitlab_http_status(:forbidden) }
+          it 'returns http status :forbidden' do
+            subject
+
+            expect(response).to have_gitlab_http_status(:forbidden)
+          end
         end
       end
 
       context 'when security dashboard feature is disabled' do
-        it { is_expected.to have_gitlab_http_status(:forbidden) }
+        it 'returns http status :forbidden' do
+          subject
+
+          expect(response).to have_gitlab_http_status(:forbidden)
+        end
       end
     end
   end
-- 
GitLab


From 5d192a7168d4b370ab536a3d04dad2e17e57b4f4 Mon Sep 17 00:00:00 2001
From: Zamir Martins Filho <zfilho@gitlab.com>
Date: Wed, 24 May 2023 10:58:52 +0200
Subject: [PATCH 4/8] Update milestone

---
 .../feature_flags/development/group_level_dependencies.yml      | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ee/config/feature_flags/development/group_level_dependencies.yml b/ee/config/feature_flags/development/group_level_dependencies.yml
index 7e5b32d06a08ddff..ea8f90653ef72c65 100644
--- a/ee/config/feature_flags/development/group_level_dependencies.yml
+++ b/ee/config/feature_flags/development/group_level_dependencies.yml
@@ -2,7 +2,7 @@
 name: group_level_dependencies
 introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/120489
 rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/411257
-milestone: '16.0'
+milestone: '16.1'
 type: development
 group: group::threat insights
 default_enabled: false
-- 
GitLab


From cc24674a1e24496df81dfcc5822d29b8873e718c Mon Sep 17 00:00:00 2001
From: Zamir Martins Filho <zfilho@gitlab.com>
Date: Wed, 24 May 2023 16:32:15 +0200
Subject: [PATCH 5/8] Add specs

---
 .../groups/dependencies_controller.rb         |  2 --
 ee/app/models/sbom/occurrence.rb              |  3 ++-
 .../finders/sbom/dependencies_finder_spec.rb  | 22 +++++++++++++++++
 ee/spec/models/sbom/occurrence_spec.rb        | 24 +++++++++++++++++++
 ee/spec/models/sbom/source_spec.rb            | 11 +++++++++
 .../groups/dependencies_controller_spec.rb    | 15 ++++++++++++
 6 files changed, 74 insertions(+), 3 deletions(-)

diff --git a/ee/app/controllers/groups/dependencies_controller.rb b/ee/app/controllers/groups/dependencies_controller.rb
index 222957e7d7a8e92b..66517605d6dbffc4 100644
--- a/ee/app/controllers/groups/dependencies_controller.rb
+++ b/ee/app/controllers/groups/dependencies_controller.rb
@@ -31,8 +31,6 @@ def dependency_list_params
     end
 
     def collect_dependencies
-      return [] unless can?(current_user, :read_security_resource, group)
-
       @collect_dependencies ||= ::Sbom::DependenciesFinder.new(group, params: dependency_list_params).execute
     end
 
diff --git a/ee/app/models/sbom/occurrence.rb b/ee/app/models/sbom/occurrence.rb
index aafc901dcc9727ac..a6f56a9cbb91861b 100644
--- a/ee/app/models/sbom/occurrence.rb
+++ b/ee/app/models/sbom/occurrence.rb
@@ -50,7 +50,8 @@ def location
     end
 
     def self.paginate(per_page, page)
-      limit = [[per_page, SBOM_OCCURRENCES_DEFAULT_LIMIT].max, SBOM_OCCURRENCES_MAX_LIMIT].min
+      per_page = per_page < 1 ? SBOM_OCCURRENCES_DEFAULT_LIMIT : per_page
+      limit = [per_page, SBOM_OCCURRENCES_MAX_LIMIT].min
       offset_multiplier = [page, 1].max - 1
       offset = offset_multiplier * limit
 
diff --git a/ee/spec/finders/sbom/dependencies_finder_spec.rb b/ee/spec/finders/sbom/dependencies_finder_spec.rb
index 546375d7c5b01e84..57172b46db72406b 100644
--- a/ee/spec/finders/sbom/dependencies_finder_spec.rb
+++ b/ee/spec/finders/sbom/dependencies_finder_spec.rb
@@ -18,6 +18,12 @@
         expect(dependencies.first.id).to eq(occurrence_1.id)
         expect(dependencies.last.id).to eq(occurrence_3.id)
       end
+
+      it 'returns records based on the default pagination' do
+        query = dependencies.to_sql
+
+        expect(query).to include('LIMIT 25 OFFSET 0')
+      end
     end
 
     context 'with params' do
@@ -107,6 +113,22 @@
         end
       end
 
+      context 'with pagination parameters' do
+        let_it_be(:params) do
+          {
+            per_page: 2,
+            page: 1
+          }
+        end
+
+        it 'returns records based on the pagination' do
+          query = dependencies.to_sql
+
+          expect(query).to include('LIMIT 2 OFFSET 0')
+          expect(dependencies).to eq([occurrence_1, occurrence_2])
+        end
+      end
+
       context 'when params is invalid' do
         let_it_be(:params) do
           {
diff --git a/ee/spec/models/sbom/occurrence_spec.rb b/ee/spec/models/sbom/occurrence_spec.rb
index a6952d12ac5da55e..28e0fb6e06264618 100644
--- a/ee/spec/models/sbom/occurrence_spec.rb
+++ b/ee/spec/models/sbom/occurrence_spec.rb
@@ -173,4 +173,28 @@
       end
     end
   end
+
+  describe '.paginate' do
+    using RSpec::Parameterized::TableSyntax
+    let(:default_limit) { described_class::SBOM_OCCURRENCES_DEFAULT_LIMIT }
+    let(:max_limit) { described_class::SBOM_OCCURRENCES_MAX_LIMIT }
+    let(:over_limit) { described_class::SBOM_OCCURRENCES_MAX_LIMIT + 1 }
+
+    where(:per_page, :page, :limit, :offset) do
+      0                | 1 | ref(:default_limit) | 0
+      1                | 0 | 1                   | 0
+      1                | 2 | 1                   | 1
+      ref(:max_limit)  | 1 | ref(:max_limit)     | 0
+      ref(:max_limit)  | 2 | ref(:max_limit)     | ref(:max_limit)
+      ref(:over_limit) | 1 | ref(:max_limit)     | 0
+    end
+
+    with_them do
+      it 'adds limit and offset to the query' do
+        query = described_class.paginate(per_page, page).to_sql
+
+        expect(query).to include("LIMIT #{limit} OFFSET #{offset}")
+      end
+    end
+  end
 end
diff --git a/ee/spec/models/sbom/source_spec.rb b/ee/spec/models/sbom/source_spec.rb
index 17b3c7c7a318cb78..12f58df459e7c004 100644
--- a/ee/spec/models/sbom/source_spec.rb
+++ b/ee/spec/models/sbom/source_spec.rb
@@ -14,6 +14,17 @@
     it { is_expected.to validate_presence_of(:source) }
   end
 
+  describe 'scope' do
+    describe '.filter_by_package_managers' do
+      let(:source_bundler) { create(:sbom_source, packager_name: 'bundler') }
+      let(:source_yarn) { create(:sbom_source, packager_name: 'yarn') }
+
+      subject { described_class.filter_by_package_managers(%w[bundler npm]) }
+
+      it { is_expected.to eq([source_bundler]) }
+    end
+  end
+
   describe 'source validation' do
     subject { build(:sbom_source, source: source_attributes) }
 
diff --git a/ee/spec/requests/groups/dependencies_controller_spec.rb b/ee/spec/requests/groups/dependencies_controller_spec.rb
index 9cb33977c914e5d0..3b7332d43842e56c 100644
--- a/ee/spec/requests/groups/dependencies_controller_spec.rb
+++ b/ee/spec/requests/groups/dependencies_controller_spec.rb
@@ -172,6 +172,21 @@
                 end
               end
             end
+
+            context 'with pagination params' do
+              let(:params) { { group_id: group.to_param, per_page: 1, page: 1 } }
+              let(:finder_params) do
+                ActionController::Parameters.new({ per_page: 1, page: 1 }).permit(:per_page, :page)
+              end
+
+              it 'propagates paginated params' do
+                expect(Sbom::DependenciesFinder).to receive(:new).with(group, params: finder_params).and_call_original
+
+                subject
+
+                expect(json_response['dependencies'].pluck('name')).to eq([sbom_occurrence_npm.name])
+              end
+            end
           end
 
           context 'when feature flag group_level_dependencies is disabled' do
-- 
GitLab


From 882b85bd66ec9ef51b0fc8b7d7d2b5c7871a646a Mon Sep 17 00:00:00 2001
From: Danger bot
 <group_9970_bot_58e69a7d2ace73c7373ac2bc2a5cabd0@noreply.gitlab.com>
Date: Wed, 24 May 2023 14:54:04 +0000
Subject: [PATCH 6/8] add suggested changes

---
 ee/spec/finders/sbom/dependencies_finder_spec.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ee/spec/finders/sbom/dependencies_finder_spec.rb b/ee/spec/finders/sbom/dependencies_finder_spec.rb
index 57172b46db72406b..3fe88274e93a04fa 100644
--- a/ee/spec/finders/sbom/dependencies_finder_spec.rb
+++ b/ee/spec/finders/sbom/dependencies_finder_spec.rb
@@ -125,7 +125,7 @@
           query = dependencies.to_sql
 
           expect(query).to include('LIMIT 2 OFFSET 0')
-          expect(dependencies).to eq([occurrence_1, occurrence_2])
+          expect(dependencies).to match_array([occurrence_1, occurrence_2])
         end
       end
 
-- 
GitLab


From f8fd465c87f12e722536751ad9e59033f0a197be Mon Sep 17 00:00:00 2001
From: Zamir Martins Filho <zfilho@gitlab.com>
Date: Thu, 25 May 2023 15:31:02 +0200
Subject: [PATCH 7/8] Extract source into another query

---
 ee/app/models/sbom/occurrence.rb   | 2 +-
 ee/app/models/sbom/source.rb       | 4 ++++
 ee/spec/models/sbom/source_spec.rb | 9 +++++++++
 3 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/ee/app/models/sbom/occurrence.rb b/ee/app/models/sbom/occurrence.rb
index a6f56a9cbb91861b..03bb9d96c07008f0 100644
--- a/ee/app/models/sbom/occurrence.rb
+++ b/ee/app/models/sbom/occurrence.rb
@@ -33,7 +33,7 @@ class Occurrence < ApplicationRecord
     end
 
     scope :filter_by_package_managers, ->(package_managers) do
-      where(source_id: Sbom::Source.filter_by_package_managers(package_managers))
+      where(source_id: Sbom::Source.get_ids_filtered_by_package_managers(package_managers))
     end
 
     scope :filter_by_component_names, ->(component_names) do
diff --git a/ee/app/models/sbom/source.rb b/ee/app/models/sbom/source.rb
index 37d357f209e4546b..b76eb3523822dde4 100644
--- a/ee/app/models/sbom/source.rb
+++ b/ee/app/models/sbom/source.rb
@@ -14,6 +14,10 @@ class Source < ApplicationRecord
       where("source->'package_manager'->>'name' IN (?)", package_managers)
     end
 
+    def self.get_ids_filtered_by_package_managers(package_managers)
+      filter_by_package_managers(package_managers).pluck(:id)
+    end
+
     def packager
       source.dig('package_manager', 'name')
     end
diff --git a/ee/spec/models/sbom/source_spec.rb b/ee/spec/models/sbom/source_spec.rb
index 12f58df459e7c004..b12d18cc2c221b6f 100644
--- a/ee/spec/models/sbom/source_spec.rb
+++ b/ee/spec/models/sbom/source_spec.rb
@@ -25,6 +25,15 @@
     end
   end
 
+  describe '.get_ids_filtered_by_package_managers' do
+    let_it_be(:source_bundler) { create(:sbom_source, packager_name: 'bundler') }
+    let_it_be(:source_yarn) { create(:sbom_source, packager_name: 'yarn') }
+
+    subject { described_class.get_ids_filtered_by_package_managers(%w[bundler npm]) }
+
+    it { is_expected.to eq([source_bundler.id]) }
+  end
+
   describe 'source validation' do
     subject { build(:sbom_source, source: source_attributes) }
 
-- 
GitLab


From bfff15b19f73ec7294fb1ea6e01253602940fa51 Mon Sep 17 00:00:00 2001
From: Zamir Martins Filho <zfilho@gitlab.com>
Date: Mon, 5 Jun 2023 12:05:16 +0200
Subject: [PATCH 8/8] add index for sbom_occurrences

---
 ...or_sbom_occurrences_on_project_id_source_id.rb | 15 +++++++++++++++
 db/schema_migrations/20230605093005               |  1 +
 db/structure.sql                                  |  2 ++
 3 files changed, 18 insertions(+)
 create mode 100644 db/post_migrate/20230605093005_add_index_for_sbom_occurrences_on_project_id_source_id.rb
 create mode 100644 db/schema_migrations/20230605093005

diff --git a/db/post_migrate/20230605093005_add_index_for_sbom_occurrences_on_project_id_source_id.rb b/db/post_migrate/20230605093005_add_index_for_sbom_occurrences_on_project_id_source_id.rb
new file mode 100644
index 0000000000000000..868cf2763543b678
--- /dev/null
+++ b/db/post_migrate/20230605093005_add_index_for_sbom_occurrences_on_project_id_source_id.rb
@@ -0,0 +1,15 @@
+# frozen_string_literal: true
+
+class AddIndexForSbomOccurrencesOnProjectIdSourceId < Gitlab::Database::Migration[2.1]
+  disable_ddl_transaction!
+
+  INDEX_NAME = 'idx_sbom_occurrences_on_project_id_and_source_id'
+
+  def up
+    add_concurrent_index :sbom_occurrences, [:project_id, :source_id], name: INDEX_NAME
+  end
+
+  def down
+    remove_concurrent_index_by_name :sbom_occurrences, INDEX_NAME
+  end
+end
diff --git a/db/schema_migrations/20230605093005 b/db/schema_migrations/20230605093005
new file mode 100644
index 0000000000000000..273961a1f40f928e
--- /dev/null
+++ b/db/schema_migrations/20230605093005
@@ -0,0 +1 @@
+df354a2edec37d3b09bc9deebe895637f8a86c90ffb2569cefe6d791458a65ba
\ No newline at end of file
diff --git a/db/structure.sql b/db/structure.sql
index 904b6a3e79c41cf3..d642460cfe8634cb 100644
--- a/db/structure.sql
+++ b/db/structure.sql
@@ -29664,6 +29664,8 @@ CREATE INDEX idx_repository_states_on_wiki_failure_partial ON project_repository
 
 CREATE INDEX idx_repository_states_outdated_checksums ON project_repository_states USING btree (project_id) WHERE (((repository_verification_checksum IS NULL) AND (last_repository_verification_failure IS NULL)) OR ((wiki_verification_checksum IS NULL) AND (last_wiki_verification_failure IS NULL)));
 
+CREATE INDEX idx_sbom_occurrences_on_project_id_and_source_id ON sbom_occurrences USING btree (project_id, source_id);
+
 CREATE UNIQUE INDEX idx_security_scans_on_build_and_scan_type ON security_scans USING btree (build_id, scan_type);
 
 CREATE INDEX idx_security_scans_on_scan_type ON security_scans USING btree (scan_type);
-- 
GitLab