atmos / warden-github

:lock: warden strategy for github oauth
MIT License
54 stars 41 forks source link

Warden::GitHub::User and Rails 4.1.0.rc2 #38

Closed skorth closed 10 years ago

skorth commented 10 years ago

I created a new Rails app using version 4.1.0.rc2, the latest warden-github release 1.0.1 and Ruby-2.1.1. After performing warden.authenticate! within my controller env['warden'].user results in an string class. Instead of a struct the result looks like a marshaled (Warden::GitHub::User) struct. I tried changing the ruby and warden-github version, but the only way to fix this problem was using the current Rails Version 4.0.4. Don't know if its the new Rails Version / dependencies. Anyone has the same Problem?

atmos commented 10 years ago

I'm using 4.0.3 here. I tend to avoid the .rc releases because they tend to break stuff. :smiley:

skorth commented 10 years ago

After some hours digging into rails actionpack i found the reason. https://github.com/rails/rails/commit/b927d67decb9d4e5103b5991b7e26a4dab4eca92. Its not a bug, its the new rails default configuration (JSON vs. Marshal). Workaround is changing the default config from Rails.application.config.action_dispatch.cookies_serializer = :json into Rails.application.config.action_dispatch.cookies_serializer = :marshal.

atmos commented 10 years ago

I just upgraded 2 apps to 4.1 and didn't have to do anything special. I wonder if there's something else at play.

skorth commented 10 years ago

I think its because you upgraded, a new app would use json (app/config/initializers/cookies_serializer.rb)

For compatibility with old applications with existing +cookies, :marshal is used when serializer option is not specified.

plukevdh commented 10 years ago

Got bit by this as well with a brand new rails 4.1 app.

plukevdh commented 10 years ago

Update: looks like the browser had cached a cookie in an invalid marshal format. Once clearing for my specific app and trying to auth again, it worked fine.

atmos commented 10 years ago

@plukevdh So things do work with the json adapter if you clear cookies or change your session key?

plukevdh commented 10 years ago

It does so long as I use the Rails.application.config.action_dispatch.cookies_serializer = :marshal workaround.

plukevdh commented 10 years ago

I may need to retry without that change, but at last check, it needed :marshal.

atmos commented 10 years ago

I'm not setting this anywhere in my code in my apps and it seems to be working just fine. Closing this until other people run into it.

gjtorikian commented 10 years ago

Just want to note that I ran into this as well. I'm on Rails 4.1.4.

atmos commented 10 years ago

@gjtorikian Can you confirm that it works w/o the changes if you clear your session?

eugeneius commented 9 years ago

I ran into this problem on a new Rails 4.1 app recently. Clearing the session didn't help; the only way to fix it was to change the cookie serialization format to :marshal.

@atmos, to reproduce on an app you upgraded from an older version of Rails, you can add the new initializer as documented here. That initializer is now generated by rails new, which means this library is broken out of the box for new apps on Rails 4.1 and upwards.

The warden docs state that you should specify how you want your user object to be serialized. How would you feel about adding #to_hash and .from_hash methods to Warden::GitHub::User, and adding a section to the README recommending this configuration:

Warden::Manager.serialize_into_session do |user|
  user.to_hash
end

Warden::Manager.serialize_from_session do |hash|
  Warden::GitHub::User.from_hash(hash)
end
justinstern commented 9 years ago

Just ran into this with a new rails 4.2 install, the only solution was the Rails.application.config.action_dispatch.cookies_serializer = :marshal mentioned previously. I got the NoMethodError (undefined method 'organization_member?' for #<Hash:0x007ff00c53cd30>) exception otherwise no matter what I did, including using an incognito window.

It would be great to be able to use the newer :json or :hybrid settings with Warden

skorth commented 9 years ago

@justinstern I already use a new 4.2 Version and it works with :json, never had this problem again. Maybe setting a new Rails secret_key_base also forces the Browser to generate a fresh session. I remember that clearing tmp and caches didn't help.

atmos commented 9 years ago

I'm working on getting everyone updating the JSON stuff. Resetting session secret might be a simple solution but I just ran into this on a fresh 4.2 app myself.

ippeiukai commented 9 years ago

Just noting that I've just ran into this with a new rails 4.2 install too, exactly same as https://github.com/atmos/warden-github/issues/38#issuecomment-76423527 down to NoMethodError and message. Appropriately came here from link in https://github.com/fphilipe/warden-github-rails/issues/10#issuecomment-68590355.

Still, it would indeed be nice to have more 'proper' solution than reverting cookies_serializer of the entire app to marshal.

ippeiukai commented 9 years ago

This seems to solve the problem: https://github.com/atmos/warden-github/commit/bd1596a

justinstern commented 9 years ago

wow! @ippeiukai I can't claim that I understand everything you did here but in initial tests, your solution seems to resolve the issue for me. Thanks!