diff --git a/app/controllers/passwords_controller.rb b/app/controllers/passwords_controller.rb index 9391d330e..d1b16b1f0 100644 --- a/app/controllers/passwords_controller.rb +++ b/app/controllers/passwords_controller.rb @@ -8,13 +8,17 @@ class PasswordsController < ApplicationController def update @user = authorize User.find(params[:user_id]), policy_class: PasswordPolicy - @user.update(user_params) + + if User.authenticate(@user.name, params[:user][:old_password]) + @user.update(password: params[:user][:password], password_confirmation: params[:user][:password_confirmation]) + elsif @user.authenticate_login_key(params[:user][:signed_user_id]) + @user.update(password: params[:user][:password], password_confirmation: params[:user][:password_confirmation]) + else + @user.errors[:base] << "Incorrect password" + end + flash[:notice] = @user.errors.none? ? "Password updated" : @user.errors.full_messages.join("; ") respond_with(@user, location: @user) end - - def user_params - params.fetch(:user, {}).permit(policy(:password).permitted_attributes) - end end diff --git a/app/logical/user_deletion.rb b/app/logical/user_deletion.rb index e06b2260e..4fa4b06a5 100644 --- a/app/logical/user_deletion.rb +++ b/app/logical/user_deletion.rb @@ -41,11 +41,7 @@ class UserDeletion end def reset_password - random = SecureRandom.hex(16) - user.password = random - user.password_confirmation = random - user.old_password = password - user.save! + user.update!(password: SecureRandom.hex(16)) end def remove_favorites diff --git a/app/models/user.rb b/app/models/user.rb index 69db7613d..106392c7b 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -67,7 +67,7 @@ class User < ApplicationRecord has_bit_flags BOOLEAN_ATTRIBUTES, :field => "bit_prefs" - attr_accessor :password, :old_password, :signed_user_id + attr_reader :password after_initialize :initialize_attributes, if: :new_record? validates :name, user_name: true, on: :create @@ -78,8 +78,6 @@ class User < ApplicationRecord validates_presence_of :comment_threshold validate :validate_sock_puppets, :on => :create, :if => -> { Danbooru.config.enable_sock_puppet_validation? } before_validation :normalize_blacklisted_tags - before_create :encrypt_password_on_create - before_update :encrypt_password_on_update before_create :promote_to_admin_if_first_user before_create :customize_new_user has_many :artists, foreign_key: :creator_id @@ -170,26 +168,19 @@ class User < ApplicationRecord bcrypt_password_hash.slice(20, 100) end - def encrypt_password_on_create - self.bcrypt_password_hash = User.bcrypt(password) - end - - def encrypt_password_on_update - return if password.blank? - - if signed_user_id.present? && id == Danbooru::MessageVerifier.new(:login).verify(signed_user_id) - self.bcrypt_password_hash = User.bcrypt(password) - elsif old_password.present? && bcrypt_password == User.sha1(old_password) - self.bcrypt_password_hash = User.bcrypt(password) - else - errors[:old_password] << "is incorrect" - end + def password=(new_password) + @password = new_password + self.bcrypt_password_hash = User.bcrypt(new_password) end end module AuthenticationMethods extend ActiveSupport::Concern + def authenticate_login_key(signed_user_id) + signed_user_id.present? && id == Danbooru::MessageVerifier.new(:login).verify(signed_user_id) + end + module ClassMethods def authenticate(name, pass) authenticate_hash(name, sha1(pass)) diff --git a/app/policies/password_policy.rb b/app/policies/password_policy.rb index de693f981..a315c507e 100644 --- a/app/policies/password_policy.rb +++ b/app/policies/password_policy.rb @@ -2,8 +2,4 @@ class PasswordPolicy < ApplicationPolicy def update? record.id == user.id || user.is_admin? end - - def permitted_attributes - [:signed_user_id, :old_password, :password, :password_confirmation] - end end diff --git a/test/functional/passwords_controller_test.rb b/test/functional/passwords_controller_test.rb index e6bab032f..83057663d 100644 --- a/test/functional/passwords_controller_test.rb +++ b/test/functional/passwords_controller_test.rb @@ -14,13 +14,38 @@ class PasswordsControllerTest < ActionDispatch::IntegrationTest end context "update action" do - should "work" do + should "update the password when given a valid old password" 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_redirected_to @user assert_equal(nil, User.authenticate(@user.name, "12345")) assert_equal(@user, User.authenticate(@user.name, "abcde")) end + + should "update the password when given a valid login key" do + signed_user_id = Danbooru::MessageVerifier.new(:login).generate(@user.id) + put_auth user_password_path(@user), @user, params: { user: { password: "abcde", password_confirmation: "abcde", signed_user_id: signed_user_id } } + + assert_redirected_to @user + assert_equal(nil, User.authenticate(@user.name, "12345")) + assert_equal(@user, User.authenticate(@user.name, "abcde")) + end + + should "not update the password when given an invalid old password" do + put_auth user_password_path(@user), @user, params: { user: { old_password: "3qoirjqe", password: "abcde", password_confirmation: "abcde" } } + + assert_response :success + assert_equal(@user, User.authenticate(@user.name, "12345")) + assert_equal(nil, User.authenticate(@user.name, "abcde")) + end + + should "not update the password when password confirmation fails for the new password" do + put_auth user_password_path(@user), @user, params: { user: { old_password: "12345", password: "abcde", password_confirmation: "qerogijqe" } } + + assert_response :success + assert_equal(@user, User.authenticate(@user.name, "12345")) + assert_equal(nil, User.authenticate(@user.name, "abcde")) + end end end end diff --git a/test/functional/users_controller_test.rb b/test/functional/users_controller_test.rb index 689471895..c002f7b6c 100644 --- a/test/functional/users_controller_test.rb +++ b/test/functional/users_controller_test.rb @@ -118,6 +118,7 @@ class UsersControllerTest < ActionDispatch::IntegrationTest assert_redirected_to User.last assert_equal("xxx", User.last.name) + assert_equal(User.last, User.authenticate("xxx", "xxxxx1")) assert_equal(nil, User.last.email_address) assert_no_enqueued_emails end @@ -127,6 +128,7 @@ class UsersControllerTest < ActionDispatch::IntegrationTest assert_redirected_to User.last assert_equal("xxx", User.last.name) + assert_equal(User.last, User.authenticate("xxx", "xxxxx1")) assert_equal("webmaster@danbooru.donmai.us", User.last.email_address.address) assert_enqueued_email_with UserMailer, :welcome_user, args: [User.last] end