Abort rendering of disabled security reports

This MR creates a new EE::MergeRequest#enabled_reports method that
indicates what MR reports are enabled/disabled. We then use this data
to populate the MergeRequestStore in order to abort the rendering of
disabled reports in the MR widget instead of allowing the reports to
show up and eventually error out when the corresponding XHR requests
resolve.

This change only affects reports rendering when the corresponding
*MergeRequestReportApi feature flag is enabled.
parent a4a81455
...@@ -275,6 +275,7 @@ export default { ...@@ -275,6 +275,7 @@ export default {
:head-blob-path="mr.headBlobPath" :head-blob-path="mr.headBlobPath"
:source-branch="mr.sourceBranch" :source-branch="mr.sourceBranch"
:base-blob-path="mr.baseBlobPath" :base-blob-path="mr.baseBlobPath"
:enabled-reports="mr.enabledSecurityReports"
:sast-head-path="mr.sast.head_path" :sast-head-path="mr.sast.head_path"
:sast-base-path="mr.sast.base_path" :sast-base-path="mr.sast.base_path"
:sast-help-path="mr.sastHelp" :sast-help-path="mr.sastHelp"
......
import { securityReportsTypes } from 'ee/vue_shared/security_reports/constants';
import CEMergeRequestStore from '~/vue_merge_request_widget/stores/mr_widget_store'; import CEMergeRequestStore from '~/vue_merge_request_widget/stores/mr_widget_store';
import { mapApprovalsResponse, mapApprovalRulesResponse } from '../mappers'; import { mapApprovalsResponse, mapApprovalRulesResponse } from '../mappers';
import CodeQualityComparisonWorker from '../workers/code_quality_comparison_worker'; import CodeQualityComparisonWorker from '../workers/code_quality_comparison_worker';
...@@ -38,6 +39,15 @@ export default class MergeRequestStore extends CEMergeRequestStore { ...@@ -38,6 +39,15 @@ export default class MergeRequestStore extends CEMergeRequestStore {
this.licenseManagement = data.license_management; this.licenseManagement = data.license_management;
this.metricsReportsPath = data.metrics_reports_path; this.metricsReportsPath = data.metrics_reports_path;
const enabledReports = data.enabled_reports || {};
this.enabledSecurityReports = {
[securityReportsTypes.SAST]: Boolean(enabledReports.sast),
[securityReportsTypes.CONTAINER_SCANNING]: Boolean(enabledReports.container_scanning),
[securityReportsTypes.DAST]: Boolean(enabledReports.dast),
[securityReportsTypes.DEPENDENCY_SCANNING]: Boolean(enabledReports.dependency_scanning),
[securityReportsTypes.LICENSE_MANAGEMENT]: Boolean(enabledReports.license_management),
};
this.blockingMergeRequests = data.blocking_merge_requests; this.blockingMergeRequests = data.blocking_merge_requests;
this.hasApprovalsAvailable = data.has_approvals_available; this.hasApprovalsAvailable = data.has_approvals_available;
......
/* eslint-disable import/prefer-default-export */
export const securityReportsTypes = {
SAST: 'sast',
CONTAINER_SCANNING: 'containerScanning',
DAST: 'dast',
DEPENDENCY_SCANNING: 'dependencyScanning',
LICENSE_MANAGEMENT: 'licenseManagement',
};
...@@ -5,6 +5,7 @@ import ReportSection from '~/reports/components/report_section.vue'; ...@@ -5,6 +5,7 @@ import ReportSection from '~/reports/components/report_section.vue';
import SummaryRow from '~/reports/components/summary_row.vue'; import SummaryRow from '~/reports/components/summary_row.vue';
import IssuesList from '~/reports/components/issues_list.vue'; import IssuesList from '~/reports/components/issues_list.vue';
import Icon from '~/vue_shared/components/icon.vue'; import Icon from '~/vue_shared/components/icon.vue';
import { securityReportsTypes } from './constants';
import IssueModal from './components/modal.vue'; import IssueModal from './components/modal.vue';
import securityReportsMixin from './mixins/security_report_mixin'; import securityReportsMixin from './mixins/security_report_mixin';
import createStore from './store'; import createStore from './store';
...@@ -20,6 +21,11 @@ export default { ...@@ -20,6 +21,11 @@ export default {
}, },
mixins: [securityReportsMixin], mixins: [securityReportsMixin],
props: { props: {
enabledReports: {
type: Object,
required: false,
default: () => ({}),
},
headBlobPath: { headBlobPath: {
type: String, type: String,
required: true, required: true,
...@@ -168,25 +174,22 @@ export default { ...@@ -168,25 +174,22 @@ export default {
securityTab() { securityTab() {
return `${this.pipelinePath}/security`; return `${this.pipelinePath}/security`;
}, },
shouldRenderSastContainer() { hasContainerScanningReports() {
const type = securityReportsTypes.CONTAINER_SCANNING;
if (gon.features && gon.features[`${type}MergeRequestReportApi`]) {
return this.enabledReports[type];
}
const { head, diffEndpoint } = this.sastContainer.paths; const { head, diffEndpoint } = this.sastContainer.paths;
return Boolean(head || diffEndpoint);
return head || diffEndpoint;
}, },
shouldRenderDependencyScanning() { hasDependencyScanningReports() {
const { head, diffEndpoint } = this.dependencyScanning.paths; return this.hasReportsType(securityReportsTypes.DEPENDENCY_SCANNING);
return head || diffEndpoint;
}, },
shouldRenderDast() { hasDastReports() {
const { head, diffEndpoint } = this.dast.paths; return this.hasReportsType(securityReportsTypes.DAST);
return head || diffEndpoint;
}, },
shouldRenderSast() { hasSastReports() {
const { head, diffEndpoint } = this.sast.paths; return this.hasReportsType(securityReportsTypes.SAST);
return head || diffEndpoint;
}, },
}, },
...@@ -209,7 +212,12 @@ export default { ...@@ -209,7 +212,12 @@ export default {
const sastDiffEndpoint = gl && gl.mrWidgetData && gl.mrWidgetData.sast_comparison_path; const sastDiffEndpoint = gl && gl.mrWidgetData && gl.mrWidgetData.sast_comparison_path;
if (gon.features && gon.features.sastMergeRequestReportApi && sastDiffEndpoint) { if (
gon.features &&
gon.features.sastMergeRequestReportApi &&
sastDiffEndpoint &&
this.hasSastReports
) {
this.setSastDiffEndpoint(sastDiffEndpoint); this.setSastDiffEndpoint(sastDiffEndpoint);
this.fetchSastDiff(); this.fetchSastDiff();
} else if (this.sastHeadPath) { } else if (this.sastHeadPath) {
...@@ -227,7 +235,8 @@ export default { ...@@ -227,7 +235,8 @@ export default {
if ( if (
gon.features && gon.features &&
gon.features.containerScanningMergeRequestReportApi && gon.features.containerScanningMergeRequestReportApi &&
sastContainerDiffEndpoint sastContainerDiffEndpoint &&
this.hasContainerScanningReports
) { ) {
this.setSastContainerDiffEndpoint(sastContainerDiffEndpoint); this.setSastContainerDiffEndpoint(sastContainerDiffEndpoint);
this.fetchSastContainerDiff(); this.fetchSastContainerDiff();
...@@ -242,7 +251,12 @@ export default { ...@@ -242,7 +251,12 @@ export default {
const dastDiffEndpoint = gl && gl.mrWidgetData && gl.mrWidgetData.dast_comparison_path; const dastDiffEndpoint = gl && gl.mrWidgetData && gl.mrWidgetData.dast_comparison_path;
if (gon.features && gon.features.dastMergeRequestReportApi && dastDiffEndpoint) { if (
gon.features &&
gon.features.dastMergeRequestReportApi &&
dastDiffEndpoint &&
this.hasDastReports
) {
this.setDastDiffEndpoint(dastDiffEndpoint); this.setDastDiffEndpoint(dastDiffEndpoint);
this.fetchDastDiff(); this.fetchDastDiff();
} else if (this.dastHeadPath) { } else if (this.dastHeadPath) {
...@@ -260,7 +274,8 @@ export default { ...@@ -260,7 +274,8 @@ export default {
if ( if (
gon.features && gon.features &&
gon.features.dependencyScanningMergeRequestReportApi && gon.features.dependencyScanningMergeRequestReportApi &&
dependencyScanningDiffEndpoint dependencyScanningDiffEndpoint &&
this.hasDependencyScanningReports
) { ) {
this.setDependencyScanningDiffEndpoint(dependencyScanningDiffEndpoint); this.setDependencyScanningDiffEndpoint(dependencyScanningDiffEndpoint);
this.fetchDependencyScanningDiff(); this.fetchDependencyScanningDiff();
...@@ -321,6 +336,13 @@ export default { ...@@ -321,6 +336,13 @@ export default {
fetchSastReports: 'fetchReports', fetchSastReports: 'fetchReports',
fetchSastDiff: 'fetchDiff', fetchSastDiff: 'fetchDiff',
}), }),
hasReportsType(type) {
if (gon.features && gon.features[`${type}MergeRequestReportApi`]) {
return this.enabledReports[type];
}
const { head, diffEndpoint } = this[type].paths;
return Boolean(head || diffEndpoint);
},
}, },
}; };
</script> </script>
...@@ -345,7 +367,7 @@ export default { ...@@ -345,7 +367,7 @@ export default {
</div> </div>
<div slot="body" class="mr-widget-grouped-section report-block"> <div slot="body" class="mr-widget-grouped-section report-block">
<template v-if="shouldRenderSast"> <template v-if="hasSastReports">
<summary-row <summary-row
:summary="groupedSastText" :summary="groupedSastText"
:status-icon="sastStatusIcon" :status-icon="sastStatusIcon"
...@@ -364,7 +386,7 @@ export default { ...@@ -364,7 +386,7 @@ export default {
/> />
</template> </template>
<template v-if="shouldRenderDependencyScanning"> <template v-if="hasDependencyScanningReports">
<summary-row <summary-row
:summary="groupedDependencyText" :summary="groupedDependencyText"
:status-icon="dependencyScanningStatusIcon" :status-icon="dependencyScanningStatusIcon"
...@@ -382,7 +404,7 @@ export default { ...@@ -382,7 +404,7 @@ export default {
/> />
</template> </template>
<template v-if="shouldRenderSastContainer"> <template v-if="hasContainerScanningReports">
<summary-row <summary-row
:summary="groupedSastContainerText" :summary="groupedSastContainerText"
:status-icon="sastContainerStatusIcon" :status-icon="sastContainerStatusIcon"
...@@ -400,7 +422,7 @@ export default { ...@@ -400,7 +422,7 @@ export default {
/> />
</template> </template>
<template v-if="shouldRenderDast"> <template v-if="hasDastReports">
<summary-row <summary-row
:summary="groupedDastText" :summary="groupedDastText"
:status-icon="dastStatusIcon" :status-icon="dastStatusIcon"
......
...@@ -134,6 +134,16 @@ module EE ...@@ -134,6 +134,16 @@ module EE
end end
end end
def enabled_reports
{
sast: !!actual_head_pipeline&.batch_lookup_report_artifact_for_file_type(:sast),
container_scanning: !!actual_head_pipeline&.batch_lookup_report_artifact_for_file_type(:container_scanning),
dast: !!actual_head_pipeline&.batch_lookup_report_artifact_for_file_type(:dast),
dependency_scanning: !!actual_head_pipeline&.batch_lookup_report_artifact_for_file_type(:dependency_scanning),
license_management: !!actual_head_pipeline&.batch_lookup_report_artifact_for_file_type(:license_management)
}
end
def has_dependency_scanning_reports? def has_dependency_scanning_reports?
!!(actual_head_pipeline&.has_reports?(::Ci::JobArtifact.dependency_list_reports)) !!(actual_head_pipeline&.has_reports?(::Ci::JobArtifact.dependency_list_reports))
end end
......
...@@ -36,6 +36,10 @@ module EE ...@@ -36,6 +36,10 @@ module EE
end end
end end
expose :enabled_reports do |merge_request|
merge_request.enabled_reports
end
expose :sast, if: -> (mr, _) { head_pipeline_downloadable_path_for_report_type(:sast) } do expose :sast, if: -> (mr, _) { head_pipeline_downloadable_path_for_report_type(:sast) } do
expose :head_path do |merge_request| expose :head_path do |merge_request|
head_pipeline_downloadable_path_for_report_type(:sast) head_pipeline_downloadable_path_for_report_type(:sast)
......
---
title: Abort rendering of security reports that aren't enabled
merge_request: 20381
author:
type: fixed
...@@ -10,6 +10,13 @@ export default Object.assign({}, mockData, { ...@@ -10,6 +10,13 @@ export default Object.assign({}, mockData, {
head_path: 'blob_path', head_path: 'blob_path',
}, },
vulnerability_feedback_help_path: '/help/user/application_security/index', vulnerability_feedback_help_path: '/help/user/application_security/index',
enabled_reports: {
sast: true,
container_scanning: false,
dast: true,
dependency_scanning: false,
license_management: true,
},
}); });
// Codeclimate // Codeclimate
......
...@@ -118,6 +118,38 @@ describe MergeRequest do ...@@ -118,6 +118,38 @@ describe MergeRequest do
end end
end end
describe '#enabled_reports' do
let(:project) { create(:project, :repository) }
where(:report_type, :with_reports) do
:sast | :with_sast_reports
:container_scanning | :with_container_scanning_reports
:dast | :with_dast_reports
:dependency_scanning | :with_dependency_scanning_reports
:license_management | :with_license_management_reports
end
with_them do
subject { merge_request.enabled_reports[report_type] }
before do
stub_licensed_features({ report_type => true })
end
context "when head pipeline has reports" do
let(:merge_request) { create(:ee_merge_request, with_reports, source_project: project) }
it { is_expected.to be_truthy }
end
context "when head pipeline does not have reports" do
let(:merge_request) { create(:ee_merge_request, source_project: project) }
it { is_expected.to be_falsy }
end
end
end
describe '#participant_approvers with approval_rules disabled' do describe '#participant_approvers with approval_rules disabled' do
let!(:approver) { create(:approver, target: project) } let!(:approver) { create(:approver, target: project) }
let(:code_owners) { [double(:code_owner)] } let(:code_owners) { [double(:code_owner)] }
......
...@@ -59,6 +59,37 @@ describe MergeRequestWidgetEntity do ...@@ -59,6 +59,37 @@ describe MergeRequestWidgetEntity do
expect { serializer.represent(merge_request) }.not_to exceed_query_limit(control) expect { serializer.represent(merge_request) }.not_to exceed_query_limit(control)
end end
describe 'enabled_reports' do
it 'marks all reports as disabled by default' do
expect(subject.as_json).to include(:enabled_reports)
expect(subject.as_json[:enabled_reports]).to eq({
sast: false,
container_scanning: false,
dast: false,
dependency_scanning: false,
license_management: false
})
end
it 'marks reports as enabled if artifacts exist' do
allow(merge_request).to receive(:enabled_reports).and_return({
sast: true,
container_scanning: true,
dast: true,
dependency_scanning: true,
license_management: true
})
expect(subject.as_json).to include(:enabled_reports)
expect(subject.as_json[:enabled_reports]).to eq({
sast: true,
container_scanning: true,
dast: true,
dependency_scanning: true,
license_management: true
})
end
end
describe 'test report artifacts', :request_store do describe 'test report artifacts', :request_store do
using RSpec::Parameterized::TableSyntax using RSpec::Parameterized::TableSyntax
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment