Skip to content

Commit

Permalink
Modified sessions controller to return 401 for destroy action with no…
Browse files Browse the repository at this point in the history
… user signed in (#4878)
  • Loading branch information
Adan Amarillas authored and mracos committed Sep 19, 2019
1 parent 96a3153 commit 195cbfb
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 5 deletions.
8 changes: 4 additions & 4 deletions app/controllers/devise/sessions_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand All @@ -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) }
end
end
Expand Down
13 changes: 12 additions & 1 deletion test/controllers/sessions_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -88,6 +88,17 @@ class SessionsControllerTest < Devise::ControllerTestCase
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
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

if defined?(ActiveRecord) && ActiveRecord::Base.respond_to?(:mass_assignment_sanitizer)
test "#new doesn't raise mass-assignment exception even if sign-in key is attr_protected" do
request.env["devise.mapping"] = Devise.mappings[:user]
Expand Down

0 comments on commit 195cbfb

Please sign in to comment.