From 59bee679ca681b4203404e7344d63f7527b11520 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Sun, 16 May 2010 14:13:43 +0200 Subject: [PATCH] Add tests to cookie domain, closes #254. --- lib/devise/hooks/activatable.rb | 5 +++- lib/devise/hooks/forgetable.rb | 4 ++-- lib/devise/hooks/rememberable.rb | 9 +++---- lib/devise/models/authenticatable.rb | 7 +++--- lib/devise/models/rememberable.rb | 3 +-- lib/devise/strategies/authenticatable.rb | 24 +++++++++++++++---- .../strategies/database_authenticatable.rb | 2 +- .../strategies/token_authenticatable.rb | 4 ++-- test/integration/rememberable_test.rb | 18 +++++++++++--- 9 files changed, 52 insertions(+), 24 deletions(-) diff --git a/lib/devise/hooks/activatable.rb b/lib/devise/hooks/activatable.rb index 1d4f2378..d2d86d41 100644 --- a/lib/devise/hooks/activatable.rb +++ b/lib/devise/hooks/activatable.rb @@ -1,4 +1,7 @@ -# Deny user access whenever his account is not active yet. +# Deny user access whenever his account is not active yet. All strategies that inherits from +# Devise::Strategies::Authenticatable and uses the validate already check if the user is active? +# before actively signing him in. However, we need this as hook to validate the user activity +# in each request and in case the user is using other strategies beside Devise ones. Warden::Manager.after_set_user do |record, warden, options| if record && record.respond_to?(:active?) && !record.active? scope = options[:scope] diff --git a/lib/devise/hooks/forgetable.rb b/lib/devise/hooks/forgetable.rb index 57c2f979..5573eb4c 100644 --- a/lib/devise/hooks/forgetable.rb +++ b/lib/devise/hooks/forgetable.rb @@ -5,7 +5,7 @@ Warden::Manager.before_logout do |record, warden, options| if record.respond_to?(:forget_me!) record.forget_me! unless record.frozen? - - warden.cookies.delete("remember_#{options[:scope].to_s}_token", :domain => record.cookie_domain, :path => "/") + options = record.cookie_domain? ? { :domain => record.cookie_domain } : {} + warden.cookies.delete("remember_#{options[:scope]}_token", options) end end diff --git a/lib/devise/hooks/rememberable.rb b/lib/devise/hooks/rememberable.rb index e246e500..3989fff2 100644 --- a/lib/devise/hooks/rememberable.rb +++ b/lib/devise/hooks/rememberable.rb @@ -11,17 +11,14 @@ module Devise if succeeded? && resource.respond_to?(:remember_me!) && remember_me? resource.remember_me! - conf = { + configuration = { :value => resource.class.serialize_into_cookie(resource), :expires => resource.remember_expires_at, :path => "/" } - conf[:domain] = resource.cookie_domain if resource.cookie_domain? - - Warden::Manager.after_set_user do |record, warden, options| - warden.cookies["remember_#{options[:scope]}_token"] = conf - end + configuration[:domain] = resource.cookie_domain if resource.cookie_domain? + cookies.signed["remember_#{scope}_token"] = configuration end end diff --git a/lib/devise/models/authenticatable.rb b/lib/devise/models/authenticatable.rb index 15182171..71506b38 100644 --- a/lib/devise/models/authenticatable.rb +++ b/lib/devise/models/authenticatable.rb @@ -44,10 +44,11 @@ module Devise self.devise_modules ||= [] end - # Check if the current object is valid for authentication. This method and find_for_authentication - # are the methods used in a Warden::Strategy to check if a model should be signed in or not. + # Check if the current object is valid for authentication. This method and + # find_for_authentication are the methods used in a Warden::Strategy to check + # if a model should be signed in or not. # - # However, you should not need to overwrite this method, you should overwrite active? and + # However, you should not overwrite this method, you should overwrite active? and # inactive_message instead. def valid_for_authentication? if active? diff --git a/lib/devise/models/rememberable.rb b/lib/devise/models/rememberable.rb index 8e9cc8ff..f4ccb992 100644 --- a/lib/devise/models/rememberable.rb +++ b/lib/devise/models/rememberable.rb @@ -86,8 +86,7 @@ module Devise record if record && !record.remember_expired? end - Devise::Models.config(self, :remember_for) - Devise::Models.config(self, :cookie_domain) + Devise::Models.config(self, :remember_for, :cookie_domain) end end end diff --git a/lib/devise/strategies/authenticatable.rb b/lib/devise/strategies/authenticatable.rb index e251fdd8..005514ad 100644 --- a/lib/devise/strategies/authenticatable.rb +++ b/lib/devise/strategies/authenticatable.rb @@ -14,12 +14,23 @@ module Devise private - # Check if this is strategy is valid for http authentication. + # Check if this is strategy is valid for http authentication by: + # + # * Validating if the model allows params authentication; + # * If any of the authorization headers were sent; + # * If all authentication keys are present; + # def valid_for_http_auth? http_authenticatable? && request.authorization && with_authentication_hash(http_auth_hash) end - # Check if this is strategy is valid for params authentication. + # Check if this is strategy is valid for params authentication by: + # + # * Validating if the model allows params authentication; + # * If the request hits the sessions controller through POST; + # * If the params[scope] returns a hash with credentials; + # * If all authentication keys are present; + # def valid_for_params_auth? params_authenticatable? && valid_request? && valid_params? && with_authentication_hash(params_auth_hash) @@ -51,12 +62,12 @@ module Devise valid_controller? && valid_verb? end - # Check if the controller is valid for params authentication. + # Check if the controller is the one registered for authentication. def valid_controller? mapping.controllers[:sessions] == params[:controller] end - # Check if the params_auth_hash is valid for params authentication. + # Check if it was a POST request. def valid_verb? request.post? end @@ -66,6 +77,11 @@ module Devise params_auth_hash.is_a?(Hash) end + # Check if password is present and is not equal to "X" (default value for token). + def valid_password? + password.present? && password != "X" + end + # Helper to decode credentials from HTTP. def decode_credentials username_and_password = request.authorization.split(' ', 2).last || '' diff --git a/lib/devise/strategies/database_authenticatable.rb b/lib/devise/strategies/database_authenticatable.rb index f47e04ad..24c9db4a 100644 --- a/lib/devise/strategies/database_authenticatable.rb +++ b/lib/devise/strategies/database_authenticatable.rb @@ -5,7 +5,7 @@ module Devise # Default strategy for signing in a user, based on his email and password in the database. class DatabaseAuthenticatable < Authenticatable def authenticate! - resource = mapping.to.find_for_database_authentication(authentication_hash) + resource = valid_password? && mapping.to.find_for_database_authentication(authentication_hash) if validate(resource){ resource.valid_password?(password) } resource.after_database_authentication diff --git a/lib/devise/strategies/token_authenticatable.rb b/lib/devise/strategies/token_authenticatable.rb index 80412098..f45724b6 100644 --- a/lib/devise/strategies/token_authenticatable.rb +++ b/lib/devise/strategies/token_authenticatable.rb @@ -7,8 +7,8 @@ module Devise # # http://myapp.example.com/?user_token=SECRET # - # For HTTP, you can pass the token as username. Since some clients may require a password, - # you can pass anything and it will simply be ignored. + # For HTTP, you can pass the token as username and blank password. Since some clients may require + # a password, you can pass "X" as password and it will simply be ignored. class TokenAuthenticatable < Authenticatable def authenticate! resource = mapping.to.find_for_token_authentication(authentication_hash) diff --git a/test/integration/rememberable_test.rb b/test/integration/rememberable_test.rb index a7850516..3abd549d 100644 --- a/test/integration/rememberable_test.rb +++ b/test/integration/rememberable_test.rb @@ -19,12 +19,24 @@ class RememberMeTest < ActionController::IntegrationTest test 'do not remember the user if he has not checked remember me option' do user = sign_in_as_user + assert_nil request.cookies["remember_user_cookie"] assert_nil user.reload.remember_token end test 'generate remember token after sign in' do user = sign_in_as_user :remember_me => true - assert_not_nil user.reload.remember_token + assert request.cookies["remember_user_token"] + assert user.reload.remember_token + end + + test 'generate remember token after sign in setting cookie domain' do + # We test this by asserting the cookie is not sent after the redirect + # since we changed the domain. This is the only difference with the + # previous test. + swap User, :cookie_domain => "omg.somewhere.com" do + user = sign_in_as_user :remember_me => true + assert_nil request.cookies["remember_user_token"] + end end test 'remember the user before sign in' do @@ -35,7 +47,7 @@ class RememberMeTest < ActionController::IntegrationTest assert warden.user(:user) == user end - test 'does not remember other scopes' do + test 'do not remember other scopes' do user = create_user_and_remember get root_path assert_response :success @@ -50,7 +62,7 @@ class RememberMeTest < ActionController::IntegrationTest assert_redirected_to new_user_session_path end - test 'do not remember with token expired' do + test 'do not remember with expired token' do user = create_user_and_remember swap Devise, :remember_for => 0 do get users_path