Merge branch 'handle-no-http-referer' into 'master'

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

See merge request !1646

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)

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

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

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

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

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

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
 

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

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
 

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

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

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

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

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

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
 

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

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

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) }
 

spec/controllers/invites_controller_spec.rb

+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

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