Commit e0ad65ab authored by Mark Florian's avatar Mark Florian Committed by Kushal Pandya

Use Security Dashboard in pipelines view

This [replaces][1] the Split Security Reports app with the Security
Dashboard in the pipelines view. This is part of a larger effort to
[move security reports logic to the backend][2].

This is behind the `pipeline_report_api` feature flag, which is
currently disabled by default.

A few other related changes were made:

- Old references to "timeline" were replaced with "history" in specs
- DRY in some specs regarding state initialisation
- Make the history and count endpoint props optional on the dashboard
  component, which, if missing/empty, disable the display of the counts
  and charts components
- Fixes a rendering discrepancy in the vulnerability modal, which was
  displaying a `File: undefined` row

[1]: https://gitlab.com/gitlab-org/gitlab-ee/issues/13496
[2]: https://gitlab.com/groups/gitlab-org/-/epics/1425
parent 563d3d00
import Vue from 'vue';
import Translate from '~/vue_shared/translate';
import createDashboardStore from 'ee/security_dashboard/store';
import SecurityDashboardApp from 'ee/security_dashboard/components/app.vue';
import SecurityReportApp from 'ee/vue_shared/security_reports/split_security_reports_app.vue';
import createStore from 'ee/vue_shared/security_reports/store';
import { updateBadgeCount } from './utils';
Vue.use(Translate);
export default () => {
const securityTab = document.getElementById('js-security-report-app');
const initSecurityDashboardApp = el => {
const {
dashboardDocumentation,
emptyStateSvgPath,
pipelineId,
projectId,
vulnerabilitiesEndpoint,
vulnerabilityFeedbackHelpPath,
} = el.dataset;
// They are being rendered under the same condition
if (securityTab) {
const datasetOptions = securityTab.dataset;
return new Vue({
el,
store: createDashboardStore(),
render(createElement) {
return createElement(SecurityDashboardApp, {
props: {
dashboardDocumentation,
emptyStateSvgPath,
lockToProject: {
id: parseInt(projectId, 10),
},
pipelineId: parseInt(pipelineId, 10),
vulnerabilitiesEndpoint,
vulnerabilityFeedbackHelpPath,
},
on: {
vulnerabilitiesCountChanged(count) {
updateBadgeCount('.js-security-counter', count);
},
},
});
},
});
};
const initSplitSecurityReportsApp = el => {
const datasetOptions = el.dataset;
const {
headBlobPath,
sourceBranch,
......@@ -33,10 +66,8 @@ export default () => {
const store = createStore();
// Tab content
// eslint-disable-next-line no-new
new Vue({
el: securityTab,
return new Vue({
el,
store,
components: {
SecurityReportApp,
......@@ -66,11 +97,22 @@ export default () => {
},
on: {
updateBadgeCount: count => {
updateBadgeCount('.js-sast-counter', count);
updateBadgeCount('.js-security-counter', count);
},
},
});
},
});
};
export default () => {
const securityTab = document.getElementById('js-security-report-app');
if (securityTab) {
if (gon.features && gon.features.pipelineReportApi) {
initSecurityDashboardApp(securityTab);
} else {
initSplitSecurityReportsApp(securityTab);
}
}
};
......@@ -23,7 +23,6 @@ document.addEventListener('DOMContentLoaded', () => {
dashboardDocumentation,
emptyStateSvgPath,
vulnerabilitiesEndpoint,
vulnerabilitiesHistoryEndpoint,
vulnerabilitiesSummaryEndpoint,
vulnerabilityFeedbackHelpPath,
securityDashboardHelpPath,
......@@ -40,7 +39,6 @@ document.addEventListener('DOMContentLoaded', () => {
dashboardDocumentation,
emptyStateSvgPath,
vulnerabilitiesEndpoint,
vulnerabilitiesHistoryEndpoint,
vulnerabilitiesSummaryEndpoint,
vulnerabilityFeedbackHelpPath,
securityDashboardHelpPath,
......@@ -80,7 +78,6 @@ document.addEventListener('DOMContentLoaded', () => {
components: {
SecurityReportApp,
},
methods: {},
render(createElement) {
return createElement('security-report-app', {
props,
......
......@@ -36,11 +36,13 @@ export default {
},
vulnerabilitiesCountEndpoint: {
type: String,
required: true,
required: false,
default: '',
},
vulnerabilitiesHistoryEndpoint: {
type: String,
required: true,
required: false,
default: '',
},
vulnerabilityFeedbackHelpPath: {
type: String,
......@@ -50,7 +52,12 @@ export default {
type: Object,
required: false,
default: null,
validator: project => !isUndefined(project.id) && !isUndefined(project.name),
validator: project => !isUndefined(project.id),
},
pipelineId: {
type: Number,
required: false,
default: null,
},
},
computed: {
......@@ -75,6 +82,15 @@ export default {
isLockedToProject() {
return this.lockToProject !== null;
},
shouldShowChart() {
return Boolean(this.vulnerabilitiesHistoryEndpoint);
},
shouldShowCountList() {
return this.isLockedToProject && Boolean(this.vulnerabilitiesCountEndpoint);
},
},
watch: {
'pageInfo.total': 'emitVulnerabilitiesCountChanged',
},
created() {
if (this.isLockedToProject) {
......@@ -83,6 +99,7 @@ export default {
optionId: this.lockToProject.id,
});
}
this.setPipelineId(this.pipelineId);
this.setProjectsEndpoint(this.projectsEndpoint);
this.setVulnerabilitiesEndpoint(this.vulnerabilitiesEndpoint);
this.setVulnerabilitiesCountEndpoint(this.vulnerabilitiesCountEndpoint);
......@@ -90,9 +107,7 @@ export default {
this.fetchVulnerabilities({ ...this.activeFilters, page: this.pageInfo.page });
this.fetchVulnerabilitiesCount(this.activeFilters);
this.fetchVulnerabilitiesHistory(this.activeFilters);
if (!this.isLockedToProject) {
this.fetchProjects();
}
},
methods: {
...mapActions('vulnerabilities', [
......@@ -106,6 +121,7 @@ export default {
'fetchVulnerabilitiesCount',
'fetchVulnerabilitiesHistory',
'openDismissalCommentBox',
'setPipelineId',
'setVulnerabilitiesCountEndpoint',
'setVulnerabilitiesEndpoint',
'setVulnerabilitiesHistoryEndpoint',
......@@ -116,6 +132,9 @@ export default {
]),
...mapActions('projects', ['setProjectsEndpoint', 'fetchProjects']),
...mapActions('filters', ['lockFilter']),
emitVulnerabilitiesCountChanged(count) {
this.$emit('vulnerabilitiesCountChanged', count);
},
},
};
</script>
......@@ -126,7 +145,7 @@ export default {
<filters />
</header>
<vulnerability-count-list v-if="isLockedToProject" class="mb-0" />
<vulnerability-count-list v-if="shouldShowCountList" class="mb-0" />
<div class="row mt-4">
<main role="main" class="col" :class="{ 'col-xl-7': !isLockedToProject }">
......@@ -136,7 +155,7 @@ export default {
/>
</main>
<aside v-if="!isLockedToProject" class="col-xl-5">
<aside v-if="shouldShowChart" class="col-xl-5">
<vulnerability-chart />
</aside>
</div>
......
......@@ -27,6 +27,10 @@ export const setProjectsEndpoint = ({ commit }, endpoint) => {
};
export const fetchProjects = ({ state, dispatch }) => {
if (!state.projectsEndpoint) {
return;
}
dispatch('requestProjects');
getAllProjects(state.projectsEndpoint)
......
......@@ -17,6 +17,8 @@ import createFlash from '~/flash';
const hideModal = () => $('#modal-mrwidget-security-issue').modal('hide');
export const setPipelineId = ({ commit }, id) => commit(types.SET_PIPELINE_ID, id);
export const setVulnerabilitiesEndpoint = ({ commit }, endpoint) => {
commit(types.SET_VULNERABILITIES_ENDPOINT, endpoint);
};
......@@ -145,7 +147,10 @@ export const receiveCreateIssueError = ({ commit }, { flashError }) => {
}
};
export const dismissVulnerability = ({ dispatch }, { vulnerability, flashError, comment }) => {
export const dismissVulnerability = (
{ dispatch, state },
{ vulnerability, flashError, comment },
) => {
dispatch('requestDismissVulnerability');
axios
......@@ -154,6 +159,7 @@ export const dismissVulnerability = ({ dispatch }, { vulnerability, flashError,
category: vulnerability.report_type,
comment,
feedback_type: 'dismissal',
pipeline_id: state.pipelineId,
project_fingerprint: vulnerability.project_fingerprint,
vulnerability_data: {
...vulnerability,
......@@ -162,9 +168,8 @@ export const dismissVulnerability = ({ dispatch }, { vulnerability, flashError,
},
})
.then(({ data }) => {
const { id } = vulnerability;
dispatch('closeDismissalCommentBox');
dispatch('receiveDismissVulnerabilitySuccess', { id, data });
dispatch('receiveDismissVulnerabilitySuccess', { vulnerability, data });
})
.catch(() => {
dispatch('receiveDismissVulnerabilityError', { flashError });
......@@ -204,9 +209,8 @@ export const addDismissalComment = ({ dispatch }, { vulnerability, comment }) =>
comment,
})
.then(({ data }) => {
const { id } = vulnerability;
dispatch('closeDismissalCommentBox');
dispatch('receiveAddDismissalCommentSuccess', { id, data });
dispatch('receiveAddDismissalCommentSuccess', { vulnerability, data });
})
.catch(() => {
dispatch('receiveAddDismissalCommentError');
......@@ -276,8 +280,7 @@ export const undoDismiss = ({ dispatch }, { vulnerability, flashError }) => {
axios
.delete(destroy_vulnerability_feedback_dismissal_path)
.then(() => {
const { id } = vulnerability;
dispatch('receiveUndoDismissSuccess', { id });
dispatch('receiveUndoDismissSuccess', { vulnerability });
})
.catch(() => {
dispatch('receiveUndoDismissError', { flashError });
......
export const SET_PIPELINE_ID = 'SET_PIPELINE_ID';
export const SET_VULNERABILITIES_ENDPOINT = 'SET_VULNERABILITIES_ENDPOINT';
export const SET_VULNERABILITIES_PAGE = 'SET_VULNERABILITIES_PAGE';
export const REQUEST_VULNERABILITIES = 'REQUEST_VULNERABILITIES';
......
......@@ -3,8 +3,12 @@ import { s__, __ } from '~/locale';
import { visitUrl } from '~/lib/utils/url_utility';
import * as types from './mutation_types';
import { DAYS } from './constants';
import { isSameVulnerability } from './utils';
export default {
[types.SET_PIPELINE_ID](state, payload) {
state.pipelineId = payload;
},
[types.SET_VULNERABILITIES_ENDPOINT](state, payload) {
state.vulnerabilitiesEndpoint = payload;
},
......@@ -106,7 +110,7 @@ export default {
}
Vue.set(state.modal.data.className, 'value', className);
Vue.set(state.modal.data.file, 'value', `${file}${lineSuffix}`);
Vue.set(state.modal.data.file, 'value', file ? `${file}${lineSuffix}` : null);
Vue.set(state.modal.data.image, 'value', image);
Vue.set(state.modal.data.namespace, 'value', namespace);
}
......@@ -164,7 +168,9 @@ export default {
Vue.set(state.modal, 'error', null);
},
[types.RECEIVE_DISMISS_VULNERABILITY_SUCCESS](state, payload) {
const vulnerability = state.vulnerabilities.find(vuln => vuln.id === payload.id);
const vulnerability = state.vulnerabilities.find(vuln =>
isSameVulnerability(vuln, payload.vulnerability),
);
vulnerability.dismissal_feedback = payload.data;
state.isDismissingVulnerability = false;
Vue.set(state.modal, 'isDismissingVulnerability', false);
......@@ -185,7 +191,9 @@ export default {
Vue.set(state.modal, 'error', null);
},
[types.RECEIVE_ADD_DISMISSAL_COMMENT_SUCCESS](state, payload) {
const vulnerability = state.vulnerabilities.find(vuln => vuln.id === payload.id);
const vulnerability = state.vulnerabilities.find(vuln =>
isSameVulnerability(vuln, payload.vulnerability),
);
if (vulnerability) {
vulnerability.dismissal_feedback = payload.data;
state.isDismissingVulnerability = false;
......@@ -223,7 +231,9 @@ export default {
Vue.set(state.modal, 'error', null);
},
[types.RECEIVE_REVERT_DISMISSAL_SUCCESS](state, payload) {
const vulnerability = state.vulnerabilities.find(vuln => vuln.id === payload.id);
const vulnerability = state.vulnerabilities.find(vuln =>
isSameVulnerability(vuln, payload.vulnerability),
);
vulnerability.dismissal_feedback = null;
state.isDismissingVulnerability = false;
Vue.set(state.modal, 'isDismissingVulnerability', false);
......
......@@ -13,6 +13,7 @@ export default () => ({
vulnerabilitiesHistoryDayRange: 90,
vulnerabilitiesHistoryMaxDayInterval: 7,
pageInfo: {},
pipelineId: null,
vulnerabilitiesCountEndpoint: null,
vulnerabilitiesHistoryEndpoint: null,
vulnerabilitiesEndpoint: null,
......
/* eslint-disable import/prefer-default-export */
import { isEqual } from 'underscore';
const isVulnerabilityLike = object =>
Boolean(object && object.location && object.identifiers && object.identifiers[0]);
/**
* Determines whether the provided objects represent the same vulnerability.
* @param {Object} vulnerability A vulnerability occurrence
* @param {Object} other Another vulnerability occurrence
* @returns {boolean}
*/
export const isSameVulnerability = (vulnerability, other) => {
if (!isVulnerabilityLike(vulnerability) || !isVulnerabilityLike(other)) {
return false;
}
// The `[location_fingerprint, identifiers[0]]` tuple is currently the most
// correct/robust set of data to compare to see if two objects represent the
// same vulnerability[1]. Unfortunately, `location_fingerprint` isn't exposed
// by the API yet, so we fall back to a slower deep equality comparison on
// `location` (which is a superset of `location_fingerprint`) if the former
// isn't present.
//
// [1]: https://gitlab.com/gitlab-org/gitlab-ee/issues/7586
let isLocationEqual = false;
if (vulnerability.location_fingerprint && other.location_fingerprint) {
isLocationEqual = vulnerability.location_fingerprint === other.location_fingerprint;
} else {
isLocationEqual = isEqual(vulnerability.location, other.location);
}
return isLocationEqual && isEqual(vulnerability.identifiers[0], other.identifiers[0]);
};
......@@ -81,11 +81,6 @@ export default {
required: false,
default: null,
},
vulnerabilitiesHistoryEndpoint: {
type: String,
required: false,
default: null,
},
},
computed: {
headline() {
......@@ -141,7 +136,6 @@ export default {
:empty-state-svg-path="emptyStateSvgPath"
:vulnerabilities-endpoint="vulnerabilitiesEndpoint"
:vulnerabilities-count-endpoint="vulnerabilitiesSummaryEndpoint"
:vulnerabilities-history-endpoint="vulnerabilitiesHistoryEndpoint"
:vulnerability-feedback-help-path="vulnerabilityFeedbackHelpPath"
/>
</template>
......
......@@ -11,6 +11,14 @@
- if pipeline.expose_security_dashboard?
#js-tab-security.build-security.tab-pane
- if Feature.enabled?(:pipeline_report_api)
#js-security-report-app{ data: { dashboard_documentation: help_page_path('user/application_security/security_dashboard/index'),
empty_state_svg_path: image_path('illustrations/security-dashboard-empty-state.svg'),
pipeline_id: pipeline.id,
project_id: project.id,
vulnerabilities_endpoint: expose_path(api_v4_projects_vulnerabilities_path(id: project.id, params: { pipeline_id: pipeline.id, scope: 'all' })),
vulnerability_feedback_help_path: help_page_path('user/application_security/index') } }
- else
#js-security-report-app{ data: { head_blob_path: blob_path,
sast_head_path: sast_endpoint,
dependency_scanning_head_path: dependency_scanning_endpoint,
......
......@@ -5,7 +5,7 @@
%li.js-security-tab-link
= link_to security_project_pipeline_path(project, pipeline), data: { target: '#js-tab-security', action: 'security', toggle: 'tab' }, class: 'security-tab qa-security-tab' do
= _("Security")
%span.badge.badge-pill.js-sast-counter.hidden
%span.badge.badge-pill.js-security-counter.hidden
- if pipeline.expose_license_management_data?
%li.js-licenses-tab-link
......
......@@ -99,7 +99,26 @@ describe 'Pipeline', :js do
context 'with a sast artifact' do
before do
create(:ee_ci_build, :sast, pipeline: pipeline)
end
context 'when feature flag is enabled' do
before do
visit security_project_pipeline_path(project, pipeline)
end
it 'shows jobs tab pane as active' do
expect(page).to have_content('Security')
expect(page).to have_css('#js-tab-security')
end
it 'shows security dashboard' do
expect(page).to have_css('.js-security-dashboard-table')
end
end
context 'when feature flag is disabled' do
before do
stub_feature_flags(pipeline_report_api: false)
visit security_project_pipeline_path(project, pipeline)
end
......@@ -112,6 +131,7 @@ describe 'Pipeline', :js do
expect(page).to have_content('SAST is loading')
end
end
end
context 'without sast artifact' do
before do
......
......@@ -13,31 +13,37 @@ import createStore from 'ee/security_dashboard/store';
const localVue = createLocalVue();
const pipelineId = 123;
const projectsEndpoint = `${TEST_HOST}/projects`;
const vulnerabilitiesEndpoint = `${TEST_HOST}/vulnerabilities`;
const vulnerabilitiesCountEndpoint = `${TEST_HOST}/vulnerabilities_summary`;
const vulnerabilitiesHistoryEndpoint = `${TEST_HOST}/vulnerabilities_history`;
describe('Card security reports app', () => {
describe('Security Dashboard app', () => {
let wrapper;
let mock;
let fetchProjectsSpy;
let lockFilterSpy;
let setPipelineIdSpy;
let store;
const setup = () => {
mock = new MockAdapter(axios);
fetchProjectsSpy = jest.fn();
lockFilterSpy = jest.fn();
setPipelineIdSpy = jest.fn();
};
const createComponent = props => {
store = createStore();
wrapper = shallowMount(SecurityDashboardApp, {
localVue,
store: createStore(),
store,
sync: false,
methods: {
lockFilter: lockFilterSpy,
fetchProjects: fetchProjectsSpy,
setPipelineId: setPipelineIdSpy,
},
propsData: {
dashboardDocumentation: '',
......@@ -46,6 +52,7 @@ describe('Card security reports app', () => {
vulnerabilitiesEndpoint,
vulnerabilitiesCountEndpoint,
vulnerabilitiesHistoryEndpoint,
pipelineId,
vulnerabilityFeedbackHelpPath: `${TEST_HOST}/vulnerabilities_feedback_help`,
...props,
},
......@@ -90,12 +97,27 @@ describe('Card security reports app', () => {
it('does not lock project filters', () => {
expect(lockFilterSpy).not.toHaveBeenCalled();
});
it('sets the pipeline id', () => {
expect(setPipelineIdSpy).toHaveBeenCalledWith(pipelineId);
});
describe('when the total number of vulnerabilities change', () => {
const newCount = 3;
beforeEach(() => {
localVue.set(store.state.vulnerabilities.pageInfo, 'total', newCount);
});
it('emits a vulnerabilitiesCountChanged event', () => {
expect(wrapper.emitted('vulnerabilitiesCountChanged')).toEqual([[newCount]]);
});
});
});
describe('with project lock', () => {
const project = {
id: 123,
name: 'my-project',
};
beforeEach(() => {
setup();
......@@ -112,8 +134,8 @@ describe('Card security reports app', () => {
expect(wrapper.vm.isLockedToProject).toBe(true);
});
it('does not fetch projects', () => {
expect(fetchProjectsSpy).not.toHaveBeenCalled();
it('fetches projects', () => {
expect(fetchProjectsSpy).toHaveBeenCalled();
});
it('locks the filters to a given project', () => {
......@@ -123,4 +145,21 @@ describe('Card security reports app', () => {
});
});
});
describe.each`
endpointProp | Component
${'vulnerabilitiesCountEndpoint'} | ${VulnerabilityCountList}
${'vulnerabilitiesHistoryEndpoint'} | ${VulnerabilityChart}
`('with an empty $endpointProp', ({ endpointProp, Component }) => {
beforeEach(() => {
setup();
createComponent({
[endpointProp]: '',
});
});
it(`does not show the ${Component.name}`, () => {
expect(wrapper.find(Component).exists()).toBe(false);
});
});
});
import { isSameVulnerability } from 'ee/security_dashboard/store/modules/vulnerabilities/utils';
import mockData from '../../../../javascripts/security_dashboard/store/vulnerabilities/data/mock_data_vulnerabilities.json';
describe('Vulnerabilities utils', () => {
const clone = serializable => JSON.parse(JSON.stringify(serializable));
const vuln = clone(mockData[0]);
const vulnWithNewLocation = { ...clone(vuln), location: { foo: 1 } };
const vulnWithNewIdentifier = { ...clone(vuln), identifiers: [{ foo: 1 }] };
describe('isSameVulnerability', () => {
describe.each`
description | vulnerability | other | result
${'identical vulnerabilities'} | ${vuln} | ${vuln} | ${true}
${'cloned vulnerabilities'} | ${vuln} | ${clone(vuln)} | ${true}
${'different locations'} | ${vuln} | ${vulnWithNewLocation} | ${false}
${'different primary identifiers'} | ${vuln} | ${vulnWithNewIdentifier} | ${false}
${'cloned non-vulnerabilities'} | ${{ foo: 1 }} | ${{ foo: 1 }} | ${false}
${'null values'} | ${null} | ${null} | ${false}
${'undefined values'} | ${undefined} | ${undefined} | ${false}
`('given $description', ({ vulnerability, other, result }) => {
it(`returns ${result}`, () => {
expect(isSameVulnerability(vulnerability, other)).toBe(result);
});
});
});
});
......@@ -12,7 +12,6 @@ const localVue = createLocalVue();
const vulnerabilitiesEndpoint = `${TEST_HOST}/vulnerabilities`;
const vulnerabilitiesSummaryEndpoint = `${TEST_HOST}/vulnerabilities_summary`;
const vulnerabilitiesHistoryEndpoint = `${TEST_HOST}/vulnerabilities_history`;
describe('Card security reports app', () => {
let wrapper;
......@@ -58,7 +57,6 @@ describe('Card security reports app', () => {
vulnerabilityFeedbackHelpPath: `${TEST_HOST}/vulnerability_feedback_help`,
vulnerabilitiesEndpoint,
vulnerabilitiesSummaryEndpoint,
vulnerabilitiesHistoryEndpoint,
...props,
},
});
......
......@@ -92,6 +92,16 @@ describe('projects actions', () => {
);
});
});
describe('with an empty endpoint', () => {
beforeEach(() => {
state.projectsEndpoint = '';
});
it('should not do anything', done => {
testAction(actions.fetchProjects, {}, state, [], [], done);
});
});
});
describe('receiveProjectsSuccess', () => {
......
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