Merge branch 'master' of dev.gitlab.org:gitlab/gitlabhq

6 files changed, 598 insertions, 139 deletions

diff --git a/spec/controllers/groups/runners_controller_spec.rb b/spec/controllers/groups/runners_controller_spec.rb
index 91f9e2c7832..14b0cf959b3 100644
--- a/spec/controllers/groups/runners_controller_spec.rb
+++ b/spec/controllers/groups/runners_controller_spec.rb
@@ -3,73 +3,202 @@
 require 'spec_helper'
 
 describe Groups::RunnersController do
-  let(:user) { create(:user) }
-  let(:group) { create(:group) }
+  let(:user)   { create(:user) }
+  let(:group)  { create(:group) }
   let(:runner) { create(:ci_runner, :group, groups: [group]) }
-
-  let(:params) do
-    {
-      group_id: group,
-      id: runner
-    }
-  end
+  let(:params) { { group_id: group, id: runner } }
 
   before do
     sign_in(user)
-    group.add_maintainer(user)
+  end
+
+  describe '#show' do
+    context 'when user is owner' do
+      before do
+        group.add_owner(user)
+      end
+
+      it 'renders show with 200 status code' do
+        get :show, params: { group_id: group, id: runner }
+
+        expect(response).to have_gitlab_http_status(200)
+        expect(response).to render_template(:show)
+      end
+    end
+
+    context 'when user is not owner' do
+      before do
+        group.add_maintainer(user)
+      end
+
+      it 'renders a 404' do
+        get :show, params: { group_id: group, id: runner }
+
+        expect(response).to have_gitlab_http_status(404)
+      end
+    end
+  end
+
+  describe '#edit' do
+    context 'when user is owner' do
+      before do
+        group.add_owner(user)
+      end
+
+      it 'renders show with 200 status code' do
+        get :edit, params: { group_id: group, id: runner }
+
+        expect(response).to have_gitlab_http_status(200)
+        expect(response).to render_template(:edit)
+      end
+    end
+
+    context 'when user is not owner' do
+      before do
+        group.add_maintainer(user)
+      end
+
+      it 'renders a 404' do
+        get :edit, params: { group_id: group, id: runner }
+
+        expect(response).to have_gitlab_http_status(404)
+      end
+    end
   end
 
   describe '#update' do
-    it 'updates the runner and ticks the queue' do
-      new_desc = runner.description.swapcase
+    context 'when user is an owner' do
+      before do
+        group.add_owner(user)
+      end
 
-      expect do
-        post :update, params: params.merge(runner: { description: new_desc } )
-      end.to change { runner.ensure_runner_queue_value }
+      it 'updates the runner, ticks the queue, and redirects' do
+        new_desc = runner.description.swapcase
 
-      runner.reload
+        expect do
+          post :update, params: params.merge(runner: { description: new_desc } )
+        end.to change { runner.ensure_runner_queue_value }
 
-      expect(response).to have_gitlab_http_status(302)
-      expect(runner.description).to eq(new_desc)
+        expect(response).to have_gitlab_http_status(302)
+        expect(runner.reload.description).to eq(new_desc)
+      end
+    end
+
+    context 'when user is not an owner' do
+      before do
+        group.add_maintainer(user)
+      end
+
+      it 'rejects the update and responds 404' do
+        old_desc = runner.description
+
+        expect do
+          post :update, params: params.merge(runner: { description: old_desc.swapcase } )
+        end.not_to change { runner.ensure_runner_queue_value }
+
+        expect(response).to have_gitlab_http_status(404)
+        expect(runner.reload.description).to eq(old_desc)
+      end
     end
   end
 
   describe '#destroy' do
-    it 'destroys the runner' do
-      delete :destroy, params: params
+    context 'when user is an owner' do
+      before do
+        group.add_owner(user)
+      end
+
+      it 'destroys the runner and redirects' do
+        delete :destroy, params: params
+
+        expect(response).to have_gitlab_http_status(302)
+        expect(Ci::Runner.find_by(id: runner.id)).to be_nil
+      end
+    end
+
+    context 'when user is not an owner' do
+      before do
+        group.add_maintainer(user)
+      end
+
+      it 'responds 404 and does not destroy the runner' do
+        delete :destroy, params: params
 
-      expect(response).to have_gitlab_http_status(302)
-      expect(Ci::Runner.find_by(id: runner.id)).to be_nil
+        expect(response).to have_gitlab_http_status(404)
+        expect(Ci::Runner.find_by(id: runner.id)).to be_present
+      end
     end
   end
 
   describe '#resume' do
-    it 'marks the runner as active and ticks the queue' do
-      runner.update(active: false)
+    context 'when user is an owner' do
+      before do
+        group.add_owner(user)
+      end
 
-      expect do
-        post :resume, params: params
-      end.to change { runner.ensure_runner_queue_value }
+      it 'marks the runner as active, ticks the queue, and redirects' do
+        runner.update(active: false)
 
-      runner.reload
+        expect do
+          post :resume, params: params
+        end.to change { runner.ensure_runner_queue_value }
 
-      expect(response).to have_gitlab_http_status(302)
-      expect(runner.active).to eq(true)
+        expect(response).to have_gitlab_http_status(302)
+        expect(runner.reload.active).to eq(true)
+      end
+    end
+
+    context 'when user is not an owner' do
+      before do
+        group.add_maintainer(user)
+      end
+
+      it 'responds 404 and does not activate the runner' do
+        runner.update(active: false)
+
+        expect do
+          post :resume, params: params
+        end.not_to change { runner.ensure_runner_queue_value }
+
+        expect(response).to have_gitlab_http_status(404)
+        expect(runner.reload.active).to eq(false)
+      end
     end
   end
 
   describe '#pause' do
-    it 'marks the runner as inactive and ticks the queue' do
-      runner.update(active: true)
+    context 'when user is an owner' do
+      before do
+        group.add_owner(user)
+      end
+
+      it 'marks the runner as inactive, ticks the queue, and redirects' do
+        runner.update(active: true)
+
+        expect do
+          post :pause, params: params
+        end.to change { runner.ensure_runner_queue_value }
+
+        expect(response).to have_gitlab_http_status(302)
+        expect(runner.reload.active).to eq(false)
+      end
+    end
+
+    context 'when user is not an owner' do
+      before do
+        group.add_maintainer(user)
+      end
 
-      expect do
-        post :pause, params: params
-      end.to change { runner.ensure_runner_queue_value }
+      it 'responds 404 and does not update the runner or queue' do
+        runner.update(active: true)
 
-      runner.reload
+        expect do
+          post :pause, params: params
+        end.not_to change { runner.ensure_runner_queue_value }
 
-      expect(response).to have_gitlab_http_status(302)
-      expect(runner.active).to eq(false)
+        expect(response).to have_gitlab_http_status(404)
+        expect(runner.reload.active).to eq(true)
+      end
     end
   end
 end
diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb
index 80f54dd258d..d0370dfaeee 100644
--- a/spec/controllers/projects/merge_requests_controller_spec.rb
+++ b/spec/controllers/projects/merge_requests_controller_spec.rb
@@ -737,19 +737,63 @@ describe Projects::MergeRequestsController do
   end
 
   describe 'GET test_reports' do
+    let(:merge_request) do
+      create(:merge_request,
+        :with_diffs,
+        :with_merge_request_pipeline,
+        target_project: project,
+        source_project: project
+      )
+    end
+
     subject do
-      get :test_reports,
-          params: {
-            namespace_id: project.namespace.to_param,
-            project_id: project,
-            id: merge_request.iid
-          },
-          format: :json
+      get :test_reports, params: {
+        namespace_id: project.namespace.to_param,
+        project_id: project,
+        id: merge_request.iid
+      },
+      format: :json
     end
 
     before do
       allow_any_instance_of(MergeRequest)
-        .to receive(:compare_test_reports).and_return(comparison_status)
+        .to receive(:compare_test_reports)
+        .and_return(comparison_status)
+
+      allow_any_instance_of(MergeRequest)
+        .to receive(:actual_head_pipeline)
+        .and_return(merge_request.all_pipelines.take)
+    end
+
+    describe 'permissions on a public project with private CI/CD' do
+      let(:project) { create :project, :repository, :public, :builds_private }
+      let(:comparison_status) { { status: :parsed, data: { summary: 1 } } }
+
+      context 'while signed out' do
+        before do
+          sign_out(user)
+        end
+
+        it 'responds with a 404' do
+          subject
+
+          expect(response).to have_gitlab_http_status(404)
+          expect(response.body).to be_blank
+        end
+      end
+
+      context 'while signed in as an unrelated user' do
+        before do
+          sign_in(create(:user))
+        end
+
+        it 'responds with a 404' do
+          subject
+
+          expect(response).to have_gitlab_http_status(404)
+          expect(response.body).to be_blank
+        end
+      end
     end
 
     context 'when comparison is being processed' do
@@ -1070,17 +1114,39 @@ describe Projects::MergeRequestsController do
 
       let(:status) { pipeline.detailed_status(double('user')) }
 
-      before do
+      it 'returns a detailed head_pipeline status in json' do
         get_pipeline_status
-      end
 
-      it 'return a detailed head_pipeline status in json' do
         expect(response).to have_gitlab_http_status(:ok)
         expect(json_response['text']).to eq status.text
         expect(json_response['label']).to eq status.label
         expect(json_response['icon']).to eq status.icon
         expect(json_response['favicon']).to match_asset_path "/assets/ci_favicons/#{status.favicon}.png"
       end
+
+      context 'with project member visibility on a public project' do
+        let(:user)    { create(:user) }
+        let(:project) { create(:project, :repository, :public, :builds_private) }
+
+        it 'returns pipeline data to project members' do
+          project.add_developer(user)
+
+          get_pipeline_status
+
+          expect(response).to have_gitlab_http_status(:ok)
+          expect(json_response['text']).to eq status.text
+          expect(json_response['label']).to eq status.label
+          expect(json_response['icon']).to eq status.icon
+          expect(json_response['favicon']).to match_asset_path "/assets/ci_favicons/#{status.favicon}.png"
+        end
+
+        it 'returns blank OK response to non-project-members' do
+          get_pipeline_status
+
+          expect(response).to have_gitlab_http_status(:ok)
+          expect(json_response).to be_empty
+        end
+      end
     end
 
     context 'when head_pipeline does not exist' do
@@ -1088,7 +1154,7 @@ describe Projects::MergeRequestsController do
         get_pipeline_status
       end
 
-      it 'return empty' do
+      it 'returns blank OK response' do
         expect(response).to have_gitlab_http_status(:ok)
         expect(json_response).to be_empty
       end
diff --git a/spec/controllers/projects/notes_controller_spec.rb b/spec/controllers/projects/notes_controller_spec.rb
index 4500c412521..4db77921f24 100644
--- a/spec/controllers/projects/notes_controller_spec.rb
+++ b/spec/controllers/projects/notes_controller_spec.rb
@@ -212,40 +212,232 @@ describe Projects::NotesController do
   describe 'POST create' do
     let(:merge_request) { create(:merge_request) }
     let(:project) { merge_request.source_project }
+    let(:note_text) { 'some note' }
     let(:request_params) do
       {
-        note: { note: 'some note', noteable_id: merge_request.id, noteable_type: 'MergeRequest' },
+        note: { note: note_text, noteable_id: merge_request.id, noteable_type: 'MergeRequest' },
         namespace_id: project.namespace,
         project_id: project,
         merge_request_diff_head_sha: 'sha',
         target_type: 'merge_request',
         target_id: merge_request.id
-      }
+      }.merge(extra_request_params)
+    end
+    let(:extra_request_params) { {} }
+
+    let(:project_visibility) { Gitlab::VisibilityLevel::PUBLIC }
+    let(:merge_requests_access_level) { ProjectFeature::ENABLED }
+
+    def create!
+      post :create, params: request_params
     end
 
     before do
+      project.update_attribute(:visibility_level, project_visibility)
+      project.project_feature.update(merge_requests_access_level: merge_requests_access_level)
       sign_in(user)
-      project.add_developer(user)
     end
 
-    it "returns status 302 for html" do
-      post :create, params: request_params
+    describe 'making the creation request' do
+      before do
+        create!
+      end
+
+      context 'the project is publically available' do
+        context 'for HTML' do
+          it "returns status 302" do
+            expect(response).to have_gitlab_http_status(302)
+          end
+        end
+
+        context 'for JSON' do
+          let(:extra_request_params) { { format: :json } }
+
+          it "returns status 200 for json" do
+            expect(response).to have_gitlab_http_status(200)
+          end
+        end
+      end
 
-      expect(response).to have_gitlab_http_status(302)
+      context 'the project is a private project' do
+        let(:project_visibility) { Gitlab::VisibilityLevel::PRIVATE }
+
+        [{}, { format: :json }].each do |extra|
+          context "format is #{extra[:format]}" do
+            let(:extra_request_params) { extra }
+
+            it "returns status 404" do
+              expect(response).to have_gitlab_http_status(404)
+            end
+          end
+        end
+      end
     end
 
-    it "returns status 200 for json" do
-      post :create, params: request_params.merge(format: :json)
+    context 'the user is a developer on a private project' do
+      let(:project_visibility) { Gitlab::VisibilityLevel::PRIVATE }
 
-      expect(response).to have_gitlab_http_status(200)
+      before do
+        project.add_developer(user)
+      end
+
+      context 'HTML requests' do
+        it "returns status 302 (redirect)" do
+          create!
+
+          expect(response).to have_gitlab_http_status(302)
+        end
+      end
+
+      context 'JSON requests' do
+        let(:extra_request_params) { { format: :json } }
+
+        it "returns status 200" do
+          create!
+
+          expect(response).to have_gitlab_http_status(200)
+        end
+      end
+
+      context 'the return_discussion param is set' do
+        let(:extra_request_params) { { format: :json, return_discussion: 'true' } }
+
+        it 'returns discussion JSON when the return_discussion param is set' do
+          create!
+
+          expect(response).to have_gitlab_http_status(200)
+          expect(json_response).to have_key 'discussion'
+          expect(json_response.dig('discussion', 'notes', 0, 'note')).to eq(request_params[:note][:note])
+        end
+      end
+
+      context 'when creating a note with quick actions' do
+        context 'with commands that return changes' do
+          let(:note_text) { "/award :thumbsup:\n/estimate 1d\n/spend 3h" }
+          let(:extra_request_params) { { format: :json } }
+
+          it 'includes changes in commands_changes ' do
+            create!
+
+            expect(response).to have_gitlab_http_status(200)
+            expect(json_response['commands_changes']).to include('emoji_award', 'time_estimate', 'spend_time')
+            expect(json_response['commands_changes']).not_to include('target_project', 'title')
+          end
+        end
+
+        context 'with commands that do not return changes' do
+          let(:issue) { create(:issue, project: project) }
+          let(:other_project) { create(:project) }
+          let(:note_text) { "/move #{other_project.full_path}\n/title AAA" }
+          let(:extra_request_params) { { format: :json, target_id: issue.id, target_type: 'issue' } }
+
+          before do
+            other_project.add_developer(user)
+          end
+
+          it 'does not include changes in commands_changes' do
+            create!
+
+            expect(response).to have_gitlab_http_status(200)
+            expect(json_response['commands_changes']).not_to include('target_project', 'title')
+          end
+        end
+      end
     end
 
-    it 'returns discussion JSON when the return_discussion param is set' do
-      post :create, params: request_params.merge(format: :json, return_discussion: 'true')
+    context 'when the internal project prohibits non-members from accessing merge requests' do
+      let(:project_visibility) { Gitlab::VisibilityLevel::INTERNAL }
+      let(:merge_requests_access_level) { ProjectFeature::PRIVATE }
 
-      expect(response).to have_gitlab_http_status(200)
-      expect(json_response).to have_key 'discussion'
-      expect(json_response['discussion']['notes'][0]['note']).to eq(request_params[:note][:note])
+      it "prevents a non-member user from creating a note on one of the project's merge requests" do
+        create!
+
+        expect(response).to have_gitlab_http_status(404)
+      end
+
+      context 'when the user is a team member' do
+        before do
+          project.add_developer(user)
+        end
+
+        it 'can add comments' do
+          expect { create! }.to change { project.notes.count }.by(1)
+        end
+      end
+
+      # Illustration of the attack vector for posting comments to discussions that should
+      # be inaccessible.
+      #
+      # This relies on posting a note to a commit that is not necessarily even in the
+      # merge request, with a value of :in_reply_to_discussion_id that points to a
+      # discussion on a merge_request that should not be accessible.
+      context 'when the request includes a :in_reply_to_discussion_id designed to fool us' do
+        let(:commit) { create(:commit, project: project) }
+
+        let(:existing_comment) do
+          create(:note_on_commit,
+                 note: 'first',
+                 project: project,
+                 commit_id: merge_request.commit_shas.first)
+        end
+
+        let(:discussion) { existing_comment.discussion }
+
+        # see !60465 for details of the structure of this request
+        let(:request_params) do
+          { "utf8" => "✓",
+            "authenticity_token" => "1",
+            "view" => "inline",
+            "line_type" => "",
+            "merge_request_diff_head_sha" => "",
+            "in_reply_to_discussion_id" => discussion.id,
+            "note_project_id" => project.id,
+            "project_id" => project.id,
+            "namespace_id" => project.namespace,
+            "target_type" => "commit",
+            "target_id" => commit.id,
+            "note" => {
+              "noteable_type" => "",
+              "noteable_id" => "",
+              "commit_id" => "",
+              "type" => "",
+              "line_code" => "",
+              "position" => "",
+              "note" => "ThisReplyWillGoToMergeRequest"
+            } }
+        end
+
+        it 'prevents the request from adding notes to the spoofed discussion' do
+          expect { create! }.not_to change { discussion.notes.count }
+        end
+
+        it 'returns an error to the user' do
+          create!
+          expect(response).to have_gitlab_http_status(404)
+        end
+      end
+    end
+
+    context 'when the public project prohibits non-members from accessing merge requests' do
+      let(:project_visibility) { Gitlab::VisibilityLevel::PUBLIC }
+      let(:merge_requests_access_level) { ProjectFeature::PRIVATE }
+
+      it "prevents a non-member user from creating a note on one of the project's merge requests" do
+        create!
+
+        expect(response).to have_gitlab_http_status(404)
+      end
+
+      context 'when the user is a team member' do
+        before do
+          project.add_developer(user)
+          create!
+        end
+
+        it 'can add comments' do
+          expect(response).to be_redirect
+        end
+      end
     end
 
     context 'when merge_request_diff_head_sha present' do
@@ -262,7 +454,7 @@ describe Projects::NotesController do
       end
 
       it "returns status 302 for html" do
-        post :create, params: request_params
+        create!
 
         expect(response).to have_gitlab_http_status(302)
       end
@@ -285,7 +477,7 @@ describe Projects::NotesController do
     end
 
     context 'when creating a commit comment from an MR fork' do
-      let(:project) { create(:project, :repository) }
+      let(:project) { create(:project, :repository, :public) }
 
       let(:forked_project) do
         fork_project(project, nil, repository: true)
@@ -299,45 +491,59 @@ describe Projects::NotesController do
         create(:note_on_commit, note: 'a note', project: forked_project, commit_id: merge_request.commit_shas.first)
       end
 
-      def post_create(extra_params = {})
-        post :create, params: {
+      let(:note_project_id) do
+        forked_project.id
+      end
+
+      let(:request_params) do
+        {
           note: { note: 'some other note', noteable_id: merge_request.id },
           namespace_id: project.namespace,
           project_id: project,
           target_type: 'merge_request',
           target_id: merge_request.id,
-          note_project_id: forked_project.id,
+          note_project_id: note_project_id,
           in_reply_to_discussion_id: existing_comment.discussion_id
-        }.merge(extra_params)
+        }
+      end
+
+      let(:fork_visibility) { Gitlab::VisibilityLevel::PUBLIC }
+
+      before do
+        forked_project.update_attribute(:visibility_level, fork_visibility)
       end
 
       context 'when the note_project_id is not correct' do
-        it 'returns a 404' do
-          post_create(note_project_id: Project.maximum(:id).succ)
+        let(:note_project_id) do
+          project.id && Project.maximum(:id).succ
+        end
 
+        it 'returns a 404' do
+          create!
           expect(response).to have_gitlab_http_status(404)
         end
       end
 
       context 'when the user has no access to the fork' do
-        it 'returns a 404' do
-          post_create
+        let(:fork_visibility) { Gitlab::VisibilityLevel::PRIVATE }
 
+        it 'returns a 404' do
+          create!
           expect(response).to have_gitlab_http_status(404)
         end
       end
 
       context 'when the user has access to the fork' do
-        let(:discussion) { forked_project.notes.find_discussion(existing_comment.discussion_id) }
+        let!(:discussion) { forked_project.notes.find_discussion(existing_comment.discussion_id) }
+        let(:fork_visibility) { Gitlab::VisibilityLevel::PUBLIC }
 
-        before do
-          forked_project.add_developer(user)
-
-          existing_comment
+        it 'is successful' do
+          create!
+          expect(response).to have_gitlab_http_status(302)
         end
 
         it 'creates the note' do
-          expect { post_create }.to change { forked_project.notes.count }.by(1)
+          expect { create! }.to change { forked_project.notes.count }.by(1)
         end
       end
     end
@@ -346,11 +552,6 @@ describe Projects::NotesController do
       let(:locked_issue) { create(:issue, :locked, project: project) }
       let(:issue) {create(:issue, project: project)}
 
-      before do
-        project.update_attribute(:visibility_level, Gitlab::VisibilityLevel::PUBLIC)
-        project.project_member(user).destroy
-      end
-
       it 'uses target_id and ignores noteable_id' do
         request_params = {
           note: { note: 'some note', noteable_type: 'Issue', noteable_id: locked_issue.id },
@@ -368,7 +569,6 @@ describe Projects::NotesController do
 
     context 'when the merge request discussion is locked' do
       before do
-        project.update_attribute(:visibility_level, Gitlab::VisibilityLevel::PUBLIC)
         merge_request.update_attribute(:discussion_locked, true)
       end
 
@@ -382,6 +582,10 @@ describe Projects::NotesController do
       end
 
       context 'when a user is a team member' do
+        before do
+          project.add_developer(user)
+        end
+
         it 'returns 302 status for html' do
           post :create, params: request_params
 
@@ -400,10 +604,6 @@ describe Projects::NotesController do
       end
 
       context 'when a user is not a team member' do
-        before do
-          project.project_member(user).destroy
-        end
-
         it 'returns 404 status' do
           post :create, params: request_params
 
@@ -415,37 +615,6 @@ describe Projects::NotesController do
         end
       end
     end
-
-    context 'when creating a note with quick actions' do
-      context 'with commands that return changes' do
-        let(:note_text) { "/award :thumbsup:\n/estimate 1d\n/spend 3h" }
-
-        it 'includes changes in commands_changes ' do
-          post :create, params: request_params.merge(note: { note: note_text }, format: :json)
-
-          expect(response).to have_gitlab_http_status(200)
-          expect(json_response['commands_changes']).to include('emoji_award', 'time_estimate', 'spend_time')
-          expect(json_response['commands_changes']).not_to include('target_project', 'title')
-        end
-      end
-
-      context 'with commands that do not return changes' do
-        let(:issue) { create(:issue, project: project) }
-        let(:other_project) { create(:project) }
-        let(:note_text) { "/move #{other_project.full_path}\n/title AAA" }
-
-        before do
-          other_project.add_developer(user)
-        end
-
-        it 'does not include changes in commands_changes' do
-          post :create, params: request_params.merge(note: { note: note_text }, target_type: 'issue', target_id: issue.id, format: :json)
-
-          expect(response).to have_gitlab_http_status(200)
-          expect(json_response['commands_changes']).not_to include('target_project', 'title')
-        end
-      end
-    end
   end
 
   describe 'PUT update' do
diff --git a/spec/controllers/projects/services_controller_spec.rb b/spec/controllers/projects/services_controller_spec.rb
index 563b61962cf..180d997a8e8 100644
--- a/spec/controllers/projects/services_controller_spec.rb
+++ b/spec/controllers/projects/services_controller_spec.rb
@@ -11,6 +11,7 @@ describe Projects::ServicesController do
   before do
     sign_in(user)
     project.add_maintainer(user)
+    allow(Gitlab::UrlBlocker).to receive(:validate!).and_return([URI.parse('http://example.com'), nil])
   end
 
   describe '#test' do
@@ -56,6 +57,8 @@ describe Projects::ServicesController do
           stub_request(:get, 'http://example.com/rest/api/2/serverInfo')
             .to_return(status: 200, body: '{}')
 
+          expect(Gitlab::HTTP).to receive(:get).with("/rest/api/2/serverInfo", any_args).and_call_original
+
           put :test, params: { namespace_id: project.namespace, project_id: project, id: service.to_param, service: service_params }
 
           expect(response.status).to eq(200)
@@ -66,6 +69,8 @@ describe Projects::ServicesController do
         stub_request(:get, 'http://example.com/rest/api/2/serverInfo')
           .to_return(status: 200, body: '{}')
 
+        expect(Gitlab::HTTP).to receive(:get).with("/rest/api/2/serverInfo", any_args).and_call_original
+
         put :test, params: { namespace_id: project.namespace, project_id: project, id: service.to_param, service: service_params }
 
         expect(response.status).to eq(200)
diff --git a/spec/controllers/sessions_controller_spec.rb b/spec/controllers/sessions_controller_spec.rb
index 9c4ddce5409..68b7bf61231 100644
--- a/spec/controllers/sessions_controller_spec.rb
+++ b/spec/controllers/sessions_controller_spec.rb
@@ -100,16 +100,8 @@ describe SessionsController do
         end
       end
 
-      context 'when reCAPTCHA is enabled' do
-        let(:user) { create(:user) }
-        let(:user_params) { { login: user.username, password: user.password } }
-
-        before do
-          stub_application_setting(recaptcha_enabled: true)
-          request.headers[described_class::CAPTCHA_HEADER] = 1
-        end
-
-        it 'displays an error when the reCAPTCHA is not solved' do
+      context 'with reCAPTCHA' do
+        def unsuccesful_login(user_params, sesion_params: {})
           # Without this, `verify_recaptcha` arbitrarily returns true in test env
           Recaptcha.configuration.skip_verify_env.delete('test')
           counter = double(:counter)
@@ -119,14 +111,10 @@ describe SessionsController do
                                       .with(:failed_login_captcha_total, anything)
                                       .and_return(counter)
 
-          post(:create, params: { user: user_params })
-
-          expect(response).to render_template(:new)
-          expect(flash[:alert]).to include 'There was an error with the reCAPTCHA. Please solve the reCAPTCHA again.'
-          expect(subject.current_user).to be_nil
+          post(:create, params: { user: user_params }, session: sesion_params)
         end
 
-        it 'successfully logs in a user when reCAPTCHA is solved' do
+        def succesful_login(user_params, sesion_params: {})
           # Avoid test ordering issue and ensure `verify_recaptcha` returns true
           Recaptcha.configuration.skip_verify_env << 'test'
           counter = double(:counter)
@@ -137,9 +125,80 @@ describe SessionsController do
                                       .and_return(counter)
           expect(Gitlab::Metrics).to receive(:counter).and_call_original
 
-          post(:create, params: { user: user_params })
+          post(:create, params: { user: user_params }, session: sesion_params)
+        end
 
-          expect(subject.current_user).to eq user
+        context 'when reCAPTCHA is enabled' do
+          let(:user) { create(:user) }
+          let(:user_params) { { login: user.username, password: user.password } }
+
+          before do
+            stub_application_setting(recaptcha_enabled: true)
+            request.headers[described_class::CAPTCHA_HEADER] = 1
+          end
+
+          it 'displays an error when the reCAPTCHA is not solved' do
+            # Without this, `verify_recaptcha` arbitrarily returns true in test env
+
+            unsuccesful_login(user_params)
+
+            expect(response).to render_template(:new)
+            expect(flash[:alert]).to include 'There was an error with the reCAPTCHA. Please solve the reCAPTCHA again.'
+            expect(subject.current_user).to be_nil
+          end
+
+          it 'successfully logs in a user when reCAPTCHA is solved' do
+            succesful_login(user_params)
+
+            expect(subject.current_user).to eq user
+          end
+        end
+
+        context 'when reCAPTCHA login protection is enabled' do
+          let(:user) { create(:user) }
+          let(:user_params) { { login: user.username, password: user.password } }
+
+          before do
+            stub_application_setting(login_recaptcha_protection_enabled: true)
+          end
+
+          context 'when user tried to login 5 times' do
+            it 'displays an error when the reCAPTCHA is not solved' do
+              unsuccesful_login(user_params, sesion_params: { failed_login_attempts: 6 })
+
+              expect(response).to render_template(:new)
+              expect(flash[:alert]).to include 'There was an error with the reCAPTCHA. Please solve the reCAPTCHA again.'
+              expect(subject.current_user).to be_nil
+            end
+
+            it 'successfully logs in a user when reCAPTCHA is solved' do
+              succesful_login(user_params, sesion_params: { failed_login_attempts: 6 })
+
+              expect(subject.current_user).to eq user
+            end
+          end
+
+          context 'when there are more than 5 anonymous session with the same IP' do
+            before do
+              allow(Gitlab::AnonymousSession).to receive_message_chain(:new, :stored_sessions).and_return(6)
+            end
+
+            it 'displays an error when the reCAPTCHA is not solved' do
+              unsuccesful_login(user_params)
+
+              expect(response).to render_template(:new)
+              expect(flash[:alert]).to include 'There was an error with the reCAPTCHA. Please solve the reCAPTCHA again.'
+              expect(subject.current_user).to be_nil
+            end
+
+            it 'successfully logs in a user when reCAPTCHA is solved' do
+              expect(Gitlab::AnonymousSession).to receive_message_chain(:new, :cleanup_session_per_ip_entries)
+
+              succesful_login(user_params)
+
+              expect(subject.current_user).to eq user
+            end
+          end
         end
       end
     end
@@ -348,4 +407,17 @@ describe SessionsController do
       expect(controller.stored_location_for(:redirect)).to eq(search_path)
     end
   end
+
+  context 'when login fails' do
+    before do
+      set_devise_mapping(context: @request)
+      @request.env["warden.options"] = { action:  'unauthenticated' }
+    end
+
+    it 'does increment failed login counts for session' do
+      get(:new, params: { user: { login: 'failed' } })
+
+      expect(session[:failed_login_attempts]).to eq(1)
+    end
+  end
 end
diff --git a/spec/controllers/uploads_controller_spec.rb b/spec/controllers/uploads_controller_spec.rb
index 0876502a899..5f4a6bf8ee7 100644
--- a/spec/controllers/uploads_controller_spec.rb
+++ b/spec/controllers/uploads_controller_spec.rb
@@ -21,8 +21,20 @@ shared_examples 'content publicly cached' do
 end
 
 describe UploadsController do
+  include WorkhorseHelpers
+
   let!(:user) { create(:user, avatar: fixture_file_upload("spec/fixtures/dk.png", "image/png")) }
 
+  describe 'POST #authorize' do
+    it_behaves_like 'handle uploads authorize' do
+      let(:uploader_class) { PersonalFileUploader }
+      let(:model) { create(:personal_snippet, :public) }
+      let(:params) do
+        { model: 'personal_snippet', id: model.id }
+      end
+    end
+  end
+
   describe 'POST create' do
     let(:jpg)     { fixture_file_upload('spec/fixtures/rails_sample.jpg', 'image/jpg') }
     let(:txt)     { fixture_file_upload('spec/fixtures/doc_sample.txt', 'text/plain') }
@@ -636,4 +648,10 @@ describe UploadsController do
       end
     end
   end
+
+  def post_authorize(verified: true)
+    request.headers.merge!(workhorse_internal_api_request_header) if verified
+
+    post :authorize, params: { model: 'personal_snippet', id: model.id }, format: :json
+  end
 end