Radar grants 'user.is_staff = True' to all teachers and assistants

[markkuriekkinen]

Setting user.is_staff = True should be only done for sysadmins, but Radar does it for all teaching staff. Fix this and fix the code that assumes that teaching staff has it. Currently, teaching staff is able to access the Django admin view, thus they can read and modify more data than they should (e.g., someone else's courses). They could also break the system by deleting something important.

• It looks like Radar could be able to limit the course list to only the teacher's own courses, but since all teachers are "staff" (admins), then all teachers see all courses.

  • radar/data/models.py

    Lines 33 to 36 in cb2b531

    	def get_available_courses(self, user):
    	if user.is_staff:
    	return self.all()
    	return self.filter(reviewers=user)

• STAFF_ROLES in setttings.py: this tells the django-lti-plugin library to make all teachers and assistants "staff" (admins). Radar STAFF_ROLES should be set to the empty list.
  • https://github.com/apluslms/django-lti-login/blob/234b323417b82181ba2e17cc86c73993aa79fc58/django_lti_login/backends.py#L25

  • radar/radar/settings.py

    Lines 11 to 14 in cb2b531

    	AUTH_LTI_LOGIN = {
    	'ACCEPTED_ROLES': ['Instructor', 'TA'],
    	'STAFF_ROLES': ['Instructor', 'TA']
    	}

• This line presumably should not filter for is_staff at all. If a user is a reviewer in the Radar course, they should be course staff in the A+ course. There is a potential problem, though. If the first() call returns a teaching assistant, then they have limited access in the A+ course and Radar could be unable to load course data from the A+ API (because the API token does not provide adequate privileges). Then, we would need a way to separate teaching assistants and teachers in Radar.

  • radar/provider/aplus.py

    Lines 69 to 74 in cb2b531

    	def get_api_client(course):
    	"""
    	Return the AplusTokenClient of the first user with staff status from list of course reviewers.
    	"""
    	api_user = course.reviewers.filter(is_staff=True).first()
    	return api_user.get_api_client(course.namespace)

  • For your information, the course reviewers many-to-many field is updated during the LTI login:

  • radar/data/models.py

    Line 50 in cb2b531

    reviewers = models.ManyToManyField(settings.AUTH_USER_MODEL, related_name="courses", blank=True, help_text="Reviewers for match analysis")

  • radar/ltilogin/receivers.py

    Line 57 in cb2b531

    user.courses.add(course)