From 2b5a068246df81b8396f4e2734d245231195b7bb Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jos=C3=A9=20Valim?= <jose.valim@gmail.com>
Date: Thu, 1 Apr 2010 17:30:55 +0200
Subject: [PATCH] Move part of the logic in SessionsController#create to the
 FailureApp. Whenever Warden is invoked with a :recall, the failure app will
 recall the chosen controller and the action given to recall.

---
 app/controllers/devise/sessions_controller.rb | 17 ++--
 lib/devise/controllers/helpers.rb             |  4 +-
 lib/devise/failure_app.rb                     | 21 +++--
 test/controllers/helpers_test.rb              |  8 +-
 test/failure_app_test.rb                      | 89 ++++++++++++++-----
 5 files changed, 94 insertions(+), 45 deletions(-)

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?('<h2>Sign in</h2>')
+      assert response.body.include?('Invalid email or password.')
+    end
   end
 end