Prevent projects to have higher visibility than groups

Prevent Groups to have smaller visibility than projects
Add default_group_visibility_level to configuration
Code improvements

4 files changed, 118 insertions, 0 deletions

diff --git a/spec/controllers/groups_controller_spec.rb b/spec/controllers/groups_controller_spec.rb
index e7ead824d20..91db3fd1ee2 100644
--- a/spec/controllers/groups_controller_spec.rb
+++ b/spec/controllers/groups_controller_spec.rb
@@ -54,6 +54,7 @@ describe GroupsController do
     let(:group) { create(:group, visibility_level: 20) }
 
     it 'checks if group can be updated' do
+      expect_any_instance_of(Groups::UpdateService).to receive(:execute)
       expect(controller).to receive(:authorize_admin_group!)
       put :update, id: group.path, group: { name: 'test' }
     end
diff --git a/spec/finders/joined_groups_finder_spec.rb b/spec/finders/joined_groups_finder_spec.rb
new file mode 100644
index 00000000000..e2f6c593638
--- /dev/null
+++ b/spec/finders/joined_groups_finder_spec.rb
@@ -0,0 +1,51 @@
+require 'spec_helper'
+
+describe JoinedGroupsFinder do
+  describe '#execute' do
+    let!(:profile_owner)    { create(:user) }
+    let!(:profile_visitor)  { create(:user) }
+
+    let!(:private_group)    { create(:group, visibility_level: Gitlab::VisibilityLevel::PRIVATE) }
+    let!(:private_group_2)  { create(:group, visibility_level: Gitlab::VisibilityLevel::PRIVATE) }
+    let!(:internal_group)   { create(:group, visibility_level: Gitlab::VisibilityLevel::INTERNAL) }
+    let!(:internal_group_2) { create(:group, visibility_level: Gitlab::VisibilityLevel::INTERNAL) }
+    let!(:public_group)     { create(:group, visibility_level: Gitlab::VisibilityLevel::PUBLIC) }
+    let!(:public_group_2)   { create(:group, visibility_level: Gitlab::VisibilityLevel::PUBLIC) }
+    let!(:finder) { described_class.new(profile_owner) }
+
+    describe 'execute' do
+      context 'without a user only shows public groups from profile owner' do
+        before { public_group.add_user(profile_owner, Gitlab::Access::MASTER)}
+        subject { finder.execute }
+
+        it { is_expected.to eq([public_group]) }
+      end
+
+      context 'only shows groups where both users are authorized to see' do
+        subject { finder.execute(profile_visitor) }
+
+        before do
+          private_group.add_user(profile_owner, Gitlab::Access::MASTER)
+          private_group.add_user(profile_visitor, Gitlab::Access::DEVELOPER)
+          internal_group.add_user(profile_owner, Gitlab::Access::MASTER)
+          public_group.add_user(profile_owner, Gitlab::Access::MASTER)
+        end
+
+        it { is_expected.to eq([public_group, internal_group, private_group]) }
+      end
+
+      context 'shows group if profile visitor is in one of its projects' do
+        before do
+          public_group.add_user(profile_owner, Gitlab::Access::MASTER)
+          private_group.add_user(profile_owner, Gitlab::Access::MASTER)
+          project = create(:project, :private, group: private_group, name: 'B', path: 'B')
+          project.team.add_user(profile_visitor, Gitlab::Access::DEVELOPER)
+        end
+
+        subject { finder.execute(profile_visitor) }
+
+        it { is_expected.to eq([public_group, private_group]) }
+      end
+    end
+  end
+end
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index 2fa38a5d3d3..9efaffbb577 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -583,6 +583,21 @@ describe Project, models: true do
       it { expect(forked_project.visibility_level_allowed?(Gitlab::VisibilityLevel::PUBLIC)).to be_falsey }
     end
 
+    context 'when checking projects from groups' do
+      let(:private_group)    { create(:group, visibility_level: 0)  }
+      let(:internal_group)   { create(:group, visibility_level: 10) }
+
+      let(:private_project)  { create :project, group: private_group, visibility_level: Gitlab::VisibilityLevel::PRIVATE   }
+      let(:internal_project) { create :project, group: internal_group, visibility_level: Gitlab::VisibilityLevel::INTERNAL }
+
+      context 'when group is private project can not be internal' do
+        it { expect(private_project.visibility_level_allowed?(Gitlab::VisibilityLevel::INTERNAL)).to be_falsey }
+      end
+
+      context 'when group is internal project can not be public' do
+        it { expect(internal_project.visibility_level_allowed?(Gitlab::VisibilityLevel::PUBLIC)).to be_falsey }
+      end
+    end
   end
 
   describe '#rename_repo' do
diff --git a/spec/services/groups/update_service_spec.rb b/spec/services/groups/update_service_spec.rb
new file mode 100644
index 00000000000..c759e32342d
--- /dev/null
+++ b/spec/services/groups/update_service_spec.rb
@@ -0,0 +1,51 @@
+require 'spec_helper'
+
+describe Groups::UpdateService, services: true do
+    let!(:user)    { create(:user) }
+    let!(:private_group)    { create(:group, visibility_level: Gitlab::VisibilityLevel::PRIVATE) }
+    let!(:internal_group)   { create(:group, visibility_level: Gitlab::VisibilityLevel::INTERNAL) }
+    let!(:public_group)     { create(:group, visibility_level: Gitlab::VisibilityLevel::PUBLIC) }
+
+  describe "execute" do
+    context "project visibility_level validation" do
+
+      context "public group with public projects" do
+        let!(:service) { described_class.new(public_group, user, visibility_level: Gitlab::VisibilityLevel::INTERNAL ) }
+
+        before do
+          public_group.add_user(user, Gitlab::Access::MASTER)
+          create(:project, :public, group: public_group, name: 'B', path: 'B')
+        end
+
+        it "cant downgrade permission level" do
+          expect(service.execute).to be_falsy
+          expect(public_group.errors.count).to eq(1)
+        end
+      end
+
+    context "internal group with internal project" do
+        let!(:service) { described_class.new(internal_group, user, visibility_level: Gitlab::VisibilityLevel::PRIVATE ) }
+
+        before do
+          internal_group.add_user(user, Gitlab::Access::MASTER)
+          create(:project, :internal, group: internal_group, name: 'B', path: 'B')
+        end
+
+        it "cant downgrade permission level" do
+          expect(service.execute).to be_falsy
+          expect(internal_group.errors.count).to eq(1)
+        end
+      end
+    end
+  end
+
+  context "unauthorized visibility_level validation" do
+    let!(:service) { described_class.new(internal_group, user, visibility_level: 99 ) }
+    before { internal_group.add_user(user, Gitlab::Access::MASTER) }
+
+    it "does not change permission level" do
+      expect(service.execute).to be_falsy
+      expect(internal_group.errors.count).to eq(1)
+    end
+  end
+end