Merge branch 'security-add-public-internal-groups-as-members-to-your-project-idor-11-6' into '11-6-stable'

Add public/internal groups as members to your Project(IDOR)

See merge request gitlab/gitlabhq!2958

6 files changed, 60 insertions, 0 deletions

diff --git a/spec/controllers/groups/shared_projects_controller_spec.rb b/spec/controllers/groups/shared_projects_controller_spec.rb
index 003c8c262e7..bf551833094 100644
--- a/spec/controllers/groups/shared_projects_controller_spec.rb
+++ b/spec/controllers/groups/shared_projects_controller_spec.rb
@@ -6,6 +6,8 @@ describe Groups::SharedProjectsController do
   end
 
   def share_project(project)
+    group.add_developer(user)
+
     Projects::GroupLinks::CreateService.new(
       project,
       user,
diff --git a/spec/controllers/projects/group_links_controller_spec.rb b/spec/controllers/projects/group_links_controller_spec.rb
index 879aff26deb..6b12a6ae720 100644
--- a/spec/controllers/projects/group_links_controller_spec.rb
+++ b/spec/controllers/projects/group_links_controller_spec.rb
@@ -63,8 +63,24 @@ describe Projects::GroupLinksController do
       end
     end
 
+    context 'when user does not have access to the public group' do
+      let(:group) { create(:group, :public) }
+
+      include_context 'link project to group'
+
+      it 'renders 404' do
+        expect(response.status).to eq 404
+      end
+
+      it 'does not share project with that group' do
+        expect(group.shared_projects).not_to include project
+      end
+    end
+
     context 'when project group id equal link group id' do
       before do
+        group2.add_developer(user)
+
         post(:create, namespace_id: project.namespace,
                       project_id: project,
                       link_group_id: group2.id,
@@ -96,5 +112,24 @@ describe Projects::GroupLinksController do
         expect(flash[:alert]).to eq('Please select a group.')
       end
     end
+
+    context 'when link is not persisted in the database' do
+      before do
+        allow(::Projects::GroupLinks::CreateService).to receive_message_chain(:new, :execute)
+          .and_return({ status: :error, http_status: 409, message: 'error' })
+
+        post(:create, namespace_id: project.namespace,
+                      project_id: project,
+                      link_group_id: group.id,
+                      link_group_access: ProjectGroupLink.default_access)
+      end
+
+      it 'redirects to project group links page' do
+        expect(response).to redirect_to(
+          project_project_members_path(project)
+        )
+        expect(flash[:alert]).to eq('error')
+      end
+    end
   end
 end
diff --git a/spec/features/projects/members/invite_group_spec.rb b/spec/features/projects/members/invite_group_spec.rb
index fceead0b45e..b2d2dba55f1 100644
--- a/spec/features/projects/members/invite_group_spec.rb
+++ b/spec/features/projects/members/invite_group_spec.rb
@@ -27,6 +27,7 @@ describe 'Project > Members > Invite group', :js do
 
       before do
         project.add_maintainer(maintainer)
+        group_to_share_with.add_guest(maintainer)
         sign_in(maintainer)
       end
 
@@ -112,6 +113,7 @@ describe 'Project > Members > Invite group', :js do
 
     before do
       project.add_maintainer(maintainer)
+      group.add_guest(maintainer)
       sign_in(maintainer)
 
       visit project_settings_members_path(project)
diff --git a/spec/features/projects/settings/user_manages_group_links_spec.rb b/spec/features/projects/settings/user_manages_group_links_spec.rb
index 676659b90c3..e5a58c44e41 100644
--- a/spec/features/projects/settings/user_manages_group_links_spec.rb
+++ b/spec/features/projects/settings/user_manages_group_links_spec.rb
@@ -10,6 +10,7 @@ describe 'Projects > Settings > User manages group links' do
 
   before do
     project.add_maintainer(user)
+    group_market.add_guest(user)
     sign_in(user)
 
     share_link = project.project_group_links.new(group_access: Gitlab::Access::MAINTAINER)
diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb
index e40db55cd20..a33b1e2f09e 100644
--- a/spec/requests/api/projects_spec.rb
+++ b/spec/requests/api/projects_spec.rb
@@ -1392,6 +1392,9 @@ describe API::Projects do
 
   describe "POST /projects/:id/share" do
     let(:group) { create(:group) }
+    before do
+      group.add_developer(user)
+    end
 
     it "shares project with group" do
       expires_at = 10.days.from_now.to_date
@@ -1442,6 +1445,15 @@ describe API::Projects do
       expect(response).to have_gitlab_http_status(400)
       expect(json_response['error']).to eq 'group_access does not have a valid value'
     end
+
+    it "returns a 409 error when link is not saved" do
+      allow(::Projects::GroupLinks::CreateService).to receive_message_chain(:new, :execute)
+        .and_return({ status: :error, http_status: 409, message: 'error' })
+
+      post api("/projects/#{project.id}/share", user), group_id: group.id, group_access: Gitlab::Access::DEVELOPER
+
+      expect(response).to have_gitlab_http_status(409)
+    end
   end
 
   describe 'DELETE /projects/:id/share/:group_id' do
diff --git a/spec/services/projects/group_links/create_service_spec.rb b/spec/services/projects/group_links/create_service_spec.rb
index ffb270d277e..68fd82b4cbe 100644
--- a/spec/services/projects/group_links/create_service_spec.rb
+++ b/spec/services/projects/group_links/create_service_spec.rb
@@ -12,6 +12,10 @@ describe Projects::GroupLinks::CreateService, '#execute' do
   end
   let(:subject) { described_class.new(project, user, opts) }
 
+  before do
+    group.add_developer(user)
+  end
+
   it 'adds group to project' do
     expect { subject.execute(group) }.to change { project.project_group_links.count }.from(0).to(1)
   end
@@ -19,4 +23,8 @@ describe Projects::GroupLinks::CreateService, '#execute' do
   it 'returns false if group is blank' do
     expect { subject.execute(nil) }.not_to change { project.project_group_links.count }
   end
+
+  it 'returns error if user is not allowed to share with a group' do
+    expect { subject.execute(create :group) }.not_to change { project.project_group_links.count }
+  end
 end