Resourceful routing for passwords

This also matches the routes used by devise

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

app/controllers/passwords_controller.rb

@@ -9,12 +9,30 @@ 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
+
+  def edit
     @title = t ".title"
 
-    if request.post?
+    if params[:token]
+      token = UserToken.find_by(:token => params[:token])
+
+      if token
+        self.current_user = token.user
+      else
+        flash[:error] = t ".flash token bad"
+        redirect_to :action => "new"
+      end
+    else
+      head :bad_request
+    end
+  end
+
+  def create
     user = User.visible.find_by(:email => params[:email])
 
     if user.nil?
@@ -30,13 +48,11 @@ class PasswordsController < ApplicationController
       redirect_to login_path
     else
       flash.now[:error] = t ".notice email cannot find"
-      end
+      render :new
     end
   end
 
-  def reset_password
+  def update
-    @title = t ".title"
-
     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

app/views/passwords/edit.html.erb

@@ -2,7 +2,7 @@
   <h1><%= t ".heading", :user => current_user.display_name %></h1>
 <% 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 => "" %>

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:

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
+  scope :user, :as => "user" do
-  match "/user/forgot-password" => "passwords#lost_password", :via => [:get, :post], :as => :user_forgot_password
+    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 => "/")

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