From 082f73c5800395ba31dbf64d8258c2f858dc71ff Mon Sep 17 00:00:00 2001
From: Alex Ghiculescu <alex@tanda.co>
Date: Tue, 2 Feb 2021 16:07:31 -0700
Subject: [PATCH] Return a 422 from `FailureApp` when login fails

Devise `FailureApp` now returns a `422` status when running the `recall` flow. This, combined with https://github.com/heartcombo/responders/pull/223, enables Devise to work correctly with the [new Rails defaults](https://github.com/rails/rails/pull/41026) for form errors, as well as with new libraries like Turbo.

By default, the `recall` flow only runs on the [`SessionsController`](https://github.com/heartcombo/devise/blob/v4.7.3/app/controllers/devise/sessions_controller.rb#L48).
---
 CHANGELOG.md                                 | 28 ++++++++++++++++++++
 lib/devise.rb                                |  2 +-
 lib/devise/failure_app.rb                    | 10 +++++--
 lib/generators/templates/devise.rb           |  4 +--
 test/controllers/sessions_controller_test.rb |  6 ++---
 test/failure_app_test.rb                     | 16 +++++++++++
 6 files changed, 58 insertions(+), 8 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 0fd505d347..5dd8220634 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -3,6 +3,34 @@
 ### 4.8.0 - 2021-04-29
 
 * enhancements
+  * Devise now treats `:turbo_stream` as a navigational format.
+    - For Turbo Drive users this ensures that Devise will return a redirect, rather than a `401`, where appropriate (this may be a **breaking change**).
+    - This has no impact if you are not using [turbo-rails](https://github.com/hotwired/turbo-rails) or declaring an equivalent MIME type.
+  * Devise `FailureApp` now returns a `422` status when running the `recall` flow.
+    - This, combined with https://github.com/heartcombo/responders/pull/223, enables Devise to work correctly with the [new Rails defaults](https://github.com/rails/rails/pull/41026) for form errors, as well as with new libraries like Turbo.
+    - By default, the `recall` flow only runs on the [`SessionsController`](https://github.com/heartcombo/devise/blob/v4.7.3/app/controllers/devise/sessions_controller.rb#L48), but you should check if it is required in other flows too.
+    - This may be a **breaking change**.
+    - If you want to keep the old behavior, or return a different code, you can override `recall_response_code` in a custom failure app:
+
+    ```ruby
+    # config/initializers/devise.rb
+    class MyFailureApp < Devise::FailureApp
+      def recall_response_code(app_response_code)
+        # Return any HTTP status code you like, or return `app_response_code` to keep whatever your app returned and not override it.
+        # By default this method returns `422`.
+        app_response_code
+      end
+    end
+
+    Devise.setup do |config|
+      # ...
+      config.warden do |manager|
+        manager.failure_app = MyFailureApp
+      end
+      # ...
+    end
+    ```
+
   * Devise now enables the upgrade of OmniAuth 2+. Previously Devise would raise an error if you'd try to upgrade. Please note that OmniAuth 2 is considered a security upgrade and recommended to everyone. You can read more about the details (and possible necessary changes to your app as part of the upgrade) in [their release notes](https://github.com/omniauth/omniauth/releases/tag/v2.0.0). [Devise's OmniAuth Overview wiki](https://github.com/heartcombo/devise/wiki/OmniAuth:-Overview) was also updated to cover OmniAuth 2.0 requirements.
     - Note that the upgrade required Devise shared links that initiate the OmniAuth flow to be changed to `method: :post`, which is now a requirement for OmniAuth, part of the security improvement. If you have copied and customized the Devise shared links partial to your app, or if you have other links in your app that initiate the OmniAuth flow, they will have to be updated to use `method: :post`, or changed to use buttons (e.g. `button_to`) to work with OmniAuth 2. (if you're using links with `method: :post`, make sure your app has `rails-ujs` or `jquery-ujs` included in order for these links to work properly.)
     - As part of the OmniAuth 2.0 upgrade you might also need to add the [`omniauth-rails_csrf_protection`](https://github.com/cookpad/omniauth-rails_csrf_protection) gem to your app if you don't have it already. (and you don't want to roll your own code to verify requests.) Check the OmniAuth v2 release notes for more info.
diff --git a/lib/devise.rb b/lib/devise.rb
index 0451876df9..81e10b4e72 100644
--- a/lib/devise.rb
+++ b/lib/devise.rb
@@ -217,7 +217,7 @@ module Test
 
   # Which formats should be treated as navigational.
   mattr_accessor :navigational_formats
-  @@navigational_formats = ["*/*", :html]
+  @@navigational_formats = ["*/*", :html, :turbo_stream]
 
   # When set to true, signing out a user signs out all other scopes.
   mattr_accessor :sign_out_all_scopes
diff --git a/lib/devise/failure_app.rb b/lib/devise/failure_app.rb
index ee8219fff1..a686af5989 100644
--- a/lib/devise/failure_app.rb
+++ b/lib/devise/failure_app.rb
@@ -71,7 +71,9 @@ def recall
       end
 
       flash.now[:alert] = i18n_message(:invalid) if is_flashing_format?
-      self.response = recall_app(warden_options[:recall]).call(request.env)
+      response_from_app = recall_app(warden_options[:recall]).call(request.env)
+      response_from_app[0] = recall_response_code(response_from_app[0])
+      self.response = response_from_app
     end
 
     def redirect
@@ -89,6 +91,10 @@ def redirect
 
   protected
 
+    def recall_response_code(_original_response_code)
+      422
+    end
+
     def i18n_options(options)
       options
     end
@@ -167,7 +173,7 @@ def scope_url
     end
 
     def skip_format?
-      %w(html */*).include? request_format.to_s
+      %w(html turbo_stream */*).include? request_format.to_s
     end
 
     # Choose whether we should respond in an HTTP authentication fashion,
diff --git a/lib/generators/templates/devise.rb b/lib/generators/templates/devise.rb
index 1dbaddaa6e..9d097be7bd 100644
--- a/lib/generators/templates/devise.rb
+++ b/lib/generators/templates/devise.rb
@@ -256,14 +256,14 @@
 
   # ==> Navigation configuration
   # Lists the formats that should be treated as navigational. Formats like
-  # :html, should redirect to the sign in page when the user does not have
+  # :html should redirect to the sign in page when the user does not have
   # access, but formats like :xml or :json, should return 401.
   #
   # If you have any extra navigational formats, like :iphone or :mobile, you
   # should add them to the navigational formats lists.
   #
   # The "*/*" below is required to match Internet Explorer requests.
-  # config.navigational_formats = ['*/*', :html]
+  # config.navigational_formats = ['*/*', :html, :turbo_stream]
 
   # The default HTTP method used to sign out a resource. Default is :delete.
   config.sign_out_via = :delete
diff --git a/test/controllers/sessions_controller_test.rb b/test/controllers/sessions_controller_test.rb
index e88cf7e908..a3e3a3f219 100644
--- a/test/controllers/sessions_controller_test.rb
+++ b/test/controllers/sessions_controller_test.rb
@@ -19,7 +19,7 @@ class SessionsControllerTest < Devise::ControllerTestCase
           password: "wrongpassword"
         }
       }
-      assert_equal 200, @response.status
+      assert_equal 422, @response.status
     ensure
       ActiveSupport::Notifications.unsubscribe(subscriber)
     end
@@ -29,7 +29,7 @@ class SessionsControllerTest < Devise::ControllerTestCase
     swap Devise, scoped_views: true do
       request.env["devise.mapping"] = Devise.mappings[:user]
       post :create
-      assert_equal 200, @response.status
+      assert_equal 422, @response.status
       assert_template "users/sessions/new"
     end
   end
@@ -70,7 +70,7 @@ class SessionsControllerTest < Devise::ControllerTestCase
         password: "wevdude"
       }
     }
-    assert_equal 200, @response.status
+    assert_equal 422, @response.status
     assert_template "devise/sessions/new"
   end
 
diff --git a/test/failure_app_test.rb b/test/failure_app_test.rb
index 809f668de4..6028fb8e54 100644
--- a/test/failure_app_test.rb
+++ b/test/failure_app_test.rb
@@ -229,9 +229,20 @@ def call_failure(env_params = {})
     test 'redirects the correct format if it is a non-html format request' do
       swap Devise, navigational_formats: [:js] do
         call_failure('formats' => Mime[:js])
+        assert_equal 302, @response.first
         assert_equal 'http://test.host/users/sign_in.js', @response.second["Location"]
       end
     end
+
+    test 'redirects turbo_stream correctly' do
+      Mime::Type.register "text/vnd.turbo-stream.html", :turbo_stream
+
+      swap Devise, navigational_formats: [:html, :turbo_stream] do
+        call_failure('formats' => Mime[:turbo_stream])
+        assert_equal 302, @response.first
+        assert_equal 'http://test.host/users/sign_in', @response.second["Location"]
+      end
+    end
   end
 
   context 'For HTTP request' do
@@ -326,6 +337,7 @@ def call_failure(env_params = {})
         "warden" => stub_everything
       }
       call_failure(env)
+      assert_equal 422, @response.first
       assert_includes @response.third.body, '<h2>Log in</h2>'
       assert_includes @response.third.body, 'Invalid Email or password.'
     end
@@ -337,6 +349,7 @@ def call_failure(env_params = {})
         "warden" => stub_everything
       }
       call_failure(env)
+      assert_equal 422, @response.first
       assert_includes @response.third.body, '<h2>Log in</h2>'
       assert_includes @response.third.body, 'You have to confirm your email address before continuing.'
     end
@@ -348,6 +361,7 @@ def call_failure(env_params = {})
         "warden" => stub_everything
       }
       call_failure(env)
+      assert_equal 422, @response.first
       assert_includes @response.third.body, '<h2>Log in</h2>'
       assert_includes @response.third.body, 'Your account is not activated yet.'
     end
@@ -361,6 +375,7 @@ def call_failure(env_params = {})
             "warden" => stub_everything
           }
           call_failure(env)
+          assert_equal 422, @response.first
           assert_includes @response.third.body, '<h2>Log in</h2>'
           assert_includes @response.third.body, 'Invalid Email or password.'
           assert_equal '/sample', @request.env["SCRIPT_NAME"]
@@ -375,6 +390,7 @@ def call_failure(env_params = {})
       assert_equal "yes it does", Devise::FailureApp.new.lazy_loading_works?
     end
   end
+
   context "Without Flash Support" do
     test "returns to the default redirect location without a flash message" do
       call_failure request_klass: RequestWithoutFlashSupport