summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorStan Hu <stanhu@gmail.com>2019-04-30 11:39:15 -0700
committerStan Hu <stanhu@gmail.com>2019-04-30 13:31:51 -0700
commit32ddc3fed616b9eebdd7b5553d4aa08c0572ff1d (patch)
tree97b7858c5d98135b836576caf1fa030c4ec772b6
parent9f592604032dd4a4d685fa359865ed9786f3e058 (diff)
downloadgitlab-ce-sh-allow-equal-level-in-subgroup-membership.tar.gz
Allow a member to have an access level equal to parent groupsh-allow-equal-level-in-subgroup-membership
Suppose you have this configuration: 1. Subgroup `hello/world` 2. Subgroup `hello/mergers`. 3. Project `hello/world/my-project` has invited group `hello/world` to access protected branches. 4. The rule allows the group to merge but no one can push. 5. User `newuser` has Owner access to the parent group `hello`. Previously, there was no way for the user `newuser` to be added to the `hello/mergers` group since the validation only allowed a user to be added at a higher access level. Since membership in a subgroup confers certain access rights, such as being able to merge or push code to protected branches, we have to loosen the validation and allow someone to be added at an equal level granted by the parent group. Closes https://gitlab.com/gitlab-org/gitlab-ee/issues/11323
-rw-r--r--app/models/member.rb4
-rw-r--r--changelogs/unreleased/sh-allow-equal-level-in-subgroup-membership.yml5
-rw-r--r--locale/gitlab.pot2
-rw-r--r--spec/models/member_spec.rb10
-rw-r--r--spec/requests/api/members_spec.rb2
-rw-r--r--spec/support/shared_examples/models/member_shared_examples.rb2
6 files changed, 20 insertions, 5 deletions
diff --git a/app/models/member.rb b/app/models/member.rb
index 8a06bff51b5..83b4f5b29c4 100644
--- a/app/models/member.rb
+++ b/app/models/member.rb
@@ -446,10 +446,10 @@ class Member < ApplicationRecord
end
def higher_access_level_than_group
- if highest_group_member && highest_group_member.access_level >= access_level
+ if highest_group_member && highest_group_member.access_level > access_level
error_parameters = { access: highest_group_member.human_access, group_name: highest_group_member.group.name }
- errors.add(:access_level, s_("should be higher than %{access} inherited membership from group %{group_name}") % error_parameters)
+ errors.add(:access_level, s_("should be greater than or equal to %{access} inherited membership from group %{group_name}") % error_parameters)
end
end
end
diff --git a/changelogs/unreleased/sh-allow-equal-level-in-subgroup-membership.yml b/changelogs/unreleased/sh-allow-equal-level-in-subgroup-membership.yml
new file mode 100644
index 00000000000..adbed52db81
--- /dev/null
+++ b/changelogs/unreleased/sh-allow-equal-level-in-subgroup-membership.yml
@@ -0,0 +1,5 @@
+---
+title: Allow a member to have an access level equal to parent group
+merge_request: 27913
+author:
+type: fixed
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index 7b5770a6075..8718df54a7e 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -11385,7 +11385,7 @@ msgstr[1] ""
msgid "score"
msgstr ""
-msgid "should be higher than %{access} inherited membership from group %{group_name}"
+msgid "should be greater than or equal to %{access} inherited membership from group %{group_name}"
msgstr ""
msgid "show less"
diff --git a/spec/models/member_spec.rb b/spec/models/member_spec.rb
index c68c3ce2abe..782a84f922b 100644
--- a/spec/models/member_spec.rb
+++ b/spec/models/member_spec.rb
@@ -70,6 +70,16 @@ describe Member do
expect(child_member).not_to be_valid
end
+ # Membership in a subgroup confers certain access rights, such as being
+ # able to merge or push code to protected branches.
+ it "is valid with an equal level" do
+ child_member.access_level = GroupMember::DEVELOPER
+
+ child_member.validate
+
+ expect(child_member).to be_valid
+ end
+
it "is valid with a higher level" do
child_member.access_level = GroupMember::MAINTAINER
diff --git a/spec/requests/api/members_spec.rb b/spec/requests/api/members_spec.rb
index 79edbb301f2..48869cab4da 100644
--- a/spec/requests/api/members_spec.rb
+++ b/spec/requests/api/members_spec.rb
@@ -236,7 +236,7 @@ describe API::Members do
params: { user_id: stranger.id, access_level: Member::REPORTER }
expect(response).to have_gitlab_http_status(400)
- expect(json_response['message']['access_level']).to eq(["should be higher than Developer inherited membership from group #{parent.name}"])
+ expect(json_response['message']['access_level']).to eq(["should be greater than or equal to Developer inherited membership from group #{parent.name}"])
end
it 'creates the member if group level is lower', :nested_groups do
diff --git a/spec/support/shared_examples/models/member_shared_examples.rb b/spec/support/shared_examples/models/member_shared_examples.rb
index 77376496854..e5375bc8280 100644
--- a/spec/support/shared_examples/models/member_shared_examples.rb
+++ b/spec/support/shared_examples/models/member_shared_examples.rb
@@ -41,7 +41,7 @@ shared_examples_for 'inherited access level as a member of entity' do
member.update(access_level: Gitlab::Access::REPORTER)
- expect(member.errors.full_messages).to eq(["Access level should be higher than Developer inherited membership from group #{parent_entity.name}"])
+ expect(member.errors.full_messages).to eq(["Access level should be greater than or equal to Developer inherited membership from group #{parent_entity.name}"])
end
it 'allows changing the level from a non existing member' do