Merge pull request #1534 from jigyasa/master

Fix for a bug: Incorrect message for locked account

lib/devise/models/lockable.rb

@@ -79,7 +79,7 @@ module Devise
         # if the user can login or not (wrong password, etc)
         unlock_access! if lock_expired?
 
-        if super
+        if super && !access_locked?
           self.failed_attempts = 0
           save(:validate => false)
           true

test/integration/lockable_test.rb

@@ -92,13 +92,6 @@ class LockTest < ActionController::IntegrationTest
     assert_not warden.authenticated?(:user)
   end
 
-  test "user should not be able to sign in when locked" do
-    user = sign_in_as_user(:locked => true)
-    assert_template 'sessions/new'
-    assert_contain 'Your account is locked.'
-    assert_not warden.authenticated?(:user)
-  end
-
   test "user should not send a new e-mail if already locked" do
     user = create_user(:locked => true)
     user.failed_attempts = User.maximum_attempts + 1
@@ -113,10 +106,29 @@ class LockTest < ActionController::IntegrationTest
 
   test 'error message is configurable by resource name' do
     store_translations :en, :devise => {
-      :failure => { :user => { :locked => "You are locked!" } }
+        :failure => {:user => {:locked => "You are locked!"}}
     } do
-      user = sign_in_as_user(:locked => true)
-      assert_contain 'You are locked!'
+
+      user = create_user(:locked => true)
+      user.failed_attempts = User.maximum_attempts + 1
+      user.save!
+
+      sign_in_as_user(:password => "invalid")
+      assert_contain "You are locked!"
+    end
+  end
+
+  test "user should not be able to sign in when locked" do
+    store_translations :en, :devise => {
+        :failure => {:user => {:locked => "You are locked!"}}
+    } do
+
+      user = create_user(:locked => true)
+      user.failed_attempts = User.maximum_attempts + 1
+      user.save!
+
+      sign_in_as_user(:password => "123456")
+      assert_contain "You are locked!"
     end
   end
 
@@ -157,7 +169,7 @@ class LockTest < ActionController::IntegrationTest
 
   test "when using json to ask a unlock request, should not return the user" do
     user = create_user(:locked => true)
-    post  user_unlock_path(:format => "json", :user => {:email => user.email})
+    post user_unlock_path(:format => "json", :user => {:email => user.email})
     assert_response :success
     assert_equal response.body, {}.to_json
   end

test/models/lockable_test.rb

@@ -23,6 +23,19 @@ class LockableTest < ActiveSupport::TestCase
     assert_equal 0, user.reload.failed_attempts
   end
 
+  test "should increment failed_attempts on successfull validation if the user is already locked" do
+    user = create_user
+    user.confirm!
+
+    swap Devise, :maximum_attempts => 2 do
+      3.times { user.valid_for_authentication?{ false } }
+      assert user.reload.access_locked?
+    end
+
+    user.valid_for_authentication?{ true }
+    assert_equal 4, user.reload.failed_attempts
+  end
+
   test "should not touch failed_attempts if lock_strategy is none" do
     user = create_user
     user.confirm!