Merge branch 'security-project-import-bypass' into 'master'

Project visibility restriction bypass

See merge request gitlab/gitlabhq!3306

2 files changed, 135 insertions, 15 deletions

diff --git a/spec/lib/gitlab/visibility_level_checker_spec.rb b/spec/lib/gitlab/visibility_level_checker_spec.rb
new file mode 100644
index 00000000000..325ac3c6f31
--- /dev/null
+++ b/spec/lib/gitlab/visibility_level_checker_spec.rb
@@ -0,0 +1,82 @@
+require 'spec_helper'
+
+describe Gitlab::VisibilityLevelChecker do
+  let(:user) { create(:user) }
+  let(:project) { create(:project) }
+  let(:visibility_level_checker) { }
+  let(:override_params) { {} }
+
+  subject { described_class.new(user, project, project_params: override_params) }
+
+  describe '#level_restricted?' do
+    context 'when visibility level is allowed' do
+      it 'returns false with nil for visibility level' do
+        result = subject.level_restricted?
+
+        expect(result.restricted?).to eq(false)
+        expect(result.visibility_level).to be_nil
+      end
+    end
+
+    context 'when visibility level is restricted' do
+      before do
+        stub_application_setting(restricted_visibility_levels: [Gitlab::VisibilityLevel::PUBLIC])
+      end
+
+      it 'returns true and visibility name' do
+        project.update!(visibility_level: Gitlab::VisibilityLevel::PUBLIC)
+        result = subject.level_restricted?
+
+        expect(result.restricted?).to eq(true)
+        expect(result.visibility_level).to eq(Gitlab::VisibilityLevel::PUBLIC)
+      end
+
+      context 'overridden visibility' do
+        let(:override_params) do
+          {
+            import_data: {
+              data: {
+                override_params: {
+                  visibility: override_visibility
+                }
+              }
+            }
+          }
+        end
+
+        context 'when restricted' do
+          let(:override_visibility) { 'public' }
+
+          it 'returns true and visibility name' do
+            result = subject.level_restricted?
+
+            expect(result.restricted?).to eq(true)
+            expect(result.visibility_level).to eq(Gitlab::VisibilityLevel::PUBLIC)
+          end
+        end
+
+        context 'when misspelled' do
+          let(:override_visibility) { 'publik' }
+
+          it 'returns false with nil for visibility level' do
+            result = subject.level_restricted?
+
+            expect(result.restricted?).to eq(false)
+            expect(result.visibility_level).to be_nil
+          end
+        end
+
+        context 'when import_data is missing' do
+          let(:override_params) { {} }
+
+          it 'returns false with nil for visibility level' do
+            result = subject.level_restricted?
+
+            expect(result.restricted?).to eq(false)
+            expect(result.visibility_level).to be_nil
+          end
+        end
+      end
+    end
+  end
+end
diff --git a/spec/services/projects/create_service_spec.rb b/spec/services/projects/create_service_spec.rb
index d4fa62fa85d..8178b7d2ba2 100644
--- a/spec/services/projects/create_service_spec.rb
+++ b/spec/services/projects/create_service_spec.rb
@@ -183,27 +183,65 @@ describe Projects::CreateService, '#execute' do
   context 'restricted visibility level' do
     before do
       stub_application_setting(restricted_visibility_levels: [Gitlab::VisibilityLevel::PUBLIC])
+    end
 
-      opts.merge!(
-        visibility_level: Gitlab::VisibilityLevel::PUBLIC
-      )
+    shared_examples 'restricted visibility' do
+      it 'does not allow a restricted visibility level for non-admins' do
+        project = create_project(user, opts)
+
+        expect(project).to respond_to(:errors)
+        expect(project.errors.messages).to have_key(:visibility_level)
+        expect(project.errors.messages[:visibility_level].first).to(
+          match('restricted by your GitLab administrator')
+        )
+      end
+
+      it 'allows a restricted visibility level for admins' do
+        admin = create(:admin)
+        project = create_project(admin, opts)
+
+        expect(project.errors.any?).to be(false)
+        expect(project.saved?).to be(true)
+      end
     end
 
-    it 'does not allow a restricted visibility level for non-admins' do
-      project = create_project(user, opts)
-      expect(project).to respond_to(:errors)
-      expect(project.errors.messages).to have_key(:visibility_level)
-      expect(project.errors.messages[:visibility_level].first).to(
-        match('restricted by your GitLab administrator')
-      )
+    context 'when visibility is project based' do
+      before do
+        opts.merge!(
+          visibility_level: Gitlab::VisibilityLevel::PUBLIC
+        )
+      end
+
+      include_examples 'restricted visibility'
     end
 
-    it 'allows a restricted visibility level for admins' do
-      admin = create(:admin)
-      project = create_project(admin, opts)
+    context 'when visibility is overridden' do
+      let(:visibility) { 'public' }
 
-      expect(project.errors.any?).to be(false)
-      expect(project.saved?).to be(true)
+      before do
+        opts.merge!(
+          import_data: {
+            data: {
+              override_params: {
+                visibility: visibility
+              }
+            }
+          }
+        )
+      end
+
+      include_examples 'restricted visibility'
+
+      context 'when visibility is misspelled' do
+        let(:visibility) { 'publik' }
+
+        it 'does not restrict project creation' do
+          project = create_project(user, opts)
+
+          expect(project.errors.any?).to be(false)
+          expect(project.saved?).to be(true)
+        end
+      end
     end
   end