diff --git a/changelogs/unreleased/10242-move-old-vulns-api-to-vuln-findings.yml b/changelogs/unreleased/10242-move-old-vulns-api-to-vuln-findings.yml new file mode 100644 index 0000000000000000000000000000000000000000..08e22948adddcb8a030e17453fd7c2102859670e --- /dev/null +++ b/changelogs/unreleased/10242-move-old-vulns-api-to-vuln-findings.yml @@ -0,0 +1,5 @@ +--- +title: Rename Vulnerabilities API to Vulnerability Findings API +merge_request: 19029 +author: +type: changed diff --git a/doc/api/api_resources.md b/doc/api/api_resources.md index eeb4bf6c5f23e881393c386a457d9922cc3db54b..c2713f54c47a12deb33c3e70feeddd704c6d0f26 100644 --- a/doc/api/api_resources.md +++ b/doc/api/api_resources.md @@ -67,8 +67,9 @@ The following API resources are available in the project context: | [Search](search.md) | `/projects/:id/search` (also available for groups and standalone) | | [Services](services.md) | `/projects/:id/services` | | [Tags](tags.md) | `/projects/:id/repository/tags` | +| [Visual Review discussions](visual_review_discussions.md) **(STARTER**) | `/projects/:id/merge_requests/:merge_request_id/visual_review_discussions` | | [Vulnerabilities](vulnerabilities.md) **(ULTIMATE)** | `/projects/:id/vulnerabilities` | -| [Visual Review discussions](visual_review_discussions.md) **(STARTER**) | `/projects/:id/merge_requests/:merge_request_id/visual_review_discussions` | +| [Vulnerability Findings](vulnerability_findings.md) **(ULTIMATE)** | `/projects/:id/vulnerability_findings` | | [Wikis](wikis.md) | `/projects/:id/wikis` | ## Group resources diff --git a/doc/api/vulnerabilities.md b/doc/api/vulnerabilities.md index eaa4c13de55783560a138f9e8a5e49bb39439d4d..21b3a6f4c96532a73275b831094a09f141eaf77c 100644 --- a/doc/api/vulnerabilities.md +++ b/doc/api/vulnerabilities.md @@ -1,115 +1,3 @@ # Vulnerabilities API **(ULTIMATE)** -Every API call to vulnerabilities must be authenticated. - -If a user is not a member of a project and the project is private, a `GET` -request on that project will result in a `404` status code. - -CAUTION: **Caution:** -This API is in an alpha stage and considered unstable. -The response payload may be subject to change or breakage -across GitLab releases. - -## Vulnerabilities pagination - -By default, `GET` requests return 20 results at a time because the API results -are paginated. - -Read more on [pagination](README.md#pagination). - -## List project vulnerabilities - -List all of a project's vulnerabilities. - -``` -GET /projects/:id/vulnerabilities -GET /projects/:id/vulnerabilities?report_type=sast -GET /projects/:id/vulnerabilities?report_type=container_scanning -GET /projects/:id/vulnerabilities?report_type=sast,dast -GET /projects/:id/vulnerabilities?scope=all -GET /projects/:id/vulnerabilities?scope=dismissed -GET /projects/:id/vulnerabilities?severity=high -GET /projects/:id/vulnerabilities?confidence=unknown,experimental -GET /projects/:id/vulnerabilities?pipeline_id=42 -``` - -| Attribute | Type | Required | Description | -| ------------- | -------------- | -------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------| -| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user. | -| `report_type` | string array | no | Returns vulnerabilities belonging to specified report type. Valid values: `sast`, `dast`, `dependency_scanning`, or `container_scanning`. | -| `scope` | string | no | Returns vulnerabilities for the given scope: `all` or `dismissed`. Defaults to `dismissed` | -| `severity` | string array | no | Returns vulnerabilities belonging to specified severity level: `undefined`, `info`, `unknown`, `low`, `medium`, `high`, or `critical`. Defaults to all' | -| `confidence` | string array | no | Returns vulnerabilities belonging to specified confidence level: `undefined`, `ignore`, `unknown`, `experimental`, `low`, `medium`, `high`, or `confirmed`. Defaults to all | -| `pipeline_id` | integer/string | no | Returns vulnerabilities belonging to specified pipeline. | - -```bash -curl --header "PRIVATE-TOKEN: " https://gitlab.example.com/api/v4/projects/4/vulnerabilities -``` - -Example response: - -```json -[ - { - "id": null, - "report_type": "dependency_scanning", - "name": "Authentication bypass via incorrect DOM traversal and canonicalization in saml2-js", - "severity": "unknown", - "confidence": "undefined", - "scanner": { - "external_id": "gemnasium", - "name": "Gemnasium" - }, - "identifiers": [ - { - "external_type": "gemnasium", - "external_id": "9952e574-7b5b-46fa-a270-aeb694198a98", - "name": "Gemnasium-9952e574-7b5b-46fa-a270-aeb694198a98", - "url": "https://deps.sec.gitlab.com/packages/npm/saml2-js/versions/1.5.0/advisories" - }, - { - "external_type": "cve", - "external_id": "CVE-2017-11429", - "name": "CVE-2017-11429", - "url": "https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2017-11429" - } - ], - "project_fingerprint": "fa6f5b6c5d240b834ac5e901dc69f9484cef89ec", - "create_vulnerability_feedback_issue_path": "/tests/yarn-remediation-test/vulnerability_feedback", - "create_vulnerability_feedback_merge_request_path": "/tests/yarn-remediation-test/vulnerability_feedback", - "create_vulnerability_feedback_dismissal_path": "/tests/yarn-remediation-test/vulnerability_feedback", - "project": { - "id": 31, - "name": "yarn-remediation-test", - "full_path": "/tests/yarn-remediation-test", - "full_name": "tests / yarn-remediation-test" - }, - "dismissal_feedback": null, - "issue_feedback": null, - "merge_request_feedback": null, - "description": "Some XML DOM traversal and canonicalization APIs may be inconsistent in handling of comments within XML nodes. Incorrect use of these APIs by some SAML libraries results in incorrect parsing of the inner text of XML nodes such that any inner text after the comment is lost prior to cryptographically signing the SAML message. Text after the comment therefore has no impact on the signature on the SAML message.\r\n\r\nA remote attacker can modify SAML content for a SAML service provider without invalidating the cryptographic signature, which may allow attackers to bypass primary authentication for the affected SAML service provider.", - "links": [ - { - "url": "https://github.com/Clever/saml2/commit/3546cb61fd541f219abda364c5b919633609ef3d#diff-af730f9f738de1c9ad87596df3f6de84R279" - }, - { - "url": "https://www.kb.cert.org/vuls/id/475445" - }, - { - "url": "https://github.com/Clever/saml2/issues/127" - } - ], - "location": { - "file": "yarn.lock", - "dependency": { - "package": { - "name": "saml2-js" - }, - "version": "1.5.0" - } - }, - "solution": "Upgrade to fixed version.\r\n", - "blob_path": "/tests/yarn-remediation-test/blob/cc6c4a0778460455ae5d16ca7025ca9ca1ca75ac/yarn.lock" - } -] -``` +This document was moved to [another location](vulnerability_findings.md). diff --git a/doc/api/vulnerability_findings.md b/doc/api/vulnerability_findings.md new file mode 100644 index 0000000000000000000000000000000000000000..3d3f12aeef53531490571078ca5ed28ffaff473b --- /dev/null +++ b/doc/api/vulnerability_findings.md @@ -0,0 +1,128 @@ +# Vulnerability Findings API **(ULTIMATE)** + +> [Introduced](https://gitlab.com/gitlab-org/gitlab/merge_requests/19029) in GitLab Ultimate 12.5. + +NOTE: **Note:** +This API resource is renamed from Vulnerabilities to Vulnerability Findings because the Vulnerabilities are reserved +for serving the upcoming [Standalone Vulnerability objects](https://gitlab.com/gitlab-org/gitlab/issues/13561). +To fix any broken integrations with the former Vulnerabilities API, change the `vulnerabilities` URL part to be +`vulnerability_findings`. + +Every API call to vulnerability findings must be [authenticated](README.md#authentication). + +Vulnerability findings are project-bound entities. If a user is not +a member of a project and the project is private, a request on +that project will result in a `404` status code. + +If a user is able to access the project but does not have permission to +[use the Project Security Dashboard](../user/permissions.md#project-members-permissions), +any request for vulnerability findings of this project will result in a `403` status code. + +CAUTION: **Caution:** +This API is in an alpha stage and considered unstable. +The response payload may be subject to change or breakage +across GitLab releases. + +## Vulnerability findings pagination + +By default, `GET` requests return 20 results at a time because the API results +are paginated. + +Read more on [pagination](README.md#pagination). + +## List project vulnerability findings + +List all of a project's vulnerability findings. + +``` +GET /projects/:id/vulnerability_findings +GET /projects/:id/vulnerability_findings?report_type=sast +GET /projects/:id/vulnerability_findings?report_type=container_scanning +GET /projects/:id/vulnerability_findings?report_type=sast,dast +GET /projects/:id/vulnerability_findings?scope=all +GET /projects/:id/vulnerability_findings?scope=dismissed +GET /projects/:id/vulnerability_findings?severity=high +GET /projects/:id/vulnerability_findings?confidence=unknown,experimental +GET /projects/:id/vulnerability_findings?pipeline_id=42 +``` + +| Attribute | Type | Required | Description | +| ------------- | -------------- | -------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| +| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) which the authenticated user is a member of. | +| `report_type` | string array | no | Returns vulnerability findings belonging to specified report type. Valid values: `sast`, `dast`, `dependency_scanning`, or `container_scanning`. Defaults to all. | +| `scope` | string | no | Returns vulnerability findings for the given scope: `all` or `dismissed`. Defaults to `dismissed`. | +| `severity` | string array | no | Returns vulnerability findings belonging to specified severity level: `undefined`, `info`, `unknown`, `low`, `medium`, `high`, or `critical`. Defaults to all. | +| `confidence` | string array | no | Returns vulnerability findings belonging to specified confidence level: `undefined`, `ignore`, `unknown`, `experimental`, `low`, `medium`, `high`, or `confirmed`. Defaults to all. | +| `pipeline_id` | integer/string | no | Returns vulnerability findings belonging to specified pipeline. | + +```bash +curl --header "PRIVATE-TOKEN: " https://gitlab.example.com/api/v4/projects/4/vulnerability_findings +``` + +Example response: + +```json +[ + { + "id": null, + "report_type": "dependency_scanning", + "name": "Authentication bypass via incorrect DOM traversal and canonicalization in saml2-js", + "severity": "unknown", + "confidence": "undefined", + "scanner": { + "external_id": "gemnasium", + "name": "Gemnasium" + }, + "identifiers": [ + { + "external_type": "gemnasium", + "external_id": "9952e574-7b5b-46fa-a270-aeb694198a98", + "name": "Gemnasium-9952e574-7b5b-46fa-a270-aeb694198a98", + "url": "https://deps.sec.gitlab.com/packages/npm/saml2-js/versions/1.5.0/advisories" + }, + { + "external_type": "cve", + "external_id": "CVE-2017-11429", + "name": "CVE-2017-11429", + "url": "https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2017-11429" + } + ], + "project_fingerprint": "fa6f5b6c5d240b834ac5e901dc69f9484cef89ec", + "create_vulnerability_feedback_issue_path": "/tests/yarn-remediation-test/vulnerability_feedback", + "create_vulnerability_feedback_merge_request_path": "/tests/yarn-remediation-test/vulnerability_feedback", + "create_vulnerability_feedback_dismissal_path": "/tests/yarn-remediation-test/vulnerability_feedback", + "project": { + "id": 31, + "name": "yarn-remediation-test", + "full_path": "/tests/yarn-remediation-test", + "full_name": "tests / yarn-remediation-test" + }, + "dismissal_feedback": null, + "issue_feedback": null, + "merge_request_feedback": null, + "description": "Some XML DOM traversal and canonicalization APIs may be inconsistent in handling of comments within XML nodes. Incorrect use of these APIs by some SAML libraries results in incorrect parsing of the inner text of XML nodes such that any inner text after the comment is lost prior to cryptographically signing the SAML message. Text after the comment therefore has no impact on the signature on the SAML message.\r\n\r\nA remote attacker can modify SAML content for a SAML service provider without invalidating the cryptographic signature, which may allow attackers to bypass primary authentication for the affected SAML service provider.", + "links": [ + { + "url": "https://github.com/Clever/saml2/commit/3546cb61fd541f219abda364c5b919633609ef3d#diff-af730f9f738de1c9ad87596df3f6de84R279" + }, + { + "url": "https://www.kb.cert.org/vuls/id/475445" + }, + { + "url": "https://github.com/Clever/saml2/issues/127" + } + ], + "location": { + "file": "yarn.lock", + "dependency": { + "package": { + "name": "saml2-js" + }, + "version": "1.5.0" + } + }, + "solution": "Upgrade to fixed version.\r\n", + "blob_path": "/tests/yarn-remediation-test/blob/cc6c4a0778460455ae5d16ca7025ca9ca1ca75ac/yarn.lock" + } +] +``` diff --git a/ee/app/helpers/ee/projects_helper.rb b/ee/app/helpers/ee/projects_helper.rb index 34007d494a5d78dd5a255badf5ee9b46e78554f8..02efa306b2c05ba0b116a857d3d4fcf76f3bd075 100644 --- a/ee/app/helpers/ee/projects_helper.rb +++ b/ee/app/helpers/ee/projects_helper.rb @@ -260,17 +260,5 @@ 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 a9b50e5fde20e36b865864e287317896224eab58..aa4eb57f2a40db93dc78e68801785788c0fb1004 100644 --- a/ee/app/views/projects/pipelines/_tabs_content.html.haml +++ b/ee/app/views/projects/pipelines/_tabs_content.html.haml @@ -8,6 +8,7 @@ - blob_path = project_blob_path(project, pipeline.sha) - license_management_settings_path = can?(current_user, :admin_software_license_policy, project) ? license_management_settings_path(project) : nil - licenses_api_path = licenses_project_pipeline_path(project, pipeline) if project.feature_available?(:license_management) +- vulnerabilities_endpoint_path = expose_path(api_v4_projects_vulnerability_findings_path(id: project.id, params: { pipeline_id: pipeline.id, scope: 'dismissed' })) - if pipeline.expose_security_dashboard? #js-tab-security.build-security.tab-pane @@ -16,7 +17,7 @@ empty_state_svg_path: image_path('illustrations/security-dashboard-empty-state.svg'), pipeline_id: pipeline.id, project_id: project.id, - vulnerabilities_endpoint: api_projects_vulnerability_findings_path(project, pipeline), + vulnerabilities_endpoint: vulnerabilities_endpoint_path, 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/helpers/vulnerability_findings_helpers.rb b/ee/lib/api/helpers/vulnerability_findings_helpers.rb deleted file mode 100644 index 33c38c470fceae90ce746e1801d0990625793007..0000000000000000000000000000000000000000 --- a/ee/lib/api/helpers/vulnerability_findings_helpers.rb +++ /dev/null @@ -1,62 +0,0 @@ -# frozen_string_literal: true - -module API - module Helpers - module VulnerabilityFindingsHelpers - extend Grape::API::Helpers - - 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 vulnerabilities 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 - - # TODO: rename to vulnerability_findings_by https://gitlab.com/gitlab-org/gitlab/issues/32963 - def vulnerability_occurrences_by(params) - pipeline = if params[:pipeline_id] - user_project.all_pipelines.find_by(id: params[:pipeline_id]) # rubocop:disable CodeReuse/ActiveRecord - else - user_project.latest_pipeline_with_security_reports - end - - return [] unless pipeline - - Security::PipelineVulnerabilitiesFinder.new(pipeline: pipeline, params: params).execute - end - - def respond_with_vulnerability_findings - authorize! :read_project_security_dashboard, user_project - - vulnerability_occurrences = paginate( - Kaminari.paginate_array( - vulnerability_occurrences_by(declared_params) - ) - ) - - Gitlab::Vulnerabilities::OccurrencesPreloader.preload_feedback!(vulnerability_occurrences) - - present vulnerability_occurrences, - with: ::Vulnerabilities::OccurrenceEntity, - request: GrapeRequestProxy.new(request, current_user) - end - end - end -end diff --git a/ee/lib/api/vulnerabilities.rb b/ee/lib/api/vulnerabilities.rb index ec381915c21f5bb4635d675b3bb882857032dcd3..1b595046c22cb11ae673308a100923243bcf6419 100644 --- a/ee/lib/api/vulnerabilities.rb +++ b/ee/lib/api/vulnerabilities.rb @@ -4,8 +4,6 @@ module API class Vulnerabilities < Grape::API include PaginationParams - helpers ::API::Helpers::VulnerabilityFindingsHelpers - helpers do def vulnerabilities_by(project) Security::VulnerabilitiesFinder.new(project).execute @@ -31,6 +29,8 @@ def render_vulnerability(vulnerability) end before do + not_found! unless Feature.enabled?(:first_class_vulnerabilities) + authenticate! end @@ -42,30 +42,22 @@ def render_vulnerability(vulnerability) success VulnerabilityEntity end post ':id/resolve' do - if Feature.enabled?(:first_class_vulnerabilities) - vulnerability = find_and_authorize_vulnerability!(:resolve_vulnerability) - break not_modified! if vulnerability.closed? + vulnerability = find_and_authorize_vulnerability!(:resolve_vulnerability) + break not_modified! if vulnerability.closed? - vulnerability = ::Vulnerabilities::ResolveService.new(current_user, vulnerability).execute - render_vulnerability(vulnerability) - else - not_found! - end + vulnerability = ::Vulnerabilities::ResolveService.new(current_user, vulnerability).execute + render_vulnerability(vulnerability) end desc 'Dismiss a vulnerability' do success VulnerabilityEntity end post ':id/dismiss' do - if Feature.enabled?(:first_class_vulnerabilities) - vulnerability = find_and_authorize_vulnerability!(:dismiss_vulnerability) - break not_modified! if vulnerability.closed? + vulnerability = find_and_authorize_vulnerability!(:dismiss_vulnerability) + break not_modified! if vulnerability.closed? - vulnerability = ::Vulnerabilities::DismissService.new(current_user, vulnerability).execute - render_vulnerability(vulnerability) - else - not_found! - end + vulnerability = ::Vulnerabilities::DismissService.new(current_user, vulnerability).execute + render_vulnerability(vulnerability) end end @@ -73,28 +65,17 @@ def render_vulnerability(vulnerability) requires :id, type: String, desc: 'The ID of a project' end resource :projects, requirements: API::NAMESPACE_OR_PROJECT_REQUIREMENTS do - params do - # These params have no effect for Vulnerabilities API but are required to support falling back to - # responding with Vulnerability Findings when :first_class_vulnerabilities feature is disabled. - # TODO: replace :vulnerability_findings_params with just :pagination when feature flag is removed - # https://gitlab.com/gitlab-org/gitlab/issues/33488 - use :vulnerability_findings_params - end desc 'Get a list of project vulnerabilities' do success VulnerabilityEntity end get ':id/vulnerabilities' do - if Feature.enabled?(:first_class_vulnerabilities) - authorize! :read_project_security_dashboard, user_project + authorize! :read_project_security_dashboard, user_project - vulnerabilities = paginate( - vulnerabilities_by(user_project) - ) + vulnerabilities = paginate( + vulnerabilities_by(user_project) + ) - present vulnerabilities, with: VulnerabilityEntity - else - respond_with_vulnerability_findings - end + present vulnerabilities, with: VulnerabilityEntity end end end diff --git a/ee/lib/api/vulnerability_findings.rb b/ee/lib/api/vulnerability_findings.rb index 2dbcd0eb3091d8b1a47c86bb4811f9f8b892a761..ea7695329acadc845ab400f773ce4f5cb218b5f9 100644 --- a/ee/lib/api/vulnerability_findings.rb +++ b/ee/lib/api/vulnerability_findings.rb @@ -4,7 +4,19 @@ module API class VulnerabilityFindings < Grape::API include PaginationParams - helpers ::API::Helpers::VulnerabilityFindingsHelpers + helpers do + def vulnerability_occurrences_by(params) + pipeline = if params[:pipeline_id] + user_project.all_pipelines.find_by(id: params[:pipeline_id]) # rubocop:disable CodeReuse/ActiveRecord + else + user_project.latest_pipeline_with_security_reports + end + + return [] unless pipeline + + Security::PipelineVulnerabilitiesFinder.new(pipeline: pipeline, params: params).execute + end + end before do authenticate! @@ -13,20 +25,47 @@ class VulnerabilityFindings < Grape::API params do requires :id, type: String, desc: 'The ID of a project' end - resource :projects, requirements: API::NAMESPACE_OR_PROJECT_REQUIREMENTS do params do - use :vulnerability_findings_params + 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 vulnerabilities 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 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 + authorize! :read_project_security_dashboard, user_project + + vulnerability_occurrences = paginate( + Kaminari.paginate_array( + vulnerability_occurrences_by(declared_params) + ) + ) + + Gitlab::Vulnerabilities::OccurrencesPreloader.preload_feedback!(vulnerability_occurrences) + + present vulnerability_occurrences, + with: ::Vulnerabilities::OccurrenceEntity, + request: GrapeRequestProxy.new(request, current_user) end end end diff --git a/ee/spec/helpers/projects_helper_spec.rb b/ee/spec/helpers/projects_helper_spec.rb index 113fa10bd69b254ff26a79a2036e5eb0f5450061..3f608cfb9eff57b1f67c40037c9a9ac2593fba00 100644 --- a/ee/spec/helpers/projects_helper_spec.rb +++ b/ee/spec/helpers/projects_helper_spec.rb @@ -155,22 +155,4 @@ 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/vulnerabilities_spec.rb index 0a4e82d77851ee56d2825696a8cf38b4e7f4757d..948db0928ea6865046886c39d88393feaa0575e2 100644 --- a/ee/spec/requests/api/vulnerabilities_spec.rb +++ b/ee/spec/requests/api/vulnerabilities_spec.rb @@ -10,16 +10,44 @@ let_it_be(:project) { create(:project, :with_vulnerabilities) } let_it_be(:user) { create(:user) } - describe "GET /projects/:id/vulnerabilities" do + shared_examples 'forbids actions on vulnerability in case of disabled features' do + context 'when "first-class vulnerabilities" feature is disabled' do + before do + stub_feature_flags(first_class_vulnerabilities: false) + end + + it 'responds with "not found"' do + subject + + expect(response).to have_gitlab_http_status(404) + end + end + + context 'when security dashboard feature is not available' do + before do + stub_licensed_features(security_dashboard: false) + end + + it 'responds with 403 Forbidden' do + subject + + expect(response).to have_gitlab_http_status(403) + end + end + end + + describe 'GET /projects/:id/vulnerabilities' do let(:project_vulnerabilities_path) { "/projects/#{project.id}/vulnerabilities" } + subject { get api(project_vulnerabilities_path, user) } + context 'with an authorized user with proper permissions' do before do project.add_developer(user) end it 'returns all vulnerabilities of a project' do - get api(project_vulnerabilities_path, user) + subject expect(response).to have_gitlab_http_status(200) expect(response).to include_pagination_headers @@ -27,23 +55,22 @@ expect(response.headers['X-Total']).to eq project.vulnerabilities.count.to_s end - it 'paginates the vulnerabilities according to the pagination params' do - get api("#{project_vulnerabilities_path}?page=2&per_page=1", user) + context 'with pagination' do + let(:project_vulnerabilities_path) { "#{super()}?page=2&per_page=1" } - expect(response).to have_gitlab_http_status(200) - expect(json_response.map { |v| v['id'] }).to contain_exactly(project.vulnerabilities.second.id) - end + it 'paginates the vulnerabilities according to the pagination params' do + subject - context 'when "first-class vulnerabilities" feature is disabled' do - before do - stub_feature_flags(first_class_vulnerabilities: false) + expect(response).to have_gitlab_http_status(200) + expect(json_response.map { |v| v['id'] }).to contain_exactly(project.vulnerabilities.second.id) end - - it_behaves_like 'getting list of vulnerability findings' end + + it_behaves_like 'forbids actions on vulnerability in case of disabled features' end - it_behaves_like 'forbids access to project vulnerabilities endpoint in expected cases' + it_behaves_like 'responds with "not found" when there is no access to the project' + it_behaves_like 'prevents working with vulnerabilities in case of insufficient privileges' end describe "POST /vulnerabilities:id/dismiss" do @@ -103,18 +130,6 @@ end end - context 'and when security dashboard feature is not available' do - before do - stub_licensed_features(security_dashboard: false) - end - - it 'responds with 403 Forbidden' do - subject - - expect(response).to have_gitlab_http_status(403) - end - end - context 'if a vulnerability is already dismissed' do let(:vulnerability) { create(:vulnerability, :closed, project: project) } @@ -124,31 +139,11 @@ expect(response).to have_gitlab_http_status(304) end end - end - - context 'when user does not have permissions to create a dismissal feedback' do - before do - project.add_reporter(user) - end - it 'responds with 403 Forbidden' do - subject - - expect(response).to have_gitlab_http_status(403) - end + it_behaves_like 'forbids actions on vulnerability in case of disabled features' end - context 'when first-class vulnerabilities feature is disabled' do - before do - stub_feature_flags(first_class_vulnerabilities: false) - end - - it 'responds with 404 Not Found' do - subject - - expect(response).to have_gitlab_http_status(404) - end - end + it_behaves_like 'prevents working with vulnerabilities in case of insufficient privileges' end describe "POST /vulnerabilities:id/resolve" do @@ -188,41 +183,9 @@ end end - context 'and when security dashboard feature is not available' do - before do - stub_licensed_features(security_dashboard: false) - end - - it 'responds with 403 Forbidden' do - subject - - expect(response).to have_gitlab_http_status(403) - end - end - end - - context 'when user does not have permissions to resolve a vulnerability' do - before do - project.add_reporter(user) - end - - it 'responds with 403 Forbidden' do - subject - - expect(response).to have_gitlab_http_status(403) - end + it_behaves_like 'forbids actions on vulnerability in case of disabled features' end - context 'when first-class vulnerabilities feature is disabled' do - before do - stub_feature_flags(first_class_vulnerabilities: false) - end - - it 'responds with 404 Not Found' do - subject - - expect(response).to have_gitlab_http_status(404) - end - end + it_behaves_like 'prevents working with vulnerabilities in case of insufficient privileges' end end diff --git a/ee/spec/requests/api/vulnerability_findings_spec.rb b/ee/spec/requests/api/vulnerability_findings_spec.rb index c4d138519e361213849d094a60ff910698b3fb10..f7b3c6f4cd225650764eb9d581d4ce5d52bd1d9b 100644 --- a/ee/spec/requests/api/vulnerability_findings_spec.rb +++ b/ee/spec/requests/api/vulnerability_findings_spec.rb @@ -6,28 +6,183 @@ let_it_be(:project) { create(:project, :public) } let_it_be(:user) { create(:user) } - describe "GET /projects/:id/vulnerability_findings" do - let(:project_vulnerabilities_path) { "/projects/#{project.id}/vulnerability_findings" } + describe 'GET /projects/:id/vulnerability_findings' do + let(:project_vulnerability_findings_path) { "/projects/#{project.id}/vulnerability_findings" } - it_behaves_like 'getting list of vulnerability findings' - it_behaves_like 'forbids access to project vulnerabilities endpoint in expected cases' + before do + stub_licensed_features(security_dashboard: true, sast: true, dependency_scanning: true, container_scanning: true) + + create(:ee_ci_job_artifact, :dependency_scanning, job: build_ds, project: project) + create(:ee_ci_job_artifact, :sast, job: build_sast, project: project) + dismissal + end + + let(:pipeline) { create(:ci_empty_pipeline, status: :created, project: project) } + let(:pipeline_without_vulnerabilities) { create(:ci_pipeline_without_jobs, status: :created, project: project) } + + let(:build_ds) { create(:ci_build, :success, name: 'ds_job', pipeline: pipeline, project: project) } + let(:build_sast) { create(:ci_build, :success, name: 'sast_job', pipeline: pipeline, project: project) } + + let(:ds_report) { pipeline.security_reports.reports['dependency_scanning'] } + let(:sast_report) { pipeline.security_reports.reports['sast'] } + + let(:dismissal) do + create(:vulnerability_feedback, :dismissal, :sast, + project: project, + pipeline: pipeline, + project_fingerprint: sast_report.occurrences.first.project_fingerprint, + vulnerability_data: sast_report.occurrences.first.raw_metadata + ) + end context 'with an authorized user with proper permissions' do before do project.add_developer(user) end - context 'when "first-class vulnerabilities" feature is disabled' do - before do - stub_feature_flags(first_class_vulnerabilities: false) + # Because fixture reports that power :ee_ci_job_artifact factory contain long report lists, + # we need to make sure that all occurrences for both SAST and Dependency Scanning are included in the response. + # That's why the page size is 40. + let(:pagination) { { per_page: 40 } } + + it 'returns all non-dismissed vulnerabilities' do + # all occurrences except one that was dismissed + occurrence_count = (sast_report.occurrences.count + ds_report.occurrences.count - 1).to_s + + get api(project_vulnerability_findings_path, user), params: pagination + + expect(response).to have_gitlab_http_status(200) + expect(response).to include_pagination_headers + expect(response).to match_response_schema('vulnerabilities/occurrence_list', dir: 'ee') + + expect(response.headers['X-Total']).to eq occurrence_count + + expect(json_response.map { |v| v['report_type'] }.uniq).to match_array %w[dependency_scanning sast] + end + + it 'does not have N+1 queries' do + control_count = ActiveRecord::QueryRecorder.new do + get api(project_vulnerability_findings_path, user), params: { report_type: 'dependency_scanning' } + end.count + + # Threshold is required for the extra query performed in Security::PipelineVulnerabilitiesFinder to load + # the Vulnerabilities providing computed states for the associated Vulnerability::Occurrences + expect { get api(project_vulnerability_findings_path, user) }.not_to exceed_query_limit(control_count).with_threshold(1) + end + + describe 'filtering' do + it 'returns vulnerabilities with sast report_type' do + occurrence_count = (sast_report.occurrences.count - 1).to_s # all SAST occurrences except one that was dismissed + + get api(project_vulnerability_findings_path, user), params: { report_type: 'sast' } + + expect(response).to have_gitlab_http_status(200) + + expect(response.headers['X-Total']).to eq occurrence_count + + expect(json_response.map { |v| v['report_type'] }.uniq).to match_array %w[sast] + + # occurrences are implicitly sorted by Security::PipelineVulnerabilitiesFinder and + # Security::MergeReportsService so their order differs from what is present in fixture file + expect(json_response.first['name']).to eq 'ECB mode is insecure' end - it 'responds with "not found"' do - get api(project_vulnerabilities_path, user) + it 'returns vulnerabilities with dependency_scanning report_type' do + occurrence_count = ds_report.occurrences.count.to_s + + get api(project_vulnerability_findings_path, user), params: { report_type: 'dependency_scanning' } + + expect(response).to have_gitlab_http_status(200) - expect(response).to have_gitlab_http_status(404) + expect(response.headers['X-Total']).to eq occurrence_count + + expect(json_response.map { |v| v['report_type'] }.uniq).to match_array %w[dependency_scanning] + + # occurrences are implicitly sorted by Security::PipelineVulnerabilitiesFinder and + # Security::MergeReportsService so their order differs from what is present in fixture file + 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_vulnerability_findings_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(project_vulnerability_findings_path, user), params: { scope: 'all' }.merge(pagination) + + expect(response).to have_gitlab_http_status(200) + + expect(response.headers['X-Total']).to eq occurrence_count + end + + it 'returns vulnerabilities with low severity' do + get api(project_vulnerability_findings_path, user), params: { severity: 'low' }.merge(pagination) + + 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_vulnerability_findings_path, user), params: { severity: 'foo' } + + expect(response).to have_gitlab_http_status(400) + end + + it 'returns vulnerabilities with high confidence' do + get api(project_vulnerability_findings_path, user), params: { confidence: 'high' }.merge(pagination) + + 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_vulnerability_findings_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(project_vulnerability_findings_path, user), params: { pipeline_id: pipeline.id }.merge(pagination) + + expect(response).to have_gitlab_http_status(200) + + expect(response.headers['X-Total']).to eq occurrence_count + end + + context 'pipeline has no reports' do + it 'returns empty results' do + get api(project_vulnerability_findings_path, user), params: { pipeline_id: pipeline_without_vulnerabilities.id }.merge(pagination) + + expect(json_response).to eq [] + end + end + + context 'with unknown pipeline' do + it 'returns empty results' do + get api(project_vulnerability_findings_path, user), params: { pipeline_id: 0 }.merge(pagination) + + expect(json_response).to eq [] + end + end end end end + + it_behaves_like 'responds with "not found" when there is no access to the project' do + subject { get api(project_vulnerability_findings_path, user) } + end + + it_behaves_like 'prevents working with vulnerabilities in case of insufficient privileges' do + subject { get api(project_vulnerability_findings_path, user) } + end end end diff --git a/ee/spec/support/shared_examples/requests/api/vulnerabilities_shared_examples.rb b/ee/spec/support/shared_examples/requests/api/vulnerabilities_shared_examples.rb index ee6f057db2ff65b276ee787f0d5f48d054ce235e..0de381a27bb73c52b00fcf5b03dd686c2b9dce47 100644 --- a/ee/spec/support/shared_examples/requests/api/vulnerabilities_shared_examples.rb +++ b/ee/spec/support/shared_examples/requests/api/vulnerabilities_shared_examples.rb @@ -1,36 +1,23 @@ # frozen_string_literal: true -shared_examples 'forbids access to project vulnerabilities endpoint in expected cases' do - context 'with authorized user without read permissions' do - before do - project.add_reporter(user) - end - +shared_examples 'prevents working with vulnerabilities in case of insufficient privileges' do + context 'with lesser access level than required' do it 'responds with 403 Forbidden' do - get api(project_vulnerabilities_path, user) - - expect(response).to have_gitlab_http_status(403) - end - end - - context 'with authorized user but when security dashboard is not available' do - before do - project.add_developer(user) - stub_licensed_features(security_dashboard: false) - end + project.add_reporter(user) - it 'responds with 403 Forbidden' do - get api(project_vulnerabilities_path, user) + subject expect(response).to have_gitlab_http_status(403) end end +end +shared_examples 'responds with "not found" when there is no access to the project' do context 'with no project access' do let(:project) { create(:project) } it 'responds with 404 Not Found' do - get api(project_vulnerabilities_path, user) + subject expect(response).to have_gitlab_http_status(404) end @@ -44,179 +31,9 @@ let(:project) { build(:project) } it 'responds with 404 Not Found' do - get api(project_vulnerabilities_path, user) + subject expect(response).to have_gitlab_http_status(404) end end end - -shared_examples 'getting list of vulnerability findings' do - before do - stub_licensed_features(security_dashboard: true, sast: true, dependency_scanning: true, container_scanning: true) - - create(:ee_ci_job_artifact, :dependency_scanning, job: build_ds, project: project) - create(:ee_ci_job_artifact, :sast, job: build_sast, project: project) - dismissal - end - - let(:pipeline) { create(:ci_empty_pipeline, status: :created, project: project) } - let(:pipeline_without_vulnerabilities) { create(:ci_pipeline_without_jobs, status: :created, project: project) } - - let(:build_ds) { create(:ci_build, :success, name: 'ds_job', pipeline: pipeline, project: project) } - let(:build_sast) { create(:ci_build, :success, name: 'sast_job', pipeline: pipeline, project: project) } - - let(:ds_report) { pipeline.security_reports.reports["dependency_scanning"] } - let(:sast_report) { pipeline.security_reports.reports["sast"] } - - let(:dismissal) do - create(:vulnerability_feedback, :dismissal, :sast, - project: project, - pipeline: pipeline, - project_fingerprint: sast_report.occurrences.first.project_fingerprint, - vulnerability_data: sast_report.occurrences.first.raw_metadata - ) - end - - context 'with an authorized user with proper permissions' do - before do - project.add_developer(user) - end - - # Because fixture reports that power :ee_ci_job_artifact factory contain long report lists, - # we need to make sure that all occurrences for both SAST and Dependency Scanning are included in the response. - # That's why the page size is 40. - let(:pagination) { { per_page: 40 } } - - it 'returns all non-dismissed vulnerabilities' do - # all occurrences except one that was dismissed - occurrence_count = (sast_report.occurrences.count + ds_report.occurrences.count - 1).to_s - - get api(project_vulnerabilities_path, user), params: pagination - - expect(response).to have_gitlab_http_status(200) - expect(response).to include_pagination_headers - expect(response).to match_response_schema('vulnerabilities/occurrence_list', dir: 'ee') - - expect(response.headers['X-Total']).to eq occurrence_count - - expect(json_response.map { |v| v['report_type'] }.uniq).to match_array %w[dependency_scanning sast] - end - - it 'does not have N+1 queries' do - control_count = ActiveRecord::QueryRecorder.new do - get api(project_vulnerabilities_path, user), params: { report_type: 'dependency_scanning' } - end.count - - # Threshold is required for the extra query performed in Security::PipelineVulnerabilitiesFinder to load - # the Vulnerabilities providing computed states for the associated Vulnerability::Occurrences - expect { get api(project_vulnerabilities_path, user) }.not_to exceed_query_limit(control_count).with_threshold(1) - end - - describe 'filtering' do - it 'returns vulnerabilities with sast report_type' do - occurrence_count = (sast_report.occurrences.count - 1).to_s # all SAST occurrences except one that was dismissed - - get api(project_vulnerabilities_path, user), params: { report_type: 'sast' } - - expect(response).to have_gitlab_http_status(200) - - expect(response.headers['X-Total']).to eq occurrence_count - - expect(json_response.map { |v| v['report_type'] }.uniq).to match_array %w[sast] - - # occurrences are implicitly sorted by Security::MergeReportsService, - # occurrences order differs from what is present in fixture file - expect(json_response.first['name']).to eq 'ECB mode is insecure' - end - - it 'returns vulnerabilities with dependency_scanning report_type' do - occurrence_count = ds_report.occurrences.count.to_s - - get api(project_vulnerabilities_path, user), params: { report_type: 'dependency_scanning' } - - expect(response).to have_gitlab_http_status(200) - - expect(response.headers['X-Total']).to eq occurrence_count - - expect(json_response.map { |v| v['report_type'] }.uniq).to match_array %w[dependency_scanning] - - # occurrences are implicitly sorted by Security::MergeReportsService, - # occurrences order differs from what is present in fixture file - 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(project_vulnerabilities_path, user), params: { scope: 'all' }.merge(pagination) - - expect(response).to have_gitlab_http_status(200) - - expect(response.headers['X-Total']).to eq occurrence_count - end - - it 'returns vulnerabilities with low severity' do - get api(project_vulnerabilities_path, user), params: { severity: 'low' }.merge(pagination) - - 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(project_vulnerabilities_path, user), params: { confidence: 'high' }.merge(pagination) - - 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(project_vulnerabilities_path, user), params: { pipeline_id: pipeline.id }.merge(pagination) - - expect(response).to have_gitlab_http_status(200) - - expect(response.headers['X-Total']).to eq occurrence_count - end - - context 'pipeline has no reports' do - it 'returns empty results' do - get api(project_vulnerabilities_path, user), params: { pipeline_id: pipeline_without_vulnerabilities.id }.merge(pagination) - - expect(json_response).to eq [] - end - end - - context 'with unknown pipeline' do - it 'returns empty results' do - get api(project_vulnerabilities_path, user), params: { pipeline_id: 0 }.merge(pagination) - - expect(json_response).to eq [] - end - end - end - 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 index b8e66a973c77bb4798899b266cae5a8d80a59d55..db251af11129561f07c2410585b0188414bf205e 100644 --- a/ee/spec/views/projects/pipelines/_tabs_content.html.haml_spec.rb +++ b/ee/spec/views/projects/pipelines/_tabs_content.html.haml_spec.rb @@ -11,27 +11,9 @@ 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 + it 'rendering the Vulnerability Findings API endpoint path' 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 + expect(rendered).to include "projects/#{pipeline.project_id}/vulnerability_findings" end end