Skip to content

Commit ab94a5a

Browse files
committed
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.
1 parent f821a53 commit ab94a5a

File tree

5 files changed

+80
-1
lines changed

5 files changed

+80
-1
lines changed

app/models/group.rb

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -382,6 +382,10 @@ def group_member(user)
382382
end
383383
end
384384

385+
def highest_group_member(user)
386+
GroupMember.where(source_id: self_and_ancestors_ids, user_id: user.id).order(:access_level).last
387+
end
388+
385389
def hashed_storage?(_feature)
386390
false
387391
end
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
title: Return the maximum group access level in the projects API
3+
merge_request: 24403
4+
author:
5+
type: changed

lib/api/entities.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -961,7 +961,7 @@ class ProjectWithAccess < Project
961961
if options[:group_members]
962962
options[:group_members].find { |member| member.source_id == project.namespace_id }
963963
else
964-
project.group.group_member(options[:current_user])
964+
project.group.highest_group_member(options[:current_user])
965965
end
966966
end
967967
end

spec/models/group_spec.rb

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -722,6 +722,42 @@ def setup_group_members(group)
722722
end
723723
end
724724

725+
describe '#highest_group_member', :nested_groups do
726+
let(:nested_group) { create(:group, parent: group) }
727+
let(:nested_group_2) { create(:group, parent: nested_group) }
728+
let(:user) { create(:user) }
729+
730+
subject(:highest_group_member) { nested_group_2.highest_group_member(user) }
731+
732+
context 'when the user is not a member of any group in the hierarchy' do
733+
it 'returns nil' do
734+
expect(highest_group_member).to be_nil
735+
end
736+
end
737+
738+
context 'when the user is only a member of one group in the hierarchy' do
739+
before do
740+
nested_group.add_developer(user)
741+
end
742+
743+
it 'returns that group member' do
744+
expect(highest_group_member.access_level).to eq(Gitlab::Access::DEVELOPER)
745+
end
746+
end
747+
748+
context 'when the user is a member of several groups in the hierarchy' do
749+
before do
750+
group.add_owner(user)
751+
nested_group.add_developer(user)
752+
nested_group_2.add_maintainer(user)
753+
end
754+
755+
it 'returns the group member with the highest access level' do
756+
expect(highest_group_member.access_level).to eq(Gitlab::Access::OWNER)
757+
end
758+
end
759+
end
760+
725761
describe '#has_parent?' do
726762
context 'when the group has a parent' do
727763
it 'should be truthy' do

spec/requests/api/projects_spec.rb

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1145,6 +1145,40 @@
11451145
.to eq(Gitlab::Access::OWNER)
11461146
end
11471147
end
1148+
1149+
context 'nested group project', :nested_groups do
1150+
let(:group) { create(:group) }
1151+
let(:nested_group) { create(:group, parent: group) }
1152+
let(:project2) { create(:project, group: nested_group) }
1153+
1154+
before do
1155+
project2.group.parent.add_owner(user)
1156+
end
1157+
1158+
it 'sets group access and return 200' do
1159+
get api("/projects/#{project2.id}", user)
1160+
1161+
expect(response).to have_gitlab_http_status(200)
1162+
expect(json_response['permissions']['project_access']).to be_nil
1163+
expect(json_response['permissions']['group_access']['access_level'])
1164+
.to eq(Gitlab::Access::OWNER)
1165+
end
1166+
1167+
context 'with various access levels across nested groups' do
1168+
before do
1169+
project2.group.add_maintainer(user)
1170+
end
1171+
1172+
it 'sets the maximum group access and return 200' do
1173+
get api("/projects/#{project2.id}", user)
1174+
1175+
expect(response).to have_gitlab_http_status(200)
1176+
expect(json_response['permissions']['project_access']).to be_nil
1177+
expect(json_response['permissions']['group_access']['access_level'])
1178+
.to eq(Gitlab::Access::OWNER)
1179+
end
1180+
end
1181+
end
11481182
end
11491183
end
11501184
end

0 commit comments

Comments
 (0)