Merge branch '6017-fixed-vulnerabilities-should-not-be-actionable-in-the-report' into 'master'

Resolve "Fixed vulnerabilities should not be actionable in the report"

Closes #6017

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

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

@@ -21,6 +21,11 @@ export default {
       type: Number,
       required: true,
     },
+    // failed || success
+    status: {
+      type: String,
+      required: true,
+    },
   },
 };
 </script>
@@ -31,6 +36,7 @@ export default {
 
       <modal-open-name
         :issue="issue"
+        :status="status"
         class="js-modal-dast"
       />
     </div>

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

@@ -57,6 +57,7 @@ export default {
   <modal
     id="modal-mrwidget-security-issue"
     :header-title-text="modal.title"
+    :class="{'modal-hide-footer': modal.isResolved}"
     class="modal-security-report-dast"
   >
     <slot>
@@ -195,38 +196,40 @@ export default {
       </div>
     </slot>
     <div slot="footer">
-      <button
-        type="button"
-        class="btn btn-default"
-        data-dismiss="modal"
-      >
-        {{ __('Cancel' ) }}
-      </button>
+      <template v-if="!modal.isResolved">
+        <button
+          type="button"
+          class="btn btn-default"
+          data-dismiss="modal"
+        >
+          {{ __('Cancel' ) }}
+        </button>
 
-      <loading-button
-        :loading="modal.isDismissingIssue"
-        :disabled="modal.isDismissingIssue"
-        :label="revertTitle"
-        container-class="js-dismiss-btn btn btn-close"
-        @click="handleDismissClick"
-      />
+        <loading-button
+          :loading="modal.isDismissingIssue"
+          :disabled="modal.isDismissingIssue"
+          :label="revertTitle"
+          container-class="js-dismiss-btn btn btn-close"
+          @click="handleDismissClick"
+        />
 
-      <a
-        v-if="modal.vulnerability.hasIssue"
-        :href="modal.vulnerability.issueFeedback && modal.vulnerability.issueFeedback.issue_url"
-        rel="noopener noreferrer nofollow"
-        class="btn btn-success btn-inverted"
-      >
-        {{ __('View issue' ) }}
-      </a>
-      <loading-button
-        v-else
-        :loading="modal.isCreatingNewIssue"
-        :disabled="modal.isCreatingNewIssue"
-        :label="__('Create issue')"
-        container-class="btn btn-success btn-inverted"
-        @click="createNewIssue"
-      />
+        <a
+          v-if="modal.vulnerability.hasIssue"
+          :href="modal.vulnerability.issueFeedback && modal.vulnerability.issueFeedback.issue_url"
+          rel="noopener noreferrer nofollow"
+          class="btn btn-success btn-inverted"
+        >
+          {{ __('View issue' ) }}
+        </a>
+        <loading-button
+          v-else
+          :loading="modal.isCreatingNewIssue"
+          :disabled="modal.isCreatingNewIssue"
+          :label="__('Create issue')"
+          container-class="btn btn-success btn-inverted"
+          @click="createNewIssue"
+        />
+      </template>
     </div>
   </modal>
 </template>

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

@@ -7,11 +7,17 @@ export default {
       type: Object,
       required: true,
     },
+    // failed || success
+    status: {
+      type: String,
+      required: true,
+    },
   },
   methods: {
     ...mapActions(['openModal']),
     handleIssueClick() {
-      this.openModal(this.issue);
+      const { issue, status, openModal } = this;
+      openModal({ issue, status });
     },
   },
 };

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

@@ -109,17 +109,20 @@ export default {
         <sast-issue
           v-if="isTypeSast"
           :issue="issue"
+          :status="status"
         />
 
         <dast-issue
           v-else-if="isTypeDast"
           :issue="issue"
           :issue-index="index"
+          :status="status"
         />
 
         <sast-container-issue
           v-else-if="isTypeSastContainer"
           :issue="issue"
+          :status="status"
         />
 
         <codequality-issue

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

@@ -17,6 +17,11 @@ export default {
       type: Object,
       required: true,
     },
+    // failed || success
+    status: {
+      type: String,
+      required: true,
+    },
   },
 };
 </script>
@@ -25,7 +30,10 @@ export default {
     <div class="report-block-list-issue-description-text">
       <template v-if="issue.severity">{{ issue.severity }}:</template>
 
-      <modal-open-name :issue="issue" />
+      <modal-open-name
+        :issue="issue"
+        :status="status"
+      />
     </div>
 
     <report-link

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

@@ -19,6 +19,11 @@ export default {
       type: Object,
       required: true,
     },
+    // failed || success
+    status: {
+      type: String,
+      required: true,
+    },
   },
 };
 </script>
@@ -36,7 +41,10 @@ export default {
       </template>
       <template v-else-if="issue.priority">{{ issue.priority }}:</template>
 
-      <modal-open-name :issue="issue" />
+      <modal-open-name
+        :issue="issue"
+        :status="status"
+      />
     </div>
 
     <report-link

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

@@ -199,13 +199,13 @@ export const fetchDependencyScanningReports = ({ state, dispatch }) => {
 export const updateDependencyScanningIssue = ({ commit }, issue) =>
   commit(types.UPDATE_DEPENDENCY_SCANNING_ISSUE, issue);
 
-export const openModal = ({ dispatch }, issue) => {
-  dispatch('setModalData', issue);
+export const openModal = ({ dispatch }, payload) => {
+  dispatch('setModalData', payload);
 
   $('#modal-mrwidget-security-issue').modal('show');
 };
 
-export const setModalData = ({ commit }, issue) => commit(types.SET_ISSUE_MODAL_DATA, issue);
+export const setModalData = ({ commit }, payload) => commit(types.SET_ISSUE_MODAL_DATA, payload);
 export const requestDismissIssue = ({ commit }) => commit(types.REQUEST_DISMISS_ISSUE);
 export const receiveDismissIssue = ({ commit }) => commit(types.RECEIVE_DISMISS_ISSUE_SUCCESS);
 export const receiveDismissIssueError = ({ commit }, error) =>

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

@@ -247,7 +247,9 @@ export default {
     state.dependencyScanning.hasError = true;
   },
 
-  [types.SET_ISSUE_MODAL_DATA](state, issue) {
+  [types.SET_ISSUE_MODAL_DATA](state, payload) {
+    const { issue, status } = payload;
+
     state.modal.title = issue.title;
     state.modal.data.description.value = issue.description;
     state.modal.data.file.value = issue.location && issue.location.file;
@@ -272,6 +274,7 @@ export default {
     }
     state.modal.data.instances.value = issue.instances;
     state.modal.vulnerability = issue;
+    state.modal.isResolved = status === 'success';
 
     // clear previous state
     state.modal.error = null;

ee/app/assets/stylesheets/pages/security_reports.scss

@@ -139,4 +139,9 @@
     width: $modal-lg;
     max-width: $modal-lg;
   }
+
+  // This is temporary till we get the new modals hooked up
+  &.modal-hide-footer .modal-footer {
+    display: none;
+  }
 }

ee/changelogs/unreleased/6017-fixed-vulnerabilities-should-not-be-actionable-in-the-report.yml

+---
+title: Removes action buttons from resolved vulnerability modal
+merge_request: 6155
+author:
+type: changed

spec/javascripts/vue_shared/security_reports/components/dast_issue_body_spec.js

@@ -20,6 +20,7 @@ describe('dast issue body', () => {
     riskcode: '1',
     riskdesc: 'Low (Medium)',
   };
+  const status = 'failed';
 
   afterEach(() => {
     vm.$destroy();
@@ -31,6 +32,7 @@ describe('dast issue body', () => {
         issue: dastIssue,
         issueIndex: 1,
         modalTargetId: '#modal-mrwidget-issue',
+        status,
       });
 
       expect(vm.$el.textContent.trim()).toContain(`${dastIssue.severity} (${dastIssue.confidence})`);
@@ -43,6 +45,7 @@ describe('dast issue body', () => {
         issue: dastIssue,
         issueIndex: 1,
         modalTargetId: '#modal-mrwidget-issue',
+        status,
       });
     });
 

spec/javascripts/vue_shared/security_reports/components/modal_open_name_spec.js

@@ -13,6 +13,7 @@ describe('Modal open name', () => {
       store,
       props: {
         issue: parsedDast[0],
+        status: 'failed',
       },
     });
   });

spec/javascripts/vue_shared/security_reports/components/modal_spec.js

@@ -21,31 +21,34 @@ describe('Security Reports modal', () => {
   describe('with dismissed issue', () => {
     beforeEach(() => {
       store.dispatch('setModalData', {
-        tool: 'bundler_audit',
-        message: 'Arbitrary file existence disclosure in Action Pack',
-        url: 'https://groups.google.com/forum/#!topic/rubyonrails-security/rMTQy4oRCGk',
-        cve: 'CVE-2014-9999',
-        file: 'Gemfile.lock',
-        solution: 'upgrade to ~> 3.2.21, ~> 4.0.11.1, ~> 4.0.12, ~> 4.1.7.1, >= 4.1.8',
-        title: 'Arbitrary file existence disclosure in Action Pack',
-        path: 'Gemfile.lock',
-        urlPath: 'path/Gemfile.lock',
-        isDismissed: true,
-        dismissalFeedback: {
-          id: 1,
-          category: 'sast',
-          feedback_type: 'dismissal',
-          issue_id: null,
-          author: {
-            name: 'John Smith',
-            username: 'jsmith',
-            web_url: 'https;//gitlab.com/user1',
-          },
-          pipeline: {
-            id: 123,
-            path: '/jsmith/awesome-project/pipelines/123',
+        issue: {
+          tool: 'bundler_audit',
+          message: 'Arbitrary file existence disclosure in Action Pack',
+          url: 'https://groups.google.com/forum/#!topic/rubyonrails-security/rMTQy4oRCGk',
+          cve: 'CVE-2014-9999',
+          file: 'Gemfile.lock',
+          solution: 'upgrade to ~> 3.2.21, ~> 4.0.11.1, ~> 4.0.12, ~> 4.1.7.1, >= 4.1.8',
+          title: 'Arbitrary file existence disclosure in Action Pack',
+          path: 'Gemfile.lock',
+          urlPath: 'path/Gemfile.lock',
+          isDismissed: true,
+          dismissalFeedback: {
+            id: 1,
+            category: 'sast',
+            feedback_type: 'dismissal',
+            issue_id: null,
+            author: {
+              name: 'John Smith',
+              username: 'jsmith',
+              web_url: 'https;//gitlab.com/user1',
+            },
+            pipeline: {
+              id: 123,
+              path: '/jsmith/awesome-project/pipelines/123',
+            },
           },
         },
+        status: 'failed',
       });
 
       vm = mountComponentWithStore(Component, {
@@ -77,15 +80,18 @@ describe('Security Reports modal', () => {
   describe('with not dismissed isssue', () => {
     beforeEach(() => {
       store.dispatch('setModalData', {
-        tool: 'bundler_audit',
-        message: 'Arbitrary file existence disclosure in Action Pack',
-        url: 'https://groups.google.com/forum/#!topic/rubyonrails-security/rMTQy4oRCGk',
-        cve: 'CVE-2014-9999',
-        file: 'Gemfile.lock',
-        solution: 'upgrade to ~> 3.2.21, ~> 4.0.11.1, ~> 4.0.12, ~> 4.1.7.1, >= 4.1.8',
-        title: 'Arbitrary file existence disclosure in Action Pack',
-        path: 'Gemfile.lock',
-        urlPath: 'path/Gemfile.lock',
+        issue: {
+          tool: 'bundler_audit',
+          message: 'Arbitrary file existence disclosure in Action Pack',
+          url: 'https://groups.google.com/forum/#!topic/rubyonrails-security/rMTQy4oRCGk',
+          cve: 'CVE-2014-9999',
+          file: 'Gemfile.lock',
+          solution: 'upgrade to ~> 3.2.21, ~> 4.0.11.1, ~> 4.0.12, ~> 4.1.7.1, >= 4.1.8',
+          title: 'Arbitrary file existence disclosure in Action Pack',
+          path: 'Gemfile.lock',
+          urlPath: 'path/Gemfile.lock',
+        },
+        status: 'failed',
       });
 
       vm = mountComponentWithStore(Component, {
@@ -112,27 +118,30 @@ describe('Security Reports modal', () => {
   describe('with instances', () => {
     beforeEach(() => {
       store.dispatch('setModalData', {
-        title: 'Absence of Anti-CSRF Tokens',
-        riskcode: '1',
-        riskdesc: 'Low (Medium)',
-        desc: '<p>No Anti-CSRF tokens were found in a HTML submission form.</p>',
-        pluginid: '123',
-        instances: [
-          {
-            uri: 'http://192.168.32.236:3001/explore?sort=latest_activity_desc',
-            method: 'GET',
-            evidence:
-              "<form class='navbar-form' action='/search' accept-charset='UTF-8' method='get'>",
-          },
-          {
-            uri: 'http://192.168.32.236:3001/help/user/group/subgroups/index.md',
-            method: 'GET',
-            evidence:
-              "<form class='navbar-form' action='/search' accept-charset='UTF-8' method='get'>",
-          },
-        ],
-        description: ' No Anti-CSRF tokens were found in a HTML submission form. ',
-        solution: '',
+        issue: {
+          title: 'Absence of Anti-CSRF Tokens',
+          riskcode: '1',
+          riskdesc: 'Low (Medium)',
+          desc: '<p>No Anti-CSRF tokens were found in a HTML submission form.</p>',
+          pluginid: '123',
+          instances: [
+            {
+              uri: 'http://192.168.32.236:3001/explore?sort=latest_activity_desc',
+              method: 'GET',
+              evidence:
+                "<form class='navbar-form' action='/search' accept-charset='UTF-8' method='get'>",
+            },
+            {
+              uri: 'http://192.168.32.236:3001/help/user/group/subgroups/index.md',
+              method: 'GET',
+              evidence:
+                "<form class='navbar-form' action='/search' accept-charset='UTF-8' method='get'>",
+            },
+          ],
+          description: ' No Anti-CSRF tokens were found in a HTML submission form. ',
+          solution: '',
+        },
+        status: 'failed',
       });
 
       vm = mountComponentWithStore(Component, {
@@ -155,19 +164,22 @@ describe('Security Reports modal', () => {
   describe('data & create issue button', () => {
     beforeEach(() => {
       store.dispatch('setModalData', {
-        tool: 'bundler_audit',
-        message: 'Arbitrary file existence disclosure in Action Pack',
-        cve: 'CVE-2014-9999',
-        solution: 'upgrade to ~> 3.2.21, ~> 4.0.11.1, ~> 4.0.12, ~> 4.1.7.1, >= 4.1.8',
-        title: 'Arbitrary file existence disclosure in Action Pack',
-        path: 'Gemfile.lock',
-        urlPath: 'path/Gemfile.lock',
-        location: {
-          file: 'Gemfile.lock',
+        issue: {
+          tool: 'bundler_audit',
+          message: 'Arbitrary file existence disclosure in Action Pack',
+          cve: 'CVE-2014-9999',
+          solution: 'upgrade to ~> 3.2.21, ~> 4.0.11.1, ~> 4.0.12, ~> 4.1.7.1, >= 4.1.8',
+          title: 'Arbitrary file existence disclosure in Action Pack',
+          path: 'Gemfile.lock',
+          urlPath: 'path/Gemfile.lock',
+          location: {
+            file: 'Gemfile.lock',
+          },
+          links: [{
+            url: 'https://groups.google.com/forum/#!topic/rubyonrails-security/rMTQy4oRCGk',
+          }],
         },
-        links: [{
-          url: 'https://groups.google.com/forum/#!topic/rubyonrails-security/rMTQy4oRCGk',
-        }],
+        status: 'failed',
       });
 
       vm = mountComponentWithStore(Component, {
@@ -190,4 +202,31 @@ describe('Security Reports modal', () => {
       expect(vm.$el.querySelector('.js-link-vulnerabilityFeedbackHelpPath').getAttribute('href')).toEqual('feedbacksHelpPath');
     });
   });
+
+  describe('with a resolved issue', () => {
+    beforeEach(() => {
+      store.dispatch('setModalData', {
+        issue: {
+          tool: 'bundler_audit',
+          message: 'Arbitrary file existence disclosure in Action Pack',
+          url: 'https://groups.google.com/forum/#!topic/rubyonrails-security/rMTQy4oRCGk',
+          cve: 'CVE-2014-9999',
+          file: 'Gemfile.lock',
+          solution: 'upgrade to ~> 3.2.21, ~> 4.0.11.1, ~> 4.0.12, ~> 4.1.7.1, >= 4.1.8',
+          title: 'Arbitrary file existence disclosure in Action Pack',
+          path: 'Gemfile.lock',
+          urlPath: 'path/Gemfile.lock',
+        },
+        status: 'success',
+      });
+
+      vm = mountComponentWithStore(Component, {
+        store,
+      });
+    });
+
+    it('does not display the footer', () => {
+      expect(vm.$el.classList.contains('modal-hide-footer')).toBeTruthy();
+    });
+  });
 });

spec/javascripts/vue_shared/security_reports/components/sast_container_issue_body_spec.js

@@ -15,6 +15,8 @@ describe('sast container issue body', () => {
     vulnerability: 'CVE-2017-11671',
   };
 
+  const status = 'failed';
+
   afterEach(() => {
     vm.$destroy();
   });
@@ -23,6 +25,7 @@ describe('sast container issue body', () => {
     it('renders severity key', () => {
       vm = mountComponent(Component, {
         issue: sastContainerIssue,
+        status,
       });
 
       expect(vm.$el.textContent.trim()).toContain(sastContainerIssue.severity);
@@ -36,6 +39,7 @@ describe('sast container issue body', () => {
 
       vm = mountComponent(Component, {
         issue: issueCopy,
+        status,
       });
 
       expect(vm.$el.textContent.trim()).not.toContain(sastContainerIssue.severity);
@@ -45,6 +49,7 @@ describe('sast container issue body', () => {
   it('renders name', () => {
     vm = mountComponent(Component, {
       issue: sastContainerIssue,
+      status,
     });
 
     expect(vm.$el.querySelector('button').textContent.trim()).toEqual(sastContainerIssue.title);
@@ -54,6 +59,7 @@ describe('sast container issue body', () => {
     it('renders path', () => {
       vm = mountComponent(Component, {
         issue: sastContainerIssue,
+        status,
       });
 
       expect(vm.$el.textContent.trim()).toContain(sastContainerIssue.path);

spec/javascripts/vue_shared/security_reports/components/sast_issue_body_spec.js

@@ -23,6 +23,8 @@ describe('sast issue body', () => {
     confidence: 'Low',
   };
 
+  const status = 'failed';
+
   afterEach(() => {
     vm.$destroy();
   });
@@ -31,6 +33,7 @@ describe('sast issue body', () => {
     it('renders severity and confidence', () => {
       vm = mountComponent(Component, {
         issue: sastIssue,
+        status,
       });
 
       expect(vm.$el.textContent.trim()).toContain(`${sastIssue.severity} (${sastIssue.confidence}):`);
@@ -43,6 +46,7 @@ describe('sast issue body', () => {
       delete issueCopy.confidence;
       vm = mountComponent(Component, {
         issue: issueCopy,
+        status,
       });
 
       expect(vm.$el.textContent.trim()).toContain(`${issueCopy.severity}:`);
@@ -55,6 +59,7 @@ describe('sast issue body', () => {
       delete issueCopy.severity;
       vm = mountComponent(Component, {
         issue: issueCopy,
+        status,
       });
 
       expect(vm.$el.textContent.trim()).toContain(`(${issueCopy.confidence}):`);
@@ -69,6 +74,7 @@ describe('sast issue body', () => {
       issueCopy.priority = 'Low';
       vm = mountComponent(Component, {
         issue: issueCopy,
+        status,
       });
 
       expect(vm.$el.textContent.trim()).toContain(issueCopy.priority);
@@ -83,6 +89,7 @@ describe('sast issue body', () => {
 
       vm = mountComponent(Component, {
         issue: issueCopy,
+        status,
       });
 
       expect(vm.$el.textContent.trim()).not.toContain(
@@ -95,6 +102,7 @@ describe('sast issue body', () => {
     it('renders title', () => {
       vm = mountComponent(Component, {
         issue: sastIssue,
+        status,
       });
 
       expect(vm.$el.textContent.trim()).toContain(
@@ -107,6 +115,7 @@ describe('sast issue body', () => {
     it('renders path', () => {
       vm = mountComponent(Component, {
         issue: sastIssue,
+        status,
       });
 
       expect(vm.$el.querySelector('a').getAttribute('href')).toEqual(

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

@@ -977,13 +977,13 @@ describe('security reports actions', () => {
     it('dispatches setModalData action', done => {
       testAction(
         openModal,
-        { id: 1 },
+        { issue: { id: 1 }, status: 'failed' },
         mockedState,
         [],
         [
           {
             type: 'setModalData',
-            payload: { id: 1 },
+            payload: { issue: { id: 1 }, status: 'failed' },
           },
         ],
         done,
@@ -995,12 +995,12 @@ describe('security reports actions', () => {
     it('commits set issue modal data', done => {
       testAction(
         setModalData,
-        { id: 1 },
+        { issue: { id: 1 }, status: 'success' },
         mockedState,
         [
           {
             type: types.SET_ISSUE_MODAL_DATA,
-            payload: { id: 1 },
+            payload: { issue: { id: 1 }, status: 'success' },
           },
         ],
         [],

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

@@ -400,8 +400,9 @@ describe('security reports mutations', () => {
         }],
         isDismissed: true,
       };
+      const status = 'success';
 
-      mutations[types.SET_ISSUE_MODAL_DATA](stateCopy, issue);
+      mutations[types.SET_ISSUE_MODAL_DATA](stateCopy, { issue, status });
 
       expect(stateCopy.modal.title).toEqual(issue.title);
       expect(stateCopy.modal.data.description.value).toEqual(issue.description);
@@ -417,6 +418,7 @@ describe('security reports mutations', () => {
       expect(stateCopy.modal.data.links.value).toEqual(issue.links);
       expect(stateCopy.modal.data.instances.value).toEqual(issue.instances);
       expect(stateCopy.modal.vulnerability).toEqual(issue);
+      expect(stateCopy.modal.isResolved).toEqual(true);
     });
   });