From 3b6a86bb689253c08e83322605eaa6be4d1065e8 Mon Sep 17 00:00:00 2001 From: David Joy Date: Thu, 2 Apr 2020 14:57:33 -0400 Subject: [PATCH] fix: Improving access check in CoursewareInformation view MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .../core/djangoapps/courseware_api/views.py | 36 +++++++++++++++---- 1 file changed, 29 insertions(+), 7 deletions(-) diff --git a/openedx/core/djangoapps/courseware_api/views.py b/openedx/core/djangoapps/courseware_api/views.py index a056d2c22e..1921323dd3 100644 --- a/openedx/core/djangoapps/courseware_api/views.py +++ b/openedx/core/djangoapps/courseware_api/views.py @@ -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):