Commit b9aec8bf authored by Bob Van Landuyt's avatar Bob Van Landuyt

Merge branch 'add_approvers_when_editing_scan_result_policies' into 'master'

Add approvers when editing scan result policies

See merge request gitlab-org/gitlab!80029
parents ef7e6556 b40ee4d1
<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