Merge branch 'add_validation_for_invalid_branches_scan_policies' into 'master'

Add validation for invalid protected branches for scan result policies

See merge request gitlab-org/gitlab!84107

app/assets/javascripts/api.js

@@ -42,6 +42,7 @@ const Api = {
   projectMergeRequestVersionsPath: '/api/:version/projects/:id/merge_requests/:mrid/versions',
   projectRunnersPath: '/api/:version/projects/:id/runners',
   projectProtectedBranchesPath: '/api/:version/projects/:id/protected_branches',
+  projectProtectedBranchesNamePath: '/api/:version/projects/:id/protected_branches/:name',
   projectSearchPath: '/api/:version/projects/:id/search',
   projectSharePath: '/api/:version/projects/:id/share',
   projectMilestonesPath: '/api/:version/projects/:id/milestones',
@@ -372,6 +373,14 @@ const Api = {
       .then(({ data }) => data);
   },
 
+  projectProtectedBranch(id, branchName) {
+    const url = Api.buildUrl(Api.projectProtectedBranchesNamePath)
+      .replace(':id', encodeURIComponent(id))
+      .replace(':name', branchName);
+
+    return axios.get(url).then(({ data }) => data);
+  },
+
   projectSearch(id, options = {}) {
     const url = Api.buildUrl(Api.projectSearchPath).replace(':id', encodeURIComponent(id));
 

ee/app/assets/javascripts/threat_monitoring/components/policy_editor/constants.js

@@ -36,3 +36,7 @@ export const GRAPHQL_ERROR_MESSAGE = s__(
 );
 
 export const NO_RULE_MESSAGE = s__('SecurityOrchestration|No rules defined - policy will not run.');
+
+export const INVALID_BRANCHES = s__(
+  'SecurityOrchestration|The following branches do not exist on this development project: %{branches}. Please review all branches to ensure the values are accurate before updating this policy.',
+);

ee/app/assets/javascripts/threat_monitoring/components/policy_editor/scan_result_policy/lib/humanize.js

 import { sprintf, s__, n__ } from '~/locale';
-import { NO_RULE_MESSAGE } from '../../constants';
+import { NO_RULE_MESSAGE, INVALID_BRANCHES } from '../../constants';
 import { convertScannersToTitleCase } from '../../utils';
 
 /**
@@ -53,9 +53,7 @@ const humanizeItems = ({
     finalSentence.push(items.join(','));
   } else {
     const lastItem = items.pop();
-    finalSentence.push(items.join(', '));
-    finalSentence.push(s__('SecurityOrchestration| or '));
-    finalSentence.push(lastItem);
+    finalSentence.push(items.join(', '), s__('SecurityOrchestration| or '), lastItem);
   }
 
   if (!hasTextBeforeItems && noun) {
@@ -215,3 +213,14 @@ export const humanizeRules = (rules) => {
   }, []);
   return humanizedRules.length ? humanizedRules : [NO_RULE_MESSAGE];
 };
+
+export const humanizeInvalidBranchesError = (branches) => {
+  const sentence = [];
+  if (branches.length > 1) {
+    const lastBranch = branches.pop();
+    sentence.push(branches.join(', '), s__('SecurityOrchestration| and '), lastBranch);
+  } else {
+    sentence.push(branches.join());
+  }
+  return sprintf(INVALID_BRANCHES, { branches: sentence.join('') });
+};

ee/app/assets/javascripts/threat_monitoring/components/policy_editor/scan_result_policy/lib/index.js

@@ -10,8 +10,7 @@ description: ''
 enabled: false
 rules:
   - type: scan_finding
-    branches:
-      - main
+    branches: []
     scanners:
       - container_scanning
     vulnerabilities_allowed: 0

ee/app/assets/javascripts/threat_monitoring/components/policy_editor/scan_result_policy/scan_result_policy_editor.vue

@@ -8,6 +8,7 @@ import {
   GlFormTextarea,
   GlAlert,
 } from '@gitlab/ui';
+import { mapActions, mapState } from 'vuex';
 import { joinPaths, visitUrl, setUrlFragment } from '~/lib/utils/url_utility';
 import { __, s__ } from '~/locale';
 import {
@@ -29,6 +30,7 @@ import {
   buildRule,
   approversOutOfSync,
   invalidScanners,
+  humanizeInvalidBranchesError,
 } from './lib';
 
 export default {
@@ -111,6 +113,7 @@ export default {
     };
   },
   computed: {
+    ...mapState('scanResultPolicies', ['invalidBranches']),
     originalName() {
       return this.existingPolicy?.name;
     },
@@ -129,7 +132,17 @@ export default {
       return this.policy.rules.length < 5;
     },
   },
+  watch: {
+    invalidBranches(branches) {
+      if (branches.length > 0) {
+        this.handleError(new Error(humanizeInvalidBranchesError([...branches])));
+      } else {
+        this.$emit('error', '');
+      }
+    },
+  },
   methods: {
+    ...mapActions('scanResultPolicies', ['fetchBranches']),
     updateAction(actionIndex, values) {
       this.policy.actions.splice(actionIndex, 1, values);
     },
@@ -218,6 +231,8 @@ export default {
       } else if (mode === EDITOR_MODE_RULE && !this.hasParsingError) {
         if (this.invalidForRuleMode()) {
           this.yamlEditorError = new Error();
+        } else {
+          this.fetchBranches({ branches: this.allBranches(), projectId: this.projectId });
         }
       }
     },
@@ -230,6 +245,9 @@ export default {
         invalidScanners(this.policy.rules)
       );
     },
+    allBranches() {
+      return this.policy.rules.flatMap((rule) => rule.branches);
+    },
   },
 };
 </script>

ee/app/assets/javascripts/threat_monitoring/store/index.js

@@ -4,6 +4,7 @@ import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils';
 import networkPolicies from './modules/network_policies';
 import threatMonitoring from './modules/threat_monitoring';
 import threatMonitoringStatistics from './modules/threat_monitoring_statistics';
+import scanResultPolicies from './modules/scan_result_policies';
 
 Vue.use(Vuex);
 
@@ -28,5 +29,6 @@ export default () =>
         };
       }),
       networkPolicies: networkPolicies(),
+      scanResultPolicies: scanResultPolicies(),
     },
   });

ee/app/assets/javascripts/threat_monitoring/store/modules/scan_result_policies/actions.js

+import Api from 'ee/api';
+import * as types from './mutation_types';
+
+export const fetchBranches = ({ commit, dispatch }, { branches, projectId }) => {
+  const uniqBranches = branches.filter((value, index, self) => self.indexOf(value) === index);
+  commit(types.LOADING_BRANCHES);
+  uniqBranches.forEach((branch) => dispatch('fetchBranch', { branch, projectId }));
+};
+
+export const fetchBranch = ({ commit }, { branch, projectId }) => {
+  return Api.projectProtectedBranch(projectId, branch).catch((error) => {
+    const decomposedUrl = error.config.url.split('/');
+    commit(types.INVALID_BRANCHES, decomposedUrl[decomposedUrl.length - 1]);
+  });
+};

ee/app/assets/javascripts/threat_monitoring/store/modules/scan_result_policies/index.js

+import * as actions from './actions';
+import mutations from './mutations';
+import state from './state';
+
+export default () => ({
+  namespaced: true,
+  actions,
+  mutations,
+  state,
+});

ee/app/assets/javascripts/threat_monitoring/store/modules/scan_result_policies/mutation_types.js

+export const LOADING_BRANCHES = 'LOADING_BRANCHES';
+export const INVALID_BRANCHES = 'INVALID_BRANCHES';

ee/app/assets/javascripts/threat_monitoring/store/modules/scan_result_policies/mutations.js

+import * as types from './mutation_types';
+
+export default {
+  [types.LOADING_BRANCHES](state) {
+    state.invalidBranches = [];
+  },
+  [types.INVALID_BRANCHES](state, invalidBranch) {
+    state.invalidBranches.push(invalidBranch);
+  },
+};

ee/app/assets/javascripts/threat_monitoring/store/modules/scan_result_policies/state.js

+export default () => ({
+  invalidBranches: [],
+});

ee/spec/frontend/threat_monitoring/components/policy_editor/scan_result_policy/lib/humanize_spec.js

 import {
   humanizeRules,
   humanizeAction,
+  humanizeInvalidBranchesError,
 } from 'ee/threat_monitoring/components/policy_editor/scan_result_policy/lib';
 
 import { NO_RULE_MESSAGE } from 'ee/threat_monitoring/components/policy_editor/constants';
@@ -138,4 +139,24 @@ describe('humanizeAction', () => {
       'Require 2 approvals from o.leticia.conner or user with id 5 or members of the group security_group/all_members or members of the group with id 10 if any of the following occur:',
     );
   });
+
+  describe('humanizeInvalidBranchesError', () => {
+    it('returns message without any branch name for an empty array', () => {
+      expect(humanizeInvalidBranchesError([])).toEqual(
+        'The following branches do not exist on this development project: . Please review all branches to ensure the values are accurate before updating this policy.',
+      );
+    });
+
+    it('returns message with a single branch name for an array with single element', () => {
+      expect(humanizeInvalidBranchesError(['main'])).toEqual(
+        'The following branches do not exist on this development project: main. Please review all branches to ensure the values are accurate before updating this policy.',
+      );
+    });
+
+    it('returns message with multiple branch names for array with multiple elements', () => {
+      expect(humanizeInvalidBranchesError(['main', 'protected', 'master'])).toEqual(
+        'The following branches do not exist on this development project: main, protected and master. Please review all branches to ensure the values are accurate before updating this policy.',
+      );
+    });
+  });
 });

ee/spec/frontend/threat_monitoring/components/policy_editor/scan_result_policy/scan_result_policy_editor_spec.js

+import Vuex from 'vuex';
 import { shallowMount } from '@vue/test-utils';
 import { GlEmptyState, GlAlert, GlFormInput, GlFormTextarea, GlToggle } from '@gitlab/ui';
-import { nextTick } from 'vue';
+import Vue from 'vue';
+import MockAdapter from 'axios-mock-adapter';
 import waitForPromises from 'helpers/wait_for_promises';
 import PolicyEditorLayout from 'ee/threat_monitoring/components/policy_editor/policy_editor_layout.vue';
 import {
@@ -24,6 +26,9 @@ import {
 import DimDisableContainer from 'ee/threat_monitoring/components/policy_editor/dim_disable_container.vue';
 import PolicyActionBuilder from 'ee/threat_monitoring/components/policy_editor/scan_result_policy/policy_action_builder.vue';
 import PolicyRuleBuilder from 'ee/threat_monitoring/components/policy_editor/scan_result_policy/policy_rule_builder.vue';
+import ScanResultPoliciesStore from 'ee/threat_monitoring/store/modules/scan_result_policies';
+import axios from '~/lib/utils/axios_utils';
+import httpStatus from '~/lib/utils/http_status';
 
 jest.mock('~/lib/utils/url_utility', () => ({
   joinPaths: jest.requireActual('~/lib/utils/url_utility').joinPaths,
@@ -43,7 +48,10 @@ jest.mock('ee/threat_monitoring/components/policy_editor/utils', () => ({
   modifyPolicy: jest.fn().mockResolvedValue({ id: '2' }),
 }));
 
+Vue.use(Vuex);
+
 describe('ScanResultPolicyEditor', () => {
+  let mock;
   let wrapper;
   const defaultProjectPath = 'path/to/project';
   const policyEditorEmptyStateSvgPath = 'path/to/svg';
@@ -55,7 +63,14 @@ describe('ScanResultPolicyEditor', () => {
   const scanResultPolicyApprovers = [{ id: 1, username: 'the.one', state: 'active' }];
 
   const factory = ({ propsData = {}, provide = {} } = {}) => {
+    const store = new Vuex.Store({
+      modules: {
+        scanResultPolicies: ScanResultPoliciesStore(),
+      },
+    });
+
     wrapper = shallowMount(ScanResultPolicyEditor, {
+      store,
       propsData: {
         assignedPolicyProject: DEFAULT_ASSIGNED_POLICY_PROJECT,
         ...propsData,
@@ -70,7 +85,7 @@ describe('ScanResultPolicyEditor', () => {
         ...provide,
       },
     });
-    nextTick();
+    Vue.nextTick();
   };
 
   const factoryWithExistingPolicy = (policy = {}) => {
@@ -96,7 +111,12 @@ describe('ScanResultPolicyEditor', () => {
   const findYamlPreview = () => wrapper.find('[data-testid="yaml-preview"]');
   const findAllRuleBuilders = () => wrapper.findAllComponents(PolicyRuleBuilder);
 
+  beforeEach(() => {
+    mock = new MockAdapter(axios);
+  });
+
   afterEach(() => {
+    mock.restore();
     wrapper.destroy();
   });
 
@@ -110,7 +130,7 @@ describe('ScanResultPolicyEditor', () => {
       );
 
       findPolicyEditorLayout().vm.$emit('update-yaml', newManifest);
-      await nextTick();
+      await Vue.nextTick();
 
       expect(findPolicyEditorLayout().attributes('yamleditorvalue')).toBe(newManifest);
     });
@@ -128,7 +148,7 @@ describe('ScanResultPolicyEditor', () => {
       expect(findAlert().exists()).toBe(false);
 
       findPolicyEditorLayout().vm.$emit('update-yaml', 'invalid manifest');
-      await nextTick();
+      await Vue.nextTick();
 
       expect(findAlert().exists()).toBe(true);
     });
@@ -137,7 +157,7 @@ describe('ScanResultPolicyEditor', () => {
       await factory();
 
       findPolicyEditorLayout().vm.$emit('update-yaml', 'invalid manifest');
-      await nextTick();
+      await Vue.nextTick();
 
       expect(findNameInput().attributes('disabled')).toBe('true');
       expect(findDescriptionTextArea().attributes('disabled')).toBe('true');
@@ -177,7 +197,7 @@ describe('ScanResultPolicyEditor', () => {
 
         currentComponent().vm.$emit(event, newValue);
         findPolicyEditorLayout().vm.$emit('update-editor-mode', EDITOR_MODE_YAML);
-        await nextTick();
+        await Vue.nextTick();
 
         expect(findPolicyEditorLayout().attributes('yamleditorvalue')).toMatch(newValue.toString());
       });
@@ -186,7 +206,7 @@ describe('ScanResultPolicyEditor', () => {
         await factory();
 
         currentComponent().vm.$emit(event, newValue);
-        await nextTick();
+        await Vue.nextTick();
 
         expect(findYamlPreview().html()).toMatch(newValue.toString());
       });
@@ -225,25 +245,21 @@ describe('ScanResultPolicyEditor', () => {
     it('adds a new rule', async () => {
       const rulesCount = 1;
       factory();
-      await nextTick();
+      await Vue.nextTick();
 
       expect(findAllRuleBuilders().length).toBe(rulesCount);
 
       findAddRuleButton().vm.$emit('click');
-      await nextTick();
+      await Vue.nextTick();
 
       expect(findAllRuleBuilders()).toHaveLength(rulesCount + 1);
     });
 
     it('hides add button when the limit of five rules has been reached', async () => {
       const limit = 5;
-      factory();
-      await nextTick();
-      findAddRuleButton().vm.$emit('click');
-      findAddRuleButton().vm.$emit('click');
-      findAddRuleButton().vm.$emit('click');
-      findAddRuleButton().vm.$emit('click');
-      await nextTick();
+      const rule = mockScanResultObject.rules[0];
+      factoryWithExistingPolicy({ rules: [rule, rule, rule, rule, rule] });
+      await Vue.nextTick();
 
       expect(findAllRuleBuilders()).toHaveLength(limit);
       expect(findAddRuleButton().exists()).toBe(false);
@@ -259,9 +275,9 @@ describe('ScanResultPolicyEditor', () => {
         vulnerability_states: [],
       };
       factory();
-      await nextTick();
+      await Vue.nextTick();
       findAllRuleBuilders().at(0).vm.$emit('changed', newValue);
-      await nextTick();
+      await Vue.nextTick();
 
       expect(wrapper.vm.policy.rules[0]).toEqual(newValue);
       expect(findYamlPreview().html()).toMatch('vulnerabilities_allowed: 1');
@@ -270,12 +286,12 @@ describe('ScanResultPolicyEditor', () => {
     it('deletes the initial rule', async () => {
       const initialRuleCount = 1;
       factory();
-      await nextTick();
+      await Vue.nextTick();
 
       expect(findAllRuleBuilders()).toHaveLength(initialRuleCount);
 
       findAllRuleBuilders().at(0).vm.$emit('remove', 0);
-      await nextTick();
+      await Vue.nextTick();
 
       expect(findAllRuleBuilders()).toHaveLength(initialRuleCount - 1);
     });
@@ -297,7 +313,7 @@ describe('ScanResultPolicyEditor', () => {
     it('renders a single policy action builder', async () => {
       factory();
 
-      await nextTick();
+      await Vue.nextTick();
 
       expect(findAllPolicyActionBuilders()).toHaveLength(1);
       expect(findPolicyActionBuilder().props('existingApprovers')).toEqual(
@@ -309,9 +325,9 @@ describe('ScanResultPolicyEditor', () => {
       const UPDATED_ACTION = { type: 'required_approval', group_approvers_ids: [1] };
       factory();
 
-      await nextTick();
+      await Vue.nextTick();
       findPolicyActionBuilder().vm.$emit('changed', UPDATED_ACTION);
-      await nextTick();
+      await Vue.nextTick();
 
       expect(findPolicyActionBuilder().props('initAction')).toEqual(UPDATED_ACTION);
     });
@@ -347,4 +363,25 @@ describe('ScanResultPolicyEditor', () => {
       expect(findAlert().exists()).toBe(true);
     });
   });
+
+  it.each`
+    status                  | errorMessage
+    ${httpStatus.OK}        | ${''}
+    ${httpStatus.NOT_FOUND} | ${'The following branches do not exist on this development project: main. Please review all branches to ensure the values are accurate before updating this policy.'}
+  `(
+    'triggers error event with content: "$errorMessage" when http status is $status',
+    async ({ status, errorMessage }) => {
+      const rule = { ...mockScanResultObject.rules[0], branches: ['main'] };
+
+      mock.onGet('/api/undefined/projects/1/protected_branches/main').replyOnce(status, {});
+
+      factoryWithExistingPolicy({ rules: [rule] });
+
+      await findPolicyEditorLayout().vm.$emit('update-editor-mode', EDITOR_MODE_RULE);
+      await waitForPromises();
+      const errors = wrapper.emitted('error');
+
+      expect(errors[errors.length - 1]).toEqual([errorMessage]);
+    },
+  );
 });

ee/spec/frontend/threat_monitoring/mocks/mock_data.js

@@ -231,8 +231,7 @@ description: This policy enforces critical vulnerability CS approvals
 enabled: true
 rules:
   - type: scan_finding
-    branches:
-      - main
+    branches: []
     scanners:
       - container_scanning
     vulnerabilities_allowed: 1
@@ -255,7 +254,7 @@ export const mockScanResultObject = {
   rules: [
     {
       type: 'scan_finding',
-      branches: ['main'],
+      branches: [],
       scanners: ['container_scanning'],
       vulnerabilities_allowed: 1,
       severity_levels: ['critical'],

ee/spec/frontend/threat_monitoring/store/modules/scan_result_policies/actions_spec.js

+import MockAdapter from 'axios-mock-adapter';
+import * as actions from 'ee/threat_monitoring/store/modules/scan_result_policies/actions';
+import * as types from 'ee/threat_monitoring/store/modules/scan_result_policies/mutation_types';
+import getInitialState from 'ee/threat_monitoring/store/modules/scan_result_policies/state';
+import testAction from 'helpers/vuex_action_helper';
+import axios from '~/lib/utils/axios_utils';
+import httpStatus from '~/lib/utils/http_status';
+
+describe('ScanResultPolicies actions', () => {
+  let state;
+  let mock;
+  const projectId = 3;
+  const branchName = 'main';
+  const branches = [branchName];
+  const duplicatedBranches = [branchName, branchName];
+  const apiEndpoint = `/api/undefined/projects/${projectId}/protected_branches/${branchName}`;
+
+  beforeEach(() => {
+    state = getInitialState();
+    mock = new MockAdapter(axios);
+  });
+
+  afterEach(() => {
+    mock.restore();
+  });
+
+  describe('fetchBranches', () => {
+    it('commits LOADING_BRANCHES and dispatch calls to fetchBranch', () =>
+      testAction(
+        actions.fetchBranches,
+        { branches, projectId },
+        state,
+        [
+          {
+            type: types.LOADING_BRANCHES,
+          },
+        ],
+        [
+          {
+            type: 'fetchBranch',
+            payload: {
+              branch: branchName,
+              projectId,
+            },
+          },
+        ],
+      ));
+
+    it('only considers unique branches', () =>
+      testAction(
+        actions.fetchBranches,
+        { branches: duplicatedBranches, projectId },
+        state,
+        [
+          {
+            type: types.LOADING_BRANCHES,
+          },
+        ],
+        [
+          {
+            type: 'fetchBranch',
+            payload: {
+              branch: branchName,
+              projectId,
+            },
+          },
+        ],
+      ));
+  });
+
+  describe('fetchBranch', () => {
+    it.each`
+      status                  | mutations
+      ${httpStatus.OK}        | ${[]}
+      ${httpStatus.NOT_FOUND} | ${[{ type: types.INVALID_BRANCHES, payload: branchName }]}
+    `('triggers $mutations.length mutation when status is $status', ({ status, mutations }) => {
+      mock.onGet(apiEndpoint).replyOnce(status);
+      testAction(actions.fetchBranch, { branch: branchName, projectId }, state, mutations, []);
+    });
+  });
+});

ee/spec/frontend/threat_monitoring/store/modules/scan_result_policies/mutations_spec.js

+import * as types from 'ee/threat_monitoring/store/modules/scan_result_policies/mutation_types';
+import mutations from 'ee/threat_monitoring/store/modules/scan_result_policies/mutations';
+import getInitialState from 'ee/threat_monitoring/store/modules/scan_result_policies/state';
+
+describe('ScanResultPolicies mutations', () => {
+  let state;
+
+  beforeEach(() => {
+    state = getInitialState();
+  });
+
+  describe(types.LOADING_BRANCHES, () => {
+    it('resets invalid branches', () => {
+      mutations[types.LOADING_BRANCHES](state);
+      expect(state.invalidBranches).toEqual([]);
+    });
+  });
+
+  describe(types.INVALID_BRANCHES, () => {
+    it('appends new branches into invalidBranches', () => {
+      const branchName = 'main';
+      mutations[types.INVALID_BRANCHES](state, branchName);
+      expect(state.invalidBranches).toEqual([branchName]);
+    });
+  });
+});

locale/gitlab.pot

@@ -33404,6 +33404,9 @@ msgstr ""
 msgid "SecurityConfiguration|Vulnerability details and statistics in the merge request"
 msgstr ""
 
+msgid "SecurityOrchestration| and "
+msgstr ""
+
 msgid "SecurityOrchestration| or "
 msgstr ""
 
@@ -33632,6 +33635,9 @@ msgstr ""
 msgid "SecurityOrchestration|Summary"
 msgstr ""
 
+msgid "SecurityOrchestration|The following branches do not exist on this development project: %{branches}. Please review all branches to ensure the values are accurate before updating this policy."
+msgstr ""
+
 msgid "SecurityOrchestration|There was a problem creating the new security policy"
 msgstr ""
 

spec/frontend/api_spec.js

@@ -1733,4 +1733,36 @@ describe('Api', () => {
       });
     });
   });
+
+  describe('projectProtectedBranch', () => {
+    const branchName = 'new-branch-name';
+    const dummyProjectId = 5;
+    const expectedUrl = `${dummyUrlRoot}/api/${dummyApiVersion}/projects/${dummyProjectId}/protected_branches/${branchName}`;
+
+    it('returns 404 for non-existing branch', () => {
+      jest.spyOn(axios, 'get');
+
+      mock.onGet(expectedUrl).replyOnce(httpStatus.NOT_FOUND, {
+        message: '404 Not found',
+      });
+
+      return Api.projectProtectedBranch(dummyProjectId, branchName).catch((error) => {
+        expect(error.response.status).toBe(httpStatus.NOT_FOUND);
+        expect(axios.get).toHaveBeenCalledWith(expectedUrl);
+      });
+    });
+
+    it('returns 200 with branch information', () => {
+      const expectedObj = { name: branchName };
+
+      jest.spyOn(axios, 'get');
+
+      mock.onGet(expectedUrl).replyOnce(httpStatus.OK, expectedObj);
+
+      return Api.projectProtectedBranch(dummyProjectId, branchName).then((data) => {
+        expect(data).toEqual(expectedObj);
+        expect(axios.get).toHaveBeenCalledWith(expectedUrl);
+      });
+    });
+  });
 });