From 67f365843186c4a2214b5d4bcfae47d59aa86309 Mon Sep 17 00:00:00 2001 From: Tom Hughes Date: Sat, 12 Dec 2015 16:08:09 +0000 Subject: [PATCH] Preserve URL fragments through external authentication Fixes #1102 --- app/assets/javascripts/login.js | 7 ++++ app/controllers/user_controller.rb | 23 +++++++---- test/integration/user_login_test.rb | 62 ++++++++++++++--------------- 3 files changed, 53 insertions(+), 39 deletions(-) diff --git a/app/assets/javascripts/login.js b/app/assets/javascripts/login.js index 755c3bf65..b7a540f6e 100644 --- a/app/assets/javascripts/login.js +++ b/app/assets/javascripts/login.js @@ -4,6 +4,13 @@ $(document).ready(function() { $("#referer").val($("#referer").val() + window.location.hash); } + // Attach referer to authentication buttons + $(".auth_button").each(function () { + var params = querystring.parse(this.search.substring(1)); + params.referer = $("#referer").val(); + this.search = querystring.stringify(params); + }); + // Add click handler to show OpenID field $("#openid_open_url").click(function() { $("#openid_url").val("http://"); diff --git a/app/controllers/user_controller.rb b/app/controllers/user_controller.rb index 6390ff19e..14e8c9460 100644 --- a/app/controllers/user_controller.rb +++ b/app/controllers/user_controller.rb @@ -257,7 +257,7 @@ class UserController < ApplicationController password_authentication(params[:username], params[:password]) elsif params[:openid_url].present? session[:remember_me] ||= params[:remember_me_openid] - redirect_to auth_url("openid", params[:openid_url]) + redirect_to auth_url("openid", params[:openid_url], params[:referer]) end end @@ -516,7 +516,7 @@ class UserController < ApplicationController when "pending" then unconfirmed_login(user) when "active", "confirmed" then - successful_login(user) + successful_login(user, env["omniauth.params"]["referer"]) when "suspended" then failed_login t("user.login.account is suspended", :webmaster => "mailto:webmaster@openstreetmap.org") else @@ -569,12 +569,19 @@ class UserController < ApplicationController ## # return the URL to use for authentication - def auth_url(provider, uid) - if provider == "openid" - auth_path(:provider => "openid", :openid_url => openid_expand_url(uid), :origin => request.path) + def auth_url(provider, uid, referer = nil) + params = { :provider => provider } + + params[:openid_url] = openid_expand_url(uid) if provider == "openid" + + if referer.nil? + params[:origin] = request.path else - auth_path(:provider => provider, :origin => request.path) + params[:origin] = request.path + "?referer=" + CGI.escape(referer) + params[:referer] = referer end + + auth_path(params) end ## @@ -596,11 +603,11 @@ class UserController < ApplicationController ## # process a successful login - def successful_login(user) + def successful_login(user, referer = nil) session[:user] = user.id session_expires_after 28.days if session[:remember_me] - target = session[:referer] || url_for(:controller => :site, :action => :index) + target = referer || session[:referer] || url_for(:controller => :site, :action => :index) # The user is logged in, so decide where to send them: # diff --git a/test/integration/user_login_test.rb b/test/integration/user_login_test.rb index a48933cf4..8037b93a9 100644 --- a/test/integration/user_login_test.rb +++ b/test/integration/user_login_test.rb @@ -740,10 +740,10 @@ class UserLoginTest < ActionDispatch::IntegrationTest assert_template "user/login" post "/login", :openid_url => "http://localhost:1123/john.doe", :referer => "/history" assert_response :redirect - assert_redirected_to auth_path(:provider => "openid", :openid_url => "http://localhost:1123/john.doe", :origin => "/login") + assert_redirected_to auth_path(:provider => "openid", :openid_url => "http://localhost:1123/john.doe", :origin => "/login?referer=%2Fhistory", :referer => "/history") follow_redirect! assert_response :redirect - assert_redirected_to auth_success_path(:provider => "openid", :openid_url => "http://localhost:1123/john.doe", :origin => "/login") + assert_redirected_to auth_success_path(:provider => "openid", :openid_url => "http://localhost:1123/john.doe", :origin => "/login?referer=%2Fhistory", :referer => "/history") follow_redirect! assert_response :redirect follow_redirect! @@ -763,10 +763,10 @@ class UserLoginTest < ActionDispatch::IntegrationTest assert_template "user/login" post "/login", :openid_url => "http://localhost:1123/john.doe", :remember_me_openid => true, :referer => "/history" assert_response :redirect - assert_redirected_to auth_path(:provider => "openid", :openid_url => "http://localhost:1123/john.doe", :origin => "/login") + assert_redirected_to auth_path(:provider => "openid", :openid_url => "http://localhost:1123/john.doe", :origin => "/login?referer=%2Fhistory", :referer => "/history") follow_redirect! assert_response :redirect - assert_redirected_to auth_success_path(:provider => "openid", :openid_url => "http://localhost:1123/john.doe", :origin => "/login") + assert_redirected_to auth_success_path(:provider => "openid", :openid_url => "http://localhost:1123/john.doe", :origin => "/login?referer=%2Fhistory", :referer => "/history") follow_redirect! assert_response :redirect follow_redirect! @@ -787,13 +787,13 @@ class UserLoginTest < ActionDispatch::IntegrationTest assert_template "user/login" post "/login", :openid_url => "http://localhost:1123/john.doe", :referer => "/history" assert_response :redirect - assert_redirected_to auth_path(:provider => "openid", :openid_url => "http://localhost:1123/john.doe", :origin => "/login") + assert_redirected_to auth_path(:provider => "openid", :openid_url => "http://localhost:1123/john.doe", :origin => "/login?referer=%2Fhistory", :referer => "/history") follow_redirect! assert_response :redirect - assert_redirected_to auth_success_path(:provider => "openid", :openid_url => "http://localhost:1123/john.doe", :origin => "/login") + assert_redirected_to auth_success_path(:provider => "openid", :openid_url => "http://localhost:1123/john.doe", :origin => "/login?referer=%2Fhistory", :referer => "/history") follow_redirect! assert_response :redirect - assert_redirected_to auth_failure_path(:strategy => "openid", :message => "connection_failed", :origin => "/login") + assert_redirected_to auth_failure_path(:strategy => "openid", :message => "connection_failed", :origin => "/login?referer=%2Fhistory") follow_redirect! assert_response :redirect follow_redirect! @@ -814,13 +814,13 @@ class UserLoginTest < ActionDispatch::IntegrationTest assert_template "user/login" post "/login", :openid_url => "http://localhost:1123/john.doe", :referer => "/history" assert_response :redirect - assert_redirected_to auth_path(:provider => "openid", :openid_url => "http://localhost:1123/john.doe", :origin => "/login") + assert_redirected_to auth_path(:provider => "openid", :openid_url => "http://localhost:1123/john.doe", :origin => "/login?referer=%2Fhistory", :referer => "/history") follow_redirect! assert_response :redirect - assert_redirected_to auth_success_path(:provider => "openid", :openid_url => "http://localhost:1123/john.doe", :origin => "/login") + assert_redirected_to auth_success_path(:provider => "openid", :openid_url => "http://localhost:1123/john.doe", :origin => "/login?referer=%2Fhistory", :referer => "/history") follow_redirect! assert_response :redirect - assert_redirected_to auth_failure_path(:strategy => "openid", :message => "invalid_credentials", :origin => "/login") + assert_redirected_to auth_failure_path(:strategy => "openid", :message => "invalid_credentials", :origin => "/login?referer=%2Fhistory") follow_redirect! assert_response :redirect follow_redirect! @@ -841,10 +841,10 @@ class UserLoginTest < ActionDispatch::IntegrationTest assert_template "user/login" post "/login", :openid_url => "http://localhost:1123/fred.bloggs", :referer => "/history" assert_response :redirect - assert_redirected_to auth_path(:provider => "openid", :openid_url => "http://localhost:1123/fred.bloggs", :origin => "/login") + assert_redirected_to auth_path(:provider => "openid", :openid_url => "http://localhost:1123/fred.bloggs", :origin => "/login?referer=%2Fhistory", :referer => "/history") follow_redirect! assert_response :redirect - assert_redirected_to auth_success_path(:provider => "openid", :openid_url => "http://localhost:1123/fred.bloggs", :origin => "/login") + assert_redirected_to auth_success_path(:provider => "openid", :openid_url => "http://localhost:1123/fred.bloggs", :origin => "/login?referer=%2Fhistory", :referer => "/history") follow_redirect! assert_response :redirect follow_redirect! @@ -864,7 +864,7 @@ class UserLoginTest < ActionDispatch::IntegrationTest follow_redirect! assert_response :success assert_template "user/login" - get auth_path(:provider => "google", :origin => "/login") + get auth_path(:provider => "google", :origin => "/login?referer=%2Fhistory", :referer => "/history") assert_response :redirect assert_redirected_to auth_success_path(:provider => "google") follow_redirect! @@ -884,12 +884,12 @@ class UserLoginTest < ActionDispatch::IntegrationTest follow_redirect! assert_response :success assert_template "user/login" - get auth_path(:provider => "google", :origin => "/login") + get auth_path(:provider => "google", :origin => "/login?referer=%2Fhistory", :referer => "/history") assert_response :redirect assert_redirected_to auth_success_path(:provider => "google") follow_redirect! assert_response :redirect - assert_redirected_to auth_failure_path(:strategy => "google", :message => "connection_failed", :origin => "/login") + assert_redirected_to auth_failure_path(:strategy => "google", :message => "connection_failed", :origin => "/login?referer=%2Fhistory") follow_redirect! assert_response :redirect follow_redirect! @@ -908,12 +908,12 @@ class UserLoginTest < ActionDispatch::IntegrationTest follow_redirect! assert_response :success assert_template "user/login" - get auth_path(:provider => "google", :origin => "/login") + get auth_path(:provider => "google", :origin => "/login?referer=%2Fhistory", :referer => "/history") assert_response :redirect assert_redirected_to auth_success_path(:provider => "google") follow_redirect! assert_response :redirect - assert_redirected_to auth_failure_path(:strategy => "google", :message => "invalid_credentials", :origin => "/login") + assert_redirected_to auth_failure_path(:strategy => "google", :message => "invalid_credentials", :origin => "/login?referer=%2Fhistory") follow_redirect! assert_response :redirect follow_redirect! @@ -934,7 +934,7 @@ class UserLoginTest < ActionDispatch::IntegrationTest follow_redirect! assert_response :success assert_template "user/login" - get auth_path(:provider => "google", :origin => "/login") + get auth_path(:provider => "google", :origin => "/login?referer=%2Fhistory", :referer => "/history") assert_response :redirect assert_redirected_to auth_success_path(:provider => "google") follow_redirect! @@ -956,7 +956,7 @@ class UserLoginTest < ActionDispatch::IntegrationTest follow_redirect! assert_response :success assert_template "user/login" - get auth_path(:provider => "google", :origin => "/login") + get auth_path(:provider => "google", :origin => "/login?referer=%2Fhistory", :referer => "/history") assert_response :redirect assert_redirected_to auth_success_path(:provider => "google") follow_redirect! @@ -980,7 +980,7 @@ class UserLoginTest < ActionDispatch::IntegrationTest follow_redirect! assert_response :success assert_template "user/login" - get auth_path(:provider => "facebook", :origin => "/login") + get auth_path(:provider => "facebook", :origin => "/login?referer=%2Fhistory", :referer => "/history") assert_response :redirect assert_redirected_to auth_success_path(:provider => "facebook") follow_redirect! @@ -1000,12 +1000,12 @@ class UserLoginTest < ActionDispatch::IntegrationTest follow_redirect! assert_response :success assert_template "user/login" - get auth_path(:provider => "facebook", :origin => "/login") + get auth_path(:provider => "facebook", :origin => "/login?referer=%2Fhistory", :referer => "/history") assert_response :redirect assert_redirected_to auth_success_path(:provider => "facebook") follow_redirect! assert_response :redirect - assert_redirected_to auth_failure_path(:strategy => "facebook", :message => "connection_failed", :origin => "/login") + assert_redirected_to auth_failure_path(:strategy => "facebook", :message => "connection_failed", :origin => "/login?referer=%2Fhistory") follow_redirect! assert_response :redirect follow_redirect! @@ -1024,12 +1024,12 @@ class UserLoginTest < ActionDispatch::IntegrationTest follow_redirect! assert_response :success assert_template "user/login" - get auth_path(:provider => "facebook", :origin => "/login") + get auth_path(:provider => "facebook", :origin => "/login?referer=%2Fhistory", :referer => "/history") assert_response :redirect assert_redirected_to auth_success_path(:provider => "facebook") follow_redirect! assert_response :redirect - assert_redirected_to auth_failure_path(:strategy => "facebook", :message => "invalid_credentials", :origin => "/login") + assert_redirected_to auth_failure_path(:strategy => "facebook", :message => "invalid_credentials", :origin => "/login?referer=%2Fhistory") follow_redirect! assert_response :redirect follow_redirect! @@ -1048,7 +1048,7 @@ class UserLoginTest < ActionDispatch::IntegrationTest follow_redirect! assert_response :success assert_template "user/login" - get auth_path(:provider => "facebook", :origin => "/login") + get auth_path(:provider => "facebook", :origin => "/login?referer=%2Fhistory", :referer => "/history") assert_response :redirect assert_redirected_to auth_success_path(:provider => "facebook") follow_redirect! @@ -1068,7 +1068,7 @@ class UserLoginTest < ActionDispatch::IntegrationTest follow_redirect! assert_response :success assert_template "user/login" - get auth_path(:provider => "windowslive", :origin => "/login") + get auth_path(:provider => "windowslive", :origin => "/login?referer=%2Fhistory", :referer => "/history") assert_response :redirect assert_redirected_to auth_success_path(:provider => "windowslive") follow_redirect! @@ -1088,12 +1088,12 @@ class UserLoginTest < ActionDispatch::IntegrationTest follow_redirect! assert_response :success assert_template "user/login" - get auth_path(:provider => "windowslive", :origin => "/login") + get auth_path(:provider => "windowslive", :origin => "/login?referer=%2Fhistory", :referer => "/history") assert_response :redirect assert_redirected_to auth_success_path(:provider => "windowslive") follow_redirect! assert_response :redirect - assert_redirected_to auth_failure_path(:strategy => "windowslive", :message => "connection_failed", :origin => "/login") + assert_redirected_to auth_failure_path(:strategy => "windowslive", :message => "connection_failed", :origin => "/login?referer=%2Fhistory") follow_redirect! assert_response :redirect follow_redirect! @@ -1112,12 +1112,12 @@ class UserLoginTest < ActionDispatch::IntegrationTest follow_redirect! assert_response :success assert_template "user/login" - get auth_path(:provider => "windowslive", :origin => "/login") + get auth_path(:provider => "windowslive", :origin => "/login?referer=%2Fhistory", :referer => "/history") assert_response :redirect assert_redirected_to auth_success_path(:provider => "windowslive") follow_redirect! assert_response :redirect - assert_redirected_to auth_failure_path(:strategy => "windowslive", :message => "invalid_credentials", :origin => "/login") + assert_redirected_to auth_failure_path(:strategy => "windowslive", :message => "invalid_credentials", :origin => "/login?referer=%2Fhistory") follow_redirect! assert_response :redirect follow_redirect! @@ -1136,7 +1136,7 @@ class UserLoginTest < ActionDispatch::IntegrationTest follow_redirect! assert_response :success assert_template "user/login" - get auth_path(:provider => "windowslive", :origin => "/login") + get auth_path(:provider => "windowslive", :origin => "/login?referer=%2Fhistory", :referer => "/history") assert_response :redirect assert_redirected_to auth_success_path(:provider => "windowslive") follow_redirect! -- 2.43.2