Commit 899991a0 authored by Tan Le's avatar Tan Le

Restrict group MR approval settings to root group

This commit restricts the permission to administer group merge request
approval settings to root group. Sub groups (i.e. groups which have
parent groups) will not be able to access this feature.

Recursive settings resolution of deeply nested group hierarchy is
computation-extensive. For the MVP, we would like to mitigate this risk
and re-evaluate this feature on nested group after collecting more user
feedback.
parent 0761da4e
......@@ -111,7 +111,7 @@ module EE
end
condition(:group_merge_request_approval_settings_enabled) do
@subject.feature_available?(:group_merge_request_approval_settings)
@subject.feature_available?(:group_merge_request_approval_settings) && @subject.root?
end
condition(:over_storage_limit, scope: :subject) { @subject.over_storage_limit? }
......
......@@ -1449,21 +1449,23 @@ RSpec.describe GroupPolicy do
let(:policy) { :admin_merge_request_approval_settings }
where(:role, :licensed, :admin_mode, :allowed) do
:guest | true | nil | false
:guest | false | nil | false
:reporter | true | nil | false
:reporter | false | nil | false
:developer | true | nil | false
:developer | false | nil | false
:maintainer | true | nil | false
:maintainer | false | nil | false
:owner | true | nil | true
:owner | false | nil | false
:admin | true | true | true
:admin | false | true | false
:admin | true | false | false
:admin | false | false | false
where(:role, :licensed, :admin_mode, :root_group, :allowed) do
:guest | true | nil | true | false
:guest | false | nil | true | false
:reporter | true | nil | true | false
:reporter | false | nil | true | false
:developer | true | nil | true | false
:developer | false | nil | true | false
:maintainer | true | nil | true | false
:maintainer | false | nil | true | false
:owner | true | nil | true | true
:owner | true | nil | false | false
:owner | false | nil | true | false
:admin | true | true | true | true
:admin | true | true | false | false
:admin | false | true | true | false
:admin | true | false | true | false
:admin | false | false | true | false
end
with_them do
......@@ -1472,6 +1474,7 @@ RSpec.describe GroupPolicy do
before do
stub_licensed_features(group_merge_request_approval_settings: licensed)
enable_admin_mode!(current_user) if admin_mode
group.parent = build(:group) unless root_group
end
it { is_expected.to(allowed ? be_allowed(policy) : be_disallowed(policy)) }
......
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