From: Andy Allan Date: Wed, 22 May 2024 11:09:31 +0000 (+0100) Subject: Merge pull request #4758 from tomhughes/login-referer X-Git-Tag: live~460 X-Git-Url: https://git.openstreetmap.org./rails.git/commitdiff_plain/8a5c9a8052489c04a64856c2f0579647a1637326?hp=7ab920de5fe349b5094cb66385b9829cfb3d8d6c Merge pull request #4758 from tomhughes/login-referer Stop using the session to persist the referer during login --- diff --git a/app/controllers/concerns/session_methods.rb b/app/controllers/concerns/session_methods.rb index 5dcddb82d..45cf0d943 100644 --- a/app/controllers/concerns/session_methods.rb +++ b/app/controllers/concerns/session_methods.rb @@ -39,7 +39,7 @@ module SessionMethods session[:fingerprint] = user.fingerprint session_expires_after 28.days if session[:remember_me] - target = referer || session[:referer] || url_for(:controller => :site, :action => :index) + target = referer || url_for(:controller => :site, :action => :index) # The user is logged in, so decide where to send them: # @@ -56,31 +56,28 @@ module SessionMethods end session.delete(:remember_me) - session.delete(:referer) end ## # process a failed login - def failed_login(message, username = nil) + def failed_login(message, username, referer = nil) flash[:error] = message - redirect_to :controller => "sessions", :action => "new", :referer => session[:referer], + redirect_to :controller => "sessions", :action => "new", :referer => referer, :username => username, :remember_me => session[:remember_me] session.delete(:remember_me) - session.delete(:referer) end ## # - def unconfirmed_login(user) + def unconfirmed_login(user, referer = nil) session[:pending_user] = user.id redirect_to :controller => "confirmations", :action => "confirm", - :display_name => user.display_name, :referer => session[:referer] + :display_name => user.display_name, :referer => referer session.delete(:remember_me) - session.delete(:referer) end ## diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index fdf2df6a7..2b6905ebb 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -14,15 +14,17 @@ class SessionsController < ApplicationController def new override_content_security_policy_directives(:form_action => []) if Settings.csp_enforce || Settings.key?(:csp_report_url) - session[:referer] = safe_referer(params[:referer]) if params[:referer] + referer = safe_referer(params[:referer]) if params[:referer] - parse_oauth_referer session[:referer] + parse_oauth_referer referer end def create session[:remember_me] ||= params[:remember_me] - session[:referer] = safe_referer(params[:referer]) if params[:referer] - password_authentication(params[:username].strip, params[:password]) + + referer = safe_referer(params[:referer]) if params[:referer] + + password_authentication(params[:username].strip, params[:password], referer) end def destroy @@ -43,15 +45,15 @@ class SessionsController < ApplicationController ## # handle password authentication - def password_authentication(username, password) + def password_authentication(username, password, referer = nil) if (user = User.authenticate(:username => username, :password => password)) - successful_login(user) + successful_login(user, referer) elsif (user = User.authenticate(:username => username, :password => password, :pending => true)) - unconfirmed_login(user) + unconfirmed_login(user, referer) elsif User.authenticate(:username => username, :password => password, :suspended => true) - failed_login({ :partial => "sessions/suspended_flash" }, username) + failed_login({ :partial => "sessions/suspended_flash" }, username, referer) else - failed_login t("sessions.new.auth failure"), username + failed_login(t("sessions.new.auth failure"), username, referer) end end end diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 2cdec642d..f3ba5df2c 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -54,11 +54,7 @@ class UsersController < ApplicationController def new @title = t ".title" - @referer = if params[:referer] - safe_referer(params[:referer]) - else - session[:referer] - end + @referer = safe_referer(params[:referer]) parse_oauth_referer @referer @@ -94,10 +90,6 @@ class UsersController < ApplicationController self.current_user = User.new(user_params) if check_signup_allowed(current_user.email) - session[:referer] = safe_referer(params[:referer]) if params[:referer] - - Rails.logger.info "create: #{session[:referer]}" - if current_user.auth_uid.present? # We are creating an account with external authentication and # no password was specified so create a random one @@ -115,7 +107,7 @@ class UsersController < ApplicationController else # Save the user record session[:new_user] = current_user.slice("email", "display_name", "pass_crypt", "pass_crypt_confirmation") - save_new_user params[:email_hmac] + save_new_user params[:email_hmac], params[:referer] end end end @@ -200,6 +192,7 @@ class UsersController < ApplicationController ## # omniauth success callback def auth_success + referer = request.env["omniauth.params"]["referer"] auth_info = request.env["omniauth.auth"] provider = auth_info[:provider] @@ -233,7 +226,7 @@ class UsersController < ApplicationController session[:new_user]["auth_uid"] = uid email_hmac = UsersController.message_hmac(email) if email_verified && email - save_new_user email_hmac + save_new_user email_hmac, referer else user = User.find_by(:auth_provider => provider, :auth_uid => uid) @@ -246,13 +239,13 @@ class UsersController < ApplicationController if user case user.status when "pending" - unconfirmed_login(user) + unconfirmed_login(user, referer) when "active", "confirmed" - successful_login(user, request.env["omniauth.params"]["referer"]) + successful_login(user, referer) when "suspended" - failed_login({ :partial => "sessions/suspended_flash" }) + failed_login({ :partial => "sessions/suspended_flash" }, user.display_name, referer) else - failed_login t("sessions.new.auth failure") + failed_login(t("sessions.new.auth failure"), user.display_name, referer) end else email_hmac = UsersController.message_hmac(email) if email_verified && email @@ -281,7 +274,7 @@ class UsersController < ApplicationController private - def save_new_user(email_hmac) + def save_new_user(email_hmac, referer = nil) new_user = session.delete(:new_user) self.current_user = User.new(new_user) if check_signup_allowed(current_user.email) @@ -306,11 +299,10 @@ class UsersController < ApplicationController flash[:matomo_goal] = Settings.matomo["goals"]["signup"] if defined?(Settings.matomo) - referer = welcome_path(welcome_options) + referer = welcome_path(welcome_options(referer)) if current_user.status == "active" - session[:referer] = referer - successful_login(current_user) + successful_login(current_user, referer) else session[:pending_user] = current_user.id UserMailer.signup_confirm(current_user, current_user.generate_token_for(:new_user), referer).deliver_later @@ -322,8 +314,8 @@ class UsersController < ApplicationController end end - def welcome_options - uri = URI(session[:referer]) if session[:referer].present? + def welcome_options(referer = nil) + uri = URI(referer) if referer.present? return { "oauth_return_url" => uri&.to_s } if uri&.path == oauth_authorization_path diff --git a/app/views/sessions/new.html.erb b/app/views/sessions/new.html.erb index b493de73b..ac1d8ed6e 100644 --- a/app/views/sessions/new.html.erb +++ b/app/views/sessions/new.html.erb @@ -16,7 +16,7 @@ <%= link_to t("sessions.new.tab_title"), "#", :class => "nav-link active" %>
  • - <%= link_to t("users.new.tab_title"), url_for(:action => :new, :controller => :users), :class => "nav-link" %> + <%= link_to t("users.new.tab_title"), url_for(:action => :new, :controller => :users, :referer => params[:referer]), :class => "nav-link" %>
    • diff --git a/app/views/users/new.html.erb b/app/views/users/new.html.erb index 865303f56..5ab9edb3f 100644 --- a/app/views/users/new.html.erb +++ b/app/views/users/new.html.erb @@ -13,7 +13,7 @@