From ab903716bc0901335b99646c13d010cb410424be Mon Sep 17 00:00:00 2001 From: Kouhei Sutou Date: Tue, 19 Aug 2014 21:21:35 +0900 Subject: [PATCH] Try to fix wrong try usage `Object#try` checks "whether receiver is nil or not". It doesn't rescue any exceptions raised in method call. > User.new(name: "admin", password_digest: "invalid-hash").authenticate("X") BCrypt::Errors::InvalidHash: invalid hash from /var/lib/gems/2.1.0/gems/bcrypt-3.1.7/lib/bcrypt/password.rb:60:in `initialize' ... --- app/controllers/application_controller.rb | 3 +-- app/models/user.rb | 6 ++++++ 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index cda9ba7..6c56883 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -16,8 +16,7 @@ class ApplicationController < ActionController::Base def current_user return unless session[:succeed_password] - # NOTE: if hashed password is invalid or broken, .authenticate would raise error. Using `try` is avoid that situation - @current_user ||= User.new(name: "admin").try(:authenticate, session[:succeed_password]) + @current_user ||= User.new(name: "admin").authenticate(session[:succeed_password]) end def login_required diff --git a/app/models/user.rb b/app/models/user.rb index e5fcf2d..776b060 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -19,6 +19,12 @@ class User validates :password, length: { minimum: 8 } validate :valid_current_password + def authenticate(unencrypted_password) + super + rescue BCrypt::Errors::InvalidHash + false + end + def password_digest @password_digest || begin