Commit 2ea929f9 authored by Mark Florian's avatar Mark Florian

Fix Security Configuration enable via MR buttons

This makes sure that the "Configure via Merge Request" button is
displayed for Secret Detection in the [new][1] Security Configuration
page for all tiers.

The button was not working in FOSS since the client-side mutation was
only defined in EE, so that was moved to FOSS. The backend
implementation of the mutation is already in FOSS.

This also ensures that a similar button is displayed for Dependency
Scanning in Ultimate projects when the sec_dependency_scanning_ui_enable
feature flag is enabled.

Addresses https://gitlab.com/gitlab-org/gitlab/-/issues/326005 and
https://gitlab.com/gitlab-org/gitlab/-/issues/328426.

[1]: https://gitlab.com/gitlab-org/gitlab/-/issues/326926
parent bc09e70b
import { helpPagePath } from '~/helpers/help_page_helper'; import { helpPagePath } from '~/helpers/help_page_helper';
import { __, s__ } from '~/locale'; import { __, s__ } from '~/locale';
import configureSastMutation from '~/security_configuration/graphql/configure_sast.mutation.graphql';
import { import {
REPORT_TYPE_SAST, REPORT_TYPE_SAST,
REPORT_TYPE_DAST, REPORT_TYPE_DAST,
...@@ -15,6 +14,9 @@ import { ...@@ -15,6 +14,9 @@ import {
REPORT_TYPE_LICENSE_COMPLIANCE, REPORT_TYPE_LICENSE_COMPLIANCE,
} from '~/vue_shared/security_reports/constants'; } from '~/vue_shared/security_reports/constants';
import configureSastMutation from '../graphql/configure_sast.mutation.graphql';
import configureSecretDetectionMutation from '../graphql/configure_secret_detection.mutation.graphql';
/** /**
* Translations & helpPagePaths for Static Security Configuration Page * Translations & helpPagePaths for Static Security Configuration Page
*/ */
...@@ -214,6 +216,10 @@ export const securityFeatures = [ ...@@ -214,6 +216,10 @@ export const securityFeatures = [
helpPath: DEPENDENCY_SCANNING_HELP_PATH, helpPath: DEPENDENCY_SCANNING_HELP_PATH,
configurationHelpPath: DEPENDENCY_SCANNING_CONFIG_HELP_PATH, configurationHelpPath: DEPENDENCY_SCANNING_CONFIG_HELP_PATH,
type: REPORT_TYPE_DEPENDENCY_SCANNING, type: REPORT_TYPE_DEPENDENCY_SCANNING,
// This field will eventually come from the backend, the progress is
// tracked in https://gitlab.com/gitlab-org/gitlab/-/issues/331621
canEnableByMergeRequest: window.gon.features?.secDependencyScanningUiEnable,
}, },
{ {
name: CONTAINER_SCANNING_NAME, name: CONTAINER_SCANNING_NAME,
...@@ -235,7 +241,16 @@ export const securityFeatures = [ ...@@ -235,7 +241,16 @@ export const securityFeatures = [
helpPath: SECRET_DETECTION_HELP_PATH, helpPath: SECRET_DETECTION_HELP_PATH,
configurationHelpPath: SECRET_DETECTION_CONFIG_HELP_PATH, configurationHelpPath: SECRET_DETECTION_CONFIG_HELP_PATH,
type: REPORT_TYPE_SECRET_DETECTION, type: REPORT_TYPE_SECRET_DETECTION,
// This field is currently hardcoded because Secret Detection is always
// available. It will eventually come from the Backend, the progress is
// tracked in https://gitlab.com/gitlab-org/gitlab/-/issues/333113
available: true, available: true,
// This field is currently hardcoded because SAST can always be enabled via MR
// It will eventually come from the Backend, the progress is tracked in
// https://gitlab.com/gitlab-org/gitlab/-/issues/331621
canEnableByMergeRequest: true,
}, },
{ {
name: API_FUZZING_NAME, name: API_FUZZING_NAME,
...@@ -273,4 +288,15 @@ export const featureToMutationMap = { ...@@ -273,4 +288,15 @@ export const featureToMutationMap = {
}, },
}), }),
}, },
[REPORT_TYPE_SECRET_DETECTION]: {
mutationId: 'configureSecretDetection',
getMutationPayload: (projectPath) => ({
mutation: configureSecretDetectionMutation,
variables: {
input: {
projectPath,
},
},
}),
},
}; };
...@@ -46,8 +46,7 @@ export default { ...@@ -46,8 +46,7 @@ export default {
return button; return button;
}, },
showManageViaMr() { showManageViaMr() {
const { available, configured, canEnableByMergeRequest } = this.feature; return ManageViaMr.canRender(this.feature);
return canEnableByMergeRequest && available && !configured;
}, },
cardClasses() { cardClasses() {
return { 'gl-bg-gray-10': !this.available }; return { 'gl-bg-gray-10': !this.available };
......
...@@ -5,6 +5,10 @@ import { redirectTo } from '~/lib/utils/url_utility'; ...@@ -5,6 +5,10 @@ import { redirectTo } from '~/lib/utils/url_utility';
import { sprintf, s__ } from '~/locale'; import { sprintf, s__ } from '~/locale';
import apolloProvider from '../provider'; import apolloProvider from '../provider';
function mutationSettingsForFeatureType(type) {
return featureToMutationMap[type];
}
export default { export default {
apolloProvider, apolloProvider,
components: { components: {
...@@ -33,17 +37,19 @@ export default { ...@@ -33,17 +37,19 @@ export default {
}; };
}, },
computed: { computed: {
featureSettings() { mutationSettings() {
return featureToMutationMap[this.feature.type]; return mutationSettingsForFeatureType(this.feature.type);
}, },
}, },
methods: { methods: {
async mutate() { async mutate() {
this.isLoading = true; this.isLoading = true;
try { try {
const mutation = this.featureSettings; const { mutationSettings } = this;
const { data } = await this.$apollo.mutate(mutation.getMutationPayload(this.projectPath)); const { data } = await this.$apollo.mutate(
const { errors, successPath } = data[mutation.mutationId]; mutationSettings.getMutationPayload(this.projectPath),
);
const { errors, successPath } = data[mutationSettings.mutationId];
if (errors.length > 0) { if (errors.length > 0) {
throw new Error(errors[0]); throw new Error(errors[0]);
...@@ -62,6 +68,22 @@ export default { ...@@ -62,6 +68,22 @@ export default {
} }
}, },
}, },
/**
* Returns a boolean representing whether this component can be rendered for
* the given feature. Useful for parent components to determine whether or
* not to render this component.
* @param {Object} feature The feature to check.
* @returns {boolean}
*/
canRender(feature) {
const { available, configured, canEnableByMergeRequest, type } = feature;
return (
canEnableByMergeRequest &&
available &&
!configured &&
Boolean(mutationSettingsForFeatureType(type))
);
},
i18n: { i18n: {
buttonLabel: s__('SecurityConfiguration|Configure via Merge Request'), buttonLabel: s__('SecurityConfiguration|Configure via Merge Request'),
noSuccessPathError: s__( noSuccessPathError: s__(
......
import { s__ } from '~/locale'; import { s__ } from '~/locale';
import { featureToMutationMap as featureToMutationMapCE } from '~/security_configuration/components/constants'; import { featureToMutationMap as featureToMutationMapCE } from '~/security_configuration/components/constants';
import { import { REPORT_TYPE_DEPENDENCY_SCANNING } from '~/vue_shared/security_reports/constants';
REPORT_TYPE_DEPENDENCY_SCANNING,
REPORT_TYPE_SECRET_DETECTION,
} from '~/vue_shared/security_reports/constants';
import configureDependencyScanningMutation from '../graphql/configure_dependency_scanning.mutation.graphql'; import configureDependencyScanningMutation from '../graphql/configure_dependency_scanning.mutation.graphql';
import configureSecretDetectionMutation from '../graphql/configure_secret_detection.mutation.graphql';
export const SMALL = 'SMALL'; export const SMALL = 'SMALL';
export const MEDIUM = 'MEDIUM'; export const MEDIUM = 'MEDIUM';
...@@ -36,17 +32,6 @@ export const featureToMutationMap = { ...@@ -36,17 +32,6 @@ export const featureToMutationMap = {
}, },
}), }),
}, },
[REPORT_TYPE_SECRET_DETECTION]: {
mutationId: 'configureSecretDetection',
getMutationPayload: (projectPath) => ({
mutation: configureSecretDetectionMutation,
variables: {
input: {
projectPath,
},
},
}),
},
}; };
export const CONFIGURATION_SNIPPET_MODAL_ID = 'CONFIGURATION_SNIPPET_MODAL_ID'; export const CONFIGURATION_SNIPPET_MODAL_ID = 'CONFIGURATION_SNIPPET_MODAL_ID';
...@@ -3,6 +3,7 @@ import { mount } from '@vue/test-utils'; ...@@ -3,6 +3,7 @@ import { mount } from '@vue/test-utils';
import { extendedWrapper } from 'helpers/vue_test_utils_helper'; import { extendedWrapper } from 'helpers/vue_test_utils_helper';
import FeatureCard from '~/security_configuration/components/feature_card.vue'; import FeatureCard from '~/security_configuration/components/feature_card.vue';
import ManageViaMr from '~/vue_shared/security_configuration/components/manage_via_mr.vue'; import ManageViaMr from '~/vue_shared/security_configuration/components/manage_via_mr.vue';
import { REPORT_TYPE_SAST } from '~/vue_shared/security_reports/constants';
import { makeFeature } from './utils'; import { makeFeature } from './utils';
describe('FeatureCard component', () => { describe('FeatureCard component', () => {
...@@ -126,21 +127,23 @@ describe('FeatureCard component', () => { ...@@ -126,21 +127,23 @@ describe('FeatureCard component', () => {
describe('actions', () => { describe('actions', () => {
describe.each` describe.each`
context | available | configured | configurationPath | canEnableByMergeRequest | action context | type | available | configured | configurationPath | canEnableByMergeRequest | action
${'unavailable'} | ${false} | ${false} | ${null} | ${false} | ${null} ${'unavailable'} | ${REPORT_TYPE_SAST} | ${false} | ${false} | ${null} | ${false} | ${null}
${'available'} | ${true} | ${false} | ${null} | ${false} | ${'guide'} ${'available'} | ${REPORT_TYPE_SAST} | ${true} | ${false} | ${null} | ${false} | ${'guide'}
${'configured'} | ${true} | ${true} | ${null} | ${false} | ${'guide'} ${'configured'} | ${REPORT_TYPE_SAST} | ${true} | ${true} | ${null} | ${false} | ${'guide'}
${'available, can enable by MR'} | ${true} | ${false} | ${null} | ${true} | ${'create-mr'} ${'available, can enable by MR'} | ${REPORT_TYPE_SAST} | ${true} | ${false} | ${null} | ${true} | ${'create-mr'}
${'configured, can enable by MR'} | ${true} | ${true} | ${null} | ${true} | ${'guide'} ${'available, can enable by MR, unknown type'} | ${'foo'} | ${true} | ${false} | ${null} | ${true} | ${'guide'}
${'available with config path'} | ${true} | ${false} | ${'foo'} | ${false} | ${'enable'} ${'configured, can enable by MR'} | ${REPORT_TYPE_SAST} | ${true} | ${true} | ${null} | ${true} | ${'guide'}
${'available with config path, can enable by MR'} | ${true} | ${false} | ${'foo'} | ${true} | ${'enable'} ${'available with config path'} | ${REPORT_TYPE_SAST} | ${true} | ${false} | ${'foo'} | ${false} | ${'enable'}
${'configured with config path'} | ${true} | ${true} | ${'foo'} | ${false} | ${'configure'} ${'available with config path, can enable by MR'} | ${REPORT_TYPE_SAST} | ${true} | ${false} | ${'foo'} | ${true} | ${'enable'}
${'configured with config path, can enable by MR'} | ${true} | ${true} | ${'foo'} | ${true} | ${'configure'} ${'configured with config path'} | ${REPORT_TYPE_SAST} | ${true} | ${true} | ${'foo'} | ${false} | ${'configure'}
${'configured with config path, can enable by MR'} | ${REPORT_TYPE_SAST} | ${true} | ${true} | ${'foo'} | ${true} | ${'configure'}
`( `(
'given $context feature', 'given $context feature',
({ available, configured, configurationPath, canEnableByMergeRequest, action }) => { ({ type, available, configured, configurationPath, canEnableByMergeRequest, action }) => {
beforeEach(() => { beforeEach(() => {
feature = makeFeature({ feature = makeFeature({
type,
available, available,
configured, configured,
configurationPath, configurationPath,
......
...@@ -9,6 +9,7 @@ import waitForPromises from 'helpers/wait_for_promises'; ...@@ -9,6 +9,7 @@ import waitForPromises from 'helpers/wait_for_promises';
import { humanize } from '~/lib/utils/text_utility'; import { humanize } from '~/lib/utils/text_utility';
import { redirectTo } from '~/lib/utils/url_utility'; import { redirectTo } from '~/lib/utils/url_utility';
import ManageViaMr from '~/vue_shared/security_configuration/components/manage_via_mr.vue'; import ManageViaMr from '~/vue_shared/security_configuration/components/manage_via_mr.vue';
import { REPORT_TYPE_SAST } from '~/vue_shared/security_reports/constants';
import { buildConfigureSecurityFeatureMockFactory } from './apollo_mocks'; import { buildConfigureSecurityFeatureMockFactory } from './apollo_mocks';
jest.mock('~/lib/utils/url_utility'); jest.mock('~/lib/utils/url_utility');
...@@ -169,6 +170,29 @@ describe('ManageViaMr component', () => { ...@@ -169,6 +170,29 @@ describe('ManageViaMr component', () => {
}, },
); );
describe('canRender static method', () => {
it.each`
context | type | available | configured | canEnableByMergeRequest | expectedValue
${'an unconfigured feature'} | ${REPORT_TYPE_SAST} | ${true} | ${false} | ${true} | ${true}
${'a configured feature'} | ${REPORT_TYPE_SAST} | ${true} | ${true} | ${true} | ${false}
${'an unavailable feature'} | ${REPORT_TYPE_SAST} | ${false} | ${false} | ${true} | ${false}
${'a feature which cannot be enabled via MR'} | ${REPORT_TYPE_SAST} | ${true} | ${false} | ${false} | ${false}
${'an unknown feature'} | ${'foo'} | ${true} | ${false} | ${true} | ${false}
`(
'given $context returns $expectedValue',
({ type, available, configured, canEnableByMergeRequest, expectedValue }) => {
expect(
ManageViaMr.canRender({
type,
available,
configured,
canEnableByMergeRequest,
}),
).toBe(expectedValue);
},
);
});
describe('button props', () => { describe('button props', () => {
it('passes the variant and category props to the GlButton', () => { it('passes the variant and category props to the GlButton', () => {
const variant = 'danger'; const variant = 'danger';
......
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