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

Ignore 'extra' in Twitter auth response to avoid CookieOverflow. Fixes #145. #179

Merged
merged 1 commit into from
Mar 12, 2015

Conversation

tbloncar
Copy link
Contributor

This addresses #145.

@lynndylanhurley
Copy link
Owner

Thx @tbloncar!!

lynndylanhurley added a commit that referenced this pull request Mar 12, 2015
Ignore 'extra' in Twitter auth response to avoid CookieOverflow. Fixes #145.
@lynndylanhurley lynndylanhurley merged commit 2a36a7e into lynndylanhurley:master Mar 12, 2015
@lynndylanhurley
Copy link
Owner

You're now an official contributor!

@tbloncar
Copy link
Contributor Author

Thanks, @lynndylanhurley.

@lynndylanhurley
Copy link
Owner

I just pushed version 0.1.32.beta7. Please pull it down and confirm that it works ok.

@nicolas-besnard
Copy link
Contributor

What does it change to remove the 'extra' parameter ?
On Thu 12 Mar 2015 at 08:54 Lynn Dylan Hurley [email protected]
wrote:

I just pushed version 0.1.32.beta7. Please pull it down and confirm that
it works ok.


Reply to this email directly or view it on GitHub
#179 (comment)
.

@lynndylanhurley
Copy link
Owner

The schema description is here. I'm not sure exactly - I think it varies by provider.

@nicolas-besnard
Copy link
Contributor

I'm not sure this is the right think to do. I'm using this extra parameters on my projects.
This extra parameters is seeded with data depending on the scope you use such as scope: 'email, user_birthday, user_about_me' when you use Facebook (for example).

@tbloncar
Copy link
Contributor Author

@nicolas-besnard Your point makes sense to me; I think the issue here is that the default behavior for Twitter auth (before this change) was a raise of ActionDispatch::Cookies::CookieOverflow. While this change ameliorates that issue, I see that it may result in missing some of the data you mentioned.

One alternative to this solution is making some mention of using an alternate session store (like ActiveRecord store) in the README. This would be "as needed," of course. Another option might be using except('extra') for only the Twitter callback (for now).

@c0mrade
Copy link

c0mrade commented Mar 12, 2015

Yeah I got the same CookieOverflow from google too, and I'm using a database as a data store to solve that problem. It all depends what info are you looking to get (as Nicolas already explained). You may or may get the same issue. Yeah doesn't sound right to restrict people who want to get that extra info (I myself don't use or need it), just thinking out loud.

@nicolas-besnard
Copy link
Contributor

As this is an API, we don't need Cookie or Session. I just think the
implementation of Omniauth is not valid on this gem. I'll dig this this
weekend.

On Thu, Mar 12, 2015 at 3:35 PM Emir Ibrahimbegovic <
[email protected]> wrote:

Yeah I got the same CookieOverflow from google too, and I'm using a
database as a data store. It all depends what info are you looking to get.
It may or may not resolve in the issue. Yeah doesn't sound right to
restrict people who want to get that extra info (I myself don't use or need
it), just thinking out loud.


Reply to this email directly or view it on GitHub
#179 (comment)
.

@c0mrade
Copy link

c0mrade commented Mar 12, 2015

Yes that was my initial thought but I had some errors and had to deliver something, so I ended up adding session middleware so the code works, thanks for reminding me I will need to remove that.
(Talking about my project, not on the actual repo here)

@medaglia
Copy link

I too am using omniauth_facebook and require the data passed via extra (age_range, gender, etc). I wasn't able to override redirect_callbacks (whereas i have no problem overriding omniauth_failure). Does anyone have any any good suggestions? Right now I'm torn between...

  1. branching omniauth_facebook to pass the data through info instead of extra
  2. branching devise_token_auth to keep extra

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 this pull request may close these issues.

5 participants