From: Tom Hughes <tom@compton.nu>
Date: Thu, 7 Dec 2023 18:30:12 +0000 (+0000)
Subject: Use rails tokens for password resets
X-Git-Tag: live~812^2~2
X-Git-Url: https://git.openstreetmap.org./rails.git/commitdiff_plain/b8fad531e47641fc402203edc732c1484d28733a

Use rails tokens for password resets
---

diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml
index 9874aa379..8c2407990 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: 305
+  Max: 307
 
 # Offense count: 59
 # Configuration parameters: AllowedMethods, AllowedPatterns.
diff --git a/app/controllers/passwords_controller.rb b/app/controllers/passwords_controller.rb
index 87d25df68..8025fd700 100644
--- a/app/controllers/passwords_controller.rb
+++ b/app/controllers/passwords_controller.rb
@@ -19,11 +19,10 @@ class PasswordsController < ApplicationController
     @title = t ".title"
 
     if params[:token]
-      token = UserToken.find_by(:token => params[:token])
+      self.current_user = User.find_by_token_for(:password_reset, params[:token]) ||
+                          UserToken.unexpired.find_by(:token => params[:token])&.user
 
-      if token
-        self.current_user = token.user
-      else
+      if current_user.nil?
         flash[:error] = t ".flash token bad"
         redirect_to :action => "new"
       end
@@ -42,7 +41,7 @@ class PasswordsController < ApplicationController
     end
 
     if user
-      token = user.tokens.create
+      token = user.generate_token_for(:password_reset)
       UserMailer.lost_password(user, token).deliver_later
       flash[:notice] = t ".notice email on way"
       redirect_to login_path
@@ -54,11 +53,10 @@ class PasswordsController < ApplicationController
 
   def update
     if params[:token]
-      token = UserToken.find_by(:token => params[:token])
-
-      if token
-        self.current_user = token.user
+      self.current_user = User.find_by_token_for(:password_reset, params[:token]) ||
+                          UserToken.unexpired.find_by(:token => params[:token])&.user
 
+      if current_user
         if params[:user]
           current_user.pass_crypt = params[:user][:pass_crypt]
           current_user.pass_crypt_confirmation = params[:user][:pass_crypt_confirmation]
@@ -66,7 +64,7 @@ class PasswordsController < ApplicationController
           current_user.email_valid = true
 
           if current_user.save
-            token.destroy
+            UserToken.delete_by(:token => params[:token])
             session[:fingerprint] = current_user.fingerprint
             flash[:notice] = t ".flash changed"
             successful_login(current_user)
diff --git a/app/mailers/user_mailer.rb b/app/mailers/user_mailer.rb
index 0894b972d..160dc1996 100644
--- a/app/mailers/user_mailer.rb
+++ b/app/mailers/user_mailer.rb
@@ -34,7 +34,7 @@ class UserMailer < ApplicationMailer
 
   def lost_password(user, token)
     with_recipient_locale user do
-      @url = user_reset_password_url(:token => token.token)
+      @url = user_reset_password_url(:token => token)
 
       mail :to => user.email,
            :subject => t(".subject")
diff --git a/app/models/user.rb b/app/models/user.rb
index 7faf748cd..28a8d051a 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 :password_reset, :expires_in => 1.week do
+    fingerprint
+  end
+
   def display_name_cannot_be_user_id_with_other_id
     display_name&.match(/^user_(\d+)$/i) do |m|
       errors.add :display_name, I18n.t("activerecord.errors.messages.display_name_is_user_n") unless m[1].to_i == id
diff --git a/test/controllers/passwords_controller_test.rb b/test/controllers/passwords_controller_test.rb
index c39e8465b..25cfdd4e5 100644
--- a/test/controllers/passwords_controller_test.rb
+++ b/test/controllers/passwords_controller_test.rb
@@ -127,21 +127,21 @@ class PasswordsControllerTest < ActionDispatch::IntegrationTest
     assert_redirected_to :action => :new
 
     # Create a valid token for a user
-    token = user.tokens.create
+    token = user.generate_token_for(:password_reset)
 
     # Test a request with a valid token
-    get user_reset_password_path, :params => { :token => token.token }
+    get user_reset_password_path, :params => { :token => token }
     assert_response :success
     assert_template :edit
 
     # Test that errors are reported for erroneous submissions
-    post user_reset_password_path, :params => { :token => token.token, :user => { :pass_crypt => "new_password", :pass_crypt_confirmation => "different_password" } }
+    post user_reset_password_path, :params => { :token => token, :user => { :pass_crypt => "new_password", :pass_crypt_confirmation => "different_password" } }
     assert_response :success
     assert_template :edit
     assert_select "div.invalid-feedback"
 
     # Test setting a new password
-    post user_reset_password_path, :params => { :token => token.token, :user => { :pass_crypt => "new_password", :pass_crypt_confirmation => "new_password" } }
+    post user_reset_password_path, :params => { :token => token, :user => { :pass_crypt => "new_password", :pass_crypt_confirmation => "new_password" } }
     assert_response :redirect
     assert_redirected_to root_path
     assert_equal user.id, session[:user]