Merge branch '9074-use-only-projects-with-sec-reports-enabled-in-gsd' into 'master'

Return only projects with security reports for Group Security Dashboard

See merge request gitlab-org/gitlab-ee!15334

app/finders/group_projects_finder.rb

@@ -23,8 +23,12 @@ class GroupProjectsFinder < ProjectsFinder
   attr_reader :group, :options
 
   def initialize(group:, params: {}, options: {}, current_user: nil, project_ids_relation: nil)
-    super(params: params, current_user: current_user, project_ids_relation: project_ids_relation)
-    @group   = group
+    super(
+      params: params,
+      current_user: current_user,
+      project_ids_relation: project_ids_relation
+    )
+    @group = group
     @options = options
   end
 
@@ -84,17 +88,17 @@ class GroupProjectsFinder < ProjectsFinder
     options.fetch(:include_subgroups, false)
   end
 
-  # rubocop: disable CodeReuse/ActiveRecord
   def owned_projects
     if include_subgroups?
-      Project.where(namespace_id: group.self_and_descendants.select(:id))
+      Project.for_group_and_its_subgroups(group)
     else
       group.projects
     end
   end
-  # rubocop: enable CodeReuse/ActiveRecord
 
   def shared_projects
     group.shared_projects
   end
 end
+
+GroupProjectsFinder.prepend_if_ee('EE::GroupProjectsFinder')

app/models/ci/job_artifact.rb

@@ -87,6 +87,8 @@ module Ci
 
     scope :expired, -> (limit) { where('expire_at < ?', Time.now).limit(limit) }
 
+    scope :scoped_project, -> { where('ci_job_artifacts.project_id = projects.id') }
+
     delegate :filename, :exists?, :open, to: :file
 
     enum file_type: {

app/models/project.rb

@@ -497,6 +497,7 @@ class Project < ApplicationRecord
   # We require an alias to the project_mirror_data_table in order to use import_state in our queries
   scope :joins_import_state, -> { joins("INNER JOIN project_mirror_data import_state ON import_state.project_id = projects.id") }
   scope :for_group, -> (group) { where(group: group) }
+  scope :for_group_and_its_subgroups, ->(group) { where(namespace_id: group.self_and_descendants.select(:id)) }
 
   class << self
     # Searches for a list of projects based on the query given in `query`.

db/migrate/20190828083843_add_index_to_ci_job_artifacts_on_project_id_for_security_reports.rb

+# frozen_string_literal: true
+
+class AddIndexToCiJobArtifactsOnProjectIdForSecurityReports < ActiveRecord::Migration[5.2]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    add_concurrent_index :ci_job_artifacts,
+                         :project_id,
+                         name: "index_ci_job_artifacts_on_project_id_for_security_reports",
+                         where: "file_type IN (5, 6, 7, 8)"
+  end
+
+  def down
+    remove_concurrent_index :ci_job_artifacts,
+                            :project_id,
+                            name: "index_ci_job_artifacts_on_project_id_for_security_reports"
+  end
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 2019_08_22_181528) do
+ActiveRecord::Schema.define(version: 2019_08_28_083843) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "pg_trgm"
@@ -658,6 +658,7 @@ ActiveRecord::Schema.define(version: 2019_08_22_181528) do
     t.index ["file_store"], name: "index_ci_job_artifacts_on_file_store"
     t.index ["job_id", "file_type"], name: "index_ci_job_artifacts_on_job_id_and_file_type", unique: true
     t.index ["project_id"], name: "index_ci_job_artifacts_on_project_id"
+    t.index ["project_id"], name: "index_ci_job_artifacts_on_project_id_for_security_reports", where: "(file_type = ANY (ARRAY[5, 6, 7, 8]))"
   end
 
   create_table "ci_job_variables", force: :cascade do |t|

doc/api/groups.md

@@ -158,6 +158,7 @@ Parameters:
 | `with_shared`                 | boolean        | no       | Include projects shared to this group. Default is `true` |
 | `include_subgroups`           | boolean        | no       | Include projects in subgroups of this group. Default is `false` |
 | `with_custom_attributes`      | boolean        | no       | Include [custom attributes](custom_attributes.md) in response (admins only) |
+| `with_security_reports`       | boolean        | no       | **(ULTIMATE)** Return only projects that have security reports artifacts present in any of their builds. This means "projects with security reports enabled". Default is `false` |
 
 Example response:
 

doc/user/application_security/security_dashboard/index.md

@@ -62,6 +62,9 @@ Once you're on the dashboard, at the top you should see a series of filters for:
 - Report type
 - Project
 
+NOTE: **Note:**
+The dashboard only shows projects with [security reports](#supported-reports) enabled in a group. 
+
 ![dashboard with action buttons and metrics](img/group_security_dashboard.png)
 
 Selecting one or more filters will filter the results in this page.

ee/app/assets/javascripts/security_dashboard/store/modules/projects/actions.js

@@ -9,6 +9,7 @@ const getAllProjects = (url, page = '1', projects = []) =>
       per_page: 100,
       page,
       include_subgroups: true,
+      with_security_reports: true,
       order_by: 'path',
       sort: 'asc',
     },

ee/app/finders/ee/group_projects_finder.rb

+# frozen_string_literal: true
+
+module EE
+  # GroupProjectsFinder
+  #
+  # Extends GroupProjectsFinder
+  #
+  # Added arguments:
+  #   params:
+  #     with_security_reports: boolean
+  module GroupProjectsFinder
+    extend ::Gitlab::Utils::Override
+
+    override :filter_projects
+    def filter_projects(collection)
+      collection = super(collection)
+      collection = by_security_reports_presence(collection)
+      collection
+    end
+
+    def by_security_reports_presence(collection)
+      if params[:with_security_reports] && group.feature_available?(:security_dashboard)
+        collection.with_security_reports
+      else
+        collection
+      end
+    end
+  end
+end

ee/app/models/ee/group.rb

@@ -179,7 +179,7 @@ module EE
     end
 
     def project_ids_with_security_reports
-      all_projects.where('EXISTS (?)', ::Vulnerabilities::Occurrence.select(1).where('vulnerability_occurrences.project_id = projects.id')).pluck_primary_key
+      all_projects.with_security_reports_stored.pluck_primary_key
     end
 
     def root_ancestor_ip_restrictions

ee/app/models/ee/project.rb

@@ -102,6 +102,9 @@ module EE
       scope :requiring_code_owner_approval,
             -> { where(merge_requests_require_code_owner_approval: true) }
 
+      scope :with_security_reports_stored, -> { where('EXISTS (?)', ::Vulnerabilities::Occurrence.scoped_project.select(1)) }
+      scope :with_security_reports, -> { where('EXISTS (?)', ::Ci::JobArtifact.security_reports.scoped_project.select(1)) }
+
       delegate :shared_runners_minutes, :shared_runners_seconds, :shared_runners_seconds_last_reset,
         to: :statistics, allow_nil: true
 

ee/app/models/vulnerabilities/occurrence.rb

@@ -87,6 +87,8 @@ module Vulnerabilities
       preload(:scanner, :identifiers, project: [:namespace, :project_feature])
     end
 
+    scope :scoped_project, -> { where('vulnerability_occurrences.project_id = projects.id') }
+
     def self.for_pipelines_with_sha(pipelines)
       joins(:pipelines)
         .where(ci_pipelines: { id: pipelines })

ee/app/services/ee/groups/update_service.rb

@@ -37,7 +37,7 @@ module EE
 
         insight_project_id = params.dig(:insight_attributes, :project_id)
         if insight_project_id
-          group_projects = GroupProjectsFinder.new(group: group, current_user: current_user, options: { only_owned: true, include_subgroups: true }).execute
+          group_projects = ::GroupProjectsFinder.new(group: group, current_user: current_user, options: { only_owned: true, include_subgroups: true }).execute
           params.delete(:insight_attributes) unless group_projects.exists?(insight_project_id) # rubocop:disable CodeReuse/ActiveRecord
         end
 

ee/changelogs/unreleased/9074-use-only-projects-with-sec-reports-enabled-in-gsd.yml

+---
+title: Group Security Dashboard shows projects with security reports only
+merge_request: 15334
+author:
+type: changed

ee/lib/ee/api/helpers.rb

@@ -115,6 +115,15 @@ module EE
 
       private
 
+      override :project_finder_params_ee
+      def project_finder_params_ee
+        if params[:with_security_reports].present?
+          { with_security_reports: true }
+        else
+          {}
+        end
+      end
+
       override :send_git_archive
       def send_git_archive(repository, **kwargs)
         AuditEvents::RepositoryDownloadStartedAuditEventService.new(

ee/lib/ee/api/helpers/groups_helpers.rb

@@ -19,6 +19,10 @@ module EE
           params :optional_update_params_ee do
             optional :file_template_project_id, type: Integer, desc: 'The ID of a project to use for custom templates in this group'
           end
+
+          params :optional_projects_params_ee do
+            optional :with_security_reports, type: ::Grape::API::Boolean, default: false, desc: 'Return only projects having security report artifacts present'
+          end
         end
       end
     end

ee/spec/finders/ee/projects_finder_spec.rb

@@ -4,18 +4,18 @@ require 'spec_helper'
 
 describe ProjectsFinder do
   describe '#execute' do
-    subject { finder.execute }
-
+    let(:finder) { described_class.new(params: params) }
     let(:user) { create(:user) }
 
+    subject { finder.execute }
+
     describe 'filter by plans' do
+      let(:params) { { plans: plans } }
       let!(:gold_project) { create_project(:gold_plan) }
       let!(:gold_project2) { create_project(:gold_plan) }
       let!(:silver_project) { create_project(:silver_plan) }
       let!(:no_plan_project) { create_project(nil) }
 
-      let(:finder) { described_class.new(params: { plans: plans }) }
-
       context 'with gold plan' do
         let(:plans) { ['gold'] }
 

ee/spec/finders/group_projects_finder_spec.rb

@@ -26,4 +26,34 @@ describe GroupProjectsFinder do
       it { is_expected.to eq([shared_project_3, shared_project_2, shared_project_1, private_project, public_project]) }
     end
   end
+
+  describe "group's projects with security reports" do
+    let(:params) { { with_security_reports: true } }
+    let(:project_with_reports) { create(:project, :public, group: group) }
+    let!(:project_without_reports) { create(:project, :public, group: group) }
+
+    before do
+      create(:ee_ci_job_artifact, :sast, project: project_with_reports)
+    end
+
+    context 'when security dashboard is enabled for a group' do
+      let(:group) { create(:group, plan: :gold_plan) } # overriding group from 'GroupProjectsFinder context'
+
+      before do
+        stub_licensed_features(security_dashboard: true)
+        enable_namespace_license_check!
+
+        create(:gitlab_subscription, hosted_plan: group.plan, namespace: group)
+      end
+
+      it { is_expected.to contain_exactly(project_with_reports) }
+    end
+
+    context 'when security dashboard is disabled for a group' do
+      let(:project_with_reports) { create(:project, :public, group: group) }
+
+      # using `include` since other projects may be added to this group from different contexts
+      it { is_expected.to include(project_with_reports, project_without_reports) }
+    end
+  end
 end

ee/spec/requests/api/groups_spec.rb

@@ -220,6 +220,45 @@ describe API::Groups do
     end
   end
 
+  describe "GET /groups/:id/projects" do
+    context "when authenticated as user" do
+      let(:project_with_reports) { create(:project, :public, group: group) }
+      let!(:project_without_reports) { create(:project, :public, group: group) }
+
+      before do
+        create(:ee_ci_job_artifact, :sast, project: project_with_reports)
+      end
+
+      subject { get api("/groups/#{group.id}/projects", user), params: { with_security_reports: true } }
+
+      context 'when security dashboard is enabled for a group' do
+        let(:group) { create(:group, plan: :gold_plan) } # overriding group from parent context
+
+        before do
+          stub_licensed_features(security_dashboard: true)
+          enable_namespace_license_check!
+
+          create(:gitlab_subscription, hosted_plan: group.plan, namespace: group)
+        end
+
+        it "returns only projects with security reports" do
+          subject
+
+          expect(json_response.map { |p| p['id'] }).to contain_exactly(project_with_reports.id)
+        end
+      end
+
+      context 'when security dashboard is disabled for a group' do
+        it "returns all projects regardless of the security reports" do
+          subject
+
+          # using `include` since other projects may be added to this group from different contexts
+          expect(json_response.map { |p| p['id'] }).to include(project_with_reports.id, project_without_reports.id)
+        end
+      end
+    end
+  end
+
   def ldap_sync(group_id, user, sidekiq_testing_method)
     Sidekiq::Testing.send(sidekiq_testing_method) do
       post api("/groups/#{group_id}/ldap_sync", user)

lib/api/groups.rb

@@ -216,6 +216,7 @@ module API
 
         use :pagination
         use :with_custom_attributes
+        use :optional_projects_params
       end
       get ":id/projects" do
         projects = find_group_projects(params)

lib/api/helpers.rb

@@ -416,17 +416,7 @@ module API
     # rubocop: enable CodeReuse/ActiveRecord
 
     def project_finder_params
-      finder_params = { without_deleted: true }
-      finder_params[:owned] = true if params[:owned].present?
-      finder_params[:non_public] = true if params[:membership].present?
-      finder_params[:starred] = true if params[:starred].present?
-      finder_params[:visibility_level] = Gitlab::VisibilityLevel.level_value(params[:visibility]) if params[:visibility]
-      finder_params[:archived] = archived_param unless params[:archived].nil?
-      finder_params[:search] = params[:search] if params[:search]
-      finder_params[:user] = params.delete(:user) if params[:user]
-      finder_params[:custom_attributes] = params[:custom_attributes] if params[:custom_attributes]
-      finder_params[:min_access_level] = params[:min_access_level] if params[:min_access_level]
-      finder_params
+      project_finder_params_ce.merge(project_finder_params_ee)
     end
 
     # file helpers
@@ -461,6 +451,27 @@ module API
       end
     end
 
+    protected
+
+    def project_finder_params_ce
+      finder_params = { without_deleted: true }
+      finder_params[:owned] = true if params[:owned].present?
+      finder_params[:non_public] = true if params[:membership].present?
+      finder_params[:starred] = true if params[:starred].present?
+      finder_params[:visibility_level] = Gitlab::VisibilityLevel.level_value(params[:visibility]) if params[:visibility]
+      finder_params[:archived] = archived_param unless params[:archived].nil?
+      finder_params[:search] = params[:search] if params[:search]
+      finder_params[:user] = params.delete(:user) if params[:user]
+      finder_params[:custom_attributes] = params[:custom_attributes] if params[:custom_attributes]
+      finder_params[:min_access_level] = params[:min_access_level] if params[:min_access_level]
+      finder_params
+    end
+
+    # Overridden in EE
+    def project_finder_params_ee
+      {}
+    end
+
     private
 
     # rubocop:disable Gitlab/ModuleWithInstanceVariables

lib/api/helpers/groups_helpers.rb

@@ -28,6 +28,13 @@ module API
         use :optional_params_ce
         use :optional_params_ee
       end
+
+      params :optional_projects_params_ee do
+      end
+
+      params :optional_projects_params do
+        use :optional_projects_params_ee
+      end
     end
   end
 end

spec/support/shared_contexts/finders/group_projects_finder_shared_contexts.rb

@@ -6,9 +6,10 @@ RSpec.shared_context 'GroupProjectsFinder context' do
   let(:group) { create(:group) }
   let(:subgroup) { create(:group, parent: group) }
   let(:current_user) { create(:user) }
+  let(:params) { {} }
   let(:options) { {} }
 
-  let(:finder) { described_class.new(group: group, current_user: current_user, options: options) }
+  let(:finder) { described_class.new(group: group, current_user: current_user, params: params, options: options) }
 
   let!(:public_project) { create(:project, :public, group: group, path: '1') }
   let!(:private_project) { create(:project, :private, group: group, path: '2') }