fix: Improving access check in CoursewareInformation view
We weren’t using has_access to check user access, which meant we were missing out on a bunch of checks. This PR adds a local _check_access function to CoursewareInformation. Ideally we would add this into access.py, but we’re adding it here for now to avoid any unexpected regressions in editing more commonly used code. This should ultimately be folded into our access system properly. TNL-7053
This commit is contained in:
@@ -80,6 +80,27 @@ class CoursewareInformation(RetrieveAPIView):
|
||||
|
||||
serializer_class = CourseInfoSerializer
|
||||
|
||||
def _check_access(self, user, overview):
|
||||
if has_access(user, 'staff', overview):
|
||||
return True
|
||||
|
||||
# We can only trust has_access in its false case because it doesn't check everything we
|
||||
# need to check.
|
||||
if not has_access(user, 'load', overview):
|
||||
return False
|
||||
|
||||
has_public_access = allow_public_access(overview, [COURSE_VISIBILITY_PUBLIC])
|
||||
if user.is_anonymous and not has_public_access:
|
||||
return False
|
||||
|
||||
if not CourseEnrollment.is_enrolled(user, overview.id) and not has_public_access:
|
||||
return False
|
||||
|
||||
# if is_survey_required_and_unanswered(user, course):
|
||||
# TODO: This.
|
||||
|
||||
return True
|
||||
|
||||
def get_object(self):
|
||||
"""
|
||||
Return the requested course object, if the user has appropriate
|
||||
@@ -91,6 +112,7 @@ class CoursewareInformation(RetrieveAPIView):
|
||||
self.request.user.username,
|
||||
CourseKey.from_string(self.kwargs['course_key_string']),
|
||||
)
|
||||
|
||||
if self.request.user.is_anonymous:
|
||||
mode = None
|
||||
is_active = False
|
||||
@@ -99,14 +121,14 @@ class CoursewareInformation(RetrieveAPIView):
|
||||
overview.effective_user,
|
||||
overview.id
|
||||
)
|
||||
|
||||
overview.enrollment = {'mode': mode, 'is_active': is_active}
|
||||
if not is_active:
|
||||
user_has_access = allow_public_access(overview, [COURSE_VISIBILITY_PUBLIC])
|
||||
else:
|
||||
user_has_access = True
|
||||
overview.user_has_access = user_has_access
|
||||
overview.user_has_staff_access = has_access(self.request.user, 'staff', overview).has_access
|
||||
|
||||
overview.can_load_course = self._check_access(self.request.user, overview)
|
||||
overview.is_staff = has_access(self.request.user, 'staff', overview).has_access
|
||||
|
||||
overview.user_has_access = overview.can_load_course # TODO: TNL-7053 Legacy: Delete once ready to contract
|
||||
overview.user_has_staff_access = overview.is_staff # TODO: TNL-7053 Legacy: Delete once ready to contract
|
||||
|
||||
return overview
|
||||
|
||||
def get_serializer_context(self):
|
||||
|
||||
Reference in New Issue
Block a user