From: Tom Hughes <tom@compton.nu>
Date: Sat, 24 Feb 2024 13:42:26 +0000 (+0000)
Subject: Use rails tokens for email changes
X-Git-Tag: live~764^2~1
X-Git-Url: https://git.openstreetmap.org./rails.git/commitdiff_plain/ad2739347b5fc7c57d8b7131580fda10cc77f108

Use rails tokens for email changes
---

diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml
index 8c2407990..3d369d558 100644
--- a/.rubocop_todo.yml
+++ b/.rubocop_todo.yml
@@ -66,7 +66,7 @@ Metrics/BlockNesting:
 # Offense count: 26
 # Configuration parameters: CountComments, CountAsOne.
 Metrics/ClassLength:
-  Max: 307
+  Max: 310
 
 # Offense count: 59
 # Configuration parameters: AllowedMethods, AllowedPatterns.
diff --git a/app/controllers/concerns/user_methods.rb b/app/controllers/concerns/user_methods.rb
index eb7d38988..8cba09827 100644
--- a/app/controllers/concerns/user_methods.rb
+++ b/app/controllers/concerns/user_methods.rb
@@ -51,7 +51,7 @@ module UserMethods
           flash[:notice] = t "accounts.update.success_confirm_needed"
 
           begin
-            UserMailer.email_confirm(user, user.tokens.create).deliver_later
+            UserMailer.email_confirm(user, user.generate_token_for(:new_email)).deliver_later
           rescue StandardError
             # Ignore errors sending email
           end
diff --git a/app/controllers/confirmations_controller.rb b/app/controllers/confirmations_controller.rb
index 65f560571..604e6b5b3 100644
--- a/app/controllers/confirmations_controller.rb
+++ b/app/controllers/confirmations_controller.rb
@@ -75,9 +75,12 @@ class ConfirmationsController < ApplicationController
 
   def confirm_email
     if request.post?
-      token = UserToken.find_by(:token => params[:confirm_string])
-      if token&.user&.new_email?
-        self.current_user = token.user
+      token = params[:confirm_string]
+
+      self.current_user = User.find_by_token_for(:new_email, token) ||
+                          UserToken.unexpired.find_by(:token => params[:confirm_string])&.user
+
+      if current_user&.new_email?
         current_user.email = current_user.new_email
         current_user.new_email = nil
         current_user.email_valid = true
@@ -94,7 +97,7 @@ class ConfirmationsController < ApplicationController
         current_user.tokens.delete_all
         session[:user] = current_user.id
         session[:fingerprint] = current_user.fingerprint
-      elsif token
+      elsif current_user
         flash[:error] = t ".failure"
       else
         flash[:error] = t ".unknown_token"
diff --git a/app/mailers/user_mailer.rb b/app/mailers/user_mailer.rb
index 160dc1996..4e15a296a 100644
--- a/app/mailers/user_mailer.rb
+++ b/app/mailers/user_mailer.rb
@@ -25,7 +25,7 @@ class UserMailer < ApplicationMailer
     with_recipient_locale user do
       @address = user.new_email
       @url = url_for(:controller => "confirmations", :action => "confirm_email",
-                     :confirm_string => token.token)
+                     :confirm_string => token)
 
       mail :to => user.new_email,
            :subject => t(".subject")
diff --git a/app/models/user.rb b/app/models/user.rb
index 28a8d051a..958a03a98 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -124,6 +124,10 @@ class User < ApplicationRecord
   before_save :update_tile
   after_save :spam_check
 
+  generates_token_for :new_email, :expires_in => 1.week do
+    fingerprint
+  end
+
   generates_token_for :password_reset, :expires_in => 1.week do
     fingerprint
   end
diff --git a/test/controllers/confirmations_controller_test.rb b/test/controllers/confirmations_controller_test.rb
index 083619962..11d2bfd2c 100644
--- a/test/controllers/confirmations_controller_test.rb
+++ b/test/controllers/confirmations_controller_test.rb
@@ -269,7 +269,7 @@ class UsersControllerTest < ActionDispatch::IntegrationTest
 
   def test_confirm_email_get
     user = create(:user)
-    confirm_string = user.tokens.create.token
+    confirm_string = user.generate_token_for(:new_email)
 
     get user_confirm_email_path, :params => { :confirm_string => confirm_string }
     assert_response :success
@@ -279,7 +279,7 @@ class UsersControllerTest < ActionDispatch::IntegrationTest
   def test_confirm_email_success
     user = create(:user, :new_email => "test-new@example.com")
     stub_gravatar_request(user.new_email)
-    confirm_string = user.tokens.create.token
+    confirm_string = user.generate_token_for(:new_email)
 
     post user_confirm_email_path, :params => { :confirm_string => confirm_string }
     assert_response :redirect
@@ -289,7 +289,7 @@ class UsersControllerTest < ActionDispatch::IntegrationTest
 
   def test_confirm_email_already_confirmed
     user = create(:user)
-    confirm_string = user.tokens.create.token
+    confirm_string = user.generate_token_for(:new_email)
 
     post user_confirm_email_path, :params => { :confirm_string => confirm_string }
     assert_response :redirect
@@ -312,7 +312,7 @@ class UsersControllerTest < ActionDispatch::IntegrationTest
     # switch to email that has a gravatar
     user = create(:user, :new_email => "test-new@example.com")
     stub_gravatar_request(user.new_email, 200)
-    confirm_string = user.tokens.create.token
+    confirm_string = user.generate_token_for(:new_email)
     # precondition gravatar should be turned off
     assert_not user.image_use_gravatar
     post user_confirm_email_path, :params => { :confirm_string => confirm_string }
@@ -327,7 +327,7 @@ class UsersControllerTest < ActionDispatch::IntegrationTest
     # switch to email without a gravatar
     user = create(:user, :new_email => "test-new@example.com", :image_use_gravatar => true)
     stub_gravatar_request(user.new_email, 404)
-    confirm_string = user.tokens.create.token
+    confirm_string = user.generate_token_for(:new_email)
     # precondition gravatar should be turned on
     assert user.image_use_gravatar
     post user_confirm_email_path, :params => { :confirm_string => confirm_string }