Prevent students from taking other students' attendances

[Veriny]

Before, we only checked whether the requesting user was the student that the attendance was being taken for, so savvy students could take attendance for their friends by directly sending an API request. Now, only mentors for the specific section are able to take attendance.

Fixed pylint stuff for student.py

[cypress]

Passing run #258 ↗︎

0

78

0

0

0

Details:

Prevent students from taking other students' attendances
Project: csm_web	Commit: f05ece1f06
Status: Passed	Duration: 02:07 💡
Started: Nov 14, 2023 3:46 AM	Ended: Nov 14, 2023 3:48 AM

Review all test suite changes for PR #450 ↗︎

[smartspot2]

Looking at this code a little bit more, I think there isn't actually any issues with the way the request checks are handled right now; see the get_queryset() method of the viewset:

csm_web/csm_web/scheduler/views/student.py

Lines 19 to 27 in 0af4cb6

	def get_queryset(self):
	own_profiles = Student.objects.filter(user=self.request.user, active=True)
	pupil_profiles = Student.objects.filter(
	section__mentor__user=self.request.user, active=True
	)
	coordinator_student_profiles = Student.objects.filter(
	section__mentor__course__coordinator__user=self.request.user, active=True
	)
	return (own_profiles | pupil_profiles | coordinator_student_profiles).distinct()

This is referenced in the first line of the attendances() method when a user tries to modify the student's attendance:

csm_web/csm_web/scheduler/views/student.py

Lines 61 to 63 in 0af4cb6

	@action(detail=True, methods=["get", "put"])
	def attendances(self, request, pk=None):
	student = get_object_or_error(self.get_queryset(), pk=pk)

In particular, this queryset filters for students in any of the following categories:

• The request user's student profiles
• The students in the request user's sections, for which the request user is a mentor for
• The students in the request user's course, for which the request user is a coordinator for

It's completely valid for the request user to modify any of the students in the last two points, and the students in the first point are filtered out by the check in the method.

Further, since student objects are unique across courses (i.e. a new student object is created if a user is in a section in a different course), there also should not be any issues with any of these student objects giving the request user too much control over their attendances. That is, a situation where the request user both a mentor and a peer for a student is impossible, since these relationships would have to occur in different courses—a user cannot be a mentor and a student of the same course.

Feel free to correct me if I'm wrong though, but on the current master branch I've tried using Postman to submit new requests for student attendances and I'm met with a 403 permission denied error due to the queryset filtering.

This PR is probably fine to keep and merge in though (with the spelling mistakes fixed), to add additional redundancies in case any of the above logic changes in the future; I'd be open to discussion about this as well.

[edwardneo]

Overall looks good, just a couple spelling mistakes, and the branch name should be fixed too (spelling mistake + not in the formate fix/*).

[edwardneo]

Nice!

[ericyche]

logic + spelling lgtm. it looks like some pre-commits are failing though