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>
This commit is contained in:
Adan Amarillas
2018-12-28 05:18:07 -08:00
committed by Carlos Antonio da Silva
parent 05bbc71446
commit 9a149ff139
3 changed files with 18 additions and 5 deletions

View File

@@ -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)

View File

@@ -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

View File

@@ -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