From 6b8b46588cc4d324233de426c072ba9261342ade Mon Sep 17 00:00:00 2001 From: Mark Florian Date: Mon, 1 Apr 2019 16:46:30 +0800 Subject: [PATCH 1/4] Extract `vulnerability-details` component --- .../security_reports/components/modal.vue | 110 +-------------- .../components/vulnerability_details.vue | 126 ++++++++++++++++++ 2 files changed, 129 insertions(+), 107 deletions(-) create mode 100644 ee/app/assets/javascripts/vue_shared/security_reports/components/vulnerability_details.vue diff --git a/ee/app/assets/javascripts/vue_shared/security_reports/components/modal.vue b/ee/app/assets/javascripts/vue_shared/security_reports/components/modal.vue index ea789777a4515d..007d82a859b432 100644 --- a/ee/app/assets/javascripts/vue_shared/security_reports/components/modal.vue +++ b/ee/app/assets/javascripts/vue_shared/security_reports/components/modal.vue @@ -2,26 +2,22 @@ import { s__ } from '~/locale'; import Modal from '~/vue_shared/components/gl_modal.vue'; import LoadingButton from '~/vue_shared/components/loading_button.vue'; -import Icon from '~/vue_shared/components/icon.vue'; import ExpandButton from '~/vue_shared/components/expand_button.vue'; import EventItem from 'ee/vue_shared/security_reports/components/event_item.vue'; -import SafeLink from 'ee/vue_shared/components/safe_link.vue'; import SolutionCard from 'ee/vue_shared/security_reports/components/solution_card.vue'; -import SeverityBadge from './severity_badge.vue'; import SplitButton from 'ee/vue_shared/security_reports/components/split_button.vue'; +import VulnerabilityDetails from 'ee/vue_shared/security_reports/components/vulnerability_details.vue'; export default { components: { EventItem, ExpandButton, - Icon, LoadingButton, Modal, - SafeLink, - SeverityBadge, SolutionCard, SplitButton, + VulnerabilityDetails, }, props: { modal: { @@ -136,24 +132,6 @@ export default { this.$emit('dismissIssue'); } }, - isLastValue(index, values) { - return index < values.length - 1; - }, - hasValue(field) { - return field.value && field.value.length > 0; - }, - hasInstances(field, key) { - return key === 'instances' && this.hasValue(field); - }, - hasIdentifiers(field, key) { - return key === 'identifiers' && this.hasValue(field); - }, - hasLinks(field, key) { - return key === 'links' && this.hasValue(field); - }, - hasSeverity(field, key) { - return key === 'severity' && this.hasValue(field); - }, }, }; @@ -165,89 +143,7 @@ export default { class="modal-security-report-dast" > -
-
- -
-
-
    -
  • -
    - -
    -
    -
    - {{ instance.method }} -
    -
    - - {{ instance.uri }} - -
    - - 
    -                      {{ instance.evidence }}
    -     -
    -
    • -
-
- - - - -
-
-
+
diff --git a/ee/app/assets/javascripts/vue_shared/security_reports/components/vulnerability_details.vue b/ee/app/assets/javascripts/vue_shared/security_reports/components/vulnerability_details.vue new file mode 100644 index 00000000000000..d139220b98821d --- /dev/null +++ b/ee/app/assets/javascripts/vue_shared/security_reports/components/vulnerability_details.vue @@ -0,0 +1,126 @@ + + + -- GitLab From 0f2ef3f00dbb3dc94ec57ea119620344d350d39f Mon Sep 17 00:00:00 2001 From: Mark Florian Date: Mon, 1 Apr 2019 18:35:10 +0800 Subject: [PATCH 2/4] Extract `vulnerability-details`-specific tests These new tests run in jest. An additional test was added to the existing karma-based modal test suite to ensure the new component is rendered. --- .../security_reports/components/modal.vue | 2 +- .../components/vulnerability_details_spec.js | 138 ++++++++++++++++++ .../security_reports/components/modal_spec.js | 120 ++------------- 3 files changed, 154 insertions(+), 106 deletions(-) create mode 100644 ee/spec/frontend/vue_shared/security_reports/components/vulnerability_details_spec.js diff --git a/ee/app/assets/javascripts/vue_shared/security_reports/components/modal.vue b/ee/app/assets/javascripts/vue_shared/security_reports/components/modal.vue index 007d82a859b432..f0fbe476d74341 100644 --- a/ee/app/assets/javascripts/vue_shared/security_reports/components/modal.vue +++ b/ee/app/assets/javascripts/vue_shared/security_reports/components/modal.vue @@ -143,7 +143,7 @@ export default { class="modal-security-report-dast" > - +
diff --git a/ee/spec/frontend/vue_shared/security_reports/components/vulnerability_details_spec.js b/ee/spec/frontend/vue_shared/security_reports/components/vulnerability_details_spec.js new file mode 100644 index 00000000000000..0892d154d48213 --- /dev/null +++ b/ee/spec/frontend/vue_shared/security_reports/components/vulnerability_details_spec.js @@ -0,0 +1,138 @@ +import { createLocalVue, shallowMount } from '@vue/test-utils'; +import createState from 'ee/vue_shared/security_reports/store/state'; +import VulnerabilityDetails from 'ee/vue_shared/security_reports/components/vulnerability_details.vue'; +import SeverityBadge from 'ee/vue_shared/security_reports/components/severity_badge.vue'; +import SafeLink from 'ee/vue_shared/components/safe_link.vue'; +import { TEST_HOST } from 'helpers/test_constants'; + +describe('VulnerabilityDetails component', () => { + let wrapper; + const localVue = createLocalVue(); + + const componentFactory = (options = {}) => { + wrapper = shallowMount(localVue.extend(VulnerabilityDetails), { + ...options, + localVue, + sync: false, + }); + }; + + const expectSafeLink = (link, { href, text }) => { + expect(link.is(SafeLink)).toBe(true); + expect(link.props('href')).toBe(href); + expect(link.text()).toBe(text); + }; + + it('renders severity with a badge', () => { + const value = 'critical'; + componentFactory({ propsData: { details: { severity: { value } } } }); + const badge = wrapper.find(SeverityBadge); + + expect(badge.props('severity')).toBe(value); + }); + + it('renders link fields with link', () => { + const details = { + somefield: { + value: 'foo', + url: `${TEST_HOST}/bar`, + isLink: true, + }, + }; + + componentFactory({ propsData: { details } }); + + expectSafeLink(wrapper.find('.js-link-somefield'), { + href: `${TEST_HOST}/bar`, + text: 'foo', + }); + }); + + describe('does not render XSS links', () => { + // eslint-disable-next-line no-script-url + const badUrl = 'javascript:alert("")'; + + beforeEach(() => { + const details = createState().modal.data; + + details.file.value = 'badFile.lock'; + details.file.url = badUrl; + details.links.value = [ + { + url: badUrl, + }, + ]; + details.identifiers.value = [ + { + type: 'CVE', + name: 'BAD_URL', + url: badUrl, + }, + ]; + details.instances.value = [ + { + param: 'X-Content-Type-Options', + method: 'GET', + uri: badUrl, + }, + ]; + + componentFactory({ propsData: { details } }); + }); + + it('for the link field', () => { + expectSafeLink(wrapper.find('.js-link-links'), { + href: badUrl, + text: badUrl, + }); + }); + + it('for the identifiers field', () => { + expectSafeLink(wrapper.find('.js-link-identifiers'), { + href: badUrl, + text: 'BAD_URL', + }); + }); + + it('for the file field', () => { + expectSafeLink(wrapper.find('.js-link-file'), { + href: badUrl, + text: 'badFile.lock', + }); + }); + + it('for the instances field', () => { + expectSafeLink(wrapper.find('.report-block-list-issue-description-link .break-link'), { + href: badUrl, + text: badUrl, + }); + }); + }); + + describe('with instances', () => { + beforeEach(() => { + const details = { + instances: { + value: [ + { uri: 'http://192.168.32.236:3001/explore?sort=latest_activity_desc' }, + { uri: 'http://192.168.32.236:3001/help/user/group/subgroups/index.md' }, + ], + }, + }; + + componentFactory({ propsData: { details } }); + }); + + it('renders instances list', () => { + const instances = wrapper.findAll('.report-block-list li').wrappers; + + expect(instances[0].text()).toContain( + 'http://192.168.32.236:3001/explore?sort=latest_activity_desc', + ); + + expect(instances[1].text()).toContain( + 'http://192.168.32.236:3001/help/user/group/subgroups/index.md', + ); + }); + }); +}); diff --git a/ee/spec/javascripts/vue_shared/security_reports/components/modal_spec.js b/ee/spec/javascripts/vue_shared/security_reports/components/modal_spec.js index ec2f1940d6e2e1..141b273033b0b3 100644 --- a/ee/spec/javascripts/vue_shared/security_reports/components/modal_spec.js +++ b/ee/spec/javascripts/vue_shared/security_reports/components/modal_spec.js @@ -2,8 +2,6 @@ import Vue from 'vue'; import component from 'ee/vue_shared/security_reports/components/modal.vue'; import createState from 'ee/vue_shared/security_reports/store/state'; import mountComponent from 'spec/helpers/vue_mount_component_helper'; -import { trimText } from 'spec/helpers/vue_component_helper'; -import { TEST_HOST } from 'spec/test_constants'; describe('Security Reports modal', () => { const Component = Vue.extend(component); @@ -115,31 +113,6 @@ describe('Security Reports modal', () => { }); }); - describe('with instances', () => { - beforeEach(() => { - const props = { - modal: createState().modal, - }; - props.modal.data.instances.value = [ - { uri: 'http://192.168.32.236:3001/explore?sort=latest_activity_desc' }, - { uri: 'http://192.168.32.236:3001/help/user/group/subgroups/index.md' }, - ]; - vm = mountComponent(Component, props); - }); - - it('renders instances list', () => { - const instances = vm.$el.querySelectorAll('.report-block-list li'); - - expect(instances[0].textContent).toContain( - 'http://192.168.32.236:3001/explore?sort=latest_activity_desc', - ); - - expect(instances[1].textContent).toContain( - 'http://192.168.32.236:3001/help/user/group/subgroups/index.md', - ); - }); - }); - describe('data', () => { beforeEach(() => { const props = { @@ -147,9 +120,6 @@ describe('Security Reports modal', () => { vulnerabilityFeedbackHelpPath: 'feedbacksHelpPath', }; props.modal.title = 'Arbitrary file existence disclosure in Action Pack'; - props.modal.data.file.value = 'Gemfile.lock'; - props.modal.data.file.url = `${TEST_HOST}/path/Gemfile.lock`; - props.modal.data.severity = { value: 'critical' }; vm = mountComponent(Component, props); }); @@ -157,23 +127,11 @@ describe('Security Reports modal', () => { expect(vm.$el.textContent).toContain('Arbitrary file existence disclosure in Action Pack'); }); - it('renders link fields with link', () => { - expect(vm.$el.querySelector('.js-link-file').getAttribute('href')).toEqual( - `${TEST_HOST}/path/Gemfile.lock`, - ); - }); - it('renders help link', () => { expect( vm.$el.querySelector('.js-link-vulnerabilityFeedbackHelpPath').getAttribute('href'), ).toEqual('feedbacksHelpPath'); }); - - it('renders severity with a badge', () => { - const badge = vm.$el.querySelector('.severity-badge'); - - expect(badge.textContent).toContain('Critical'); - }); }); }); @@ -209,6 +167,21 @@ describe('Security Reports modal', () => { }); }); + describe('Vulnerability Details', () => { + it('is rendered', () => { + const props = { + modal: createState().modal, + }; + props.modal.data.namespace.value = 'foobar'; + vm = mountComponent(Component, props); + + const vulnerabilityDetails = vm.$el.querySelector('.js-vulnerability-details'); + + expect(vulnerabilityDetails).not.toBeNull(); + expect(vulnerabilityDetails.textContent).toContain('foobar'); + }); + }); + describe('Solution Card', () => { it('is rendered if the vulnerability has a solution', () => { const props = { @@ -253,67 +226,4 @@ describe('Security Reports modal', () => { expect(vm.$el.querySelector('hr')).not.toBeNull(); }); }); - - describe('does not render XSS links', () => { - // eslint-disable-next-line no-script-url - const badUrl = 'javascript:alert("")'; - - beforeEach(() => { - const props = { - modal: createState().modal, - }; - - props.modal.data.file.value = 'badFile.lock'; - props.modal.data.file.url = badUrl; - props.modal.data.links.value = [ - { - url: badUrl, - }, - ]; - props.modal.data.identifiers.value = [ - { - type: 'CVE', - name: 'BAD_URL', - url: badUrl, - }, - ]; - props.modal.data.instances.value = [ - { - param: 'X-Content-Type-Options', - method: 'GET', - uri: badUrl, - }, - ]; - - vm = mountComponent(Component, props); - }); - - it('for the link field', () => { - const linkEl = vm.$el.querySelector('.js-link-links'); - - expect(linkEl.tagName).not.toBe('A'); - expect(trimText(linkEl.textContent)).toBe(badUrl); - }); - - it('for the identifiers field', () => { - const linkEl = vm.$el.querySelector('.js-link-identifiers'); - - expect(linkEl.tagName).not.toBe('A'); - expect(trimText(linkEl.textContent)).toBe('BAD_URL'); - }); - - it('for the file field', () => { - const linkEl = vm.$el.querySelector('.js-link-file'); - - expect(linkEl.tagName).not.toBe('A'); - expect(trimText(linkEl.textContent)).toBe('badFile.lock'); - }); - - it('for the instances field', () => { - const linkEl = vm.$el.querySelector('.report-block-list-issue-description-link .break-link'); - - expect(linkEl.tagName).not.toBe('A'); - expect(trimText(linkEl.textContent)).toBe(badUrl); - }); - }); }); -- GitLab From 2a900ae66e29c6d2ec42a00a7fcf47daea5f55b0 Mon Sep 17 00:00:00 2001 From: Mark Florian Date: Tue, 2 Apr 2019 17:30:59 +0800 Subject: [PATCH 3/4] Rename existing test This test has nothing to do with a `data` field, but it does appear to verify the title is rendered as expected. --- .../vue_shared/security_reports/components/modal_spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ee/spec/javascripts/vue_shared/security_reports/components/modal_spec.js b/ee/spec/javascripts/vue_shared/security_reports/components/modal_spec.js index 141b273033b0b3..63e91bd804e4bc 100644 --- a/ee/spec/javascripts/vue_shared/security_reports/components/modal_spec.js +++ b/ee/spec/javascripts/vue_shared/security_reports/components/modal_spec.js @@ -123,7 +123,7 @@ describe('Security Reports modal', () => { vm = mountComponent(Component, props); }); - it('renders keys in `data`', () => { + it('renders title', () => { expect(vm.$el.textContent).toContain('Arbitrary file existence disclosure in Action Pack'); }); -- GitLab From a835f4641bb4850251e66c060959e39623be952e Mon Sep 17 00:00:00 2001 From: Mark Florian Date: Tue, 2 Apr 2019 16:32:26 +0800 Subject: [PATCH 4/4] Change method name to match implementation It literally does the opposite of what the original name suggested. --- .../security_reports/components/vulnerability_details.vue | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/ee/app/assets/javascripts/vue_shared/security_reports/components/vulnerability_details.vue b/ee/app/assets/javascripts/vue_shared/security_reports/components/vulnerability_details.vue index d139220b98821d..08f608c72707ed 100644 --- a/ee/app/assets/javascripts/vue_shared/security_reports/components/vulnerability_details.vue +++ b/ee/app/assets/javascripts/vue_shared/security_reports/components/vulnerability_details.vue @@ -17,7 +17,7 @@ export default { }, }, methods: { - isLastValue(index, values) { + hasMoreValues(index, values) { return index < values.length - 1; }, hasValue(field) { @@ -88,7 +88,7 @@ export default { {{ identifier.name }} {{ identifier.name }} - ,  + ,