search

voidlily / timeboard

Georgia Tech CoC TA timesheets
4 stars 1 forks source link

Security #17

Closed voidlily closed 13 years ago

voidlily commented 13 years ago

The big ones

Make sure students can't see other students timesheets Make sure only admins can edit users

Mass assignment might come into play again if we let users edit themselves in which case setting admin should be done separately and make sure only admins can set that one

jking31cs commented 13 years ago

So I should have already committed the students and professors being unable to view timesheets that aren't theirs. I also have it so that finance users can only see approved timesheets. Double check and make sure that I didn't forget any edge cases though

jking31cs commented 13 years ago

Right now, anyone can access the user admin pages. We should have a before filter on both the courses and the users controller making sure that the users are user admin. For finance users, we should allow them to edit users, but not add nor remove.

voidlily commented 13 years ago

Someone uncommented the line for the "wild controller" which seems like a VERY bad idea from a security standpoint. It makes every action accessible via GET.

apmonroe commented 13 years ago

removed the wild controller, had to fudge around some admin routes to get it working right.

apmonroe commented 13 years ago

only admins and finance users can edit users. is this a trusted group or do you think further steps to prevent mass assignment is necessary?

voidlily commented 13 years ago

In the future can you link the commit you did the change in? You can just put in the first ~8 digits of the hash and github makes the link for you. For example, 483dbd19

voidlily commented 13 years ago

As long as students can't edit themselves, this isn't much of a problem. If they could, then we'd need to check harder against mass assignment.