search

songkick / oauth2-provider

Simple OAuth 2.0 provider toolkit
MIT License
529 stars 148 forks source link

Fix rails 3.2.3 mass assignment issue in authorizations #29

Closed edgar closed 11 years ago

jcoglan commented 12 years ago

What problem does this solve? Marking all the attributes as mass-assignable is probably not necessary and invites security breaches. I ought to apply attr_accessible here but not on such a broad scale.

edgar commented 12 years ago

Well, mass-assignment is a problem if you expose the model directly through a controller. Maybe I'm wrong but currently this is not the case for authorization model

Edgar González @edgar http://edgar.com.ve

http://piictu.com

On Jul 9, 2012, at 7:08 PM, James Coglan reply@reply.github.com wrote:

What problem does this solve? Marking all the attributes as mass-assignable is probably not necessary and invites security breaches. I ought to apply attr_accessible here but not on such a broad scale.

Reply to this email directly or view it on GitHub: https://github.com/songkick/oauth2-provider/pull/29#issuecomment-6862560

edgar commented 11 years ago

@jcoglan you're right I took a lazy approach. I changed to use attr_accessible nil in Authorization model. Please let me know what you think.

thanks

jcoglan commented 11 years ago

I'm not going to pull this because of the large volume of whitespace and spec changes. I've implemented attr_accessible nil in this commit: https://github.com/songkick/oauth2-provider/commit/67ff04fab231af3b1787d7133a9b8e884f89205b

edgar commented 11 years ago

Ok, same approach