Do not add unlock routes unless unlock strategy is email or both, closes #373

CHANGELOG.rdoc

@@ -10,6 +10,7 @@
   * Fix a bug in Devise::TestHelpers where current_user was returning a Response object for non active accounts
   * Devise should respect script_name and path_info contracts
   * Fix a bug when accessing a path with (.:format) (by github.com/klacointe)
+  * Do not add unlock routes unless unlock strategy is email or both
 
 * deprecations
   * use_default_scope is deprecated and has no effect. Use :as or :devise_scope in the router instead

app/controllers/devise/unlocks_controller.rb

@@ -1,5 +1,4 @@
 class Devise::UnlocksController < ApplicationController
-  prepend_before_filter :ensure_email_as_unlock_strategy
   prepend_before_filter :require_no_authentication
   include Devise::Controllers::InternalHelpers
 
@@ -32,10 +31,4 @@ class Devise::UnlocksController < ApplicationController
       render_with_scope :new
     end
   end
-
-  protected
-
-  def ensure_email_as_unlock_strategy
-    raise ActionController::UnknownAction unless resource_class.unlock_strategy_enabled?(:email)
-  end
 end

lib/devise/rails/routes.rb

@@ -220,8 +220,10 @@ module ActionDispatch::Routing
       end
  
       def devise_unlock(mapping, controllers) #:nodoc:
-        resource :unlock, :only => [:new, :create, :show],
+        if mapping.to.unlock_strategy_enabled?(:email)
-          :path => mapping.path_names[:unlock], :controller => controllers[:unlocks]
+          resource :unlock, :only => [:new, :create, :show],
+            :path => mapping.path_names[:unlock], :controller => controllers[:unlocks]
+        end
       end
 
       def devise_registration(mapping, controllers) #:nodoc:

test/integration/lockable_test.rb

@@ -37,27 +37,25 @@ class LockTest < ActionController::IntegrationTest
   end
 
   test 'unlocked pages should not be available if email strategy is disabled' do
-    visit "/users/sign_in"
+    visit "/admins/sign_in"
-    click_link "Didn't receive unlock instructions?"
 
-    swap Devise, :unlock_strategy => :time do
+    assert_raise Webrat::NotFoundError do
-      visit "/users/sign_in"
+      click_link "Didn't receive unlock instructions?"
-
-      assert_raise Webrat::NotFoundError do
-        click_link "Didn't receive unlock instructions?"
-      end
-
-      assert_raise AbstractController::ActionNotFound do
-        visit new_user_unlock_path
-      end
     end
+
+    assert_raise NameError do
+      visit new_admin_unlock_path
+    end
+
+    visit "/admins/unlock/new"
+    assert_response :not_found
   end
 
   test 'user with invalid unlock token should not be able to unlock an account' do
     visit_user_unlock_with_token('invalid_token')
 
     assert_response :success
-    assert_template 'unlocks/new'
+    assert_current_url '/users/unlock?unlock_token=invalid_token'
     assert_have_selector '#error_explanation'
     assert_contain /Unlock token(.*)invalid/
   end
@@ -68,7 +66,7 @@ class LockTest < ActionController::IntegrationTest
 
     visit_user_unlock_with_token(user.unlock_token)
 
-    assert_template 'home/index'
+    assert_current_url '/'
     assert_contain 'Your account was successfully unlocked.'
 
     assert_not user.reload.access_locked?

test/mapping_test.rb

@@ -78,8 +78,8 @@ class MappingTest < ActiveSupport::TestCase
     mapping = Devise.mappings[:admin]
     assert mapping.authenticatable?
     assert mapping.recoverable?
+    assert mapping.lockable?
     assert_not mapping.confirmable?
-    assert_not mapping.lockable?
     assert_not mapping.rememberable?
   end
 end

test/models_test.rb

@@ -26,16 +26,16 @@ class ActiveRecordTest < ActiveSupport::TestCase
   end
 
   test 'can cherry pick modules' do
-    assert_include_modules Admin, :database_authenticatable, :registerable, :timeoutable, :recoverable
+    assert_include_modules Admin, :database_authenticatable, :registerable, :timeoutable, :recoverable, :lockable
   end
 
   test 'chosen modules are inheritable' do
-    assert_include_modules Inheritable, :database_authenticatable, :registerable, :timeoutable, :recoverable
+    assert_include_modules Inheritable, :database_authenticatable, :registerable, :timeoutable, :recoverable, :lockable
   end
 
   test 'order of module inclusion' do
-    correct_module_order   = [:database_authenticatable, :recoverable, :registerable, :timeoutable]
+    correct_module_order   = [:database_authenticatable, :recoverable, :registerable, :lockable, :timeoutable]
-    incorrect_module_order = [:database_authenticatable, :timeoutable, :registerable, :recoverable]
+    incorrect_module_order = [:database_authenticatable, :timeoutable, :registerable, :recoverable, :lockable]
 
     assert_include_modules Admin, *incorrect_module_order
 

test/rails_app/app/active_record/admin.rb

@@ -1,3 +1,3 @@
 class Admin < ActiveRecord::Base
-  devise :database_authenticatable, :registerable, :timeoutable, :recoverable
+  devise :database_authenticatable, :registerable, :timeoutable, :recoverable, :lockable, :unlock_strategy => :time
 end

test/rails_app/app/mongoid/admin.rb

@@ -2,5 +2,5 @@ class Admin
   include Mongoid::Document
   include Shim
 
-  devise :database_authenticatable, :timeoutable, :registerable, :recoverable
+  devise :database_authenticatable, :timeoutable, :registerable, :recoverable, :lockable, :unlock_strategy => :time
 end