diff --git a/CHANGELOG.rdoc b/CHANGELOG.rdoc index 2f5ac673..723f7997 100644 --- a/CHANGELOG.rdoc +++ b/CHANGELOG.rdoc @@ -3,12 +3,16 @@ * enhancements * Rememberable module allows user to be remembered across browsers and is enabled by default (by github.com/trevorturk) * devise_for can now be used together with scope method in routes but with a few limitations (check the documentation) + * Support :as or :devise_scope in the router to specify controller access scope * bug fix * 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) +* deprecations + * use_default_scope is deprecated and has no effect. Use :as or :devise_scope in the router instead + == 1.1.rc2 * enhancements diff --git a/Gemfile.lock b/Gemfile.lock index 26acb566..fc0ef96b 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,17 +1,17 @@ GIT remote: git://github.com/durran/mongoid.git - revision: 79b4d37 + revision: 54e8d67 specs: - mongoid (2.0.0.beta7) + mongoid (2.0.0.beta9) activemodel (~> 3.0.0.beta) - bson (~> 1.0.1) - mongo (~> 1.0.1) + bson (~> 1.0.3) + mongo (~> 1.0.3) tzinfo (~> 0.3.22) will_paginate (~> 3.0.pre) - mongoid (2.0.0.beta7) + mongoid (2.0.0.beta9) activemodel (~> 3.0.0.beta) - bson (~> 1.0.1) - mongo (~> 1.0.1) + bson (~> 1.0.3) + mongo (~> 1.0.3) tzinfo (~> 0.3.22) will_paginate (~> 3.0.pre) diff --git a/lib/devise.rb b/lib/devise.rb index d939642d..22c598e3 100644 --- a/lib/devise.rb +++ b/lib/devise.rb @@ -137,10 +137,6 @@ module Devise mattr_accessor :unlock_in @@unlock_in = 1.hour - # Tell when to use the default scope, if one cannot be found from routes. - mattr_accessor :use_default_scope - @@use_default_scope = false - # The default scope which is used by warden. mattr_accessor :default_scope @@default_scope = nil @@ -166,9 +162,12 @@ module Devise mattr_accessor :sign_out_all_scopes @@sign_out_all_scopes = false - # When set to true, optional segments in Devise no longer raises an error. - mattr_accessor :ignore_optional_segments - @@ignore_otional_segments = false + def self.use_default_scope=(*) + ActiveSupport::Deprecation.warn "config.use_default_scope is deprecated and removed from Devise. " << + "If you are using non conventional routes in Devise, all you need to do is to pass the devise " << + "scope in the router DSL:\n\n as :user do\n get \"sign_in\", :to => \"devise/sessions\"\n end\n\n" << + "The method :as is also aliased to :devise_scope. Choose the one you prefer.", caller + end # Default way to setup Devise. Run rails generate devise_install to create # a fresh initializer with all configuration values. @@ -187,10 +186,8 @@ module Devise end self.mailer = "Devise::Mailer" - # Register a model in Devise. You can call this manually if you don't want - # to use devise routes. Check devise_for in routes to know which options - # are available. - def self.add_model(resource, options) + # Small method that adds a mapping to Devise. + def self.add_mapping(resource, options) mapping = Devise::Mapping.new(resource, options) self.mappings[mapping.name] = mapping self.default_scope ||= mapping.name diff --git a/lib/devise/controllers/internal_helpers.rb b/lib/devise/controllers/internal_helpers.rb index eeec2c15..7be60c94 100644 --- a/lib/devise/controllers/internal_helpers.rb +++ b/lib/devise/controllers/internal_helpers.rb @@ -37,11 +37,7 @@ module Devise # Attempt to find the mapped route for devise based on request path def devise_mapping - @devise_mapping ||= begin - mapping = Devise::Mapping.find_by_path(request) - mapping ||= Devise.mappings[Devise.default_scope] if Devise.use_default_scope - mapping - end + @devise_mapping ||= request.env["devise.mapping"] end # Overwrites devise_controller? to return true @@ -53,8 +49,7 @@ module Devise # Checks whether it's a devise mapped resource or not. def is_devise_resource? #:nodoc: - raise ActionController::UnknownAction unless devise_mapping && - devise_mapping.allowed_controllers.include?(controller_path) + raise ActionController::UnknownAction unless devise_mapping end # Sets the resource creating an instance variable diff --git a/lib/devise/mapping.rb b/lib/devise/mapping.rb index c24fc7e7..c8a20e80 100644 --- a/lib/devise/mapping.rb +++ b/lib/devise/mapping.rb @@ -22,24 +22,9 @@ module Devise # # is the modules included in the class # class Mapping #:nodoc: - attr_reader :singular, :plural, :path, :controllers, :path_names, :path_prefix, :class_name + attr_reader :singular, :plural, :path, :controllers, :path_names, :class_name alias :name :singular - # Loop through all mappings looking for a map that matches with the requested - # path (ie /users/sign_in). If a path prefix is given, it's taken into account. - def self.find_by_path(request) - Devise.mappings.each_value do |mapping| - route, extra = request.path_info.split("/")[mapping.segment_position, 2] - next unless route - - if !extra && (format = request.params[:format]) - route.sub!(/\.#{format}$/, '') - end - return mapping if mapping.path == route.to_sym - end - nil - end - # Receives an object and find a scope for it. If a scope cannot be found, # raises an error. If a symbol is given, it's considered to be the scope. def self.find_scope!(duck) @@ -56,28 +41,22 @@ module Devise end def initialize(name, options) #:nodoc: - @plural = (options[:as] ? "#{options.delete(:as)}_#{name}" : name).to_sym - @singular = (options.delete(:singular) || @plural.to_s.singularize).to_sym + @plural = (options[:as] ? "#{options[:as]}_#{name}" : name).to_sym + @singular = (options[:singular] || @plural.to_s.singularize).to_sym - @class_name = (options.delete(:class_name) || name.to_s.classify).to_s + @class_name = (options[:class_name] || name.to_s.classify).to_s @ref = ActiveSupport::Dependencies.ref(@class_name) - @path = (options.delete(:path) || name).to_sym - @path_prefix = "/#{options.delete(:path_prefix)}/".squeeze("/") + @path = (options[:path] || name).to_s + @path_prefix = options[:path_prefix] - if @path_prefix =~ /\(.*\)/ && Devise.ignore_optional_segments != true - raise ScriptError, "It seems that you are scoping devise_for with an optional segment #{@path_prefix.inspect} " << - "which Devise does not support. Please remove the optional segment or alternatively, if you are *sure* of " << - "what you are doing, you can set config.ignore_optional_segments = true in your devise initializer." - end - - mod = options.delete(:module) || "devise" + mod = options[:module] || "devise" @controllers = Hash.new { |h,k| h[k] = "#{mod}/#{k}" } - @controllers.merge!(options.delete(:controllers) || {}) + @controllers.merge!(options[:controllers] || {}) - @path_names = Hash.new { |h,k| h[k] = k.to_s } + @path_names = Hash.new { |h,k| h[k] = k.to_s } @path_names.merge!(:registration => "") - @path_names.merge!(options.delete(:path_names) || {}) + @path_names.merge!(options[:path_names] || {}) end # Return modules for the mapping. @@ -98,30 +77,14 @@ module Devise @routes ||= ROUTES.values_at(*self.modules).compact.uniq end - # Keep a list of allowed controllers for this mapping. It's useful to ensure - # that an Admin cannot access the registrations controller unless it has - # :registerable in the model. - def allowed_controllers - @allowed_controllers ||= begin - canonical = CONTROLLERS.values_at(*self.modules).compact - @controllers.values_at(*canonical) - end - end - - # Returns in which position in the path prefix devise should find the as mapping. - def segment_position - self.path_prefix.count("/") - end - - # Returns fullpath for route generation. - def fullpath - @path_prefix + @path.to_s - end - def authenticatable? @authenticatable ||= self.modules.any? { |m| m.to_s =~ /authenticatable/ } end + def fullpath + "#{@path_prefix}/#{@path}".squeeze("/") + end + # Create magic predicates for verifying what module is activated by this map. # Example: # diff --git a/lib/devise/rails/routes.rb b/lib/devise/rails/routes.rb index 386d6e73..6abc2bc6 100644 --- a/lib/devise/rails/routes.rb +++ b/lib/devise/rails/routes.rb @@ -97,8 +97,8 @@ module ActionDispatch::Routing # devise_for :users # end # - # However, since Devise uses the request path to retrieve the current user, this has a few caveats. - # First, if you are using a dynamic segment, as below: + # However, since Devise uses the request path to retrieve the current user, it has one caveats. + # If you are using a dynamic segment, as below: # # scope ":locale" do # devise_for :users @@ -113,13 +113,6 @@ module ActionDispatch::Routing # end # end # - # Finally, Devise does not (and cannot) support optional segments, either static or dynamic. That - # said, the following does not work: - # - # scope "(/:locale)" do # THIS WILL FAIL - # devise_for :users - # end - # def devise_for(*resources) options = resources.extract_options! @@ -141,7 +134,7 @@ module ActionDispatch::Routing resources.map!(&:to_sym) resources.each do |resource| - mapping = Devise.add_model(resource, options) + mapping = Devise.add_mapping(resource, options) begin raise_no_devise_method_error!(mapping.class_name) unless mapping.to.respond_to?(:devise) @@ -158,8 +151,10 @@ module ActionDispatch::Routing routes = mapping.routes routes -= Array(options.delete(:skip)).map { |s| s.to_s.singularize.to_sym } - with_devise_scope mapping.fullpath, mapping.name do - routes.each { |mod| send(:"devise_#{mod}", mapping, mapping.controllers) } + with_devise_exclusive_scope mapping.fullpath, mapping.name do + devise_scope mapping.name do + routes.each { |mod| send(:"devise_#{mod}", mapping, mapping.controllers) } + end end end end @@ -180,9 +175,32 @@ module ActionDispatch::Routing end end + # Sets the devise scope to be used in the controller. If you have custom routes, + # you are required to call this method (also aliased as :as) in order to specify + # to which controller it is targetted. + # + # as :user do + # get "sign_in", :to => "devise/sessions#new" + # end + # + # Notice you cannot have two scopes mapping to the same URL. And remember, if + # you try to access a devise controller without specifying a scope, it will + # raise ActionNotFound error. + def devise_scope(scope) + constraint = lambda do |request| + request.env["devise.mapping"] = Devise.mappings[scope] + true + end + + constraints(constraint) do + yield + end + end + alias :as :devise_scope + protected - def devise_session(mapping, controllers) + def devise_session(mapping, controllers) #:nodoc: scope :controller => controllers[:sessions], :as => :session do get :new, :path => mapping.path_names[:sign_in] post :create, :path => mapping.path_names[:sign_in], :as => "" @@ -190,27 +208,27 @@ module ActionDispatch::Routing end end - def devise_password(mapping, controllers) + def devise_password(mapping, controllers) #:nodoc: resource :password, :only => [:new, :create, :edit, :update], :path => mapping.path_names[:password], :controller => controllers[:passwords] end - def devise_confirmation(mapping, controllers) + def devise_confirmation(mapping, controllers) #:nodoc: resource :confirmation, :only => [:new, :create, :show], :path => mapping.path_names[:confirmation], :controller => controllers[:confirmations] end - def devise_unlock(mapping, controllers) + def devise_unlock(mapping, controllers) #:nodoc: resource :unlock, :only => [:new, :create, :show], :path => mapping.path_names[:unlock], :controller => controllers[:unlocks] end - def devise_registration(mapping, controllers) + def devise_registration(mapping, controllers) #:nodoc: resource :registration, :only => [:new, :create, :edit, :update, :destroy], :path => mapping.path_names[:registration], :path_names => { :new => mapping.path_names[:sign_up] }, :controller => controllers[:registrations] end - def with_devise_scope(new_path, new_as) + def with_devise_exclusive_scope(new_path, new_as) #:nodoc: old_as, old_path, old_module = @scope[:as], @scope[:path], @scope[:module] @scope[:as], @scope[:path], @scope[:module] = new_as, new_path, nil yield @@ -218,7 +236,7 @@ module ActionDispatch::Routing @scope[:as], @scope[:path], @scope[:module] = old_as, old_path, old_module end - def raise_no_devise_method_error!(klass) + def raise_no_devise_method_error!(klass) #:nodoc: raise "#{klass} does not respond to 'devise' method. This usually means you haven't " << "loaded your ORM file or it's being loaded too late. To fix it, be sure to require 'devise/orm/YOUR_ORM' " << "inside 'config/initializers/devise.rb' or before your application definition in 'config/application.rb'" diff --git a/lib/generators/devise/templates/devise.rb b/lib/generators/devise/templates/devise.rb index bfcd8149..871cc5aa 100644 --- a/lib/generators/devise/templates/devise.rb +++ b/lib/generators/devise/templates/devise.rb @@ -103,16 +103,8 @@ Devise.setup do |config| # are using only default views. # config.scoped_views = true - # By default, devise detects the role accessed based on the url. So whenever - # accessing "/users/sign_in", it knows you are accessing an User. This makes - # routes as "/sign_in" not possible, unless you tell Devise to use the default - # scope, setting true below. - # Note that devise does not generate default routes. You also have to - # specify them in config/routes.rb - # config.use_default_scope = true - - # Configure the default scope used by Devise. By default it's the first devise - # role declared in your routes. + # Configure the default scope given to Warden. By default it's the first + # devise role declared in your routes. # config.default_scope = :user # Configure sign_out behavior. diff --git a/test/controllers/internal_helpers_test.rb b/test/controllers/internal_helpers_test.rb index 242575d8..fc08e6aa 100644 --- a/test/controllers/internal_helpers_test.rb +++ b/test/controllers/internal_helpers_test.rb @@ -10,37 +10,28 @@ class HelpersTest < ActionController::TestCase def setup @mock_warden = OpenStruct.new @controller.request.env['warden'] = @mock_warden + @controller.request.env['devise.mapping'] = Devise.mappings[:user] end - test 'get resource name from request path' do - @request.path = '/users/session' + test 'get resource name from env' do assert_equal :user, @controller.resource_name end - test 'get resource name from specific request path' do - @request.path = '/admin_area/session' - assert_equal :admin, @controller.resource_name - end - - test 'get resource class from request path' do - @request.path = '/users/session' + test 'get resource class from env' do assert_equal User, @controller.resource_class end - test 'get resource instance variable from request path' do - @request.path = '/admin_area/session' - @controller.instance_variable_set(:@admin, admin = Admin.new) + test 'get resource instance variable from env' do + @controller.instance_variable_set(:@user, admin = Admin.new) assert_equal admin, @controller.resource end - test 'set resource instance variable from request path' do - @request.path = '/admin_area/session' - + test 'set resource instance variable from env' do admin = @controller.send(:resource_class).new @controller.send(:resource=, admin) assert_equal admin, @controller.send(:resource) - assert_equal admin, @controller.instance_variable_get(:@admin) + assert_equal admin, @controller.instance_variable_get(:@user) end test 'resources methods are not controller actions' do diff --git a/test/failure_app_test.rb b/test/failure_app_test.rb index 9e8e4c97..3a1c47a9 100644 --- a/test/failure_app_test.rb +++ b/test/failure_app_test.rb @@ -100,6 +100,7 @@ class FailureTest < ActiveSupport::TestCase env = { "action_dispatch.request.parameters" => { :controller => "devise/sessions" }, "warden.options" => { :recall => "new", :attempted_path => "/users/sign_in" }, + "devise.mapping" => Devise.mappings[:user], "warden" => stub_everything } call_failure(env) diff --git a/test/integration/authenticatable_test.rb b/test/integration/authenticatable_test.rb index 382cd5db..4d67232e 100644 --- a/test/integration/authenticatable_test.rb +++ b/test/integration/authenticatable_test.rb @@ -277,27 +277,6 @@ class AuthenticationWithScopesTest < ActionController::IntegrationTest end end end - - test 'uses the mapping from the default scope if specified' do - swap Devise, :use_default_scope => true do - get '/sign_in' - assert_response :ok - assert_contain 'Sign in' - end - end - - test 'sign in with script name' do - assert_nothing_raised do - get new_user_session_path, {}, "SCRIPT_NAME" => "/omg" - fill_in "email", "user@test.com" - end - end - - test 'registration in xml format' do - assert_nothing_raised do - post user_registration_path(:format => 'xml', :user => {:email => "test@example.com", :password => "invalid"} ) - end - end end class AuthenticationOthersTest < ActionController::IntegrationTest @@ -318,4 +297,17 @@ class AuthenticationOthersTest < ActionController::IntegrationTest get '/sign_in' end end + + test 'sign in with script name' do + assert_nothing_raised do + get new_user_session_path, {}, "SCRIPT_NAME" => "/omg" + fill_in "email", "user@test.com" + end + end + + test 'registration in xml format' do + assert_nothing_raised do + post user_registration_path(:format => 'xml', :user => {:email => "test@example.com", :password => "invalid"} ) + end + end end diff --git a/test/mapping_test.rb b/test/mapping_test.rb index e51d1877..386d0156 100644 --- a/test/mapping_test.rb +++ b/test/mapping_test.rb @@ -14,27 +14,15 @@ class MappingTest < ActiveSupport::TestCase assert_equal User.devise_modules, mapping.modules assert_equal :users, mapping.plural assert_equal :user, mapping.singular - assert_equal :users, mapping.path + assert_equal "users", mapping.path end test 'allows path to be given' do - assert_equal :admin_area, Devise.mappings[:admin].path + assert_equal "admin_area", Devise.mappings[:admin].path end test 'allows custom singular to be given' do - assert_equal :accounts, Devise.mappings[:manager].path - end - - test 'allows a controller depending on the mapping' do - allowed = Devise.mappings[:user].allowed_controllers - assert allowed.include?("devise/sessions") - assert allowed.include?("devise/confirmations") - assert allowed.include?("devise/passwords") - - allowed = Devise.mappings[:admin].allowed_controllers - assert allowed.include?("sessions") - assert_not allowed.include?("devise/confirmations") - assert_not allowed.include?("devise/unlocks") + assert_equal "accounts", Devise.mappings[:manager].path end test 'has strategies depending on the model declaration' do @@ -42,19 +30,6 @@ class MappingTest < ActiveSupport::TestCase assert_equal [:database_authenticatable], Devise.mappings[:admin].strategies end - test 'find mapping by path' do - assert_nil Devise::Mapping.find_by_path(fake_request("/foo/bar")) - assert_equal Devise.mappings[:user], Devise::Mapping.find_by_path(fake_request("/users/session")) - end - - test 'find mapping by customized path' do - assert_equal Devise.mappings[:admin], Devise::Mapping.find_by_path(fake_request("/admin_area/session")) - end - - test 'find mapping by path strips format' do - assert_equal Devise.mappings[:user], Devise::Mapping.find_by_path(fake_request("/users.xml", :format => "xml")) - end - test 'find scope for a given object' do assert_equal :user, Devise::Mapping.find_scope!(User) assert_equal :user, Devise::Mapping.find_scope!(:user) @@ -92,21 +67,6 @@ class MappingTest < ActiveSupport::TestCase assert_equal 'unblock', mapping.path_names[:unlock] end - test 'has an empty path as default path prefix' do - mapping = Devise.mappings[:user] - assert_equal '/', mapping.path_prefix - end - - test 'allow path prefix to be configured' do - mapping = Devise.mappings[:manager] - assert_equal '/:locale/', mapping.path_prefix - end - - test 'retrieve as from the proper position' do - assert_equal 1, Devise.mappings[:user].segment_position - assert_equal 2, Devise.mappings[:manager].segment_position - end - test 'magic predicates' do mapping = Devise.mappings[:user] assert mapping.authenticatable?