devise: sign_out doesn't properly clear cookie-based sessions

We have a Rails 4.0.5 app with Devise 3.2.4.

For some reason, when we call SessionsController#destroy, it correctly logs the current user out, but it does not correctly destroy the session when the OurApp::Application.config.session_store is set to :cookie_store. Installing the new activerecord-session_store gem and changing session_store to active_record_store resolves the issue.

Here’s the order of what happens:

  1. We DELETE /users/sign_out.
  2. #destroy gets called, and does seem to nuke the session. current_user becomes nil, and Warden doesn’t know about any users. However, the session cookie is not removed from the browser.
  3. after_sign_out_path_for returns the new_user_session_url, just like we asked it to.
  4. The page redirects to the new_user_session_url.
  5. We hit SessionsController#new. However, Devise somehow thinks the user is still logged in, and treats them as such, by moving on to 6:
  6. after_sign_in_path_for gets called. This is the step that we know shouldn’t be happening, but we can’t figure out how it’s getting here. It should never get called, because the user was logged out in step 2 above.

With the :cookie_store, if we manually delete the session cookie from the browser and then hit the #destroy endpoint, it works. If we use the active_record_store, none of this happens. It behaves as expected.

I’ve tried reverting to several previous Devise versions to see when it was introduced, but the behaviour is the same all the way back to 3.

If I can provide any more information to help in troubleshooting, please let me know. We’ve tried stepping into and all the way through the Devise code and nothing is jumping out at us.

About this issue

  • Original URL
  • State: closed
  • Created 10 years ago
  • Reactions: 8
  • Comments: 19 (5 by maintainers)

Commits related to this issue

Most upvoted comments

I found an article demonstrating a solution without having to change the storage:

You can add a session_token column to your devise model (e.g. User) and override Devise #authenticatable_salt method to contain your session token:

class User < ApplicationRecord

  devise :database_authenticatable, :recoverable, :rememberable
         
  def authenticatable_salt
    "#{super}#{session_token}"
  end

  def invalidate_all_sessions!
    self.session_token = SecureRandom.hex
  end
  
  ...
end

Now you need to invalidate the session cookie by resetting the session_token of a user on logout:

class Users::SessionsController < Devise::SessionsController
  
  def destroy
    current_user.invalidate_all_sessions!
    super
  end

end

I hope that helps.

Isn’t this bad though? Shouldn’t this be fixed so cookies are invalidated on the server instead?

  1. Log in.
  2. Get a _app_session cookie. Copy this cookie.
  3. Work
  4. Log out
  5. Cookie is destroyed in the browser
  6. Do NOT log in but replay an old request with the cookie in Step 2
  7. I can access the feature and its data

IMO this is not good at all. Have I missed something completely? Or is devise still expected to behave this way.

Yes, I know I can write my own stuff and store sessions in the DB or whatever but ideally Devise should handle this seamlessly. 😃

There is, use another session store instead of cookies.

No, we cannot handle this. This is the whole trade-off of using cookie-based sessions. They are replayable. Just use another storage if you don’t want this behaviour.

I found an article demonstrating a solution without having to change the storage:

You can add a session_token column to your devise model (e.g. User) and override Devise #authenticatable_salt method to contain your session token:

Anyone had luck with this? For some reason, it’s not working for me.

@sandipsubedi You’re right, this method isn’t working.

authenticatable_salt is only used when user logs in and when rememberable module refreshes user session. It won’t be verified if user already has valid (or, well, stolen) session cookie.

Devise already has remember_user_token invalidation mechanism based on remember_created_at column. So there is no reason to add additional custom session_token column.

Just use any store other than :cookie_store as José Valim suggested above.