From: Andy Allan <git@gravitystorm.co.uk>
Date: Wed, 22 May 2024 11:09:31 +0000 (+0100)
Subject: Merge pull request #4758 from tomhughes/login-referer
X-Git-Tag: live~902
X-Git-Url: https://git.openstreetmap.org./rails.git/commitdiff_plain/8a5c9a8052489c04a64856c2f0579647a1637326?hp=-c

Merge pull request #4758 from tomhughes/login-referer

Stop using the session to persist the referer during login
---

8a5c9a8052489c04a64856c2f0579647a1637326
diff --combined app/controllers/users_controller.rb
index 2cdec642d,688692b05..f3ba5df2c
--- a/app/controllers/users_controller.rb
+++ b/app/controllers/users_controller.rb
@@@ -54,16 -54,12 +54,12 @@@ class UsersController < ApplicationCont
  
    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
  
      append_content_security_policy_directives(
 -      :form_action => %w[accounts.google.com *.facebook.com login.live.com github.com meta.wikimedia.org]
 +      :form_action => %w[accounts.google.com *.facebook.com login.microsoftonline.com github.com meta.wikimedia.org]
      )
  
      if current_user
@@@ -94,10 -90,6 +90,6 @@@
      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 +107,7 @@@
        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 +192,7 @@@
    ##
    # 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 +226,7 @@@
        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 +239,13 @@@
        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 +274,7 @@@
  
    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 +299,10 @@@
  
          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 +314,8 @@@
      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 --combined app/views/sessions/new.html.erb
index b493de73b,57d621b97..ac1d8ed6e
--- a/app/views/sessions/new.html.erb
+++ b/app/views/sessions/new.html.erb
@@@ -7,7 -7,7 +7,7 @@@
  
  <% content_for :heading do %>
    <% if @client_app_name %>
 -    <p class="text-center text-muted fs-6 py-2 mb-0 bg-white"><%= t(".login_to_authorize_html", :client_app_name => @client_app_name) %></p>
 +    <p class="text-center text-muted fs-6 py-2 mb-0 bg-body"><%= t(".login_to_authorize_html", :client_app_name => @client_app_name) %></p>
    <% end %>
  
    <div class="header-illustration new-user-main auth-container mx-auto">
@@@ -16,7 -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>
@@@ -35,7 -35,7 +35,7 @@@
    <%= bootstrap_form_tag(:action => "login", :html => { :id => "login_form" }) do |f| %>
      <%= hidden_field_tag("referer", h(params[:referer]), :autocomplete => "off") %>
  
 -    <%= f.text_field :username, :label => t(".email or username"), :tabindex => 1, :value => params[:username] %>
 +    <%= f.text_field :username, :label => t(".email or username"), :autofocus => true, :tabindex => 1, :value => params[:username] %>
  
      <div class="row">
        <div class="col">
diff --combined app/views/users/new.html.erb
index 865303f56,3eaf84c16..5ab9edb3f
--- a/app/views/users/new.html.erb
+++ b/app/views/users/new.html.erb
@@@ -7,13 -7,13 +7,13 @@@
  
  <% content_for :heading do %>
    <% if @client_app_name %>
 -    <p class="text-center text-muted fs-6 py-2 mb-0 bg-white"><%= t(".signup_to_authorize_html", :client_app_name => @client_app_name) %></p>
 +    <p class="text-center text-muted fs-6 py-2 mb-0 bg-body"><%= t(".signup_to_authorize_html", :client_app_name => @client_app_name) %></p>
    <% end %>
  
    <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" %>
@@@ -53,7 -53,6 +53,7 @@@
                                                                             t(".privacy_policy_url"),
                                                                             :title => t(".privacy_policy_title"),
                                                                             :target => :new)),
 +                                :autofocus => true,
                                  :tabindex => 1 %>
      <% else %>
        <%= f.hidden_field :email %>