Merge commit 'dev/security' into 'master'

Signed-off-by: Rémy Coutable <remy@rymai.me>

10 files changed, 114 insertions, 13 deletions

diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb
index 54a2d3d9460..19a8b1fe524 100644
--- a/spec/controllers/users_controller_spec.rb
+++ b/spec/controllers/users_controller_spec.rb
@@ -73,8 +73,8 @@ describe UsersController do
     end
 
     context 'forked project' do
-      let!(:project) { create(:project) }
-      let!(:forked_project) { Projects::ForkService.new(project, user).execute }
+      let(:project) { create(:project) }
+      let(:forked_project) { Projects::ForkService.new(project, user).execute }
 
       before do
         sign_in(user)
diff --git a/spec/features/projects/import_export/export_file_spec.rb b/spec/features/projects/import_export/export_file_spec.rb
index 27c986c5187..52d08982c7a 100644
--- a/spec/features/projects/import_export/export_file_spec.rb
+++ b/spec/features/projects/import_export/export_file_spec.rb
@@ -47,6 +47,8 @@ feature 'Import/Export - project export integration test', feature: true, js: tr
 
       expect(page).to have_content('Download export')
 
+      expect(file_permissions(project.export_path)).to eq(0700)
+
       in_directory_with_expanded_export(project) do |exit_status, tmpdir|
         expect(exit_status).to eq(0)
 
diff --git a/spec/helpers/projects_helper_spec.rb b/spec/helpers/projects_helper_spec.rb
index 70032e7df94..bcd53440cb4 100644
--- a/spec/helpers/projects_helper_spec.rb
+++ b/spec/helpers/projects_helper_spec.rb
@@ -11,7 +11,7 @@ describe ProjectsHelper do
 
   describe "can_change_visibility_level?" do
     let(:project) { create(:project) }
-    let(:user) { create(:user) }
+    let(:user) { create(:project_member, :reporter, user: create(:user), project: project).user }
     let(:fork_project) { Projects::ForkService.new(project, user).execute }
 
     it "returns false if there are no appropriate permissions" do
diff --git a/spec/models/forked_project_link_spec.rb b/spec/models/forked_project_link_spec.rb
index 9c81d159cdf..1863581f57b 100644
--- a/spec/models/forked_project_link_spec.rb
+++ b/spec/models/forked_project_link_spec.rb
@@ -6,6 +6,7 @@ describe ForkedProjectLink, "add link on fork" do
   let(:user) { create(:user, namespace: namespace) }
 
   before do
+    create(:project_member, :reporter, user: user, project: project_from)
     @project_to = fork_project(project_from, user)
   end
 
diff --git a/spec/requests/api/api_helpers_spec.rb b/spec/requests/api/api_helpers_spec.rb
index e66faeed705..0f41f8dc7f1 100644
--- a/spec/requests/api/api_helpers_spec.rb
+++ b/spec/requests/api/api_helpers_spec.rb
@@ -10,7 +10,8 @@ describe API::Helpers, api: true do
   let(:key) { create(:key, user: user) }
 
   let(:params) { {} }
-  let(:env) { {} }
+  let(:env) { { 'REQUEST_METHOD' => 'GET' } }
+  let(:request) { Rack::Request.new(env) }
 
   def set_env(token_usr, identifier)
     clear_env
@@ -52,17 +53,43 @@ describe API::Helpers, api: true do
   describe ".current_user" do
     subject { current_user }
 
-    describe "when authenticating via Warden" do
+    describe "Warden authentication" do
       before { doorkeeper_guard_returns false }
 
-      context "fails" do
-        it { is_expected.to be_nil }
+      context "with invalid credentials" do
+        context "GET request" do
+          before { env['REQUEST_METHOD'] = 'GET' }
+          it { is_expected.to be_nil }
+        end
       end
 
-      context "succeeds" do
+      context "with valid credentials" do
         before { warden_authenticate_returns user }
 
-        it { is_expected.to eq(user) }
+        context "GET request" do
+          before { env['REQUEST_METHOD'] = 'GET' }
+          it { is_expected.to eq(user) }
+        end
+
+        context "HEAD request" do
+          before { env['REQUEST_METHOD'] = 'HEAD' }
+          it { is_expected.to eq(user) }
+        end
+
+        context "PUT request" do
+          before { env['REQUEST_METHOD'] = 'PUT' }
+          it { is_expected.to be_nil }
+        end
+
+        context "POST request" do
+          before { env['REQUEST_METHOD'] = 'POST' }
+          it { is_expected.to be_nil }
+        end
+
+        context "DELETE request" do
+          before { env['REQUEST_METHOD'] = 'DELETE' }
+          it { is_expected.to be_nil }
+        end
       end
     end
 
diff --git a/spec/requests/api/fork_spec.rb b/spec/requests/api/fork_spec.rb
index 34f84f78952..e38d5745d44 100644
--- a/spec/requests/api/fork_spec.rb
+++ b/spec/requests/api/fork_spec.rb
@@ -18,7 +18,7 @@ describe API::API, api: true  do
   end
 
   let(:project_user2) do
-    create(:project_member, :guest, user: user2, project: project)
+    create(:project_member, :reporter, user: user2, project: project)
   end
 
   describe 'POST /projects/fork/:id' do
diff --git a/spec/services/projects/fork_service_spec.rb b/spec/services/projects/fork_service_spec.rb
index ef2036c78b1..64d15c0523c 100644
--- a/spec/services/projects/fork_service_spec.rb
+++ b/spec/services/projects/fork_service_spec.rb
@@ -12,12 +12,26 @@ describe Projects::ForkService, services: true do
                              description: 'wow such project')
       @to_namespace = create(:namespace)
       @to_user = create(:user, namespace: @to_namespace)
+      @from_project.add_user(@to_user, :developer)
     end
 
     context 'fork project' do
+      context 'when forker is a guest' do
+        before do
+          @guest = create(:user)
+          @from_project.add_user(@guest, :guest)
+        end
+        subject { fork_project(@from_project, @guest) }
+
+        it { is_expected.not_to be_persisted }
+        it { expect(subject.errors[:forked_from_project_id]).to eq(['is forbidden']) }
+      end
+
       describe "successfully creates project in the user namespace" do
         let(:to_project) { fork_project(@from_project, @to_user) }
 
+        it { expect(to_project).to be_persisted }
+        it { expect(to_project.errors).to be_empty }
         it { expect(to_project.owner).to eq(@to_user) }
         it { expect(to_project.namespace).to eq(@to_user.namespace) }
         it { expect(to_project.star_count).to be_zero }
@@ -29,7 +43,9 @@ describe Projects::ForkService, services: true do
       it "fails due to validation, not transaction failure" do
         @existing_project = create(:project, creator_id: @to_user.id, name: @from_project.name, namespace: @to_namespace)
         @to_project = fork_project(@from_project, @to_user)
-        expect(@existing_project.persisted?).to be_truthy
+        expect(@existing_project).to be_persisted
+
+        expect(@to_project).not_to be_persisted
         expect(@to_project.errors[:name]).to eq(['has already been taken'])
         expect(@to_project.errors[:path]).to eq(['has already been taken'])
       end
@@ -81,18 +97,23 @@ describe Projects::ForkService, services: true do
       @group = create(:group)
       @group.add_user(@group_owner, GroupMember::OWNER)
       @group.add_user(@developer,   GroupMember::DEVELOPER)
+      @project.add_user(@developer,   :developer)
+      @project.add_user(@group_owner, :developer)
       @opts = { namespace: @group }
     end
 
     context 'fork project for group' do
       it 'group owner successfully forks project into the group' do
         to_project = fork_project(@project, @group_owner, @opts)
+
+        expect(to_project).to             be_persisted
+        expect(to_project.errors).to      be_empty
         expect(to_project.owner).to       eq(@group)
         expect(to_project.namespace).to   eq(@group)
         expect(to_project.name).to        eq(@project.name)
         expect(to_project.path).to        eq(@project.path)
         expect(to_project.description).to eq(@project.description)
-        expect(to_project.star_count).to     be_zero
+        expect(to_project.star_count).to  be_zero
       end
     end
 
diff --git a/spec/services/system_note_service_spec.rb b/spec/services/system_note_service_spec.rb
index b16840a1238..c22dd9ab77a 100644
--- a/spec/services/system_note_service_spec.rb
+++ b/spec/services/system_note_service_spec.rb
@@ -451,7 +451,7 @@ describe SystemNoteService, services: true do
     end
 
     context 'commit with cross-reference from fork' do
-      let(:author2) { create(:user) }
+      let(:author2) { create(:project_member, :reporter, user: create(:user), project: project).user }
       let(:forked_project) { Projects::ForkService.new(project, author2).execute }
       let(:commit2) { forked_project.commit }
 
diff --git a/spec/support/import_export/export_file_helper.rb b/spec/support/import_export/export_file_helper.rb
index be0772d6a4a..1b0a4583f5c 100644
--- a/spec/support/import_export/export_file_helper.rb
+++ b/spec/support/import_export/export_file_helper.rb
@@ -130,4 +130,8 @@ module ExportFileHelper
 
     (parsed_model_attributes - parent.keys - excluded_attributes).empty?
   end
+
+  def file_permissions(file)
+    File.stat(file).mode & 0777
+  end
 end
diff --git a/spec/views/ci/lints/show.html.haml_spec.rb b/spec/views/ci/lints/show.html.haml_spec.rb
index 793b747e7eb..2dac5ee23c8 100644
--- a/spec/views/ci/lints/show.html.haml_spec.rb
+++ b/spec/views/ci/lints/show.html.haml_spec.rb
@@ -1,6 +1,52 @@
 require 'spec_helper'
 
 describe 'ci/lints/show' do
+  include Devise::TestHelpers
+
+  describe 'XSS protection' do
+    let(:config_processor) { Ci::GitlabCiYamlProcessor.new(YAML.dump(content)) }
+    before do
+      assign(:status, true)
+      assign(:builds, config_processor.builds)
+      assign(:stages, config_processor.stages)
+      assign(:jobs, config_processor.jobs)
+    end
+
+    context 'when builds attrbiutes contain HTML nodes' do
+      let(:content) do
+        {
+          rspec: {
+            script: '<h1>rspec</h1>',
+            stage: 'test'
+          }
+        }
+      end
+
+      it 'does not render HTML elements' do
+        render
+
+        expect(rendered).not_to have_css('h1', text: 'rspec')
+      end
+    end
+
+    context 'when builds attributes do not contain HTML nodes' do
+      let(:content) do
+        {
+          rspec: {
+            script: 'rspec',
+            stage: 'test'
+          }
+        }
+      end
+
+      it 'shows configuration in the table' do
+        render
+
+        expect(rendered).to have_css('td pre', text: 'rspec')
+      end
+    end
+  end
+
   let(:content) do
     {
       build_template: {