Commit b40ee4d1 authored by Zamir Martins's avatar Zamir Martins Committed by Bob Van Landuyt

Add approvers when editing scan result policies

taking into consideration the existing feature
flag. It also split a couple of fields which
were previously only used for scan execution scan.

Changelog: changed
EE: true
parent f68a9fdc
<script>
import { GlEmptyState } from '@gitlab/ui';
import { joinPaths, visitUrl } from '~/lib/utils/url_utility';
import { joinPaths, visitUrl, setUrlFragment } from '~/lib/utils/url_utility';
import { __, s__ } from '~/locale';
import {
EDITOR_MODES,
......@@ -28,11 +28,11 @@ export default {
PolicyEditorLayout,
},
inject: [
'disableScanExecutionUpdate',
'disableScanPolicyUpdate',
'policyEditorEmptyStateSvgPath',
'projectId',
'projectPath',
'scanExecutionDocumentationPath',
'scanPolicyDocumentationPath',
],
props: {
assignedPolicyProject: {
......@@ -62,6 +62,10 @@ export default {
newlyCreatedPolicyProject: null,
policy: fromYaml(yamlEditorValue),
yamlEditorValue,
documentationPath: setUrlFragment(
this.scanPolicyDocumentationPath,
'scan-execution-policy-editor',
),
};
},
computed: {
......@@ -137,7 +141,7 @@ export default {
<template>
<policy-editor-layout
v-if="!disableScanExecutionUpdate"
v-if="!disableScanPolicyUpdate"
:custom-save-button-text="$options.i18n.createMergeRequest"
:default-editor-mode="$options.DEFAULT_EDITOR_MODE"
:editor-modes="$options.EDITOR_MODES"
......@@ -153,7 +157,7 @@ export default {
<gl-empty-state
v-else
:description="$options.i18n.notOwnerDescription"
:primary-button-link="scanExecutionDocumentationPath"
:primary-button-link="documentationPath"
:primary-button-text="$options.i18n.notOwnerButtonText"
:svg-path="policyEditorEmptyStateSvgPath"
title=""
......
......@@ -16,10 +16,9 @@ rules:
severity_levels:
- critical
vulnerability_states:
- newly_added
- newly_detected
actions:
- type: require_approval
approvals_required: 1
user_approvers:
- security_user
user_approvers: []
`;
<script>
import { GlEmptyState } from '@gitlab/ui';
import { joinPaths, visitUrl } from '~/lib/utils/url_utility';
import { joinPaths, visitUrl, setUrlFragment } from '~/lib/utils/url_utility';
import { __, s__ } from '~/locale';
import {
EDITOR_MODES,
......@@ -28,11 +28,12 @@ export default {
PolicyEditorLayout,
},
inject: [
'disableScanExecutionUpdate',
'disableScanPolicyUpdate',
'policyEditorEmptyStateSvgPath',
'projectId',
'projectPath',
'scanExecutionDocumentationPath',
'scanPolicyDocumentationPath',
'scanResultPolicyApprovers',
],
props: {
assignedPolicyProject: {
......@@ -62,6 +63,10 @@ export default {
newlyCreatedPolicyProject: null,
policy: fromYaml(yamlEditorValue),
yamlEditorValue,
documentationPath: setUrlFragment(
this.scanPolicyDocumentationPath,
'scan-result-policy-editor',
),
};
},
computed: {
......@@ -138,7 +143,7 @@ export default {
<template>
<policy-editor-layout
v-if="!disableScanExecutionUpdate"
v-if="!disableScanPolicyUpdate"
:custom-save-button-text="$options.i18n.createMergeRequest"
:default-editor-mode="$options.DEFAULT_EDITOR_MODE"
:editor-modes="$options.EDITOR_MODES"
......@@ -154,7 +159,7 @@ export default {
<gl-empty-state
v-else
:description="$options.i18n.notOwnerDescription"
:primary-button-link="scanExecutionDocumentationPath"
:primary-button-link="documentationPath"
:primary-button-text="$options.i18n.notOwnerButtonText"
:svg-path="policyEditorEmptyStateSvgPath"
title=""
......
......@@ -17,7 +17,7 @@ export default () => {
const {
assignedPolicyProject,
defaultEnvironmentId,
disableScanExecutionUpdate,
disableScanPolicyUpdate,
environmentsEndpoint,
createAgentHelpPath,
networkPoliciesEndpoint,
......@@ -29,7 +29,8 @@ export default () => {
projectPath,
projectId,
environmentId,
scanExecutionDocumentationPath,
scanPolicyDocumentationPath,
scanResultApprovers,
} = el.dataset;
// We require the project to have at least one available environment.
......@@ -59,19 +60,22 @@ export default () => {
props.existingPolicy = { type: policyType, ...JSON.parse(policy) };
}
const scanResultPolicyApprovers = scanResultApprovers ? JSON.parse(scanResultApprovers) : [];
return new Vue({
el,
apolloProvider,
provide: {
createAgentHelpPath,
disableScanExecutionUpdate: parseBoolean(disableScanExecutionUpdate),
disableScanPolicyUpdate: parseBoolean(disableScanPolicyUpdate),
networkDocumentationPath,
policyEditorEmptyStateSvgPath,
policyType,
projectId,
projectPath,
policiesPath,
scanExecutionDocumentationPath,
scanPolicyDocumentationPath,
scanResultPolicyApprovers,
},
store,
render(createElement) {
......
......@@ -22,6 +22,7 @@ module Projects
def edit
@policy_name = URI.decode_www_form_component(params[:id])
@policy = policy
@approvers = approvers
render_404 if @policy.nil?
end
......@@ -89,6 +90,20 @@ module Projects
def policy_configuration
@policy_configuration ||= project.security_orchestration_policy_configuration
end
def approvers
return unless Feature.enabled?(:scan_result_policy, project) && @policy_type == :scan_result_policy
result = ::Security::SecurityOrchestrationPolicies::FetchPolicyApproversService.new(
policy: @policy,
project: project,
current_user: @current_user
).execute
return unless result[:status] == :success
API::Entities::UserBasic.represent(result[:users]) + API::Entities::PublicGroupDetails.represent(result[:groups])
end
end
end
end
......@@ -15,15 +15,15 @@ module Projects::Security::PoliciesHelper
}
end
def orchestration_policy_data(project, policy_type = nil, policy = nil, environment = nil)
def orchestration_policy_data(project, policy_type = nil, policy = nil, environment = nil, approvers = nil)
return unless project
disable_scan_execution_update = !can_update_security_orchestration_policy_project?(project)
disable_scan_policy_update = !can_update_security_orchestration_policy_project?(project)
{
assigned_policy_project: assigned_policy_project(project).to_json,
default_environment_id: project.default_environment&.id || -1,
disable_scan_execution_update: disable_scan_execution_update.to_s,
disable_scan_policy_update: disable_scan_policy_update.to_s,
network_policies_endpoint: project_security_network_policies_path(project),
create_agent_help_path: help_page_url('user/clusters/agent/install/index'),
environments_endpoint: project_environments_path(project),
......@@ -35,7 +35,8 @@ module Projects::Security::PoliciesHelper
project_path: project.full_path,
project_id: project.id,
policies_path: project_security_policies_path(project),
scan_execution_documentation_path: help_page_path('user/application_security/policies/index', anchor: 'scan-execution-policy-editor')
scan_policy_documentation_path: help_page_path('user/application_security/policies/index'),
scan_result_approvers: approvers&.to_json
}
end
end
......@@ -4,8 +4,11 @@ module Security
module ScanResultPolicy
extend ActiveSupport::Concern
# Used for both policies and rules
LIMIT = 5
APPROVERS_LIMIT = 300
SCAN_FINDING = 'scan_finding'
REQUIRE_APPROVAL = 'require_approval'
......
# frozen_string_literal: true
module Security
module SecurityOrchestrationPolicies
class FetchPolicyApproversService
include BaseServiceUtility
GROUP_FINDER_PARAMS = { with_shared: true, shared_visible_only: true, shared_min_access_level: 30 }.freeze
def initialize(policy:, project:, current_user:)
@policy = policy
@project = project
@current_user = current_user
end
def execute
action = required_approval(policy)
return success({ users: [], groups: [] }) unless action
success({ users: user_approvers(action), groups: group_approvers(action) })
end
private
attr_reader :policy, :project, :current_user
def required_approval(policy)
policy&.fetch(:actions)&.find { |action| action&.fetch(:type) == Security::ScanResultPolicy::REQUIRE_APPROVAL }
end
def user_approvers(action)
return [] unless action[:user_approvers] || action[:user_approvers_ids]
user_names, user_ids = approvers_within_limit(action[:user_approvers], action[:user_approvers_ids])
project.team.users.by_ids_or_usernames(user_ids, user_names)
end
def group_approvers(action)
return [] unless action[:group_approvers] || action[:group_approvers_ids]
group_paths, group_ids = approvers_within_limit(action[:group_approvers], action[:group_approvers_ids])
Projects::GroupsFinder.new(project: project, current_user: current_user, params: GROUP_FINDER_PARAMS).execute.by_ids_or_paths(group_ids, group_paths)
end
def approvers_within_limit(names, ids)
filtered_names = names&.first(Security::ScanResultPolicy::APPROVERS_LIMIT) || []
filtered_ids = []
if filtered_names.count < Security::ScanResultPolicy::APPROVERS_LIMIT
filtered_ids = ids&.first(Security::ScanResultPolicy::APPROVERS_LIMIT - filtered_names.count)
end
[filtered_names, filtered_ids]
end
end
end
end
- add_to_breadcrumbs s_("SecurityOrchestration|Policies"), project_security_policies_path(@project)
- breadcrumb_title s_("SecurityOrchestration|Edit policy")
- page_title s_("SecurityOrchestration|Edit policy")
- data = orchestration_policy_data(@project, @policy_type, @policy, @environment)
- data = orchestration_policy_data(@project, @policy_type, @policy, @environment, @approvers)
#js-policy-builder-app{ data: data }
......@@ -21,6 +21,7 @@ import { SECURITY_POLICY_ACTIONS } from 'ee/threat_monitoring/components/policy_
jest.mock('~/lib/utils/url_utility', () => ({
joinPaths: jest.requireActual('~/lib/utils/url_utility').joinPaths,
visitUrl: jest.fn().mockName('visitUrlMock'),
setUrlFragment: jest.requireActual('~/lib/utils/url_utility').setUrlFragment,
}));
const newlyCreatedPolicyProject = {
......@@ -55,7 +56,7 @@ describe('ScanExecutionPolicyEditor', () => {
let wrapper;
const defaultProjectPath = 'path/to/project';
const policyEditorEmptyStateSvgPath = 'path/to/svg';
const scanExecutionDocumentationPath = 'path/to/docs';
const scanPolicyDocumentationPath = 'path/to/docs';
const assignedPolicyProject = {
branch: 'main',
fullPath: 'path/to/existing-project',
......@@ -68,11 +69,11 @@ describe('ScanExecutionPolicyEditor', () => {
...propsData,
},
provide: {
disableScanExecutionUpdate: false,
disableScanPolicyUpdate: false,
policyEditorEmptyStateSvgPath,
projectId: 1,
projectPath: defaultProjectPath,
scanExecutionDocumentationPath,
scanPolicyDocumentationPath,
...provide,
},
});
......@@ -141,12 +142,13 @@ describe('ScanExecutionPolicyEditor', () => {
describe('when a user is not an owner of the project', () => {
it('displays the empty state with the appropriate properties', async () => {
factory({ provide: { disableScanExecutionUpdate: true } });
factory({ provide: { disableScanPolicyUpdate: true } });
await nextTick();
expect(findEmptyState().props()).toMatchObject({
primaryButtonLink: scanExecutionDocumentationPath,
svgPath: policyEditorEmptyStateSvgPath,
});
const emptyState = findEmptyState();
expect(emptyState.props('primaryButtonLink')).toMatch(scanPolicyDocumentationPath);
expect(emptyState.props('primaryButtonLink')).toMatch('scan-execution-policy-editor');
expect(emptyState.props('svgPath')).toBe(policyEditorEmptyStateSvgPath);
});
});
});
......@@ -21,6 +21,7 @@ import { SECURITY_POLICY_ACTIONS } from 'ee/threat_monitoring/components/policy_
jest.mock('~/lib/utils/url_utility', () => ({
joinPaths: jest.requireActual('~/lib/utils/url_utility').joinPaths,
visitUrl: jest.fn().mockName('visitUrlMock'),
setUrlFragment: jest.requireActual('~/lib/utils/url_utility').setUrlFragment,
}));
const newlyCreatedPolicyProject = {
......@@ -39,11 +40,12 @@ describe('ScanResultPolicyEditor', () => {
let wrapper;
const defaultProjectPath = 'path/to/project';
const policyEditorEmptyStateSvgPath = 'path/to/svg';
const scanExecutionDocumentationPath = 'path/to/docs';
const scanPolicyDocumentationPath = 'path/to/docs';
const assignedPolicyProject = {
branch: 'main',
fullPath: 'path/to/existing-project',
};
const scanResultPolicyApprovers = [];
const factory = ({ propsData = {}, provide = {} } = {}) => {
wrapper = shallowMount(ScanResultPolicyEditor, {
......@@ -52,11 +54,12 @@ describe('ScanResultPolicyEditor', () => {
...propsData,
},
provide: {
disableScanExecutionUpdate: false,
disableScanPolicyUpdate: false,
policyEditorEmptyStateSvgPath,
projectId: 1,
projectPath: defaultProjectPath,
scanExecutionDocumentationPath,
scanPolicyDocumentationPath,
scanResultPolicyApprovers,
...provide,
},
});
......@@ -122,11 +125,12 @@ describe('ScanResultPolicyEditor', () => {
describe('when a user is not an owner of the project', () => {
it('displays the empty state with the appropriate properties', async () => {
factory({ provide: { disableScanExecutionUpdate: true } });
expect(findEmptyState().props()).toMatchObject({
primaryButtonLink: scanExecutionDocumentationPath,
svgPath: policyEditorEmptyStateSvgPath,
});
factory({ provide: { disableScanPolicyUpdate: true } });
const emptyState = findEmptyState();
expect(emptyState.props('primaryButtonLink')).toMatch(scanPolicyDocumentationPath);
expect(emptyState.props('primaryButtonLink')).toMatch('scan-result-policy-editor');
expect(emptyState.props('svgPath')).toBe(policyEditorEmptyStateSvgPath);
});
});
});
......@@ -35,12 +35,13 @@ RSpec.describe Projects::Security::PoliciesHelper do
end
describe '#orchestration_policy_data' do
let(:approvers) { %w(approver1 approver2) }
let(:owner) { project.first_owner }
let(:base_data) do
{
assigned_policy_project: "null",
default_environment_id: -1,
disable_scan_execution_update: "false",
disable_scan_policy_update: "false",
network_policies_endpoint: kind_of(String),
create_agent_help_path: kind_of(String),
environments_endpoint: kind_of(String),
......@@ -52,7 +53,8 @@ RSpec.describe Projects::Security::PoliciesHelper do
environment_id: environment&.id,
policy: policy&.to_json,
policy_type: policy_type,
scan_execution_documentation_path: kind_of(String)
scan_policy_documentation_path: kind_of(String),
scan_result_approvers: approvers&.to_json
}
end
......@@ -61,12 +63,13 @@ RSpec.describe Projects::Security::PoliciesHelper do
allow(helper).to receive(:can?).with(owner, :update_security_orchestration_policy_project, project) { true }
end
subject { helper.orchestration_policy_data(project, policy_type, policy, environment) }
subject { helper.orchestration_policy_data(project, policy_type, policy, environment, approvers) }
context 'when a new policy is being created' do
let(:environment) { nil }
let(:policy) { nil }
let(:policy_type) { nil }
let(:approvers) { nil }
it { is_expected.to match(base_data) }
end
......
......@@ -11,10 +11,11 @@ RSpec.describe Projects::Security::PoliciesController, type: :request do
let_it_be(:policy) { build(:scan_execution_policy) }
let_it_be(:type) { 'scan_execution_policy' }
let_it_be(:index) { project_security_policies_url(project) }
let_it_be(:edit) { edit_project_security_policy_url(project, id: policy[:name], type: type) }
let_it_be(:new) { new_project_security_policy_url(project) }
let_it_be(:feature_enabled) { true }
let(:edit) { edit_project_security_policy_url(project, id: policy[:name], type: type) }
before do
project.add_developer(user)
sign_in(user)
......@@ -39,6 +40,13 @@ RSpec.describe Projects::Security::PoliciesController, type: :request do
expect(app.attributes['data-policy-type'].value).to eq(type)
end
it 'does not contain any approver data' do
get edit
app = Nokogiri::HTML.parse(response.body).at_css('div#js-policy-builder-app')
expect(app['data-scan-result-approvers']).to be_nil
end
context 'when type is container_runtime' do
let_it_be(:type) { 'container_policy' }
let_it_be(:environment) { create(:environment, :with_review_app, project: project) }
......@@ -84,6 +92,59 @@ RSpec.describe Projects::Security::PoliciesController, type: :request do
expect(app.attributes['data-policy-type'].value).to eq(type)
expect(app.attributes['data-environment-id'].value).to eq(environment_id.to_s)
end
it 'does not contain any approver data' do
get edit
app = Nokogiri::HTML.parse(response.body).at_css('div#js-policy-builder-app')
expect(app['data-scan-result-approvers']).to be_nil
end
end
context 'with scan result policy type' do
let_it_be(:type) {'scan_result_policy'}
let_it_be(:policy) {build(:scan_result_policy)}
let_it_be(:group) { create(:group) }
let_it_be(:service_result) { { users: [user], groups: [group], status: :success } }
let(:service) { instance_double('::Security::SecurityOrchestrationPolicies::FetchPolicyApproversService', execute: service_result) }
before do
stub_feature_flags(scan_result_policy: true)
allow_next_instance_of(Repository) do |repository|
allow(repository).to receive(:blob_data_at).and_return({ scan_result_policy: [policy] }.to_yaml)
end
allow(::Security::SecurityOrchestrationPolicies::FetchPolicyApproversService).to receive(:new).with(policy: policy, project: project, current_user: user).and_return(service)
end
it 'renders the edit page with approvers data' do
get edit
expect(response).to have_gitlab_http_status(:ok)
expect(response).to render_template(:edit)
app = Nokogiri::HTML.parse(response.body).at_css('div#js-policy-builder-app')
expect(app['data-policy']).to eq(policy.to_json)
expect(app['data-policy-type']).to eq(type)
expect(app['data-scan-result-approvers']).to include(user.name, user.id.to_s, group.full_path, group.id.to_s)
end
context 'with feature flag disabled' do
before do
stub_feature_flags(scan_result_policy: false)
end
it 'renders the edit page without approvers data' do
get edit
app = Nokogiri::HTML.parse(response.body).at_css('div#js-policy-builder-app')
expect(app['data-policy']).to eq(policy.to_json)
expect(app['data-policy-type']).to eq(type)
expect(app['data-scan-result-approvers']).to be_nil
end
end
end
context 'when type is missing' do
......@@ -143,7 +204,7 @@ RSpec.describe Projects::Security::PoliciesController, type: :request do
end
context 'when policy yaml is invalid' do
let_it_be(:policy) { 'invalid' }
let_it_be(:policy) { { name: 'invalid' } }
it 'redirects to policy file' do
get edit
......
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe Security::SecurityOrchestrationPolicies::FetchPolicyApproversService do
describe '#execute' do
let_it_be(:group) { create(:group) }
let_it_be(:project) { create(:project, :public, namespace: group) }
let_it_be(:policy_configuration) { create(:security_orchestration_policy_configuration, project: project) }
let_it_be(:user) { create(:user) }
let(:policy) { build(:scan_result_policy, actions: [action]) }
subject(:service) do
described_class.new(policy: policy, current_user: user, project: project)
end
before do
group.add_user(user, :owner)
end
context 'with group outside of the scope' do
let(:unrelated_group) { create(:group) }
let(:action) { { type: "require_approval", approvals_required: 1, group_approvers_ids: [unrelated_group.id, group.id] } }
it 'does not return the unrelated group' do
response = service.execute
expect(response[:groups]).to contain_exactly(group)
end
end
context 'with user approver' do
let(:action) { { type: "require_approval", approvals_required: 1, user_approvers: [user.username] } }
it 'returns user approvers' do
response = service.execute
expect(response[:status]).to eq(:success)
expect(response[:users]).to match_array([user])
expect(response[:groups]).to be_empty
end
end
context 'with group approver' do
let(:action) { { type: "require_approval", approvals_required: 1, group_approvers_ids: [group.id] } }
it 'returns group approvers' do
response = service.execute
expect(response[:status]).to eq(:success)
expect(response[:groups]).to match_array([group])
expect(response[:users]).to be_empty
end
end
context 'with both user and group approvers' do
let(:action) { { type: "require_approval", approvals_required: 1, group_approvers: [group.path], user_approvers_ids: [user.id] } }
it 'returns all approvers' do
response = service.execute
expect(response[:status]).to eq(:success)
expect(response[:users]).to match_array([user])
expect(response[:groups]).to match_array([group])
end
end
context 'with policy equals to nil' do
let(:policy) { nil }
it 'returns no approver' do
response = service.execute
expect(response[:status]).to eq(:success)
expect(response[:users]).to be_empty
expect(response[:groups]).to be_empty
end
end
context 'with action equals to nil' do
let(:action) { nil }
it 'returns no approver' do
response = service.execute
expect(response[:status]).to eq(:success)
expect(response[:users]).to be_empty
expect(response[:groups]).to be_empty
end
end
context 'with action of an unknown type' do
let(:action) { { type: "random_type", approvals_required: 1, group_approvers_ids: [group.id] } }
it 'returns no approver' do
response = service.execute
expect(response[:status]).to eq(:success)
expect(response[:users]).to be_empty
expect(response[:groups]).to be_empty
end
end
context 'with more users than the limit' do
using RSpec::Parameterized::TableSyntax
let(:user_ids) { [user.id] }
let(:user_names) { [user.username] }
where(:ids_multiplier, :names_multiplier, :ids_expected, :names_expected) do
150 | 150 | 150 | 150
300 | 300 | 0 | 300
300 | 200 | 100 | 200
600 | 600 | 0 | 300
end
with_them do
let(:user_ids_multiplied) { user_ids * ids_multiplier }
let(:user_name_multiplied) { user_names * names_multiplier }
let(:user_ids_expected) { user_ids * ids_expected }
let(:user_name_expected) { user_names * names_expected }
let(:action) { { type: "require_approval", approvals_required: 1, user_approvers: user_name_multiplied, user_approvers_ids: user_ids_multiplied } }
it 'considers only the first within the limit' do
expect(project).to receive_message_chain(:team, :users, :by_ids_or_usernames).with(user_ids_expected, user_name_expected)
service.execute
expect((user_ids_expected + user_name_expected).count).not_to be > Security::ScanResultPolicy::APPROVERS_LIMIT
end
end
end
context 'with more groups than the limit' do
using RSpec::Parameterized::TableSyntax
let(:group_ids) { [group.id] }
let(:group_paths) { [group.path] }
where(:ids_multiplier, :paths_multiplier, :ids_expected, :paths_expected) do
150 | 150 | 150 | 150
300 | 300 | 0 | 300
300 | 200 | 100 | 200
600 | 600 | 0 | 300
end
with_them do
let(:group_ids_multiplied) { group_ids * ids_multiplier }
let(:group_path_multiplied) { group_paths * paths_multiplier }
let(:group_ids_expected) { group_ids * ids_expected }
let(:group_path_expected) { group_paths * paths_expected }
let(:action) { { type: "require_approval", approvals_required: 1, group_approvers: group_path_multiplied, group_approvers_ids: group_ids_multiplied } }
it 'considers only the first within the limit' do
expect(Projects::GroupsFinder).to receive_message_chain(:new, :execute, :by_ids_or_paths).with(group_ids_expected, group_path_expected)
service.execute
expect((group_ids_expected + group_path_expected).count).not_to be > Security::ScanResultPolicy::APPROVERS_LIMIT
end
end
end
end
end
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