Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Session can get lost after a Devise sign in #56

Closed
BoboFraggins opened this issue Apr 10, 2020 · 7 comments
Closed

Session can get lost after a Devise sign in #56

BoboFraggins opened this issue Apr 10, 2020 · 7 comments

Comments

@BoboFraggins
Copy link

When upgrading the various redis-* gems recently, I ran into a problem where the contents of the session appears to get mangled when signing in via Devise. I've tried to narrow this down further, but I'm not familiar enough with the session internals to get to the bottom.

My best reproduction was creating a brand new Rails app. Add devise, redis, and redis-rails. Make a session_store initializer:

Rails.application.config.session_store :redis_store, key: '_my_session', signed: true, expire_after: 1.week

Make the ApplicationController be:

class ApplicationController < ActionController::Base
  protect_from_forgery with: :exception
  before_action :authenticate_user!
end

Make a root route and attempt to sign up on the website.

redis-rack: 2.1.2, redis-actionpack: 5.2.0

The authenticity token fails to match.

If you downgrade to redis-rack: 2.0.6, redis-actionpack: 5.1.0

All of the devise actions work.

I have uploaded my demo app here: https://github.com/BoboFraggins/redis-rack-error

@tubbo
Copy link
Contributor

tubbo commented Apr 24, 2020

thanks for the demo app, I'll check it out.

@tubbo
Copy link
Contributor

tubbo commented Apr 24, 2020

Removing signed: true seemed to fix the problem. I'm looking into why that might be. But if that's acceptable to you, it's a quick workaround for now.

@BoboFraggins
Copy link
Author

BoboFraggins commented Apr 24, 2020

We're not blocked; I just wanted you to know about it.

@tubbo
Copy link
Contributor

tubbo commented Jun 11, 2020

Thanks @BoboFraggins, I'm gonna start investigating why this occurred. Think it might have to do with some changes made to redis-store recently.

@kazk
Copy link

kazk commented Oct 17, 2020

I'm having the same issue.
@tubbo Have you found anything in redis-store? I'd like to help, but I'm not even sure what to look for.

@tubbo
Copy link
Contributor

tubbo commented Oct 18, 2020

Haven't had a chance to get around to this. Pull requests welcome :)

@kazk
Copy link

kazk commented Oct 19, 2020

@tubbo I think I found the cause and a fix. At least, it fixes the demo app.

I wasn't familiar with the codebase, so I started by stepping through with byebug. Then I noticed find_session and write_session from Rack::Session::Redis are receiving different sid when signed is true because the encrypted/signed value is passed to find_session.

After reading some code in rack, I found that extract_session_id can be overridden in ActionDispatch::Session::RedisStore to fix this.

I'll open a PR soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants