Only redirect if needed.

app/controllers/sessions_controller.rb

@@ -17,7 +17,7 @@ class SessionsController < ApplicationController
       set_flash_message :success, :signed_in
       redirect_back_or_to home_or_root_path
     else
-      set_now_flash_message :failure, :invalid
+      set_now_flash_message :failure, warden.message || :invalid
       build_resource
       render :new
     end

lib/devise.rb

@@ -110,7 +110,7 @@ module Devise
     def configure_warden_manager(manager) #:nodoc:
       manager.default_strategies *Devise::STRATEGIES
       manager.default_serializers *Devise::SERIALIZERS
-      manager.failure_app = Devise::Failure
+      manager.failure_app = Devise::FailureApp
       manager.silence_missing_strategies!
       manager.silence_missing_serializers!
 
@@ -135,4 +135,4 @@ end
 require 'devise/strategies/base'
 require 'devise/serializers/base'
 
-require 'devise/rails'
+require 'devise/rails'

lib/devise/failure_app.rb

@@ -1,5 +1,5 @@
 module Devise
-  module Failure
+  module FailureApp
     mattr_accessor :default_url
 
     # Failure application that will be called every time :warden is thrown from
@@ -9,13 +9,15 @@ module Devise
     def self.call(env)
       options = env['warden.options']
       scope   = options[:scope]
-      params  = case env['warden'].try(:message)
+      message = env['warden'].try(:message) || options[:message]
+
+      params  = case message
         when Symbol
-          { env['warden'].message => true }
+          { message => true }
         when String
-          { :message => env['warden'].message }
+          { :message => message }
         else
-          options[:params]
+          {}
       end
 
       redirect_path = if mapping = Devise.mappings[scope]
@@ -23,14 +25,14 @@ module Devise
       else
         "/#{default_url}"
       end
+      query_string = Rack::Utils.build_query(params)
 
       headers = {}
       headers["Location"] = redirect_path
-      headers["Location"] << "?" << Rack::Utils.build_query(params) if params
+      headers["Location"] << "?" << query_string unless query_string.empty?
       headers["Content-Type"] = 'text/plain'
 
-      message = options[:message] || "You are being redirected to #{redirect_path}"
-      [302, headers, [message]]
+      [302, headers, ["You are being redirected to #{redirect_path}"]]
     end
   end
 end

lib/devise/hooks/confirmable.rb

@@ -6,6 +6,13 @@ Warden::Manager.after_set_user do |record, warden, options|
   if record && record.respond_to?(:active?) && !record.active?
     scope = options[:scope]
     warden.logout(scope)
-    throw :warden, :scope => scope, :params => { :unconfirmed => true }
+
+    if warden.winning_strategy
+      # If winning strategy was set, this is being called after authenticate and
+      # there is no need to force a redirect.
+      warden.winning_strategy.fail!(:unconfirmed)
+    else
+      throw :warden, :scope => scope, :message => :unconfirmed
+    end
   end
 end

lib/devise/strategies/authenticatable.rb

@@ -13,8 +13,12 @@ module Devise
       # 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? && resource = mapping.to.authenticate(params[scope])
-          success!(resource)
+        if valid_attributes?
+          if resource = mapping.to.authenticate(params[scope])
+            success!(resource)
+          else
+            fail!(:invalid)
+          end
         else
           store_location
           fail!(:unauthenticated)
@@ -33,7 +37,7 @@ module Devise
         # 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?
+          session[:"#{mapping.name}.return_to"] ||= request.request_uri if request.get?
         end
     end
   end

test/devise_test.rb

@@ -61,7 +61,7 @@ class DeviseTest < ActiveSupport::TestCase
     manager = MockManager.new
     Devise.configure_warden_manager(manager)
 
-    assert_equal Devise::Failure, manager.failure_app
+    assert_equal Devise::FailureApp, manager.failure_app
     assert_equal [:authenticatable], manager.default_strategies
     assert manager.silence_missing_strategies
   end

test/failure_app_test.rb

@@ -1,10 +1,11 @@
 require 'test/test_helper'
+require 'ostruct' 
 
 class FailureTest < ActiveSupport::TestCase
 
   def call_failure(env_params={})
-    env = {'warden.options' => {:scope => :user}.update(env_params)}
-    Devise::Failure.call(env)
+    env = {'warden.options' => { :scope => :user }}.merge!(env_params)
+    Devise::FailureApp.call(env)
   end
 
   test 'return 302 status' do
@@ -15,8 +16,9 @@ class FailureTest < ActiveSupport::TestCase
     assert_equal '/users/sign_in', call_failure.second['Location']
   end
 
-  test 'add params to redirect location' do
-    location = call_failure(:params => {:test => true}).second['Location']
+  test 'uses the proxy failure message' do
+    warden = OpenStruct.new(:message => :test)
+    location = call_failure('warden' => warden).second['Location']
     assert_equal '/users/sign_in?test=true', location
   end
 
@@ -27,8 +29,4 @@ class FailureTest < ActiveSupport::TestCase
   test 'setup a default message' do
     assert_equal ['You are being redirected to /users/sign_in'], call_failure.last
   end
-
-  test 'pass in a different message' do
-    assert_equal ['Hello world'], call_failure(:message => 'Hello world').last
-  end
 end

test/integration/confirmable_test.rb

@@ -62,7 +62,7 @@ class ConfirmationTest < ActionController::IntegrationTest
     Devise.confirm_within = 0
     user = sign_in_as_user(:confirm => false)
 
-    assert_redirected_to new_user_session_path(:unconfirmed => true)
+    assert_contain 'You have to confirm your account before continuing'
     assert_not warden.authenticated?(:user)
   end