From f79bb31ee68a71922ea544adec9a74814e6a5b13 Mon Sep 17 00:00:00 2001 From: Drew Ulmer Date: Thu, 25 Oct 2012 16:20:21 -0500 Subject: [PATCH 1/3] Add failing test for Issue #1994 Lockable should not leak information about account existence if paranoid mode is on. --- test/integration/lockable_test.rb | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/test/integration/lockable_test.rb b/test/integration/lockable_test.rb index cbea8755..50d4b519 100644 --- a/test/integration/lockable_test.rb +++ b/test/integration/lockable_test.rb @@ -221,4 +221,22 @@ class LockTest < ActionController::IntegrationTest end end + test "in paranoid mode, when locking a user that exists it should not say that the user was locked" do + swap Devise, :paranoid => true, :maximum_attempts => 1 do + user = create_user(:locked => false) + + visit new_user_session_path + fill_in 'email', :with => user.email + fill_in 'password', :with => "abadpassword" + click_button 'Sign in' + + fill_in 'email', :with => user.email + fill_in 'password', :with => "abadpassword" + click_button 'Sign in' + + assert_current_url "/users/sign_in" + assert_not_contain "locked" + end + end + end From 00e8841e9351b3e4792cb58fd2f21fe471ad862e Mon Sep 17 00:00:00 2001 From: Drew Ulmer Date: Thu, 25 Oct 2012 16:51:02 -0500 Subject: [PATCH 2/3] Unit test for Lockable#unauthenticated_message not leaking account existence This is another failing test for #1994, a unit test for Lockable. Lockable's #uanauthenticated_message should not return :locked if paranoid mode is on and instead should return the generic :invalid response. --- test/models/lockable_test.rb | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/test/models/lockable_test.rb b/test/models/lockable_test.rb index 18ea2edd..9bc5a43c 100644 --- a/test/models/lockable_test.rb +++ b/test/models/lockable_test.rb @@ -260,4 +260,14 @@ class LockableTest < ActiveSupport::TestCase end end end + + test 'should not return a locked unauthenticated message if in paranoid mode' do + swap Devise, :paranoid => :true do + user = create_user + user.failed_attempts = Devise.maximum_attempts + 1 + user.lock_access! + + assert_equal :invalid, user.unauthenticated_message + end + end end From 00a01c2bc494ce17269036fadd62ff14a76833ca Mon Sep 17 00:00:00 2001 From: Drew Ulmer Date: Thu, 25 Oct 2012 16:51:56 -0500 Subject: [PATCH 3/3] Fix #1994 Lockable no longer leaks account existence --- lib/devise/models/lockable.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/devise/models/lockable.rb b/lib/devise/models/lockable.rb index 98b810f8..56ccd6c6 100644 --- a/lib/devise/models/lockable.rb +++ b/lib/devise/models/lockable.rb @@ -105,7 +105,11 @@ module Devise end def unauthenticated_message - if lock_strategy_enabled?(:failed_attempts) && attempts_exceeded? + # If set to paranoid mode, do not show the locked message because it + # leaks the existence of an account. + if Devise.paranoid + super + elsif lock_strategy_enabled?(:failed_attempts) && attempts_exceeded? :locked else super