From 4bca24a7beb4a78cecfeaa775d41662497f9930f Mon Sep 17 00:00:00 2001 From: Andy Allan Date: Wed, 6 Dec 2023 18:41:00 +0000 Subject: [PATCH] Resourceful routing for passwords This also matches the routes used by devise --- app/abilities/ability.rb | 2 +- app/controllers/passwords_controller.rb | 54 ++++++++++++------- ...{reset_password.html.erb => edit.html.erb} | 2 +- .../{lost_password.html.erb => new.html.erb} | 0 config/locales/en.yml | 7 ++- config/routes.rb | 8 ++- test/controllers/passwords_controller_test.rb | 20 +++---- 7 files changed, 59 insertions(+), 34 deletions(-) rename app/views/passwords/{reset_password.html.erb => edit.html.erb} (74%) rename app/views/passwords/{lost_password.html.erb => new.html.erb} (100%) diff --git a/app/abilities/ability.rb b/app/abilities/ability.rb index cdf28bd4f..f9348f68e 100644 --- a/app/abilities/ability.rb +++ b/app/abilities/ability.rb @@ -19,7 +19,7 @@ class Ability can [:confirm, :confirm_resend, :confirm_email], :confirmation can [:index, :rss, :show, :comments], DiaryEntry can [:index], Note - can [:lost_password, :reset_password], :password + can [:new, :create, :edit, :update], :password can [:index, :show], Redaction can [:new, :create, :destroy], :session can [:index, :show, :data, :georss, :picture, :icon], Trace diff --git a/app/controllers/passwords_controller.rb b/app/controllers/passwords_controller.rb index 08df9f7a4..87d25df68 100644 --- a/app/controllers/passwords_controller.rb +++ b/app/controllers/passwords_controller.rb @@ -9,34 +9,50 @@ class PasswordsController < ApplicationController authorize_resource :class => false - before_action :check_database_writable, :only => [:lost_password, :reset_password] + before_action :check_database_writable - def lost_password + def new @title = t ".title" + end - if request.post? - user = User.visible.find_by(:email => params[:email]) - - if user.nil? - users = User.visible.where("LOWER(email) = LOWER(?)", params[:email]) + def edit + @title = t ".title" - user = users.first if users.count == 1 - end + if params[:token] + token = UserToken.find_by(:token => params[:token]) - if user - token = user.tokens.create - UserMailer.lost_password(user, token).deliver_later - flash[:notice] = t ".notice email on way" - redirect_to login_path + if token + self.current_user = token.user else - flash.now[:error] = t ".notice email cannot find" + flash[:error] = t ".flash token bad" + redirect_to :action => "new" end + else + head :bad_request end end - def reset_password - @title = t ".title" + def create + user = User.visible.find_by(:email => params[:email]) + + if user.nil? + users = User.visible.where("LOWER(email) = LOWER(?)", params[:email]) + + user = users.first if users.count == 1 + end + + if user + token = user.tokens.create + UserMailer.lost_password(user, token).deliver_later + flash[:notice] = t ".notice email on way" + redirect_to login_path + else + flash.now[:error] = t ".notice email cannot find" + render :new + end + end + def update if params[:token] token = UserToken.find_by(:token => params[:token]) @@ -54,11 +70,13 @@ class PasswordsController < ApplicationController session[:fingerprint] = current_user.fingerprint flash[:notice] = t ".flash changed" successful_login(current_user) + else + render :edit end end else flash[:error] = t ".flash token bad" - redirect_to :action => "lost_password" + redirect_to :action => "new" end else head :bad_request diff --git a/app/views/passwords/reset_password.html.erb b/app/views/passwords/edit.html.erb similarity index 74% rename from app/views/passwords/reset_password.html.erb rename to app/views/passwords/edit.html.erb index 99f07cab6..ae7900ece 100644 --- a/app/views/passwords/reset_password.html.erb +++ b/app/views/passwords/edit.html.erb @@ -2,7 +2,7 @@

<%= t ".heading", :user => current_user.display_name %>

<% end %> -<%= bootstrap_form_for current_user, :url => { :action => "reset_password" }, :html => { :method => :post } do |f| %> +<%= bootstrap_form_for current_user, :url => { :action => "update" }, :html => { :method => :post } do |f| %> <%= f.hidden_field :token, :name => "token", :value => params[:token] %> <%= f.password_field :pass_crypt, :value => "" %> <%= f.password_field :pass_crypt_confirmation, :value => "" %> diff --git a/app/views/passwords/lost_password.html.erb b/app/views/passwords/new.html.erb similarity index 100% rename from app/views/passwords/lost_password.html.erb rename to app/views/passwords/new.html.erb diff --git a/config/locales/en.yml b/config/locales/en.yml index 8b0745118..c48a94017 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1732,18 +1732,21 @@ en: destroy: destroyed: "Message deleted" passwords: - lost_password: + new: title: "Lost password" heading: "Forgotten Password?" email address: "Email Address:" new password button: "Reset password" help_text: "Enter the email address you used to sign up, we will send a link to it that you can use to reset your password." + create: notice email on way: "Sorry you lost it :-( but an email is on its way so you can reset it soon." notice email cannot find: "Could not find that email address, sorry." - reset_password: + edit: title: "Reset password" heading: "Reset Password for %{user}" reset: "Reset Password" + flash token bad: "Did not find that token, check the URL maybe?" + update: flash changed: "Your password has been changed." flash token bad: "Did not find that token, check the URL maybe?" preferences: diff --git a/config/routes.rb b/config/routes.rb index 43c43a793..2b67e360e 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -172,8 +172,12 @@ OpenStreetMap::Application.routes.draw do match "/user/confirm" => "confirmations#confirm", :via => [:get, :post] match "/user/confirm-email" => "confirmations#confirm_email", :via => [:get, :post] post "/user/go_public" => "users#go_public" - match "/user/reset-password" => "passwords#reset_password", :via => [:get, :post], :as => :user_reset_password - match "/user/forgot-password" => "passwords#lost_password", :via => [:get, :post], :as => :user_forgot_password + scope :user, :as => "user" do + get "forgot-password" => "passwords#new" + post "forgot-password" => "passwords#create" + get "reset-password" => "passwords#edit" + post "reset-password" => "passwords#update" + end get "/user/suspended" => "users#suspended" get "/index.html", :to => redirect(:path => "/") diff --git a/test/controllers/passwords_controller_test.rb b/test/controllers/passwords_controller_test.rb index 0a3a32c52..c39e8465b 100644 --- a/test/controllers/passwords_controller_test.rb +++ b/test/controllers/passwords_controller_test.rb @@ -6,19 +6,19 @@ class PasswordsControllerTest < ActionDispatch::IntegrationTest def test_routes assert_routing( { :path => "/user/forgot-password", :method => :get }, - { :controller => "passwords", :action => "lost_password" } + { :controller => "passwords", :action => "new" } ) assert_routing( { :path => "/user/forgot-password", :method => :post }, - { :controller => "passwords", :action => "lost_password" } + { :controller => "passwords", :action => "create" } ) assert_routing( { :path => "/user/reset-password", :method => :get }, - { :controller => "passwords", :action => "reset_password" } + { :controller => "passwords", :action => "edit" } ) assert_routing( { :path => "/user/reset-password", :method => :post }, - { :controller => "passwords", :action => "reset_password" } + { :controller => "passwords", :action => "update" } ) end @@ -26,7 +26,7 @@ class PasswordsControllerTest < ActionDispatch::IntegrationTest # Test fetching the lost password page get user_forgot_password_path assert_response :success - assert_template :lost_password + assert_template :new assert_select "div#notice", false # Test resetting using the address as recorded for a user that has an @@ -41,7 +41,7 @@ class PasswordsControllerTest < ActionDispatch::IntegrationTest end end assert_response :success - assert_template :lost_password + assert_template :new # Resetting with POST should work assert_difference "ActionMailer::Base.deliveries.size", 1 do @@ -80,7 +80,7 @@ class PasswordsControllerTest < ActionDispatch::IntegrationTest end end assert_response :success - assert_template :lost_password + assert_template :new assert_select ".alert.alert-danger", /^Could not find that email address/ # Test resetting using the address as recorded for a user that has an @@ -124,7 +124,7 @@ class PasswordsControllerTest < ActionDispatch::IntegrationTest # Test a request with a bogus token get user_reset_password_path, :params => { :token => "made_up_token" } assert_response :redirect - assert_redirected_to :action => :lost_password + assert_redirected_to :action => :new # Create a valid token for a user token = user.tokens.create @@ -132,12 +132,12 @@ class PasswordsControllerTest < ActionDispatch::IntegrationTest # Test a request with a valid token get user_reset_password_path, :params => { :token => token.token } assert_response :success - assert_template :reset_password + 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" } } assert_response :success - assert_template :reset_password + assert_template :edit assert_select "div.invalid-feedback" # Test setting a new password -- 2.45.1