From 9a149ff139303edf3b832129054c62066baea98f Mon Sep 17 00:00:00 2001
From: Adan Amarillas <aamarill.engr@gmail.com>
Date: Fri, 28 Dec 2018 05:18:07 -0800
Subject: [PATCH] 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 <carlosantoniodasilva@gmail.com>
---
 CHANGELOG.md                                  |  1 +
 app/controllers/devise/sessions_controller.rb |  8 ++++----
 test/controllers/sessions_controller_test.rb  | 14 +++++++++++++-
 3 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index dc95bae4..0e6ba844 100644
--- a/CHANGELOG.md
+++ b/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)
diff --git a/app/controllers/devise/sessions_controller.rb b/app/controllers/devise/sessions_controller.rb
index 76b78020..41b74f39 100644
--- a/app/controllers/devise/sessions_controller.rb
+++ b/app/controllers/devise/sessions_controller.rb
@@ -28,7 +28,7 @@ class Devise::SessionsController < DeviseController
     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(non_navigational_status: :no_content)
   end
 
   protected
@@ -62,7 +62,7 @@ class Devise::SessionsController < DeviseController
     if all_signed_out?
       set_flash_message! :notice, :already_signed_out
 
-      respond_to_on_destroy
+      respond_to_on_destroy(non_navigational_status: :unauthorized)
     end
   end
 
@@ -72,11 +72,11 @@ class Devise::SessionsController < DeviseController
     users.all?(&:blank?)
   end
 
-  def respond_to_on_destroy
+  def respond_to_on_destroy(non_navigational_status: :no_content)
     # 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 non_navigational_status }
       format.any(*navigational_formats) { redirect_to after_sign_out_path_for(resource_name), status: Devise.responder.redirect_status }
     end
   end
diff --git a/test/controllers/sessions_controller_test.rb b/test/controllers/sessions_controller_test.rb
index 21b3c09f..9c970ab5 100644
--- a/test/controllers/sessions_controller_test.rb
+++ b/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