diff --git a/ee/app/helpers/ee/projects_helper.rb b/ee/app/helpers/ee/projects_helper.rb index 199760c8ad5dc53d3a6dfa6f13800c468c6951b5..fdd8c2a523338c5cd1e71ed461d4c137cfd397f0 100644 --- a/ee/app/helpers/ee/projects_helper.rb +++ b/ee/app/helpers/ee/projects_helper.rb @@ -235,5 +235,17 @@ def project_incident_management_setting def can_import_members? super && !membership_locked? end + + def api_projects_vulnerability_findings_path(project, pipeline) + params = { id: project.id, params: { pipeline_id: pipeline.id, scope: 'dismissed' } } + + path = if ::Feature.enabled?(:first_class_vulnerabilities) + api_v4_projects_vulnerability_findings_path(params) + else + api_v4_projects_vulnerabilities_path(params) + end + + expose_path(path) + end end end diff --git a/ee/app/views/projects/pipelines/_tabs_content.html.haml b/ee/app/views/projects/pipelines/_tabs_content.html.haml index 2b1902c0b078b2cbb29c2e15100ebdd7cd1efec3..bc402388712b6e14ccde2b06ed6859f436b13bdb 100644 --- a/ee/app/views/projects/pipelines/_tabs_content.html.haml +++ b/ee/app/views/projects/pipelines/_tabs_content.html.haml @@ -16,7 +16,7 @@ empty_state_svg_path: image_path('illustrations/security-dashboard-empty-state.svg'), pipeline_id: pipeline.id, project_id: project.id, - vulnerabilities_endpoint: expose_path(api_v4_projects_vulnerabilities_path(id: project.id, params: { pipeline_id: pipeline.id, scope: 'dismissed' })), + vulnerabilities_endpoint: api_projects_vulnerability_findings_path(project, pipeline), vulnerability_feedback_help_path: help_page_path('user/application_security/index') } } - else #js-security-report-app{ data: { head_blob_path: blob_path, diff --git a/ee/lib/api/vulnerabilities.rb b/ee/lib/api/vulnerability_findings.rb similarity index 51% rename from ee/lib/api/vulnerabilities.rb rename to ee/lib/api/vulnerability_findings.rb index 12e9dcd3007252b971bf325845ad26e8b05c5435..5322d1cafce016e99005913d351cab379ba51347 100644 --- a/ee/lib/api/vulnerabilities.rb +++ b/ee/lib/api/vulnerability_findings.rb @@ -1,10 +1,34 @@ # frozen_string_literal: true module API - class Vulnerabilities < Grape::API + class VulnerabilityFindings < Grape::API include PaginationParams helpers do + params :vulnerability_findings_params do + optional :report_type, type: Array[String], desc: 'The type of report vulnerability belongs to', + values: ::Vulnerabilities::Occurrence.report_types.keys, + default: ::Vulnerabilities::Occurrence.report_types.keys + optional :scope, type: String, desc: 'Return vulnerabilities for the given scope: `dismissed` or `all`', + default: 'dismissed', values: %w[all dismissed] + optional :severity, + type: Array[String], + desc: 'Returns issues belonging to specified severity level: '\ + '`undefined`, `info`, `unknown`, `low`, `medium`, `high`, or `critical`. Defaults to all', + values: ::Vulnerabilities::Occurrence.severities.keys, + default: ::Vulnerabilities::Occurrence.severities.keys + optional :confidence, + type: Array[String], + desc: 'Returns vulnerabilities belonging to specified confidence level: '\ + '`undefined`, `ignore`, `unknown`, `experimental`, `low`, `medium`, `high`, or `confirmed`. '\ + 'Defaults to all', + values: ::Vulnerabilities::Occurrence.confidences.keys, + default: ::Vulnerabilities::Occurrence.confidences.keys + optional :pipeline_id, type: String, desc: 'The ID of the pipeline' + + use :pagination + end + def vulnerability_occurrences_by(params) pipeline = if params[:pipeline_id] params[:project].all_pipelines.find_by(id: params[:pipeline_id]) # rubocop:disable CodeReuse/ActiveRecord @@ -16,6 +40,28 @@ def vulnerability_occurrences_by(params) Security::PipelineVulnerabilitiesFinder.new(pipeline: pipeline, params: params).execute end + + def respond_with_vulnerabilities + # TODO: implement the "Get a list of project's Vulnerabilities" step + # of https://gitlab.com/gitlab-org/gitlab-ee/issues/10242#status + not_found! + end + + def respond_with_vulnerability_findings + authorize! :read_project_security_dashboard, user_project + + vulnerability_occurrences = paginate( + Kaminari.paginate_array( + vulnerability_occurrences_by(declared_params.merge(project: user_project)) + ) + ) + + Gitlab::Vulnerabilities::OccurrencesPreloader.preload_feedback!(vulnerability_occurrences) + + present vulnerability_occurrences, + with: ::Vulnerabilities::OccurrenceEntity, + request: GrapeRequestProxy.new(request, current_user) + end end before do @@ -27,40 +73,32 @@ def vulnerability_occurrences_by(params) end resource :projects, requirements: API::NAMESPACE_OR_PROJECT_REQUIREMENTS do + params do + use :vulnerability_findings_params + end desc 'Get a list of project vulnerabilities' do success ::Vulnerabilities::OccurrenceEntity end + get ':id/vulnerabilities' do + if Feature.enabled?(:first_class_vulnerabilities) + respond_with_vulnerabilities + else + respond_with_vulnerability_findings + end + end params do - optional :report_type, type: Array[String], desc: 'The type of report vulnerability belongs to', default: ::Vulnerabilities::Occurrence.report_types.keys - optional :scope, type: String, desc: 'Return vulnerabilities for the given scope: `dismissed` or `all`', default: 'dismissed', values: %w[all dismissed] - optional :severity, - type: Array[String], - desc: 'Returns issues belonging to specified severity level: `undefined`, `info`, `unknown`, `low`, `medium`, `high`, or `critical`. Defaults to all', - default: ::Vulnerabilities::Occurrence.severities.keys - optional :confidence, - type: Array[String], - desc: 'Returns vulnerabilities belonging to specified confidence level: `undefined`, `ignore`, `unknown`, `experimental`, `low`, `medium`, `high`, or `confirmed`. Defaults to all', - default: ::Vulnerabilities::Occurrence.confidences.keys - optional :pipeline_id, type: String, desc: 'The ID of the pipeline' - - use :pagination + use :vulnerability_findings_params end - - get ':id/vulnerabilities' do - authorize! :read_project_security_dashboard, user_project - - vulnerability_occurrences = paginate( - Kaminari.paginate_array( - vulnerability_occurrences_by(declared_params.merge(project: user_project)) - ) - ) - - Gitlab::Vulnerabilities::OccurrencesPreloader.preload_feedback!(vulnerability_occurrences) - - present vulnerability_occurrences, - with: ::Vulnerabilities::OccurrenceEntity, - request: GrapeRequestProxy.new(request, current_user) + desc 'Get a list of project vulnerability findings' do + success ::Vulnerabilities::OccurrenceEntity + end + get ':id/vulnerability_findings' do + if Feature.enabled?(:first_class_vulnerabilities) + respond_with_vulnerability_findings + else + not_found! + end end end end diff --git a/ee/lib/ee/api/api.rb b/ee/lib/ee/api/api.rb index ca946f97de8ca885615130b319010b70b270b192..bc7de4f1e07a31284ea933a56685baa7d2143af2 100644 --- a/ee/lib/ee/api/api.rb +++ b/ee/lib/ee/api/api.rb @@ -35,7 +35,7 @@ module API mount ::API::Scim mount ::API::ManagedLicenses mount ::API::ProjectApprovals - mount ::API::Vulnerabilities + mount ::API::VulnerabilityFindings mount ::API::MergeRequestApprovals mount ::API::MergeRequestApprovalRules mount ::API::ProjectAliases diff --git a/ee/spec/helpers/projects_helper_spec.rb b/ee/spec/helpers/projects_helper_spec.rb index 0182eccdafb7cb04a426ec8e23d223a74550127c..70d601dc97e1a01abfb776d56224c6c8a1942bf4 100644 --- a/ee/spec/helpers/projects_helper_spec.rb +++ b/ee/spec/helpers/projects_helper_spec.rb @@ -97,18 +97,24 @@ end end - describe '#project_security_dashboard_config' do + shared_context 'project with owner and pipeline' do let(:user) { create(:user) } let(:group) { create(:group).tap { |g| g.add_owner(user) } } - let(:project) { create(:project, :repository, group: group) } let(:pipeline) do create(:ee_ci_pipeline, - :with_sast_report, - user: user, - project: project, - ref: project.default_branch, - sha: project.commit.sha) + :with_sast_report, + user: user, + project: project, + ref: project.default_branch, + sha: project.commit.sha) end + let(:project) { create(:project, :repository, group: group) } + end + + describe '#project_security_dashboard_config' do + include_context 'project with owner and pipeline' + + let(:project) { create(:project, :repository, group: group) } context 'project without pipeline' do subject { helper.project_security_dashboard_config(project, nil) } @@ -128,4 +134,22 @@ end end end + + describe '#api_projects_vulnerability_findings_path' do + include_context 'project with owner and pipeline' + + subject { helper.api_projects_vulnerability_findings_path(project, pipeline) } + + context 'when Vulnerability Findings API enabled' do + it { is_expected.to include("projects/#{project.id}/vulnerability_findings") } + end + + context 'when the Vulnerability Findings API is disabled' do + before do + stub_feature_flags(first_class_vulnerabilities: false) + end + + it { is_expected.to include("projects/#{project.id}/vulnerabilities") } + end + end end diff --git a/ee/spec/requests/api/vulnerabilities_spec.rb b/ee/spec/requests/api/vulnerability_findings_spec.rb similarity index 63% rename from ee/spec/requests/api/vulnerabilities_spec.rb rename to ee/spec/requests/api/vulnerability_findings_spec.rb index 3d7703efe16778052db76b577d99a871342f5138..d10c3a712342ff198e61f21ca9c96c3fd1be41da 100644 --- a/ee/spec/requests/api/vulnerabilities_spec.rb +++ b/ee/spec/requests/api/vulnerability_findings_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe API::Vulnerabilities do +describe API::VulnerabilityFindings do set(:project) { create(:project, :public) } set(:user) { create(:user) } @@ -32,7 +32,9 @@ dismissal end - describe "GET /projects/:id/vulnerabilities" do + shared_examples 'getting list of vulnerability findings' do + let(:project_vulnerabilities_path) { "/projects/#{project.id}/#{api_resource_name}" } + context 'with an authorized user with proper permissions' do before do project.add_developer(user) @@ -41,7 +43,7 @@ it 'returns all non-dismissed vulnerabilities' do occurrence_count = (sast_report.occurrences.count + ds_report.occurrences.count - 1).to_s - get api("/projects/#{project.id}/vulnerabilities?per_page=40", user) + get api(project_vulnerabilities_path, user), params: { per_page: 40 } expect(response).to have_gitlab_http_status(200) expect(response).to include_pagination_headers @@ -54,17 +56,17 @@ it 'does not have N+1 queries' do control_count = ActiveRecord::QueryRecorder.new do - get api("/projects/#{project.id}/vulnerabilities", user), params: { report_type: 'dependency_scanning' } + get api(project_vulnerabilities_path, user), params: { report_type: 'dependency_scanning' } end.count - expect { get api("/projects/#{project.id}/vulnerabilities", user) }.not_to exceed_query_limit(control_count) + expect { get api(project_vulnerabilities_path, user) }.not_to exceed_query_limit(control_count) end describe 'filtering' do it 'returns vulnerabilities with sast report_type' do occurrence_count = (sast_report.occurrences.count - 1).to_s - get api("/projects/#{project.id}/vulnerabilities", user), params: { report_type: 'sast' } + get api(project_vulnerabilities_path, user), params: { report_type: 'sast' } expect(response).to have_gitlab_http_status(200) @@ -80,7 +82,7 @@ it 'returns vulnerabilities with dependency_scanning report_type' do occurrence_count = ds_report.occurrences.count.to_s - get api("/projects/#{project.id}/vulnerabilities", user), params: { report_type: 'dependency_scanning' } + get api(project_vulnerabilities_path, user), params: { report_type: 'dependency_scanning' } expect(response).to have_gitlab_http_status(200) @@ -93,10 +95,16 @@ expect(json_response.first['name']).to eq 'ruby-ffi DDL loading issue on Windows OS' end + it 'returns a "bad request" response for an unknown report type' do + get api(project_vulnerabilities_path, user), params: { report_type: 'blah' } + + expect(response).to have_gitlab_http_status(400) + end + it 'returns dismissed vulnerabilities with `all` scope' do occurrence_count = (sast_report.occurrences.count + ds_report.occurrences.count).to_s - get api("/projects/#{project.id}/vulnerabilities", user), params: { per_page: 40, scope: 'all' } + get api(project_vulnerabilities_path, user), params: { per_page: 40, scope: 'all' } expect(response).to have_gitlab_http_status(200) @@ -104,26 +112,38 @@ end it 'returns vulnerabilities with low severity' do - get api("/projects/#{project.id}/vulnerabilities", user), params: { per_page: 40, severity: 'low' } + get api(project_vulnerabilities_path, user), params: { per_page: 40, severity: 'low' } expect(response).to have_gitlab_http_status(200) expect(json_response.map { |v| v['severity'] }.uniq).to eq %w[low] end + it 'returns a "bad request" response for an unknown severity value' do + get api(project_vulnerabilities_path, user), params: { severity: 'foo' } + + expect(response).to have_gitlab_http_status(400) + end + it 'returns vulnerabilities with high confidence' do - get api("/projects/#{project.id}/vulnerabilities", user), params: { per_page: 40, confidence: 'high' } + get api(project_vulnerabilities_path, user), params: { per_page: 40, confidence: 'high' } expect(response).to have_gitlab_http_status(200) expect(json_response.map { |v| v['confidence'] }.uniq).to eq %w[high] end + it 'returns a "bad request" response for an unknown confidence value' do + get api(project_vulnerabilities_path, user), params: { confidence: 'qux' } + + expect(response).to have_gitlab_http_status(400) + end + context 'when pipeline_id is supplied' do it 'returns vulnerabilities from supplied pipeline' do occurrence_count = (sast_report.occurrences.count + ds_report.occurrences.count - 1).to_s - get api("/projects/#{project.id}/vulnerabilities", user), params: { per_page: 40, pipeline_id: pipeline.id } + get api(project_vulnerabilities_path, user), params: { per_page: 40, pipeline_id: pipeline.id } expect(response).to have_gitlab_http_status(200) @@ -132,7 +152,7 @@ context 'pipeline has no reports' do it 'returns empty results' do - get api("/projects/#{project.id}/vulnerabilities", user), params: { per_page: 40, pipeline_id: pipeline_without_vulnerabilities.id } + get api(project_vulnerabilities_path, user), params: { per_page: 40, pipeline_id: pipeline_without_vulnerabilities.id } expect(json_response).to eq [] end @@ -140,7 +160,7 @@ context 'with unknown pipeline' do it 'returns empty results' do - get api("/projects/#{project.id}/vulnerabilities", user), params: { per_page: 40, pipeline_id: 0 } + get api(project_vulnerabilities_path, user), params: { per_page: 40, pipeline_id: 0 } expect(json_response).to eq [] end @@ -156,7 +176,7 @@ end it 'responds with 403 Forbidden' do - get api("/projects/#{project.id}/vulnerabilities", user) + get api(project_vulnerabilities_path, user) expect(response).to have_gitlab_http_status(403) end @@ -166,7 +186,7 @@ it 'responds with 404 Not Found' do private_project = create(:project) - get api("/projects/#{private_project.id}/vulnerabilities", user) + get api("/projects/#{private_project.id}/#{api_resource_name}", user) expect(response).to have_gitlab_http_status(404) end @@ -174,10 +194,46 @@ context 'with unknown project' do it 'responds with 404 Not Found' do - get api("/projects/0/vulnerabilities", user) + get api("/projects/0/#{api_resource_name}", user) expect(response).to have_gitlab_http_status(404) end end end + + shared_examples 'not found vulnerabilities endpoint' do + it do + get api("/projects/#{project.id}/#{api_resource_name}?", user), params: { per_page: 40 } + + expect(response).to have_gitlab_http_status(404) + end + end + + describe "GET /projects/:id/vulnerabilities" do + let(:api_resource_name) { 'vulnerabilities' } + + it_behaves_like 'not found vulnerabilities endpoint' + + context 'when vulnerability findings API is disabled' do + before do + stub_feature_flags(first_class_vulnerabilities: false) + end + + it_behaves_like 'getting list of vulnerability findings' + end + end + + describe "GET /projects/:id/vulnerability_findings" do + let(:api_resource_name) { 'vulnerability_findings' } + + it_behaves_like 'getting list of vulnerability findings' + + context 'when vulnerability findings API is disabled' do + before do + stub_feature_flags(first_class_vulnerabilities: false) + end + + it_behaves_like 'not found vulnerabilities endpoint' + end + end end diff --git a/ee/spec/views/projects/pipelines/_tabs_content.html.haml_spec.rb b/ee/spec/views/projects/pipelines/_tabs_content.html.haml_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..b8e66a973c77bb4798899b266cae5a8d80a59d55 --- /dev/null +++ b/ee/spec/views/projects/pipelines/_tabs_content.html.haml_spec.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe 'projects/pipelines/_tabs_content' do + set(:user) { create(:user) } + let(:pipeline) { create(:ci_pipeline).present(current_user: user) } + let(:locals) { { pipeline: pipeline, project: pipeline.project } } + + before do + allow(pipeline).to receive(:expose_security_dashboard?).and_return(true) + end + + shared_examples 'rendering the appropriate API endpoint path' do + it do + render partial: 'projects/pipelines/tabs_content', locals: locals + + expect(rendered).to include expected_api_path + end + end + + context 'when Vulnerability Findings API enabled' do + it_behaves_like 'rendering the appropriate API endpoint path' do + let(:expected_api_path) { "projects/#{pipeline.project_id}/vulnerability_findings" } + end + end + + context 'when the Vulnerability Findings API is disabled' do + before do + stub_feature_flags(first_class_vulnerabilities: false) + end + + it_behaves_like 'rendering the appropriate API endpoint path' do + let(:expected_api_path) { "projects/#{pipeline.project_id}/vulnerabilities" } + end + end +end