Commit 75d757fe authored by Sean McGivern's avatar Sean McGivern

Merge branch '39060-disable-self-approval-at-the-instance-level' into 'master'

Resolve "Disable self-approval at the Instance level"

Closes #39060

See merge request gitlab-org/gitlab!23731
parents 744e7c17 203bfa65
# frozen_string_literal: true
class AddFieldsToApplicationSettingsForMergeRequestsApprovals < ActiveRecord::Migration[5.2]
DOWNTIME = false
def up
add_column(:application_settings,
:disable_overriding_approvers_per_merge_request,
:boolean,
default: false,
null: false)
add_column(:application_settings,
:prevent_merge_requests_author_approval,
:boolean,
default: false,
null: false)
add_column(:application_settings,
:prevent_merge_requests_committers_approval,
:boolean,
default: false,
null: false)
end
def down
remove_column(:application_settings, :disable_overriding_approvers_per_merge_request)
remove_column(:application_settings, :prevent_merge_requests_author_approval)
remove_column(:application_settings, :prevent_merge_requests_committers_approval)
end
end
...@@ -346,6 +346,9 @@ ActiveRecord::Schema.define(version: 2020_02_13_204737) do ...@@ -346,6 +346,9 @@ ActiveRecord::Schema.define(version: 2020_02_13_204737) do
t.integer "elasticsearch_indexed_field_length_limit", default: 0, null: false t.integer "elasticsearch_indexed_field_length_limit", default: 0, null: false
t.integer "elasticsearch_max_bulk_size_mb", limit: 2, default: 10, null: false t.integer "elasticsearch_max_bulk_size_mb", limit: 2, default: 10, null: false
t.integer "elasticsearch_max_bulk_concurrency", limit: 2, default: 10, null: false t.integer "elasticsearch_max_bulk_concurrency", limit: 2, default: 10, null: false
t.boolean "disable_overriding_approvers_per_merge_request", default: false, null: false
t.boolean "prevent_merge_requests_author_approval", default: false, null: false
t.boolean "prevent_merge_requests_committers_approval", default: false, null: false
t.index ["custom_project_templates_group_id"], name: "index_application_settings_on_custom_project_templates_group_id" t.index ["custom_project_templates_group_id"], name: "index_application_settings_on_custom_project_templates_group_id"
t.index ["file_template_project_id"], name: "index_application_settings_on_file_template_project_id" t.index ["file_template_project_id"], name: "index_application_settings_on_file_template_project_id"
t.index ["instance_administration_project_id"], name: "index_applicationsettings_on_instance_administration_project_id" t.index ["instance_administration_project_id"], name: "index_applicationsettings_on_instance_administration_project_id"
......
...@@ -28,7 +28,7 @@ The Admin Area is made up of the following sections: ...@@ -28,7 +28,7 @@ The Admin Area is made up of the following sections:
| Abuse Reports | Manage [abuse reports](abuse_reports.md) submitted by your users. | | Abuse Reports | Manage [abuse reports](abuse_reports.md) submitted by your users. |
| License **(STARTER ONLY)** | Upload, display, and remove [licenses](license.md). | | License **(STARTER ONLY)** | Upload, display, and remove [licenses](license.md). |
| Kubernetes | Create and manage instance-level [Kubernetes clusters](../instance/clusters/index.md). | | Kubernetes | Create and manage instance-level [Kubernetes clusters](../instance/clusters/index.md). |
| Push Rules **(STARTER)** | Configure pre-defined Git [push rules](../../push_rules/push_rules.md) for projects. | Push Rules **(STARTER ONLY)** | Configure pre-defined Git [push rules](../../push_rules/push_rules.md) for projects. Also, configure [merge requests approvers rules](merge_requests_approvals.md). **(PREMIUM ONLY)** |
| Geo **(PREMIUM ONLY)** | Configure and maintain [Geo nodes](geo_nodes.md). | | Geo **(PREMIUM ONLY)** | Configure and maintain [Geo nodes](geo_nodes.md). |
| Deploy Keys | Create instance-wide [SSH deploy keys](../../ssh/README.md#deploy-keys). | | Deploy Keys | Create instance-wide [SSH deploy keys](../../ssh/README.md#deploy-keys). |
| Credentials **(ULTIMATE ONLY)** | View [credentials](credentials_inventory.md) that can be used to access your instance. | | Credentials **(ULTIMATE ONLY)** | View [credentials](credentials_inventory.md) that can be used to access your instance. |
......
---
type: reference, concepts
---
# Instance-level merge request approval rules **(PREMIUM ONLY)**
> Introduced in [GitLab Premium](https://gitlab.com/gitlab-org/gitlab/issues/39060) 12.8.
Merge request approvals rules prevent users overriding certain settings on a project
level. When configured, only administrators can change these settings on a project level
if they are enabled at an instance level.
To enable merge request approval rules for an instance:
1. Navigate to **{admin}** **Admin Area >** **{push-rules}** **Push Rules** and expand **Merge
requests approvals**.
1. Set the required rule.
1. Click **Save changes**.
GitLab administrators can later override these settings in a project’s settings.
## Available rules
Merge request approval rules that can be set at an instance level are:
- **Prevent approval of merge requests by merge request author**. Prevents non-admins
from allowing merge request authors to merge their own merge requests in individual
projects.
- **Prevent approval of merge requests by merge request committers**. Prevents
non-admins from allowing merge request committers to merge merge requests they were
committing to in individual projects.
- **Prevent users from modifying merge request approvers list**. Prevents non-admins
from modifying approvers list in project settings and in individual merge requests.
...@@ -16,6 +16,7 @@ export default function mountProjectSettingsApprovals(el) { ...@@ -16,6 +16,7 @@ export default function mountProjectSettingsApprovals(el) {
...el.dataset, ...el.dataset,
prefix: 'project-settings', prefix: 'project-settings',
allowMultiRule: parseBoolean(el.dataset.allowMultiRule), allowMultiRule: parseBoolean(el.dataset.allowMultiRule),
canEdit: parseBoolean(el.dataset.canEdit),
}); });
return new Vue({ return new Vue({
......
import initSettingsPanels from '~/settings_panels';
document.addEventListener('DOMContentLoaded', () => {
initSettingsPanels();
});
...@@ -3,6 +3,7 @@ ...@@ -3,6 +3,7 @@
class Admin::PushRulesController < Admin::ApplicationController class Admin::PushRulesController < Admin::ApplicationController
before_action :check_push_rules_available! before_action :check_push_rules_available!
before_action :push_rule before_action :push_rule
before_action :set_application_setting
respond_to :html respond_to :html
...@@ -46,4 +47,8 @@ class Admin::PushRulesController < Admin::ApplicationController ...@@ -46,4 +47,8 @@ class Admin::PushRulesController < Admin::ApplicationController
@push_rule ||= PushRule.find_or_initialize_by(is_sample: true) @push_rule ||= PushRule.find_or_initialize_by(is_sample: true)
end end
# rubocop: enable CodeReuse/ActiveRecord # rubocop: enable CodeReuse/ActiveRecord
def set_application_setting
@application_setting = ApplicationSetting.current_without_cache
end
end end
...@@ -50,6 +50,10 @@ module EE ...@@ -50,6 +50,10 @@ module EE
attrs << :updating_name_disabled_for_users attrs << :updating_name_disabled_for_users
end end
if License.feature_available?(:admin_merge_request_approvers_rules)
attrs += EE::ApplicationSettingsHelper.merge_request_appovers_rules_attributes
end
attrs attrs
end end
......
...@@ -73,15 +73,12 @@ module EE ...@@ -73,15 +73,12 @@ module EE
approver_ids approver_ids
issues_template issues_template
merge_requests_template merge_requests_template
disable_overriding_approvers_per_merge_request
repository_size_limit repository_size_limit
reset_approvals_on_push reset_approvals_on_push
service_desk_enabled service_desk_enabled
ci_cd_only ci_cd_only
use_custom_template use_custom_template
packages_enabled packages_enabled
merge_requests_author_approval
merge_requests_disable_committers_approval
require_password_to_approve require_password_to_approve
group_with_project_templates_id group_with_project_templates_id
] ]
...@@ -90,6 +87,8 @@ module EE ...@@ -90,6 +87,8 @@ module EE
attrs << %i[merge_pipelines_enabled] attrs << %i[merge_pipelines_enabled]
end end
attrs += merge_request_rules_params
if allow_mirror_params? if allow_mirror_params?
attrs + mirror_params attrs + mirror_params
else else
...@@ -113,6 +112,24 @@ module EE ...@@ -113,6 +112,24 @@ module EE
end end
end end
def merge_request_rules_params
attrs = []
if can?(current_user, :modify_merge_request_committer_setting, project)
attrs << :merge_requests_disable_committers_approval
end
if can?(current_user, :modify_approvers_rules, project)
attrs << :disable_overriding_approvers_per_merge_request
end
if can?(current_user, :modify_merge_request_author_setting, project)
attrs << :merge_requests_author_approval
end
attrs
end
def allow_merge_pipelines_params? def allow_merge_pipelines_params?
project&.feature_available?(:merge_pipelines) project&.feature_available?(:merge_pipelines)
end end
......
...@@ -87,7 +87,8 @@ module EE ...@@ -87,7 +87,8 @@ module EE
end end
def self.possible_licensed_attributes def self.possible_licensed_attributes
repository_mirror_attributes + %i[ repository_mirror_attributes + merge_request_appovers_rules_attributes +
%i[
email_additional_text email_additional_text
file_template_project_id file_template_project_id
default_project_deletion_protection default_project_deletion_protection
...@@ -95,5 +96,13 @@ module EE ...@@ -95,5 +96,13 @@ module EE
updating_name_disabled_for_users updating_name_disabled_for_users
] ]
end end
def self.merge_request_appovers_rules_attributes
%i[
disable_overriding_approvers_per_merge_request
prevent_merge_requests_author_approval
prevent_merge_requests_committers_approval
]
end
end end
end end
...@@ -258,7 +258,7 @@ module EE ...@@ -258,7 +258,7 @@ module EE
end end
def can_override_approvers? def can_override_approvers?
!disable_overriding_approvers_per_merge_request? !disable_overriding_approvers_per_merge_request
end end
def shared_runners_available? def shared_runners_available?
...@@ -723,6 +723,28 @@ module EE ...@@ -723,6 +723,28 @@ module EE
packages.where(package_type: package_type).exists? packages.where(package_type: package_type).exists?
end end
def disable_overriding_approvers_per_merge_request
return super unless License.feature_available?(:admin_merge_request_approvers_rules)
::Gitlab::CurrentSettings.disable_overriding_approvers_per_merge_request? ||
super
end
def merge_requests_author_approval
return super unless License.feature_available?(:admin_merge_request_approvers_rules)
return false if ::Gitlab::CurrentSettings.prevent_merge_requests_author_approval?
super
end
def merge_requests_disable_committers_approval
return super unless License.feature_available?(:admin_merge_request_approvers_rules)
::Gitlab::CurrentSettings.prevent_merge_requests_committers_approval? ||
super
end
def license_compliance def license_compliance
strong_memoize(:license_compliance) { SCA::LicenseCompliance.new(self) } strong_memoize(:license_compliance) { SCA::LicenseCompliance.new(self) }
end end
......
...@@ -80,6 +80,7 @@ class License < ApplicationRecord ...@@ -80,6 +80,7 @@ class License < ApplicationRecord
ldap_group_sync_filter ldap_group_sync_filter
merge_pipelines merge_pipelines
merge_request_performance_metrics merge_request_performance_metrics
admin_merge_request_approvers_rules
merge_trains merge_trains
metrics_reports metrics_reports
multiple_approval_rules multiple_approval_rules
......
...@@ -46,6 +46,27 @@ module EE ...@@ -46,6 +46,27 @@ module EE
!PushRule.global&.commit_committer_check !PushRule.global&.commit_committer_check
end end
with_scope :global
condition(:owner_cannot_modify_approvers_rules) do
License.feature_available?(:admin_merge_request_approvers_rules) &&
::Gitlab::CurrentSettings.current_application_settings
.disable_overriding_approvers_per_merge_request
end
with_scope :global
condition(:owner_cannot_modify_merge_request_author_setting) do
License.feature_available?(:admin_merge_request_approvers_rules) &&
::Gitlab::CurrentSettings.current_application_settings
.prevent_merge_requests_author_approval
end
with_scope :global
condition(:owner_cannot_modify_merge_request_committer_setting) do
License.feature_available?(:admin_merge_request_approvers_rules) &&
::Gitlab::CurrentSettings.current_application_settings
.prevent_merge_requests_committers_approval
end
with_scope :subject with_scope :subject
condition(:commit_committer_check_available) do condition(:commit_committer_check_available) do
@subject.feature_available?(:commit_committer_check) @subject.feature_available?(:commit_committer_check)
...@@ -201,6 +222,10 @@ module EE ...@@ -201,6 +222,10 @@ module EE
enable :update_approvers enable :update_approvers
enable :destroy_package enable :destroy_package
enable :admin_feature_flags_client enable :admin_feature_flags_client
enable :modify_approvers_rules
enable :modify_approvers_list
enable :modify_merge_request_author_setting
enable :modify_merge_request_committer_setting
end end
rule { license_management_enabled & can?(:maintainer_access) }.enable :admin_software_license_policy rule { license_management_enabled & can?(:maintainer_access) }.enable :admin_software_license_policy
...@@ -298,6 +323,22 @@ module EE ...@@ -298,6 +323,22 @@ module EE
prevent :read_project prevent :read_project
end end
rule { owner_cannot_modify_approvers_rules & ~admin }.policy do
prevent :modify_approvers_rules
end
rule { owner_cannot_modify_merge_request_author_setting & ~admin }.policy do
prevent :modify_merge_request_author_setting
end
rule { owner_cannot_modify_merge_request_committer_setting & ~admin }.policy do
prevent :modify_merge_request_committer_setting
end
rule { owner_cannot_modify_approvers_rules & ~admin }.policy do
prevent :modify_approvers_list
end
rule { web_ide_terminal_available & can?(:create_pipeline) & can?(:maintainer_access) }.enable :create_web_ide_terminal rule { web_ide_terminal_available & can?(:create_pipeline) & can?(:maintainer_access) }.enable :create_web_ide_terminal
# Design abilities could also be prevented in the issue policy. # Design abilities could also be prevented in the issue policy.
......
- return unless License.feature_available?(:admin_merge_request_approvers_rules)
%section.settings.merge-request-approval-settings.no-animate{ class: ('expanded' if expanded_by_default?) }
.settings-header
%h4
= _('Merge requests approvals')
%button.btn.js-settings-toggle{ type: 'button' }
= expanded_by_default? ? _('Collapse') : _('Expand')
%p
= _('Settings to prevent self-approval across all projects in the instance. Only an administrator can modify these settings.')
.settings-content
%hr.clearfix.mt-0
= form_for @application_setting, url: general_admin_application_settings_path(anchor: 'merge-request-approval-settings'), html: { class: 'fieldset-form' } do |f|
= form_errors(@application_setting)
%fieldset
.form-group
.form-check
= f.check_box :prevent_merge_requests_author_approval, class: 'form-check-input'
= f.label :prevent_merge_requests_author_approval, class: 'form-check-label' do
= _('Prevent approval of merge requests by merge request author')
.form-check
= f.check_box :prevent_merge_requests_committers_approval, class: 'form-check-input'
= f.label :prevent_merge_requests_committers_approval, class: 'form-check-label' do
= _('Prevent approval of merge requests by merge request committers')
.form-check
= f.check_box :disable_overriding_approvers_per_merge_request , class: 'form-check-input'
= f.label :disable_overriding_approvers_per_merge_request , class: 'form-check-label' do
= _('Prevent users from modifing merge request approvers list')
= f.submit _('Save changes'), class: "btn btn-success"
= form_for @push_rule, url: admin_push_rule_path, method: :put, html: { class: 'fieldset-form' } do |f|
- if @push_rule.errors.any?
.alert.alert-danger
- @push_rule.errors.full_messages.each do |msg|
%p= msg
= render "shared/push_rules/form", f: f
- page_title "Push Rules" - breadcrumb_title _("Push Rules")
%h3.page-title - page_title _("Push Rules")
Pre-defined push rules. - @content_class = "limit-container-width" unless fluid_layout
%p.light
Rules that define what git pushes are accepted for a project. All newly created projects will use this settings.
%hr.clearfix %section.settings.no-animate{ class: ('expanded' if expanded_by_default?) }
.settings-header
%h4
= _('Push Rules')
%button.btn.js-settings-toggle{ type: 'button' }
= expanded_by_default? ? _('Collapse') : _('Expand')
%p
= _('Rules that define what git pushes are accepted for a project. All newly created projects will use these settings.')
.settings-content
= render 'push_rules'
= form_for @push_rule, url: admin_push_rule_path, method: :put do |f| = render 'merge_request_approvals'
- if @push_rule.errors.any?
.alert.alert-danger
- @push_rule.errors.full_messages.each do |msg|
%p= msg
= render "shared/push_rules/form", f: f
- can_override_approvers = project.can_override_approvers? - can_override_approvers = project.can_override_approvers?
- can_modify_approvers = can?(current_user, :modify_approvers_rules, @project)
- can_modify_merge_request_author_settings = can?(current_user, :modify_merge_request_author_setting, @project)
- can_modify_merge_request_committer_settings = can?(current_user, :modify_merge_request_committer_setting, @project)
.form-group .form-group
= form.label :approver_ids, class: 'label-bold' do = form.label :approver_ids, class: 'label-bold' do
= _("Approval rules") = _("Approval rules")
#js-mr-approvals-settings{ data: { 'project_id': @project.id, #js-mr-approvals-settings{ data: { 'project_id': @project.id,
'can_edit': can_modify_approvers.to_s,
'project_path': expose_path(api_v4_projects_path(id: @project.id)), 'project_path': expose_path(api_v4_projects_path(id: @project.id)),
'settings_path': expose_path(api_v4_projects_approval_settings_path(id: @project.id)), 'settings_path': expose_path(api_v4_projects_approval_settings_path(id: @project.id)),
'rules_path': expose_path(api_v4_projects_approval_settings_rules_path(id: @project.id)), 'rules_path': expose_path(api_v4_projects_approval_settings_rules_path(id: @project.id)),
...@@ -26,7 +30,7 @@ ...@@ -26,7 +30,7 @@
.form-group .form-group
.form-check .form-check
= form.check_box(:disable_overriding_approvers_per_merge_request, { checked: can_override_approvers, class: 'form-check-input' }, false, true) = form.check_box(:disable_overriding_approvers_per_merge_request, { checked: can_override_approvers, class: 'form-check-input', disabled: !can_modify_approvers }, false, true)
= form.label :disable_overriding_approvers_per_merge_request, class: 'form-check-label' do = form.label :disable_overriding_approvers_per_merge_request, class: 'form-check-label' do
%span= _('Can override approvers and approvals required per merge request') %span= _('Can override approvers and approvals required per merge request')
= link_to icon('question-circle'), help_page_path('user/project/merge_requests/merge_request_approvals', anchor: 'prevent-overriding-default-approvals'), target: '_blank' = link_to icon('question-circle'), help_page_path('user/project/merge_requests/merge_request_approvals', anchor: 'prevent-overriding-default-approvals'), target: '_blank'
...@@ -39,7 +43,7 @@ ...@@ -39,7 +43,7 @@
.form-group.self-approval .form-group.self-approval
.form-check .form-check
= form.check_box :merge_requests_author_approval, { class: 'form-check-input', checked: !project.merge_requests_author_approval? }, false, true = form.check_box :merge_requests_author_approval, { class: 'form-check-input', checked: !project.merge_requests_author_approval?, disabled: !can_modify_merge_request_author_settings }, false, true
= form.label :merge_requests_author_approval, class: 'form-check-label' do = form.label :merge_requests_author_approval, class: 'form-check-label' do
%span= _('Prevent approval of merge requests by merge request author') %span= _('Prevent approval of merge requests by merge request author')
= link_to icon('question-circle'), help_page_path('user/project/merge_requests/merge_request_approvals', = link_to icon('question-circle'), help_page_path('user/project/merge_requests/merge_request_approvals',
...@@ -47,7 +51,7 @@ ...@@ -47,7 +51,7 @@
.form-group.committers-approval .form-group.committers-approval
.form-check .form-check
= form.check_box :merge_requests_disable_committers_approval, class: 'form-check-input' = form.check_box :merge_requests_disable_committers_approval, { disabled: !can_modify_merge_request_committer_settings, class: 'form-check-input' }
= form.label :merge_requests_disable_committers_approval, class: 'form-check-label' do = form.label :merge_requests_disable_committers_approval, class: 'form-check-label' do
%span= _('Prevent approval of merge requests by merge request committers') %span= _('Prevent approval of merge requests by merge request committers')
= link_to icon('question-circle'), help_page_path('user/project/merge_requests/merge_request_approvals', = link_to icon('question-circle'), help_page_path('user/project/merge_requests/merge_request_approvals',
......
---
title: Resolve Disable self-approval at the Instance level
merge_request: 23731
author:
type: added
...@@ -7,6 +7,24 @@ module API ...@@ -7,6 +7,24 @@ module API
ARRAY_COERCION_LAMBDA = ->(val) { val.empty? ? [] : Array.wrap(val) } ARRAY_COERCION_LAMBDA = ->(val) { val.empty? ? [] : Array.wrap(val) }
helpers do
def filter_params(params)
unless can?(current_user, :modify_merge_request_committer_setting, user_project)
params.delete(:merge_requests_disable_committers_approval)
end
unless can?(current_user, :modify_approvers_rules, user_project)
params.delete(:disable_overriding_approvers_per_merge_request)
end
unless can?(current_user, :modify_merge_request_author_setting, user_project)
params.delete(:merge_requests_author_approval)
end
params
end
end
params do params do
requires :id, type: String, desc: 'The ID of a project' requires :id, type: String, desc: 'The ID of a project'
end end
...@@ -34,8 +52,8 @@ module API ...@@ -34,8 +52,8 @@ module API
at_least_one_of :approvals_before_merge, :reset_approvals_on_push, :disable_overriding_approvers_per_merge_request, :merge_requests_author_approval, :merge_requests_disable_committers_approval, :require_password_to_approve at_least_one_of :approvals_before_merge, :reset_approvals_on_push, :disable_overriding_approvers_per_merge_request, :merge_requests_author_approval, :merge_requests_disable_committers_approval, :require_password_to_approve
end end
post '/' do post '/' do
project_params = declared(params, include_missing: false, include_parent_namespaces: false) declared_params = declared(params, include_missing: false, include_parent_namespaces: false)
project_params = filter_params(declared_params)
result = ::Projects::UpdateService.new(user_project, current_user, project_params).execute result = ::Projects::UpdateService.new(user_project, current_user, project_params).execute
if result[:status] == :success if result[:status] == :success
......
...@@ -217,6 +217,9 @@ module EE ...@@ -217,6 +217,9 @@ module EE
expose(*EE::ApplicationSettingsHelper.repository_mirror_attributes, if: ->(_instance, _options) do expose(*EE::ApplicationSettingsHelper.repository_mirror_attributes, if: ->(_instance, _options) do
::License.feature_available?(:repository_mirrors) ::License.feature_available?(:repository_mirrors)
end) end)
expose(*EE::ApplicationSettingsHelper.merge_request_appovers_rules_attributes, if: ->(_instance, _options) do
::License.feature_available?(:admin_merge_request_approvers_rules)
end)
expose :email_additional_text, if: ->(_instance, _opts) { ::License.feature_available?(:email_additional_text) } expose :email_additional_text, if: ->(_instance, _opts) { ::License.feature_available?(:email_additional_text) }
expose :file_template_project_id, if: ->(_instance, _opts) { ::License.feature_available?(:custom_file_templates) } expose :file_template_project_id, if: ->(_instance, _opts) { ::License.feature_available?(:custom_file_templates) }
expose :default_project_deletion_protection, if: ->(_instance, _opts) { ::License.feature_available?(:default_project_deletion_protection) } expose :default_project_deletion_protection, if: ->(_instance, _opts) { ::License.feature_available?(:default_project_deletion_protection) }
......
...@@ -38,6 +38,9 @@ module EE ...@@ -38,6 +38,9 @@ module EE
optional :repository_storages, type: Array[String], desc: 'A list of names of enabled storage paths, taken from `gitlab.yml`. New projects will be created in one of these stores, chosen at random.' optional :repository_storages, type: Array[String], desc: 'A list of names of enabled storage paths, taken from `gitlab.yml`. New projects will be created in one of these stores, chosen at random.'
optional :usage_ping_enabled, type: Grape::API::Boolean, desc: 'Every week GitLab will report license usage back to GitLab, Inc.' optional :usage_ping_enabled, type: Grape::API::Boolean, desc: 'Every week GitLab will report license usage back to GitLab, Inc.'
optional :updating_name_disabled_for_users, type: Grape::API::Boolean, desc: 'Flag indicating if users are permitted to update their profile name' optional :updating_name_disabled_for_users, type: Grape::API::Boolean, desc: 'Flag indicating if users are permitted to update their profile name'
optional :disable_overriding_approvers_per_merge_request, type: Grape::API::Boolean, desc: 'Disable Users ability to overwrite approvers in merge requests.'
optional :prevent_merge_requests_author_approval, type: Grape::API::Boolean, desc: 'Disable Merge request author ability to approve request.'
optional :prevent_merge_requests_committers_approval, type: Grape::API::Boolean, desc: 'Disable Merge request committer ability to approve request.'
end end
end end
......
...@@ -35,6 +35,10 @@ module EE ...@@ -35,6 +35,10 @@ module EE
attrs = attrs.except(:updating_name_disabled_for_users) attrs = attrs.except(:updating_name_disabled_for_users)
end end
unless License.feature_available?(:admin_merge_request_approvers_rules)
attrs = attrs.except(*EE::ApplicationSettingsHelper.merge_request_appovers_rules_attributes)
end
attrs attrs
end end
end end
......
...@@ -133,6 +133,19 @@ describe Admin::ApplicationSettingsController do ...@@ -133,6 +133,19 @@ describe Admin::ApplicationSettingsController do
it_behaves_like 'settings for licensed features' it_behaves_like 'settings for licensed features'
end end
context 'merge request approvers rules' do
let(:settings) do
{
disable_overriding_approvers_per_merge_request: true,
prevent_merge_requests_author_approval: true,
prevent_merge_requests_committers_approval: true
}
end
let(:feature) { :admin_merge_request_approvers_rules }
it_behaves_like 'settings for licensed features'
end
it 'updates repository_size_limit' do it 'updates repository_size_limit' do
put :update, params: { application_setting: { repository_size_limit: '100' } } put :update, params: { application_setting: { repository_size_limit: '100' } }
......
...@@ -317,6 +317,63 @@ describe ProjectsController do ...@@ -317,6 +317,63 @@ describe ProjectsController do
end end
end end
end end
context 'merge request approvers settings' do
shared_examples 'merge request approvers rules' do
using RSpec::Parameterized::TableSyntax
where(:license_value, :setting_value, :param_value, :final_value) do
false | false | false | false
false | true | false | false
false | false | true | true
false | true | true | true
true | false | false | false
true | true | false | nil
true | false | true | true
true | true | true | nil
end
with_them do
before do
stub_licensed_features(admin_merge_request_approvers_rules: license_value)
stub_application_setting(app_setting => setting_value)
end
it 'updates project if needed' do
put :update,
params: {
namespace_id: project.namespace,
id: project,
project: { setting => param_value }
}
project.reload
expect(project[setting]).to eq(final_value)
end
end
end
describe ':disable_overriding_approvers_per_merge_request' do
it_behaves_like 'merge request approvers rules' do
let(:app_setting) { :disable_overriding_approvers_per_merge_request }
let(:setting) { :disable_overriding_approvers_per_merge_request }
end
end
describe ':merge_requests_author_approval' do
it_behaves_like 'merge request approvers rules' do
let(:app_setting) { :prevent_merge_requests_author_approval }
let(:setting) { :merge_requests_author_approval }
end
end
describe ':merge_requests_disable_committers_approval' do
it_behaves_like 'merge request approvers rules' do
let(:app_setting) { :prevent_merge_requests_committers_approval }
let(:setting) { :merge_requests_disable_committers_approval }
end
end
end
end end
describe '#download_export' do describe '#download_export' do
......
...@@ -482,6 +482,85 @@ describe Project do ...@@ -482,6 +482,85 @@ describe Project do
end end
end end
context 'merge requests related settings' do
shared_examples 'setting modified by application setting' do
using RSpec::Parameterized::TableSyntax
where(:app_setting, :project_setting, :feature_enabled, :final_setting) do
true | true | true | true
false | true | true | true
true | false | true | true
false | false | true | false
true | true | false | true
false | true | false | true
true | false | false | false
false | false | false | false
end
with_them do
let(:project) { create(:project) }
before do
stub_licensed_features(feature => feature_enabled)
stub_application_setting(application_setting => app_setting)
project.update(setting => project_setting)
end
it 'shows proper setting' do
expect(project.send(setting)).to eq(final_setting)
end
end
end
describe '#disable_overriding_approvers_per_merge_request' do
it_behaves_like 'setting modified by application setting' do
let(:feature) { :admin_merge_request_approvers_rules }
let(:setting) { :disable_overriding_approvers_per_merge_request }
let(:application_setting) { :disable_overriding_approvers_per_merge_request }
end
end
describe '#merge_requests_disable_committers_approval' do
it_behaves_like 'setting modified by application setting' do
let(:feature) { :admin_merge_request_approvers_rules }
let(:setting) { :merge_requests_disable_committers_approval }
let(:application_setting) { :prevent_merge_requests_committers_approval }
end
end
describe '#merge_requests_author_approval' do
using RSpec::Parameterized::TableSyntax
where(:app_setting, :project_setting, :feature_enabled, :final_setting) do
true | true | true | false
false | true | true | true
true | false | true | false
false | false | true | false
true | true | false | true
false | true | false | true
true | false | false | false
false | false | false | false
end
with_them do
let(:project) { create(:project) }
let(:feature) { :admin_merge_request_approvers_rules }
let(:setting) { :merge_requests_author_approval }
let(:application_setting) { :prevent_merge_requests_author_approval }
before do
stub_licensed_features(feature => feature_enabled)
stub_application_setting(application_setting => app_setting)
project.update(setting => project_setting)
end
it 'shows proper setting' do
expect(project.send(setting)).to eq(final_setting)
end
end
end
end
describe '#has_active_hooks?' do describe '#has_active_hooks?' do
context "with group hooks" do context "with group hooks" do
let(:group) { create(:group) } let(:group) { create(:group) }
......
...@@ -1213,4 +1213,145 @@ describe ProjectPolicy do ...@@ -1213,4 +1213,145 @@ describe ProjectPolicy do
it { is_expected.to be_disallowed(:read_code_review_analytics) } it { is_expected.to be_disallowed(:read_code_review_analytics) }
end end
end end
shared_examples 'merge request rules' do
let(:project) { create(:project, namespace: owner.namespace) }
using RSpec::Parameterized::TableSyntax
context 'with merge request approvers rules available in license' do
where(:role, :setting, :allowed) do
:guest | true | false
:reporter | true | false
:developer | true | false
:maintainer | false | true
:maintainer | true | false
:owner | false | true
:owner | true | false
:admin | false | true
:admin | true | true
end
with_them do
let(:current_user) { public_send(role) }
before do
stub_licensed_features(admin_merge_request_approvers_rules: true)
stub_application_setting(setting_name => setting)
end
it do
is_expected.to(allowed ? be_allowed(policy) : be_disallowed(policy))
end
end
end
context 'with merge request approvers not available in license' do
where(:role, :setting, :allowed) do
:guest | true | false
:reporter | true | false
:developer | true | false
:maintainer | false | true
:maintainer | true | true
:owner | false | true
:owner | true | true
:admin | true | true
:admin | false | true
end
with_them do
let(:current_user) { public_send(role) }
before do
stub_licensed_features(admin_merge_request_approvers_rules: false)
stub_application_setting(setting_name => setting)
end
it do
is_expected.to(allowed ? be_allowed(policy) : be_disallowed(policy))
end
end
end
end
describe ':modify_approvers_rules' do
it_behaves_like 'merge request rules' do
let(:setting_name) { :disable_overriding_approvers_per_merge_request }
let(:policy) { :modify_approvers_rules }
end
end
describe ':modify_merge_request_author_setting' do
it_behaves_like 'merge request rules' do
let(:setting_name) { :prevent_merge_requests_author_approval }
let(:policy) { :modify_merge_request_author_setting }
end
end
describe ':modify_merge_request_committer_setting' do
it_behaves_like 'merge request rules' do
let(:setting_name) { :prevent_merge_requests_committers_approval }
let(:policy) { :modify_merge_request_committer_setting }
end
end
describe ':modify_approvers_list' do
let(:setting_name) { :disable_overriding_approvers_per_merge_request }
let(:policy) { :modify_approvers_list }
let(:project) { create(:project, namespace: owner.namespace) }
using RSpec::Parameterized::TableSyntax
context 'with merge request approvers rules available in license' do
where(:role, :setting, :allowed) do
:guest | true | false
:reporter | true | false
:developer | true | false
:maintainer | false | true
:maintainer | true | false
:owner | false | true
:owner | true | false
:admin | false | true
:admin | true | true
end
with_them do
let(:current_user) { public_send(role) }
before do
stub_licensed_features(admin_merge_request_approvers_rules: true)
stub_application_setting(setting_name => setting)
end
it do
is_expected.to(allowed ? be_allowed(policy) : be_disallowed(policy))
end
end
end
context 'with merge request approvers not available in license' do
where(:role, :setting, :allowed) do
:guest | true | false
:reporter | true | false
:developer | true | false
:maintainer | false | true
:maintainer | true | true
:owner | false | true
:owner | true | true
:admin | true | true
:admin | false | true
end
with_them do
let(:current_user) { public_send(role) }
before do
stub_licensed_features(admin_merge_request_approvers_rules: false)
stub_application_setting(setting_name => setting)
end
it do
is_expected.to(allowed ? be_allowed(policy) : be_disallowed(policy))
end
end
end
end
end end
...@@ -98,6 +98,39 @@ describe API::ProjectApprovals do ...@@ -98,6 +98,39 @@ describe API::ProjectApprovals do
end end
end end
shared_examples 'updates merge requests settings when possible' do
using RSpec::Parameterized::TableSyntax
where(:license_value, :setting_value, :param_value, :final_value) do
false | false | false | false
false | true | false | false
false | false | true | true
false | true | true | true
true | false | false | false
true | true | false | false
true | false | true | true
true | true | true | true
end
with_them do
before do
stub_licensed_features(admin_merge_request_approvers_rules: license_value)
stub_application_setting(app_setting => setting_value)
end
it 'changes settings properly' do
settings = {
setting => param_value
}
post api(url, current_user), params: settings
project.reload
expect(project[setting]).to eq(final_value)
end
end
end
context 'as a project admin' do context 'as a project admin' do
it_behaves_like 'a user with access' do it_behaves_like 'a user with access' do
let(:current_user) { user } let(:current_user) { user }
...@@ -110,6 +143,26 @@ describe API::ProjectApprovals do ...@@ -110,6 +143,26 @@ describe API::ProjectApprovals do
let(:current_user) { admin } let(:current_user) { admin }
let(:visible_approver_groups_count) { 1 } let(:visible_approver_groups_count) { 1 }
end end
context 'updates merge requests settings' do
it_behaves_like 'updates merge requests settings when possible' do
let(:current_user) { admin }
let(:app_setting) { :disable_overriding_approvers_per_merge_request }
let(:setting) { :disable_overriding_approvers_per_merge_request }
end
it_behaves_like 'updates merge requests settings when possible' do
let(:current_user) { admin }
let(:app_setting) { :prevent_merge_requests_committers_approval }
let(:setting) { :merge_requests_disable_committers_approval }
end
it_behaves_like 'updates merge requests settings when possible' do
let(:current_user) { admin }
let(:app_setting) { :prevent_merge_requests_committers_approval }
let(:setting) { :merge_requests_disable_committers_approval }
end
end
end end
context 'as a user without access' do context 'as a user without access' do
......
...@@ -163,4 +163,17 @@ describe API::Settings, 'EE Settings' do ...@@ -163,4 +163,17 @@ describe API::Settings, 'EE Settings' do
it_behaves_like 'settings for licensed features' it_behaves_like 'settings for licensed features'
end end
context 'merge request approvers rules' do
let(:settings) do
{
disable_overriding_approvers_per_merge_request: true,
prevent_merge_requests_author_approval: true,
prevent_merge_requests_committers_approval: true
}
end
let(:feature) { :admin_merge_request_approvers_rules }
it_behaves_like 'settings for licensed features'
end
end end
...@@ -11917,6 +11917,9 @@ msgstr "" ...@@ -11917,6 +11917,9 @@ msgstr ""
msgid "Merge requests" msgid "Merge requests"
msgstr "" msgstr ""
msgid "Merge requests approvals"
msgstr ""
msgid "Merge requests are a place to propose changes you've made to a project and discuss those changes with others" msgid "Merge requests are a place to propose changes you've made to a project and discuss those changes with others"
msgstr "" msgstr ""
...@@ -14161,6 +14164,9 @@ msgstr "" ...@@ -14161,6 +14164,9 @@ msgstr ""
msgid "Prevent users from changing their profile name" msgid "Prevent users from changing their profile name"
msgstr "" msgstr ""
msgid "Prevent users from modifing merge request approvers list"
msgstr ""
msgid "Preview" msgid "Preview"
msgstr "" msgstr ""
...@@ -16385,6 +16391,9 @@ msgstr "" ...@@ -16385,6 +16391,9 @@ msgstr ""
msgid "Rook" msgid "Rook"
msgstr "" msgstr ""
msgid "Rules that define what git pushes are accepted for a project. All newly created projects will use these settings."
msgstr ""
msgid "Run CI/CD pipelines for external repositories" msgid "Run CI/CD pipelines for external repositories"
msgstr "" msgstr ""
...@@ -17427,6 +17436,9 @@ msgstr "" ...@@ -17427,6 +17436,9 @@ msgstr ""
msgid "Settings" msgid "Settings"
msgstr "" msgstr ""
msgid "Settings to prevent self-approval across all projects in the instance. Only an administrator can modify these settings."
msgstr ""
msgid "Severity: %{severity}" msgid "Severity: %{severity}"
msgstr "" msgstr ""
......
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