From 7a400e29170c837f943d787134c8b5c54fa30836 Mon Sep 17 00:00:00 2001 From: David Joy Date: Wed, 15 Apr 2020 10:29:24 -0400 Subject: [PATCH] Factoring redirects out of check_course_access so it can be used with courseware_api (#23651) TNL-7053 The courseware_api view will use check_course_access - which now returns AccessResponse objects, and all other uses of check_course_access will now use check_course_access_with_redirect, which is a drop-in replacement for the original check_course_access implementation. We also added a few new helpers to access_utils: - check_public_access is a replacement for allow_public_access, which now returns AccessResponse objects - check_enrollment checks if the learner is enrolled, and uses check_public_access to account for COURSE_ENABLE_UNENROLLED_ACCESS_FLAG - check_survey checks whether there is a required survey that the learner must complete prior to accessing the course. There are two new kinds of AccessError subclasses: - SurveyRequiredAccessError - EnrollmentRequiredAccessError --- lms/djangoapps/courseware/access_response.py | 22 ++++ lms/djangoapps/courseware/access_utils.py | 69 +++++++++++- lms/djangoapps/courseware/courses.py | 104 +++++++++++++----- lms/djangoapps/courseware/views/index.py | 43 +++----- lms/djangoapps/courseware/views/views.py | 6 +- lms/djangoapps/survey/tests/test_utils.py | 12 +- lms/djangoapps/survey/utils.py | 35 ++++-- .../djangoapps/courseware_api/serializers.py | 6 +- .../courseware_api/tests/test_views.py | 27 +++-- .../core/djangoapps/courseware_api/views.py | 38 ++----- .../views/course_home_messages.py | 4 +- 11 files changed, 243 insertions(+), 123 deletions(-) diff --git a/lms/djangoapps/courseware/access_response.py b/lms/djangoapps/courseware/access_response.py index d6a4b86fda..025ac50dd5 100644 --- a/lms/djangoapps/courseware/access_response.py +++ b/lms/djangoapps/courseware/access_response.py @@ -214,3 +214,25 @@ class NoAllowedPartitionGroupsError(AccessError): error_code = "no_allowed_user_groups" developer_message = u"Group access for {} excludes all students".format(partition.name) super(NoAllowedPartitionGroupsError, self).__init__(error_code, developer_message, user_message) + + +class EnrollmentRequiredAccessError(AccessError): + """ + Access denied because the user must be enrolled in the course + """ + def __init__(self): + error_code = "enrollment_required" + developer_message = u"User must be enrolled in the course" + user_message = _(u"You must be enrolled in the course") + super(EnrollmentRequiredAccessError, self).__init__(error_code, developer_message, user_message) + + +class AuthenticationRequiredAccessError(AccessError): + """ + Access denied because the user must be authenticated to see it + """ + def __init__(self): + error_code = "authentication_required" + developer_message = u"User must be authenticated to view the course" + user_message = _(u"You must be logged in to see this course") + super(AuthenticationRequiredAccessError, self).__init__(error_code, developer_message, user_message) diff --git a/lms/djangoapps/courseware/access_utils.py b/lms/djangoapps/courseware/access_utils.py index 81cc6975e1..43b9a2520f 100644 --- a/lms/djangoapps/courseware/access_utils.py +++ b/lms/djangoapps/courseware/access_utils.py @@ -10,14 +10,23 @@ from logging import getLogger from django.conf import settings from django.utils.translation import ugettext as _ from pytz import UTC - -from lms.djangoapps.courseware.access_response import AccessResponse, StartDateError +from lms.djangoapps.courseware.access_response import ( + AccessResponse, + StartDateError, + EnrollmentRequiredAccessError, + AuthenticationRequiredAccessError, +) from lms.djangoapps.courseware.masquerade import get_course_masquerade, is_masquerading_as_student from openedx.core.djangoapps.util.user_messages import PageLevelMessages from openedx.core.djangolib.markup import HTML -from openedx.features.course_experience import COURSE_PRE_START_ACCESS_FLAG +from openedx.features.course_experience import ( + COURSE_PRE_START_ACCESS_FLAG, + COURSE_ENABLE_UNENROLLED_ACCESS_FLAG, +) +from student.models import CourseEnrollment from student.roles import CourseBetaTesterRole from xmodule.util.xmodule_django import get_current_request_hostname +from xmodule.course_module import COURSE_VISIBILITY_PUBLIC DEBUG_ACCESS = False log = getLogger(__name__) @@ -103,3 +112,57 @@ def check_course_open_for_learner(user, course): if COURSE_PRE_START_ACCESS_FLAG.is_enabled(): return ACCESS_GRANTED return check_start_date(user, course.days_early_for_beta, course.start, course.id) + + +def check_enrollment(user, course): + """ + Check if the course requires a learner to be enrolled for access. + + Returns: + AccessResponse: Either ACCESS_GRANTED or EnrollmentRequiredAccessError. + """ + if check_public_access(course, [COURSE_VISIBILITY_PUBLIC]): + return ACCESS_GRANTED + + if CourseEnrollment.is_enrolled(user, course.id): + return ACCESS_GRANTED + + return EnrollmentRequiredAccessError() + + +def check_authentication(user, course): + """ + Grants access if the user is authenticated, or if the course allows public access. + + Returns: + AccessResponse: Either ACCESS_GRANTED or AuthenticationRequiredAccessError + """ + if user.is_authenticated: + return ACCESS_GRANTED + + if check_public_access(course, [COURSE_VISIBILITY_PUBLIC]): + return ACCESS_GRANTED + + return AuthenticationRequiredAccessError() + + +def check_public_access(course, visibilities): + """ + This checks if the unenrolled access waffle flag for the course is set + and the course visibility matches any of the input visibilities. + + The "visibilities" argument is one of these constants from xmodule.course_module: + - COURSE_VISIBILITY_PRIVATE + - COURSE_VISIBILITY_PUBLIC + - COURSE_VISIBILITY_PUBLIC_OUTLINE + + Returns: + AccessResponse: Either ACCESS_GRANTED or ACCESS_DENIED. + """ + + unenrolled_access_flag = COURSE_ENABLE_UNENROLLED_ACCESS_FLAG.is_enabled(course.id) + allow_access = unenrolled_access_flag and course.course_visibility in visibilities + if allow_access: + return ACCESS_GRANTED + + return ACCESS_DENIED diff --git a/lms/djangoapps/courseware/courses.py b/lms/djangoapps/courseware/courses.py index 4db0313b9d..f670eb6576 100644 --- a/lms/djangoapps/courseware/courses.py +++ b/lms/djangoapps/courseware/courses.py @@ -26,7 +26,12 @@ from six import text_type import branding from course_modes.models import CourseMode from lms.djangoapps.courseware.access import has_access -from lms.djangoapps.courseware.access_response import MilestoneAccessError, StartDateError +from lms.djangoapps.courseware.access_response import ( + AuthenticationRequiredAccessError, + EnrollmentRequiredAccessError, + MilestoneAccessError, + StartDateError, +) from lms.djangoapps.courseware.date_summary import ( CertificateAvailableDate, CourseAssignmentDate, @@ -42,6 +47,10 @@ from lms.djangoapps.courseware.model_data import FieldDataCache from lms.djangoapps.courseware.module_render import get_module from edxmako.shortcuts import render_to_string from lms.djangoapps.certificates import api as certs_api +from lms.djangoapps.courseware.access_utils import ( + check_authentication, + check_enrollment, +) from lms.djangoapps.courseware.courseware_access_exception import CoursewareAccessException from lms.djangoapps.courseware.exceptions import CourseAccessRedirect from openedx.core.djangoapps.content.course_overviews.models import CourseOverview @@ -52,7 +61,7 @@ from openedx.features.course_duration_limits.access import AuditExpiredError from openedx.features.course_experience import COURSE_ENABLE_UNENROLLED_ACCESS_FLAG, RELATIVE_DATES_FLAG from static_replace import replace_static_urls from student.models import CourseEnrollment -from survey.utils import is_survey_required_and_unanswered +from survey.utils import SurveyRequiredAccessError, check_survey_required_and_unanswered from util.date_utils import strftime_localized from xmodule.modulestore.django import modulestore from xmodule.modulestore.exceptions import ItemNotFoundError @@ -99,7 +108,7 @@ def get_course_by_id(course_key, depth=0): raise Http404(u"Course not found: {}.".format(six.text_type(course_key))) -def get_course_with_access(user, action, course_key, depth=0, check_if_enrolled=False, check_survey_complete=True): +def get_course_with_access(user, action, course_key, depth=0, check_if_enrolled=False, check_survey_complete=True, check_if_authenticated=False): """ Given a course_key, look up the corresponding course descriptor, check that the user has the access to perform the specified action @@ -118,7 +127,7 @@ def get_course_with_access(user, action, course_key, depth=0, check_if_enrolled= be plugged in as additional callback checks for different actions. """ course = get_course_by_id(course_key, depth) - check_course_access(course, user, action, check_if_enrolled, check_survey_complete) + check_course_access_with_redirect(course, user, action, check_if_enrolled, check_survey_complete, check_if_authenticated) return course @@ -137,11 +146,11 @@ def get_course_overview_with_access(user, action, course_key, check_if_enrolled= course_overview = CourseOverview.get_from_id(course_key) except CourseOverview.DoesNotExist: raise Http404("Course not found.") - check_course_access(course_overview, user, action, check_if_enrolled) + check_course_access_with_redirect(course_overview, user, action, check_if_enrolled) return course_overview -def check_course_access(course, user, action, check_if_enrolled=False, check_survey_complete=True): +def check_course_access(course, user, action, check_if_enrolled=False, check_survey_complete=True, check_if_authenticated=False): """ Check that the user has the access to perform the specified action on the course (CourseDescriptor|CourseOverview). @@ -149,14 +158,57 @@ def check_course_access(course, user, action, check_if_enrolled=False, check_sur check_if_enrolled: If true, additionally verifies that the user is enrolled. check_survey_complete: If true, additionally verifies that the user has completed the survey. """ - # Allow staff full access to the course even if not enrolled - if has_access(user, 'staff', course.id): - return + def _check_nonstaff_access(): + # Below is a series of checks that must all pass for a user to be granted access + # to a course. (Essentially check this AND check that AND...) + # Also note: access_response (AccessResponse) objects are compared as booleans + access_response = has_access(user, action, course, course.id) + if not access_response: + return access_response + if check_if_authenticated: + authentication_access_response = check_authentication(user, course) + if not authentication_access_response: + return authentication_access_response + + if check_if_enrolled: + enrollment_access_response = check_enrollment(user, course) + if not enrollment_access_response: + return enrollment_access_response + + # Redirect if the user must answer a survey before entering the course. + if check_survey_complete and action == 'load': + survey_access_response = check_survey_required_and_unanswered(user, course) + if not survey_access_response: + return survey_access_response + + # This access_response will be ACCESS_GRANTED + return access_response + + # Allow staff full access to the course even if other checks fail + nonstaff_access_response = _check_nonstaff_access() + if not nonstaff_access_response: + staff_access_response = has_access(user, 'staff', course.id) + if staff_access_response: + return staff_access_response + + # This access_response will be ACCESS_GRANTED + return nonstaff_access_response + + +def check_course_access_with_redirect(course, user, action, check_if_enrolled=False, check_survey_complete=True, check_if_authenticated=False): + """ + Check that the user has the access to perform the specified action + on the course (CourseDescriptor|CourseOverview). + + check_if_enrolled: If true, additionally verifies that the user is enrolled. + check_survey_complete: If true, additionally verifies that the user has completed the survey. + """ request = get_current_request() check_content_start_date_for_masquerade_user(course.id, user, request, course.start) - access_response = has_access(user, action, course, course.id) + access_response = check_course_access(course, user, action, check_if_enrolled, check_survey_complete, check_if_authenticated) + if not access_response: # Redirect if StartDateError if isinstance(access_response, StartDateError): @@ -183,20 +235,22 @@ def check_course_access(course, user, action, check_if_enrolled=False, check_sur dashboard_url=reverse('dashboard'), ), access_response) + # Redirect if the user is not enrolled and must be to see content + if isinstance(access_response, EnrollmentRequiredAccessError): + raise CourseAccessRedirect(reverse('about_course', args=[str(course.id)])) + + # Redirect if user must be authenticated to view the content + if isinstance(access_response, AuthenticationRequiredAccessError): + raise CourseAccessRedirect(reverse('about_course', args=[str(course.id)])) + + # Redirect if the user must answer a survey before entering the course. + if isinstance(access_response, SurveyRequiredAccessError): + raise CourseAccessRedirect(reverse('course_survey', args=[str(course.id)])) + # Deliberately return a non-specific error message to avoid # leaking info about access control settings raise CoursewareAccessException(access_response) - if check_if_enrolled: - # If the user is not enrolled, redirect them to the about page - if not CourseEnrollment.is_enrolled(user, course.id): - raise CourseAccessRedirect(reverse('about_course', args=[six.text_type(course.id)])) - - # Redirect if the user must answer a survey before entering the course. - if check_survey_complete and action == 'load': - if is_survey_required_and_unanswered(user, course): - raise CourseAccessRedirect(reverse('course_survey', args=[six.text_type(course.id)])) - def can_self_enroll_in_course(course_key): """ @@ -745,13 +799,3 @@ def get_course_chapter_ids(course_key): log.exception('Failed to retrieve course from modulestore.') return [] return [six.text_type(chapter_key) for chapter_key in chapter_keys if chapter_key.block_type == 'chapter'] - - -def allow_public_access(course, visibilities): - """ - This checks if the unenrolled access waffle flag for the course is set - and the course visibility matches any of the input visibilities. - """ - unenrolled_access_flag = COURSE_ENABLE_UNENROLLED_ACCESS_FLAG.is_enabled(course.id) - allow_access = unenrolled_access_flag and course.course_visibility in visibilities - return allow_access diff --git a/lms/djangoapps/courseware/views/index.py b/lms/djangoapps/courseware/views/index.py index 3f38eaab5e..123e63368b 100644 --- a/lms/djangoapps/courseware/views/index.py +++ b/lms/djangoapps/courseware/views/index.py @@ -7,7 +7,6 @@ View for Courseware Index import logging -from datetime import timedelta import six import six.moves.urllib as urllib # pylint: disable=import-error import six.moves.urllib.error # pylint: disable=import-error @@ -20,7 +19,6 @@ from django.db import transaction from django.http import Http404 from django.template.context_processors import csrf from django.urls import reverse -from django.utils import timezone from django.utils.decorators import method_decorator from django.utils.functional import cached_property from django.utils.translation import ugettext as _ @@ -32,7 +30,6 @@ from opaque_keys import InvalidKeyError from opaque_keys.edx.keys import CourseKey, UsageKey from web_fragments.fragment import Fragment -from course_modes.models import CourseMode from edxmako.shortcuts import render_to_response, render_to_string from lms.djangoapps.courseware.exceptions import CourseAccessRedirect, Redirect from lms.djangoapps.experiments.utils import get_experiment_user_metadata_context @@ -52,7 +49,6 @@ from openedx.features.course_experience import ( RELATIVE_DATES_FLAG, ) from openedx.features.course_experience.urls import COURSE_HOME_VIEW_NAME -from openedx.features.course_experience.utils import get_course_outline_block_tree from openedx.features.course_experience.utils import reset_deadlines_banner_should_display from openedx.features.course_experience.views.course_sock import CourseSockFragmentView from openedx.features.enterprise_support.api import data_sharing_consent_required @@ -65,9 +61,9 @@ from xmodule.modulestore.django import modulestore from xmodule.x_module import PUBLIC_VIEW, STUDENT_VIEW from ..access import has_access +from ..access_utils import check_public_access from ..courses import ( - allow_public_access, - check_course_access, + check_course_access_with_redirect, get_course_with_access, get_current_child, get_studio_url @@ -152,34 +148,25 @@ class CoursewareIndex(View): self.view = STUDENT_VIEW - # Do the enrollment check if enable_unenrolled_access is not enabled. self.course = get_course_with_access( request.user, 'load', self.course_key, depth=CONTENT_DEPTH, - check_if_enrolled=not self.enable_unenrolled_access, + check_if_enrolled=True, + check_if_authenticated=True ) self.course_overview = CourseOverview.get_from_id(self.course.id) + self.is_staff = has_access(request.user, 'staff', self.course) - if self.enable_unenrolled_access: - # Check if the user is considered enrolled (i.e. is an enrolled learner or staff). - try: - check_course_access( - self.course, request.user, 'load', check_if_enrolled=True, - ) - except CourseAccessRedirect as exception: - # If the user is not considered enrolled: - if self.course.course_visibility == COURSE_VISIBILITY_PUBLIC: - # If course visibility is public show the XBlock public_view. - self.view = PUBLIC_VIEW - else: - # Otherwise deny them access. - raise exception - else: - # If the user is considered enrolled show the default XBlock student_view. - pass + # There's only one situation where we want to show the public view + if ( + not self.is_staff and + self.enable_unenrolled_access and + self.course.course_visibility == COURSE_VISIBILITY_PUBLIC and + not CourseEnrollment.is_enrolled(request.user, self.course_key) + ): + self.view = PUBLIC_VIEW self.can_masquerade = request.user.has_perm(MASQUERADE_AS_STUDENT, self.course) - self.is_staff = has_access(request.user, 'staff', self.course) self._setup_masquerade_for_effective_user() return self.render(request) @@ -250,7 +237,7 @@ class CoursewareIndex(View): 'email_opt_in': False, }) - allow_anonymous = allow_public_access(self.course, [COURSE_VISIBILITY_PUBLIC]) + allow_anonymous = check_public_access(self.course, [COURSE_VISIBILITY_PUBLIC]) if not allow_anonymous: PageLevelMessages.register_warning_message( @@ -462,7 +449,7 @@ class CoursewareIndex(View): ) staff_access = self.is_staff - allow_anonymous = allow_public_access(self.course, [COURSE_VISIBILITY_PUBLIC]) + allow_anonymous = check_public_access(self.course, [COURSE_VISIBILITY_PUBLIC]) display_reset_dates_banner = False if not allow_anonymous and RELATIVE_DATES_FLAG.is_enabled(self.course.id): display_reset_dates_banner = reset_deadlines_banner_should_display(self.course_key, request) diff --git a/lms/djangoapps/courseware/views/views.py b/lms/djangoapps/courseware/views/views.py index 0fcbba66c0..421fe1afb8 100644 --- a/lms/djangoapps/courseware/views/views.py +++ b/lms/djangoapps/courseware/views/views.py @@ -83,7 +83,7 @@ from lms.djangoapps.ccx.custom_exception import CCXLocatorValidationException from lms.djangoapps.certificates import api as certs_api from lms.djangoapps.certificates.models import CertificateStatuses from lms.djangoapps.commerce.utils import EcommerceService -from lms.djangoapps.courseware.courses import allow_public_access +from lms.djangoapps.courseware.access_utils import check_public_access from lms.djangoapps.courseware.exceptions import CourseAccessRedirect, Redirect from lms.djangoapps.experiments.utils import get_experiment_user_metadata_context from lms.djangoapps.grades.api import CourseGradeFactory @@ -621,7 +621,7 @@ class CourseTabView(EdxFragmentView): """ Register messages to be shown to the user if they have limited access. """ - allow_anonymous = allow_public_access(course, [COURSE_VISIBILITY_PUBLIC]) + allow_anonymous = check_public_access(course, [COURSE_VISIBILITY_PUBLIC]) if request.user.is_anonymous and not allow_anonymous: if CourseTabView.course_open_for_learner_enrollment(course): @@ -982,7 +982,7 @@ def course_about(request, course_id): sidebar_html_enabled = course_experience_waffle().is_enabled(ENABLE_COURSE_ABOUT_SIDEBAR_HTML) - allow_anonymous = allow_public_access(course, [COURSE_VISIBILITY_PUBLIC, COURSE_VISIBILITY_PUBLIC_OUTLINE]) + allow_anonymous = check_public_access(course, [COURSE_VISIBILITY_PUBLIC, COURSE_VISIBILITY_PUBLIC_OUTLINE]) # This local import is due to the circularity of lms and openedx references. # This may be resolved by using stevedore to allow web fragments to be used diff --git a/lms/djangoapps/survey/tests/test_utils.py b/lms/djangoapps/survey/tests/test_utils.py index e0aedf5462..1bbc6bcab9 100644 --- a/lms/djangoapps/survey/tests/test_utils.py +++ b/lms/djangoapps/survey/tests/test_utils.py @@ -9,7 +9,7 @@ from django.contrib.auth.models import User from django.test.client import Client from survey.models import SurveyForm -from survey.utils import is_survey_required_and_unanswered, is_survey_required_for_course +from survey.utils import check_survey_required_and_unanswered, is_survey_required_for_course from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory @@ -91,28 +91,28 @@ class SurveyModelsTests(ModuleStoreTestCase): """ Assert that a new course which has a required survey but user has not answered it yet """ - self.assertTrue(is_survey_required_and_unanswered(self.student, self.course)) + self.assertFalse(check_survey_required_and_unanswered(self.student, self.course)) temp_course = CourseFactory.create( course_survey_required=False ) - self.assertFalse(is_survey_required_and_unanswered(self.student, temp_course)) + self.assertTrue(check_survey_required_and_unanswered(self.student, temp_course)) temp_course = CourseFactory.create( course_survey_required=True, course_survey_name="NonExisting" ) - self.assertFalse(is_survey_required_and_unanswered(self.student, temp_course)) + self.assertTrue(check_survey_required_and_unanswered(self.student, temp_course)) def test_user_has_answered_required_survey(self): """ Assert that a new course which has a required survey and user has answers for it """ self.survey.save_user_answers(self.student, self.student_answers, None) - self.assertFalse(is_survey_required_and_unanswered(self.student, self.course)) + self.assertTrue(check_survey_required_and_unanswered(self.student, self.course)) def test_staff_must_answer_survey(self): """ Assert that someone with staff level permissions does not have to answer the survey """ - self.assertFalse(is_survey_required_and_unanswered(self.staff, self.course)) + self.assertTrue(check_survey_required_and_unanswered(self.staff, self.course)) diff --git a/lms/djangoapps/survey/utils.py b/lms/djangoapps/survey/utils.py index 462ffcbfbe..7435938001 100644 --- a/lms/djangoapps/survey/utils.py +++ b/lms/djangoapps/survey/utils.py @@ -2,11 +2,25 @@ Utilities for determining whether or not a survey needs to be completed. """ +from django.utils.translation import ugettext as _ from lms.djangoapps.courseware.access import has_access +from lms.djangoapps.courseware.access_response import AccessError +from lms.djangoapps.courseware.access_utils import ACCESS_GRANTED from survey.models import SurveyAnswer, SurveyForm +class SurveyRequiredAccessError(AccessError): + """ + Access denied because the user has not completed a required survey + """ + def __init__(self): + error_code = "survey_required" + developer_message = u"User must complete a survey" + user_message = _(u"You must complete a survey") + super(SurveyRequiredAccessError, self).__init__(error_code, developer_message, user_message) + + def is_survey_required_for_course(course_descriptor): """ Returns whether a Survey is required for this course @@ -14,31 +28,36 @@ def is_survey_required_for_course(course_descriptor): # Check to see that the survey is required in the CourseDescriptor. if not getattr(course_descriptor, 'course_survey_required', False): - return False + return SurveyRequiredAccessError() # Check that the specified Survey for the course exists. return SurveyForm.get(course_descriptor.course_survey_name, throw_if_not_found=False) -def is_survey_required_and_unanswered(user, course_descriptor): +def check_survey_required_and_unanswered(user, course_descriptor): """ - Returns whether a user is required to answer the survey and has yet to do so. + Checks whether a user is required to answer the survey and has yet to do so. + + Returns: + AccessResponse: Either ACCESS_GRANTED or SurveyRequiredAccessError. """ if not is_survey_required_for_course(course_descriptor): - return False + return ACCESS_GRANTED # anonymous users do not need to answer the survey if user.is_anonymous: - return False + return ACCESS_GRANTED # course staff do not need to answer survey has_staff_access = has_access(user, 'staff', course_descriptor) if has_staff_access: - return False + return ACCESS_GRANTED # survey is required and it exists, let's see if user has answered the survey survey = SurveyForm.get(course_descriptor.course_survey_name) answered_survey = SurveyAnswer.do_survey_answers_exist(survey, user) - if not answered_survey: - return True + if answered_survey: + return ACCESS_GRANTED + + return SurveyRequiredAccessError() diff --git a/openedx/core/djangoapps/courseware_api/serializers.py b/openedx/core/djangoapps/courseware_api/serializers.py index 0cf720d556..5784023b6f 100644 --- a/openedx/core/djangoapps/courseware_api/serializers.py +++ b/openedx/core/djangoapps/courseware_api/serializers.py @@ -88,13 +88,9 @@ class CourseInfoSerializer(serializers.Serializer): # pylint: disable=abstract- verified_mode = serializers.SerializerMethodField() show_calculator = serializers.BooleanField() is_staff = serializers.BooleanField() - can_load_courseware = serializers.BooleanField() + can_load_courseware = serializers.DictField() notes = serializers.SerializerMethodField() - # TODO: TNL-7053 Legacy: Delete these two once ready to contract - user_has_access = serializers.BooleanField() - user_has_staff_access = serializers.BooleanField() - def __init__(self, *args, **kwargs): """ Initialize the serializer. diff --git a/openedx/core/djangoapps/courseware_api/tests/test_views.py b/openedx/core/djangoapps/courseware_api/tests/test_views.py index b8f2645f01..45d7afac9f 100644 --- a/openedx/core/djangoapps/courseware_api/tests/test_views.py +++ b/openedx/core/djangoapps/courseware_api/tests/test_views.py @@ -8,6 +8,10 @@ import mock from django.conf import settings +from lms.djangoapps.courseware.access_utils import ( + ACCESS_DENIED, + ACCESS_GRANTED +) from xmodule.modulestore.django import modulestore from xmodule.modulestore.tests.django_utils import ( @@ -60,17 +64,17 @@ class CourseApiTestViews(BaseCoursewareTests): Tests for the courseware REST API """ @ddt.data( - (True, None, False), - (True, 'audit', False), - (True, 'verified', False), - (False, None, False), - (False, None, True), + (True, None, ACCESS_DENIED), + (True, 'audit', ACCESS_DENIED), + (True, 'verified', ACCESS_DENIED), + (False, None, ACCESS_DENIED), + (False, None, ACCESS_GRANTED), ) @ddt.unpack def test_course_metadata(self, logged_in, enrollment_mode, enable_anonymous): - allow_public_access = mock.Mock() - allow_public_access.return_value = enable_anonymous - with mock.patch('openedx.core.djangoapps.courseware_api.views.allow_public_access', allow_public_access): + check_public_access = mock.Mock() + check_public_access.return_value = enable_anonymous + with mock.patch('lms.djangoapps.courseware.access_utils.check_public_access', check_public_access): if not logged_in: self.client.logout() if enrollment_mode: @@ -83,11 +87,12 @@ class CourseApiTestViews(BaseCoursewareTests): assert enrollment['is_active'] assert len(response.data['tabs']) == 4 elif enable_anonymous and not logged_in: - allow_public_access.assert_called_once() + # multiple checks use this handler + check_public_access.assert_called() assert response.data['enrollment']['mode'] is None - assert response.data['user_has_access'] + assert response.data['can_load_courseware']['has_access'] else: - assert not response.data['user_has_access'] + assert not response.data['can_load_courseware']['has_access'] class SequenceApiTestViews(BaseCoursewareTests): diff --git a/openedx/core/djangoapps/courseware_api/views.py b/openedx/core/djangoapps/courseware_api/views.py index d30e2d3846..018650f27c 100644 --- a/openedx/core/djangoapps/courseware_api/views.py +++ b/openedx/core/djangoapps/courseware_api/views.py @@ -11,7 +11,7 @@ from rest_framework.views import APIView from lms.djangoapps.course_api.api import course_detail from lms.djangoapps.courseware.access import has_access -from lms.djangoapps.courseware.courses import allow_public_access +from lms.djangoapps.courseware.courses import check_course_access from lms.djangoapps.courseware.module_render import get_module_by_usage_id from student.models import CourseEnrollment @@ -61,7 +61,8 @@ class CoursewareInformation(RetrieveAPIView): * enrollment: Enrollment status of authenticated user * mode: `audit`, `verified`, etc * is_active: boolean - * user_has_access: Whether the user can view the course + * can_load_course: Whether the user can view the course (AccessResponse object) + * is_staff: Whether the user has staff access to the course **Parameters:** @@ -80,26 +81,6 @@ class CoursewareInformation(RetrieveAPIView): serializer_class = CourseInfoSerializer - def _check_access(self, user, overview, is_staff): - if is_staff: - 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 - - # Anonymous or unenrolled users - if user.is_anonymous or not CourseEnrollment.is_enrolled(user, overview.id): - # do not have access if the course is not public - if not allow_public_access(overview, [COURSE_VISIBILITY_PUBLIC]): - 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 @@ -123,11 +104,14 @@ class CoursewareInformation(RetrieveAPIView): overview.enrollment = {'mode': mode, 'is_active': is_active} overview.is_staff = has_access(self.request.user, 'staff', overview).has_access - overview.can_load_courseware = self._check_access(self.request.user, overview, overview.is_staff) - - # TODO: TNL-7053 Legacy: Delete these two once ready to contract - overview.user_has_access = overview.can_load_courseware - overview.user_has_staff_access = overview.is_staff + overview.can_load_courseware = check_course_access( + overview, + self.request.user, + 'load', + check_if_enrolled=True, + check_survey_complete=False, + check_if_authenticated=True, + ).to_json() return overview diff --git a/openedx/features/course_experience/views/course_home_messages.py b/openedx/features/course_experience/views/course_home_messages.py index f78c520c86..e60ab6e7d1 100644 --- a/openedx/features/course_experience/views/course_home_messages.py +++ b/openedx/features/course_experience/views/course_home_messages.py @@ -25,7 +25,7 @@ from lms.djangoapps.course_goals.api import ( valid_course_goals_ordered ) from lms.djangoapps.course_goals.models import GOAL_KEY_CHOICES -from lms.djangoapps.courseware.courses import allow_public_access +from lms.djangoapps.courseware.access_utils import check_public_access from openedx.core.djangoapps.plugin_api.views import EdxFragmentView from openedx.core.djangolib.markup import HTML, Text from openedx.features.course_experience import CourseHomeMessages @@ -111,7 +111,7 @@ def _register_course_home_messages(request, course, user_access, course_start_da """ Register messages to be shown in the course home content page. """ - allow_anonymous = allow_public_access(course, [COURSE_VISIBILITY_PUBLIC]) + allow_anonymous = check_public_access(course, [COURSE_VISIBILITY_PUBLIC]) if user_access['is_anonymous'] and not allow_anonymous: sign_in_or_register_text = (_(u'{sign_in_link} or {register_link} and then enroll in this course.')