3 ms·
Hi Jason, Great work! The website has a great feel to it and it seems like a good first project to get out in the open. Definitely looks/works a gazillion time
by alinajaf 13y ago
Hi Jason,
Great work! The website has a great feel to it and it seems like a good first project to get out in the open. Definitely looks/works a gazillion times better than my first website!
There's an issue I'd like to point out that I've seen come up over and over again with people like yourself who've taught themselves to code via treehouse, codecademy or some of the other webdev-education based startups. I've met developers with 10+ years of experience who make the same error though, so I wouldn't beat yourself up too much about it.
This may have been deliberate, but it appears as though you can view all the rewards on the site (including other users) simply by modifying the URL. It also seems like you can modify or delete any of the rewards on the site too (try changing the form action/fields using chrome web inspector and submit the form). Since this is your first rails app and I'm assuming you're not hoping it's going to hold any sensitive data, this isn't that big a deal.
Do keep in mind though that in its current state, it's trivial to create a script that deletes all the rewards on the website or worse, fills all the reward info with spam data.
For the last person I pointed this out to, I put together a gist of what the before => after of the code should look like for fixing this https://gist.github.com/Najaf/5431024 https://gist.github.com/Najaf/5431024 (tl&dr, you need to whitelist those params and implement authorization).
If you'd like any further help with this, feel free to ping me via email.
Best of luck!
- jasonshen 13y agoThat's super helpful and thank you for the very kind and welcoming way you delivered that feedback. I wish more HN comments were this nice. It takes longer to write but is more likely to be implemented. edit - Made the update! Sorry anyone who went to the site when it was down for 20 seconds prev edit - ans -> and
- jlogsdon 13y agoCheck out CanCan[1] for authorization. It's pretty simple to get started with, but provides a lot of power for controlling who can access what. [1] https://github.com/ryanb/cancan https://github.com/ryanb/cancan
- alinajaf 13y agoSeconded! Using current_user.rewards.find(...) is a good stopgap if you're not keen on going as far as using cancan. This won't take care of the parameter whitelisting for you though.
- mikeyouse 13y agoChrome v28.0.1500.6 dev on a MBP is rendering the page a little strange: http://i.imgur.com/KfKFGjd.png http://i.imgur.com/KfKFGjd.png I assume 1, 2, and 3 are supposed to be on the same line?
- jasonshen 13y agoOh weird. Yeah, they are 3 divs that are supposed to take up 33% of the width until they collapse ontop of each other as the screen shrinks. That shouldn't happen - will try to reproduce and fix.
- alinajaf 13y agoFYI it looks like you've fixed the update controller (as per the gist), but I can still: * See other peoples rewards (just go to /rewards/whatever-id), not that big a deal, unless you want to keep them private. * Go to the edit form of any reward (just go to /rewards/whatever-id/edit), big-deal factor is same as above. * Delete other peoples rewards (inspect the delete link of any reward in chrome, modify the id in the href, then click it), this is somewhat of a big deal. Sorry to whoever had reward with id 4! PITA I know, security doesn't get easier I'm afraid :) EDIT: updated gist to be a little more useful, though you probably worked the required code out for yourself.
- deleted 13y ago[deleted]
- jasonshen 13y agoThat's super helpful and thank you for the very kind and welcoming way you delivered that feedback. I wish more HN comments were this nice. It takes longer to write but is more likely to be implemented. edit - ans -> and