Commit 595fc4d9 authored by Dmitriy Zaporozhets's avatar Dmitriy Zaporozhets Committed by Robert Speicher

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
parent acc1af5f
...@@ -2,6 +2,7 @@ Please view this file on the master branch, on stable branches it's out of date. ...@@ -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) v 8.2.0 (unreleased)
- Fix duplicate repositories in GitHub import page (Stan Hu) - 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 - Show last project commit to default branch on project home page
- Highlight comment based on anchor in URL - Highlight comment based on anchor in URL
- Adds ability to remove the forked relationship from project settings screen. (Han Loong Liauw) - Adds ability to remove the forked relationship from project settings screen. (Han Loong Liauw)
......
...@@ -19,7 +19,7 @@ class Admin::BroadcastMessagesController < Admin::ApplicationController ...@@ -19,7 +19,7 @@ class Admin::BroadcastMessagesController < Admin::ApplicationController
BroadcastMessage.find(params[:id]).destroy BroadcastMessage.find(params[:id]).destroy
respond_to do |format| respond_to do |format|
format.html { redirect_to :back } format.html { redirect_back_or_default(default: { action: 'index' }) }
format.js { render nothing: true } format.js { render nothing: true }
end end
end end
......
...@@ -35,7 +35,7 @@ class Admin::HooksController < Admin::ApplicationController ...@@ -35,7 +35,7 @@ class Admin::HooksController < Admin::ApplicationController
} }
@hook.execute(data, 'system_hooks') @hook.execute(data, 'system_hooks')
redirect_to :back redirect_back_or_default
end end
def hook_params def hook_params
......
...@@ -33,33 +33,33 @@ class Admin::UsersController < Admin::ApplicationController ...@@ -33,33 +33,33 @@ class Admin::UsersController < Admin::ApplicationController
def block def block
if user.block if user.block
redirect_to :back, notice: "Successfully blocked" redirect_back_or_admin_user(notice: "Successfully blocked")
else 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
end end
def unblock def unblock
if user.activate if user.activate
redirect_to :back, notice: "Successfully unblocked" redirect_back_or_admin_user(notice: "Successfully unblocked")
else 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
end end
def unlock def unlock
if user.unlock_access! if user.unlock_access!
redirect_to :back, alert: "Successfully unlocked" redirect_back_or_admin_user(alert: "Successfully unlocked")
else 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
end end
def confirm def confirm
if user.confirm if user.confirm
redirect_to :back, notice: "Successfully confirmed" redirect_back_or_admin_user(notice: "Successfully confirmed")
else 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
end end
...@@ -138,7 +138,7 @@ class Admin::UsersController < Admin::ApplicationController ...@@ -138,7 +138,7 @@ class Admin::UsersController < Admin::ApplicationController
user.update_secondary_emails! user.update_secondary_emails!
respond_to do |format| 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 } format.js { render nothing: true }
end end
end end
...@@ -157,4 +157,12 @@ class Admin::UsersController < Admin::ApplicationController ...@@ -157,4 +157,12 @@ class Admin::UsersController < Admin::ApplicationController
:projects_limit, :can_create_group, :admin, :key_id :projects_limit, :can_create_group, :admin, :key_id
) )
end 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 end
...@@ -33,6 +33,10 @@ class ApplicationController < ActionController::Base ...@@ -33,6 +33,10 @@ class ApplicationController < ActionController::Base
render_404 render_404
end end
def redirect_back_or_default(default: root_path, options: {})
redirect_to request.referer.present? ? :back : default, options
end
protected protected
# From https://github.com/plataformatec/devise/wiki/How-To:-Simple-Token-Authentication-Example # From https://github.com/plataformatec/devise/wiki/How-To:-Simple-Token-Authentication-Example
......
...@@ -10,18 +10,18 @@ class Import::GoogleCodeController < Import::BaseController ...@@ -10,18 +10,18 @@ class Import::GoogleCodeController < Import::BaseController
dump_file = params[:dump_file] dump_file = params[:dump_file]
unless dump_file.respond_to?(:read) 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 end
begin begin
dump = JSON.parse(dump_file.read) dump = JSON.parse(dump_file.read)
rescue 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 end
client = Gitlab::GoogleCodeImport::Client.new(dump) client = Gitlab::GoogleCodeImport::Client.new(dump)
unless client.valid? 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 end
session[:google_code_dump] = dump session[:google_code_dump] = dump
......
...@@ -14,7 +14,7 @@ class InvitesController < ApplicationController ...@@ -14,7 +14,7 @@ class InvitesController < ApplicationController
redirect_to path, notice: "You have been granted #{member.human_access} access to #{label}." redirect_to path, notice: "You have been granted #{member.human_access} access to #{label}."
else 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
end end
...@@ -31,7 +31,7 @@ class InvitesController < ApplicationController ...@@ -31,7 +31,7 @@ class InvitesController < ApplicationController
redirect_to path, notice: "You have declined the invitation to join #{label}." redirect_to path, notice: "You have declined the invitation to join #{label}."
else 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
end end
......
...@@ -29,7 +29,7 @@ class Profiles::NotificationsController < Profiles::ApplicationController ...@@ -29,7 +29,7 @@ class Profiles::NotificationsController < Profiles::ApplicationController
flash[:alert] = "Failed to save new settings" flash[:alert] = "Failed to save new settings"
end end
redirect_to :back redirect_back_or_default(default: profile_notifications_path)
end end
format.js format.js
......
...@@ -26,7 +26,7 @@ class ProfilesController < Profiles::ApplicationController ...@@ -26,7 +26,7 @@ class ProfilesController < Profiles::ApplicationController
end end
respond_to do |format| respond_to do |format|
format.html { redirect_to :back } format.html { redirect_back_or_default(default: { action: 'show' }) }
end end
end end
......
...@@ -30,7 +30,7 @@ class Projects::CiServicesController < Projects::ApplicationController ...@@ -30,7 +30,7 @@ class Projects::CiServicesController < Projects::ApplicationController
message = { alert: 'We tried to test the service but error occurred' } message = { alert: 'We tried to test the service but error occurred' }
end end
redirect_to :back, message redirect_back_or_default(options: message)
end end
private private
......
...@@ -24,7 +24,7 @@ class Projects::CiWebHooksController < Projects::ApplicationController ...@@ -24,7 +24,7 @@ class Projects::CiWebHooksController < Projects::ApplicationController
def test def test
Ci::TestHookService.new.execute(hook, current_user) Ci::TestHookService.new.execute(hook, current_user)
redirect_to :back redirect_back_or_default(default: { action: 'index' })
end end
def destroy def destroy
......
...@@ -46,7 +46,7 @@ class Projects::DeployKeysController < Projects::ApplicationController ...@@ -46,7 +46,7 @@ class Projects::DeployKeysController < Projects::ApplicationController
def disable def disable
@project.deploy_keys_projects.find_by(deploy_key_id: params[:id]).destroy @project.deploy_keys_projects.find_by(deploy_key_id: params[:id]).destroy
redirect_to :back redirect_back_or_default(default: { action: 'index' })
end end
protected protected
......
...@@ -37,7 +37,7 @@ class Projects::HooksController < Projects::ApplicationController ...@@ -37,7 +37,7 @@ class Projects::HooksController < Projects::ApplicationController
flash[:alert] = 'Hook execution failed. Ensure the project has commits.' flash[:alert] = 'Hook execution failed. Ensure the project has commits.'
end end
redirect_to :back redirect_back_or_default(default: { action: 'index' })
end end
def destroy def destroy
......
...@@ -106,7 +106,7 @@ class Projects::IssuesController < Projects::ApplicationController ...@@ -106,7 +106,7 @@ class Projects::IssuesController < Projects::ApplicationController
def bulk_update def bulk_update
result = Issues::BulkUpdateService.new(project, current_user, bulk_update_params).execute 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 end
def toggle_subscription def toggle_subscription
......
...@@ -25,7 +25,7 @@ class Projects::NotesController < Projects::ApplicationController ...@@ -25,7 +25,7 @@ class Projects::NotesController < Projects::ApplicationController
respond_to do |format| respond_to do |format|
format.json { render_note_json(@note) } format.json { render_note_json(@note) }
format.html { redirect_to :back } format.html { redirect_back_or_default }
end end
end end
...@@ -34,7 +34,7 @@ class Projects::NotesController < Projects::ApplicationController ...@@ -34,7 +34,7 @@ class Projects::NotesController < Projects::ApplicationController
respond_to do |format| respond_to do |format|
format.json { render_note_json(@note) } format.json { render_note_json(@note) }
format.html { redirect_to :back } format.html { redirect_back_or_default }
end end
end end
......
...@@ -72,7 +72,8 @@ class Projects::ProjectMembersController < Projects::ApplicationController ...@@ -72,7 +72,8 @@ class Projects::ProjectMembersController < Projects::ApplicationController
def leave def leave
if @project.namespace == current_user.namespace 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 end
@project.project_members.find_by(user_id: current_user).destroy @project.project_members.find_by(user_id: current_user).destroy
......
...@@ -12,7 +12,7 @@ class Projects::ServicesController < Projects::ApplicationController ...@@ -12,7 +12,7 @@ class Projects::ServicesController < Projects::ApplicationController
# Parameters to ignore if no value is specified # Parameters to ignore if no value is specified
FILTER_BLANK_PARAMS = [:password] FILTER_BLANK_PARAMS = [:password]
# Authorize # Authorize
before_action :authorize_admin_project! before_action :authorize_admin_project!
before_action :service, only: [:edit, :update, :test] before_action :service, only: [:edit, :update, :test]
...@@ -52,7 +52,7 @@ class Projects::ServicesController < Projects::ApplicationController ...@@ -52,7 +52,7 @@ class Projects::ServicesController < Projects::ApplicationController
message = { alert: error_message } message = { alert: error_message }
end end
redirect_to :back, message redirect_back_or_default(options: message)
end end
private private
......
...@@ -37,6 +37,32 @@ describe Admin::UsersController do ...@@ -37,6 +37,32 @@ describe Admin::UsersController do
end end
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 describe 'PUT unlock/:id' do
let(:user) { create(:user) } let(:user) { create(:user) }
......
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
...@@ -10,26 +10,43 @@ describe Projects::ServicesController do ...@@ -10,26 +10,43 @@ describe Projects::ServicesController do
project.team << [user, :master] project.team << [user, :master]
controller.instance_variable_set(:@project, project) controller.instance_variable_set(:@project, project)
controller.instance_variable_set(:@service, service) controller.instance_variable_set(:@service, service)
request.env["HTTP_REFERER"] = "/"
end end
describe "#test" do shared_examples_for 'services controller' do |referrer|
context 'success' do before do
it "should redirect and show success message" do request.env["HTTP_REFERER"] = referrer
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 end
context 'failure' do describe "#test" do
it "should redirect and show failure message" do context 'success' do
expect(service).to receive(:test).and_return({ success: false, result: 'Bad test' }) it "should redirect and show success message" do
get :test, namespace_id: project.namespace.id, project_id: project.id, id: service.id, format: :html expect(service).to receive(:test).and_return({ success: true, result: 'done' })
expect(response.status).to redirect_to('/') get :test, namespace_id: project.namespace.id, project_id: project.id, id: service.id, format: :html
expect(flash[:alert]).to eq('We tried to send a request to the provided URL but an error occurred: Bad test') 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 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 end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment