Commit 1a87f7fb authored by Zamir Martins's avatar Zamir Martins Committed by Natalia Tepluhina

Add validation for rule/yaml modes switch

it also fix a related inconsistency.

EE: true
Changelog: fixed
parent 7148f7c5
......@@ -53,3 +53,47 @@ export function decomposeApprovers(action, approvers) {
);
return { ...newAction, ...approversInfo };
}
/*
Check if users are present in approvers
*/
function usersOutOfSync(action, approvers) {
const users = approvers.filter((approver) => approver.type === USER_TYPE);
const usersIDs =
action?.user_approvers_ids?.some((id) => !users.find((approver) => approver.id === id)) ||
false;
const usersNames =
action?.user_approvers?.some(
(userName) => !users.find((approver) => approver.username === userName),
) || false;
const userLength =
(action?.user_approvers?.length || 0) + (action?.user_approvers_ids?.length || 0);
return usersIDs || usersNames || userLength !== users.length;
}
/*
Check if groups are present in approvers
*/
function groupsOutOfSync(action, approvers) {
const groups = approvers.filter((approver) => approver.type === GROUP_TYPE);
const groupsIDs =
action?.group_approvers_ids?.some((id) => !groups.find((approver) => approver.id === id)) ||
false;
const groupsPaths =
action?.group_approvers?.some(
(path) => !groups.find((approver) => approver.full_path === path),
) || false;
const groupLength =
(action?.group_approvers?.length || 0) + (action?.group_approvers_ids?.length || 0);
return groupsIDs || groupsPaths || groupLength !== groups.length;
}
/*
Check if yaml is out of sync with available approvers
*/
export function approversOutOfSync(action, existingApprovers) {
const approvers = groupApprovers(existingApprovers);
return usersOutOfSync(action, approvers) || groupsOutOfSync(action, approvers);
}
export { fromYaml } from './from_yaml';
export { toYaml } from './to_yaml';
export { buildRule } from './rules';
export { approversOutOfSync } from './actions';
export * from './humanize';
export const DEFAULT_SCAN_RESULT_POLICY = `type: scan_result_policy
......
......@@ -46,6 +46,7 @@ export default {
watch: {
approvers(values) {
this.action = decomposeApprovers(this.action, values);
this.$emit('approversUpdated', this.approvers);
},
approversToAdd(val) {
this.approvers.push(val[0]);
......
......@@ -22,7 +22,7 @@ import { assignSecurityPolicyProject, modifyPolicy } from '../utils';
import DimDisableContainer from '../dim_disable_container.vue';
import PolicyActionBuilder from './policy_action_builder.vue';
import PolicyRuleBuilder from './policy_rule_builder.vue';
import { DEFAULT_SCAN_RESULT_POLICY, fromYaml, toYaml, buildRule } from './lib';
import { DEFAULT_SCAN_RESULT_POLICY, fromYaml, toYaml, buildRule, approversOutOfSync } from './lib';
export default {
SECURITY_POLICY_ACTIONS,
......@@ -100,6 +100,7 @@ export default {
),
yamlEditorError: null,
mode: EDITOR_MODE_RULE,
existingApprovers: this.scanResultPolicyApprovers,
};
},
computed: {
......@@ -207,8 +208,15 @@ export default {
this.mode = mode;
if (mode === EDITOR_MODE_YAML && !this.hasParsingError) {
this.yamlEditorValue = toYaml(this.policy);
} else if (mode === EDITOR_MODE_RULE && !this.hasParsingError) {
if (approversOutOfSync(this.policy.actions[0], this.existingApprovers)) {
this.yamlEditorError = new Error();
}
}
},
updatePolicyApprovers(values) {
this.existingApprovers = values;
},
},
};
</script>
......@@ -296,8 +304,9 @@ export default {
:key="index"
class="gl-mb-4"
:init-action="action"
:existing-approvers="scanResultPolicyApprovers"
:existing-approvers="existingApprovers"
@changed="updateAction(index, $event)"
@approversUpdated="updatePolicyApprovers"
/>
</dim-disable-container>
</template>
......
......@@ -3,6 +3,7 @@ import {
userIds,
groupApprovers,
decomposeApprovers,
approversOutOfSync,
} from 'ee/threat_monitoring/components/policy_editor/scan_result_policy/lib/actions';
// As returned by endpoints based on API::Entities::UserBasic
......@@ -10,7 +11,7 @@ const userApprover = {
id: 1,
name: null,
state: null,
username: null,
username: 'user name',
avatar_url: null,
web_url: null,
};
......@@ -20,7 +21,7 @@ const groupApprover = {
id: 2,
name: null,
full_name: null,
full_path: null,
full_path: 'full path',
avatar_url: null,
web_url: null,
};
......@@ -48,13 +49,13 @@ describe('groupApprovers', () => {
name: null,
state: null,
type: 'user',
username: null,
username: 'user name',
web_url: null,
},
{
avatar_url: null,
full_name: null,
full_path: null,
full_path: 'full path',
id: groupApprover.id,
name: null,
type: 'group',
......@@ -79,7 +80,7 @@ describe('groupApprovers', () => {
{
avatar_url: null,
full_name: null,
full_path: null,
full_path: 'full path',
id: groupApprover.id,
name: null,
type: 'group',
......@@ -96,7 +97,7 @@ describe('groupApprovers', () => {
name: null,
state: null,
type: 'user',
username: null,
username: 'user name',
web_url: null,
},
]);
......@@ -168,3 +169,174 @@ describe('groupIds', () => {
expect(groupIds(groupedApprovers)).toStrictEqual([groupApprover.id]);
});
});
describe('approversOutOfSync', () => {
describe('with user_approvers_ids only', () => {
it.each`
ids | approvers | result
${[1]} | ${[userApprover]} | ${false}
${[]} | ${[]} | ${false}
${[]} | ${[userApprover]} | ${true}
${[1]} | ${[]} | ${true}
${[1, 2]} | ${[userApprover]} | ${true}
${[2]} | ${[]} | ${true}
${[2]} | ${[userApprover]} | ${true}
`(
'return $result when ids and approvers length equal to $ids and $approvers.length',
({ ids, approvers, result }) => {
const action = {
approvals_required: 1,
type: 'require_approval',
user_approvers_ids: ids,
};
expect(approversOutOfSync(action, approvers)).toBe(result);
},
);
});
describe('with user_approvers only', () => {
it.each`
usernames | approvers | result
${['user name']} | ${[userApprover]} | ${false}
${[]} | ${[]} | ${false}
${[]} | ${[userApprover]} | ${true}
${['user name']} | ${[]} | ${true}
${['user name', 'not present']} | ${[userApprover]} | ${true}
${['not present']} | ${[]} | ${true}
${['not present']} | ${[userApprover]} | ${true}
`(
'return $result when usernames and approvers length equal to $usernames and $approvers.length',
({ usernames, approvers, result }) => {
const action = {
approvals_required: 1,
type: 'require_approval',
user_approvers: usernames,
};
expect(approversOutOfSync(action, approvers)).toBe(result);
},
);
});
describe('with user_approvers and user_approvers_ids', () => {
it.each`
ids | usernames | approvers | result
${[]} | ${['user name']} | ${[userApprover]} | ${false}
${[1]} | ${[]} | ${[userApprover]} | ${false}
${[]} | ${[]} | ${[]} | ${false}
${[1]} | ${['user name']} | ${[userApprover]} | ${true}
${[1]} | ${['not present']} | ${[userApprover]} | ${true}
${[2]} | ${['user name']} | ${[userApprover]} | ${true}
`(
'return $result when ids, usernames and approvers length equal to $ids, $usernames and $approvers.length',
({ ids, usernames, approvers, result }) => {
const action = {
approvals_required: 1,
type: 'require_approval',
user_approvers: usernames,
user_approvers_ids: ids,
};
expect(approversOutOfSync(action, approvers)).toBe(result);
},
);
});
describe('with group_approvers_ids only', () => {
it.each`
ids | approvers | result
${[2]} | ${[groupApprover]} | ${false}
${[]} | ${[]} | ${false}
${[]} | ${[groupApprover]} | ${true}
${[2]} | ${[]} | ${true}
${[2, 3]} | ${[groupApprover]} | ${true}
${[3]} | ${[]} | ${true}
${[3]} | ${[groupApprover]} | ${true}
`(
'return $result when ids and approvers length equal to $ids and $approvers.length',
({ ids, approvers, result }) => {
const action = {
approvals_required: 1,
type: 'require_approval',
group_approvers_ids: ids,
};
expect(approversOutOfSync(action, approvers)).toBe(result);
},
);
});
describe('with user_approvers, user_approvers_ids and group_approvers_ids', () => {
it.each`
user_ids | usernames | group_ids | approvers | result
${[]} | ${['user name']} | ${[2]} | ${allApprovers} | ${false}
${[1]} | ${[]} | ${[2]} | ${allApprovers} | ${false}
${[]} | ${[]} | ${[]} | ${[]} | ${false}
${[1]} | ${['user name']} | ${[2]} | ${allApprovers} | ${true}
${[]} | ${['user name']} | ${[3]} | ${allApprovers} | ${true}
${[1]} | ${[]} | ${[3]} | ${allApprovers} | ${true}
${[]} | ${[]} | ${[2]} | ${[groupApprover]} | ${false}
${[1]} | ${[]} | ${[2]} | ${[groupApprover]} | ${true}
${[]} | ${['user name']} | ${[2]} | ${[groupApprover]} | ${true}
${[]} | ${['user name']} | ${[]} | ${[userApprover]} | ${false}
${[1]} | ${[]} | ${[]} | ${[userApprover]} | ${false}
${[1]} | ${[]} | ${[2]} | ${[userApprover]} | ${true}
`(
'return $result when user_ids, usernames, group_ids and approvers length equal to $user_ids, $usernames, $group_ids and $approvers.length',
({ user_ids, usernames, group_ids, approvers, result }) => {
const action = {
approvals_required: 1,
type: 'require_approval',
user_approvers: usernames,
user_approvers_ids: user_ids,
group_approvers_ids: group_ids,
};
expect(approversOutOfSync(action, approvers)).toBe(result);
},
);
});
describe('with group_approvers only', () => {
it.each`
fullPath | approvers | result
${['full path']} | ${[groupApprover]} | ${false}
${[]} | ${[]} | ${false}
${[]} | ${[groupApprover]} | ${true}
${['full path']} | ${[]} | ${true}
${['full path', 'not present']} | ${[groupApprover]} | ${true}
${['not present']} | ${[]} | ${true}
${['not present']} | ${[groupApprover]} | ${true}
`(
'return $result when fullPath and approvers length equal to $fullPath and $approvers.length',
({ fullPath, approvers, result }) => {
const action = {
approvals_required: 1,
type: 'require_approval',
group_approvers: fullPath,
};
expect(approversOutOfSync(action, approvers)).toBe(result);
},
);
});
describe('with user_approvers, user_approvers_ids, group_approvers_ids and group_approvers', () => {
it.each`
user_ids | usernames | group_ids | group_paths | approvers | result
${[]} | ${['user name']} | ${[2]} | ${[]} | ${allApprovers} | ${false}
${[1]} | ${[]} | ${[2]} | ${[]} | ${allApprovers} | ${false}
${[1]} | ${[]} | ${[]} | ${['full path']} | ${allApprovers} | ${false}
${[]} | ${['user name']} | ${[]} | ${['full path']} | ${allApprovers} | ${false}
${[]} | ${[]} | ${[]} | ${[]} | ${[]} | ${false}
${[]} | ${['user name']} | ${[3]} | ${[]} | ${allApprovers} | ${true}
${[1]} | ${[]} | ${[3]} | ${[]} | ${allApprovers} | ${true}
${[1]} | ${[]} | ${[]} | ${['not present']} | ${allApprovers} | ${true}
${[]} | ${['user name']} | ${[]} | ${['not present']} | ${allApprovers} | ${true}
${[1]} | ${[]} | ${[]} | ${['full path']} | ${[groupApprovers]} | ${true}
${[]} | ${['user name']} | ${[]} | ${['full path']} | ${[groupApprovers]} | ${true}
`(
'return $result when user_ids, usernames, group_ids, group_paths and approvers length equal to $user_ids, $usernames, $group_ids, $group_paths and $approvers.length',
({ user_ids, usernames, group_ids, group_paths, approvers, result }) => {
const action = {
approvals_required: 1,
type: 'require_approval',
user_approvers: usernames,
user_approvers_ids: user_ids,
group_approvers_ids: group_ids,
group_approvers: group_paths,
};
expect(approversOutOfSync(action, approvers)).toBe(result);
},
);
});
});
......@@ -52,7 +52,7 @@ describe('ScanResultPolicyEditor', () => {
branch: 'main',
fullPath: 'path/to/existing-project',
};
const scanResultPolicyApprovers = [{ id: 1, username: 'username', state: 'active' }];
const scanResultPolicyApprovers = [{ id: 1, username: 'the.one', state: 'active' }];
const factory = ({ propsData = {}, provide = {} } = {}) => {
wrapper = shallowMount(ScanResultPolicyEditor, {
......@@ -315,5 +315,26 @@ describe('ScanResultPolicyEditor', () => {
expect(findPolicyActionBuilder().props('initAction')).toEqual(UPDATED_ACTION);
});
it('does not show alert when policy matches existing approvers', async () => {
factoryWithExistingPolicy();
expect(findAlert().exists()).toBe(false);
await findPolicyEditorLayout().vm.$emit('update-editor-mode', EDITOR_MODE_RULE);
expect(findAlert().exists()).toBe(false);
});
it('shows alert when policy does not match existing approvers', async () => {
factory();
expect(findAlert().exists()).toBe(false);
await findPolicyEditorLayout().vm.$emit('update-editor-mode', EDITOR_MODE_RULE);
expect(findAlert().exists()).toBe(true);
expect(findAlert().isVisible()).toBe(true);
});
});
});
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