Allow several authentications to share a common path.

Gemfile

@@ -1,7 +1,7 @@
 source "http://gemcutter.org"
 
 # Need to install Rails from source
-gem "rails", :git => "git://github.com/rails/rails.git"
+gem "rails", "3.0.0.beta1"#:git => "git://github.com/rails/rails.git"
 gem "warden", "0.10.2"
 gem "sqlite3-ruby", :require => "sqlite3"
 gem "webrat", "0.7"

lib/devise/models/authenticatable.rb

@@ -14,6 +14,12 @@ module Devise
     module Authenticatable
       extend ActiveSupport::Concern
 
+      # Yields the given block. This method is overwritten by other modules to provide
+      # hooks around authentication.
+      def valid_for_authentication?
+        yield
+      end
+
       module ClassMethods
         Devise::Models.config(self, :authentication_keys, :http_authenticatable)
 

lib/devise/models/database_authenticatable.rb

@@ -22,15 +22,13 @@ module Devise
     #
     # Examples:
     #
-    #    User.authenticate('email@test.com', 'password123')  # returns authenticated user or nil
     #    User.find(1).valid_password?('password123')         # returns true/false
     #
     module DatabaseAuthenticatable
       extend  ActiveSupport::Concern
-
-      included do
       include Devise::Models::Authenticatable
 
+      included do
         attr_reader :password, :current_password
         attr_accessor :password_confirmation
       end
@@ -51,11 +49,6 @@ module Devise
         password_digest(incoming_password) == self.encrypted_password
       end
 
-      # Checks if a resource is valid upon authentication.
-      def valid_for_authentication?(attributes)
-        valid_password?(attributes[:password])
-      end
-
       # Set password and password confirmation to nil
       def clean_up_passwords
         self.password = self.password_confirmation = nil
@@ -92,13 +85,6 @@ module Devise
       module ClassMethods
         Devise::Models.config(self, :pepper, :stretches, :encryptor)
 
-        # Authenticate a user based on configured attribute keys. Returns the
-        # authenticated user if it's valid or nil.
-        def authenticate(conditions)
-          resource = find_for_database_authentication(conditions.except(:password))
-          resource if resource.try(:valid_for_authentication?, conditions)
-        end
-
         # Returns the class for the configured encryptor.
         def encryptor_class
           @encryptor_class ||= ::Devise::Encryptors.const_get(encryptor.to_s.classify)

lib/devise/models/lockable.rb

@@ -23,7 +23,6 @@ module Devise
 
       # Lock an user setting it's locked_at to actual time.
       def lock_access!
-        return true if access_locked?
         self.locked_at = Time.now
 
         if self.class.unlock_strategy_enabled?(:email)
@@ -35,6 +34,7 @@ module Devise
       end
 
       # Unlock an user by cleaning locket_at and failed_attempts.
+      # TODO Check if unlock_token is available.
       def unlock_access!
         if_access_locked do
           self.locked_at = nil
@@ -74,19 +74,31 @@ module Devise
       # Overwrites valid_for_authentication? from Devise::Models::Authenticatable
       # for verifying whether an user is allowed to sign in or not. If the user
       # is locked, it should never be allowed.
-      def valid_for_authentication?(attributes)
+      def valid_for_authentication?
+        return :locked if access_locked?
+        return super unless persisted?
+
         if result = super
           self.failed_attempts = 0
         else
           self.failed_attempts += 1
-          lock_access! if failed_attempts > self.class.maximum_attempts
+
+          if attempts_exceeded?
+            lock_access!
+            return :locked
           end
+        end
+
         save(:validate => false) if changed?
         result
       end
 
       protected
 
+        def attempts_exceeded?
+          self.failed_attempts > self.class.maximum_attempts
+        end
+
         # Generates unlock token
         def generate_unlock_token
           self.unlock_token = self.class.unlock_token

lib/devise/strategies/authenticatable.rb

@@ -2,6 +2,9 @@ require 'devise/strategies/base'
 
 module Devise
   module Strategies
+    # This strategy should be used as basis for authentication strategies. It retrieves
+    # parameters both from params or from http authorization headers. See database_authenticatable
+    # for an example.
     class Authenticatable < Base
       attr_accessor :authentication_hash, :password
 
@@ -11,6 +14,18 @@ module Devise
 
     private
 
+      # Simply invokes valid_for_authentication? with the given block and deal with the result.
+      def validate(resource, &block)
+        result = resource && resource.valid_for_authentication?(&block)
+
+        case result
+        when Symbol, String
+          fail!(result)
+        else
+          result
+        end 
+      end
+
       def valid_for_http_auth?
         mapping.to.http_authenticatable? && request.authorization && set_http_auth_hash
       end

lib/devise/strategies/base.rb

@@ -12,10 +12,10 @@ module Devise
         end
       end
 
-      # TODO Move to a module
-      def success!(record)
-        if record.respond_to?(:active?) && !record.active?
-          fail!(record.inactive_message)
+      # Check if the resource is active before signing him in once and for all.
+      def success!(resource)
+        if resource.respond_to?(:active?) && !resource.active?
+          fail!(resource.inactive_message)
         else
           super
         end

lib/devise/strategies/database_authenticatable.rb

@@ -2,29 +2,17 @@ require 'devise/strategies/authenticatable'
 
 module Devise
   module Strategies
-    # Default strategy for signing in a user, based on his email and password.
-    # Redirects to sign_in page if it's not authenticated
+    # Default strategy for signing in a user, based on his email and password in the database.
     class DatabaseAuthenticatable < Authenticatable
-      # Authenticate a user based on email and password params, returning to warden
-      # success and the authenticated user if everything is okay. Otherwise redirect
-      # to sign in page.
       def authenticate!
-        if resource = mapping.to.authenticate(authentication_hash.merge(:password => password))
+        resource = mapping.to.find_for_database_authentication(authentication_hash)
+
+        if validate(resource){ resource.valid_password?(password) }
           success!(resource)
         else
           fail(:invalid)
         end
       end
-
-      protected
-
-        def valid_controller?
-          mapping.controllers[:sessions] == params[:controller]
-        end
-
-        def valid_params?
-          params[scope] && params[scope][:password].present?
-        end
     end
   end
 end

test/integration/lockable_test.rb

@@ -85,7 +85,7 @@ class LockTest < ActionController::IntegrationTest
     ActionMailer::Base.deliveries.clear
 
     sign_in_as_user(:password => "invalid")
-    assert_contain 'Invalid email or password.'
+    assert_contain 'Your account is locked.'
     assert ActionMailer::Base.deliveries.empty?
   end
 

test/models/confirmable_test.rb

@@ -79,13 +79,6 @@ class ConfirmableTest < ActiveSupport::TestCase
     assert_equal "was already confirmed", confirmed_user.errors[:email].join
   end
 
-  test 'should authenticate a confirmed user' do
-    user = create_user
-    user.confirm!
-    authenticated_user = User.authenticate(:email => user.email, :password => user.password)
-    assert_equal authenticated_user, user
-  end
-
   test 'should send confirmation instructions by email' do
     assert_email_sent do
       create_user

test/models/lockable_test.rb

@@ -1,42 +1,33 @@
 require 'test_helper'
 
 class LockableTest < ActiveSupport::TestCase
-
   def setup
     setup_mailer
   end
 
-  test "should increment failed attempts on unsuccessful authentication" do
-    user = create_user
-    assert_equal 0, user.failed_attempts
-
-    authenticated_user = User.authenticate(:email => user.email, :password => "anotherpassword")
-    assert_equal 1, user.reload.failed_attempts
-  end
-
-  test "should lock account base on maximum_attempts" do
-    user = create_user
-    attempts = Devise.maximum_attempts + 1
-    attempts.times { authenticated_user = User.authenticate(:email => user.email, :password => "anotherpassword") }
-    assert user.reload.access_locked?
-  end
-
   test "should respect maximum attempts configuration" do
     user = create_user
     swap Devise, :maximum_attempts => 2 do
-      3.times { authenticated_user = User.authenticate(:email => user.email, :password => "anotherpassword") }
+      3.times { user.valid_for_authentication?{ false } }
       assert user.reload.access_locked?
     end
   end
 
-  test "should clear failed_attempts on successfull sign in" do
+  test "should clear failed_attempts on successfull validation" do
     user = create_user
-    User.authenticate(:email => user.email, :password => "anotherpassword")
+    user.valid_for_authentication?{ false }
     assert_equal 1, user.reload.failed_attempts
-    User.authenticate(:email => user.email, :password => "123456")
+    user.valid_for_authentication?{ true }
     assert_equal 0, user.reload.failed_attempts
   end
 
+  test 'should be valid for authentication with a unlocked user' do
+    user = create_user
+    user.lock_access!
+    user.unlock_access!
+    assert user.valid_for_authentication?{ true }
+  end
+
   test "should verify whether a user is locked or not" do
     user = create_user
     assert_not user.access_locked?
@@ -64,14 +55,6 @@ class LockableTest < ActiveSupport::TestCase
     assert 0, user.reload.failed_attempts
   end
 
-  test "should not lock a locked account" do
-    user = create_user
-    user.lock_access!
-    assert_no_difference "ActionMailer::Base.deliveries.size" do
-      user.lock_access!
-    end
-  end
-
   test 'should not unlock an unlocked user' do
     user = create_user
     assert_not user.unlock_access!
@@ -166,14 +149,6 @@ class LockableTest < ActiveSupport::TestCase
     assert_equal "can't be blank", locked_user.errors[:unlock_token].join
   end
 
-  test 'should authenticate a unlocked user' do
-    user = create_user
-    user.lock_access!
-    user.unlock_access!
-    authenticated_user = User.authenticate(:email => user.email, :password => user.password)
-    assert_equal authenticated_user, user
-  end
-
   test 'should find a user to send unlock instructions' do
     user = create_user
     user.lock_access!