Merge branch 'master' into live-trace-v2

6 files changed, 273 insertions, 1 deletions

diff --git a/spec/controllers/application_controller_spec.rb b/spec/controllers/application_controller_spec.rb
index fe95d1ef9cd..f0caac40afd 100644
--- a/spec/controllers/application_controller_spec.rb
+++ b/spec/controllers/application_controller_spec.rb
@@ -1,6 +1,8 @@
 require 'spec_helper'
 
 describe ApplicationController do
+  include TermsHelper
+
   let(:user) { create(:user) }
 
   describe '#check_password_expiration' do
@@ -406,4 +408,65 @@ describe ApplicationController do
       end
     end
   end
+
+  context 'terms' do
+    controller(described_class) do
+      def index
+        render text: 'authenticated'
+      end
+    end
+
+    before do
+      stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'false')
+      sign_in user
+    end
+
+    it 'does not query more when terms are enforced' do
+      control = ActiveRecord::QueryRecorder.new { get :index }
+
+      enforce_terms
+
+      expect { get :index }.not_to exceed_query_limit(control)
+    end
+
+    context 'when terms are enforced' do
+      before do
+        enforce_terms
+      end
+
+      it 'redirects if the user did not accept the terms'  do
+        get :index
+
+        expect(response).to have_gitlab_http_status(302)
+      end
+
+      it 'does not redirect when the user accepted terms' do
+        accept_terms(user)
+
+        get :index
+
+        expect(response).to have_gitlab_http_status(200)
+      end
+
+      context 'for sessionless users' do
+        before do
+          sign_out user
+        end
+
+        it 'renders a 403 when the sessionless user did not accept the terms' do
+          get :index, rss_token: user.rss_token, format: :atom
+
+          expect(response).to have_gitlab_http_status(403)
+        end
+
+        it 'renders a 200 when the sessionless user accepted the terms' do
+          accept_terms(user)
+
+          get :index, rss_token: user.rss_token, format: :atom
+
+          expect(response).to have_gitlab_http_status(200)
+        end
+      end
+    end
+  end
 end
diff --git a/spec/controllers/concerns/continue_params_spec.rb b/spec/controllers/concerns/continue_params_spec.rb
new file mode 100644
index 00000000000..e2f683ae393
--- /dev/null
+++ b/spec/controllers/concerns/continue_params_spec.rb
@@ -0,0 +1,45 @@
+require 'spec_helper'
+
+describe ContinueParams do
+  let(:controller_class) do
+    Class.new(ActionController::Base) do
+      include ContinueParams
+
+      def request
+        @request ||= Struct.new(:host, :port).new('test.host', 80)
+      end
+    end
+  end
+  subject(:controller) { controller_class.new }
+
+  def strong_continue_params(params)
+    ActionController::Parameters.new(continue: params)
+  end
+
+  it 'cleans up any params that are not allowed' do
+    allow(controller).to receive(:params) do
+      strong_continue_params(to: '/hello',
+                             notice: 'world',
+                             notice_now: '!',
+                             something: 'else')
+    end
+
+    expect(controller.continue_params.keys).to contain_exactly(*%w(to notice notice_now))
+  end
+
+  it 'does not allow cross host redirection' do
+    allow(controller).to receive(:params) do
+      strong_continue_params(to: '//example.com')
+    end
+
+    expect(controller.continue_params[:to]).to be_nil
+  end
+
+  it 'allows redirecting to a path with querystring' do
+    allow(controller).to receive(:params) do
+      strong_continue_params(to: '/hello/world?query=string')
+    end
+
+    expect(controller.continue_params[:to]).to eq('/hello/world?query=string')
+  end
+end
diff --git a/spec/controllers/concerns/internal_redirect_spec.rb b/spec/controllers/concerns/internal_redirect_spec.rb
new file mode 100644
index 00000000000..a0ee13b2352
--- /dev/null
+++ b/spec/controllers/concerns/internal_redirect_spec.rb
@@ -0,0 +1,66 @@
+require 'spec_helper'
+
+describe InternalRedirect do
+  let(:controller_class) do
+    Class.new do
+      include InternalRedirect
+
+      def request
+        @request ||= Struct.new(:host, :port).new('test.host', 80)
+      end
+    end
+  end
+  subject(:controller) { controller_class.new }
+
+  describe '#safe_redirect_path' do
+    it 'is `nil` for invalid uris' do
+      expect(controller.safe_redirect_path('Hello world')).to be_nil
+    end
+
+    it 'is `nil` for paths trying to include a host' do
+      expect(controller.safe_redirect_path('//example.com/hello/world')).to be_nil
+    end
+
+    it 'returns the path if it is valid' do
+      expect(controller.safe_redirect_path('/hello/world')).to eq('/hello/world')
+    end
+
+    it 'returns the path with querystring if it is valid' do
+      expect(controller.safe_redirect_path('/hello/world?hello=world#L123'))
+        .to eq('/hello/world?hello=world#L123')
+    end
+  end
+
+  describe '#safe_redirect_path_for_url' do
+    it 'is `nil` for invalid urls' do
+      expect(controller.safe_redirect_path_for_url('Hello world')).to be_nil
+    end
+
+    it 'is `nil` for urls from a with a different host' do
+      expect(controller.safe_redirect_path_for_url('http://example.com/hello/world')).to be_nil
+    end
+
+    it 'is `nil` for urls from a with a different port' do
+      expect(controller.safe_redirect_path_for_url('http://test.host:3000/hello/world')).to be_nil
+    end
+
+    it 'returns the path if the url is on the same host' do
+      expect(controller.safe_redirect_path_for_url('http://test.host/hello/world')).to eq('/hello/world')
+    end
+
+    it 'returns the path including querystring if the url is on the same host' do
+      expect(controller.safe_redirect_path_for_url('http://test.host/hello/world?hello=world#L123'))
+        .to eq('/hello/world?hello=world#L123')
+    end
+  end
+
+  describe '#host_allowed?' do
+    it 'allows uris with the same host and port' do
+      expect(controller.host_allowed?(URI('http://test.host/test'))).to be(true)
+    end
+
+    it 'rejects uris with other host and port' do
+      expect(controller.host_allowed?(URI('http://example.com/test'))).to be(false)
+    end
+  end
+end
diff --git a/spec/controllers/projects/settings/ci_cd_controller_spec.rb b/spec/controllers/projects/settings/ci_cd_controller_spec.rb
index 7dae9b85d78..a91c868cbaf 100644
--- a/spec/controllers/projects/settings/ci_cd_controller_spec.rb
+++ b/spec/controllers/projects/settings/ci_cd_controller_spec.rb
@@ -17,6 +17,23 @@ describe Projects::Settings::CiCdController do
       expect(response).to have_gitlab_http_status(200)
       expect(response).to render_template(:show)
     end
+
+    context 'with group runners' do
+      let(:group_runner) { create(:ci_runner) }
+      let(:parent_group) { create(:group) }
+      let(:group) { create(:group, runners: [group_runner], parent: parent_group) }
+      let(:other_project) { create(:project, group: group) }
+      let!(:project_runner) { create(:ci_runner, projects: [other_project]) }
+      let!(:shared_runner) { create(:ci_runner, :shared) }
+
+      it 'sets assignable project runners only' do
+        group.add_master(user)
+
+        get :show, namespace_id: project.namespace, project_id: project
+
+        expect(assigns(:assignable_runners)).to eq [project_runner]
+      end
+    end
   end
 
   describe '#reset_cache' do
diff --git a/spec/controllers/sessions_controller_spec.rb b/spec/controllers/sessions_controller_spec.rb
index 55bd4352bd3..555b186fe31 100644
--- a/spec/controllers/sessions_controller_spec.rb
+++ b/spec/controllers/sessions_controller_spec.rb
@@ -265,7 +265,7 @@ describe SessionsController do
     it 'redirects correctly for referer on same host with params' do
       search_path = '/search?search=seed_project'
       allow(controller.request).to receive(:referer)
-        .and_return('http://%{host}%{path}' % { host: Gitlab.config.gitlab.host, path: search_path })
+        .and_return('http://%{host}%{path}' % { host: 'test.host', path: search_path })
 
       get(:new, redirect_to_referer: :yes)
 
diff --git a/spec/controllers/users/terms_controller_spec.rb b/spec/controllers/users/terms_controller_spec.rb
new file mode 100644
index 00000000000..a744463413c
--- /dev/null
+++ b/spec/controllers/users/terms_controller_spec.rb
@@ -0,0 +1,81 @@
+require 'spec_helper'
+
+describe Users::TermsController do
+  let(:user) { create(:user) }
+  let(:term) { create(:term) }
+
+  before do
+    sign_in user
+  end
+
+  describe 'GET #index' do
+    it 'redirects when no terms exist' do
+      get :index
+
+      expect(response).to have_gitlab_http_status(:redirect)
+    end
+
+    it 'shows terms when they exist' do
+      term
+
+      expect(response).to have_gitlab_http_status(:success)
+    end
+  end
+
+  describe 'POST #accept' do
+    it 'saves that the user accepted the terms' do
+      post :accept, id: term.id
+
+      agreement = user.term_agreements.find_by(term: term)
+
+      expect(agreement.accepted).to eq(true)
+    end
+
+    it 'redirects to a path when specified' do
+      post :accept, id: term.id, redirect: groups_path
+
+      expect(response).to redirect_to(groups_path)
+    end
+
+    it 'redirects to the referer when no redirect specified' do
+      request.env["HTTP_REFERER"] = groups_url
+
+      post :accept, id: term.id
+
+      expect(response).to redirect_to(groups_path)
+    end
+
+    context 'redirecting to another domain' do
+      it 'is prevented when passing a redirect param' do
+        post :accept, id: term.id, redirect: '//example.com/random/path'
+
+        expect(response).to redirect_to(root_path)
+      end
+
+      it 'is prevented when redirecting to the referer' do
+        request.env["HTTP_REFERER"] = 'http://example.com/and/a/path'
+
+        post :accept, id: term.id
+
+        expect(response).to redirect_to(root_path)
+      end
+    end
+  end
+
+  describe 'POST #decline' do
+    it 'stores that the user declined the terms' do
+      post :decline, id: term.id
+
+      agreement = user.term_agreements.find_by(term: term)
+
+      expect(agreement.accepted).to eq(false)
+    end
+
+    it 'signs out the user' do
+      post :decline, id: term.id
+
+      expect(response).to redirect_to(root_path)
+      expect(assigns(:current_user)).to be_nil
+    end
+  end
+end