Redirect to a default path if HTTP_REFERER is not set

Safari 9.0 does not yet honor the HTML5 `origin-when-cross-origin` mode,
and it's possible load balancers/proxies strip the HTTP_REFERER from
the request header. In these cases, default to some default path.

Closes #3122

Closes https://github.com/gitlabhq/gitlabhq/issues/9731

20 files changed, 133 insertions, 43 deletions

diff --git a/CHANGELOG b/CHANGELOG
index 5671d8b1d81..8ea1161a4cf 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -2,6 +2,7 @@ Please view this file on the master branch, on stable branches it's out of date.
 
 v 8.2.0 (unreleased)
   - Fix duplicate repositories in GitHub import page (Stan Hu)
+  - Redirect to a default path if HTTP_REFERER is not set (Stan Hu)
   - Show last project commit to default branch on project home page
   - Highlight comment based on anchor in URL
   - Adds ability to remove the forked relationship from project settings screen. (Han Loong Liauw)
diff --git a/app/controllers/admin/broadcast_messages_controller.rb b/app/controllers/admin/broadcast_messages_controller.rb
index 0808024fc39..497c34f8f49 100644
--- a/app/controllers/admin/broadcast_messages_controller.rb
+++ b/app/controllers/admin/broadcast_messages_controller.rb
@@ -19,7 +19,7 @@ class Admin::BroadcastMessagesController < Admin::ApplicationController
     BroadcastMessage.find(params[:id]).destroy
 
     respond_to do |format|
-      format.html { redirect_to :back }
+      format.html { redirect_back_or_default(default: { action: 'index' }) }
       format.js { render nothing: true }
     end
   end
diff --git a/app/controllers/admin/hooks_controller.rb b/app/controllers/admin/hooks_controller.rb
index d670386f8c6..0bd19c49d8f 100644
--- a/app/controllers/admin/hooks_controller.rb
+++ b/app/controllers/admin/hooks_controller.rb
@@ -35,7 +35,7 @@ class Admin::HooksController < Admin::ApplicationController
     }
     @hook.execute(data, 'system_hooks')
 
-    redirect_to :back
+    redirect_back_or_default
   end
 
   def hook_params
diff --git a/app/controllers/admin/users_controller.rb b/app/controllers/admin/users_controller.rb
index 00f41a10dd1..c63d0793e31 100644
--- a/app/controllers/admin/users_controller.rb
+++ b/app/controllers/admin/users_controller.rb
@@ -33,33 +33,33 @@ class Admin::UsersController < Admin::ApplicationController
 
   def block
     if user.block
-      redirect_to :back, notice: "Successfully blocked"
+      redirect_back_or_admin_user(notice: "Successfully blocked")
     else
-      redirect_to :back, alert: "Error occurred. User was not blocked"
+      redirect_back_or_admin_user(alert: "Error occurred. User was not blocked")
     end
   end
 
   def unblock
     if user.activate
-      redirect_to :back, notice: "Successfully unblocked"
+      redirect_back_or_admin_user(notice: "Successfully unblocked")
     else
-      redirect_to :back, alert: "Error occurred. User was not unblocked"
+      redirect_back_or_admin_user(alert: "Error occurred. User was not unblocked")
     end
   end
 
   def unlock
     if user.unlock_access!
-      redirect_to :back, alert: "Successfully unlocked"
+      redirect_back_or_admin_user(alert: "Successfully unlocked")
     else
-      redirect_to :back, alert: "Error occurred. User was not unlocked"
+      redirect_back_or_admin_user(alert: "Error occurred. User was not unlocked")
     end
   end
 
   def confirm
     if user.confirm
-      redirect_to :back, notice: "Successfully confirmed"
+      redirect_back_or_admin_user(notice: "Successfully confirmed")
     else
-      redirect_to :back, alert: "Error occurred. User was not confirmed"
+      redirect_back_or_admin_user(alert: "Error occurred. User was not confirmed")
     end
   end
 
@@ -138,7 +138,7 @@ class Admin::UsersController < Admin::ApplicationController
     user.update_secondary_emails!
 
     respond_to do |format|
-      format.html { redirect_to :back, notice: "Successfully removed email." }
+      format.html { redirect_back_or_admin_user(notice: "Successfully removed email.") }
       format.js { render nothing: true }
     end
   end
@@ -157,4 +157,12 @@ class Admin::UsersController < Admin::ApplicationController
       :projects_limit, :can_create_group, :admin, :key_id
     )
   end
+
+  def redirect_back_or_admin_user(options = {})
+    redirect_back_or_default(default: default_route, options: options)
+  end
+
+  def default_route
+    [:admin, @user]
+  end
 end
diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index f0124c6bd60..865deb7d46a 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -33,6 +33,10 @@ class ApplicationController < ActionController::Base
     render_404
   end
 
+  def redirect_back_or_default(default: root_path, options: {})
+    redirect_to request.referer.present? ? :back : default, options
+  end
+
   protected
 
   # From https://github.com/plataformatec/devise/wiki/How-To:-Simple-Token-Authentication-Example
diff --git a/app/controllers/import/google_code_controller.rb b/app/controllers/import/google_code_controller.rb
index 41472a6fe6c..e0de31f2251 100644
--- a/app/controllers/import/google_code_controller.rb
+++ b/app/controllers/import/google_code_controller.rb
@@ -10,18 +10,18 @@ class Import::GoogleCodeController < Import::BaseController
     dump_file = params[:dump_file]
 
     unless dump_file.respond_to?(:read)
-      return redirect_to :back, alert: "You need to upload a Google Takeout archive."
+      return redirect_back_or_default(options: { alert: "You need to upload a Google Takeout archive." })
     end
 
     begin
       dump = JSON.parse(dump_file.read)
     rescue
-      return redirect_to :back, alert: "The uploaded file is not a valid Google Takeout archive."
+      return redirect_back_or_default(options: { alert: "The uploaded file is not a valid Google Takeout archive." })
     end
 
     client = Gitlab::GoogleCodeImport::Client.new(dump)
     unless client.valid?
-      return redirect_to :back, alert: "The uploaded file is not a valid Google Takeout archive."
+      return redirect_back_or_default(options: { alert: "The uploaded file is not a valid Google Takeout archive." })
     end
 
     session[:google_code_dump] = dump
diff --git a/app/controllers/invites_controller.rb b/app/controllers/invites_controller.rb
index 8ef10a17f55..94bb108c5f5 100644
--- a/app/controllers/invites_controller.rb
+++ b/app/controllers/invites_controller.rb
@@ -14,7 +14,7 @@ class InvitesController < ApplicationController
 
       redirect_to path, notice: "You have been granted #{member.human_access} access to #{label}."
     else
-      redirect_to :back, alert: "The invitation could not be accepted."
+      redirect_back_or_default(options: { alert: "The invitation could not be accepted." })
     end
   end
 
@@ -31,7 +31,7 @@ class InvitesController < ApplicationController
 
       redirect_to path, notice: "You have declined the invitation to join #{label}."
     else
-      redirect_to :back, alert: "The invitation could not be declined."
+      redirect_back_or_default(options: { alert: "The invitation could not be declined." })
     end
   end
 
diff --git a/app/controllers/profiles/notifications_controller.rb b/app/controllers/profiles/notifications_controller.rb
index 22423651c17..1fd1d6882df 100644
--- a/app/controllers/profiles/notifications_controller.rb
+++ b/app/controllers/profiles/notifications_controller.rb
@@ -29,7 +29,7 @@ class Profiles::NotificationsController < Profiles::ApplicationController
           flash[:alert] = "Failed to save new settings"
         end
 
-        redirect_to :back
+        redirect_back_or_default(default: profile_notifications_path)
       end
 
       format.js
diff --git a/app/controllers/profiles_controller.rb b/app/controllers/profiles_controller.rb
index 26a4de15462..8da7b4d50ea 100644
--- a/app/controllers/profiles_controller.rb
+++ b/app/controllers/profiles_controller.rb
@@ -26,7 +26,7 @@ class ProfilesController < Profiles::ApplicationController
     end
 
     respond_to do |format|
-      format.html { redirect_to :back }
+      format.html { redirect_back_or_default(default: { action: 'show' }) }
     end
   end
 
diff --git a/app/controllers/projects/ci_services_controller.rb b/app/controllers/projects/ci_services_controller.rb
index 6d2756eba3d..406f313ae79 100644
--- a/app/controllers/projects/ci_services_controller.rb
+++ b/app/controllers/projects/ci_services_controller.rb
@@ -30,7 +30,7 @@ class Projects::CiServicesController < Projects::ApplicationController
       message = { alert: 'We tried to test the service but error occurred' }
     end
 
-    redirect_to :back, message
+    redirect_back_or_default(options: message)
   end
 
   private
diff --git a/app/controllers/projects/ci_web_hooks_controller.rb b/app/controllers/projects/ci_web_hooks_controller.rb
index 7f40ddcb3f3..a2d470d4a69 100644
--- a/app/controllers/projects/ci_web_hooks_controller.rb
+++ b/app/controllers/projects/ci_web_hooks_controller.rb
@@ -24,7 +24,7 @@ class Projects::CiWebHooksController < Projects::ApplicationController
   def test
     Ci::TestHookService.new.execute(hook, current_user)
 
-    redirect_to :back
+    redirect_back_or_default(default: { action: 'index' })
   end
 
   def destroy
diff --git a/app/controllers/projects/deploy_keys_controller.rb b/app/controllers/projects/deploy_keys_controller.rb
index 40e2b37912b..7d09288bc80 100644
--- a/app/controllers/projects/deploy_keys_controller.rb
+++ b/app/controllers/projects/deploy_keys_controller.rb
@@ -46,7 +46,7 @@ class Projects::DeployKeysController < Projects::ApplicationController
   def disable
     @project.deploy_keys_projects.find_by(deploy_key_id: params[:id]).destroy
 
-    redirect_to :back
+    redirect_back_or_default(default: { action: 'index' })
   end
 
   protected
diff --git a/app/controllers/projects/hooks_controller.rb b/app/controllers/projects/hooks_controller.rb
index 4e5b4125f5a..c7569541899 100644
--- a/app/controllers/projects/hooks_controller.rb
+++ b/app/controllers/projects/hooks_controller.rb
@@ -37,7 +37,7 @@ class Projects::HooksController < Projects::ApplicationController
       flash[:alert] = 'Hook execution failed. Ensure the project has commits.'
     end
 
-    redirect_to :back
+    redirect_back_or_default(default: { action: 'index' })
   end
 
   def destroy
diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb
index cc8321d97ad..e767efbdc0c 100644
--- a/app/controllers/projects/issues_controller.rb
+++ b/app/controllers/projects/issues_controller.rb
@@ -106,7 +106,7 @@ class Projects::IssuesController < Projects::ApplicationController
 
   def bulk_update
     result = Issues::BulkUpdateService.new(project, current_user, bulk_update_params).execute
-    redirect_to :back, notice: "#{result[:count]} issues updated"
+    redirect_back_or_default(default: { action: 'index' }, options: { notice: "#{result[:count]} issues updated" })
   end
 
   def toggle_subscription
diff --git a/app/controllers/projects/notes_controller.rb b/app/controllers/projects/notes_controller.rb
index 0f5d82ce133..41cd08c93c6 100644
--- a/app/controllers/projects/notes_controller.rb
+++ b/app/controllers/projects/notes_controller.rb
@@ -25,7 +25,7 @@ class Projects::NotesController < Projects::ApplicationController
 
     respond_to do |format|
       format.json { render_note_json(@note) }
-      format.html { redirect_to :back }
+      format.html { redirect_back_or_default }
     end
   end
 
@@ -34,7 +34,7 @@ class Projects::NotesController < Projects::ApplicationController
 
     respond_to do |format|
       format.json { render_note_json(@note) }
-      format.html { redirect_to :back }
+      format.html { redirect_back_or_default }
     end
   end
 
diff --git a/app/controllers/projects/project_members_controller.rb b/app/controllers/projects/project_members_controller.rb
index cf73bc01c8f..9de5269cd25 100644
--- a/app/controllers/projects/project_members_controller.rb
+++ b/app/controllers/projects/project_members_controller.rb
@@ -72,7 +72,8 @@ class Projects::ProjectMembersController < Projects::ApplicationController
 
   def leave
     if @project.namespace == current_user.namespace
-      return redirect_to(:back, alert: 'You can not leave your own project. Transfer or delete the project.')
+      message = 'You can not leave your own project. Transfer or delete the project.'
+      return redirect_back_or_default(default: { action: 'index' }, options: { alert: message })
     end
 
     @project.project_members.find_by(user_id: current_user).destroy
diff --git a/app/controllers/projects/services_controller.rb b/app/controllers/projects/services_controller.rb
index 129068ef019..42dbb497e01 100644
--- a/app/controllers/projects/services_controller.rb
+++ b/app/controllers/projects/services_controller.rb
@@ -12,7 +12,7 @@ class Projects::ServicesController < Projects::ApplicationController
 
   # Parameters to ignore if no value is specified
   FILTER_BLANK_PARAMS = [:password]
-  
+
   # Authorize
   before_action :authorize_admin_project!
   before_action :service, only: [:edit, :update, :test]
@@ -52,7 +52,7 @@ class Projects::ServicesController < Projects::ApplicationController
       message = { alert: error_message }
     end
 
-    redirect_to :back, message
+    redirect_back_or_default(options: message)
   end
 
   private
diff --git a/spec/controllers/admin/users_controller_spec.rb b/spec/controllers/admin/users_controller_spec.rb
index 7168db117d6..fcbe62cace8 100644
--- a/spec/controllers/admin/users_controller_spec.rb
+++ b/spec/controllers/admin/users_controller_spec.rb
@@ -37,6 +37,32 @@ describe Admin::UsersController do
     end
   end
 
+  describe 'PUT block/:id' do
+    let(:user) { create(:user) }
+
+    it 'blocks user' do
+      put :block, id: user.username
+      user.reload
+      expect(user.blocked?).to be_truthy
+      expect(flash[:notice]).to eq 'Successfully blocked'
+    end
+  end
+
+  describe 'PUT unblock/:id' do
+    let(:user) { create(:user) }
+
+    before do
+      user.block
+    end
+
+    it 'unblocks user' do
+      put :unblock, id: user.username
+      user.reload
+      expect(user.blocked?).to be_falsey
+      expect(flash[:notice]).to eq 'Successfully unblocked'
+    end
+  end
+
   describe 'PUT unlock/:id' do
     let(:user) { create(:user) }
 
diff --git a/spec/controllers/invites_controller_spec.rb b/spec/controllers/invites_controller_spec.rb
new file mode 100644
index 00000000000..3c6e54839b5
--- /dev/null
+++ b/spec/controllers/invites_controller_spec.rb
@@ -0,0 +1,33 @@
+require 'spec_helper'
+
+describe InvitesController do
+  let(:token) { '123456' }
+  let(:user) { create(:user) }
+  let(:member) { create(:project_member, invite_token: token, invite_email: 'test@abc.com', user: user) }
+
+  before do
+    controller.instance_variable_set(:@member, member)
+    sign_in(user)
+  end
+
+  describe 'GET #accept' do
+    it 'accepts user' do
+      get :accept, id: token
+      member.reload
+
+      expect(response.status).to eq(302)
+      expect(member.user).to eq(user)
+      expect(flash[:notice]).to include 'You have been granted'
+    end
+  end
+
+  describe 'GET #decline' do
+    it 'declines user' do
+      get :decline, id: token
+      expect{member.reload}.to raise_error ActiveRecord::RecordNotFound
+
+      expect(response.status).to eq(302)
+      expect(flash[:notice]).to include 'You have declined the invitation to join'
+    end
+  end
+end
diff --git a/spec/controllers/projects/services_controller_spec.rb b/spec/controllers/projects/services_controller_spec.rb
index d4ecd98e12d..ccd8c741c83 100644
--- a/spec/controllers/projects/services_controller_spec.rb
+++ b/spec/controllers/projects/services_controller_spec.rb
@@ -10,26 +10,43 @@ describe Projects::ServicesController do
     project.team << [user, :master]
     controller.instance_variable_set(:@project, project)
     controller.instance_variable_set(:@service, service)
-    request.env["HTTP_REFERER"] = "/"
   end
 
-  describe "#test" do
-    context 'success' do
-      it "should redirect and show success message" do
-        expect(service).to receive(:test).and_return({ success: true, result: 'done' })
-        get :test, namespace_id: project.namespace.id, project_id: project.id, id: service.id, format: :html
-        expect(response.status).to redirect_to('/')
-        expect(flash[:notice]).to eq('We sent a request to the provided URL')
-      end
+  shared_examples_for 'services controller' do |referrer|
+    before do
+      request.env["HTTP_REFERER"] = referrer
     end
 
-    context 'failure' do
-      it "should redirect and show failure message" do
-        expect(service).to receive(:test).and_return({ success: false, result: 'Bad test' })
-        get :test, namespace_id: project.namespace.id, project_id: project.id, id: service.id, format: :html
-        expect(response.status).to redirect_to('/')
-        expect(flash[:alert]).to eq('We tried to send a request to the provided URL but an error occurred: Bad test')
+    describe "#test" do
+      context 'success' do
+        it "should redirect and show success message" do
+          expect(service).to receive(:test).and_return({ success: true, result: 'done' })
+          get :test, namespace_id: project.namespace.id, project_id: project.id, id: service.id, format: :html
+          expect(response.status).to redirect_to('/')
+          expect(flash[:notice]).to eq('We sent a request to the provided URL')
+        end
+      end
+
+      context 'failure' do
+        it "should redirect and show failure message" do
+          expect(service).to receive(:test).and_return({ success: false, result: 'Bad test' })
+          get :test, namespace_id: project.namespace.id, project_id: project.id, id: service.id, format: :html
+          expect(response.status).to redirect_to('/')
+          expect(flash[:alert]).to eq('We tried to send a request to the provided URL but an error occurred: Bad test')
+        end
       end
     end
   end
+
+  describe 'referrer defined' do
+    it_should_behave_like 'services controller' do
+      let!(:referrer) { "/" }
+    end
+  end
+
+  describe 'referrer undefined' do
+    it_should_behave_like 'services controller' do
+      let!(:referrer) { nil }
+    end
+  end
 end