Use rails tokens for signup confirmations

.rubocop_todo.yml

@@ -66,7 +66,7 @@ Metrics/BlockNesting:
 # Offense count: 26
 # Configuration parameters: CountComments, CountAsOne.
 Metrics/ClassLength:
-  Max: 310
+  Max: 313
 
 # Offense count: 59
 # Configuration parameters: AllowedMethods, AllowedPatterns.

app/controllers/concerns/session_methods.rb

@@ -62,9 +62,10 @@ def failed_login(message, username = nil)
   ##
   #
   def unconfirmed_login(user)
-    session[:token] = user.tokens.create.token
+    session[:pending_user] = user.id
 
-    redirect_to :controller => "confirmations", :action => "confirm", :display_name => user.display_name
+    redirect_to :controller => "confirmations", :action => "confirm",
+                :display_name => user.display_name, :referer => session[:referer]
 
     session.delete(:remember_me)
     session.delete(:referer)

app/controllers/confirmations_controller.rb

@@ -15,41 +15,37 @@ class ConfirmationsController < ApplicationController
 
   def confirm
     if request.post?
-      token = UserToken.find_by(:token => params[:confirm_string])
-      if token&.user&.active?
-        flash[:error] = t(".already active")
-        redirect_to login_path
-      elsif !token || token.expired?
+      token = params[:confirm_string]
+
+      user = User.find_by_token_for(:new_user, token) ||
+             UserToken.unexpired.find_by(:token => token)&.user
+
+      if !user
         flash[:error] = t(".unknown token")
         redirect_to :action => "confirm"
-      elsif !token.user.visible?
-        render_unknown_user token.user.display_name
+      elsif user.active?
+        flash[:error] = t(".already active")
+        redirect_to login_path
+      elsif !user.visible?
+        render_unknown_user user.display_name
       else
-        user = token.user
         user.activate
         user.email_valid = true
         flash[:notice] = gravatar_status_message(user) if gravatar_enable(user)
         user.save!
-        referer = safe_referer(token.referer) if token.referer
-        token.destroy
-
-        if session[:token]
-          token = UserToken.find_by(:token => session[:token])
-          session.delete(:token)
-        else
-          token = nil
-        end
+        referer = safe_referer(params[:referer]) if params[:referer]
+        UserToken.delete_by(:token => token)
 
-        if token.nil? || token.user != user
-          flash[:notice] = t(".success")
-          redirect_to login_path(:referer => referer)
-        else
-          token.destroy
+        pending_user = session.delete(:pending_user)
 
+        if user.id == pending_user
           session[:user] = user.id
           session[:fingerprint] = user.fingerprint
 
           redirect_to referer || welcome_path
+        else
+          flash[:notice] = t(".success")
+          redirect_to login_path(:referer => referer)
         end
       end
     else
@@ -61,12 +57,11 @@ def confirm
 
   def confirm_resend
     user = User.visible.find_by(:display_name => params[:display_name])
-    token = UserToken.find_by(:token => session[:token])
 
-    if user.nil? || token.nil? || token.user != user
+    if user.nil? || user.id != session[:pending_user]
       flash[:error] = t ".failure", :name => params[:display_name]
     else
-      UserMailer.signup_confirm(user, user.tokens.create).deliver_later
+      UserMailer.signup_confirm(user, user.generate_token_for(:new_user)).deliver_later
       flash[:notice] = { :partial => "confirmations/resend_success_flash", :locals => { :email => user.email, :sender => Settings.email_from } }
     end
 

app/controllers/sessions_controller.rb

@@ -27,12 +27,7 @@ def destroy
     @title = t ".title"
 
     if request.post?
-      if session[:token]
-        token = UserToken.find_by(:token => session[:token])
-        token&.destroy
-        session.delete(:token)
-      end
-
+      session.delete(:pending_user)
       session.delete(:user)
       session_expires_automatically
 

app/controllers/users_controller.rb

@@ -203,8 +203,8 @@ def save
             session[:referer] = referer
             successful_login(current_user)
           else
-            session[:token] = current_user.tokens.create.token
-            UserMailer.signup_confirm(current_user, current_user.tokens.create(:referer => referer)).deliver_later
+            session[:pending_user] = current_user.id
+            UserMailer.signup_confirm(current_user, current_user.generate_token_for(:new_user), referer).deliver_later
             redirect_to :controller => :confirmations, :action => :confirm, :display_name => current_user.display_name
           end
         else

app/mailers/user_mailer.rb

@@ -10,11 +10,12 @@ class UserMailer < ApplicationMailer
   before_action :set_shared_template_vars
   before_action :attach_project_logo
 
-  def signup_confirm(user, token)
+  def signup_confirm(user, token, referer = nil)
     with_recipient_locale user do
       @url = url_for(:controller => "confirmations", :action => "confirm",
                      :display_name => user.display_name,
-                     :confirm_string => token.token)
+                     :confirm_string => token,
+                     :referer => referer)
 
       mail :to => user.email,
            :subject => t(".subject")

app/models/user.rb

@@ -124,6 +124,10 @@ class User < ApplicationRecord
   before_save :update_tile
   after_save :spam_check
 
+  generates_token_for :new_user, :expires_in => 1.week do
+    fingerprint
+  end
+
   generates_token_for :new_email, :expires_in => 1.week do
     fingerprint
   end

test/controllers/confirmations_controller_test.rb

@@ -39,7 +39,7 @@ def test_confirm_get
     user = build(:user, :pending)
     post user_new_path, :params => { :user => user.attributes }
     post user_save_path, :params => { :read_ct => 1, :read_tou => 1 }
-    confirm_string = User.find_by(:email => user.email).tokens.create.token
+    confirm_string = User.find_by(:email => user.email).generate_token_for(:new_user)
 
     get user_confirm_path, :params => { :display_name => user.display_name, :confirm_string => confirm_string }
     assert_response :success
@@ -51,7 +51,7 @@ def test_confirm_get_already_confirmed
     stub_gravatar_request(user.email)
     post user_new_path, :params => { :user => user.attributes }
     post user_save_path, :params => { :read_ct => 1, :read_tou => 1 }
-    confirm_string = User.find_by(:email => user.email).tokens.create.token
+    confirm_string = User.find_by(:email => user.email).generate_token_for(:new_user)
 
     # Get the confirmation page
     get user_confirm_path, :params => { :display_name => user.display_name, :confirm_string => confirm_string }
@@ -73,7 +73,7 @@ def test_confirm_success_no_token_no_referer
     stub_gravatar_request(user.email)
     post user_new_path, :params => { :user => user.attributes }
     post user_save_path, :params => { :read_ct => 1, :read_tou => 1 }
-    confirm_string = User.find_by(:email => user.email).tokens.create.token
+    confirm_string = User.find_by(:email => user.email).generate_token_for(:new_user)
 
     post logout_path
 
@@ -87,7 +87,7 @@ def test_confirm_success_good_token_no_referer
     stub_gravatar_request(user.email)
     post user_new_path, :params => { :user => user.attributes }
     post user_save_path, :params => { :read_ct => 1, :read_tou => 1 }
-    confirm_string = User.find_by(:email => user.email).tokens.create.token
+    confirm_string = User.find_by(:email => user.email).generate_token_for(:new_user)
 
     post user_confirm_path, :params => { :display_name => user.display_name, :confirm_string => confirm_string }
     assert_redirected_to welcome_path
@@ -98,7 +98,7 @@ def test_confirm_success_bad_token_no_referer
     stub_gravatar_request(user.email)
     post user_new_path, :params => { :user => user.attributes }
     post user_save_path, :params => { :read_ct => 1, :read_tou => 1 }
-    confirm_string = User.find_by(:email => user.email).tokens.create.token
+    confirm_string = User.find_by(:email => user.email).generate_token_for(:new_user)
 
     post logout_path
     session_for(create(:user))
@@ -113,11 +113,11 @@ def test_confirm_success_no_token_with_referer
     stub_gravatar_request(user.email)
     post user_new_path, :params => { :user => user.attributes }
     post user_save_path, :params => { :read_ct => 1, :read_tou => 1 }
-    confirm_string = User.find_by(:email => user.email).tokens.create(:referer => new_diary_entry_path).token
+    confirm_string = User.find_by(:email => user.email).generate_token_for(:new_user)
 
     post logout_path
 
-    post user_confirm_path, :params => { :display_name => user.display_name, :confirm_string => confirm_string }
+    post user_confirm_path, :params => { :display_name => user.display_name, :confirm_string => confirm_string, :referer => new_diary_entry_path }
     assert_redirected_to login_path(:referer => new_diary_entry_path)
     assert_match(/Confirmed your account/, flash[:notice])
   end
@@ -127,9 +127,9 @@ def test_confirm_success_good_token_with_referer
     stub_gravatar_request(user.email)
     post user_new_path, :params => { :user => user.attributes }
     post user_save_path, :params => { :read_ct => 1, :read_tou => 1 }
-    confirm_string = User.find_by(:email => user.email).tokens.create(:referer => new_diary_entry_path).token
+    confirm_string = User.find_by(:email => user.email).generate_token_for(:new_user)
 
-    post user_confirm_path, :params => { :display_name => user.display_name, :confirm_string => confirm_string }
+    post user_confirm_path, :params => { :display_name => user.display_name, :confirm_string => confirm_string, :referer => new_diary_entry_path }
     assert_redirected_to new_diary_entry_path
   end
 
@@ -138,12 +138,12 @@ def test_confirm_success_bad_token_with_referer
     stub_gravatar_request(user.email)
     post user_new_path, :params => { :user => user.attributes }
     post user_save_path, :params => { :read_ct => 1, :read_tou => 1 }
-    confirm_string = User.find_by(:email => user.email).tokens.create(:referer => new_diary_entry_path).token
+    confirm_string = User.find_by(:email => user.email).generate_token_for(:new_user)
 
     post logout_path
     session_for(create(:user))
 
-    post user_confirm_path, :params => { :display_name => user.display_name, :confirm_string => confirm_string }
+    post user_confirm_path, :params => { :display_name => user.display_name, :confirm_string => confirm_string, :referer => new_diary_entry_path }
     assert_redirected_to login_path(:referer => new_diary_entry_path)
     assert_match(/Confirmed your account/, flash[:notice])
   end
@@ -153,9 +153,11 @@ def test_confirm_expired_token
     stub_gravatar_request(user.email)
     post user_new_path, :params => { :user => user.attributes }
     post user_save_path, :params => { :read_ct => 1, :read_tou => 1 }
-    confirm_string = User.find_by(:email => user.email).tokens.create(:expiry => 1.day.ago).token
+    confirm_string = User.find_by(:email => user.email).generate_token_for(:new_user)
 
-    post user_confirm_path, :params => { :display_name => user.display_name, :confirm_string => confirm_string }
+    travel 2.weeks do
+      post user_confirm_path, :params => { :display_name => user.display_name, :confirm_string => confirm_string }
+    end
     assert_redirected_to :action => "confirm"
     assert_match(/confirmation code has expired/, flash[:error])
   end
@@ -165,15 +167,15 @@ def test_confirm_already_confirmed
     stub_gravatar_request(user.email)
     post user_new_path, :params => { :user => user.attributes }
     post user_save_path, :params => { :read_ct => 1, :read_tou => 1 }
-    confirm_string = User.find_by(:email => user.email).tokens.create(:referer => new_diary_entry_path).token
+    confirm_string = User.find_by(:email => user.email).generate_token_for(:new_user)
 
-    post user_confirm_path, :params => { :display_name => user.display_name, :confirm_string => confirm_string }
+    post user_confirm_path, :params => { :display_name => user.display_name, :confirm_string => confirm_string, :referer => new_diary_entry_path }
     assert_redirected_to new_diary_entry_path
 
     post logout_path
 
-    confirm_string = User.find_by(:email => user.email).tokens.create(:referer => new_diary_entry_path).token
-    post user_confirm_path, :params => { :display_name => user.display_name, :confirm_string => confirm_string }
+    confirm_string = User.find_by(:email => user.email).generate_token_for(:new_user)
+    post user_confirm_path, :params => { :display_name => user.display_name, :confirm_string => confirm_string, :referer => new_diary_entry_path }
     assert_redirected_to login_path
     assert_match(/already been confirmed/, flash[:error])
   end
@@ -183,7 +185,7 @@ def test_confirm_deleted
     stub_gravatar_request(user.email)
     post user_new_path, :params => { :user => user.attributes }
     post user_save_path, :params => { :read_ct => 1, :read_tou => 1 }
-    confirm_string = User.find_by(:email => user.email).tokens.create.token
+    confirm_string = User.find_by(:email => user.email).generate_token_for(:new_user)
 
     User.find_by(:display_name => user.display_name).hide!
 

test/controllers/sessions_controller_test.rb

@@ -88,10 +88,7 @@ def test_logout_removes_session_token
     user = build(:user, :pending)
     post user_new_path, :params => { :user => user.attributes }
     post user_save_path, :params => { :read_ct => 1, :read_tou => 1 }
-
-    assert_difference "User.find_by(:email => user.email).tokens.count", -1 do
-      post logout_path
-    end
+    post logout_path
     assert_response :redirect
     assert_redirected_to root_path
   end

test/controllers/users_controller_test.rb

@@ -312,15 +312,13 @@ def test_save_referer_params
 
     assert_difference "User.count", 1 do
       assert_difference "ActionMailer::Base.deliveries.size", 1 do
-        perform_enqueued_jobs do
-          post user_save_path, :params => { :read_ct => 1, :read_tou => 1 }
-        end
+        post user_save_path, :params => { :read_ct => 1, :read_tou => 1 }
+        assert_enqueued_with :job => ActionMailer::MailDeliveryJob,
+                             :args => proc { |args| args[3][:args][2] == welcome_path(:editor => "id", :zoom => 1, :lat => 2, :lon => 3) }
+        perform_enqueued_jobs
       end
     end
 
-    assert_equal welcome_path(:editor => "id", :zoom => 1, :lat => 2, :lon => 3),
-                 User.find_by(:email => user.email).tokens.order("id DESC").first.referer
-
     ActionMailer::Base.deliveries.clear
   end