Merge pull request #5573 from heartcombo/ca/failure-app-respect-redirect

Respect redirect status code when recalling the action

CHANGELOG.md

@@ -5,6 +5,7 @@
   * Refactor conditional dirty tracking logic to a centralized module to simplify usage throughout the codebase. [#5575](https://github.com/heartcombo/devise/pull/5575)
 
 * bug fixes
+  * Failure app will respond with configured `redirect_status` instead of `error_status` if the recall app returns a redirect status (300..399) [#5573](https://github.com/heartcombo/devise/pull/5573)
   * Fix frozen string exception in validatable. [#5563](https://github.com/heartcombo/devise/pull/5563) [#5465](https://github.com/heartcombo/devise/pull/5465) [@mameier](https://github.com/mameier)
 
 ### 4.9.0 - 2023-02-17

lib/devise/failure_app.rb

@@ -72,7 +72,9 @@ module Devise
 
       flash.now[:alert] = i18n_message(:invalid) if is_flashing_format?
       self.response = recall_app(warden_options[:recall]).call(request.env).tap { |response|
-        response[0] = Rack::Utils.status_code(Devise.responder.error_status)
+        response[0] = Rack::Utils.status_code(
+          response[0].in?(300..399) ? Devise.responder.redirect_status : Devise.responder.error_status
+        )
       }
     end
 

test/failure_app_test.rb

@@ -371,6 +371,59 @@ class FailureTest < ActiveSupport::TestCase
         end
       end
     end
+
+    # TODO: remove conditional/else when supporting only responders 3.1+
+    if ActionController::Responder.respond_to?(:error_status=)
+      test 'respects the configured responder `error_status` for the status code' do
+        swap Devise.responder, error_status: :unprocessable_entity do
+          env = {
+            "warden.options" => { recall: "devise/sessions#new", attempted_path: "/users/sign_in" },
+            "devise.mapping" => Devise.mappings[:user],
+            "warden" => stub_everything
+          }
+          call_failure(env)
+
+          assert_equal 422, @response.first
+          assert_includes @response.third.body, 'Invalid Email or password.'
+        end
+      end
+
+      test 'respects the configured responder `redirect_status` if the recall app returns a redirect status code' do
+        swap Devise.responder, redirect_status: :see_other do
+          env = {
+            "warden.options" => { recall: "devise/registrations#cancel", attempted_path: "/users/cancel" },
+            "devise.mapping" => Devise.mappings[:user],
+            "warden" => stub_everything
+          }
+          call_failure(env)
+
+          assert_equal 303, @response.first
+        end
+      end
+    else
+      test 'uses default hardcoded responder `error_status` for the status code since responders version does not support configuring it' do
+        env = {
+          "warden.options" => { recall: "devise/sessions#new", attempted_path: "/users/sign_in" },
+          "devise.mapping" => Devise.mappings[:user],
+          "warden" => stub_everything
+        }
+        call_failure(env)
+
+        assert_equal 200, @response.first
+        assert_includes @response.third.body, 'Invalid Email or password.'
+      end
+
+      test 'users default hardcoded responder `redirect_status` for the status code since responders version does not support configuring it' do
+        env = {
+          "warden.options" => { recall: "devise/registrations#cancel", attempted_path: "/users/cancel" },
+          "devise.mapping" => Devise.mappings[:user],
+          "warden" => stub_everything
+        }
+        call_failure(env)
+
+        assert_equal 302, @response.first
+      end
+    end
   end
 
   context "Lazy loading" do