From 2602ef41cfc76a0e60c387a57118dc6bf95b27a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Mon, 12 Jul 2010 07:24:21 +0200 Subject: [PATCH] Do not add unlock routes unless unlock strategy is email or both, closes #373 --- CHANGELOG.rdoc | 1 + app/controllers/devise/unlocks_controller.rb | 7 ------ lib/devise/rails/routes.rb | 6 +++-- test/integration/lockable_test.rb | 26 +++++++++----------- test/mapping_test.rb | 2 +- test/models_test.rb | 8 +++--- test/rails_app/app/active_record/admin.rb | 2 +- test/rails_app/app/mongoid/admin.rb | 2 +- 8 files changed, 24 insertions(+), 30 deletions(-) diff --git a/CHANGELOG.rdoc b/CHANGELOG.rdoc index 6e35224b..533d3b9e 100644 --- a/CHANGELOG.rdoc +++ b/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 diff --git a/app/controllers/devise/unlocks_controller.rb b/app/controllers/devise/unlocks_controller.rb index a813ddfc..d82c96b9 100644 --- a/app/controllers/devise/unlocks_controller.rb +++ b/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 diff --git a/lib/devise/rails/routes.rb b/lib/devise/rails/routes.rb index 550fe1d1..5f93d4e3 100644 --- a/lib/devise/rails/routes.rb +++ b/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], - :path => mapping.path_names[:unlock], :controller => controllers[:unlocks] + if mapping.to.unlock_strategy_enabled?(:email) + resource :unlock, :only => [:new, :create, :show], + :path => mapping.path_names[:unlock], :controller => controllers[:unlocks] + end end def devise_registration(mapping, controllers) #:nodoc: diff --git a/test/integration/lockable_test.rb b/test/integration/lockable_test.rb index 02b02175..d98ea29f 100644 --- a/test/integration/lockable_test.rb +++ b/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" - click_link "Didn't receive unlock instructions?" + visit "/admins/sign_in" - swap Devise, :unlock_strategy => :time do - visit "/users/sign_in" - - assert_raise Webrat::NotFoundError do - click_link "Didn't receive unlock instructions?" - end - - assert_raise AbstractController::ActionNotFound do - visit new_user_unlock_path - end + assert_raise Webrat::NotFoundError do + click_link "Didn't receive unlock instructions?" 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? diff --git a/test/mapping_test.rb b/test/mapping_test.rb index 386d0156..a02220e3 100644 --- a/test/mapping_test.rb +++ b/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 diff --git a/test/models_test.rb b/test/models_test.rb index 64ea4655..55966abf 100644 --- a/test/models_test.rb +++ b/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] - incorrect_module_order = [:database_authenticatable, :timeoutable, :registerable, :recoverable] + correct_module_order = [:database_authenticatable, :recoverable, :registerable, :lockable, :timeoutable] + incorrect_module_order = [:database_authenticatable, :timeoutable, :registerable, :recoverable, :lockable] assert_include_modules Admin, *incorrect_module_order diff --git a/test/rails_app/app/active_record/admin.rb b/test/rails_app/app/active_record/admin.rb index 5d8f1f6d..5b36afa6 100644 --- a/test/rails_app/app/active_record/admin.rb +++ b/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 diff --git a/test/rails_app/app/mongoid/admin.rb b/test/rails_app/app/mongoid/admin.rb index 2fd0f10b..76a5bfd0 100644 --- a/test/rails_app/app/mongoid/admin.rb +++ b/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