diff --git a/CHANGELOG.rdoc b/CHANGELOG.rdoc index 7d3f13ea..618c69c7 100644 --- a/CHANGELOG.rdoc +++ b/CHANGELOG.rdoc @@ -1,4 +1,6 @@ * enhancements + * Remove store_location from authenticatable strategy and add it to failure app + * Allow a strategy to be placed after authenticatable * [#45] Do not rely attribute? methods, since they are not added on Datamapper == 0.5.6 diff --git a/lib/devise/failure_app.rb b/lib/devise/failure_app.rb index 1a47c5fb..3827db2d 100644 --- a/lib/devise/failure_app.rb +++ b/lib/devise/failure_app.rb @@ -1,31 +1,34 @@ module Devise - module FailureApp - mattr_accessor :default_url + # Failure application that will be called every time :warden is thrown from + # any strategy or hook. Responsible for redirect the user to the sign in + # page based on current scope and mapping. If no scope is given, redirect + # to the default_url. + class FailureApp + attr_reader :env + include Warden::Mixins::Common + + cattr_accessor :default_url, :default_message, :instance_writer => false + @@default_message = :unauthenticated - # Failure application that will be called every time :warden is thrown from - # any strategy or hook. Responsible for redirect the user to the sign in - # page based on current scope and mapping. If no scope is given, redirect - # to the default_url. def self.call(env) - options = env['warden.options'] - scope = options[:scope] - message = env['warden'].try(:message) || options[:message] + new(env).respond! + end - params = case message - when Symbol - { message => true } - when String - { :message => message } - else - {} - end + def initialize(env) + @env = env + end + + def respond! + options = @env['warden.options'] + scope = options[:scope] redirect_path = if mapping = Devise.mappings[scope] "#{mapping.parsed_path}/#{mapping.path_names[:sign_in]}" else "/#{default_url}" end - query_string = Rack::Utils.build_query(params) + query_string = query_string_for(options) + store_location!(scope) headers = {} headers["Location"] = redirect_path @@ -34,5 +37,29 @@ module Devise [302, headers, ["You are being redirected to #{redirect_path}"]] end + + # Build the proper query string based on the given message. + def query_string_for(options) + message = @env['warden'].try(:message) || options[:message] || default_message + + params = case message + when Symbol + { message => true } + when String + { :message => message } + else + {} + end + + Rack::Utils.build_query(params) + end + + # Stores requested uri to redirect the user after signing in. We cannot use + # scoped session provided by warden here, since the user is not authenticated + # yet, but we still need to store the uri based on scope, so different scopes + # would never use the same uri to redirect. + def store_location!(scope) + session[:"#{scope}.return_to"] ||= request.request_uri if request && request.get? + end end end diff --git a/lib/devise/strategies/authenticatable.rb b/lib/devise/strategies/authenticatable.rb index 541451d9..35543f81 100644 --- a/lib/devise/strategies/authenticatable.rb +++ b/lib/devise/strategies/authenticatable.rb @@ -5,40 +5,20 @@ module Devise class Authenticatable < Warden::Strategies::Base include Devise::Strategies::Base + def valid? + super && params[scope] && params[scope][:password].present? + end + # Authenticate a user based on email and password params, returning to warden # success and the authenticated user if everything is okay. Otherwise redirect # to sign in page. - # - # Please notice the semantic difference between calling fail! and throw :warden. - # The first does not perform any action when calling authenticate, just - # when authenticate! is invoked. The second always perform the action. def authenticate! - if valid_attributes? - if resource = mapping.to.authenticate(params[scope]) - success!(resource) - else - fail!(:invalid) - end + if resource = mapping.to.authenticate(params[scope]) + success!(resource) else - store_location - fail!(:unauthenticated) + fail!(:invalid) end end - - private - - # Check if params and password are given. Others are checked inside authenticate. - def valid_attributes? - params[scope] && params[scope][:password].present? - end - - # Stores requested uri to redirect the user after signing in. We cannot use - # scoped session provided by warden here, since the user is not authenticated - # yet, but we still need to store the uri based on scope, so different scopes - # would never use the same uri to redirect. - def store_location - session[:"#{mapping.name}.return_to"] ||= request.request_uri if request.get? - end end end end diff --git a/test/failure_app_test.rb b/test/failure_app_test.rb index 07ea2ab5..4f9a4e4c 100644 --- a/test/failure_app_test.rb +++ b/test/failure_app_test.rb @@ -12,8 +12,8 @@ class FailureTest < ActiveSupport::TestCase assert_equal 302, call_failure.first end - test 'return redirect location based on mapping with params' do - assert_equal '/users/sign_in', call_failure.second['Location'] + test 'return to the default redirect location' do + assert_equal '/users/sign_in?unauthenticated=true', call_failure.second['Location'] end test 'uses the proxy failure message' do @@ -27,6 +27,6 @@ class FailureTest < ActiveSupport::TestCase end test 'setup a default message' do - assert_equal ['You are being redirected to /users/sign_in'], call_failure.last + assert_equal ['You are being redirected to /users/sign_in?unauthenticated=true'], call_failure.last end end