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

28 files changed, 757 insertions, 231 deletions

diff --git a/spec/controllers/admin/impersonation_controller_spec.rb b/spec/controllers/admin/impersonation_controller_spec.rb
deleted file mode 100644
index d7a7ba1c5b6..00000000000
--- a/spec/controllers/admin/impersonation_controller_spec.rb
+++ /dev/null
@@ -1,19 +0,0 @@
-require 'spec_helper'
-
-describe Admin::ImpersonationController do
-  let(:admin) { create(:admin) }
-
-  before do
-    sign_in(admin)
-  end
-
-  describe 'CREATE #impersonation when blocked' do
-    let(:blocked_user) { create(:user, state: :blocked) }
-
-    it 'does not allow impersonation' do
-      post :create, id: blocked_user.username
-
-      expect(flash[:alert]).to eq 'You cannot impersonate a blocked user'
-    end
-  end
-end
diff --git a/spec/controllers/admin/impersonations_controller_spec.rb b/spec/controllers/admin/impersonations_controller_spec.rb
new file mode 100644
index 00000000000..eb82476b179
--- /dev/null
+++ b/spec/controllers/admin/impersonations_controller_spec.rb
@@ -0,0 +1,95 @@
+require 'spec_helper'
+
+describe Admin::ImpersonationsController do
+  let(:impersonator) { create(:admin) }
+  let(:user) { create(:user) }
+
+  describe "DELETE destroy" do
+    context "when not signed in" do
+      it "redirects to the sign in page" do
+        delete :destroy
+
+        expect(response).to redirect_to(new_user_session_path)
+      end
+    end
+
+    context "when signed in" do
+      before do
+        sign_in(user)
+      end
+
+      context "when not impersonating" do
+        it "responds with status 404" do
+          delete :destroy
+
+          expect(response.status).to eq(404)
+        end
+
+        it "doesn't sign us in" do
+          delete :destroy
+
+          expect(warden.user).to eq(user)
+        end
+      end
+
+      context "when impersonating" do
+        before do
+          session[:impersonator_id] = impersonator.id
+        end
+
+        context "when the impersonator is not admin (anymore)" do
+          before do
+            impersonator.admin = false
+            impersonator.save
+          end
+
+          it "responds with status 404" do
+            delete :destroy
+
+            expect(response.status).to eq(404)
+          end
+
+          it "doesn't sign us in as the impersonator" do
+            delete :destroy
+
+            expect(warden.user).to eq(user)
+          end
+        end
+
+        context "when the impersonator is admin" do
+          context "when the impersonator is blocked" do
+            before do
+              impersonator.block!
+            end
+
+            it "responds with status 404" do
+              delete :destroy
+
+              expect(response.status).to eq(404)
+            end
+
+            it "doesn't sign us in as the impersonator" do
+              delete :destroy
+
+              expect(warden.user).to eq(user)
+            end
+          end
+
+          context "when the impersonator is not blocked" do
+            it "redirects to the impersonated user's page" do
+              delete :destroy
+
+              expect(response).to redirect_to(admin_user_path(user))
+            end
+
+            it "signs us in as the impersonator" do
+              delete :destroy
+
+              expect(warden.user).to eq(impersonator)
+            end
+          end
+        end
+      end
+    end
+  end
+end
diff --git a/spec/controllers/admin/users_controller_spec.rb b/spec/controllers/admin/users_controller_spec.rb
index 9ef8ba1b097..ce2a62ae1fd 100644
--- a/spec/controllers/admin/users_controller_spec.rb
+++ b/spec/controllers/admin/users_controller_spec.rb
@@ -2,9 +2,10 @@ require 'spec_helper'
 
 describe Admin::UsersController do
   let(:user) { create(:user) }
+  let(:admin) { create(:admin) }
 
   before do
-    sign_in(create(:admin))
+    sign_in(admin)
   end
 
   describe 'DELETE #user with projects' do
@@ -112,4 +113,50 @@ describe Admin::UsersController do
       patch :disable_two_factor, id: user.to_param
     end
   end
+
+  describe "POST impersonate" do
+    context "when the user is blocked" do
+      before do
+        user.block!
+      end
+
+      it "shows a notice" do
+        post :impersonate, id: user.username
+
+        expect(flash[:alert]).to eq("You cannot impersonate a blocked user")
+      end
+
+      it "doesn't sign us in as the user" do
+        post :impersonate, id: user.username
+
+        expect(warden.user).to eq(admin)
+      end
+    end
+
+    context "when the user is not blocked" do
+      it "stores the impersonator in the session" do
+        post :impersonate, id: user.username
+
+        expect(session[:impersonator_id]).to eq(admin.id)
+      end
+
+      it "signs us in as the user" do
+        post :impersonate, id: user.username
+
+        expect(warden.user).to eq(user)
+      end
+
+      it "redirects to root" do
+        post :impersonate, id: user.username
+
+        expect(response).to redirect_to(root_path)
+      end
+
+      it "shows a notice" do
+        post :impersonate, id: user.username
+
+        expect(flash[:alert]).to eq("You are now impersonating #{user.username}")
+      end
+    end
+  end
 end
diff --git a/spec/features/merge_requests/create_new_mr_spec.rb b/spec/features/merge_requests/create_new_mr_spec.rb
index 00b60bd0e75..e296078bad8 100644
--- a/spec/features/merge_requests/create_new_mr_spec.rb
+++ b/spec/features/merge_requests/create_new_mr_spec.rb
@@ -30,4 +30,14 @@ feature 'Create New Merge Request', feature: true, js: true do
 
     expect(page).to have_content 'git checkout -b orphaned-branch origin/orphaned-branch'
   end
+
+  context 'when target project cannot be viewed by the current user' do
+    it 'does not leak the private project name & namespace' do
+      private_project = create(:project, :private)
+
+      visit new_namespace_project_merge_request_path(project.namespace, project, merge_request: { target_project_id: private_project.id })
+
+      expect(page).not_to have_content private_project.to_reference
+    end
+  end
 end
diff --git a/spec/helpers/issues_helper_spec.rb b/spec/helpers/issues_helper_spec.rb
index 543593cf389..bffe2c18b6f 100644
--- a/spec/helpers/issues_helper_spec.rb
+++ b/spec/helpers/issues_helper_spec.rb
@@ -30,6 +30,18 @@ describe IssuesHelper do
       expect(url_for_project_issues).to eq ""
     end
 
+    it 'returns an empty string if project_url is invalid' do
+      expect(project).to receive_message_chain('issues_tracker.project_url') { 'javascript:alert("foo");' }
+
+      expect(url_for_project_issues(project)).to eq ''
+    end
+
+    it 'returns an empty string if project_path is invalid' do
+      expect(project).to receive_message_chain('issues_tracker.project_path') { 'javascript:alert("foo");' }
+
+      expect(url_for_project_issues(project, only_path: true)).to eq ''
+    end
+
     describe "when external tracker was enabled and then config removed" do
       before do
         @project = ext_project
@@ -68,6 +80,18 @@ describe IssuesHelper do
       expect(url_for_issue(issue.iid)).to eq ""
     end
 
+    it 'returns an empty string if issue_url is invalid' do
+      expect(project).to receive_message_chain('issues_tracker.issue_url') { 'javascript:alert("foo");' }
+
+      expect(url_for_issue(issue.iid, project)).to eq ''
+    end
+
+    it 'returns an empty string if issue_path is invalid' do
+      expect(project).to receive_message_chain('issues_tracker.issue_path') { 'javascript:alert("foo");' }
+
+      expect(url_for_issue(issue.iid, project, only_path: true)).to eq ''
+    end
+
     describe "when external tracker was enabled and then config removed" do
       before do
         @project = ext_project
@@ -105,6 +129,18 @@ describe IssuesHelper do
       expect(url_for_new_issue).to eq ""
     end
 
+    it 'returns an empty string if issue_url is invalid' do
+      expect(project).to receive_message_chain('issues_tracker.new_issue_url') { 'javascript:alert("foo");' }
+
+      expect(url_for_new_issue(project)).to eq ''
+    end
+
+    it 'returns an empty string if issue_path is invalid' do
+      expect(project).to receive_message_chain('issues_tracker.new_issue_path') { 'javascript:alert("foo");' }
+
+      expect(url_for_new_issue(project, only_path: true)).to eq ''
+    end
+
     describe "when external tracker was enabled and then config removed" do
       before do
         @project = ext_project
diff --git a/spec/models/project_services/bamboo_service_spec.rb b/spec/models/project_services/bamboo_service_spec.rb
index 31b2c90122d..e771f35811e 100644
--- a/spec/models/project_services/bamboo_service_spec.rb
+++ b/spec/models/project_services/bamboo_service_spec.rb
@@ -27,86 +27,51 @@ describe BambooService, models: true do
   end
 
   describe 'Validations' do
-    describe '#bamboo_url' do
-      it 'does not validate the presence of bamboo_url if service is not active' do
-        bamboo_service = service
-        bamboo_service.active = false
-
-        expect(bamboo_service).not_to validate_presence_of(:bamboo_url)
-      end
-
-      it 'validates the presence of bamboo_url if service is active' do
-        bamboo_service = service
-        bamboo_service.active = true
-
-        expect(bamboo_service).to validate_presence_of(:bamboo_url)
-      end
-    end
+    subject { service }
 
-    describe '#build_key' do
-      it 'does not validate the presence of build_key if service is not active' do
-        bamboo_service = service
-        bamboo_service.active = false
+    context 'when service is active' do
+      before { subject.active = true }
 
-        expect(bamboo_service).not_to validate_presence_of(:build_key)
-      end
+      it { is_expected.to validate_presence_of(:build_key) }
+      it { is_expected.to validate_presence_of(:bamboo_url) }
+      it_behaves_like 'issue tracker service URL attribute', :bamboo_url
 
-      it 'validates the presence of build_key if service is active' do
-        bamboo_service = service
-        bamboo_service.active = true
+      describe '#username' do
+        it 'does not validate the presence of username if password is nil' do
+          subject.password = nil
 
-        expect(bamboo_service).to validate_presence_of(:build_key)
-      end
-    end
+          expect(subject).not_to validate_presence_of(:username)
+        end
 
-    describe '#username' do
-      it 'does not validate the presence of username if service is not active' do
-        bamboo_service = service
-        bamboo_service.active = false
+        it 'validates the presence of username if password is present' do
+          subject.password = 'secret'
 
-        expect(bamboo_service).not_to validate_presence_of(:username)
+          expect(subject).to validate_presence_of(:username)
+        end
       end
 
-      it 'does not validate the presence of username if username is nil' do
-        bamboo_service = service
-        bamboo_service.active = true
-        bamboo_service.password = nil
+      describe '#password' do
+        it 'does not validate the presence of password if username is nil' do
+          subject.username = nil
 
-        expect(bamboo_service).not_to validate_presence_of(:username)
-      end
+          expect(subject).not_to validate_presence_of(:password)
+        end
 
-      it 'validates the presence of username if service is active and username is present' do
-        bamboo_service = service
-        bamboo_service.active = true
-        bamboo_service.password = 'secret'
+        it 'validates the presence of password if username is present' do
+          subject.username = 'john'
 
-        expect(bamboo_service).to validate_presence_of(:username)
+          expect(subject).to validate_presence_of(:password)
+        end
       end
     end
 
-    describe '#password' do
-      it 'does not validate the presence of password if service is not active' do
-        bamboo_service = service
-        bamboo_service.active = false
-
-        expect(bamboo_service).not_to validate_presence_of(:password)
-      end
-
-      it 'does not validate the presence of password if username is nil' do
-        bamboo_service = service
-        bamboo_service.active = true
-        bamboo_service.username = nil
-
-        expect(bamboo_service).not_to validate_presence_of(:password)
-      end
-
-      it 'validates the presence of password if service is active and username is present' do
-        bamboo_service = service
-        bamboo_service.active = true
-        bamboo_service.username = 'john'
+    context 'when service is inactive' do
+      before { subject.active = false }
 
-        expect(bamboo_service).to validate_presence_of(:password)
-      end
+      it { is_expected.not_to validate_presence_of(:build_key) }
+      it { is_expected.not_to validate_presence_of(:bamboo_url) }
+      it { is_expected.not_to validate_presence_of(:username) }
+      it { is_expected.not_to validate_presence_of(:password) }
     end
   end
 
diff --git a/spec/models/project_services/buildkite_service_spec.rb b/spec/models/project_services/buildkite_service_spec.rb
index 88cd624877a..60364df2015 100644
--- a/spec/models/project_services/buildkite_service_spec.rb
+++ b/spec/models/project_services/buildkite_service_spec.rb
@@ -26,6 +26,23 @@ describe BuildkiteService, models: true do
     it { is_expected.to have_one :service_hook }
   end
 
+  describe 'Validations' do
+    context 'when service is active' do
+      before { subject.active = true }
+
+      it { is_expected.to validate_presence_of(:project_url) }
+      it { is_expected.to validate_presence_of(:token) }
+      it_behaves_like 'issue tracker service URL attribute', :project_url
+    end
+
+    context 'when service is inactive' do
+      before { subject.active = false }
+
+      it { is_expected.not_to validate_presence_of(:project_url) }
+      it { is_expected.not_to validate_presence_of(:token) }
+    end
+  end
+
   describe 'commits methods' do
     before do
       @project = Project.new
diff --git a/spec/models/project_services/builds_email_service_spec.rb b/spec/models/project_services/builds_email_service_spec.rb
index 7c23c2efccd..236df8f047d 100644
--- a/spec/models/project_services/builds_email_service_spec.rb
+++ b/spec/models/project_services/builds_email_service_spec.rb
@@ -1,76 +1,71 @@
 require 'spec_helper'
 
 describe BuildsEmailService do
-  let(:build) { create(:ci_build) }
-  let(:data) { Gitlab::BuildDataBuilder.build(build) }
-  let!(:project) { create(:project, :public, ci_id: 1) }
-  let(:service) { described_class.new(project: project, active: true) }
+  let(:data) { Gitlab::BuildDataBuilder.build(create(:ci_build)) }
+
+  describe 'Validations' do
+    context 'when service is active' do
+      before { subject.active = true }
+
+      it { is_expected.to validate_presence_of(:recipients) }
+
+      context 'when pusher is added' do
+        before { subject.add_pusher = true }
+
+        it { is_expected.not_to validate_presence_of(:recipients) }
+      end
+    end
+
+    context 'when service is inactive' do
+      before { subject.active = false }
+
+      it { is_expected.not_to validate_presence_of(:recipients) }
+    end
+  end
 
   describe '#execute' do
     it 'sends email' do
-      service.recipients = 'test@gitlab.com'
+      subject.recipients = 'test@gitlab.com'
       data[:build_status] = 'failed'
+
       expect(BuildEmailWorker).to receive(:perform_async)
-      service.execute(data)
+
+      subject.execute(data)
     end
 
     it 'does not send email with succeeded build and notify_only_broken_builds on' do
-      expect(service).to receive(:notify_only_broken_builds).and_return(true)
+      expect(subject).to receive(:notify_only_broken_builds).and_return(true)
       data[:build_status] = 'success'
+
       expect(BuildEmailWorker).not_to receive(:perform_async)
-      service.execute(data)
+
+      subject.execute(data)
     end
 
     it 'does not send email with failed build and build_allow_failure is true' do
       data[:build_status] = 'failed'
       data[:build_allow_failure] = true
+
       expect(BuildEmailWorker).not_to receive(:perform_async)
-      service.execute(data)
+
+      subject.execute(data)
     end
 
     it 'does not send email with unknown build status' do
       data[:build_status] = 'foo'
-      expect(BuildEmailWorker).not_to receive(:perform_async)
-      service.execute(data)
-    end
 
-    it 'does not send email when recipients list is empty' do
-      service.recipients = ' ,, '
-      data[:build_status] = 'failed'
       expect(BuildEmailWorker).not_to receive(:perform_async)
-      service.execute(data)
-    end
-  end
-
-  describe 'validations' do
-
-    context 'when pusher is not added' do
-      before { service.add_pusher = false }
-
-      it 'does not allow empty recipient input' do
-        service.recipients = ''
-        expect(service.valid?).to be false
-      end
-
-      it 'does allow non-empty recipient input' do
-        service.recipients = 'test@example.com'
-        expect(service.valid?).to be true
-      end
 
+      subject.execute(data)
     end
 
-    context 'when pusher is added' do
-      before { service.add_pusher = true }
+    it 'does not send email when recipients list is empty' do
+      subject.recipients = ' ,, '
+      data[:build_status] = 'failed'
 
-      it 'does allow empty recipient input' do
-        service.recipients = ''
-        expect(service.valid?).to be true
-      end
+      expect(BuildEmailWorker).not_to receive(:perform_async)
 
-      it 'does allow non-empty recipient input' do
-        service.recipients = 'test@example.com'
-        expect(service.valid?).to be true
-      end
+      subject.execute(data)
     end
   end
 end
diff --git a/spec/models/project_services/campfire_service_spec.rb b/spec/models/project_services/campfire_service_spec.rb
new file mode 100644
index 00000000000..3e6da42803b
--- /dev/null
+++ b/spec/models/project_services/campfire_service_spec.rb
@@ -0,0 +1,42 @@
+# == Schema Information
+#
+# Table name: services
+#
+#  id                    :integer          not null, primary key
+#  type                  :string(255)
+#  title                 :string(255)
+#  project_id            :integer
+#  created_at            :datetime
+#  updated_at            :datetime
+#  active                :boolean          default(FALSE), not null
+#  properties            :text
+#  template              :boolean          default(FALSE)
+#  push_events           :boolean          default(TRUE)
+#  issues_events         :boolean          default(TRUE)
+#  merge_requests_events :boolean          default(TRUE)
+#  tag_push_events       :boolean          default(TRUE)
+#  note_events           :boolean          default(TRUE), not null
+#
+
+require 'spec_helper'
+
+describe CampfireService, models: true do
+  describe 'Associations' do
+    it { is_expected.to belong_to :project }
+    it { is_expected.to have_one :service_hook }
+  end
+
+  describe 'Validations' do
+    context 'when service is active' do
+      before { subject.active = true }
+
+      it { is_expected.to validate_presence_of(:token) }
+    end
+
+    context 'when service is inactive' do
+      before { subject.active = false }
+
+      it { is_expected.not_to validate_presence_of(:token) }
+    end
+  end
+end
diff --git a/spec/models/project_services/custom_issue_tracker_service_spec.rb b/spec/models/project_services/custom_issue_tracker_service_spec.rb
new file mode 100644
index 00000000000..ff976f8ec59
--- /dev/null
+++ b/spec/models/project_services/custom_issue_tracker_service_spec.rb
@@ -0,0 +1,49 @@
+# == Schema Information
+#
+# Table name: services
+#
+#  id                    :integer          not null, primary key
+#  type                  :string(255)
+#  title                 :string(255)
+#  project_id            :integer
+#  created_at            :datetime
+#  updated_at            :datetime
+#  active                :boolean          default(FALSE), not null
+#  properties            :text
+#  template              :boolean          default(FALSE)
+#  push_events           :boolean          default(TRUE)
+#  issues_events         :boolean          default(TRUE)
+#  merge_requests_events :boolean          default(TRUE)
+#  tag_push_events       :boolean          default(TRUE)
+#  note_events           :boolean          default(TRUE), not null
+#
+
+require 'spec_helper'
+
+describe CustomIssueTrackerService, models: true do
+  describe 'Associations' do
+    it { is_expected.to belong_to :project }
+    it { is_expected.to have_one :service_hook }
+  end
+
+  describe 'Validations' do
+    context 'when service is active' do
+      before { subject.active = true }
+
+      it { is_expected.to validate_presence_of(:project_url) }
+      it { is_expected.to validate_presence_of(:issues_url) }
+      it { is_expected.to validate_presence_of(:new_issue_url) }
+      it_behaves_like 'issue tracker service URL attribute', :project_url
+      it_behaves_like 'issue tracker service URL attribute', :issues_url
+      it_behaves_like 'issue tracker service URL attribute', :new_issue_url
+    end
+
+    context 'when service is inactive' do
+      before { subject.active = false }
+
+      it { is_expected.not_to validate_presence_of(:project_url) }
+      it { is_expected.not_to validate_presence_of(:issues_url) }
+      it { is_expected.not_to validate_presence_of(:new_issue_url) }
+    end
+  end
+end
diff --git a/spec/models/project_services/drone_ci_service_spec.rb b/spec/models/project_services/drone_ci_service_spec.rb
index a2cf68a9e38..3a8e67438fc 100644
--- a/spec/models/project_services/drone_ci_service_spec.rb
+++ b/spec/models/project_services/drone_ci_service_spec.rb
@@ -28,25 +28,18 @@ describe DroneCiService, models: true do
 
   describe 'validations' do
     context 'active' do
-      before { allow(subject).to receive(:activated?).and_return(true) }
+      before { subject.active = true }
 
       it { is_expected.to validate_presence_of(:token) }
       it { is_expected.to validate_presence_of(:drone_url) }
-      it { is_expected.to allow_value('ewf9843kdnfdfs89234n').for(:token) }
-      it { is_expected.to allow_value('http://ci.example.com').for(:drone_url) }
-      it { is_expected.not_to allow_value('this is not url').for(:drone_url) }
-      it { is_expected.not_to allow_value('http//noturl').for(:drone_url) }
-      it { is_expected.not_to allow_value('ftp://ci.example.com').for(:drone_url) }
+      it_behaves_like 'issue tracker service URL attribute', :drone_url
     end
 
     context 'inactive' do
-      before { allow(subject).to receive(:activated?).and_return(false) }
+      before { subject.active = false }
 
       it { is_expected.not_to validate_presence_of(:token) }
       it { is_expected.not_to validate_presence_of(:drone_url) }
-      it { is_expected.to allow_value('ewf9843kdnfdfs89234n').for(:token) }
-      it { is_expected.to allow_value('http://drone.example.com').for(:drone_url) }
-      it { is_expected.to allow_value('ftp://drone.example.com').for(:drone_url) }
     end
   end
 
diff --git a/spec/models/project_services/emails_on_push_service_spec.rb b/spec/models/project_services/emails_on_push_service_spec.rb
new file mode 100644
index 00000000000..e6f78898c82
--- /dev/null
+++ b/spec/models/project_services/emails_on_push_service_spec.rb
@@ -0,0 +1,17 @@
+require 'spec_helper'
+
+describe EmailsOnPushService do
+  describe 'Validations' do
+    context 'when service is active' do
+      before { subject.active = true }
+
+      it { is_expected.to validate_presence_of(:recipients) }
+    end
+
+    context 'when service is inactive' do
+      before { subject.active = false }
+
+      it { is_expected.not_to validate_presence_of(:recipients) }
+    end
+  end
+end
diff --git a/spec/models/external_wiki_service_spec.rb b/spec/models/project_services/external_wiki_service_spec.rb
index d37978720bf..5fe5ea7d2df 100644
--- a/spec/models/external_wiki_service_spec.rb
+++ b/spec/models/project_services/external_wiki_service_spec.rb
@@ -28,13 +28,18 @@ describe ExternalWikiService, models: true do
     it { should have_one :service_hook }
   end
 
-  describe "Validations" do
-    context "active" do
-      before do
-        subject.active = true
-      end
+  describe 'Validations' do
+    context 'when service is active' do
+      before { subject.active = true }
+
+      it { is_expected.to validate_presence_of(:external_wiki_url) }
+      it_behaves_like 'issue tracker service URL attribute', :external_wiki_url
+    end
+
+    context 'when service is inactive' do
+      before { subject.active = false }
 
-      it { should validate_presence_of :external_wiki_url }
+      it { is_expected.not_to validate_presence_of(:external_wiki_url) }
     end
   end
 
diff --git a/spec/models/project_services/flowdock_service_spec.rb b/spec/models/project_services/flowdock_service_spec.rb
index ff7fbcaa004..b7e627e6518 100644
--- a/spec/models/project_services/flowdock_service_spec.rb
+++ b/spec/models/project_services/flowdock_service_spec.rb
@@ -26,6 +26,20 @@ describe FlowdockService, models: true do
     it { is_expected.to have_one :service_hook }
   end
 
+  describe 'Validations' do
+    context 'when service is active' do
+      before { subject.active = true }
+
+      it { is_expected.to validate_presence_of(:token) }
+    end
+
+    context 'when service is inactive' do
+      before { subject.active = false }
+
+      it { is_expected.not_to validate_presence_of(:token) }
+    end
+  end
+
   describe "Execute" do
     let(:user)    { create(:user) }
     let(:project) { create(:project) }
diff --git a/spec/models/project_services/gemnasium_service_spec.rb b/spec/models/project_services/gemnasium_service_spec.rb
index ecb3ccb1673..a08f1ac229f 100644
--- a/spec/models/project_services/gemnasium_service_spec.rb
+++ b/spec/models/project_services/gemnasium_service_spec.rb
@@ -26,6 +26,22 @@ describe GemnasiumService, models: true do
     it { is_expected.to have_one :service_hook }
   end
 
+  describe 'Validations' do
+    context 'when service is active' do
+      before { subject.active = true }
+
+      it { is_expected.to validate_presence_of(:token) }
+      it { is_expected.to validate_presence_of(:api_key) }
+    end
+
+    context 'when service is inactive' do
+      before { subject.active = false }
+
+      it { is_expected.not_to validate_presence_of(:token) }
+      it { is_expected.not_to validate_presence_of(:api_key) }
+    end
+  end
+
   describe "Execute" do
     let(:user)    { create(:user) }
     let(:project) { create(:project) }
diff --git a/spec/models/project_services/gitlab_issue_tracker_service_spec.rb b/spec/models/project_services/gitlab_issue_tracker_service_spec.rb
index 3518dbd1728..7a1f106d6e3 100644
--- a/spec/models/project_services/gitlab_issue_tracker_service_spec.rb
+++ b/spec/models/project_services/gitlab_issue_tracker_service_spec.rb
@@ -26,6 +26,20 @@ describe GitlabIssueTrackerService, models: true do
     it { is_expected.to have_one :service_hook }
   end
 
+  describe 'Validations' do
+    context 'when service is active' do
+      subject { described_class.new(project: create(:project), active: true) }
+
+      it { is_expected.to validate_presence_of(:issues_url) }
+      it_behaves_like 'issue tracker service URL attribute', :issues_url
+    end
+
+    context 'when service is inactive' do
+      subject { described_class.new(project: create(:project), active: false) }
+
+      it { is_expected.not_to validate_presence_of(:issues_url) }
+    end
+  end
 
   describe 'project and issue urls' do
     let(:project) { create(:project) }
diff --git a/spec/models/project_services/hipchat_service_spec.rb b/spec/models/project_services/hipchat_service_spec.rb
index d878162a220..6fb5cad5011 100644
--- a/spec/models/project_services/hipchat_service_spec.rb
+++ b/spec/models/project_services/hipchat_service_spec.rb
@@ -26,6 +26,20 @@ describe HipchatService, models: true do
     it { is_expected.to have_one :service_hook }
   end
 
+  describe 'Validations' do
+    context 'when service is active' do
+      before { subject.active = true }
+
+      it { is_expected.to validate_presence_of(:token) }
+    end
+
+    context 'when service is inactive' do
+      before { subject.active = false }
+
+      it { is_expected.not_to validate_presence_of(:token) }
+    end
+  end
+
   describe "Execute" do
     let(:hipchat) { HipchatService.new }
     let(:user)    { create(:user, username: 'username') }
diff --git a/spec/models/project_services/irker_service_spec.rb b/spec/models/project_services/irker_service_spec.rb
index b783b1a576e..4ee022a5171 100644
--- a/spec/models/project_services/irker_service_spec.rb
+++ b/spec/models/project_services/irker_service_spec.rb
@@ -29,14 +29,16 @@ describe IrkerService, models: true do
   end
 
   describe 'Validations' do
-    before do
-      subject.active = true
-      subject.properties['recipients'] = _recipients
+    context 'when service is active' do
+      before { subject.active = true }
+
+      it { is_expected.to validate_presence_of(:recipients) }
     end
 
-    context 'active' do
-      let(:_recipients) { nil }
-      it { should validate_presence_of :recipients }
+    context 'when service is inactive' do
+      before { subject.active = false }
+
+      it { is_expected.not_to validate_presence_of(:recipients) }
     end
   end
 
diff --git a/spec/models/project_services/jira_service_spec.rb b/spec/models/project_services/jira_service_spec.rb
index 2f8193170ae..5309cfb99ff 100644
--- a/spec/models/project_services/jira_service_spec.rb
+++ b/spec/models/project_services/jira_service_spec.rb
@@ -26,6 +26,30 @@ describe JiraService, models: true do
     it { is_expected.to have_one :service_hook }
   end
 
+  describe 'Validations' do
+    context 'when service is active' do
+      before { subject.active = true }
+
+      it { is_expected.to validate_presence_of(:api_url) }
+      it { is_expected.to validate_presence_of(:project_url) }
+      it { is_expected.to validate_presence_of(:issues_url) }
+      it { is_expected.to validate_presence_of(:new_issue_url) }
+      it_behaves_like 'issue tracker service URL attribute', :api_url
+      it_behaves_like 'issue tracker service URL attribute', :project_url
+      it_behaves_like 'issue tracker service URL attribute', :issues_url
+      it_behaves_like 'issue tracker service URL attribute', :new_issue_url
+    end
+
+    context 'when service is inactive' do
+      before { subject.active = false }
+
+      it { is_expected.not_to validate_presence_of(:api_url) }
+      it { is_expected.not_to validate_presence_of(:project_url) }
+      it { is_expected.not_to validate_presence_of(:issues_url) }
+      it { is_expected.not_to validate_presence_of(:new_issue_url) }
+    end
+  end
+
   describe "Execute" do
     let(:user)    { create(:user) }
     let(:project) { create(:project) }
@@ -72,7 +96,7 @@ describe JiraService, models: true do
 
     context "when a password was previously set" do
       before do
-        @jira_service = JiraService.create(
+        @jira_service = JiraService.create!(
           project: create(:project),
           properties: {
             api_url: 'http://jira.example.com/rest/api/2',
diff --git a/spec/models/project_services/pivotaltracker_service_spec.rb b/spec/models/project_services/pivotaltracker_service_spec.rb
new file mode 100644
index 00000000000..f37edd4d970
--- /dev/null
+++ b/spec/models/project_services/pivotaltracker_service_spec.rb
@@ -0,0 +1,42 @@
+# == Schema Information
+#
+# Table name: services
+#
+#  id                    :integer          not null, primary key
+#  type                  :string(255)
+#  title                 :string(255)
+#  project_id            :integer
+#  created_at            :datetime
+#  updated_at            :datetime
+#  active                :boolean          default(FALSE), not null
+#  properties            :text
+#  template              :boolean          default(FALSE)
+#  push_events           :boolean          default(TRUE)
+#  issues_events         :boolean          default(TRUE)
+#  merge_requests_events :boolean          default(TRUE)
+#  tag_push_events       :boolean          default(TRUE)
+#  note_events           :boolean          default(TRUE), not null
+#
+
+require 'spec_helper'
+
+describe PivotaltrackerService, models: true do
+  describe 'Associations' do
+    it { is_expected.to belong_to :project }
+    it { is_expected.to have_one :service_hook }
+  end
+
+  describe 'Validations' do
+    context 'when service is active' do
+      before { subject.active = true }
+
+      it { is_expected.to validate_presence_of(:token) }
+    end
+
+    context 'when service is inactive' do
+      before { subject.active = false }
+
+      it { is_expected.not_to validate_presence_of(:token) }
+    end
+  end
+end
diff --git a/spec/models/project_services/pushover_service_spec.rb b/spec/models/project_services/pushover_service_spec.rb
index 96039f9491b..555d9757b47 100644
--- a/spec/models/project_services/pushover_service_spec.rb
+++ b/spec/models/project_services/pushover_service_spec.rb
@@ -27,14 +27,20 @@ describe PushoverService, models: true do
   end
 
   describe 'Validations' do
-    context 'active' do
-      before do
-        subject.active = true
-      end
+    context 'when service is active' do
+      before { subject.active = true }
 
-      it { is_expected.to validate_presence_of :api_key }
-      it { is_expected.to validate_presence_of :user_key }
-      it { is_expected.to validate_presence_of :priority }
+      it { is_expected.to validate_presence_of(:api_key) }
+      it { is_expected.to validate_presence_of(:user_key) }
+      it { is_expected.to validate_presence_of(:priority) }
+    end
+
+    context 'when service is inactive' do
+      before { subject.active = false }
+
+      it { is_expected.not_to validate_presence_of(:api_key) }
+      it { is_expected.not_to validate_presence_of(:user_key) }
+      it { is_expected.not_to validate_presence_of(:priority) }
     end
   end
 
diff --git a/spec/models/project_services/redmine_service_spec.rb b/spec/models/project_services/redmine_service_spec.rb
new file mode 100644
index 00000000000..7d14f6e8280
--- /dev/null
+++ b/spec/models/project_services/redmine_service_spec.rb
@@ -0,0 +1,49 @@
+# == Schema Information
+#
+# Table name: services
+#
+#  id                    :integer          not null, primary key
+#  type                  :string(255)
+#  title                 :string(255)
+#  project_id            :integer
+#  created_at            :datetime
+#  updated_at            :datetime
+#  active                :boolean          default(FALSE), not null
+#  properties            :text
+#  template              :boolean          default(FALSE)
+#  push_events           :boolean          default(TRUE)
+#  issues_events         :boolean          default(TRUE)
+#  merge_requests_events :boolean          default(TRUE)
+#  tag_push_events       :boolean          default(TRUE)
+#  note_events           :boolean          default(TRUE), not null
+#
+
+require 'spec_helper'
+
+describe RedmineService, models: true do
+  describe 'Associations' do
+    it { is_expected.to belong_to :project }
+    it { is_expected.to have_one :service_hook }
+  end
+
+  describe 'Validations' do
+    context 'when service is active' do
+      before { subject.active = true }
+
+      it { is_expected.to validate_presence_of(:project_url) }
+      it { is_expected.to validate_presence_of(:issues_url) }
+      it { is_expected.to validate_presence_of(:new_issue_url) }
+      it_behaves_like 'issue tracker service URL attribute', :project_url
+      it_behaves_like 'issue tracker service URL attribute', :issues_url
+      it_behaves_like 'issue tracker service URL attribute', :new_issue_url
+    end
+
+    context 'when service is inactive' do
+      before { subject.active = false }
+
+      it { is_expected.not_to validate_presence_of(:project_url) }
+      it { is_expected.not_to validate_presence_of(:issues_url) }
+      it { is_expected.not_to validate_presence_of(:new_issue_url) }
+    end
+  end
+end
diff --git a/spec/models/project_services/slack_service_spec.rb b/spec/models/project_services/slack_service_spec.rb
index 478d59be08b..a97b7560137 100644
--- a/spec/models/project_services/slack_service_spec.rb
+++ b/spec/models/project_services/slack_service_spec.rb
@@ -26,13 +26,18 @@ describe SlackService, models: true do
     it { is_expected.to have_one :service_hook }
   end
 
-  describe "Validations" do
-    context "active" do
-      before do
-        subject.active = true
-      end
+  describe 'Validations' do
+    context 'when service is active' do
+      before { subject.active = true }
 
-      it { is_expected.to validate_presence_of :webhook }
+      it { is_expected.to validate_presence_of(:webhook) }
+      it_behaves_like 'issue tracker service URL attribute', :webhook
+    end
+
+    context 'when service is inactive' do
+      before { subject.active = false }
+
+      it { is_expected.not_to validate_presence_of(:webhook) }
     end
   end
 
diff --git a/spec/models/project_services/teamcity_service_spec.rb b/spec/models/project_services/teamcity_service_spec.rb
index bc7423cee69..ad24b895170 100644
--- a/spec/models/project_services/teamcity_service_spec.rb
+++ b/spec/models/project_services/teamcity_service_spec.rb
@@ -27,86 +27,51 @@ describe TeamcityService, models: true do
   end
 
   describe 'Validations' do
-    describe '#teamcity_url' do
-      it 'does not validate the presence of teamcity_url if service is not active' do
-        teamcity_service = service
-        teamcity_service.active = false
-
-        expect(teamcity_service).not_to validate_presence_of(:teamcity_url)
-      end
-
-      it 'validates the presence of teamcity_url if service is active' do
-        teamcity_service = service
-        teamcity_service.active = true
-
-        expect(teamcity_service).to validate_presence_of(:teamcity_url)
-      end
-    end
+    subject { service }
 
-    describe '#build_type' do
-      it 'does not validate the presence of build_type if service is not active' do
-        teamcity_service = service
-        teamcity_service.active = false
+    context 'when service is active' do
+      before { subject.active = true }
 
-        expect(teamcity_service).not_to validate_presence_of(:build_type)
-      end
+      it { is_expected.to validate_presence_of(:build_type) }
+      it { is_expected.to validate_presence_of(:teamcity_url) }
+      it_behaves_like 'issue tracker service URL attribute', :teamcity_url
 
-      it 'validates the presence of build_type if service is active' do
-        teamcity_service = service
-        teamcity_service.active = true
+      describe '#username' do
+        it 'does not validate the presence of username if password is nil' do
+          subject.password = nil
 
-        expect(teamcity_service).to validate_presence_of(:build_type)
-      end
-    end
+          expect(subject).not_to validate_presence_of(:username)
+        end
 
-    describe '#username' do
-      it 'does not validate the presence of username if service is not active' do
-        teamcity_service = service
-        teamcity_service.active = false
+        it 'validates the presence of username if password is present' do
+          subject.password = 'secret'
 
-        expect(teamcity_service).not_to validate_presence_of(:username)
+          expect(subject).to validate_presence_of(:username)
+        end
       end
 
-      it 'does not validate the presence of username if username is nil' do
-        teamcity_service = service
-        teamcity_service.active = true
-        teamcity_service.password = nil
+      describe '#password' do
+        it 'does not validate the presence of password if username is nil' do
+          subject.username = nil
 
-        expect(teamcity_service).not_to validate_presence_of(:username)
-      end
+          expect(subject).not_to validate_presence_of(:password)
+        end
 
-      it 'validates the presence of username if service is active and username is present' do
-        teamcity_service = service
-        teamcity_service.active = true
-        teamcity_service.password = 'secret'
+        it 'validates the presence of password if username is present' do
+          subject.username = 'john'
 
-        expect(teamcity_service).to validate_presence_of(:username)
+          expect(subject).to validate_presence_of(:password)
+        end
       end
     end
 
-    describe '#password' do
-      it 'does not validate the presence of password if service is not active' do
-        teamcity_service = service
-        teamcity_service.active = false
-
-        expect(teamcity_service).not_to validate_presence_of(:password)
-      end
-
-      it 'does not validate the presence of password if username is nil' do
-        teamcity_service = service
-        teamcity_service.active = true
-        teamcity_service.username = nil
-
-        expect(teamcity_service).not_to validate_presence_of(:password)
-      end
-
-      it 'validates the presence of password if service is active and username is present' do
-        teamcity_service = service
-        teamcity_service.active = true
-        teamcity_service.username = 'john'
+    context 'when service is inactive' do
+      before { subject.active = false }
 
-        expect(teamcity_service).to validate_presence_of(:password)
-      end
+      it { is_expected.not_to validate_presence_of(:build_type) }
+      it { is_expected.not_to validate_presence_of(:teamcity_url) }
+      it { is_expected.not_to validate_presence_of(:username) }
+      it { is_expected.not_to validate_presence_of(:password) }
     end
   end
 
diff --git a/spec/requests/api/milestones_spec.rb b/spec/requests/api/milestones_spec.rb
index 344f0fe0b7f..241995041bb 100644
--- a/spec/requests/api/milestones_spec.rb
+++ b/spec/requests/api/milestones_spec.rb
@@ -127,7 +127,7 @@ describe API::API, api: true  do
 
   describe 'GET /projects/:id/milestones/:milestone_id/issues' do
     before do
-      milestone.issues << create(:issue)
+      milestone.issues << create(:issue, project: project)
     end
     it 'should return project issues for a particular milestone' do
       get api("/projects/#{project.id}/milestones/#{milestone.id}/issues", user)
@@ -140,5 +140,34 @@ describe API::API, api: true  do
       get api("/projects/#{project.id}/milestones/#{milestone.id}/issues")
       expect(response.status).to eq(401)
     end
+
+    describe 'confidential issues' do
+      let(:public_project) { create(:project, :public) }
+      let(:milestone) { create(:milestone, project: public_project) }
+      let(:issue) { create(:issue, project: public_project) }
+      let(:confidential_issue) { create(:issue, confidential: true, project: public_project) }
+      before do
+        public_project.team << [user, :developer]
+        milestone.issues << issue << confidential_issue
+      end
+
+      it 'returns confidential issues to team members' do
+        get api("/projects/#{public_project.id}/milestones/#{milestone.id}/issues", user)
+
+        expect(response.status).to eq(200)
+        expect(json_response).to be_an Array
+        expect(json_response.size).to eq(2)
+        expect(json_response.map { |issue| issue['id'] }).to include(issue.id, confidential_issue.id)
+      end
+
+      it 'does not return confidential issues to regular users' do
+        get api("/projects/#{public_project.id}/milestones/#{milestone.id}/issues", create(:user))
+
+        expect(response.status).to eq(200)
+        expect(json_response).to be_an Array
+        expect(json_response.size).to eq(1)
+        expect(json_response.map { |issue| issue['id'] }).to include(issue.id)
+      end
+    end
   end
 end
diff --git a/spec/requests/api/project_snippets_spec.rb b/spec/requests/api/project_snippets_spec.rb
index 3722ddf5a33..9706d060cfa 100644
--- a/spec/requests/api/project_snippets_spec.rb
+++ b/spec/requests/api/project_snippets_spec.rb
@@ -15,4 +15,91 @@ describe API::API, api: true do
       expect(json_response['expires_at']).to be_nil
     end
   end
+
+  describe 'GET /projects/:project_id/snippets/' do
+    it 'all snippets available to team member' do
+      project = create(:project, :public)
+      user = create(:user)
+      project.team << [user, :developer]
+      public_snippet = create(:project_snippet, :public, project: project)
+      internal_snippet = create(:project_snippet, :internal, project: project)
+      private_snippet = create(:project_snippet, :private, project: project)
+
+      get api("/projects/#{project.id}/snippets/", user)
+
+      expect(response.status).to eq(200)
+      expect(json_response.size).to eq(3)
+      expect(json_response.map{ |snippet| snippet['id']} ).to include(public_snippet.id, internal_snippet.id, private_snippet.id)
+    end
+
+    it 'hides private snippets from regular user' do
+      project = create(:project, :public)
+      user = create(:user)
+      create(:project_snippet, :private, project: project)
+
+      get api("/projects/#{project.id}/snippets/", user)
+      expect(response.status).to eq(200)
+      expect(json_response.size).to eq(0)
+    end
+  end
+
+  describe 'POST /projects/:project_id/snippets/' do
+    it 'creates a new snippet' do
+      admin = create(:admin)
+      project = create(:project)
+      params = {
+        title: 'Test Title',
+        file_name: 'test.rb',
+        code: 'puts "hello world"',
+        visibility_level: Gitlab::VisibilityLevel::PUBLIC
+      }
+
+      post api("/projects/#{project.id}/snippets/", admin), params
+
+      expect(response.status).to eq(201)
+      snippet = ProjectSnippet.find(json_response['id'])
+      expect(snippet.content).to eq(params[:code])
+      expect(snippet.title).to eq(params[:title])
+      expect(snippet.file_name).to eq(params[:file_name])
+      expect(snippet.visibility_level).to eq(params[:visibility_level])
+    end
+  end
+
+  describe 'PUT /projects/:project_id/snippets/:id/' do
+    it 'updates snippet' do
+      admin = create(:admin)
+      snippet = create(:project_snippet, author: admin)
+      new_content = 'New content'
+
+      put api("/projects/#{snippet.project.id}/snippets/#{snippet.id}/", admin), code: new_content
+
+      expect(response.status).to eq(200)
+      snippet.reload
+      expect(snippet.content).to eq(new_content)
+    end
+  end
+
+  describe 'DELETE /projects/:project_id/snippets/:id/' do
+    it 'deletes snippet' do
+      admin = create(:admin)
+      snippet = create(:project_snippet, author: admin)
+
+      delete api("/projects/#{snippet.project.id}/snippets/#{snippet.id}/", admin)
+
+      expect(response.status).to eq(200)
+    end
+  end
+
+  describe 'GET /projects/:project_id/snippets/:id/raw' do
+    it 'returns raw text' do
+      admin = create(:admin)
+      snippet = create(:project_snippet, author: admin)
+
+      get api("/projects/#{snippet.project.id}/snippets/#{snippet.id}/raw", admin)
+
+      expect(response.status).to eq(200)
+      expect(response.content_type).to eq 'text/plain'
+      expect(response.body).to eq(snippet.content)
+    end
+  end
 end
diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb
index fccd08bd6da..66193eac051 100644
--- a/spec/requests/api/projects_spec.rb
+++ b/spec/requests/api/projects_spec.rb
@@ -11,7 +11,7 @@ describe API::API, api: true  do
   let(:project) { create(:project, creator_id: user.id, namespace: user.namespace) }
   let(:project2) { create(:project, path: 'project2', creator_id: user.id, namespace: user.namespace) }
   let(:project3) { create(:project, path: 'project3', creator_id: user.id, namespace: user.namespace) }
-  let(:snippet) { create(:project_snippet, author: user, project: project, title: 'example') }
+  let(:snippet) { create(:project_snippet, :public, author: user, project: project, title: 'example') }
   let(:project_member) { create(:project_member, :master, user: user, project: project) }
   let(:project_member2) { create(:project_member, :developer, user: user3, project: project) }
   let(:user4) { create(:user) }
diff --git a/spec/support/issue_tracker_service_shared_example.rb b/spec/support/issue_tracker_service_shared_example.rb
new file mode 100644
index 00000000000..b6d7436c360
--- /dev/null
+++ b/spec/support/issue_tracker_service_shared_example.rb
@@ -0,0 +1,7 @@
+RSpec.shared_examples 'issue tracker service URL attribute' do |url_attr|
+  it { is_expected.to allow_value('https://example.com').for(url_attr) }
+
+  it { is_expected.not_to allow_value('example.com').for(url_attr) }
+  it { is_expected.not_to allow_value('ftp://example.com').for(url_attr) }
+  it { is_expected.not_to allow_value('herp-and-derp').for(url_attr) }
+end