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

NoMethodError: undefined method `destroy_session' for nil:NilClass #464

Closed
lephyrius opened this issue Oct 9, 2013 · 15 comments
Closed
Milestone

Comments

@lephyrius
Copy link

Im seeing sometimes that a user gets this error when he tries to login.

/vendor/bundle/ruby/2.0.0/gems/rack-1.5.2/lib/rack/session/abstract/id.rb:85 in "destroy"

/vendor/bundle/ruby/2.0.0/gems/actionpack-4.0.0/lib/action_dispatch/http/request.rb:257 in "reset_session"

/vendor/bundle/ruby/2.0.0/gems/actionpack-4.0.0/lib/action_controller/metal/rack_delegation.rb:22 in "reset_session"

/vendor/bundle/ruby/2.0.0/gems/sorcery-0.8.2/lib/sorcery/controller.rb:36 in "login"

/app/controllers/session_controller.rb:15 in "create" 

Any idea why? or how to fix it?

@lephyrius
Copy link
Author

Im using Puma + Rails 4 .
Are these related somehow?
heartcombo/devise#2488
And
heartcombo/devise#2467

Or is it just me trying to see something that isn't there?

@kirs
Copy link
Collaborator

kirs commented Oct 9, 2013

I'm using Sorcery with Rails 4 and Puma too but I didn't get such errors yet. Maybe I just have too small number of visitors there.

It has very similar affects like those Devise issues you linked. I will try to sort it out.

@jhilden
Copy link
Contributor

jhilden commented Oct 10, 2013

We were also experiencing this issue.

To reproduce it we go to our login page, delete cookies, and submit the login form.

The following change in our controller seems to have solved the problem:

ApplicationController
  protect_from_forgery with: :reset_session
end

But I guess that sorcery should also work the the Rails 4 default, which is :null_session

@paulkoegel
Copy link

+1

1 similar comment
@rosswilson
Copy link

+1

@kirs
Copy link
Collaborator

kirs commented Nov 3, 2013

Sorry for delay, I will take some on this week to work on this issue.

@kirs
Copy link
Collaborator

kirs commented Nov 3, 2013

I couldn't reproduce it on the empty app with Rails 4 and Puma.
My steps:

  1. Visit login page
  2. Delete cookies
  3. Submit the login form.

All i've got here was the ActionController::InvalidAuthenticityToken.

Here is the app: https://github.com/kirs/demo-sorcery-puma

Am I doing something wrong?

@jhilden
Copy link
Contributor

jhilden commented Nov 4, 2013

Thanks a lot for looking into this @kirs

The issue appears only if you use protect_from_forgery in your application_controller, either without any options (:null_session is the default) or explicitly using with: :null_session

see: http://api.rubyonrails.org/classes/ActionController/RequestForgeryProtection/ClassMethods.html#method-i-protect_from_forgery

I altered your test app accordingly: kirs/demo-sorcery-puma#1

I hope this helps in figuring out how to handle the issue.

@jhilden
Copy link
Contributor

jhilden commented Nov 4, 2013

After looking into this a little deeper I would think that this is actually an issue in Rails' reset_session method: https://github.com/rails/rails/blob/v4.0.0/actionpack/lib/action_dispatch/http/request.rb#L255-L262

In the :null_session case the session is just an empty Hash object, which unfortunately also responds to a destroy method, which leads to destroy_session being called on the the empty hash.

jhilden referenced this issue in rails/rails Nov 4, 2013
Previously it was raising a NilException
@jhilden
Copy link
Contributor

jhilden commented Nov 4, 2013

The underlying issue was fixed in Rails master in the commit above. Let's hope it will get released soon.

kirs added a commit that referenced this issue Nov 4, 2013
fix for #464
can be removed when Rails 4.1 is out
@kirs
Copy link
Collaborator

kirs commented Nov 4, 2013

@jhilden here is my PR with the hotfix until Rails 4.1 is out: #475
Could you check it, please?

@jhilden
Copy link
Contributor

jhilden commented Nov 5, 2013

I can confirm, that this fixed the issue in our app. Thanks.

@kirs
Copy link
Collaborator

kirs commented Nov 5, 2013

Great! I've merged it into master. New release will be later this week.

@kirs kirs closed this as completed Nov 5, 2013
@AlekSi
Copy link

AlekSi commented Dec 19, 2013

Was it released? If not, may you release it? If yes, may you update README?

@kirs
Copy link
Collaborator

kirs commented Dec 19, 2013

Sorry for delay and thanks for pinging me. I've just released 0.8.5, which is fully Rails 4 -ready.
Also with Rails 4 notes in README. Enjoy!

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

No branches or pull requests

6 participants