From d4e5424360d49f7355602e064eecf0d3b93ee075 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Tue, 22 May 2012 14:10:06 +0200 Subject: [PATCH] Simplify validation logic inside strategies --- CHANGELOG.rdoc | 3 +++ lib/devise/strategies/authenticatable.rb | 15 +++++++++++++-- lib/devise/strategies/database_authenticatable.rb | 3 +-- lib/devise/strategies/rememberable.rb | 8 +++++--- lib/devise/strategies/token_authenticatable.rb | 3 +-- 5 files changed, 23 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.rdoc b/CHANGELOG.rdoc index e87dd4e3..678a1648 100644 --- a/CHANGELOG.rdoc +++ b/CHANGELOG.rdoc @@ -1,3 +1,6 @@ +* deprecations + * Strategy#validate() no longer validates nil resources + == 2.1.0 Notes: https://github.com/plataformatec/devise/wiki/How-to:-upgrade-to-devise-2.1 diff --git a/lib/devise/strategies/authenticatable.rb b/lib/devise/strategies/authenticatable.rb index 16185fb1..1abd2afe 100644 --- a/lib/devise/strategies/authenticatable.rb +++ b/lib/devise/strategies/authenticatable.rb @@ -18,13 +18,24 @@ module Devise private - # Simply invokes valid_for_authentication? with the given block and deal with the result. + # Receives a resource and check if it is valid by calling valid_for_authentication? + # An optional block that will be triggered while validating can be optionally + # given as parameter. Check Devise::Models::Authenticable.valid_for_authentication? + # for more information. + # + # In case the resource can't be validated, it will fail with the given + # unauthenticated_message. def validate(resource, &block) + unless resource + ActiveSupport::Depreation.warn "an empty resource was given to #{self.class.name}#validate. " \ + "Please ensure the resource is not nil", caller + end + result = resource && resource.valid_for_authentication?(&block) case result when Symbol, String - ActiveSupport::Deprecation.warn "valid_for_authentication should return a boolean value" + ActiveSupport::Deprecation.warn "valid_for_authentication? should return a boolean value" fail!(result) return false end diff --git a/lib/devise/strategies/database_authenticatable.rb b/lib/devise/strategies/database_authenticatable.rb index 447963c5..4552d7a3 100644 --- a/lib/devise/strategies/database_authenticatable.rb +++ b/lib/devise/strategies/database_authenticatable.rb @@ -6,12 +6,11 @@ module Devise class DatabaseAuthenticatable < Authenticatable def authenticate! resource = valid_password? && mapping.to.find_for_database_authentication(authentication_hash) + return fail(:invalid) unless resource if validate(resource){ resource.valid_password?(password) } resource.after_database_authentication success!(resource) - elsif !halted? - fail(:invalid) end end end diff --git a/lib/devise/strategies/rememberable.rb b/lib/devise/strategies/rememberable.rb index 5bd95b23..68b91b1c 100644 --- a/lib/devise/strategies/rememberable.rb +++ b/lib/devise/strategies/rememberable.rb @@ -19,11 +19,13 @@ module Devise def authenticate! resource = mapping.to.serialize_from_cookie(*remember_cookie) + unless resource + cookies.delete(remember_key) + return pass + end + if validate(resource) success!(resource) - elsif !halted? - cookies.delete(remember_key) - pass end end diff --git a/lib/devise/strategies/token_authenticatable.rb b/lib/devise/strategies/token_authenticatable.rb index 1e1d9cb7..6a9df223 100644 --- a/lib/devise/strategies/token_authenticatable.rb +++ b/lib/devise/strategies/token_authenticatable.rb @@ -16,12 +16,11 @@ module Devise def authenticate! resource = mapping.to.find_for_token_authentication(authentication_hash) + return fail(:invalid_token) unless resource if validate(resource) resource.after_token_authentication success!(resource) - elsif !halted? - fail(:invalid_token) end end