Merge branch...

Merge branch '212497-update-data-sent-to-vulnerabilities-page-to-include-less-information-and-use-the-new-data' into 'master'

Send less data to Vulnerabilites page

See merge request gitlab-org/gitlab!31616

ee/app/assets/javascripts/pages/projects/security/vulnerabilities/show/index.js

 import Vue from 'vue';
+import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils';
 import HeaderApp from 'ee/vulnerabilities/components/header.vue';
 import DetailsApp from 'ee/vulnerabilities/components/details.vue';
 import FooterApp from 'ee/vulnerabilities/components/footer.vue';
+import { VULNERABILITY_STATE_OBJECTS } from 'ee/vulnerabilities/constants';
 
 function createHeaderApp() {
   const el = document.getElementById('js-vulnerability-header');
-  const initialVulnerability = JSON.parse(el.dataset.vulnerabilityJson);
-  const pipeline = JSON.parse(el.dataset.pipelineJson);
-  const finding = JSON.parse(el.dataset.findingJson);
-
-  const { projectFingerprint, createIssueUrl, createMrUrl } = el.dataset;
+  const vulnerability = JSON.parse(el.dataset.vulnerability);
 
   return new Vue({
     el,
@@ -17,12 +15,7 @@ function createHeaderApp() {
     render: h =>
       h(HeaderApp, {
         props: {
-          createMrUrl,
-          initialVulnerability,
-          finding,
-          pipeline,
-          projectFingerprint,
-          createIssueUrl,
+          initialVulnerability: vulnerability,
         },
       }),
   });
@@ -30,12 +23,11 @@ function createHeaderApp() {
 
 function createDetailsApp() {
   const el = document.getElementById('js-vulnerability-details');
-  const vulnerability = JSON.parse(el.dataset.vulnerabilityJson);
-  const finding = JSON.parse(el.dataset.findingJson);
+  const vulnerability = JSON.parse(el.dataset.vulnerability);
 
   return new Vue({
     el,
-    render: h => h(DetailsApp, { props: { vulnerability, finding } }),
+    render: h => h(DetailsApp, { props: { vulnerability } }),
   });
 }
 
@@ -46,25 +38,43 @@ function createFooterApp() {
     return false;
   }
 
-  const { vulnerabilityFeedbackHelpPath, hasMr, discussionsUrl, notesUrl } = el.dataset;
-  const vulnerability = JSON.parse(el.dataset.vulnerabilityJson);
-  const finding = JSON.parse(el.dataset.findingJson);
+  const {
+    vulnerabilityFeedbackHelpPath,
+    hasMr,
+    discussionsUrl,
+    state,
+    issueFeedback,
+    mergeRequestFeedback,
+    notesUrl,
+    project,
+    remediations,
+    solution,
+  } = convertObjectPropsToCamelCase(JSON.parse(el.dataset.vulnerability));
+
+  const remediation = remediations?.length ? remediations[0] : null;
   const hasDownload = Boolean(
-    vulnerability.state !== 'resolved' && finding.remediation?.diff?.length && !hasMr,
+    state !== VULNERABILITY_STATE_OBJECTS.resolved.state && remediation?.diff?.length && !hasMr,
   );
+  const hasRemediation = Boolean(remediation);
 
   const props = {
     discussionsUrl,
     notesUrl,
-    finding,
     solutionInfo: {
-      solution: finding.solution,
-      remediation: finding.remediation,
+      solution,
+      remediation,
       hasDownload,
       hasMr,
+      hasRemediation,
       vulnerabilityFeedbackHelpPath,
       isStandaloneVulnerability: true,
     },
+    issueFeedback,
+    mergeRequestFeedback,
+    project: {
+      url: project.full_path,
+      value: project.full_name,
+    },
   };
 
   return new Vue({

ee/app/assets/javascripts/vulnerabilities/components/details.vue

@@ -11,17 +11,13 @@ export default {
       type: Object,
       required: true,
     },
-    finding: {
-      type: Object,
-      required: true,
-    },
   },
   computed: {
     location() {
-      return this.finding.location || {};
+      return this.vulnerability.location || {};
     },
     scanner() {
-      return this.finding.scanner || {};
+      return this.vulnerability.scanner || {};
     },
     fileText() {
       return (this.location.file || '') + (this.lineNumber ? `:${this.lineNumber}` : '');
@@ -57,19 +53,27 @@ export default {
 </script>
 
 <template>
-  <div class="md">
-    <h1 class="mt-3 mb-2 border-bottom-0" data-testid="title">{{ vulnerability.title }}</h1>
+  <div class="md" data-qa-selector="vulnerability_details">
+    <h1
+      class="mt-3 mb-2 border-bottom-0"
+      data-testid="title"
+      data-qa-selector="vulnerability_title"
+    >
+      {{ vulnerability.title }}
+    </h1>
     <h3 class="mt-0">{{ __('Description') }}</h3>
-    <p data-testid="description">{{ finding.description }}</p>
+    <p data-testid="description" data-qa-selector="vulnerability_description">
+      {{ vulnerability.description }}
+    </p>
 
     <ul>
       <detail-item :sprintf-message="__('%{labelStart}Severity:%{labelEnd} %{severity}')">
         <severity-badge :severity="vulnerability.severity" class="gl-display-inline ml-1" />
       </detail-item>
       <detail-item
-        v-if="finding.evidence"
+        v-if="vulnerability.evidence"
         :sprintf-message="__('%{labelStart}Evidence:%{labelEnd} %{evidence}')"
-        >{{ finding.evidence }}
+        >{{ vulnerability.evidence }}
       </detail-item>
       <detail-item :sprintf-message="__('%{labelStart}Report Type:%{labelEnd} %{reportType}')"
         >{{ vulnerability.report_type }}
@@ -125,10 +129,10 @@ export default {
       </ul>
     </template>
 
-    <template v-if="finding.links && finding.links.length">
+    <template v-if="vulnerability.links && vulnerability.links.length">
       <h3>{{ __('Links') }}</h3>
       <ul>
-        <li v-for="link in finding.links" :key="link.url">
+        <li v-for="link in vulnerability.links" :key="link.url">
           <gl-link
             :href="link.url"
             data-testid="link"
@@ -142,10 +146,10 @@ export default {
       </ul>
     </template>
 
-    <template v-if="finding.identifiers && finding.identifiers.length">
+    <template v-if="vulnerability.identifiers && vulnerability.identifiers.length">
       <h3>{{ __('Identifiers') }}</h3>
       <ul>
-        <li v-for="identifier in finding.identifiers" :key="identifier.url">
+        <li v-for="identifier in vulnerability.identifiers" :key="identifier.url">
           <gl-link :href="identifier.url" data-testid="identifier" target="_blank">
             {{ identifier.name }}
           </gl-link>

ee/app/assets/javascripts/vulnerabilities/components/footer.vue

@@ -22,7 +22,7 @@ export default {
       type: String,
       required: true,
     },
-    finding: {
+    project: {
       type: Object,
       required: true,
     },
@@ -30,6 +30,16 @@ export default {
       type: Object,
       required: true,
     },
+    issueFeedback: {
+      type: Object,
+      required: false,
+      default: () => null,
+    },
+    mergeRequestFeedback: {
+      type: Object,
+      required: false,
+      default: () => null,
+    },
   },
 
   data: () => ({
@@ -52,12 +62,6 @@ export default {
     hasSolution() {
       return Boolean(this.solutionInfo.solution || this.solutionInfo.remediation);
     },
-    project() {
-      return {
-        url: this.finding.project?.full_path,
-        value: this.finding.project?.full_name,
-      };
-    },
   },
 
   created() {
@@ -153,19 +157,19 @@ export default {
 };
 </script>
 <template>
-  <div>
+  <div data-qa-selector="vulnerability_footer">
     <solution-card v-if="hasSolution" v-bind="solutionInfo" />
 
-    <div v-if="finding.issue_feedback || finding.merge_request_feedback" class="card">
+    <div v-if="issueFeedback || mergeRequestFeedback" class="card">
       <issue-note
-        v-if="finding.issue_feedback"
-        :feedback="finding.issue_feedback"
+        v-if="issueFeedback"
+        :feedback="issueFeedback"
         :project="project"
         class="card-body"
       />
       <merge-request-note
-        v-if="finding.merge_request_feedback"
-        :feedback="finding.merge_request_feedback"
+        v-if="mergeRequestFeedback"
+        :feedback="mergeRequestFeedback"
         :project="project"
         class="card-body"
       />

ee/app/assets/javascripts/vulnerabilities/components/header.vue

@@ -26,30 +26,10 @@ export default {
   },
 
   props: {
-    createMrUrl: {
-      type: String,
-      required: true,
-    },
     initialVulnerability: {
       type: Object,
       required: true,
     },
-    finding: {
-      type: Object,
-      required: true,
-    },
-    pipeline: {
-      type: Object,
-      required: true,
-    },
-    createIssueUrl: {
-      type: String,
-      required: true,
-    },
-    projectFingerprint: {
-      type: String,
-      required: true,
-    },
   },
 
   data() {
@@ -88,16 +68,16 @@ export default {
       );
     },
     hasIssue() {
-      return Boolean(this.finding.issue_feedback?.issue_iid);
+      return Boolean(this.vulnerability.issue_feedback?.issue_iid);
     },
     hasRemediation() {
-      const { remediations } = this.finding;
+      const { remediations } = this.vulnerability;
       return Boolean(remediations && remediations[0]?.diff?.length > 0);
     },
     canCreateMergeRequest() {
       return (
-        !this.finding.merge_request_feedback?.merge_request_path &&
-        Boolean(this.createMrUrl) &&
+        !this.vulnerability.merge_request_feedback?.merge_request_path &&
+        Boolean(this.vulnerability.create_mr_url) &&
         this.hasRemediation
       );
     },
@@ -163,17 +143,23 @@ export default {
     },
     createIssue() {
       this.isProcessingAction = true;
+
+      const {
+        report_type: category,
+        project_fingerprint: projectFingerprint,
+        id,
+      } = this.vulnerability;
+
       axios
-        .post(this.createIssueUrl, {
+        .post(this.vulnerability.create_issue_url, {
           vulnerability_feedback: {
             feedback_type: FEEDBACK_TYPES.ISSUE,
-            category: this.vulnerability.report_type,
-            project_fingerprint: this.projectFingerprint,
+            category,
+            project_fingerprint: projectFingerprint,
             vulnerability_data: {
               ...this.vulnerability,
-              ...this.finding,
-              category: this.vulnerability.report_type,
-              vulnerability_id: this.vulnerability.id,
+              category,
+              vulnerability_id: id,
             },
           },
         })
@@ -189,17 +175,23 @@ export default {
     },
     createMergeRequest() {
       this.isProcessingAction = true;
+
+      const {
+        report_type: category,
+        pipeline: { sourceBranch },
+        project_fingerprint: projectFingerprint,
+      } = this.vulnerability;
+
       axios
-        .post(this.createMrUrl, {
+        .post(this.vulnerability.create_mr_url, {
           vulnerability_feedback: {
             feedback_type: FEEDBACK_TYPES.MERGE_REQUEST,
-            category: this.vulnerability.report_type,
-            project_fingerprint: this.projectFingerprint,
+            category,
+            project_fingerprint: projectFingerprint,
             vulnerability_data: {
               ...this.vulnerability,
-              ...this.finding,
-              category: this.vulnerability.report_type,
-              target_branch: this.pipeline.sourceBranch,
+              category,
+              target_branch: sourceBranch,
             },
           },
         })
@@ -214,14 +206,17 @@ export default {
         });
     },
     downloadPatch() {
-      download({ fileData: this.finding.remediations[0].diff, fileName: `remediation.patch` });
+      download({
+        fileData: this.vulnerability.remediations[0].diff,
+        fileName: `remediation.patch`,
+      });
     },
   },
 };
 </script>
 
 <template>
-  <div>
+  <div data-qa-selector="vulnerability_header">
     <resolution-alert
       v-if="showResolutionAlert"
       :vulnerability-id="vulnerability.id"
@@ -243,7 +238,6 @@ export default {
         <status-description
           class="issuable-meta"
           :vulnerability="vulnerability"
-          :pipeline="pipeline"
           :user="user"
           :is-loading-vulnerability="isLoadingVulnerability"
           :is-loading-user="isLoadingUser"

ee/app/assets/javascripts/vulnerabilities/components/status_description.vue

@@ -19,10 +19,6 @@ export default {
       type: Object,
       required: true,
     },
-    pipeline: {
-      type: Object,
-      required: true,
-    },
     user: {
       type: Object,
       required: false,
@@ -42,7 +38,7 @@ export default {
     time() {
       const { state } = this.vulnerability;
       return state === 'detected'
-        ? this.pipeline.created_at
+        ? this.vulnerability.pipeline.created_at
         : this.vulnerability[`${this.vulnerability.state}_at`];
     },
 
@@ -86,9 +82,9 @@ export default {
           img-css-classes="avatar-inline"
         />
       </template>
-      <template v-if="pipeline" #pipelineLink>
-        <gl-link :href="pipeline.url" target="_blank" class="link">
-          {{ pipeline.id }}
+      <template v-if="vulnerability.pipeline" #pipelineLink>
+        <gl-link :href="vulnerability.pipeline.url" target="_blank" class="link">
+          {{ vulnerability.pipeline.id }}
         </gl-link>
       </template>
     </gl-sprintf>

ee/app/assets/javascripts/vulnerabilities/components/vulnerability_list.vue

@@ -199,7 +199,12 @@ export default {
 
       <template #cell(title)="{ item }">
         <div class="d-flex flex-column flex-sm-row align-items-end align-items-sm-start">
-          <gl-link class="text-body js-description" :href="item.vulnerabilityPath">
+          <gl-link
+            class="text-body js-description"
+            :href="item.vulnerabilityPath"
+            data-qa-selector="vulnerability"
+            :data-qa-vulnerability-description="`${item.title}`"
+          >
             {{ item.title }}
           </gl-link>
           <issue-link v-if="issue(item)" :issue="issue(item)" />

ee/app/helpers/vulnerabilities_helper.rb

 # frozen_string_literal: true
 
 module VulnerabilitiesHelper
-  def vulnerability_data(vulnerability, pipeline)
+  def vulnerability_details_json(vulnerability, pipeline)
+    vulnerability_details(vulnerability, pipeline).to_json
+  end
+
+  def vulnerability_details(vulnerability, pipeline)
     return unless vulnerability
 
-    {
-      vulnerability_json: VulnerabilitySerializer.new.represent(vulnerability).to_json,
-      project_fingerprint: vulnerability.finding.project_fingerprint,
+    result = {
+      timestamp: Time.now.to_i,
       create_issue_url: create_vulnerability_feedback_issue_path(vulnerability.finding.project),
-      notes_url: project_security_vulnerability_notes_path(vulnerability.project, vulnerability),
-      discussions_url: discussions_project_security_vulnerability_path(vulnerability.project, vulnerability),
-      pipeline_json: vulnerability_pipeline_data(pipeline).to_json,
       has_mr: !!vulnerability.finding.merge_request_feedback.try(:merge_request_iid),
-      vulnerability_feedback_help_path: help_page_path('user/application_security/index', anchor: 'interacting-with-the-vulnerabilities'),
-      finding_json: vulnerability_finding_data(vulnerability).to_json,
       create_mr_url: create_vulnerability_feedback_merge_request_path(vulnerability.finding.project),
-      timestamp: Time.now.to_i
+      discussions_url: discussions_project_security_vulnerability_path(vulnerability.project, vulnerability),
+      notes_url: project_security_vulnerability_notes_path(vulnerability.project, vulnerability),
+      vulnerability_feedback_help_path: help_page_path('user/application_security/index', anchor: 'interacting-with-the-vulnerabilities'),
+      pipeline: vulnerability_pipeline_data(pipeline)
     }
+
+    result.merge(vulnerability_data(vulnerability), vulnerability_finding_data(vulnerability))
   end
 
   def vulnerability_pipeline_data(pipeline)
@@ -30,9 +33,12 @@ module VulnerabilitiesHelper
     }
   end
 
+  def vulnerability_data(vulnerability)
+    VulnerabilitySerializer.new.represent(vulnerability)
+  end
+
   def vulnerability_finding_data(vulnerability)
     finding = Vulnerabilities::FindingSerializer.new(current_user: current_user).represent(vulnerability.finding)
-    remediation = finding[:remediations]&.first
 
     data = finding.slice(
       :description,
@@ -43,11 +49,11 @@ module VulnerabilitiesHelper
       :issue_feedback,
       :merge_request_feedback,
       :project,
+      :project_fingerprint,
       :remediations,
       :evidence,
-      :scanner
-    ).merge(
-      solution: remediation ? remediation['summary'] : finding[:solution]
+      :scanner,
+      :solution
     )
 
     if data[:location]['file']

ee/app/views/projects/security/vulnerabilities/show.html.haml

@@ -3,8 +3,8 @@
 - breadcrumb_title @vulnerability.id
 - page_title @vulnerability.title
 - page_description @vulnerability.description
-- vulnerability_data = vulnerability_data(@vulnerability, @pipeline)
+- vulnerability_init_details = { vulnerability: vulnerability_details_json(@vulnerability, @pipeline)}
 
-#js-vulnerability-header{ data: vulnerability_data }
-#js-vulnerability-details{ data: vulnerability_data }
-#js-vulnerability-footer{ data: vulnerability_data }
+#js-vulnerability-header{ data: vulnerability_init_details }
+#js-vulnerability-details{ data: vulnerability_init_details }
+#js-vulnerability-footer{ data: vulnerability_init_details }

ee/spec/frontend/vulnerabilities/details_spec.js

@@ -11,16 +11,12 @@ describe('Vulnerability Details', () => {
     severity: 'bad severity',
     confidence: 'high confidence',
     report_type: 'nice report_type',
+    description: 'vulnerability description',
   };
 
-  const finding = {
-    description: 'finding description',
-  };
-
-  const createWrapper = findingOverrides => {
+  const createWrapper = vulnerabilityOverrides => {
     const propsData = {
-      vulnerability,
-      finding: { ...finding, ...findingOverrides },
+      vulnerability: { ...vulnerability, ...vulnerabilityOverrides },
     };
 
     wrapper = mount(VulnerabilityDetails, { propsData });
@@ -37,7 +33,7 @@ describe('Vulnerability Details', () => {
   it('shows the properties that should always be shown', () => {
     createWrapper();
     expect(getText('title')).toBe(vulnerability.title);
-    expect(getText('description')).toBe(finding.description);
+    expect(getText('description')).toBe(vulnerability.description);
     expect(wrapper.find(SeverityBadge).props('severity')).toBe(vulnerability.severity);
     expect(getText('reportType')).toBe(`Report Type: ${vulnerability.report_type}`);
 
@@ -62,12 +58,12 @@ describe('Vulnerability Details', () => {
     expect(getText('namespace')).toBe(`Namespace: linux`);
   });
 
-  it('shows the finding class if it exists', () => {
+  it('shows the vulnerability class if it exists', () => {
     createWrapper({ location: { file: 'file', class: 'class name' } });
     expect(getText('class')).toBe(`Class: class name`);
   });
 
-  it('shows the finding method if it exists', () => {
+  it('shows the vulnerability method if it exists', () => {
     createWrapper({ location: { file: 'file', method: 'method name' } });
     expect(getText('method')).toBe(`Method: method name`);
   });
@@ -89,7 +85,7 @@ describe('Vulnerability Details', () => {
     });
   });
 
-  it('shows the finding identifiers if they exist', () => {
+  it('shows the vulnerability identifiers if they exist', () => {
     createWrapper({
       identifiers: [{ url: '0', name: '00' }, { url: '1', name: '11' }, { url: '2', name: '22' }],
     });

ee/spec/frontend/vulnerabilities/footer_spec.js

@@ -29,11 +29,10 @@ describe('Vulnerability Footer', () => {
     },
     finding: {},
     notesUrl: '/notes',
-  };
-
-  const project = {
+    project: {
       full_path: '/root/security-reports',
       full_name: 'Administrator / Security Reports',
+    },
   };
 
   const solutionInfoProp = {
@@ -82,21 +81,17 @@ describe('Vulnerability Footer', () => {
 
   describe.each`
     type               | prop                      | component
-    ${'issue'}         | ${'issue_feedback'}         | ${IssueNote}
-    ${'merge request'} | ${'merge_request_feedback'} | ${MergeRequestNote}
+    ${'issue'}         | ${'issueFeedback'}        | ${IssueNote}
+    ${'merge request'} | ${'mergeRequestFeedback'} | ${MergeRequestNote}
   `('$type note', ({ prop, component }) => {
     // The object itself does not matter, we just want to make sure it's passed to the issue note.
     const feedback = {};
 
     it('shows issue note when an issue exists for the vulnerability', () => {
-      createWrapper({ ...minimumProps, finding: { project, [prop]: feedback } });
+      createWrapper({ ...minimumProps, [prop]: feedback });
       expect(wrapper.contains(component)).toBe(true);
       expect(wrapper.find(component).props()).toMatchObject({
         feedback,
-        project: {
-          url: project.full_path,
-          value: project.full_name,
-        },
       });
     });
 

ee/spec/frontend/vulnerabilities/status_description_spec.js

@@ -27,32 +27,30 @@ describe('Vulnerability status description component', () => {
   const createDate = value => (value ? new Date(value) : new Date()).toISOString();
 
   const createWrapper = ({
-    vulnerability = {},
-    pipeline = {},
+    vulnerability = { pipeline: {} },
     user,
     isLoadingVulnerability = false,
     isLoadingUser = false,
   } = {}) => {
     const v = vulnerability;
-    const p = pipeline;
 
     // Automatically create the ${v.state}_at property if it doesn't exist. Otherwise, every test would need to create
     // it manually for the component to mount properly.
     if (v.state === 'detected') {
-      p.created_at = p.created_at || createDate();
+      v.pipeline.created_at = v.pipeline.created_at || createDate();
     } else {
       const propertyName = `${v.state}_at`;
       v[propertyName] = v[propertyName] || createDate();
     }
 
     wrapper = mount(StatusText, {
-      propsData: { vulnerability, pipeline, user, isLoadingVulnerability, isLoadingUser },
+      propsData: { vulnerability, user, isLoadingVulnerability, isLoadingUser },
     });
   };
 
   describe('state text', () => {
     it.each(ALL_STATES)('shows the correct string for the vulnerability state "%s"', state => {
-      createWrapper({ vulnerability: { state } });
+      createWrapper({ vulnerability: { state, pipeline: {} } });
 
       expect(wrapper.text()).toMatch(new RegExp(`^${capitalize(state)}`));
     });
@@ -62,8 +60,7 @@ describe('Vulnerability status description component', () => {
     it('uses the pipeline created date when the vulnerability state is "detected"', () => {
       const pipelineDateString = createDate('2001');
       createWrapper({
-        vulnerability: { state: 'detected' },
-        pipeline: { created_at: pipelineDateString },
+        vulnerability: { state: 'detected', pipeline: { created_at: pipelineDateString } },
       });
 
       expect(timeAgo().props('time')).toBe(pipelineDateString);
@@ -75,8 +72,11 @@ describe('Vulnerability status description component', () => {
       state => {
         const expectedDate = createDate();
         createWrapper({
-          vulnerability: { state, [`${state}_at`]: expectedDate },
+          vulnerability: {
+            state,
             pipeline: { created_at: 'pipeline_created_at' },
+            [`${state}_at`]: expectedDate,
+          },
         });
 
         expect(timeAgo().props('time')).toBe(expectedDate);
@@ -87,8 +87,7 @@ describe('Vulnerability status description component', () => {
   describe('pipeline link', () => {
     it('shows the pipeline link when the vulnerability state is "detected"', () => {
       createWrapper({
-        vulnerability: { state: 'detected' },
-        pipeline: { url: 'pipeline/url' },
+        vulnerability: { state: 'detected', pipeline: { url: 'pipeline/url' } },
       });
 
       expect(pipelineLink().attributes('href')).toBe('pipeline/url');
@@ -98,8 +97,7 @@ describe('Vulnerability status description component', () => {
       'does not show the pipeline link when the vulnerability state is "%s"',
       state => {
         createWrapper({
-          vulnerability: { state },
-          pipeline: { url: 'pipeline/url' },
+          vulnerability: { state, pipeline: { url: 'pipeline/url' } },
         });
 
         expect(pipelineLink().exists()).toBe(false); // The user avatar should be shown instead, those tests are handled separately.
@@ -152,7 +150,7 @@ describe('Vulnerability status description component', () => {
     });
 
     it('hides the skeleton loader and shows everything else when the vulnerability is not loading', () => {
-      createWrapper({ vulnerability: { state: 'detected' } });
+      createWrapper({ vulnerability: { state: 'detected', pipeline: {} } });
 
       expect(skeletonLoader().exists()).toBe(false);
       expect(timeAgo().exists()).toBe(true);

ee/spec/helpers/vulnerabilities_helper_spec.rb

@@ -58,35 +58,31 @@ RSpec.describe VulnerabilitiesHelper do
 
     it 'has expected vulnerability properties' do
       expect(subject).to include(
-        vulnerability_json: kind_of(String),
-        project_fingerprint: vulnerability.finding.project_fingerprint,
+        timestamp: Time.now.to_i,
         create_issue_url: "/#{project.full_path}/-/vulnerability_feedback",
-        notes_url: "/#{project.full_path}/-/security/vulnerabilities/#{vulnerability.id}/notes",
-        discussions_url: "/#{project.full_path}/-/security/vulnerabilities/#{vulnerability.id}/discussions",
         has_mr: anything,
-        vulnerability_feedback_help_path: kind_of(String),
-        finding_json: kind_of(String),
         create_mr_url: "/#{project.full_path}/-/vulnerability_feedback",
-        timestamp: Time.now.to_i
+        discussions_url: "/#{project.full_path}/-/security/vulnerabilities/#{vulnerability.id}/discussions",
+        notes_url: "/#{project.full_path}/-/security/vulnerabilities/#{vulnerability.id}/notes",
+        vulnerability_feedback_help_path: kind_of(String),
+        pipeline: anything
       )
     end
   end
 
-  describe '#vulnerability_data' do
-    subject { helper.vulnerability_data(vulnerability, pipeline) }
+  describe '#vulnerability_details' do
+    subject { helper.vulnerability_details(vulnerability, pipeline) }
 
     describe 'when pipeline exists' do
       let(:pipeline) { create(:ci_pipeline) }
-      let(:pipelineData) { Gitlab::Json.parse(subject[:pipeline_json]) }
 
       include_examples 'vulnerability properties'
 
       it 'returns expected pipeline data' do
-        expect(pipelineData).to include(
-          'id' => pipeline.id,
-          'created_at' => pipeline.created_at.iso8601,
-          'url' => be_present,
-          'source_branch' => pipeline.ref
+        expect(subject[:pipeline]).to include(
+          id: pipeline.id,
+          created_at: pipeline.created_at.iso8601,
+          url: be_present
         )
       end
     end
@@ -110,11 +106,12 @@ RSpec.describe VulnerabilitiesHelper do
         description: finding.description,
         identifiers: kind_of(Array),
         issue_feedback: anything,
-        merge_request_feedback: anything,
         links: finding.links,
         location: finding.location,
         name: finding.name,
+        merge_request_feedback: anything,
         project: kind_of(Grape::Entity::Exposure::NestingExposure::OutputBuilder),
+        project_fingerprint: finding.project_fingerprint,
         remediations: finding.remediations,
         solution: kind_of(String),
         evidence: kind_of(String),

qa/qa/ee.rb

@@ -153,6 +153,8 @@ module QA
         module Secure
           autoload :Show, 'qa/ee/page/project/secure/show'
           autoload :DependencyList, 'qa/ee/page/project/secure/dependency_list'
+          autoload :SecurityDashboard, 'qa/ee/page/project/secure/security_dashboard'
+          autoload :VulnerabilityDetails, 'qa/ee/page/project/secure/vulnerability_details'
         end
 
         module PathLocks

qa/qa/ee/fixtures/secure_premade_reports/gl-container-scanning-report.json

@@ -4,7 +4,7 @@
     {
       "category": "container_scanning",
       "message": "CVE-2017-18269 in glibc",
-      "description": "An SSE2-optimized memmove implementation for i386 in sysdeps/i386/i686/multiarch/memcpy-sse2-unaligned.S in the GNU C Library (aka glibc or libc6) 2.21 through 2.27 does not correctly perform the overlapping memory check if the source memory range spans the middle of the address space, resulting in corrupt data being produced by the copy operation. This may disclose information to context-dependent attackers, or result in a denial of service, or, possibly, code execution.",
+      "description": "Short description to match in specs",
       "cve": "debian:9:glibc:CVE-2017-18269",
       "severity": "Critical",
       "confidence": "Unknown",

qa/qa/ee/page/project/secure/security_dashboard.rb

+# frozen_string_literal: true
+
+module QA
+  module EE
+    module Page
+      module Project
+        module Secure
+          class SecurityDashboard < QA::Page::Base
+            view 'ee/app/assets/javascripts/vulnerabilities/components/vulnerability_list.vue' do
+              element :vulnerability
+            end
+
+            def has_vulnerability?(description:)
+              has_element?(:vulnerability, vulnerability_description: description)
+            end
+
+            def click_vulnerability(description:)
+              return false unless has_vulnerability?(description: description)
+
+              click_element(:vulnerability, vulnerability_description: description)
+            end
+          end
+        end
+      end
+    end
+  end
+end

qa/qa/ee/page/project/secure/vulnerability_details.rb

+# frozen_string_literal: true
+
+module QA
+  module EE
+    module Page
+      module Project
+        module Secure
+          class VulnerabilityDetails < QA::Page::Base
+            view 'ee/app/assets/javascripts/vulnerabilities/components/header.vue' do
+              element :vulnerability_header
+            end
+
+            view 'ee/app/assets/javascripts/vulnerabilities/components/details.vue' do
+              element :vulnerability_details
+              element :vulnerability_title
+              element :vulnerability_description
+            end
+
+            view 'ee/app/assets/javascripts/vulnerabilities/components/footer.vue' do
+              element :vulnerability_footer
+            end
+
+            def has_component?(component_name:)
+              has_element?(component_name.to_sym)
+            end
+
+            def has_vulnerability_title?(title:)
+              has_element?(:vulnerability_title, text: title)
+            end
+
+            def has_vulnerability_description?(description:)
+              has_element?(:vulnerability_description, text: description)
+            end
+          end
+        end
+      end
+    end
+  end
+end

qa/qa/resource/repository/commit.rb

@@ -29,6 +29,30 @@ module QA
           @add_files = files
         end
 
+        def add_directory(dir)
+          raise "Must set directory as a Pathname" unless dir.is_a?(Pathname)
+
+          files_to_add = []
+
+          dir.each_child do |child|
+            case child.ftype?
+            when "file"
+              files_to_add.append({
+                file_path: child.to_s,
+                content: child.read
+              })
+            when "directory"
+              add_directory(child)
+            else
+              continue
+            end
+          end
+
+          validate_files!(files_to_add)
+
+          @add_files.merge(files_to_add)
+        end
+
         def update_files(files)
           validate_files!(files)
 

qa/qa/specs/features/ee/browser_ui/secure/first_class_project_security_dashboard_spec.rb

+# frozen_string_literal: true
+
+module QA
+  context 'Secure', :docker, :runner do
+    describe 'Security Dashboard in a Project' do
+      let(:vulnerability_name) { "CVE-2017-18269 in glibc" }
+      let(:vulnerability_description) { "Short description to match in specs" }
+
+      before(:all) do
+        @executor = "qa-runner-#{Time.now.to_i}"
+
+        Flow::Login.sign_in
+
+        @project = Resource::Project.fabricate_via_api! do |p|
+          p.name = Runtime::Env.auto_devops_project_name || 'project-with-secure'
+          p.description = 'Project with Secure'
+          p.auto_devops_enabled = false
+          p.initialize_with_readme = true
+        end
+
+        @runner = Resource::Runner.fabricate! do |runner|
+          runner.project = @project
+          runner.name = @executor
+          runner.tags = %w[qa test]
+        end
+
+        # Push fixture to generate Secure reports
+        @source = Resource::Repository::ProjectPush.fabricate! do |push|
+          push.project = @project
+          push.directory = Pathname
+            .new(__dir__)
+            .join('../../../../../ee/fixtures/secure_premade_reports')
+          push.commit_message = 'Create Secure compatible application to serve premade reports'
+          push.branch_name = 'secure-mr'
+        end
+
+        @merge_request = Resource::MergeRequest.fabricate_via_api! do |mr|
+          mr.project = @project
+          mr.source_branch = 'secure-mr'
+          mr.target_branch = 'master'
+          mr.source = @source
+          mr.target = 'master'
+          mr.target_new_branch = false
+        end
+
+        @merge_request.visit!
+        Page::MergeRequest::Show.perform do |merge_request|
+          merge_request.merge!
+        end
+        Page::Project::Menu.perform(&:click_ci_cd_pipelines)
+        Page::Project::Pipeline::Index.perform(&:wait_for_latest_pipeline_success)
+      end
+
+      after(:all) do
+        @runner.remove_via_api!
+      end
+
+      it 'shows vulnerability details', quarantine: { type: :investigating } do
+        @project.visit!
+
+        Page::Project::Menu.perform(&:click_on_security_dashboard)
+
+        EE::Page::Project::Secure::SecurityDashboard.perform do |security_dashboard|
+          expect(security_dashboard).to have_vulnerability(description: vulnerability_name)
+          security_dashboard.click_vulnerability(description: vulnerability_name)
+        end
+
+        EE::Page::Project::Secure::VulnerabilityDetails.perform do |vulnerability_details|
+          expect(vulnerability_details).to have_component(component_name: :vulnerability_header)
+          expect(vulnerability_details).to have_component(component_name: :vulnerability_details)
+          expect(vulnerability_details).to have_vulnerability_title(title: vulnerability_name)
+          expect(vulnerability_details).to have_vulnerability_description(description: vulnerability_description)
+          expect(vulnerability_details).to have_component(component_name: :vulnerability_footer)
+        end
+      end
+    end
+  end
+end

qa/qa/specs/features/ee/browser_ui/secure/vulnerability_management_spec.rb

@@ -25,8 +25,6 @@ module QA
         @project = Resource::Project.fabricate_via_api! do |p|
           p.name = Runtime::Env.auto_devops_project_name || 'project-with-secure'
           p.description = 'Project with Secure'
-          p.auto_devops_enabled = false
-          p.initialize_with_readme = true
         end
 
         @runner = Resource::Runner.fabricate! do |runner|
@@ -35,15 +33,16 @@ module QA
           runner.tags = %w[qa test]
         end
 
-        # Push fixture to generate Secure reports
-        @source = Resource::Repository::ProjectPush.fabricate! do |push|
-          push.project = @project
-          push.directory = Pathname
-            .new(__dir__)
-            .join('../../../../../ee/fixtures/secure_premade_reports')
-          push.commit_message = 'Create Secure compatible application to serve premade reports'
-          push.branch_name = 'secure-mr'
+        commit_message = 'Add premade security reports'
+        @commit = Resource::Repository::Commit.fabricate_via_api! do |commit|
+          commit.project = @project
+          commit.commit_message = commit_message
+          commit.branch = 'secure-mr'
+          commit.add_directory(
+            Pathname.new(__dir__).join('../../../../../ee/fixtures/secure_premade_reports')
+          )
         end
+        @project.wait_for_push(commit_message)
 
         @merge_request = Resource::MergeRequest.fabricate_via_api! do |mr|
           mr.project = @project