From f25bace76618fac82a4d4fcfd779a99f9b5c25a0 Mon Sep 17 00:00:00 2001
From: evazion <noizave@gmail.com>
Date: Sun, 8 Mar 2020 16:07:34 -0500
Subject: [PATCH] users: refactor change password page.

* Fix users being redirected back to the change password page after
  successfully changing their password.
* Move passwords controller out of /maintenance/ namespace.
* Add tests.
---
 .../maintenance/user/passwords_controller.rb  |  9 ------
 app/controllers/passwords_controller.rb       | 31 +++++++++++++++++++
 .../maintenance/user/passwords/edit.html.erb  | 13 --------
 app/views/passwords/edit.html.erb             | 14 +++++++++
 config/routes.rb                              |  2 +-
 test/functional/passwords_controller_test.rb  | 26 ++++++++++++++++
 6 files changed, 72 insertions(+), 23 deletions(-)
 delete mode 100644 app/controllers/maintenance/user/passwords_controller.rb
 create mode 100644 app/controllers/passwords_controller.rb
 delete mode 100644 app/views/maintenance/user/passwords/edit.html.erb
 create mode 100644 app/views/passwords/edit.html.erb
 create mode 100644 test/functional/passwords_controller_test.rb

diff --git a/app/controllers/maintenance/user/passwords_controller.rb b/app/controllers/maintenance/user/passwords_controller.rb
deleted file mode 100644
index c14592059..000000000
--- a/app/controllers/maintenance/user/passwords_controller.rb
+++ /dev/null
@@ -1,9 +0,0 @@
-module Maintenance
-  module User
-    class PasswordsController < ApplicationController
-      def edit
-        @user = CurrentUser.user
-      end
-    end
-  end
-end
diff --git a/app/controllers/passwords_controller.rb b/app/controllers/passwords_controller.rb
new file mode 100644
index 000000000..2d147f07f
--- /dev/null
+++ b/app/controllers/passwords_controller.rb
@@ -0,0 +1,31 @@
+class PasswordsController < ApplicationController
+  before_action :member_only
+  respond_to :html, :xml, :json
+
+  def edit
+    @user = User.find(params[:user_id])
+    check_privilege(@user)
+
+    respond_with(@user)
+  end
+
+  def update
+    @user = User.find(params[:user_id])
+    check_privilege(@user)
+
+    @user.update(user_params)
+    flash[:notice] = @user.errors.none? ? "Password updated" : @user.errors.full_messages.join("; ")
+
+    respond_with(@user, location: @user)
+  end
+
+  private
+
+  def check_privilege(user)
+    raise User::PrivilegeError unless user.id == CurrentUser.id || CurrentUser.is_admin?
+  end
+
+  def user_params
+    params.require(:user).permit(%i[old_password password password_confirmation])
+  end
+end
diff --git a/app/views/maintenance/user/passwords/edit.html.erb b/app/views/maintenance/user/passwords/edit.html.erb
deleted file mode 100644
index 1a0ca613d..000000000
--- a/app/views/maintenance/user/passwords/edit.html.erb
+++ /dev/null
@@ -1,13 +0,0 @@
-<% page_title "Change Password" %>
-
-<div id="c-maintenance-user-passwords">
-  <div id="a-edit">
-    <h1>Change Password</h1>
-
-    <%= edit_form_for @user do |f| %>
-      <%= f.input :old_password, :as => :password, :input_html => {:autocomplete => "off"} %>
-      <%= f.input :password, :label => "New password", :input_html => {:autocomplete => "off"} %>
-      <%= f.button :submit, "Submit" %>
-    <% end %>
-  </div>
-</div>
diff --git a/app/views/passwords/edit.html.erb b/app/views/passwords/edit.html.erb
new file mode 100644
index 000000000..5f495ad1d
--- /dev/null
+++ b/app/views/passwords/edit.html.erb
@@ -0,0 +1,14 @@
+<% page_title "Change Password" %>
+
+<div id="c-passwords">
+  <div id="a-edit">
+    <h1>Change Password</h1>
+
+    <%= edit_form_for(@user, url: user_password_path(@user)) do |f| %>
+      <%= f.input :old_password, as: :password, hint: "Re-enter your current password." %>
+      <%= f.input :password, label: "New password", hint: "Must be at least 5 characters long." %>
+      <%= f.input :password_confirmation, label: "Confirm new password" %>
+      <%= f.submit "Save" %>
+    <% end %>
+  </div>
+</div>
diff --git a/config/routes.rb b/config/routes.rb
index c68dc9641..131ff675f 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -246,7 +246,7 @@ Rails.application.routes.draw do
   end
   resources :users do
     resources :favorite_groups, controller: "favorite_groups", only: [:index], as: "favorite_groups"
-    resource :password, :only => [:edit], :controller => "maintenance/user/passwords"
+    resource :password, only: [:edit, :update]
     resource :api_key, :only => [:show, :view, :update, :destroy], :controller => "maintenance/user/api_keys" do
       post :view
     end
diff --git a/test/functional/passwords_controller_test.rb b/test/functional/passwords_controller_test.rb
new file mode 100644
index 000000000..e6bab032f
--- /dev/null
+++ b/test/functional/passwords_controller_test.rb
@@ -0,0 +1,26 @@
+require 'test_helper'
+
+class PasswordsControllerTest < ActionDispatch::IntegrationTest
+  context "The passwords controller" do
+    setup do
+      @user = create(:user, password: "12345")
+    end
+
+    context "edit action" do
+      should "work" do
+        get_auth edit_user_password_path(@user), @user
+        assert_response :success
+      end
+    end
+
+    context "update action" do
+      should "work" do
+        put_auth user_password_path(@user), @user, params: { user: { old_password: "12345", password: "abcde", password_confirmation: "abcde" } }
+
+        assert_redirected_to user_path(@user)
+        assert_equal(nil, User.authenticate(@user.name, "12345"))
+        assert_equal(@user, User.authenticate(@user.name, "abcde"))
+      end
+    end
+  end
+end