Merge branch '8150_split_vulnerability_feedback_urls' into 'master'

Group Security Dashboard: Split vulnerability feedback urls

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

ee/app/assets/javascripts/security_dashboard/components/app.vue

 <script>
+import _ from 'underscore';
 import { mapActions, mapState, mapGetters } from 'vuex';
 import IssueModal from 'ee/vue_shared/security_reports/components/modal.vue';
 import Filters from './filters.vue';
@@ -49,6 +50,14 @@ export default {
     ...mapState('vulnerabilities', ['modal']),
     ...mapState('projects', ['projects']),
     ...mapGetters('filters', ['activeFilters']),
+    canCreateIssuePermission() {
+      const path = this.modal.vulnerability.vulnerability_feedback_issue_path;
+      return _.isString(path) && !_.isEmpty(path);
+    },
+    canCreateFeedbackPermission() {
+      const path = this.modal.vulnerability.vulnerability_feedback_dismissal_path;
+      return _.isString(path) && !_.isEmpty(path);
+    },
   },
   created() {
     this.setProjectsEndpoint(this.projectsEndpoint);
@@ -94,8 +103,8 @@ export default {
     <issue-modal
       :modal="modal"
       :vulnerability-feedback-help-path="vulnerabilityFeedbackHelpPath"
-      :can-create-issue-permission="true"
-      :can-create-feedback-permission="true"
+      :can-create-issue-permission="canCreateIssuePermission"
+      :can-create-feedback-permission="canCreateFeedbackPermission"
       @createNewIssue="createIssue({ vulnerability: modal.vulnerability })"
       @dismissIssue="dismissVulnerability({ vulnerability: modal.vulnerability })"
       @revertDismissIssue="revertDismissal({ vulnerability: modal.vulnerability })"

ee/app/assets/javascripts/security_dashboard/components/security_dashboard_table_row.vue

 <script>
+import _ from 'underscore';
 import { mapActions } from 'vuex';
 import { GlButton, GlSkeletonLoading } from '@gitlab/ui';
 import SeverityBadge from 'ee/vue_shared/security_reports/components/severity_badge.vue';
@@ -44,10 +45,12 @@ export default {
       return Boolean(this.vulnerability.issue_feedback);
     },
     canDismissVulnerability() {
-      return Boolean(this.vulnerability.vulnerability_feedback_url);
+      const path = this.vulnerability.vulnerability_feedback_dismissal_path;
+      return _.isString(path) && !_.isEmpty(path);
     },
     canCreateIssue() {
-      return this.canDismissVulnerability && !this.hasIssue;
+      const path = this.vulnerability.vulnerability_feedback_issue_path;
+      return _.isString(path) && !_.isEmpty(path) && !this.hasIssue;
     },
   },
   methods: {

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

@@ -84,7 +84,7 @@ export const openModal = ({ commit }, payload = {}) => {
 export const createIssue = ({ dispatch }, { vulnerability, flashError }) => {
   dispatch('requestCreateIssue');
   axios
-    .post(vulnerability.vulnerability_feedback_url, {
+    .post(vulnerability.vulnerability_feedback_issue_path, {
       vulnerability_feedback: {
         feedback_type: 'issue',
         category: vulnerability.report_type,
@@ -127,7 +127,7 @@ export const dismissVulnerability = ({ dispatch }, { vulnerability, flashError }
   dispatch('requestDismissVulnerability');
 
   axios
-    .post(vulnerability.vulnerability_feedback_url, {
+    .post(vulnerability.vulnerability_feedback_dismissal_path, {
       vulnerability_feedback: {
         feedback_type: 'dismissal',
         category: vulnerability.report_type,
@@ -167,9 +167,9 @@ export const receiveDismissVulnerabilityError = ({ commit }, { flashError }) =>
 };
 
 export const revertDismissal = ({ dispatch }, { vulnerability, flashError }) => {
-  const { vulnerability_feedback_url, dismissal_feedback } = vulnerability;
+  const { vulnerability_feedback_dismissal_path, dismissal_feedback } = vulnerability;
   // eslint-disable-next-line camelcase
-  const url = `${vulnerability_feedback_url}/${dismissal_feedback.id}`;
+  const url = `${vulnerability_feedback_dismissal_path}/${dismissal_feedback.id}`;
 
   dispatch('requestRevertDismissal');
 

ee/app/models/vulnerabilities/occurrence.rb

@@ -68,7 +68,7 @@ module Vulnerabilities
     scope :by_severities, -> (values) { where(severity: values) }
 
     scope :all_preloaded, -> do
-      preload(:scanner, :identifiers, :project)
+      preload(:scanner, :identifiers, project: [:namespace, :project_feature])
     end
 
     def self.for_pipelines(pipelines)

ee/app/serializers/vulnerabilities/occurrence_entity.rb

@@ -7,7 +7,8 @@ class Vulnerabilities::OccurrenceEntity < Grape::Entity
   expose :scanner, using: Vulnerabilities::ScannerEntity
   expose :identifiers, using: Vulnerabilities::IdentifierEntity
   expose :project_fingerprint
-  expose :vulnerability_feedback_url, if: ->(*) { can_admin_vulnerability_feedback? }
+  expose :vulnerability_feedback_path, as: :vulnerability_feedback_issue_path, if: ->(_, _) { can_admin_vulnerability_feedback? && can_create_issue? }
+  expose :vulnerability_feedback_path, as: :vulnerability_feedback_dismissal_path, if: ->(_, _) { can_admin_vulnerability_feedback? }
   expose :project, using: ::ProjectEntity
   expose :dismissal_feedback, using: Vulnerabilities::FeedbackEntity
   expose :issue_feedback, using: Vulnerabilities::FeedbackEntity
@@ -23,11 +24,15 @@ class Vulnerabilities::OccurrenceEntity < Grape::Entity
 
   private
 
-  def vulnerability_feedback_url
-    project_vulnerability_feedback_index_url(occurrence.project)
+  def vulnerability_feedback_path
+    project_vulnerability_feedback_index_path(occurrence.project)
   end
 
   def can_admin_vulnerability_feedback?
     can?(request.current_user, :admin_vulnerability_feedback, occurrence.project)
   end
+
+  def can_create_issue?
+    can?(request.current_user, :create_issue, occurrence.project)
+  end
 end

ee/spec/controllers/groups/security/vulnerabilities_controller_spec.rb

@@ -88,19 +88,19 @@ describe Groups::Security::VulnerabilitiesController do
         end
 
         context 'with vulnerability feedback' do
-          it "avoids N+1 queries" do
+          it "avoids N+1 queries", :with_request_store do
             create_vulnerabilities(2, project_dev, with_feedback: true)
 
-            control_count = ActiveRecord::QueryRecorder.new { get_summary }
+            control_count = ActiveRecord::QueryRecorder.new { get_index }
 
             create_vulnerabilities(2, project_guest, with_feedback: true)
 
-            expect { get_summary }.not_to exceed_all_query_limit(control_count)
+            expect { get_index }.not_to exceed_all_query_limit(control_count)
           end
 
           private
 
-          def get_summary
+          def get_index
             get :index, params: { group_id: group }, format: :json
           end
         end

ee/spec/fixtures/api/schemas/vulnerabilities/occurrence.json

@@ -12,7 +12,8 @@
   "properties" : {
     "name" : { "type": "string" },
     "project_fingerprint": { "type": "string" },
-    "vulnerability_feedback_url": { "type": "string" },
+    "vulnerability_feedback_issue_path": { "type": "string" },
+    "vulnerability_feedback_dismissal_path": { "type": "string" },
     "confidence" : {
       "type": "string",
       "enum": ["undefined", "ignore", "unknown", "experimental", "low", "medium", "high", "critical"]

ee/spec/javascripts/security_dashboard/store/vulnerabilities/actions_spec.js

@@ -363,7 +363,7 @@ describe('issue creation', () => {
 
     describe('on success', () => {
       beforeEach(() => {
-        mock.onPost(vulnerability.vulnerability_feedback_url).replyOnce(200, { data });
+        mock.onPost(vulnerability.vulnerability_feedback_issue_path).replyOnce(200, { data });
       });
 
       it('should dispatch the request and success actions', done => {
@@ -386,7 +386,7 @@ describe('issue creation', () => {
 
     describe('on error', () => {
       beforeEach(() => {
-        mock.onPost(vulnerability.vulnerability_feedback_url).replyOnce(404, {});
+        mock.onPost(vulnerability.vulnerability_feedback_issue_path).replyOnce(404, {});
       });
 
       it('should dispatch the request and error actions', done => {
@@ -475,7 +475,7 @@ describe('vulnerability dismissal', () => {
 
     describe('on success', () => {
       beforeEach(() => {
-        mock.onPost(vulnerability.vulnerability_feedback_url).replyOnce(200, data);
+        mock.onPost(vulnerability.vulnerability_feedback_dismissal_path).replyOnce(200, data);
       });
 
       it('should dispatch the request and success actions', done => {
@@ -498,7 +498,7 @@ describe('vulnerability dismissal', () => {
 
     describe('on error', () => {
       beforeEach(() => {
-        mock.onPost(vulnerability.vulnerability_feedback_url).replyOnce(404, {});
+        mock.onPost(vulnerability.vulnerability_feedback_dismissal_path).replyOnce(404, {});
       });
 
       it('should dispatch the request and error actions', done => {
@@ -574,7 +574,7 @@ describe('vulnerability dismissal', () => {
 describe('revert vulnerability dismissal', () => {
   describe('revertDismissal', () => {
     const vulnerability = mockDataVulnerabilities[2];
-    const url = `${vulnerability.vulnerability_feedback_url}/${
+    const url = `${vulnerability.vulnerability_feedback_dismissal_path}/${
       vulnerability.dismissal_feedback.id
     }`;
     let mock;

ee/spec/javascripts/security_dashboard/store/vulnerabilities/data/mock_data_vulnerabilities.json

@@ -32,7 +32,8 @@
     },
     "dismissal_feedback": null,
     "issue_feedback": null,
-    "vulnerability_feedback_url": "https://example.com/vulnerability_feedback",
+    "vulnerability_feedback_issue_path": "https://example.com/vulnerability_feedback",
+    "vulnerability_feedback_dismissal_path": "https://example.com/vulnerability_feedback",
     "description": "The cipher does not provide data integrity update 1",
     "solution": "GCM mode introduces an HMAC into the resulting encrypted data, providing integrity of the result.",
     "location": {
@@ -94,7 +95,8 @@
     },
     "dismissal_feedback": null,
     "issue_feedback": null,
-    "vulnerability_feedback_url": "https://example.com/vulnerability_feedback",
+    "vulnerability_feedback_issue_path": "https://example.com/vulnerability_feedback",
+    "vulnerability_feedback_dismissal_path": "https://example.com/vulnerability_feedback",
     "description": "The cipher does not provide data integrity update 1",
     "solution": "GCM mode introduces an HMAC into the resulting encrypted data, providing integrity of the result.",
     "location": {
@@ -166,7 +168,8 @@
       "project_fingerprint": "4e5b6966dd100170b4b1ad599c7058cce91b57b4"
     },
     "issue_feedback": null,
-    "vulnerability_feedback_url": "https://example.com/vulnerability_feedback",
+    "vulnerability_feedback_issue_path": "https://example.com/vulnerability_feedback",
+    "vulnerability_feedback_dismissal_path": "https://example.com/vulnerability_feedback",
     "description": "The cipher does not provide data integrity update 1",
     "solution": "GCM mode introduces an HMAC into the resulting encrypted data, providing integrity of the result.",
     "location": {
@@ -239,7 +242,8 @@
       "branch": "master",
       "project_fingerprint": "4e5b6966dd100170b4b1ad599c7058cce91b57b4"
     },
-    "vulnerability_feedback_url": "https://example.com/vulnerability_feedback",
+    "vulnerability_feedback_issue_path": "https://example.com/vulnerability_feedback",
+    "vulnerability_feedback_dismissal_path": "https://example.com/vulnerability_feedback",
     "description": "The cipher does not provide data integrity update 1",
     "solution": "GCM mode introduces an HMAC into the resulting encrypted data, providing integrity of the result.",
     "location": {
@@ -334,7 +338,8 @@
       "branch": "master",
       "project_fingerprint": "4e5b6966dd100170b4b1ad599c7058cce91b57b4"
     },
-    "vulnerability_feedback_url": "https://example.com/vulnerability_feedback",
+    "vulnerability_feedback_issue_path": "https://example.com/vulnerability_feedback",
+    "vulnerability_feedback_dismissal_path": "https://example.com/vulnerability_feedback",
     "description": "The cipher does not provide data integrity update 1",
     "solution": "GCM mode introduces an HMAC into the resulting encrypted data, providing integrity of the result.",
     "location": {
@@ -384,7 +389,8 @@
     },
     "dismissal_feedback": null,
     "issue_feedback": null,
-    "vulnerability_feedback_url": "https://example.com/vulnerability_feedback",
+    "vulnerability_feedback_issue_path": "https://example.com/vulnerability_feedback",
+    "vulnerability_feedback_dismissal_path": "https://example.com/vulnerability_feedback",
     "description": "The cipher does not provide data integrity update 1",
     "solution": "GCM mode introduces an HMAC into the resulting encrypted data, providing integrity of the result.",
     "location": {

ee/spec/serializers/vulnerabilities/occurrence_entity_spec.rb

@@ -61,8 +61,9 @@ describe Vulnerabilities::OccurrenceEntity do
         project.add_guest(user)
       end
 
-      it 'does not contain vulnerability feedback URL' do
-        expect(subject).not_to include(:vulnerability_feedback_url)
+      it 'does not contain vulnerability feedback paths' do
+        expect(subject).not_to include(:vulnerability_feedback_issue_path)
+        expect(subject).not_to include(:vulnerability_feedback_dismissal_path)
       end
     end
 
@@ -71,8 +72,24 @@ describe Vulnerabilities::OccurrenceEntity do
         project.add_developer(user)
       end
 
-      it 'contains vulnerability feedback URL' do
-        expect(subject).to include(:vulnerability_feedback_url)
+      it 'contains vulnerability feedback dismissal path' do
+        expect(subject).to include(:vulnerability_feedback_dismissal_path)
+      end
+
+      it 'contains vulnerability feedback issue path' do
+        expect(subject).to include(:vulnerability_feedback_issue_path)
+      end
+
+      context 'when disallowed to create issue' do
+        let(:project) { create(:project, issues_access_level: ProjectFeature::DISABLED) }
+
+        it 'does not contain vulnerability feedback issue path' do
+          expect(subject).not_to include(:vulnerability_feedback_issue_path)
+        end
+
+        it 'contains vulnerability feedback dismissal path' do
+          expect(subject).to include(:vulnerability_feedback_dismissal_path)
+        end
       end
     end
   end