From 9bfc531ec611d108c45af239a1e5e016b892231b Mon Sep 17 00:00:00 2001
From: Stan Hu <stanhu@gmail.com>
Date: Tue, 20 Oct 2015 00:28:28 -0700
Subject: [PATCH] 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
---
CHANGELOG | 1 +
.../admin/broadcast_messages_controller.rb | 2 +-
app/controllers/admin/hooks_controller.rb | 2 +-
app/controllers/admin/users_controller.rb | 26 ++++++----
app/controllers/application_controller.rb | 4 ++
.../import/google_code_controller.rb | 6 +--
app/controllers/invites_controller.rb | 4 +-
.../profiles/notifications_controller.rb | 2 +-
app/controllers/profiles_controller.rb | 2 +-
.../projects/ci_services_controller.rb | 2 +-
.../projects/ci_web_hooks_controller.rb | 2 +-
.../projects/deploy_keys_controller.rb | 2 +-
app/controllers/projects/hooks_controller.rb | 2 +-
app/controllers/projects/issues_controller.rb | 2 +-
app/controllers/projects/notes_controller.rb | 4 +-
.../projects/project_members_controller.rb | 3 +-
.../projects/services_controller.rb | 4 +-
.../admin/users_controller_spec.rb | 26 ++++++++++
spec/controllers/invites_controller_spec.rb | 33 +++++++++++++
.../projects/services_controller_spec.rb | 47 +++++++++++++------
20 files changed, 133 insertions(+), 43 deletions(-)
create mode 100644 spec/controllers/invites_controller_spec.rb
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
--
GitLab