Merge branch '6078-add-permissions-checks-dismiss-issue' into 'master'

Resolve "Hide or disable security reports feedback actions when not authorized"

Closes #6078

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

app/assets/javascripts/pipelines/pipeline_details_bundle.js

@@ -122,6 +122,8 @@ export default () => {
       sastContainerHelpPath,
     } = datasetOptions;
     const pipelineId = parseInt(datasetOptions.pipelineId, 10);
+    const canCreateIssue = datasetOptions.canCreateIssue;
+    const canCreateFeedback = datasetOptions.canCreateFeedback;
 
     const store = createStore();
     // Widget summary
@@ -174,6 +176,8 @@ export default () => {
             sastContainerHeadPath: sastContainerEndpoint,
             dastHelpPath,
             sastContainerHelpPath,
+            canCreateFeedback,
+            canCreateIssue,
           },
           on: {
             updateBadgeCount: this.updateBadge,

app/views/projects/pipelines/_with_tabs.html.haml

@@ -100,4 +100,6 @@
       sast_help_path: help_page_path('user/project/merge_requests/sast'),
       dependency_scanning_help_path: help_page_path('user/project/merge_requests/dependency_scanning'),
       dast_help_path: help_page_path('user/project/merge_requests/dast'),
-      sast_container_help_path: help_page_path('user/project/merge_requests/sast_container')} }
+      sast_container_help_path: help_page_path('user/project/merge_requests/sast_container'),
+      can_create_feedback: can?(current_user, :admin_vulnerability_feedback, @project),
+      can_create_issue: show_new_issue_link?(@project)} }

ee/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.vue

@@ -288,6 +288,8 @@ export default {
       :vulnerability-feedback-path="mr.vulnerabilityFeedbackPath"
       :vulnerability-feedback-help-path="mr.vulnerabilityFeedbackHelpPath"
       :pipeline-id="mr.securityReportsPipelineId"
+      :can-create-issue="mr.canCreateIssue"
+      :can-create-feedback="mr.canCreateFeedback"
     />
     <report-section
       v-if="shouldRenderLicenseReport"

ee/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js

@@ -20,6 +20,7 @@ export default class MergeRequestStore extends CEMergeRequestStore {
     this.vulnerabilityFeedbackHelpPath = data.vulnerability_feedback_help_path;
     this.approvalsHelpPath = data.approvals_help_path;
     this.securityReportsPipelineId = data.pipeline_id;
+    this.canCreateFeedback = data.can_create_feedback || false;
 
     this.initCodeclimate(data);
     this.initPerformanceReport(data);

ee/app/assets/javascripts/vue_shared/security_reports/components/help_popover.vue

@@ -2,11 +2,7 @@
 import $ from 'jquery';
 import Icon from '~/vue_shared/components/icon.vue';
 import { inserted } from '~/feature_highlight/feature_highlight_helper';
-import {
-  mouseenter,
-  debouncedMouseleave,
-  togglePopover,
-} from '~/shared/popover';
+import { mouseenter, debouncedMouseleave, togglePopover } from '~/shared/popover';
 
 export default {
   name: 'SecurityReportsHelpPopover',
@@ -22,21 +18,22 @@ export default {
   mounted() {
     const $el = $(this.$el);
 
-    $el.popover({
-      html: true,
-      trigger: 'focus',
-      container: 'body',
-      placement: 'top',
-      template:
-        '<div class="popover" role="tooltip"><div class="arrow"></div><p class="popover-header"></p><div class="popover-body"></div></div>',
-      ...this.options,
-    })
-    .on('mouseenter', mouseenter)
-    .on('mouseleave', debouncedMouseleave(300))
-    .on('inserted.bs.popover', inserted)
-    .on('show.bs.popover', () => {
-      window.addEventListener('scroll', togglePopover.bind($el, false), { once: true });
-    });
+    $el
+      .popover({
+        html: true,
+        trigger: 'focus',
+        container: 'body',
+        placement: 'top',
+        template:
+          '<div class="popover" role="tooltip"><div class="arrow"></div><p class="popover-header"></p><div class="popover-body"></div></div>',
+        ...this.options,
+      })
+      .on('mouseenter', mouseenter)
+      .on('mouseleave', debouncedMouseleave(300))
+      .on('inserted.bs.popover', inserted)
+      .on('show.bs.popover', () => {
+        window.addEventListener('scroll', togglePopover.bind($el, false), { once: true });
+      });
   },
 };
 </script>

ee/app/assets/javascripts/vue_shared/security_reports/components/modal.vue

 <script>
-import { mapActions, mapState } from 'vuex';
-import { s__ } from '~/locale';
-import Modal from '~/vue_shared/components/gl_modal.vue';
-import LoadingButton from '~/vue_shared/components/loading_button.vue';
-import Icon from '~/vue_shared/components/icon.vue';
-import ExpandButton from '~/vue_shared/components/expand_button.vue';
+  import { mapActions, mapState } from 'vuex';
+  import { s__ } from '~/locale';
+  import Modal from '~/vue_shared/components/gl_modal.vue';
+  import LoadingButton from '~/vue_shared/components/loading_button.vue';
+  import Icon from '~/vue_shared/components/icon.vue';
+  import ExpandButton from '~/vue_shared/components/expand_button.vue';
 
-export default {
-  components: {
-    Modal,
-    LoadingButton,
-    ExpandButton,
-    Icon,
-  },
-  computed: {
-    ...mapState(['modal', 'vulnerabilityFeedbackHelpPath']),
-    revertTitle() {
-      return this.modal.vulnerability.isDismissed
-        ? s__('ciReport|Revert dismissal')
-        : s__('ciReport|Dismiss vulnerability');
+  export default {
+    components: {
+      Modal,
+      LoadingButton,
+      ExpandButton,
+      Icon,
     },
-    hasDismissedBy() {
-      return this.modal.vulnerability.dismissalFeedback &&
-        this.modal.vulnerability.dismissalFeedback.pipeline &&
-        this.modal.vulnerability.dismissalFeedback.author;
+    computed: {
+      ...mapState([
+        'modal',
+        'vulnerabilityFeedbackHelpPath',
+        'canCreateIssuePermission',
+        'canCreateFeedbackPermission',
+      ]),
+      revertTitle() {
+        return this.modal.vulnerability.isDismissed
+          ? s__('ciReport|Revert dismissal')
+          : s__('ciReport|Dismiss vulnerability');
+      },
+      hasDismissedBy() {
+        return (
+          this.modal.vulnerability.dismissalFeedback &&
+          this.modal.vulnerability.dismissalFeedback.pipeline &&
+          this.modal.vulnerability.dismissalFeedback.author
+        );
+      },
+      /**
+       * The slot for the footer should be rendered if any of the conditions is true.
+       */
+      shouldRenderFooterSection() {
+        return (
+          !this.modal.isResolved &&
+          (this.canCreateFeedbackPermission || this.canCreateIssuePermission)
+        );
+      },
     },
-  },
-  methods: {
-    ...mapActions(['dismissIssue', 'revertDismissIssue', 'createNewIssue']),
-    handleDismissClick() {
-      if (this.modal.vulnerability.isDismissed) {
-        this.revertDismissIssue();
-      } else {
-        this.dismissIssue();
-      }
+    methods: {
+      ...mapActions(['dismissIssue', 'revertDismissIssue', 'createNewIssue']),
+      handleDismissClick() {
+        if (this.modal.vulnerability.isDismissed) {
+          this.revertDismissIssue();
+        } else {
+          this.dismissIssue();
+        }
+      },
+      isLastValue(index, values) {
+        return index < values.length - 1;
+      },
+      hasValue(field) {
+        return field.value && field.value.length > 0;
+      },
+      hasInstances(field, key) {
+        return key === 'instances' && this.hasValue(field);
+      },
+      hasIdentifiers(field, key) {
+        return key === 'identifiers' && this.hasValue(field);
+      },
+      hasLinks(field, key) {
+        return key === 'links' && this.hasValue(field);
+      },
     },
-    isLastValue(index, values) {
-      return index < values.length - 1;
-    },
-    hasValue(field) {
-      return field.value && field.value.length > 0;
-    },
-    hasInstances(field, key) {
-      return key === 'instances' && this.hasValue(field);
-    },
-    hasIdentifiers(field, key) {
-      return key === 'identifiers' && this.hasValue(field);
-    },
-    hasLinks(field, key) {
-      return key === 'links' && this.hasValue(field);
-    },
-  },
-};
+  };
 </script>
 <template>
   <modal
     id="modal-mrwidget-security-issue"
     :header-title-text="modal.title"
-    :class="{'modal-hide-footer': modal.isResolved}"
+    :class="{ 'modal-hide-footer': !shouldRenderFooterSection }"
     class="modal-security-report-dast"
   >
     <slot>
@@ -183,7 +199,7 @@ export default {
             :href="vulnerabilityFeedbackHelpPath"
             class="js-link-vulnerabilityFeedbackHelpPath"
           >
-            Learn more about interacting with security reports (Alpha).
+            {{ s__('ciReport|Learn more about interacting with security reports (Alpha).') }}
           </a>
         </div>
       </div>
@@ -196,7 +212,7 @@ export default {
       </div>
     </slot>
     <div slot="footer">
-      <template v-if="!modal.isResolved">
+      <template v-if="shouldRenderFooterSection">
         <button
           type="button"
           class="btn btn-default"
@@ -206,6 +222,7 @@ export default {
         </button>
 
         <loading-button
+          v-if="canCreateFeedbackPermission"
           :loading="modal.isDismissingIssue"
           :disabled="modal.isDismissingIssue"
           :label="revertTitle"
@@ -221,12 +238,13 @@ export default {
         >
           {{ __('View issue' ) }}
         </a>
+
         <loading-button
-          v-else
+          v-else-if="!modal.vulnerability.hasIssue && canCreateIssuePermission"
           :loading="modal.isCreatingNewIssue"
           :disabled="modal.isCreatingNewIssue"
           :label="__('Create issue')"
-          container-class="btn btn-success btn-inverted"
+          container-class="js-create-issue-btn btn btn-success btn-inverted"
           @click="createNewIssue"
         />
       </template>

ee/app/assets/javascripts/vue_shared/security_reports/grouped_security_reports_app.vue

@@ -102,6 +102,14 @@ export default {
       required: false,
       default: null,
     },
+    canCreateFeedback: {
+      type: Boolean,
+      required: true,
+    },
+    canCreateIssue: {
+      type: Boolean,
+      required: true,
+    },
   },
   sast: SAST,
   dast: DAST,
@@ -130,6 +138,9 @@ export default {
     this.setVulnerabilityFeedbackHelpPath(this.vulnerabilityFeedbackHelpPath);
     this.setPipelineId(this.pipelineId);
 
+    this.setCanCreateIssuePermission(this.canCreateIssue);
+    this.setCanCreateFeedbackPermission(this.canCreateFeedback);
+
     if (this.sastHeadPath) {
       this.setSastHeadPath(this.sastHeadPath);
 
@@ -186,6 +197,8 @@ export default {
       'setVulnerabilityFeedbackPath',
       'setVulnerabilityFeedbackHelpPath',
       'setPipelineId',
+      'setCanCreateIssuePermission',
+      'setCanCreateFeedbackPermission',
     ]),
   },
 };

ee/app/assets/javascripts/vue_shared/security_reports/split_security_reports_app.vue

@@ -74,6 +74,14 @@ export default {
       required: false,
       default: null,
     },
+    canCreateFeedback: {
+      type: Boolean,
+      required: true,
+    },
+    canCreateIssue: {
+      type: Boolean,
+      required: true,
+    },
   },
   sast: SAST,
   dast: DAST,
@@ -106,6 +114,8 @@ export default {
     this.setVulnerabilityFeedbackPath(this.vulnerabilityFeedbackPath);
     this.setVulnerabilityFeedbackHelpPath(this.vulnerabilityFeedbackHelpPath);
     this.setPipelineId(this.pipelineId);
+    this.setCanCreateIssuePermission(this.canCreateIssue);
+    this.setCanCreateFeedbackPermission(this.canCreateFeedback);
 
     if (this.sastHeadPath) {
       this.setSastHeadPath(this.sastHeadPath);
@@ -168,6 +178,8 @@ export default {
       'setVulnerabilityFeedbackPath',
       'setVulnerabilityFeedbackHelpPath',
       'setPipelineId',
+      'setCanCreateIssuePermission',
+      'setCanCreateFeedbackPermission',
     ]),
 
     summaryTextBuilder(type, issuesCount = 0) {

ee/app/assets/javascripts/vue_shared/security_reports/store/actions.js

@@ -16,6 +16,11 @@ export const setVulnerabilityFeedbackHelpPath = ({ commit }, path) =>
 
 export const setPipelineId = ({ commit }, id) => commit(types.SET_PIPELINE_ID, id);
 
+export const setCanCreateIssuePermission = ({ commit }, permission) =>
+  commit(types.SET_CAN_CREATE_ISSUE_PERMISSION, permission);
+export const setCanCreateFeedbackPermission = ({ commit }, permission) =>
+  commit(types.SET_CAN_CREATE_FEEDBACK_PERMISSION, permission);
+
 /**
  * SAST
  */
@@ -45,16 +50,16 @@ export const fetchSastReports = ({ state, dispatch }) => {
       },
     }),
   ])
-  .then(values => {
-    dispatch('receiveSastReports', {
-      head: values && values[0] ? values[0].data : null,
-      base: values && values[1] ? values[1].data : null,
-      enrichData: values && values[2] ? values[2].data : [],
+    .then(values => {
+      dispatch('receiveSastReports', {
+        head: values && values[0] ? values[0].data : null,
+        base: values && values[1] ? values[1].data : null,
+        enrichData: values && values[2] ? values[2].data : [],
+      });
+    })
+    .catch(() => {
+      dispatch('receiveSastError');
     });
-  })
-  .catch(() => {
-    dispatch('receiveSastError');
-  });
 };
 
 export const updateSastIssue = ({ commit }, issue) => commit(types.UPDATE_SAST_ISSUE, issue);
@@ -180,16 +185,16 @@ export const fetchDependencyScanningReports = ({ state, dispatch }) => {
       },
     }),
   ])
-  .then(values => {
-    dispatch('receiveDependencyScanningReports', {
-      head: values[0] ? values[0].data : null,
-      base: values[1] ? values[1].data : null,
-      enrichData: values && values[2] ? values[2].data : [],
+    .then(values => {
+      dispatch('receiveDependencyScanningReports', {
+        head: values[0] ? values[0].data : null,
+        base: values[1] ? values[1].data : null,
+        enrichData: values && values[2] ? values[2].data : [],
+      });
+    })
+    .catch(() => {
+      dispatch('receiveDependencyScanningError');
     });
-  })
-  .catch(() => {
-    dispatch('receiveDependencyScanningError');
-  });
 };
 
 export const updateDependencyScanningIssue = ({ commit }, issue) =>
@@ -211,13 +216,15 @@ export const dismissIssue = ({ state, dispatch }) => {
   dispatch('requestDismissIssue');
 
   return axios
-    .post(state.vulnerabilityFeedbackPath, { vulnerability_feedback: {
-      feedback_type: 'dismissal',
-      category: state.modal.vulnerability.category,
-      project_fingerprint: state.modal.vulnerability.project_fingerprint,
-      pipeline_id: state.pipelineId,
-      vulnerability_data: state.modal.vulnerability,
-    } })
+    .post(state.vulnerabilityFeedbackPath, {
+      vulnerability_feedback: {
+        feedback_type: 'dismissal',
+        category: state.modal.vulnerability.category,
+        project_fingerprint: state.modal.vulnerability.project_fingerprint,
+        pipeline_id: state.pipelineId,
+        vulnerability_data: state.modal.vulnerability,
+      },
+    })
     .then(({ data }) => {
       dispatch('receiveDismissIssue');
 
@@ -302,13 +309,15 @@ export const createNewIssue = ({ state, dispatch }) => {
   dispatch('requestCreateIssue');
 
   return axios
-    .post(state.vulnerabilityFeedbackPath, { vulnerability_feedback: {
-      feedback_type: 'issue',
-      category: state.modal.vulnerability.category,
-      project_fingerprint: state.modal.vulnerability.project_fingerprint,
-      pipeline_id: state.pipelineId,
-      vulnerability_data: state.modal.vulnerability,
-    } })
+    .post(state.vulnerabilityFeedbackPath, {
+      vulnerability_feedback: {
+        feedback_type: 'issue',
+        category: state.modal.vulnerability.category,
+        project_fingerprint: state.modal.vulnerability.project_fingerprint,
+        pipeline_id: state.pipelineId,
+        vulnerability_data: state.modal.vulnerability,
+      },
+    })
     .then(response => {
       dispatch('receiveCreateIssue');
       // redirect the user to the created issue

ee/app/assets/javascripts/vue_shared/security_reports/store/mutation_types.js

@@ -3,6 +3,8 @@ export const SET_BASE_BLOB_PATH = 'SET_BASE_BLOB_PATH';
 export const SET_VULNERABILITY_FEEDBACK_PATH = 'SET_VULNERABILITY_FEEDBACK_PATH';
 export const SET_VULNERABILITY_FEEDBACK_HELP_PATH = 'SET_VULNERABILITY_FEEDBACK_HELP_PATH';
 export const SET_PIPELINE_ID = 'SET_PIPELINE_ID';
+export const SET_CAN_CREATE_ISSUE_PERMISSION = 'SET_CAN_CREATE_ISSUE_PERMISSION';
+export const SET_CAN_CREATE_FEEDBACK_PERMISSION = 'SET_CAN_CREATE_FEEDBACK_PERMISSION';
 
 // SAST
 export const SET_SAST_HEAD_PATH = 'SET_SAST_HEAD_PATH';

ee/app/assets/javascripts/vue_shared/security_reports/store/mutations.js

@@ -32,6 +32,14 @@ export default {
     state.pipelineId = id;
   },
 
+  [types.SET_CAN_CREATE_ISSUE_PERMISSION](state, permission) {
+    state.canCreateIssuePermission = permission;
+  },
+
+  [types.SET_CAN_CREATE_FEEDBACK_PERMISSION](state, permission) {
+    state.canCreateFeedbackPermission = permission;
+  },
+
   // SAST
   [types.SET_SAST_HEAD_PATH](state, path) {
     state.sast.paths.head = path;

ee/app/assets/javascripts/vue_shared/security_reports/store/state.js

@@ -14,6 +14,8 @@ export default () => ({
   vulnerabilityFeedbackPath: null,
   vulnerabilityFeedbackHelpPath: null,
   pipelineId: null,
+  canCreateIssuePermission: false,
+  canCreateFeedbackPermission: false,
 
   sast: {
     paths: {

ee/app/serializers/ee/merge_request_widget_entity.rb

@@ -153,6 +153,10 @@ module EE
         project_vulnerability_feedback_index_path(merge_request.project)
       end
 
+      expose :can_create_feedback do |merge_request|
+        can?(current_user, :admin_vulnerability_feedback, merge_request)
+      end
+
       expose :rebase_commit_sha
       expose :rebase_in_progress?, as: :rebase_in_progress
 

ee/changelogs/unreleased/6078-add-permissions-checks-dismiss-issue.yml

+---
+title: Adds permission checks to dismiss issue in security reports
+merge_request:
+author:
+type: fixed

spec/fixtures/api/schemas/entities/merge_request_widget.json

@@ -129,7 +129,8 @@
       "head_path": { "type": "string" },
       "base_path": { "type": "string" }
     },
-    "vulnerability_feedback_path": { "type": "string" }
+    "vulnerability_feedback_path": { "type": "string" },
+    "can_create_feedback": { "type": "boolean" }
   },
   "additionalProperties": false
 }

spec/javascripts/vue_shared/security_reports/grouped_security_reports_app_spec.js

@@ -67,6 +67,8 @@ describe('Grouped security reports app', () => {
         vulnerabilityFeedbackPath: 'vulnerability_feedback_path.json',
         vulnerabilityFeedbackHelpPath: 'path',
         pipelineId: 123,
+        canCreateIssue: true,
+        canCreateFeedback: true,
       });
     });
 
@@ -123,6 +125,8 @@ describe('Grouped security reports app', () => {
         vulnerabilityFeedbackPath: 'vulnerability_feedback_path.json',
         vulnerabilityFeedbackHelpPath: 'path',
         pipelineId: 123,
+        canCreateIssue: true,
+        canCreateFeedback: true,
       });
     });
 
@@ -174,6 +178,8 @@ describe('Grouped security reports app', () => {
         vulnerabilityFeedbackPath: 'vulnerability_feedback_path.json',
         vulnerabilityFeedbackHelpPath: 'path',
         pipelineId: 123,
+        canCreateIssue: true,
+        canCreateFeedback: true,
       });
     });
 
@@ -252,6 +258,8 @@ describe('Grouped security reports app', () => {
         vulnerabilityFeedbackPath: 'vulnerability_feedback_path.json',
         vulnerabilityFeedbackHelpPath: 'path',
         pipelineId: 123,
+        canCreateIssue: true,
+        canCreateFeedback: true,
       });
     });
 

spec/javascripts/vue_shared/security_reports/split_security_reports_app_spec.js

@@ -54,6 +54,8 @@ describe('Slipt security reports app', () => {
           dastHelpPath: 'path',
           sastContainerHelpPath: 'path',
           pipelineId: 123,
+          canCreateIssue: true,
+          canCreateFeedback: true,
         },
       });
     });
@@ -96,6 +98,8 @@ describe('Slipt security reports app', () => {
           dastHelpPath: 'path',
           sastContainerHelpPath: 'path',
           pipelineId: 123,
+          canCreateIssue: true,
+          canCreateFeedback: true,
         },
       });
     });
@@ -144,6 +148,8 @@ describe('Slipt security reports app', () => {
           dastHelpPath: 'path',
           sastContainerHelpPath: 'path',
           pipelineId: 123,
+          canCreateIssue: true,
+          canCreateFeedback: true,
         },
       });
     });

spec/javascripts/vue_shared/security_reports/store/actions_spec.js

@@ -6,6 +6,8 @@ import actions, {
   setVulnerabilityFeedbackPath,
   setVulnerabilityFeedbackHelpPath,
   setPipelineId,
+  setCanCreateIssuePermission,
+  setCanCreateFeedbackPermission,
   setSastHeadPath,
   setSastBasePath,
   requestSastReports,
@@ -164,6 +166,42 @@ describe('security reports actions', () => {
     });
   });
 
+  describe('setCanCreateIssuePermission', () => {
+    it('should commit set can create issue permission', done => {
+      testAction(
+        setCanCreateIssuePermission,
+        true,
+        mockedState,
+        [
+          {
+            type: types.SET_CAN_CREATE_ISSUE_PERMISSION,
+            payload: true,
+          },
+        ],
+        [],
+        done,
+      );
+    });
+  });
+
+  describe('setCanCreateFeedbackPermission', () => {
+    it('should commit set can create feedback permission', done => {
+      testAction(
+        setCanCreateFeedbackPermission,
+        true,
+        mockedState,
+        [
+          {
+            type: types.SET_CAN_CREATE_FEEDBACK_PERMISSION,
+            payload: true,
+          },
+        ],
+        [],
+        done,
+      );
+    });
+  });
+
   describe('setSastHeadPath', () => {
     it('should commit set head blob path', done => {
       testAction(

spec/javascripts/vue_shared/security_reports/store/mutations_spec.js

@@ -66,6 +66,20 @@ describe('security reports mutations', () => {
     });
   });
 
+  describe('SET_CAN_CREATE_ISSUE_PERMISSION', () => {
+    it('should set permission for create issue', () => {
+      mutations[types.SET_CAN_CREATE_ISSUE_PERMISSION](stateCopy, true);
+      expect(stateCopy.canCreateIssuePermission).toEqual(true);
+    });
+  });
+
+  describe('SET_CAN_CREATE_FEEDBACK_PERMISSION', () => {
+    it('should set permission for create feedback', () => {
+      mutations[types.SET_CAN_CREATE_FEEDBACK_PERMISSION](stateCopy, true);
+      expect(stateCopy.canCreateFeedbackPermission).toEqual(true);
+    });
+  });
+
   describe('SET_SAST_HEAD_PATH', () => {
     it('should set sast head path', () => {
       mutations[types.SET_SAST_HEAD_PATH](stateCopy, 'sast_head_path');