Merge branch...

Merge branch '220573-frontend-modify-secure-configuration-page-to-provide-entry-point-to-sast-config-ui-page-2' into 'master'

Add button to Security Configuration to enable SAST scanning

See merge request gitlab-org/gitlab!36432

doc/user/application_security/configuration/index.md

@@ -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,

ee/app/assets/javascripts/security_configuration/components/app.vue

@@ -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,15 +174,25 @@ export default {
         <div class="gl-text-gray-900">{{ item.name }}</div>
         <div>
           {{ item.description }}
-          <gl-link
-            target="_blank"
-            :href="item.link"
-            :aria-label="getFeatureDocumentationLinkLabel(item.name)"
-          >
-            {{ __('More information') }}
-          </gl-link>
         </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)"
+        >
+          {{ s__('SecurityConfiguration|See documentation') }}
+        </gl-link>
+      </template>
     </gl-table>
     <auto-fix-settings v-if="glFeatures.securityAutoFix" v-bind="autoFixSettingsProps" />
   </article>

ee/app/assets/javascripts/security_configuration/components/create_merge_request_button.vue

+<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>

ee/app/assets/javascripts/security_configuration/index.js

@@ -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',

ee/app/controllers/projects/security/sast_configuration_controller.rb

@@ -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

ee/app/presenters/projects/security/configuration_presenter.rb

@@ -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]),

ee/changelogs/unreleased/220573-frontend-modify-secure-configuration-page-to-provide-entry-poin.yml

+---
+title: Add "Manage" column and tweak "Status" wording in the Security Configuration page
+merge_request: 36432
+author:
+type: changed

ee/spec/frontend/security_configuration/components/app_spec.js

@@ -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);
+          });
+        }
+      },
+    );
   });
 });

ee/spec/frontend/security_configuration/components/create_merge_request_button_spec.js

+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);
+        });
+      });
+    });
+  });
+});

ee/spec/presenters/projects/security/configuration_presenter_spec.rb

@@ -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]),

locale/gitlab.pot

@@ -20567,18 +20567,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 ""