Skip to content

Commit

Permalink
Stop using the session to persist the referer during login
Browse files Browse the repository at this point in the history
  • Loading branch information
tomhughes committed May 6, 2024
1 parent b625eef commit 74cc88f
Show file tree
Hide file tree
Showing 5 changed files with 31 additions and 40 deletions.
13 changes: 5 additions & 8 deletions app/controllers/concerns/session_methods.rb
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ def successful_login(user, referer = nil)
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:
#
Expand All @@ -56,31 +56,28 @@ def successful_login(user, referer = nil)
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

##
Expand Down
20 changes: 11 additions & 9 deletions app/controllers/sessions_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -43,15 +45,15 @@ def destroy

##
# 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
34 changes: 13 additions & 21 deletions app/controllers/users_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -54,11 +54,7 @@ def show

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

Expand Down Expand Up @@ -94,10 +90,6 @@ def create
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
Expand All @@ -115,7 +107,7 @@ def create
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
Expand Down Expand Up @@ -200,6 +192,7 @@ def set_status
##
# omniauth success callback
def auth_success
referer = request.env["omniauth.params"]["referer"]
auth_info = request.env["omniauth.auth"]

provider = auth_info[:provider]
Expand Down Expand Up @@ -233,7 +226,7 @@ def auth_success
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)

Expand All @@ -246,13 +239,13 @@ def auth_success
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
Expand Down Expand Up @@ -281,7 +274,7 @@ def self.message_hmac(text)

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)
Expand All @@ -306,11 +299,10 @@ def save_new_user(email_hmac)

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
Expand All @@ -322,8 +314,8 @@ def save_new_user(email_hmac)
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

Expand Down
2 changes: 1 addition & 1 deletion app/views/sessions/new.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
<%= link_to t("sessions.new.tab_title"), "#", :class => "nav-link active" %>
</li>
<li class="nav-item">
<%= 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" %>
</li>
</ul>
</div>
Expand Down
2 changes: 1 addition & 1 deletion app/views/users/new.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
<div class="header-illustration new-user-main auth-container mx-auto">
<ul class="nav nav-tabs position-absolute bottom-0 px-3 fs-6 w-100">
<li class="nav-item">
<%= link_to t("sessions.new.tab_title"), url_for(:action => :new, :controller => :sessions), :class => "nav-link" %>
<%= link_to t("sessions.new.tab_title"), url_for(:action => :new, :controller => :sessions, :referer => @referer), :class => "nav-link" %>
</li>
<li class="nav-item">
<%= link_to t("users.new.tab_title"), "#", :class => "nav-link active" %>
Expand Down

0 comments on commit 74cc88f

Please sign in to comment.