Commit 4d9f4a7a authored by Mark Florian's avatar Mark Florian Committed by Olena Horal-Koretska

Add button to enable SAST scanning

This adds a button to the Security Configuration page, allowing the user
to enable SAST scanning via a merge request with a simple click. This
button is only displayed when:

1. The sast_configuration_by_click feature flag is enabled.
1. The project does not have an existing .gitlab-ci.yml file.

Some other changes include (and are _not_ behind any feature flag):

 -  Add third status text string for when Auto DevOps is enabled.
 -  Add third column headed "Manage" to the Security Configuration
    table, which contains a link to that scanner's documentation. This
    is where the button appears in the SAST case when the
    sast_configuration_by_click feature flag is enabled.
 -  Extract thClass constant in app component.
 -  Do not guard the SASTConfigurationController#create action behind
    the sast_configuration_ui feature flag. This feature flag is
    intended for a future iteration when an actual configuration UI
    exists for SAST, which is what that controller is actually for. In
    this iteration, it's simply a placeholder for the temporary REST
    endpoint, which will be [replaced][gql] by a GraphQL mutation in
    another iteration.

This is an MVC for creating a [Configuration UI for SAST][1]. Some
technical debt has been incurred here:

1. The REST (POST) endpoint used to create the merge request will be
   replaced by a [GraphQL mutation][gql].
1. The `type` of each feature has been added to the security
   configuration features exposed to the fronted. This should be
   [replaced][type] by a field exposing the scanner's configuration UI
   path (or similar).
1. The CreateMergeRequestButton component will likely be removed in the
   next iteration of the SAST Configuration UI, particularly if the
   GraphQL mutation is ready by then.

Addresses https://gitlab.com/gitlab-org/gitlab/-/issues/220573, part of
[Configuration UI for SAST][1].

[1]: https://gitlab.com/groups/gitlab-org/-/epics/3262
[gql]: https://gitlab.com/gitlab-org/gitlab/-/issues/227465
[type]: https://gitlab.com/gitlab-org/gitlab/-/issues/227575
parent e8288da4
......@@ -14,7 +14,7 @@ info: To determine the technical writer assigned to the Stage/Group associated w
The security configuration page displays the configuration state of each of the security
features and can be accessed through a project's sidebar nav.
![Screenshot of security configuration page](../img/security_configuration_page_v13_1.png)
![Screenshot of security configuration page](../img/security_configuration_page_v13_2.png)
The page uses the project's latest default branch [CI pipeline](../../../ci/pipelines/index.md) to determine the configuration
state of each feature. If a job with the expected security report artifact exists in the pipeline,
......
......@@ -3,6 +3,7 @@ import { GlAlert, GlLink, GlSprintf, GlTable } from '@gitlab/ui';
import { s__, __, sprintf } from '~/locale';
import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
import AutoFixSettings from './auto_fix_settings.vue';
import CreateMergeRequestButton from './create_merge_request_button.vue';
export default {
components: {
......@@ -11,6 +12,7 @@ export default {
GlSprintf,
GlTable,
AutoFixSettings,
CreateMergeRequestButton,
},
mixins: [glFeatureFlagsMixin()],
props: {
......@@ -55,6 +57,12 @@ export default {
required: false,
default: false,
},
// TODO: Remove as part of https://gitlab.com/gitlab-org/gitlab/-/issues/227575
createSastMergeRequestPath: {
type: String,
required: false,
default: '',
},
},
computed: {
devopsMessage() {
......@@ -70,18 +78,26 @@ export default {
return this.autoDevopsEnabled ? this.autoDevopsHelpPagePath : this.latestPipelinePath;
},
fields() {
const borderClasses = 'gl-border-b-1! gl-border-b-solid! gl-border-gray-100!';
const thClass = `gl-text-gray-900 gl-bg-transparent! ${borderClasses}`;
return [
{
key: 'feature',
label: s__('SecurityConfiguration|Security Control'),
thClass: 'gl-text-gray-900 bg-transparent border-bottom',
thClass,
},
{
key: 'configured',
label: s__('SecurityConfiguration|Status'),
thClass: 'gl-text-gray-900 bg-transparent border-bottom',
thClass,
formatter: this.getStatusText,
},
{
key: 'manage',
label: s__('SecurityConfiguration|Manage'),
thClass,
},
];
},
shouldShowAutoDevopsAlert() {
......@@ -95,15 +111,28 @@ export default {
},
methods: {
getStatusText(value) {
return value
? s__('SecurityConfiguration|Enabled')
: s__('SecurityConfiguration|Not yet enabled');
if (value) {
return this.autoDevopsEnabled
? s__('SecurityConfiguration|Enabled with Auto DevOps')
: s__('SecurityConfiguration|Enabled');
}
return s__('SecurityConfiguration|Not enabled');
},
getFeatureDocumentationLinkLabel(featureName) {
return sprintf(s__('SecurityConfiguration|Feature documentation for %{featureName}'), {
featureName,
});
},
// TODO: Remove as part of https://gitlab.com/gitlab-org/gitlab/-/issues/227575
canCreateSASTMergeRequest(feature) {
return Boolean(
this.glFeatures.sastConfigurationByClick &&
feature.type === 'sast' &&
this.createSastMergeRequestPath &&
!this.gitlabCiPresent,
);
},
},
autoDevopsAlertMessage: s__(`
SecurityConfiguration|You can quickly enable all security scanning tools by
......@@ -145,14 +174,24 @@ export default {
<div class="gl-text-gray-900">{{ item.name }}</div>
<div>
{{ item.description }}
</div>
</template>
<template #cell(manage)="{ item }">
<create-merge-request-button
v-if="canCreateSASTMergeRequest(item)"
:auto-devops-enabled="autoDevopsEnabled"
:endpoint="createSastMergeRequestPath"
/>
<gl-link
v-else
target="_blank"
:href="item.link"
:aria-label="getFeatureDocumentationLinkLabel(item.name)"
>
{{ __('More information') }}
{{ s__('SecurityConfiguration|See documentation') }}
</gl-link>
</div>
</template>
</gl-table>
<auto-fix-settings v-if="glFeatures.securityAutoFix" v-bind="autoFixSettingsProps" />
......
<script>
import * as Sentry from '@sentry/browser';
import { GlButton } from '@gitlab/ui';
import axios from '~/lib/utils/axios_utils';
import { s__ } from '~/locale';
import { redirectTo } from '~/lib/utils/url_utility';
import createFlash from '~/flash';
export default {
components: {
GlButton,
},
props: {
autoDevopsEnabled: {
type: Boolean,
required: true,
},
endpoint: {
type: String,
required: true,
},
},
data() {
return {
isCreatingMergeRequest: false,
};
},
computed: {
buttonProps() {
if (this.autoDevopsEnabled) {
return {
text: this.$options.i18n.autoDevOps,
};
}
return {
text: this.$options.i18n.noAutoDevOps,
category: 'primary',
variant: 'success',
};
},
},
methods: {
createMergeRequest() {
this.isCreatingMergeRequest = true;
return axios
.post(this.endpoint)
.then(({ data }) => {
const { filePath } = data;
if (!filePath) {
// eslint-disable-next-line @gitlab/require-i18n-strings
throw new Error('SAST merge request creation failed');
}
redirectTo(filePath);
})
.catch(error => {
this.isCreatingMergeRequest = false;
createFlash(
s__('SecurityConfiguration|An error occurred while creating the merge request.'),
);
Sentry.captureException(error);
});
},
},
i18n: {
autoDevOps: s__('SecurityConfiguration|Configure'),
noAutoDevOps: s__('SecurityConfiguration|Enable via Merge Request'),
},
};
</script>
<template>
<gl-button :loading="isCreatingMergeRequest" v-bind="buttonProps" @click="createMergeRequest">{{
buttonProps.text
}}</gl-button>
</template>
......@@ -16,6 +16,7 @@ export default function init() {
containerScanningHelpPath,
dependencyScanningHelpPath,
toggleAutofixSettingEndpoint,
createSastMergeRequestPath,
} = el.dataset;
return new Vue({
......@@ -31,6 +32,7 @@ export default function init() {
features: JSON.parse(features),
helpPagePath,
latestPipelinePath,
createSastMergeRequestPath,
...parseBooleanDataAttributes(el, [
'autoDevopsEnabled',
'canEnableAutoDevops',
......
......@@ -8,7 +8,7 @@ module Projects
alias_method :vulnerable, :project
before_action :ensure_sast_configuration_enabled!
before_action :ensure_sast_configuration_enabled!, except: [:create]
before_action :authorize_edit_tree!, only: [:create]
def show
......
......@@ -142,6 +142,7 @@ module Projects
def scan(type, configured: false)
{
type: type,
configured: configured,
description: self.class.localized_scan_descriptions[type],
link: help_page_path(SCAN_DOCS[type]),
......
---
title: Add "Manage" column and tweak "Status" wording in the Security Configuration page
merge_request: 36432
author:
type: changed
......@@ -2,6 +2,7 @@ import { mount } from '@vue/test-utils';
import { merge } from 'lodash';
import { GlAlert, GlLink } from '@gitlab/ui';
import SecurityConfigurationApp from 'ee/security_configuration/components/app.vue';
import CreateMergeRequestButton from 'ee/security_configuration/components/create_merge_request_button.vue';
import stubChildren from 'helpers/stub_children';
const propsData = {
......@@ -12,6 +13,7 @@ const propsData = {
autoDevopsPath: 'http://autoDevopsPath',
helpPagePath: 'http://helpPagePath',
autoFixSettingsProps: {},
createSastMergeRequestPath: 'http://createSastMergeRequestPath',
};
describe('Security Configuration App', () => {
......@@ -36,20 +38,29 @@ describe('Security Configuration App', () => {
afterEach(() => {
wrapper.destroy();
wrapper = null;
});
const generateFeatures = n => {
const generateFeatures = (n, overrides = {}) => {
return [...Array(n).keys()].map(i => ({
type: `scan-type-${i}`,
name: `name-feature-${i}`,
description: `description-feature-${i}`,
link: `link-feature-${i}`,
configured: i % 2 === 0,
...overrides,
}));
};
const getPipelinesLink = () => wrapper.find({ ref: 'pipelinesLink' });
const getFeaturesTable = () => wrapper.find({ ref: 'securityControlTable' });
const getFeaturesRows = () => getFeaturesTable().findAll('tbody tr');
const getAlert = () => wrapper.find(GlAlert);
const getCreateMergeRequestButton = () => wrapper.find(CreateMergeRequestButton);
const getRowCells = row => {
const [feature, status, manage] = row.findAll('td').wrappers;
return { feature, status, manage };
};
describe('header', () => {
it.each`
......@@ -131,16 +142,70 @@ describe('Security Configuration App', () => {
createComponent({ propsData: { features } });
expect(getFeaturesTable().classes('b-table-stacked-md')).toBeTruthy();
const rows = getFeaturesTable().findAll('tbody tr');
const rows = getFeaturesRows();
expect(rows).toHaveLength(5);
for (let i = 0; i < features.length; i += 1) {
const [feature, status] = rows.at(i).findAll('td').wrappers;
const { feature, status, manage } = getRowCells(rows.at(i));
expect(feature.text()).toMatch(features[i].name);
expect(feature.text()).toMatch(features[i].description);
expect(feature.find(GlLink).attributes('href')).toBe(features[i].link);
expect(status.text()).toMatch(features[i].configured ? 'Enabled' : 'Not yet enabled');
expect(status.text()).toMatch(features[i].configured ? 'Enabled' : 'Not enabled');
expect(manage.find(GlLink).attributes('href')).toBe(features[i].link);
}
});
describe('given a feature enabled by Auto DevOps', () => {
it('displays the expected status text', () => {
const features = generateFeatures(1, { configured: true });
createComponent({ propsData: { features, autoDevopsEnabled: true } });
const { status } = getRowCells(getFeaturesRows().at(0));
expect(status.text()).toMatch('Enabled with Auto DevOps');
});
});
});
describe('enabling SAST by merge request', () => {
describe.each`
sastConfigurationByClick | gitlabCiPresent | autoDevopsEnabled | buttonExpected
${true} | ${false} | ${false} | ${true}
${true} | ${false} | ${true} | ${true}
${true} | ${true} | ${false} | ${false}
${false} | ${false} | ${false} | ${false}
`(
'given sastConfigurationByClick is $sastConfigurationByClick, gitlabCiPresent is $gitlabCiPresent, autoDevopsEnabled is $autoDevopsEnabled',
({ sastConfigurationByClick, gitlabCiPresent, autoDevopsEnabled, buttonExpected }) => {
beforeEach(() => {
const features = generateFeatures(1, { type: 'sast', configured: false });
createComponent({
propsData: { features, gitlabCiPresent, autoDevopsEnabled },
provide: { glFeatures: { sastConfigurationByClick } },
});
});
if (buttonExpected) {
it('renders the CreateMergeRequestButton component', () => {
const button = getCreateMergeRequestButton();
expect(button.exists()).toBe(true);
expect(button.props()).toMatchObject({
endpoint: propsData.createSastMergeRequestPath,
autoDevopsEnabled,
});
});
it('does not render the documentation link', () => {
const { manage } = getRowCells(getFeaturesRows().at(0));
expect(manage.contains(GlLink)).toBe(false);
});
} else {
it('does not render the CreateMergeRequestButton component', () => {
expect(getCreateMergeRequestButton().exists()).toBe(false);
});
}
},
);
});
});
import * as Sentry from '@sentry/browser';
import { shallowMount } from '@vue/test-utils';
import AxiosMockAdapter from 'axios-mock-adapter';
import { GlButton } from '@gitlab/ui';
import createFlash from '~/flash';
import axios from '~/lib/utils/axios_utils';
import CreateMergeRequestButton from 'ee/security_configuration/components/create_merge_request_button.vue';
import { redirectTo } from '~/lib/utils/url_utility';
import waitForPromises from 'helpers/wait_for_promises';
jest.mock('~/flash.js');
jest.mock('~/lib/utils/url_utility', () => ({
redirectTo: jest.fn(),
}));
const endpoint = '/endpoint';
const { i18n } = CreateMergeRequestButton;
const DEFAULT_BUTTON_PROPS = {
category: 'tertiary',
variant: 'default',
};
const SUCCESS_BUTTON_PROPS = {
category: 'primary',
variant: 'success',
};
const MERGE_REQUEST_PATH = '/merge_requests/new';
describe('CreateMergeRequestButton component', () => {
let axiosMock;
let wrapper;
const createComponent = (props = {}) => {
wrapper = shallowMount(CreateMergeRequestButton, {
propsData: {
endpoint,
autoDevopsEnabled: false,
...props,
},
});
};
beforeEach(() => {
axiosMock = new AxiosMockAdapter(axios);
});
afterEach(() => {
wrapper.destroy();
wrapper = null;
axiosMock.restore();
});
const findButton = () => wrapper.find(GlButton);
describe.each`
autoDevopsEnabled | buttonText | buttonProps
${false} | ${i18n.noAutoDevOps} | ${SUCCESS_BUTTON_PROPS}
${true} | ${i18n.autoDevOps} | ${DEFAULT_BUTTON_PROPS}
`(
'when autoDevopsEnabled is $autoDevopsEnabled',
({ autoDevopsEnabled, buttonText, buttonProps }) => {
beforeEach(() => {
createComponent({ autoDevopsEnabled });
});
it('uses the right button label', () => {
expect(findButton().text()).toEqual(buttonText);
});
it('passes the correct data to the button', () => {
expect(findButton().props()).toMatchObject({
loading: false,
...buttonProps,
});
});
},
);
describe('when clicking the button', () => {
describe.each`
context | filePath | statusCode | partialErrorMessage
${'a response error code'} | ${MERGE_REQUEST_PATH} | ${500} | ${'500'}
${'no filePath'} | ${''} | ${200} | ${/merge request.*fail/}
`(
'given an unsuccessful endpoint response due to $context',
({ filePath, statusCode, partialErrorMessage }) => {
beforeEach(() => {
axiosMock.onPost(endpoint).replyOnce(statusCode, { filePath });
jest.spyOn(Sentry, 'captureException').mockImplementation();
createComponent();
findButton().vm.$emit('click');
});
it('sets the loading prop to true', () => {
expect(findButton().props().loading).toBe(true);
});
describe('after async tasks', () => {
beforeEach(() => waitForPromises());
it('does not call redirectTo', () => {
expect(redirectTo).not.toHaveBeenCalled();
});
it('creates a flash message', () => {
expect(createFlash).toHaveBeenCalledWith(expect.any(String));
});
it('sends the error to Sentry', () => {
expect(Sentry.captureException.mock.calls).toMatchObject([
[{ message: expect.stringMatching(partialErrorMessage) }],
]);
});
it('sets the loading prop to false', () => {
expect(findButton().props().loading).toBe(false);
});
});
},
);
describe('given a successful endpoint response', () => {
beforeEach(() => {
axiosMock.onPost(endpoint).replyOnce(200, { filePath: MERGE_REQUEST_PATH });
jest.spyOn(Sentry, 'captureException').mockImplementation();
createComponent();
findButton().vm.$emit('click');
});
it('sets the loading prop to true', () => {
expect(findButton().props().loading).toBe(true);
});
describe('after async tasks', () => {
beforeEach(() => waitForPromises());
it('calls redirectTo', () => {
expect(redirectTo).toHaveBeenCalledWith(MERGE_REQUEST_PATH);
});
it('does not create a flash message', () => {
expect(createFlash).not.toHaveBeenCalled();
});
it('does not call Sentry.captureException', () => {
expect(Sentry.captureException).not.toHaveBeenCalled();
});
it('keeps the loading prop set to true', () => {
// This is done for UX reasons. If the loading prop is set to false
// on success, then there's a period where the button is clickable
// again. Instead, we want the button to display a loading indicator
// for the remainder of the lifetime of the page (i.e., until the
// browser can start painting the new page it's been redirected to).
expect(findButton().props().loading).toBe(true);
});
});
});
});
});
......@@ -215,6 +215,7 @@ RSpec.describe Projects::Security::ConfigurationPresenter do
def security_scan(type, configured:)
{
"type" => type.to_s,
"configured" => configured,
"description" => described_class.localized_scan_descriptions[type],
"link" => help_page_path(described_class::SCAN_DOCS[type]),
......
......@@ -20531,18 +20531,36 @@ msgstr ""
msgid "Security report is out of date. Run %{newPipelineLinkStart}a new pipeline%{newPipelineLinkEnd} for the target branch (%{targetBranchName})"
msgstr ""
msgid "SecurityConfiguration|An error occurred while creating the merge request."
msgstr ""
msgid "SecurityConfiguration|Configure"
msgstr ""
msgid "SecurityConfiguration|Enable via Merge Request"
msgstr ""
msgid "SecurityConfiguration|Enabled"
msgstr ""
msgid "SecurityConfiguration|Enabled with Auto DevOps"
msgstr ""
msgid "SecurityConfiguration|Feature documentation for %{featureName}"
msgstr ""
msgid "SecurityConfiguration|Not yet enabled"
msgid "SecurityConfiguration|Manage"
msgstr ""
msgid "SecurityConfiguration|Not enabled"
msgstr ""
msgid "SecurityConfiguration|Security Control"
msgstr ""
msgid "SecurityConfiguration|See documentation"
msgstr ""
msgid "SecurityConfiguration|Status"
msgstr ""
......
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