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

Fix callback controllers not working when using a non-default MIME type. #688

Closed
wants to merge 1 commit into from
Closed

Conversation

andredigenova
Copy link

This code makes "on_path?" comparisons in Strategy.rb MIME type agnostic. Please review carefully as I am inexperienced with RoR work and just needed a fix for something I was working on.

Basically, on_path? being MIME type sensitive becomes a problem if someone wants to use a non-default type in their callbacks. In my case I had the route /users/auth/:action/callback(.:format) leading to my omniauth callbacks controller.

I have special template and redirect behaviour for the .tpl format and I used /users/auth/facebook/callback.tpl as a callback which resulted in request.env["omniauth.auth"] not being populated because OmniAuth incorrectly determined I was not on a callback path.

I'm not sure if there is a better way to accomplish this check. Any form of simple pattern matching has the chance of unintended positive matches if people get creative with their routes.

…n_path? We are trying to determine if we are on the callback path and the use of a non-default format would result in erroneously not running the callback routines.
@coveralls
Copy link

Coverage Status

Coverage decreased (-0%) when pulling 315c546 on fwny:master into 0ce4596 on intridea:master.

@andredigenova
Copy link
Author

This is Rails specific code. I'm not entirely sure how to go about generalizing it or if this is even a problem in other frameworks.

@andredigenova
Copy link
Author

This problem can be worked around via sending a 'format=blah' GET parameter.

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.

None yet

2 participants