Return 401 for sessions#destroy action with no user signed in (#4878)

It's an unauthenticated request, so return 401 Unauthorized like most
other similar requests.

Signed-off-by: Carlos Antonio da Silva <[email protected]>

Author: Adan Amarillas

CHANGELOG.md

@@ -53,6 +53,7 @@
   * Handle `on` and `ON` as true values to check params [#5514](https://github.com/heartcombo/devise/pull/5514)
   * Fix passing `format` option to `devise_for` [#5732](https://github.com/heartcombo/devise/pull/5732)
   * Use `ActiveRecord::SecurityUtils.secure_compare` in `Devise.secure_compare` to match two empty strings correctly. [#4829](https://github.com/heartcombo/devise/pull/4829)
+  * Respond with `401 Unauthorized` for non-navigational requests to destroy the session when there is no authenticated resource. [#4878](https://github.com/heartcombo/devise/pull/4878)
 
 
 Please check [4-stable](https://github.com/heartcombo/devise/blob/4-stable/CHANGELOG.md)

app/controllers/devise/sessions_controller.rb

@@ -28,7 +28,7 @@ def destroy
     signed_out = (Devise.sign_out_all_scopes ? sign_out : sign_out(resource_name))
     set_flash_message! :notice, :signed_out if signed_out
     yield if block_given?
-    respond_to_on_destroy
+    respond_to_on_destroy(status: :no_content)
   end
 
   protected
@@ -62,7 +62,7 @@ def verify_signed_out_user
     if all_signed_out?
       set_flash_message! :notice, :already_signed_out
 
-      respond_to_on_destroy
+      respond_to_on_destroy(status: :unauthorized)
     end
   end
 
@@ -72,11 +72,11 @@ def all_signed_out?
     users.all?(&:blank?)
   end
 
-  def respond_to_on_destroy
+  def respond_to_on_destroy(status: status)
     # We actually need to hardcode this as Rails default responder doesn't
     # support returning empty response on GET request
     respond_to do |format|
-      format.all { head :no_content }
+      format.all { head status }
       format.any(*navigational_formats) { redirect_to after_sign_out_path_for(resource_name), status: Devise.responder.redirect_status }
     end
   end

test/controllers/sessions_controller_test.rb

@@ -74,7 +74,7 @@ class SessionsControllerTest < Devise::ControllerTestCase
     assert_template "devise/sessions/new"
   end
 
-  test "#destroy doesn't set the flash if the requested format is not navigational" do
+  test "#destroy doesn't set the flash and returns 204 status if the requested format is not navigational" do
     request.env["devise.mapping"] = Devise.mappings[:user]
     user = create_user
     user.confirm
@@ -87,4 +87,16 @@ class SessionsControllerTest < Devise::ControllerTestCase
     assert flash[:notice].blank?, "flash[:notice] should be blank, not #{flash[:notice].inspect}"
     assert_equal 204, @response.status
   end
+
+  test "#destroy returns 401 status if user is not signed in and the requested format is not navigational" do
+    request.env["devise.mapping"] = Devise.mappings[:user]
+    delete :destroy, format: 'json'
+    assert_equal 401, @response.status
+  end
+
+  test "#destroy returns 302 status if user is not signed in and the requested format is navigational" do
+    request.env["devise.mapping"] = Devise.mappings[:user]
+    delete :destroy
+    assert_equal 302, @response.status
+  end
 end