Skip to content

Commit

Permalink
Do not add unlock routes unless unlock strategy is email or both, closes
Browse files Browse the repository at this point in the history
  • Loading branch information
josevalim committed Jul 12, 2010
1 parent a87bc4a commit 2602ef4
Show file tree
Hide file tree
Showing 8 changed files with 23 additions and 29 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.rdoc
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 0 additions & 7 deletions app/controllers/devise/unlocks_controller.rb
Original file line number Diff line number Diff line change
@@ -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

Expand Down Expand Up @@ -32,10 +31,4 @@ def show
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
6 changes: 4 additions & 2 deletions lib/devise/rails/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -220,8 +220,10 @@ def devise_confirmation(mapping, controllers) #:nodoc:
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:
Expand Down
24 changes: 11 additions & 13 deletions test/integration/lockable_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -37,27 +37,25 @@ def visit_user_unlock_with_token(unlock_token)
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?"

swap Devise, :unlock_strategy => :time do
visit "/users/sign_in"
visit "/admins/sign_in"

assert_raise Webrat::NotFoundError do
click_link "Didn't receive unlock instructions?"
end
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 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
Expand All @@ -68,7 +66,7 @@ def visit_user_unlock_with_token(unlock_token)

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?
Expand Down
2 changes: 1 addition & 1 deletion test/mapping_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,8 @@ def fake_request(path, params={})
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
8 changes: 4 additions & 4 deletions test/models_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,16 +26,16 @@ def assert_include_modules(klass, *modules)
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

Expand Down
2 changes: 1 addition & 1 deletion test/rails_app/app/active_record/admin.rb
Original file line number Diff line number Diff line change
@@ -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
2 changes: 1 addition & 1 deletion test/rails_app/app/mongoid/admin.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

0 comments on commit 2602ef4

Please sign in to comment.