Return 422 status when login fails + treat turbo_stream as a navigational format

CHANGELOG.md

@@ -1,7 +1,33 @@
 ### 4.8.1 - 2021-12-16
 
 * enhancements
-  * Add support for Rails 7.0. Please note that Turbo integration is not fully supported by Devise yet.
+  * Add support for Rails 7.0.
+  * 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
+    ```
 
 ### 4.8.0 - 2021-04-29
 

app/controllers/devise/passwords_controller.rb

@@ -47,7 +47,7 @@ def update
       respond_with resource, location: after_resetting_password_path_for(resource)
     else
       set_minimum_password_length
-      respond_with resource
+      respond_with resource, status: :unprocessable_entity
     end
   end
 

app/controllers/devise/sessions_controller.rb

@@ -77,7 +77,7 @@ def respond_to_on_destroy
     # support returning empty response on GET request
     respond_to do |format|
       format.all { head :no_content }
-      format.any(*navigational_formats) { redirect_to after_sign_out_path_for(resource_name) }
+      format.any(*navigational_formats) { redirect_to after_sign_out_path_for(resource_name), status: :see_other }
     end
   end
 end

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

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,

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

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
 

test/failure_app_test.rb

@@ -228,9 +228,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
@@ -325,6 +336,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
@@ -336,6 +348,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
@@ -347,6 +360,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
@@ -360,6 +374,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"]
@@ -374,6 +389,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

test/integration/authenticatable_test.rb

@@ -119,7 +119,7 @@ class AuthenticationSanityTest < Devise::IntegrationTest
 
     delete destroy_admin_session_path
     assert_response :redirect
-    assert_redirected_to root_path
+    assert_response :see_other
 
     get root_path
     assert_contain 'Signed out successfully'
@@ -129,7 +129,7 @@ class AuthenticationSanityTest < Devise::IntegrationTest
   test 'unauthenticated admin set message on sign out' do
     delete destroy_admin_session_path
     assert_response :redirect
-    assert_redirected_to root_path
+    assert_response :see_other
 
     get root_path
     assert_contain 'Signed out successfully'

test/integration/recoverable_test.rb

@@ -156,7 +156,7 @@ def reset_password(options = {}, &block)
     user = create_user
     reset_password reset_password_token: 'invalid_reset_password'
 
-    assert_response :success
+    assert_response :unprocessable_entity
     assert_current_url '/users/password'
     assert_have_selector '#error_explanation'
     assert_contain %r{Reset password token(.*)invalid}
@@ -170,7 +170,7 @@ def reset_password(options = {}, &block)
       fill_in 'Confirm new password', with: 'other_password'
     end
 
-    assert_response :success
+    assert_response :unprocessable_entity
     assert_current_url '/users/password'
     assert_have_selector '#error_explanation'
     assert_contain "Password confirmation doesn't match Password"
@@ -192,7 +192,7 @@ def reset_password(options = {}, &block)
     request_forgot_password
 
     reset_password {  fill_in 'Confirm new password', with: 'other_password' }
-    assert_response :success
+    assert_response :unprocessable_entity
     assert_have_selector '#error_explanation'
     refute user.reload.valid_password?('987654321')
 

test/integration/timeoutable_test.rb

@@ -94,7 +94,7 @@ def last_request_at
     delete destroy_user_session_path
 
     assert_response :redirect
-    assert_redirected_to root_path
+    assert_response :see_other
     follow_redirect!
     assert_contain 'Signed out successfully'
   end