diff --git a/CHANGELOG.md b/CHANGELOG.md index 49bb8628..83226636 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ * Allow a resource to be found based on its encrypted password token (by @karlentwistle) * bug fix + * Check if there is a signed in user before executing the `SessionsController#destroy`. * `SessionsController#destroy` no longer yields the `resource` to receiving block, since the resource isn't loaded in the action. If you need access to the current resource when overring the action use the scope helper (like `current_user`) before diff --git a/app/controllers/devise/sessions_controller.rb b/app/controllers/devise/sessions_controller.rb index 003ad2f7..8c41f653 100644 --- a/app/controllers/devise/sessions_controller.rb +++ b/app/controllers/devise/sessions_controller.rb @@ -1,6 +1,7 @@ class Devise::SessionsController < DeviseController prepend_before_filter :require_no_authentication, only: [ :new, :create ] prepend_before_filter :allow_params_authentication!, only: :create + prepend_before_filter :verify_signed_out_user, only: :destroy prepend_before_filter only: [ :create, :destroy ] { request.env["devise.skip_timeout"] = true } # GET /resource/sign_in @@ -21,17 +22,11 @@ class Devise::SessionsController < DeviseController # DELETE /resource/sign_out def destroy - redirect_path = after_sign_out_path_for(resource_name) signed_out = (Devise.sign_out_all_scopes ? sign_out : sign_out(resource_name)) set_flash_message :notice, :signed_out if signed_out && is_flashing_format? yield if block_given? - # 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.any(*navigational_formats) { redirect_to redirect_path } - end + respond_to_on_destroy end protected @@ -50,4 +45,33 @@ class Devise::SessionsController < DeviseController def auth_options { scope: resource_name, recall: "#{controller_path}#new" } end + + private + + # Check if there is no signed in user before doing the sign out. + # + # If there is no signed in user, it will set the flash message and redirect + # to the after_sign_out path. + def verify_signed_out_user + if all_signed_out? + set_flash_message :notice, :already_signed_out if is_flashing_format? + + respond_to_on_destroy + end + end + + def all_signed_out? + users = Devise.mappings.keys.map { |s| warden.user(scope: s, run_callbacks: false) } + + users.all?(&:blank?) + end + + def respond_to_on_destroy + # 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.any(*navigational_formats) { redirect_to after_sign_out_path_for(resource_name) } + end + end end diff --git a/config/locales/en.yml b/config/locales/en.yml index 54e936ba..e419f779 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -43,6 +43,7 @@ en: sessions: signed_in: "Signed in successfully." signed_out: "Signed out successfully." + already_signed_out: "Signed out successfully." unlocks: send_instructions: "You will receive an email with instructions for how to unlock your account in a few minutes." send_paranoid_instructions: "If your account exists, you will receive an email with instructions for how to unlock it in a few minutes." diff --git a/test/integration/authenticatable_test.rb b/test/integration/authenticatable_test.rb index 03940832..c98c9d0e 100644 --- a/test/integration/authenticatable_test.rb +++ b/test/integration/authenticatable_test.rb @@ -118,13 +118,13 @@ class AuthenticationSanityTest < ActionDispatch::IntegrationTest assert_not warden.authenticated?(:admin) end - test 'unauthenticated admin does not set message on sign out' do + test 'unauthenticated admin set message on sign out' do get destroy_admin_session_path assert_response :redirect assert_redirected_to root_path get root_path - assert_not_contain 'Signed out successfully' + assert_contain 'Signed out successfully' end test 'scope uses custom failure app' do @@ -711,3 +711,19 @@ class DoubleAuthenticationRedirectTest < ActionDispatch::IntegrationTest assert_redirected_to '/admin_area/home' end end + +class DoubleSignOutRedirectTest < ActionDispatch::IntegrationTest + test 'sign out after already having signed out redirects to sign in' do + sign_in_as_user + + post destroy_sign_out_via_delete_or_post_session_path + + get root_path + assert_contain 'Signed out successfully.' + + post destroy_sign_out_via_delete_or_post_session_path + + get root_path + assert_contain 'Signed out successfully.' + end +end