diff options
| -rw-r--r-- | changelogs/unreleased/rs-security-group-api.yml | 5 | ||||
| -rw-r--r-- | lib/api/entities.rb | 17 | ||||
| -rw-r--r-- | spec/requests/api/groups_spec.rb | 62 |
3 files changed, 82 insertions, 2 deletions
diff --git a/changelogs/unreleased/rs-security-group-api.yml b/changelogs/unreleased/rs-security-group-api.yml new file mode 100644 index 00000000000..34a39ddd6dc --- /dev/null +++ b/changelogs/unreleased/rs-security-group-api.yml @@ -0,0 +1,5 @@ +--- +title: Prevent an information disclosure in the Groups API +merge_request: +author: +type: security diff --git a/lib/api/entities.rb b/lib/api/entities.rb index d96e7f2770f..928706dfda7 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -248,8 +248,21 @@ module API end class GroupDetail < Group - expose :projects, using: Entities::Project - expose :shared_projects, using: Entities::Project + expose :projects, using: Entities::Project do |group, options| + GroupProjectsFinder.new( + group: group, + current_user: options[:current_user], + options: { only_owned: true } + ).execute + end + + expose :shared_projects, using: Entities::Project do |group, options| + GroupProjectsFinder.new( + group: group, + current_user: options[:current_user], + options: { only_shared: true } + ).execute + end end class Commit < Grape::Entity diff --git a/spec/requests/api/groups_spec.rb b/spec/requests/api/groups_spec.rb index 554723d6b1e..6330c140246 100644 --- a/spec/requests/api/groups_spec.rb +++ b/spec/requests/api/groups_spec.rb @@ -173,6 +173,28 @@ describe API::Groups do end describe "GET /groups/:id" do + # Given a group, create one project for each visibility level + # + # group - Group to add projects to + # share_with - If provided, each project will be shared with this Group + # + # Returns a Hash of visibility_level => Project pairs + def add_projects_to_group(group, share_with: nil) + projects = { + public: create(:project, :public, namespace: group), + internal: create(:project, :internal, namespace: group), + private: create(:project, :private, namespace: group) + } + + if share_with + create(:project_group_link, project: projects[:public], group: share_with) + create(:project_group_link, project: projects[:internal], group: share_with) + create(:project_group_link, project: projects[:private], group: share_with) + end + + projects + end + context 'when unauthenticated' do it 'returns 404 for a private group' do get api("/groups/#{group2.id}") @@ -183,6 +205,26 @@ describe API::Groups do get api("/groups/#{group1.id}") expect(response).to have_gitlab_http_status(200) end + + it 'returns only public projects in the group' do + public_group = create(:group, :public) + projects = add_projects_to_group(public_group) + + get api("/groups/#{public_group.id}") + + expect(json_response['projects'].map { |p| p['id'].to_i }) + .to contain_exactly(projects[:public].id) + end + + it 'returns only public projects shared with the group' do + public_group = create(:group, :public) + projects = add_projects_to_group(public_group, share_with: group1) + + get api("/groups/#{group1.id}") + + expect(json_response['shared_projects'].map { |p| p['id'].to_i }) + .to contain_exactly(projects[:public].id) + end end context "when authenticated as user" do @@ -222,6 +264,26 @@ describe API::Groups do expect(response).to have_gitlab_http_status(404) end + + it 'returns only public and internal projects in the group' do + public_group = create(:group, :public) + projects = add_projects_to_group(public_group) + + get api("/groups/#{public_group.id}", user2) + + expect(json_response['projects'].map { |p| p['id'].to_i }) + .to contain_exactly(projects[:public].id, projects[:internal].id) + end + + it 'returns only public and internal projects shared with the group' do + public_group = create(:group, :public) + projects = add_projects_to_group(public_group, share_with: group1) + + get api("/groups/#{group1.id}", user2) + + expect(json_response['shared_projects'].map { |p| p['id'].to_i }) + .to contain_exactly(projects[:public].id, projects[:internal].id) + end end context "when authenticated as admin" do |