diff --git a/app/controllers/devise/sessions_controller.rb b/app/controllers/devise/sessions_controller.rb index 2a1f6a5a..bcc36ab9 100644 --- a/app/controllers/devise/sessions_controller.rb +++ b/app/controllers/devise/sessions_controller.rb @@ -5,24 +5,17 @@ class Devise::SessionsController < ApplicationController # GET /resource/sign_in def new Devise::FLASH_MESSAGES.each do |message| - set_now_flash_message :alert, message if params.try(:[], message) == "true" + set_now_flash_message :alert, message if params[message] == "true" end unless flash[:notice] - build_resource({}) + clean_up_passwords(build_resource) render_with_scope :new end # POST /resource/sign_in def create - if resource = warden.authenticate(:scope => resource_name) - set_flash_message :notice, :signed_in - sign_in_and_redirect(resource_name, resource, true) - elsif warden.winning_strategy && warden.result != :failure - throw :warden, :scope => resource_name - else - set_now_flash_message :alert, (warden.message || :invalid) - clean_up_passwords(build_resource) - render_with_scope :new - end + resource = warden.authenticate!(:scope => resource_name, :recall => "new") + set_flash_message :notice, :signed_in + sign_in_and_redirect(resource_name, resource) end # GET /resource/sign_out diff --git a/lib/devise/controllers/helpers.rb b/lib/devise/controllers/helpers.rb index d918b2e0..84a4a175 100644 --- a/lib/devise/controllers/helpers.rb +++ b/lib/devise/controllers/helpers.rb @@ -117,10 +117,10 @@ module Devise # # If just a symbol is given, consider that the user was already signed in # through other means and just perform the redirection. - def sign_in_and_redirect(resource_or_scope, resource=nil, skip=false) + def sign_in_and_redirect(resource_or_scope, resource=nil) scope = Devise::Mapping.find_scope!(resource_or_scope) resource ||= resource_or_scope - sign_in(scope, resource) unless skip + sign_in(scope, resource) unless warden.user(scope) == resource redirect_to stored_location_for(scope) || after_sign_in_path_for(resource) end diff --git a/lib/devise/failure_app.rb b/lib/devise/failure_app.rb index 9c176fd3..ba753445 100644 --- a/lib/devise/failure_app.rb +++ b/lib/devise/failure_app.rb @@ -10,9 +10,6 @@ module Devise include ActionController::UrlFor include ActionController::Redirecting - mattr_accessor :default_message - self.default_message = :unauthenticated - def self.call(env) action(:respond).call(env) end @@ -27,6 +24,11 @@ module Devise self.headers["WWW-Authenticate"] = %(Basic realm=#{Devise.http_authentication_realm.inspect}) self.content_type = request.format.to_s self.response_body = http_auth_body + elsif action = warden_options[:recall] + default_message :invalid + env["PATH_INFO"] = attempted_path + params.merge!(query_string_params) + self.response = recall_controller.action(action).call(env) else scope = warden_options[:scope] store_location!(scope) @@ -37,7 +39,12 @@ module Devise protected def message - @message ||= warden.try(:message) || warden_options[:message] || self.class.default_message + @message ||= warden.message || warden_options[:message] || default_message + end + + def default_message(message=nil) + @default_message = message if message + @default_message ||= :unauthenticated end def http_auth? @@ -59,7 +66,7 @@ module Devise def query_string_params case message when Symbol - { message => true } + { message => "true" } when String { :message => message } else @@ -67,6 +74,10 @@ module Devise end end + def recall_controller + "#{params[:controller].camelize}Controller".constantize + end + def warden env['warden'] end diff --git a/test/controllers/helpers_test.rb b/test/controllers/helpers_test.rb index 34336f03..c66c9e92 100644 --- a/test/controllers/helpers_test.rb +++ b/test/controllers/helpers_test.rb @@ -153,6 +153,7 @@ class ControllerAuthenticableTest < ActionController::TestCase test 'sign in and redirect uses the stored location' do user = User.new @controller.session[:"user.return_to"] = "/foo.bar" + @mock_warden.expects(:user).with(:user).returns(nil) @mock_warden.expects(:set_user).with(user, :scope => :user).returns(true) @controller.expects(:redirect_to).with("/foo.bar") @controller.sign_in_and_redirect(user) @@ -160,15 +161,18 @@ class ControllerAuthenticableTest < ActionController::TestCase test 'sign in and redirect uses the configured after sign in path' do admin = Admin.new + @mock_warden.expects(:user).with(:admin).returns(nil) @mock_warden.expects(:set_user).with(admin, :scope => :admin).returns(true) @controller.expects(:redirect_to).with(admin_root_path) @controller.sign_in_and_redirect(admin) end - test 'only redirect if skip is given' do + test 'sign in and redirect does not sign in again if user is already signed' do admin = Admin.new + @mock_warden.expects(:user).with(:admin).returns(admin) + @mock_warden.expects(:set_user).never @controller.expects(:redirect_to).with(admin_root_path) - @controller.sign_in_and_redirect(:admin, admin, true) + @controller.sign_in_and_redirect(admin) end test 'sign out and redirect uses the configured after sign out path' do diff --git a/test/failure_app_test.rb b/test/failure_app_test.rb index a1037fac..11a29d81 100644 --- a/test/failure_app_test.rb +++ b/test/failure_app_test.rb @@ -2,6 +2,9 @@ require 'test_helper' require 'ostruct' class FailureTest < ActiveSupport::TestCase + def self.context(name, &block) + instance_eval(&block) + end def call_failure(env_params={}) env = { @@ -9,38 +12,76 @@ class FailureTest < ActiveSupport::TestCase 'REQUEST_URI' => 'http://test.host/', 'HTTP_HOST' => 'test.host', 'REQUEST_METHOD' => 'GET', - 'rack.session' => {} + 'rack.session' => {}, + 'rack.input' => "", + 'warden' => OpenStruct.new(:message => nil) }.merge!(env_params) - Devise::FailureApp.call(env) + Devise::FailureApp.call(env).to_a end - test 'return 302 status' do - assert_equal 302, call_failure.first + def call_failure_with_http(env_params={}) + env = { "HTTP_AUTHORIZATION" => "Basic #{ActiveSupport::Base64.encode64("foo:bar")}" } + call_failure(env_params.merge!(env)) end - test 'return to the default redirect location' do - assert_equal 'http://test.host/users/sign_in?unauthenticated=true', call_failure.second['Location'] + context 'When redirecting' do + test 'return 302 status' do + assert_equal 302, call_failure.first + end + + test 'return to the default redirect location' do + assert_equal 'http://test.host/users/sign_in?unauthenticated=true', call_failure.second['Location'] + end + + test 'uses the proxy failure message as symbol' do + warden = OpenStruct.new(:message => :test) + location = call_failure('warden' => warden).second['Location'] + assert_equal 'http://test.host/users/sign_in?test=true', location + end + + test 'uses the proxy failure message as string' do + warden = OpenStruct.new(:message => 'Hello world') + location = call_failure('warden' => warden).second['Location'] + assert_equal 'http://test.host/users/sign_in?message=Hello+world', location + end + + test 'set content type to default text/html' do + assert_equal 'text/html; charset=utf-8', call_failure.second['Content-Type'] + end + + test 'setup a default message' do + assert_match /You are being/, call_failure.last.body + assert_match /redirected/, call_failure.last.body + assert_match /\?unauthenticated=true/, call_failure.last.body + end end - test 'uses the proxy failure message' do - warden = OpenStruct.new(:message => :test) - location = call_failure('warden' => warden).second['Location'] - assert_equal 'http://test.host/users/sign_in?test=true', location + context 'For HTTP request' do + test 'return 401 status' do + assert_equal 401, call_failure_with_http.first + end + + test 'return WWW-authenticate headers' do + assert_equal 'Basic realm="Application"', call_failure_with_http.second["WWW-Authenticate"] + end + + test 'uses the proxy failure message as response body' do + warden = OpenStruct.new(:message => :invalid) + response = call_failure_with_http('warden' => warden).third + assert_equal 'Invalid email or password.', response.body + end end - test 'uses the given message' do - warden = OpenStruct.new(:message => 'Hello world') - location = call_failure('warden' => warden).second['Location'] - assert_equal 'http://test.host/users/sign_in?message=Hello+world', location - end - - test 'set content type to default text/html' do - assert_equal 'text/html; charset=utf-8', call_failure.second['Content-Type'] - end - - test 'setup a default message' do - assert_match /You are being/, call_failure.last.body - assert_match /redirected/, call_failure.last.body - assert_match /\?unauthenticated=true/, call_failure.last.body + context 'With recall' do + test 'calls the original controller' do + env = { + "action_dispatch.request.parameters" => { :controller => "devise/sessions" }, + "warden.options" => { :recall => "new", :attempted_path => "/users/sign_in" }, + "warden" => stub_everything + } + response = call_failure(env).third + assert response.body.include?('