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

Protect upmin controllers from CSRF attacks #178

Merged
merged 1 commit into from Mar 31, 2016

Conversation

@jsyeo
Copy link
Contributor

jsyeo commented Mar 31, 2016

Both DashboardController and ModelsController inherit from ApplicationController and it is a direct subclass of ActionController::Base, but it doesn't call protect_from_forgery to turn on CSRF protection. Thus all the controllers in upmin are potentially vulnerable to CSRF attacks. This PR inserts a call in upmin's ApplicationController to turn on CSRF verification in the application.

@jsyeo jsyeo force-pushed the jsyeo:jsyeo-csrf-protection branch from e0f27fa to e4e3125 Mar 31, 2016
@mbrookes
Copy link
Contributor

mbrookes commented Mar 31, 2016

Upmin is no longer maintained, you might want to look at https://github.com/thoughtbot/administrate instead.

If you're actively using Upmin, and wish to take over development, I"m sure the owner would be open to it.

@jsyeo
Copy link
Contributor Author

jsyeo commented Mar 31, 2016

@mbrookes yep, I am aware of that. I am just submitting this PR just in case there are people still using upmin. Do note that the maintainer announced that he is no longer maintaining this only just five months ago.

@mbrookes mbrookes merged commit 49252e6 into upmin:master Mar 31, 2016
@mbrookes
Copy link
Contributor

mbrookes commented Mar 31, 2016

Just saying. 😄 I'll merge without review, but it's unlikely to get published.

@jsyeo jsyeo deleted the jsyeo:jsyeo-csrf-protection branch Mar 31, 2016
@jsyeo
Copy link
Contributor Author

jsyeo commented Mar 31, 2016

Thanks!
dance

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.