diff --git a/common/djangoapps/student/tests/test_authz.py b/common/djangoapps/student/tests/test_authz.py index 655e240aab..1ac7aeb3ec 100644 --- a/common/djangoapps/student/tests/test_authz.py +++ b/common/djangoapps/student/tests/test_authz.py @@ -9,7 +9,7 @@ from django.core.exceptions import PermissionDenied from student.roles import CourseInstructorRole, CourseStaffRole, CourseCreatorRole from student.tests.factories import AdminFactory -from student.auth import has_access, add_users, remove_users +from student.auth import has_role, add_users, remove_users from opaque_keys.edx.locations import SlashSeparatedCourseKey @@ -30,30 +30,30 @@ class CreatorGroupTest(TestCase): Tests that CourseCreatorRole().has_user always returns True if ENABLE_CREATOR_GROUP and DISABLE_COURSE_CREATION are both not turned on. """ - self.assertTrue(has_access(self.user, CourseCreatorRole())) + self.assertTrue(has_role(self.user, CourseCreatorRole())) def test_creator_group_enabled_but_empty(self): """ Tests creator group feature on, but group empty. """ with mock.patch.dict('django.conf.settings.FEATURES', {"ENABLE_CREATOR_GROUP": True}): - self.assertFalse(has_access(self.user, CourseCreatorRole())) + self.assertFalse(has_role(self.user, CourseCreatorRole())) # Make user staff. This will cause CourseCreatorRole().has_user to return True. self.user.is_staff = True - self.assertTrue(has_access(self.user, CourseCreatorRole())) + self.assertTrue(has_role(self.user, CourseCreatorRole())) def test_creator_group_enabled_nonempty(self): """ Tests creator group feature on, user added. """ with mock.patch.dict('django.conf.settings.FEATURES', {"ENABLE_CREATOR_GROUP": True}): add_users(self.admin, CourseCreatorRole(), self.user) - self.assertTrue(has_access(self.user, CourseCreatorRole())) + self.assertTrue(has_role(self.user, CourseCreatorRole())) # check that a user who has not been added to the group still returns false user_not_added = User.objects.create_user('testuser2', 'test+courses2@edx.org', 'foo2') - self.assertFalse(has_access(user_not_added, CourseCreatorRole())) + self.assertFalse(has_role(user_not_added, CourseCreatorRole())) # remove first user from the group and verify that CourseCreatorRole().has_user now returns false remove_users(self.admin, CourseCreatorRole(), self.user) - self.assertFalse(has_access(self.user, CourseCreatorRole())) + self.assertFalse(has_role(self.user, CourseCreatorRole())) def test_course_creation_disabled(self): """ Tests that the COURSE_CREATION_DISABLED flag overrides course creator group settings. """ @@ -63,15 +63,15 @@ class CreatorGroupTest(TestCase): add_users(self.admin, CourseCreatorRole(), self.user) # DISABLE_COURSE_CREATION overrides (user is not marked as staff). - self.assertFalse(has_access(self.user, CourseCreatorRole())) + self.assertFalse(has_role(self.user, CourseCreatorRole())) # Mark as staff. Now CourseCreatorRole().has_user returns true. self.user.is_staff = True - self.assertTrue(has_access(self.user, CourseCreatorRole())) + self.assertTrue(has_role(self.user, CourseCreatorRole())) # Remove user from creator group. CourseCreatorRole().has_user still returns true because is_staff=True remove_users(self.admin, CourseCreatorRole(), self.user) - self.assertTrue(has_access(self.user, CourseCreatorRole())) + self.assertTrue(has_role(self.user, CourseCreatorRole())) def test_add_user_not_authenticated(self): """ @@ -84,7 +84,7 @@ class CreatorGroupTest(TestCase): anonymous_user = AnonymousUser() role = CourseCreatorRole() add_users(self.admin, role, anonymous_user) - self.assertFalse(has_access(anonymous_user, role)) + self.assertFalse(has_role(anonymous_user, role)) def test_add_user_not_active(self): """ @@ -96,7 +96,7 @@ class CreatorGroupTest(TestCase): ): self.user.is_active = False add_users(self.admin, CourseCreatorRole(), self.user) - self.assertFalse(has_access(self.user, CourseCreatorRole())) + self.assertFalse(has_role(self.user, CourseCreatorRole())) def test_add_user_to_group_requires_staff_access(self): with self.assertRaises(PermissionDenied): @@ -150,15 +150,15 @@ class CourseGroupTest(TestCase): Tests adding user to course group (happy path). """ # Create groups for a new course (and assign instructor role to the creator). - self.assertFalse(has_access(self.creator, CourseInstructorRole(self.course_key))) + self.assertFalse(has_role(self.creator, CourseInstructorRole(self.course_key))) add_users(self.global_admin, CourseInstructorRole(self.course_key), self.creator) add_users(self.global_admin, CourseStaffRole(self.course_key), self.creator) - self.assertTrue(has_access(self.creator, CourseInstructorRole(self.course_key))) + self.assertTrue(has_role(self.creator, CourseInstructorRole(self.course_key))) # Add another user to the staff role. - self.assertFalse(has_access(self.staff, CourseStaffRole(self.course_key))) + self.assertFalse(has_role(self.staff, CourseStaffRole(self.course_key))) add_users(self.creator, CourseStaffRole(self.course_key), self.staff) - self.assertTrue(has_access(self.staff, CourseStaffRole(self.course_key))) + self.assertTrue(has_role(self.staff, CourseStaffRole(self.course_key))) def test_add_user_to_course_group_permission_denied(self): """ @@ -177,13 +177,13 @@ class CourseGroupTest(TestCase): add_users(self.global_admin, CourseStaffRole(self.course_key), self.creator) add_users(self.creator, CourseStaffRole(self.course_key), self.staff) - self.assertTrue(has_access(self.staff, CourseStaffRole(self.course_key))) + self.assertTrue(has_role(self.staff, CourseStaffRole(self.course_key))) remove_users(self.creator, CourseStaffRole(self.course_key), self.staff) - self.assertFalse(has_access(self.staff, CourseStaffRole(self.course_key))) + self.assertFalse(has_role(self.staff, CourseStaffRole(self.course_key))) remove_users(self.creator, CourseInstructorRole(self.course_key), self.creator) - self.assertFalse(has_access(self.creator, CourseInstructorRole(self.course_key))) + self.assertFalse(has_role(self.creator, CourseInstructorRole(self.course_key))) def test_remove_user_from_course_group_permission_denied(self): """ diff --git a/lms/djangoapps/class_dashboard/tests/test_dashboard_data.py b/lms/djangoapps/class_dashboard/tests/test_dashboard_data.py index 782a75a7bf..a04de986d6 100644 --- a/lms/djangoapps/class_dashboard/tests/test_dashboard_data.py +++ b/lms/djangoapps/class_dashboard/tests/test_dashboard_data.py @@ -316,5 +316,5 @@ class TestGetProblemGradeDistribution(ModuleStoreTestCase): """ Test for instructor access """ - ret_val = has_instructor_access_for_class(self.instructor, self.course.id) + ret_val = bool(has_instructor_access_for_class(self.instructor, self.course.id)) self.assertEquals(ret_val, True) diff --git a/lms/djangoapps/class_dashboard/views.py b/lms/djangoapps/class_dashboard/views.py index 37cc413904..afb89c1e10 100644 --- a/lms/djangoapps/class_dashboard/views.py +++ b/lms/djangoapps/class_dashboard/views.py @@ -22,7 +22,7 @@ def has_instructor_access_for_class(user, course_id): """ course = get_course_with_access(user, 'staff', course_id, depth=None) - return has_access(user, 'staff', course) + return bool(has_access(user, 'staff', course)) def all_sequential_open_distrib(request, course_id): diff --git a/lms/djangoapps/course_structure_api/v0/views.py b/lms/djangoapps/course_structure_api/v0/views.py index ce2e1ec007..7fdf8c3cf4 100644 --- a/lms/djangoapps/course_structure_api/v0/views.py +++ b/lms/djangoapps/course_structure_api/v0/views.py @@ -83,9 +83,11 @@ class CourseViewMixin(object): Determines if the user is staff or an instructor for the course. Always returns True if DEBUG mode is enabled. """ - return (settings.DEBUG - or has_access(user, CourseStaffRole.ROLE, course) - or has_access(user, CourseInstructorRole.ROLE, course)) + return bool( + settings.DEBUG + or has_access(user, CourseStaffRole.ROLE, course) + or has_access(user, CourseInstructorRole.ROLE, course) + ) def check_course_permissions(self, user, course): """ diff --git a/lms/djangoapps/courseware/access.py b/lms/djangoapps/courseware/access.py index 5713e4cbac..3a8eb7eb66 100644 --- a/lms/djangoapps/courseware/access.py +++ b/lms/djangoapps/courseware/access.py @@ -23,8 +23,10 @@ from opaque_keys.edx.keys import CourseKey, UsageKey from xblock.core import XBlock from xmodule.course_module import ( - CourseDescriptor, CATALOG_VISIBILITY_CATALOG_AND_ABOUT, - CATALOG_VISIBILITY_ABOUT) + CourseDescriptor, + CATALOG_VISIBILITY_CATALOG_AND_ABOUT, + CATALOG_VISIBILITY_ABOUT, +) from xmodule.error_module import ErrorDescriptor from xmodule.x_module import XModule, DEPRECATION_VSCOMPAT_EVENT from xmodule.split_test_module import get_split_user_partitions @@ -37,8 +39,12 @@ from openedx.core.djangoapps.content.course_overviews.models import CourseOvervi from student import auth from student.models import CourseEnrollmentAllowed from student.roles import ( - GlobalStaff, CourseStaffRole, CourseInstructorRole, - OrgStaffRole, OrgInstructorRole, CourseBetaTesterRole + CourseBetaTesterRole, + CourseInstructorRole, + CourseStaffRole, + GlobalStaff, + OrgInstructorRole, + OrgStaffRole, ) from util.milestones_helpers import ( get_pre_requisite_courses_not_completed, @@ -48,7 +54,17 @@ from ccx_keys.locator import CCXLocator import dogstats_wrapper as dog_stats_api +from courseware.access_response import ( + AccessResponse, + MilestoneError, + MobileAvailabilityError, + StartDateError, + VisibilityError, +) + DEBUG_ACCESS = False +ACCESS_GRANTED = AccessResponse(True) +ACCESS_DENIED = AccessResponse(False) log = logging.getLogger(__name__) @@ -86,8 +102,8 @@ def has_access(user, action, obj, course_key=None): Required when accessing anything other than a CourseDescriptor, 'global', or a location with category 'course' - Returns a bool. It is up to the caller to actually deny access in a way - that makes sense in context. + Returns an AccessResponse object. It is up to the caller to actually + deny access in a way that makes sense in context. """ # Just in case user is passed in as None, make them anonymous if not user: @@ -146,13 +162,18 @@ def _can_access_descriptor_with_start_date(user, descriptor, course_key): # pyl Arguments: user (User): the user whose descriptor access we are checking. - descriptor (AType): the descriptor for which we are checking access. - where AType is any descriptor that has the attributes .location and - .days_early_for_beta + descriptor (AType): the descriptor for which we are checking access, + where AType is CourseDescriptor, CourseOverview, or any other class + that represents a descriptor and has the attributes .location, .id, + .start, and .days_early_for_beta. + + Returns: + AccessResponse: The result of this access check. Possible results are + ACCESS_GRANTED or a StartDateError. """ start_dates_disabled = settings.FEATURES['DISABLE_START_DATES'] if start_dates_disabled and not is_masquerading_as_student(user, course_key): - return True + return ACCESS_GRANTED else: now = datetime.now(UTC()) effective_start = _adjust_start_date_for_beta_testers( @@ -160,11 +181,14 @@ def _can_access_descriptor_with_start_date(user, descriptor, course_key): # pyl descriptor, course_key=course_key ) - return ( + if ( descriptor.start is None or now > effective_start or in_preview_mode() - ) + ): + return ACCESS_GRANTED + + return StartDateError(descriptor.start) def _can_view_courseware_with_prerequisites(user, course): # pylint: disable=invalid-name @@ -177,15 +201,22 @@ def _can_view_courseware_with_prerequisites(user, course): # pylint: disable=in Arguments: user (User): the user whose course access we are checking. course (AType): the course for which we are checking access. - where AType is CourseDescriptor, CourseOverview, or any other class that - represents a course and has the attributes .location and .id. + where AType is CourseDescriptor, CourseOverview, or any other + class that represents a course and has the attributes .location + and .id. """ + + def _is_prerequisites_disabled(): + """ + Checks if prerequisites are disabled in the settings. + """ + return ACCESS_DENIED if settings.FEATURES['ENABLE_PREREQUISITE_COURSES'] else ACCESS_GRANTED + return ( - not settings.FEATURES['ENABLE_PREREQUISITE_COURSES'] + _is_prerequisites_disabled() or _has_staff_access_to_descriptor(user, course, course.id) - or not course.pre_requisite_courses or user.is_anonymous() - or not get_pre_requisite_courses_not_completed(user, [course.id]) + or _has_fulfilled_prerequisites(user, [course.id]) ) @@ -209,7 +240,7 @@ def _can_load_course_on_mobile(user, course): is_mobile_available_for_user(user, course) and ( _has_staff_access_to_descriptor(user, course, course.id) or - not any_unfulfilled_milestones(course.id, user.id) + _has_fulfilled_all_milestones(user, course.id) ) ) @@ -275,19 +306,19 @@ def _has_access_course_desc(user, action, course): # (sorry that it's confusing :( ) if user is not None and user.is_authenticated() and CourseEnrollmentAllowed: if CourseEnrollmentAllowed.objects.filter(email=user.email, course_id=course.id): - return True + return ACCESS_GRANTED if _has_staff_access_to_descriptor(user, course, course.id): - return True + return ACCESS_GRANTED # Invitation_only doesn't apply to CourseEnrollmentAllowed or has_staff_access_access if course.invitation_only: debug("Deny: invitation only") - return False + return ACCESS_DENIED if reg_method_ok and start < now < end: debug("Allow: in enrollment period") - return True + return ACCESS_GRANTED def see_exists(): """ @@ -314,10 +345,10 @@ def _has_access_course_desc(user, action, course): # seen by non-staff if course.ispublic: debug("Allow: ACCESS_REQUIRE_STAFF_FOR_COURSE and ispublic") - return True + return ACCESS_GRANTED return _has_staff_access_to_descriptor(user, course, course.id) - return can_enroll() or can_load() + return ACCESS_GRANTED if (can_enroll() or can_load()) else ACCESS_DENIED def can_see_in_catalog(): """ @@ -326,8 +357,8 @@ def _has_access_course_desc(user, action, course): but also allow course staff to see this. """ return ( - course.catalog_visibility == CATALOG_VISIBILITY_CATALOG_AND_ABOUT or - _has_staff_access_to_descriptor(user, course, course.id) + _has_catalog_visibility(course, CATALOG_VISIBILITY_CATALOG_AND_ABOUT) + or _has_staff_access_to_descriptor(user, course, course.id) ) def can_see_about_page(): @@ -337,9 +368,9 @@ def _has_access_course_desc(user, action, course): but also allow course staff to see this. """ return ( - course.catalog_visibility == CATALOG_VISIBILITY_CATALOG_AND_ABOUT or - course.catalog_visibility == CATALOG_VISIBILITY_ABOUT or - _has_staff_access_to_descriptor(user, course, course.id) + _has_catalog_visibility(course, CATALOG_VISIBILITY_CATALOG_AND_ABOUT) + or _has_catalog_visibility(course, CATALOG_VISIBILITY_ABOUT) + or _has_staff_access_to_descriptor(user, course, course.id) ) checkers = { @@ -370,11 +401,15 @@ def _can_load_course_overview(user, course_overview): The user doesn't have to be enrolled in the course in order to have load load access. """ - return ( - not course_overview.visible_to_staff_only + response = ( + _visible_to_nonstaff_users(course_overview) and _can_access_descriptor_with_start_date(user, course_overview, course_overview.id) - ) or _has_staff_access_to_descriptor(user, course_overview, course_overview.id) + ) + return ( + ACCESS_GRANTED if (response or _has_staff_access_to_descriptor(user, course_overview, course_overview.id)) + else response + ) _COURSE_OVERVIEW_CHECKERS = { 'load': _can_load_course_overview, @@ -432,7 +467,7 @@ def _has_group_access(descriptor, user, course_key): # Short-circuit the process, since there are no defined user partitions that are not # user_partitions used by the split_test module. The split_test module handles its own access # via updating the children of the split_test module. - return True + return ACCESS_GRANTED # use merged_group_access which takes group access on the block's # parents / ancestors into account @@ -441,20 +476,20 @@ def _has_group_access(descriptor, user, course_key): # partition's group list excludes all students. if False in merged_access.values(): log.warning("Group access check excludes all students, access will be denied.", exc_info=True) - return False + return ACCESS_DENIED # resolve the partition IDs in group_access to actual # partition objects, skipping those which contain empty group directives. # if a referenced partition could not be found, access will be denied. try: partitions = [ - descriptor._get_user_partition(partition_id) # pylint:disable=protected-access + descriptor._get_user_partition(partition_id) # pylint: disable=protected-access for partition_id, group_ids in merged_access.items() if group_ids is not None ] except NoSuchUserPartitionError: log.warning("Error looking up user partition, access will be denied.", exc_info=True) - return False + return ACCESS_DENIED # next resolve the group IDs specified within each partition partition_groups = [] @@ -468,7 +503,7 @@ def _has_group_access(descriptor, user, course_key): partition_groups.append((partition, groups)) except NoSuchUserPartitionGroupError: log.warning("Error looking up referenced user partition group, access will be denied.", exc_info=True) - return False + return ACCESS_DENIED # look up the user's group for each partition user_groups = {} @@ -482,10 +517,10 @@ def _has_group_access(descriptor, user, course_key): # finally: check that the user has a satisfactory group assignment # for each partition. if not all(user_groups.get(partition.id) in groups for partition, groups in partition_groups): - return False + return ACCESS_DENIED # all checks passed. - return True + return ACCESS_GRANTED def _has_access_descriptor(user, action, descriptor, course_key=None): @@ -507,14 +542,20 @@ def _has_access_descriptor(user, action, descriptor, course_key=None): students to see modules. If not, views should check the course, so we don't have to hit the enrollments table on every module load. """ - return ( - not descriptor.visible_to_staff_only + response = ( + _visible_to_nonstaff_users(descriptor) and _has_group_access(descriptor, user, course_key) - and ( - 'detached' in descriptor._class_tags # pylint: disable=protected-access + and + ( + _has_detached_class_tag(descriptor) or _can_access_descriptor_with_start_date(user, descriptor, course_key) ) - ) or _has_staff_access_to_descriptor(user, descriptor, course_key) + ) + + return ( + ACCESS_GRANTED if (response or _has_staff_access_to_descriptor(user, descriptor, course_key)) + else response + ) checkers = { 'load': can_load, @@ -592,10 +633,13 @@ def _has_access_string(user, action, perm): """ def check_staff(): + """ + Checks for staff access + """ if perm != 'global': debug("Deny: invalid permission '%s'", perm) - return False - return GlobalStaff().has_user(user) + return ACCESS_DENIED + return ACCESS_GRANTED if GlobalStaff().has_user(user) else ACCESS_DENIED checkers = { 'staff': check_staff @@ -674,38 +718,37 @@ def _has_staff_access_to_location(user, location, course_key=None): def _has_access_to_course(user, access_level, course_key): - ''' + """ Returns True if the given user has access_level (= staff or instructor) access to the course with the given course_key. This ensures the user is authenticated and checks if global staff or has staff / instructor access. access_level = string, either "staff" or "instructor" - ''' + """ if user is None or (not user.is_authenticated()): debug("Deny: no user or anon user") - return False + return ACCESS_DENIED if is_masquerading_as_student(user, course_key): - return False + return ACCESS_DENIED if GlobalStaff().has_user(user): debug("Allow: user.is_staff") - return True + return ACCESS_GRANTED if access_level not in ('staff', 'instructor'): log.debug("Error in access._has_access_to_course access_level=%s unknown", access_level) debug("Deny: unknown access level") - return False + return ACCESS_DENIED staff_access = ( CourseStaffRole(course_key).has_user(user) or OrgStaffRole(course_key.org).has_user(user) ) - if staff_access and access_level == 'staff': debug("Allow: user has course staff access") - return True + return ACCESS_GRANTED instructor_access = ( CourseInstructorRole(course_key).has_user(user) or @@ -714,10 +757,10 @@ def _has_access_to_course(user, access_level, course_key): if instructor_access and access_level in ('staff', 'instructor'): debug("Allow: user has course instructor access") - return True + return ACCESS_GRANTED debug("Deny: user did not have correct access") - return False + return ACCESS_DENIED def _has_instructor_access_to_descriptor(user, descriptor, course_key): # pylint: disable=invalid-name @@ -738,6 +781,64 @@ def _has_staff_access_to_descriptor(user, descriptor, course_key): return _has_staff_access_to_location(user, descriptor.location, course_key) +def _visible_to_nonstaff_users(descriptor): + """ + Returns if the object is visible to nonstaff users. + + Arguments: + descriptor: object to check + """ + return VisibilityError() if descriptor.visible_to_staff_only else ACCESS_GRANTED + + +def _has_detached_class_tag(descriptor): + """ + Returns if the given descriptor's type is marked as detached. + + Arguments: + descriptor: object to check + """ + return ACCESS_GRANTED if 'detached' in descriptor._class_tags else ACCESS_DENIED # pylint: disable=protected-access + + +def _has_fulfilled_all_milestones(user, course_id): + """ + Returns whether the given user has fulfilled all milestones for the + given course. + + Arguments: + course_id: ID of the course to check + user_id: ID of the user to check + """ + return MilestoneError() if any_unfulfilled_milestones(course_id, user.id) else ACCESS_GRANTED + + +def _has_fulfilled_prerequisites(user, course_id): + """ + Returns whether the given user has fulfilled all prerequisites for the + given course. + + Arguments: + user: user to check + course_id: ID of the course to check + """ + return MilestoneError() if get_pre_requisite_courses_not_completed(user, course_id) else ACCESS_GRANTED + + +def _has_catalog_visibility(course, visibility_type): + """ + Returns whether the given course has the given visibility type + """ + return ACCESS_GRANTED if course.catalog_visibility == visibility_type else ACCESS_DENIED + + +def _is_descriptor_mobile_available(descriptor): + """ + Returns if descriptor is available on mobile. + """ + return ACCESS_GRANTED if descriptor.mobile_available else MobileAvailabilityError() + + def is_mobile_available_for_user(user, descriptor): """ Returns whether the given course is mobile_available for the given user. @@ -747,10 +848,11 @@ def is_mobile_available_for_user(user, descriptor): Arguments: descriptor (CourseDescriptor|CourseOverview): course or overview of course in question """ + return ( - descriptor.mobile_available or - auth.has_access(user, CourseBetaTesterRole(descriptor.id)) or - _has_staff_access_to_descriptor(user, descriptor, descriptor.id) + auth.has_access(user, CourseBetaTesterRole(descriptor.id)) + or _has_staff_access_to_descriptor(user, descriptor, descriptor.id) + or _is_descriptor_mobile_available(descriptor) ) diff --git a/lms/djangoapps/courseware/access_response.py b/lms/djangoapps/courseware/access_response.py new file mode 100644 index 0000000000..00c120d833 --- /dev/null +++ b/lms/djangoapps/courseware/access_response.py @@ -0,0 +1,126 @@ +""" +This file contains all the classes used by has_access for error handling +""" + +from django.utils.translation import ugettext as _ + + +class AccessResponse(object): + """Class that represents a response from a has_access permission check.""" + def __init__(self, has_access, error_code=None, developer_message=None, user_message=None): + """ + Creates an AccessResponse object. + + Arguments: + has_access (bool): if the user is granted access or not + error_code (String): optional - default is None. Unique identifier + for the specific type of error + developer_message (String): optional - default is None. Message + to show the developer + user_message (String): optional - default is None. Message to + show the user + """ + self.has_access = has_access + self.error_code = error_code + self.developer_message = developer_message + self.user_message = user_message + if has_access: + assert error_code is None + + def __nonzero__(self): + """ + Overrides bool(). + + Allows for truth value testing of AccessResponse objects, so callers + who do not need the specific error information can check if access + is granted. + + Returns: + bool: whether or not access is granted + + """ + return self.has_access + + def to_json(self): + """ + Creates a serializable JSON representation of an AccessResponse object. + + Returns: + dict: JSON representation + """ + return { + "has_access": self.has_access, + "error_code": self.error_code, + "developer_message": self.developer_message, + "user_message": self.user_message + } + + +class AccessError(AccessResponse): + """ + Class that holds information about the error in the case of an access + denial in has_access. Contains the error code, user and developer + messages. Subclasses represent specific errors. + """ + def __init__(self, error_code, developer_message, user_message): + """ + Creates an AccessError object. + + An AccessError object represents an AccessResponse where access is + denied (has_access is False). + + Arguments: + error_code (String): unique identifier for the specific type of + error developer_message (String): message to show the developer + user_message (String): message to show the user + + """ + super(AccessError, self).__init__(False, error_code, developer_message, user_message) + + +class StartDateError(AccessError): + """ + Access denied because the course has not started yet and the user + is not staff + """ + def __init__(self, start_date): + error_code = "course_not_started" + developer_message = "Course does not start until {}".format(start_date) + user_message = _("Course does not start until {}" # pylint: disable=translation-of-non-string + .format("{:%B %d, %Y}".format(start_date))) + + super(StartDateError, self).__init__(error_code, developer_message, user_message) + + +class MilestoneError(AccessError): + """ + Access denied because the user has unfulfilled milestones + """ + def __init__(self): + error_code = "unfulfilled_milestones" + developer_message = "User has unfulfilled milestones" + user_message = _("You have unfulfilled milestones") + super(MilestoneError, self).__init__(error_code, developer_message, user_message) + + +class VisibilityError(AccessError): + """ + Access denied because the user does have the correct role to view this + course. + """ + def __init__(self): + error_code = "not_visible_to_user" + developer_message = "Course is not visible to this user" + user_message = _("You do not have access to this course") + super(VisibilityError, self).__init__(error_code, developer_message, user_message) + + +class MobileAvailabilityError(AccessError): + """ + Access denied because the course is not available on mobile for the user + """ + def __init__(self): + error_code = "mobile_unavailable" + developer_message = "Course is not available on mobile for this user" + user_message = _("You do not have access to this course on a mobile device") + super(MobileAvailabilityError, self).__init__(error_code, developer_message, user_message) diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index 9418aa7605..52f338bde0 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -605,11 +605,11 @@ def get_module_system_for_user(user, student_data, # TODO # pylint: disable=to # the result would always be "False". masquerade_settings = user.real_user.masquerade_settings del user.real_user.masquerade_settings - instructor_access = has_access(user.real_user, 'instructor', descriptor, course_id) + instructor_access = bool(has_access(user.real_user, 'instructor', descriptor, course_id)) user.real_user.masquerade_settings = masquerade_settings else: staff_access = has_access(user, 'staff', descriptor, course_id) - instructor_access = has_access(user, 'instructor', descriptor, course_id) + instructor_access = bool(has_access(user, 'instructor', descriptor, course_id)) if staff_access: block_wrappers.append(partial(add_staff_markup, user, instructor_access, disable_staff_debug_info)) @@ -629,7 +629,7 @@ def get_module_system_for_user(user, student_data, # TODO # pylint: disable=to field_data = LmsFieldData(descriptor._field_data, student_data) # pylint: disable=protected-access - user_is_staff = has_access(user, u'staff', descriptor.location, course_id) + user_is_staff = bool(has_access(user, u'staff', descriptor.location, course_id)) system = LmsModuleSystem( track_function=track_function, @@ -703,7 +703,7 @@ def get_module_system_for_user(user, student_data, # TODO # pylint: disable=to ) system.set(u'user_is_staff', user_is_staff) - system.set(u'user_is_admin', has_access(user, u'staff', 'global')) + system.set(u'user_is_admin', bool(has_access(user, u'staff', 'global'))) system.set(u'user_is_beta_tester', CourseBetaTesterRole(course_id).has_user(user)) system.set(u'days_early_for_beta', getattr(descriptor, 'days_early_for_beta')) diff --git a/lms/djangoapps/courseware/tabs.py b/lms/djangoapps/courseware/tabs.py index d23de14ceb..a3f54c614a 100644 --- a/lms/djangoapps/courseware/tabs.py +++ b/lms/djangoapps/courseware/tabs.py @@ -20,7 +20,7 @@ class EnrolledTab(CourseTab): def is_enabled(cls, course, user=None): if user is None: return True - return CourseEnrollment.is_enrolled(user, course.id) or has_access(user, 'staff', course, course.id) + return bool(CourseEnrollment.is_enrolled(user, course.id) or has_access(user, 'staff', course, course.id)) class CoursewareTab(EnrolledTab): diff --git a/lms/djangoapps/courseware/tests/test_access.py b/lms/djangoapps/courseware/tests/test_access.py index 8c2715c9e5..8d32023d5c 100644 --- a/lms/djangoapps/courseware/tests/test_access.py +++ b/lms/djangoapps/courseware/tests/test_access.py @@ -1,3 +1,7 @@ +# -*- coding: utf-8 -*- +""" +Test the access control framework +""" import datetime import ddt import itertools @@ -10,18 +14,29 @@ from nose.plugins.attrib import attr from opaque_keys.edx.locations import SlashSeparatedCourseKey import courseware.access as access +import courseware.access_response as access_response from courseware.masquerade import CourseMasquerade -from courseware.tests.factories import UserFactory, StaffFactory, InstructorFactory, BetaTesterFactory +from courseware.tests.factories import ( + BetaTesterFactory, + GlobalStaffFactory, + InstructorFactory, + StaffFactory, + UserFactory, +) from courseware.tests.helpers import LoginEnrollmentTestCase from openedx.core.djangoapps.content.course_overviews.models import CourseOverview -from student.tests.factories import AnonymousUserFactory, CourseEnrollmentAllowedFactory, CourseEnrollmentFactory +from student.tests.factories import ( + AnonymousUserFactory, + CourseEnrollmentAllowedFactory, + CourseEnrollmentFactory, +) from xmodule.course_module import ( - CATALOG_VISIBILITY_CATALOG_AND_ABOUT, CATALOG_VISIBILITY_ABOUT, - CATALOG_VISIBILITY_NONE + CATALOG_VISIBILITY_CATALOG_AND_ABOUT, + CATALOG_VISIBILITY_ABOUT, + CATALOG_VISIBILITY_NONE, ) from xmodule.modulestore.tests.factories import CourseFactory from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase -from util.milestones_helpers import fulfill_course_milestone from util.milestones_helpers import ( set_prerequisite_courses, @@ -34,26 +49,36 @@ from util.milestones_helpers import ( @attr('shard_1') +@ddt.ddt class AccessTestCase(LoginEnrollmentTestCase, ModuleStoreTestCase): """ Tests for the various access controls on the student dashboard """ + TOMORROW = datetime.datetime.now(pytz.utc) + datetime.timedelta(days=1) + YESTERDAY = datetime.datetime.now(pytz.utc) - datetime.timedelta(days=1) + def setUp(self): super(AccessTestCase, self).setUp() course_key = SlashSeparatedCourseKey('edX', 'toy', '2012_Fall') self.course = course_key.make_usage_key('course', course_key.run) self.anonymous_user = AnonymousUserFactory() + self.beta_user = BetaTesterFactory(course_key=self.course.course_key) self.student = UserFactory() self.global_staff = UserFactory(is_staff=True) self.course_staff = StaffFactory(course_key=self.course.course_key) self.course_instructor = InstructorFactory(course_key=self.course.course_key) + self.staff = GlobalStaffFactory() - def verify_access(self, mock_unit, student_should_have_access): + def verify_access(self, mock_unit, student_should_have_access, expected_error_type=None): """ Verify the expected result from _has_access_descriptor """ - self.assertEqual( - student_should_have_access, - access._has_access_descriptor(self.anonymous_user, 'load', mock_unit, course_key=self.course.course_key) - ) + response = access._has_access_descriptor(self.anonymous_user, 'load', + mock_unit, course_key=self.course.course_key) + self.assertEqual(student_should_have_access, bool(response)) + + if expected_error_type is not None: + self.assertIsInstance(response, expected_error_type) + self.assertIsNotNone(response.to_json()['error_code']) + self.assertTrue( access._has_access_descriptor(self.course_staff, 'load', mock_unit, course_key=self.course.course_key) ) @@ -102,6 +127,10 @@ class AccessTestCase(LoginEnrollmentTestCase, ModuleStoreTestCase): self.student, 'instructor', self.course.course_key )) + self.assertFalse(access._has_access_to_course( + self.student, 'not_staff_or_instructor', self.course.course_key + )) + def test__has_access_string(self): user = Mock(is_staff=True) self.assertFalse(access._has_access_string(user, 'staff', 'not_global')) @@ -111,20 +140,24 @@ class AccessTestCase(LoginEnrollmentTestCase, ModuleStoreTestCase): self.assertRaises(ValueError, access._has_access_string, user, 'not_staff', 'global') - def test__has_access_error_desc(self): + @ddt.data( + ('load', False, True, True), + ('staff', False, True, True), + ('instructor', False, False, True) + ) + @ddt.unpack + def test__has_access_error_desc(self, action, expected_student, expected_staff, expected_instructor): descriptor = Mock() - self.assertFalse(access._has_access_error_desc(self.student, 'load', descriptor, self.course.course_key)) - self.assertTrue(access._has_access_error_desc(self.course_staff, 'load', descriptor, self.course.course_key)) - self.assertTrue(access._has_access_error_desc(self.course_instructor, 'load', descriptor, self.course.course_key)) - - self.assertFalse(access._has_access_error_desc(self.student, 'staff', descriptor, self.course.course_key)) - self.assertTrue(access._has_access_error_desc(self.course_staff, 'staff', descriptor, self.course.course_key)) - self.assertTrue(access._has_access_error_desc(self.course_instructor, 'staff', descriptor, self.course.course_key)) - - self.assertFalse(access._has_access_error_desc(self.student, 'instructor', descriptor, self.course.course_key)) - self.assertFalse(access._has_access_error_desc(self.course_staff, 'instructor', descriptor, self.course.course_key)) - self.assertTrue(access._has_access_error_desc(self.course_instructor, 'instructor', descriptor, self.course.course_key)) + for (user, expected_response) in ( + (self.student, expected_student), + (self.course_staff, expected_staff), + (self.course_instructor, expected_instructor) + ): + self.assertEquals( + bool(access._has_access_error_desc(user, action, descriptor, self.course.course_key)), + expected_response + ) with self.assertRaises(ValueError): access._has_access_error_desc(self.course_instructor, 'not_load_or_staff', descriptor, self.course.course_key) @@ -133,6 +166,7 @@ class AccessTestCase(LoginEnrollmentTestCase, ModuleStoreTestCase): # TODO: override DISABLE_START_DATES and test the start date branch of the method user = Mock() descriptor = Mock(user_partitions=[]) + descriptor._class_tags = {} # Always returns true because DISABLE_START_DATES is set in test.py self.assertTrue(access._has_access_descriptor(user, 'load', descriptor)) @@ -140,81 +174,68 @@ class AccessTestCase(LoginEnrollmentTestCase, ModuleStoreTestCase): with self.assertRaises(ValueError): access._has_access_descriptor(user, 'not_load_or_staff', descriptor) + @ddt.data( + (True, None, access_response.VisibilityError), + (False, None), + (True, YESTERDAY, access_response.VisibilityError), + (False, YESTERDAY), + (True, TOMORROW, access_response.VisibilityError), + (False, TOMORROW, access_response.StartDateError) + ) + @ddt.unpack @patch.dict('django.conf.settings.FEATURES', {'DISABLE_START_DATES': False}) - def test__has_access_descriptor_staff_lock(self): + def test__has_access_descriptor_staff_lock(self, visible_to_staff_only, start, expected_error_type=None): """ Tests that "visible_to_staff_only" overrides start date. """ + expected_access = expected_error_type is None mock_unit = Mock(user_partitions=[]) mock_unit._class_tags = {} # Needed for detached check in _has_access_descriptor + mock_unit.visible_to_staff_only = visible_to_staff_only + mock_unit.start = start + self.verify_access(mock_unit, expected_access, expected_error_type) - # No start date, staff lock on - mock_unit.visible_to_staff_only = True - self.verify_access(mock_unit, False) - - # No start date, staff lock off. + def test__has_access_descriptor_beta_user(self): + mock_unit = Mock(user_partitions=[]) + mock_unit._class_tags = {} + mock_unit.days_early_for_beta = 2 + mock_unit.start = self.TOMORROW mock_unit.visible_to_staff_only = False - self.verify_access(mock_unit, True) - # Start date in the past, staff lock on. - mock_unit.start = datetime.datetime.now(pytz.utc) - datetime.timedelta(days=1) - mock_unit.visible_to_staff_only = True - self.verify_access(mock_unit, False) - - # Start date in the past, staff lock off. - mock_unit.visible_to_staff_only = False - self.verify_access(mock_unit, True) - - # Start date in the future, staff lock on. - mock_unit.start = datetime.datetime.now(pytz.utc) + datetime.timedelta(days=1) # release date in the future - mock_unit.visible_to_staff_only = True - self.verify_access(mock_unit, False) - - # Start date in the future, staff lock off. - mock_unit.visible_to_staff_only = False - self.verify_access(mock_unit, False) + self.assertTrue(bool(access._has_access_descriptor( + self.beta_user, 'load', mock_unit, course_key=self.course.course_key))) + @ddt.data(None, YESTERDAY, TOMORROW) @patch.dict('django.conf.settings.FEATURES', {'DISABLE_START_DATES': False}) @patch('courseware.access.get_current_request_hostname', Mock(return_value='preview.localhost')) - def test__has_access_descriptor_in_preview_mode(self): + def test__has_access_descriptor_in_preview_mode(self, start): """ Tests that descriptor has access in preview mode. """ mock_unit = Mock(user_partitions=[]) mock_unit._class_tags = {} # Needed for detached check in _has_access_descriptor - - # No start date. mock_unit.visible_to_staff_only = False + mock_unit.start = start self.verify_access(mock_unit, True) - # Start date in the past. - mock_unit.start = datetime.datetime.now(pytz.utc) - datetime.timedelta(days=1) - self.verify_access(mock_unit, True) - - # Start date in the future. - mock_unit.start = datetime.datetime.now(pytz.utc) + datetime.timedelta(days=1) # release date in the future - self.verify_access(mock_unit, True) - + @ddt.data( + (TOMORROW, access_response.StartDateError), + (None, None), + (YESTERDAY, None) + ) # ddt throws an error if I don't put the None argument there + @ddt.unpack @patch.dict('django.conf.settings.FEATURES', {'DISABLE_START_DATES': False}) @patch('courseware.access.get_current_request_hostname', Mock(return_value='localhost')) - def test__has_access_descriptor_when_not_in_preview_mode(self): + def test__has_access_descriptor_when_not_in_preview_mode(self, start, expected_error_type): """ Tests that descriptor has no access when start date in future & without preview. """ + expected_access = expected_error_type is None mock_unit = Mock(user_partitions=[]) mock_unit._class_tags = {} # Needed for detached check in _has_access_descriptor - - # No start date. mock_unit.visible_to_staff_only = False - self.verify_access(mock_unit, True) - - # Start date in the past. - mock_unit.start = datetime.datetime.now(pytz.utc) - datetime.timedelta(days=1) - self.verify_access(mock_unit, True) - - # Start date in the future. - mock_unit.start = datetime.datetime.now(pytz.utc) + datetime.timedelta(days=1) # release date in the future - self.verify_access(mock_unit, False) + mock_unit.start = start + self.verify_access(mock_unit, expected_access, expected_error_type) def test__has_access_course_desc_can_enroll(self): yesterday = datetime.datetime.now(pytz.utc) - datetime.timedelta(days=1) @@ -301,6 +322,16 @@ class AccessTestCase(LoginEnrollmentTestCase, ModuleStoreTestCase): self.assertTrue(access._has_access_course_desc(staff, 'see_in_catalog', course)) self.assertTrue(access._has_access_course_desc(staff, 'see_about_page', course)) + @ddt.data(True, False) + @patch.dict("django.conf.settings.FEATURES", {'ACCESS_REQUIRE_STAFF_FOR_COURSE': True}) + def test_see_exists(self, ispublic): + """ + Test if user can see course + """ + user = UserFactory.create(is_staff=False) + course = Mock(ispublic=ispublic) + self.assertEquals(bool(access._has_access_course_desc(user, 'see_exists', course)), ispublic) + @patch.dict("django.conf.settings.FEATURES", {'ENABLE_PREREQUISITE_COURSES': True, 'MILESTONES_APP': True}) def test_access_on_course_with_pre_requisites(self): """ @@ -319,10 +350,11 @@ class AccessTestCase(LoginEnrollmentTestCase, ModuleStoreTestCase): ) set_prerequisite_courses(course.id, pre_requisite_courses) - #user should not be able to load course even if enrolled + # user should not be able to load course even if enrolled CourseEnrollmentFactory(user=user, course_id=course.id) - self.assertFalse(access._has_access_course_desc(user, 'view_courseware_with_prerequisites', course)) - + response = access._has_access_course_desc(user, 'view_courseware_with_prerequisites', course) + self.assertFalse(response) + self.assertIsInstance(response, access_response.MilestoneError) # Staff can always access course staff = StaffFactory.create(course_key=course.id) self.assertTrue(access._has_access_course_desc(staff, 'view_courseware_with_prerequisites', course)) @@ -331,6 +363,26 @@ class AccessTestCase(LoginEnrollmentTestCase, ModuleStoreTestCase): fulfill_course_milestone(pre_requisite_course.id, user) self.assertTrue(access._has_access_course_desc(user, 'view_courseware_with_prerequisites', course)) + @ddt.data( + (True, True, True), + (False, False, True) + ) + @ddt.unpack + def test__access_on_mobile(self, mobile_available, student_expected, staff_expected): + """ + Test course access on mobile for staff and students. + """ + descriptor = Mock(user_partitions=[]) + descriptor._class_tags = {} + descriptor.visible_to_staff_only = False + descriptor.mobile_available = mobile_available + + self.assertEqual( + bool(access._has_access_course_desc(self.student, 'load_mobile', descriptor)), + student_expected + ) + self.assertEqual(bool(access._has_access_course_desc(self.staff, 'load_mobile', descriptor)), staff_expected) + @patch.dict("django.conf.settings.FEATURES", {'ENABLE_PREREQUISITE_COURSES': True, 'MILESTONES_APP': True}) def test_courseware_page_unfulfilled_prereqs(self): """ @@ -504,11 +556,11 @@ class CourseOverviewAccessTestCase(ModuleStoreTestCase): course_overview = CourseOverview.get_from_id(course.id) self.assertEqual( - access.has_access(user, action, course, course_key=course.id), - access.has_access(user, action, course_overview, course_key=course.id) + bool(access.has_access(user, action, course, course_key=course.id)), + bool(access.has_access(user, action, course_overview, course_key=course.id)) ) - def test_course_overivew_unsupported_action(self): + def test_course_overview_unsupported_action(self): """ Check that calling has_access with an unsupported action raises a ValueError. diff --git a/lms/djangoapps/courseware/tests/test_group_access.py b/lms/djangoapps/courseware/tests/test_group_access.py index 65554b792c..4fbd8cd281 100644 --- a/lms/djangoapps/courseware/tests/test_group_access.py +++ b/lms/djangoapps/courseware/tests/test_group_access.py @@ -185,7 +185,7 @@ class GroupAccessTestCase(ModuleStoreTestCase): DRY helper. """ self.assertIs( - access.has_access(user, 'load', modulestore().get_item(block_location), self.course.id), + bool(access.has_access(user, 'load', modulestore().get_item(block_location), self.course.id)), is_accessible ) diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py index 403d47a2d9..2b8a4f1285 100644 --- a/lms/djangoapps/courseware/views.py +++ b/lms/djangoapps/courseware/views.py @@ -766,7 +766,7 @@ def syllabus(request, course_id): course_key = SlashSeparatedCourseKey.from_deprecated_string(course_id) course = get_course_with_access(request.user, 'load', course_key) - staff_access = has_access(request.user, 'staff', course) + staff_access = bool(has_access(request.user, 'staff', course)) return render_to_response('courseware/syllabus.html', { 'course': course, @@ -828,7 +828,7 @@ def course_about(request, course_id): registered = registered_for_course(course, request.user) - staff_access = has_access(request.user, 'staff', course) + staff_access = bool(has_access(request.user, 'staff', course)) studio_url = get_studio_url(course, 'settings/details') if has_access(request.user, 'load', course): @@ -836,7 +836,7 @@ def course_about(request, course_id): else: course_target = reverse('about_course', args=[course.id.to_deprecated_string()]) - show_courseware_link = ( + show_courseware_link = bool( ( has_access(request.user, 'load', course) and has_access(request.user, 'view_courseware_with_prerequisites', course) @@ -865,7 +865,7 @@ def course_about(request, course_id): can_add_course_to_cart = _is_shopping_cart_enabled and registration_price # Used to provide context to message to student if enrollment not allowed - can_enroll = has_access(request.user, 'enroll', course) + can_enroll = bool(has_access(request.user, 'enroll', course)) invitation_only = course.invitation_only is_course_full = CourseEnrollment.objects.is_course_full(course) @@ -931,10 +931,10 @@ def mktg_course_about(request, course_id): else: course_target = reverse('about_course', args=[course.id.to_deprecated_string()]) - allow_registration = has_access(request.user, 'enroll', course) + allow_registration = bool(has_access(request.user, 'enroll', course)) - show_courseware_link = (has_access(request.user, 'load', course) or - settings.FEATURES.get('ENABLE_LMS_MIGRATION')) + show_courseware_link = bool(has_access(request.user, 'load', course) or + settings.FEATURES.get('ENABLE_LMS_MIGRATION')) course_modes = CourseMode.modes_for_course_dict(course.id) context = { @@ -1028,7 +1028,7 @@ def _progress(request, course_key, student_id): if survey.utils.must_answer_survey(course, request.user): return redirect(reverse('course_survey', args=[unicode(course.id)])) - staff_access = has_access(request.user, 'staff', course) + staff_access = bool(has_access(request.user, 'staff', course)) if student_id is None or student_id == request.user.id: # always allowed to see your own profile @@ -1175,7 +1175,7 @@ def submission_history(request, course_id, student_username, location): return HttpResponse(escape(_(u'Invalid location.'))) course = get_course_with_access(request.user, 'load', course_key) - staff_access = has_access(request.user, 'staff', course) + staff_access = bool(has_access(request.user, 'staff', course)) # Permission Denied if they don't have staff access and are trying to see # somebody else's submission history. @@ -1478,7 +1478,7 @@ def render_xblock(request, usage_key_string, check_if_enrolled=True): 'disable_header': True, 'disable_window_wrap': True, 'disable_preview_menu': True, - 'staff_access': has_access(request.user, 'staff', course), + 'staff_access': bool(has_access(request.user, 'staff', course)), 'xqa_server': settings.FEATURES.get('XQA_SERVER', 'http://your_xqa_server.com'), } return render_to_response('courseware/courseware-chromeless.html', context) diff --git a/lms/djangoapps/django_comment_client/base/views.py b/lms/djangoapps/django_comment_client/base/views.py index e035b29b45..f4c16280ba 100644 --- a/lms/djangoapps/django_comment_client/base/views.py +++ b/lms/djangoapps/django_comment_client/base/views.py @@ -484,7 +484,7 @@ def un_flag_abuse_for_thread(request, course_id, thread_id): course_key = SlashSeparatedCourseKey.from_deprecated_string(course_id) course = get_course_by_id(course_key) thread = cc.Thread.find(thread_id) - remove_all = ( + remove_all = bool( has_permission(request.user, 'openclose_thread', course_key) or has_access(request.user, 'staff', course) ) @@ -519,7 +519,7 @@ def un_flag_abuse_for_comment(request, course_id, comment_id): user = cc.User.from_django_user(request.user) course_key = SlashSeparatedCourseKey.from_deprecated_string(course_id) course = get_course_by_id(course_key) - remove_all = ( + remove_all = bool( has_permission(request.user, 'openclose_thread', course_key) or has_access(request.user, 'staff', course) ) diff --git a/lms/djangoapps/django_comment_client/forum/views.py b/lms/djangoapps/django_comment_client/forum/views.py index 8608f1bce8..aeb4a7ba75 100644 --- a/lms/djangoapps/django_comment_client/forum/views.py +++ b/lms/djangoapps/django_comment_client/forum/views.py @@ -272,11 +272,11 @@ def forum_form_discussion(request, course_key): 'csrf': csrf(request)['csrf_token'], 'course': course, #'recent_active_threads': recent_active_threads, - 'staff_access': has_access(request.user, 'staff', course), + 'staff_access': bool(has_access(request.user, 'staff', course)), 'threads': _attr_safe_json(threads), 'thread_pages': query_params['num_pages'], 'user_info': _attr_safe_json(user_info), - 'flag_moderator': ( + 'flag_moderator': bool( has_permission(request.user, 'openclose_thread', course.id) or has_access(request.user, 'staff', course) ), @@ -385,7 +385,7 @@ def single_thread(request, course_key, discussion_id, thread_id): 'is_moderator': is_moderator, 'thread_pages': query_params['num_pages'], 'is_course_cohorted': is_course_cohorted(course_key), - 'flag_moderator': ( + 'flag_moderator': bool( has_permission(request.user, 'openclose_thread', course.id) or has_access(request.user, 'staff', course) ), diff --git a/lms/djangoapps/instructor/paidcourse_enrollment_report.py b/lms/djangoapps/instructor/paidcourse_enrollment_report.py index cc7697fb16..d746a2d130 100644 --- a/lms/djangoapps/instructor/paidcourse_enrollment_report.py +++ b/lms/djangoapps/instructor/paidcourse_enrollment_report.py @@ -24,7 +24,7 @@ class PaidCourseEnrollmentReportProvider(BaseAbstractEnrollmentReportProvider): Returns the User Enrollment information. """ course = get_course_by_id(course_id, depth=0) - is_course_staff = has_access(user, 'staff', course) + is_course_staff = bool(has_access(user, 'staff', course)) # check the user enrollment role if user.is_staff: diff --git a/lms/djangoapps/instructor/views/instructor_dashboard.py b/lms/djangoapps/instructor/views/instructor_dashboard.py index 4339a7a7a8..405ebf4b91 100644 --- a/lms/djangoapps/instructor/views/instructor_dashboard.py +++ b/lms/djangoapps/instructor/views/instructor_dashboard.py @@ -62,7 +62,7 @@ class InstructorDashboardTab(CourseTab): """ Returns true if the specified user has staff access. """ - return user and has_access(user, 'staff', course, course.id) + return bool(user and has_access(user, 'staff', course, course.id)) @ensure_csrf_cookie @@ -79,10 +79,10 @@ def instructor_dashboard_2(request, course_id): access = { 'admin': request.user.is_staff, - 'instructor': has_access(request.user, 'instructor', course), + 'instructor': bool(has_access(request.user, 'instructor', course)), 'finance_admin': CourseFinanceAdminRole(course_key).has_user(request.user), 'sales_admin': CourseSalesAdminRole(course_key).has_user(request.user), - 'staff': has_access(request.user, 'staff', course), + 'staff': bool(has_access(request.user, 'staff', course)), 'forum_admin': has_forum_access(request.user, course_key, FORUM_ROLE_ADMINISTRATOR), } diff --git a/lms/djangoapps/instructor/views/legacy.py b/lms/djangoapps/instructor/views/legacy.py index 173a36ae41..b9118e4ec1 100644 --- a/lms/djangoapps/instructor/views/legacy.py +++ b/lms/djangoapps/instructor/views/legacy.py @@ -84,7 +84,7 @@ def instructor_dashboard(request, course_id): course_key = SlashSeparatedCourseKey.from_deprecated_string(course_id) course = get_course_with_access(request.user, 'staff', course_key, depth=None) - instructor_access = has_access(request.user, 'instructor', course) # an instructor can manage staff lists + instructor_access = bool(has_access(request.user, 'instructor', course)) # an instructor can manage staff lists forum_admin_access = has_forum_access(request.user, course_key, FORUM_ROLE_ADMINISTRATOR) diff --git a/lms/djangoapps/open_ended_grading/open_ended_notifications.py b/lms/djangoapps/open_ended_grading/open_ended_notifications.py index 079a25a36b..f4ef7335b1 100644 --- a/lms/djangoapps/open_ended_grading/open_ended_notifications.py +++ b/lms/djangoapps/open_ended_grading/open_ended_notifications.py @@ -118,7 +118,7 @@ def combined_notifications(course, user): #Initialize controller query service using our mock system controller_qs = ControllerQueryService(settings.OPEN_ENDED_GRADING_INTERFACE, render_to_string) student_id = unique_id_for_user(user) - user_is_staff = has_access(user, 'staff', course) + user_is_staff = bool(has_access(user, 'staff', course)) course_id = course.id notification_type = "combined" diff --git a/lms/djangoapps/staticbook/views.py b/lms/djangoapps/staticbook/views.py index dadeb56568..b828ccd9e5 100644 --- a/lms/djangoapps/staticbook/views.py +++ b/lms/djangoapps/staticbook/views.py @@ -22,7 +22,7 @@ def index(request, course_id, book_index, page=None): """ course_key = SlashSeparatedCourseKey.from_deprecated_string(course_id) course = get_course_with_access(request.user, 'load', course_key) - staff_access = has_access(request.user, 'staff', course) + staff_access = bool(has_access(request.user, 'staff', course)) book_index = int(book_index) if book_index < 0 or book_index >= len(course.textbooks): @@ -79,7 +79,7 @@ def pdf_index(request, course_id, book_index, chapter=None, page=None): """ course_key = SlashSeparatedCourseKey.from_deprecated_string(course_id) course = get_course_with_access(request.user, 'load', course_key) - staff_access = has_access(request.user, 'staff', course) + staff_access = bool(has_access(request.user, 'staff', course)) book_index = int(book_index) if book_index < 0 or book_index >= len(course.pdf_textbooks): @@ -147,7 +147,7 @@ def html_index(request, course_id, book_index, chapter=None): """ course_key = SlashSeparatedCourseKey.from_deprecated_string(course_id) course = get_course_with_access(request.user, 'load', course_key) - staff_access = has_access(request.user, 'staff', course) + staff_access = bool(has_access(request.user, 'staff', course)) notes_enabled = notes_enabled_for_course(course) book_index = int(book_index) diff --git a/lms/lib/courseware_search/lms_search_initializer.py b/lms/lib/courseware_search/lms_search_initializer.py index ebcfc88ae2..eec36e1a57 100644 --- a/lms/lib/courseware_search/lms_search_initializer.py +++ b/lms/lib/courseware_search/lms_search_initializer.py @@ -21,5 +21,5 @@ class LmsSearchInitializer(SearchInitializer): course_key = CourseKey.from_string(kwargs['course_id']) except InvalidKeyError: course_key = SlashSeparatedCourseKey.from_deprecated_string(kwargs['course_id']) - staff_access = has_access(request.user, 'staff', course_key) + staff_access = bool(has_access(request.user, 'staff', course_key)) setup_masquerade(request, course_key, staff_access)