From ab94a5a53712740df3836413bf26e4856b5f7cb2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Rodr=C3=ADguez?= Date: Tue, 15 Jan 2019 14:57:17 -0300 Subject: Return max group access level in the projects API Currently if a project is inside a nested group and a user doesn't have specific permissions for that group but does have permissions on a parent group the `GET /projects/:id` API call will return the following permissions: ```json permissions: { project_access: null, group_access: null } ``` It could also happen that the group specific permissions are of lower level than the ones the user has in parent groups. This patch makes it so that the permission returned for `group_access` is the highest from amongst the hierarchy, which is (ostensibly) the information that the API user is interested in for that field. --- app/models/group.rb | 4 +++ .../unreleased/api-nested-group-permission.yml | 5 +++ lib/api/entities.rb | 2 +- spec/models/group_spec.rb | 36 ++++++++++++++++++++++ spec/requests/api/projects_spec.rb | 34 ++++++++++++++++++++ 5 files changed, 80 insertions(+), 1 deletion(-) create mode 100644 changelogs/unreleased/api-nested-group-permission.yml diff --git a/app/models/group.rb b/app/models/group.rb index edac2444c4d..22814e35ca8 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -382,6 +382,10 @@ class Group < Namespace end end + def highest_group_member(user) + GroupMember.where(source_id: self_and_ancestors_ids, user_id: user.id).order(:access_level).last + end + def hashed_storage?(_feature) false end diff --git a/changelogs/unreleased/api-nested-group-permission.yml b/changelogs/unreleased/api-nested-group-permission.yml new file mode 100644 index 00000000000..3ec0df6893f --- /dev/null +++ b/changelogs/unreleased/api-nested-group-permission.yml @@ -0,0 +1,5 @@ +--- +title: Return the maximum group access level in the projects API +merge_request: 24403 +author: +type: changed diff --git a/lib/api/entities.rb b/lib/api/entities.rb index a2a3c0a16d7..f13741f80d2 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -961,7 +961,7 @@ module API if options[:group_members] options[:group_members].find { |member| member.source_id == project.namespace_id } else - project.group.group_member(options[:current_user]) + project.group.highest_group_member(options[:current_user]) end end end diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index e63881242f6..9dc32a815d8 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -722,6 +722,42 @@ describe Group do end end + describe '#highest_group_member', :nested_groups do + let(:nested_group) { create(:group, parent: group) } + let(:nested_group_2) { create(:group, parent: nested_group) } + let(:user) { create(:user) } + + subject(:highest_group_member) { nested_group_2.highest_group_member(user) } + + context 'when the user is not a member of any group in the hierarchy' do + it 'returns nil' do + expect(highest_group_member).to be_nil + end + end + + context 'when the user is only a member of one group in the hierarchy' do + before do + nested_group.add_developer(user) + end + + it 'returns that group member' do + expect(highest_group_member.access_level).to eq(Gitlab::Access::DEVELOPER) + end + end + + context 'when the user is a member of several groups in the hierarchy' do + before do + group.add_owner(user) + nested_group.add_developer(user) + nested_group_2.add_maintainer(user) + end + + it 'returns the group member with the highest access level' do + expect(highest_group_member.access_level).to eq(Gitlab::Access::OWNER) + end + end + end + describe '#has_parent?' do context 'when the group has a parent' do it 'should be truthy' do diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index ffe4512fa6f..0c48c796ceb 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -1145,6 +1145,40 @@ describe API::Projects do .to eq(Gitlab::Access::OWNER) end end + + context 'nested group project', :nested_groups do + let(:group) { create(:group) } + let(:nested_group) { create(:group, parent: group) } + let(:project2) { create(:project, group: nested_group) } + + before do + project2.group.parent.add_owner(user) + end + + it 'sets group access and return 200' do + get api("/projects/#{project2.id}", user) + + expect(response).to have_gitlab_http_status(200) + expect(json_response['permissions']['project_access']).to be_nil + expect(json_response['permissions']['group_access']['access_level']) + .to eq(Gitlab::Access::OWNER) + end + + context 'with various access levels across nested groups' do + before do + project2.group.add_maintainer(user) + end + + it 'sets the maximum group access and return 200' do + get api("/projects/#{project2.id}", user) + + expect(response).to have_gitlab_http_status(200) + expect(json_response['permissions']['project_access']).to be_nil + expect(json_response['permissions']['group_access']['access_level']) + .to eq(Gitlab::Access::OWNER) + end + end + end end end end -- cgit v1.2.1