From 371a9bb0d020cf6e1f820f8c09140d14808b0550 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Tue, 3 Nov 2009 09:35:11 -0200 Subject: [PATCH] Major refactoring. Allow Warden::Manager to be configured through Devise, undeprecate Devise.confirm_within, Devise.pepper and friends and move Rails hooks to their own file. --- CHANGELOG.rdoc | 1 - lib/devise.rb | 57 ++++++++++++++------- lib/devise/failure.rb | 11 ++-- lib/devise/migrations.rb | 7 --- lib/devise/models.rb | 24 ++------- lib/devise/rails.rb | 17 +++++++ lib/devise/{ => rails}/routes.rb | 0 lib/devise/rails/warden_compat.rb | 26 ++++++++++ lib/devise/warden.rb | 41 --------------- test/devise_test.rb | 72 +++++++++++++++++++++++++++ test/integration/confirmable_test.rb | 4 +- test/integration/rememberable_test.rb | 4 +- test/mapping_test.rb | 2 +- test/models/authenticatable_test.rb | 14 +++--- test/models/confirmable_test.rb | 16 +++--- test/models/rememberable_test.rb | 20 ++++---- test/support/model_tests_helper.rb | 15 ++++++ 17 files changed, 211 insertions(+), 120 deletions(-) create mode 100644 lib/devise/rails.rb rename lib/devise/{ => rails}/routes.rb (100%) create mode 100644 lib/devise/rails/warden_compat.rb create mode 100644 test/devise_test.rb diff --git a/CHANGELOG.rdoc b/CHANGELOG.rdoc index d1c3d4c1..b0a0a849 100644 --- a/CHANGELOG.rdoc +++ b/CHANGELOG.rdoc @@ -2,7 +2,6 @@ * Notifier is deprecated, use DeviseMailer instead. Remember to rename app/views/notifier to app/views/devise_mailer and I18n key from devise.notifier to devise.mailer - * Model configuration is on Devise::Models.config and not on Devise.config anymore. * enhancement * [#16] Allow devise to be more agnostic. Do not require ActiveRecord to be loaded. diff --git a/lib/devise.rb b/lib/devise.rb index cf932b14..2be716a0 100644 --- a/lib/devise.rb +++ b/lib/devise.rb @@ -8,27 +8,50 @@ module Devise :confirmations => :confirmable }.freeze + STRATEGIES = [:rememberable, :authenticatable].freeze TRUE_VALUES = [true, 1, '1', 't', 'T', 'true', 'TRUE'].freeze -end -# Devise initialization process goes like this: -# -# 1) Includes in Devise::ActiveRecord and Devise::Migrations -# 2) Load and config warden -# 3) Load devise mapping structure -# 4) Add routes extensions -# 5) Load routes definitions -# 6) Include filters and helpers in controllers and views -# -Rails.configuration.after_initialize do - if defined?(ActiveRecord) - ActiveRecord::Base.extend Devise::Models - ActiveRecord::ConnectionAdapters::TableDefinition.send :include, Devise::Migrations + class << self + # Default way to setup Devise. Run script/generate devise_install to create + # a fresh initializer with all configuration values. + def setup + yield self + end + + # Sets the sender in DeviseMailer. + def mail_sender=(value) + DeviseMailer.sender = value + end + alias :sender= :mail_sender= + + # Sets warden configuration using a block that will be invoked on warden + # initialization. + # + # Devise.initialize do |config| + # config.confirm_within = 2.days + # + # config.warden do |manager| + # # Configure warden to use other strategies, like oauth. + # manager.oauth(:twitter) + # end + # end + def warden(&block) + @warden_config = block + end + + # A method used internally to setup warden manager from the Rails initialize + # block. + def configure_warden_manager(manager) #:nodoc: + manager.default_strategies *Devise::STRATEGIES + manager.failure_app = Devise::Failure + manager.silence_missing_strategies! + + # If the user provided a warden hook, call it now. + @warden_config.try :call, manager + end end - - I18n.load_path.unshift File.expand_path(File.join(File.dirname(__FILE__), 'devise', 'locales', 'en.yml')) end require 'devise/warden' require 'devise/mapping' -require 'devise/routes' +require 'devise/rails' diff --git a/lib/devise/failure.rb b/lib/devise/failure.rb index 52b89027..3d4ebcb9 100644 --- a/lib/devise/failure.rb +++ b/lib/devise/failure.rb @@ -9,10 +9,13 @@ module Devise def self.call(env) options = env['warden.options'] scope = options[:scope] - params = if env['warden'].try(:message) - { env['warden'].message => true } - else - options[:params] + params = case env['warden'].try(:message) + when Symbol + { env['warden'].message => true } + when String + { :message => env['warden'].message } + else + options[:params] end redirect_path = if mapping = Devise.mappings[scope] diff --git a/lib/devise/migrations.rb b/lib/devise/migrations.rb index edbdd620..822dec26 100644 --- a/lib/devise/migrations.rb +++ b/lib/devise/migrations.rb @@ -26,13 +26,6 @@ module Devise string :password_salt, :limit => 20, :null => null end - # TODO Remove me in a next release. - # - def authenticable(*args) - ActiveSupport::Deprecation.warn "authenticable in migrations is deprecated, use authenticatable instead" - authenticatable(*args) - end - # Creates confirmation_token, confirmed_at and confirmation_sent_at. # def confirmable diff --git a/lib/devise/models.rb b/lib/devise/models.rb index 41dad225..d94d2825 100644 --- a/lib/devise/models.rb +++ b/lib/devise/models.rb @@ -6,7 +6,7 @@ module Devise # # The line above creates: # - # 1) An accessor called Devise::Models.stretches, which value is used by default; + # 1) An accessor called Devise.stretches, which value is used by default; # # 2) Some class methods for your model Model.stretches and Model.stretches= # which have higher priority than Devise.stretches; @@ -17,17 +17,8 @@ module Devise # inside the given class. # def self.config(mod, accessor, default=nil) #:nodoc: - mattr_accessor accessor - send(:"#{accessor}=", default) - - # TODO Remove me in a next release - Devise.class_eval <<-METHOD, __FILE__, __LINE__ - def self.#{accessor}=(value) - ActiveSupport::Deprecation.warn "Devise.#{accessor}= is deprecated, " << - "use Devise::Models.#{accessor}= instead." - Devise::Models.#{accessor} = value - end - METHOD + Devise.send :mattr_accessor, accessor + Devise.send :"#{accessor}=", default mod.class_eval <<-METHOD, __FILE__, __LINE__ def #{accessor} @@ -42,7 +33,7 @@ module Devise elsif superclass.respond_to?(:#{accessor}) superclass.#{accessor} else - Devise::Models.#{accessor} + Devise.#{accessor} end end @@ -111,13 +102,6 @@ module Devise def devise(*modules) options = modules.extract_options! - # TODO Remove me in a next release - if modules.include?(:authenticable) - modules.delete(:authenticable) - modules.unshift(:authenticatable) - ActiveSupport::Deprecation.warn "devise :authenticate is deprecated, use authenticatable instead" - end - modules = Devise::ALL if modules.include?(:all) modules -= Array(options.delete(:except)) modules = [:authenticatable] | modules diff --git a/lib/devise/rails.rb b/lib/devise/rails.rb new file mode 100644 index 00000000..43c80edf --- /dev/null +++ b/lib/devise/rails.rb @@ -0,0 +1,17 @@ +require 'devise/rails/routes' +require 'devise/rails/warden_compat' + +Rails.configuration.after_initialize do + if defined?(ActiveRecord) + ActiveRecord::Base.extend Devise::Models + ActiveRecord::ConnectionAdapters::TableDefinition.send :include, Devise::Migrations + end + + # Adds Warden Manager to Rails middleware stack, configuring default devise + # strategy and also the failure app. + Rails.configuration.middleware.use Warden::Manager do |manager| + Devise.configure_warden_manager(manager) + end + + I18n.load_path.unshift File.expand_path(File.join(File.dirname(__FILE__), 'locales', 'en.yml')) +end diff --git a/lib/devise/routes.rb b/lib/devise/rails/routes.rb similarity index 100% rename from lib/devise/routes.rb rename to lib/devise/rails/routes.rb diff --git a/lib/devise/rails/warden_compat.rb b/lib/devise/rails/warden_compat.rb new file mode 100644 index 00000000..9e578390 --- /dev/null +++ b/lib/devise/rails/warden_compat.rb @@ -0,0 +1,26 @@ +# Taken from RailsWarden, thanks to Hassox. http://github.com/hassox/rails_warden +module Warden::Mixins::Common + # Gets the rails request object by default if it's available + def request + return @request if @request + if env['action_controller.rescue.request'] + @request = env['action_controller.rescue.request'] + else + Rack::Request.new(env) + end + end + + def raw_session + request.session + end + + def reset_session! + raw_session.inspect # why do I have to inspect it to get it to clear? + raw_session.clear + end + + # Proxy to request cookies + def cookies + request.cookies + end +end diff --git a/lib/devise/warden.rb b/lib/devise/warden.rb index 89b5d544..808990cc 100644 --- a/lib/devise/warden.rb +++ b/lib/devise/warden.rb @@ -5,33 +5,6 @@ rescue require 'warden' end -# Taken from RailsWarden, thanks to Hassox. http://github.com/hassox/rails_warden -module Warden::Mixins::Common - # Gets the rails request object by default if it's available - def request - return @request if @request - if env['action_controller.rescue.request'] - @request = env['action_controller.rescue.request'] - else - Rack::Request.new(env) - end - end - - def raw_session - request.session - end - - def reset_session! - raw_session.inspect # why do I have to inspect it to get it to clear? - raw_session.clear - end - - # Proxy to request cookies - def cookies - request.cookies - end -end - # Session Serialization in. This block determines how the user will be stored # in the session. If you're using a complex object like an ActiveRecord model, # it is not a good idea to store the complete object. An ID is sufficient. @@ -43,19 +16,5 @@ Warden::Manager.serialize_from_session do |klass, id| klass.find(id) end -# Be a good citizen and always set the controller action, even if Devise is -# never calling the failure app through warden. -Warden::Manager.before_failure do |env, opts| - env['warden'].request.params['action'] = 'new' -end - # Setup devise strategies for Warden require 'devise/strategies/base' - -# Adds Warden Manager to Rails middleware stack, configuring default devise -# strategy and also the controller who will manage not authenticated users. -Rails.configuration.middleware.use Warden::Manager do |manager| - manager.default_strategies :rememberable, :authenticatable - manager.failure_app = Devise::Failure - manager.silence_missing_strategies! -end diff --git a/test/devise_test.rb b/test/devise_test.rb new file mode 100644 index 00000000..4b7c7ba1 --- /dev/null +++ b/test/devise_test.rb @@ -0,0 +1,72 @@ +require 'test/test_helper' + +module Devise + def self.clean_warden_config! + @warden_config = nil + end +end + +class DeviseTest < ActiveSupport::TestCase + class MockManager + attr_accessor :failure_app + attr_reader :default_strategies, :silence_missing_strategies + + def silence_missing_strategies! + @silence_missing_strategies = true + end + + def default_strategies(*args) + if args.empty? + @default_strategies + else + @default_strategies = args + end + end + end + + test 'DeviseMailer.sender can be configured through Devise' do + swap DeviseMailer, :sender => "foo@bar" do + assert_equal "foo@bar", DeviseMailer.sender + Devise.mail_sender = "bar@foo" + assert_equal "bar@foo", DeviseMailer.sender + end + end + + test 'model options can be configured through Devise' do + swap Devise, :confirm_within => 113, :pepper => "foo" do + assert_equal 113, Devise.confirm_within + assert_equal "foo", Devise.pepper + end + end + + test 'setup block yields self' do + Devise.setup do |config| + assert_equal Devise, config + end + end + + test 'warden manager configuration' do + manager = MockManager.new + Devise.configure_warden_manager(manager) + + assert_equal Devise::Failure, manager.failure_app + assert_equal [:rememberable, :authenticatable], manager.default_strategies + assert manager.silence_missing_strategies + end + + test 'warden manager user configuration through a block' do + begin + @executed = false + Devise.warden do |manager| + @executed = true + assert_kind_of MockManager, manager + end + + manager = MockManager.new + Devise.configure_warden_manager(manager) + assert @executed + ensure + Devise.clean_warden_config! + end + end +end diff --git a/test/integration/confirmable_test.rb b/test/integration/confirmable_test.rb index f77050fd..39914bb1 100644 --- a/test/integration/confirmable_test.rb +++ b/test/integration/confirmable_test.rb @@ -59,7 +59,7 @@ class ConfirmationTest < ActionController::IntegrationTest end test 'not confirmed user and setup to block without confirmation should not be able to sign in' do - Devise::Models.confirm_within = 0 + Devise.confirm_within = 0 user = sign_in_as_user(:confirm => false) assert_redirected_to new_user_session_path(:unconfirmed => true) @@ -67,7 +67,7 @@ class ConfirmationTest < ActionController::IntegrationTest end test 'not confirmed user but configured with some days to confirm should be able to sign in' do - Devise::Models.confirm_within = 1 + Devise.confirm_within = 1 user = sign_in_as_user(:confirm => false) assert_response :success diff --git a/test/integration/rememberable_test.rb b/test/integration/rememberable_test.rb index 271112d7..9145491a 100644 --- a/test/integration/rememberable_test.rb +++ b/test/integration/rememberable_test.rb @@ -3,7 +3,7 @@ require 'test/test_helper' class RememberMeTest < ActionController::IntegrationTest def create_user_and_remember(add_to_token='') - Devise::Models.remember_for = 1 + Devise.remember_for = 1 user = create_user user.remember_me! cookies['remember_token'] = User.serialize_into_cookie(user) + add_to_token @@ -39,7 +39,7 @@ class RememberMeTest < ActionController::IntegrationTest test 'do not remember with token expired' do user = create_user_and_remember - Devise::Models.remember_for = 0 + Devise.remember_for = 0 get users_path assert_response :success assert_not warden.authenticated?(:user) diff --git a/test/mapping_test.rb b/test/mapping_test.rb index 35cd77ff..58d8658d 100644 --- a/test/mapping_test.rb +++ b/test/mapping_test.rb @@ -1,6 +1,6 @@ require 'test/test_helper' -class MapTest < ActiveSupport::TestCase +class MappingTest < ActiveSupport::TestCase test 'store options' do mapping = Devise.mappings[:user] diff --git a/test/models/authenticatable_test.rb b/test/models/authenticatable_test.rb index 0dbb375b..8219c17e 100644 --- a/test/models/authenticatable_test.rb +++ b/test/models/authenticatable_test.rb @@ -66,28 +66,28 @@ class AuthenticatableTest < ActiveSupport::TestCase test 'should fallback to devise pepper default configuring' do begin - Devise::Models.pepper = '' + Devise.pepper = '' user = new_user assert_equal encrypt_password(user), user.encrypted_password - Devise::Models.pepper = 'new_pepper' + Devise.pepper = 'new_pepper' user = new_user assert_equal encrypt_password(user, 'new_pepper'), user.encrypted_password - Devise::Models.pepper = '123456' + Devise.pepper = '123456' user = new_user assert_equal encrypt_password(user, '123456'), user.encrypted_password ensure - Devise::Models.pepper = nil + Devise.pepper = nil end end test 'should fallback to devise stretches default configuring' do begin - default_stretches = Devise::Models.stretches - Devise::Models.stretches = 1 + default_stretches = Devise.stretches + Devise.stretches = 1 user = new_user assert_equal encrypt_password(user, nil, nil), user.encrypted_password ensure - Devise::Models.stretches = default_stretches + Devise.stretches = default_stretches end end diff --git a/test/models/confirmable_test.rb b/test/models/confirmable_test.rb index 69222f2a..723d855e 100644 --- a/test/models/confirmable_test.rb +++ b/test/models/confirmable_test.rb @@ -179,20 +179,20 @@ class ConfirmableTest < ActiveSupport::TestCase test 'confirm time should fallback to devise confirm in default configuration' do begin - confirm_within = Devise::Models.confirm_within - Devise::Models.confirm_within = 1.day + confirm_within = Devise.confirm_within + Devise.confirm_within = 1.day user = new_user user.confirmation_sent_at = 2.days.ago assert_not user.active? - Devise::Models.confirm_within = 3.days + Devise.confirm_within = 3.days assert user.active? ensure - Devise::Models.confirm_within = confirm_within + Devise.confirm_within = confirm_within end end test 'should be active when confirmation sent at is not overpast' do - Devise::Models.confirm_within = 5.days + Devise.confirm_within = 5.days user = create_user user.confirmation_sent_at = 4.days.ago assert user.active? @@ -208,21 +208,21 @@ class ConfirmableTest < ActiveSupport::TestCase end test 'should not be active when confirmation was sent within the limit' do - Devise::Models.confirm_within = 5.days + Devise.confirm_within = 5.days user = create_user user.confirmation_sent_at = 5.days.ago assert_not user.active? end test 'should be active when confirm in is zero' do - Devise::Models.confirm_within = 0.days + Devise.confirm_within = 0.days user = create_user user.confirmation_sent_at = Date.today assert_not user.active? end test 'should not be active when confirmation was sent before confirm in time' do - Devise::Models.confirm_within = 4.days + Devise.confirm_within = 4.days user = create_user user.confirmation_sent_at = 5.days.ago assert_not user.active? diff --git a/test/models/rememberable_test.rb b/test/models/rememberable_test.rb index a302240c..396f35c0 100644 --- a/test/models/rememberable_test.rb +++ b/test/models/rememberable_test.rb @@ -3,7 +3,7 @@ require 'test/test_helper' class RememberableTest < ActiveSupport::TestCase def setup - Devise::Models.remember_for = 1 + Devise.remember_for = 1 end test 'should respond to remember_me attribute' do @@ -83,37 +83,37 @@ class RememberableTest < ActiveSupport::TestCase test 'remember for should fallback to devise remember for default configuration' do begin - remember_for = Devise::Models.remember_for + remember_for = Devise.remember_for user = create_user - Devise::Models.remember_for = 1.day + Devise.remember_for = 1.day user.remember_me! assert_not user.remember_expired? - Devise::Models.remember_for = 0.days + Devise.remember_for = 0.days user.remember_me! assert user.remember_expired? ensure - Devise::Models.remember_for = remember_for + Devise.remember_for = remember_for end end test 'remember expires at should sum date of creation with remember for configuration' do - Devise::Models.remember_for = 3.days + Devise.remember_for = 3.days user = create_user user.remember_me! assert_equal 3.days.from_now.to_date, user.remember_expires_at.to_date - Devise::Models.remember_for = 5.days + Devise.remember_for = 5.days assert_equal 5.days.from_now.to_date, user.remember_expires_at.to_date end test 'remember should be expired if remember_for is zero' do - Devise::Models.remember_for = 0.days + Devise.remember_for = 0.days user = create_user user.remember_me! assert user.remember_expired? end test 'remember should be expired if it was created before limit time' do - Devise::Models.remember_for = 1.day + Devise.remember_for = 1.day user = create_user user.remember_me! user.update_attribute(:remember_created_at, 2.days.ago) @@ -121,7 +121,7 @@ class RememberableTest < ActiveSupport::TestCase end test 'remember should not be expired if it was created whitin the limit time' do - Devise::Models.remember_for = 30.days + Devise.remember_for = 30.days user = create_user user.remember_me! user.update_attribute(:remember_created_at, 30.days.ago + 2.minutes) diff --git a/test/support/model_tests_helper.rb b/test/support/model_tests_helper.rb index 610ff7bd..8deb8d4b 100644 --- a/test/support/model_tests_helper.rb +++ b/test/support/model_tests_helper.rb @@ -33,4 +33,19 @@ class ActiveSupport::TestCase def create_user(attributes={}) User.create!(valid_attributes(attributes)) end + + # Execute the block setting the given values and restoring old values after + # the block is executed. + def swap(object, new_values) + old_values = {} + new_values.each do |key, value| + old_values[key] = object.send key + object.send :"#{key}=", value + end + yield + ensure + old_values.each do |key, value| + object.send :"#{key}=", value + end + end end