Add tests to cookie domain, closes #254.

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]

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

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
 

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?

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

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 || ''

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

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)

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