Improve previous patch.

lib/devise/models/authenticatable.rb

@@ -100,12 +100,7 @@ def http_authenticatable?(strategy)
         #   end
         #
         def find_for_authentication(conditions)
-          #the to_s is here to avoid mongodb injection where 'field => value' becomes 'field => {$ne => value}' thourgh the magic of rails
-          #still this does not prevent the leak if user1.token == '$ne' + user2.token (the chance of that is poor though)
-          #this might not be the best place or the best method, please change
-          conditions.each do |k, v|
-            conditions[k] = v.to_s
-          end
+          filter_auth_params(conditions)
           case_insensitive_keys.each { |k| conditions[k].try(:downcase!) }
           to_adapter.find_first(conditions)
         end
@@ -123,7 +118,7 @@ def find_or_initialize_with_errors(required_attributes, attributes, error=:inval
           attributes.delete_if { |key, value| value.blank? }
 
           if attributes.size == required_attributes.size
-            record = to_adapter.find_first(attributes)
+            record = to_adapter.find_first(filter_auth_params(attributes))
           end
 
           unless record
@@ -139,6 +134,15 @@ def find_or_initialize_with_errors(required_attributes, attributes, error=:inval
           record
         end
 
+        protected
+
+        # Force keys to be string to avoid injection on mongoid related database.
+        def filter_auth_params(conditions)
+          conditions.each do |k, v|
+            conditions[k] = v.to_s
+          end
+        end
+
         # Generate a token by looping and ensuring does not already exist.
         def generate_token(column)
           loop do

test/integration/token_authenticatable_test.rb

@@ -92,12 +92,16 @@ class TokenAuthenticationTest < ActionController::IntegrationTest
   test 'should not be subject to injection' do
     swap Devise, :token_authentication_key => :secret_token do
       user1 = create_user_with_authentication_token()
-      user2 = create_another_user_with_authentication_token(:auth_token => "ANOTHERTOKEN")
 
-      visit users_path(Devise.token_authentication_key.to_s + '[$ne]' => user1.authentication_token)
+      # Clean up user cache
+      @user = nil
 
-      assert warden.user(:user) == nil
+      user2 = create_user_with_authentication_token(:email => "another@test.com")
+      user2.update_attribute(:authentication_token, "ANOTHERTOKEN")
 
+      assert_not_equal user1, user2
+      visit users_path(Devise.token_authentication_key.to_s + '[$ne]' => user1.authentication_token)
+      assert_nil warden.user(:user) 
     end
   end
 
@@ -119,30 +123,13 @@ def sign_in_as_new_user_with_token(options = {})
       user
     end
 
-    def create_user_with_authentication_token(options = {})
+    def create_user_with_authentication_token(options={})
       user = create_user(options)
-      user.authentication_token = options[:auth_token] || VALID_AUTHENTICATION_TOKEN
+      user.authentication_token = VALID_AUTHENTICATION_TOKEN
       user.save
       user
     end
 
-    def create_another_user_with_authentication_token(options = {})
-      @anotheruser ||= begin
-        user = User.create!(
-          :username => 'anotherusertest',
-          :email => options[:email] || 'anotheruser@test.com',
-          :password => options[:password] || '123456',
-          :password_confirmation => options[:password] || '123456',
-          :created_at => Time.now.utc
-        )
-        user.confirm! unless options[:confirm] == false
-        user.lock_access! if options[:locked] == true
-        user.authentication_token = options[:auth_token] || VALID_AUTHENTICATION_TOKEN
-        user.save
-        user
-      end
-    end
-
     def get_users_path_as_existing_user(user)
       sign_in_as_new_user_with_token(:user => user)
     end

test/models/token_authenticatable_test.rb

@@ -41,25 +41,15 @@ class TokenAuthenticatableTest < ActiveSupport::TestCase
   end
 
   test 'should not be subject to injection' do
+    user1 = create_user
+    user1.ensure_authentication_token!
+    user1.confirm!
 
-    if DEVISE_ORM != :mongoid
-      assert_nil(nil)
-    else
-
-      user1 = create_user
-      user1.ensure_authentication_token!
-      user1.confirm!
-
-      user2 = create_user
-      user2.ensure_authentication_token!
-      user2.confirm!
-
-      #now trick it
-      user = User.find_for_token_authentication(:auth_token => {'$ne' => user1.authentication_token})
-
-      assert_nil user
-    end
+    user2 = create_user
+    user2.ensure_authentication_token!
+    user2.confirm!
 
+    user = User.find_for_token_authentication(:auth_token => {'$ne' => user1.authentication_token})
+    assert_nil user
   end
-
 end