Hacker News new | ask | show | jobs
by MartinMond 5223 days ago
I agree with you, but even if this was a reaction to public outcry the real reason to be disturbed is that top-notch Ruby devs like the GitHub guys didn't use attr_accessible. I can't wrap my mind around that.

That you have to use attr_accessible is known throughout the Rails community since "ever". Only toy apps don't use it.

It's like saving passwords in plaintext, only arguably even worse.

2 comments

This is true. We actually don't use mass assignment that often. He happened to catch 2 our of 3 spots that still used it. Everywhere else is explicit about what to accept.
Ok, that is good to hear. Trust restored :) I was fearing you were using unsafe mass assignment everywhere...

Would you mind sharing any patters you use to DRY up explicit assignment?

We use this (posted by @dhh) https://gist.github.com/1975644 in some spots, or simply just Hash#slice. We have some other thoughts on making params access more explicit. The problem with explicit patterns is they can get left out if a developer forgets.
Or more specifically, since the public keys objects require an associated user, the old chestnut:

    @user.public_keys.build(...)
.. where @user is retrieved in a role based manner (that is, you only get the right @user if you are authorized to get it.)

Ultimately, this is less an issue of mass assignment specifically and more an overarching one of allowing a user to perform an action in the guise of another. But, of course, these mistakes are commonly made by developers of all skill levels! :-) (me included)

No, that's not entirely correct.

You can do @user.public_keys.find(params[:id]).update_attributes(:user_id => 25)

Its the mass assignment protection on foreign keys that prevents you assigning one of your public keys to someone else, ensuring the chain is correct doesn't necessarily help with this scenario.