Skip to content
This repository has been archived by the owner on May 25, 2022. It is now read-only.

Update gems #682

Merged
merged 14 commits into from
Jul 30, 2014
Merged

Update gems #682

merged 14 commits into from
Jul 30, 2014

Conversation

sferik
Copy link
Contributor

@sferik sferik commented Jul 30, 2014

I updated various gem dependencies.

@rdsubhas
Copy link
Member

References rapidftr/tracker#80 - Thanks @sferik this looks awesome!

@sferik
Copy link
Contributor Author

sferik commented Jul 30, 2014

Cleaned up a few warnings and the build is passing now. Should be good to merge.

@rdsubhas
Copy link
Member

@sferik great! Just one thing though - the protect_from_forgery with: :null_session in ApplicationController, while its great for security, it think it may interfere with the Mobile client. I'm merging now but disabling the protect_from_forgery.

A little background: As with many Rails projects, RapidFTR started off with the same controller responding to multiple formats (json/xml/html) for both APIs as well as normal Web UI. Now Web apps will send CSRF/forgery tokens since they submit from a form, but the API clients do not send them since they're directly hitting the endpoint. We are slowly moving all these API requests into their own namespace and controllers (under controllers/api), and then we're planning to enable CSRF protection for the web apps, we have one or two places left.

rdsubhas added a commit that referenced this pull request Jul 30, 2014
@rdsubhas rdsubhas merged commit 1b80057 into rapidftr:master Jul 30, 2014
@sferik
Copy link
Contributor Author

sferik commented Jul 30, 2014

@rdsubhas I thought protect_from_forgery with: :null_session was safe for API clients. Can you please verify that this breaks the Android app before reverting this change? Either way, it would be nice to have a test for this.

@rdsubhas
Copy link
Member

@sferik Yep sure! That's a good suggestion, will do that...

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

Successfully merging this pull request may close these issues.

2 participants