Merge pull request #8838 from edx/lliu/has-access-rebased

Changes to has_access

cms/djangoapps/contentstore/views/access.py

@@ -16,7 +16,7 @@ def get_user_role(user, course_id):
     :param course_id: the course_id of the course we're interested in
     """
     # afaik, this is only used in lti
-    if auth.has_access(user, CourseInstructorRole(course_id)):
+    if auth.user_has_role(user, CourseInstructorRole(course_id)):
         return 'instructor'
     else:
         return 'staff'

cms/djangoapps/contentstore/views/course.py

@@ -662,7 +662,7 @@ def _create_or_rerun_course(request):
     Returns the destination course_key and overriding fields for the new course.
     Raises DuplicateCourseError and InvalidKeyError
     """
-    if not auth.has_access(request.user, CourseCreatorRole()):
+    if not auth.user_has_role(request.user, CourseCreatorRole()):
         raise PermissionDenied()
 
     try:

cms/djangoapps/contentstore/views/tests/test_user.py

@@ -76,8 +76,8 @@ class UsersTestCase(CourseTestCase):
         # reload user from DB
         ext_user = User.objects.get(email=self.ext_user.email)
         # no content: should not be in any roles
-        self.assertFalse(auth.has_access(ext_user, CourseStaffRole(self.course.id)))
-        self.assertFalse(auth.has_access(ext_user, CourseInstructorRole(self.course.id)))
+        self.assertFalse(auth.user_has_role(ext_user, CourseStaffRole(self.course.id)))
+        self.assertFalse(auth.user_has_role(ext_user, CourseInstructorRole(self.course.id)))
         self.assert_not_enrolled()
 
     def test_detail_post_staff(self):
@@ -90,8 +90,8 @@ class UsersTestCase(CourseTestCase):
         self.assertEqual(resp.status_code, 204)
         # reload user from DB
         ext_user = User.objects.get(email=self.ext_user.email)
-        self.assertTrue(auth.has_access(ext_user, CourseStaffRole(self.course.id)))
-        self.assertFalse(auth.has_access(ext_user, CourseInstructorRole(self.course.id)))
+        self.assertTrue(auth.user_has_role(ext_user, CourseStaffRole(self.course.id)))
+        self.assertFalse(auth.user_has_role(ext_user, CourseInstructorRole(self.course.id)))
         self.assert_enrolled()
 
     def test_detail_post_staff_other_inst(self):
@@ -106,12 +106,12 @@ class UsersTestCase(CourseTestCase):
         self.assertEqual(resp.status_code, 204)
         # reload user from DB
         ext_user = User.objects.get(email=self.ext_user.email)
-        self.assertTrue(auth.has_access(ext_user, CourseStaffRole(self.course.id)))
-        self.assertFalse(auth.has_access(ext_user, CourseInstructorRole(self.course.id)))
+        self.assertTrue(auth.user_has_role(ext_user, CourseStaffRole(self.course.id)))
+        self.assertFalse(auth.user_has_role(ext_user, CourseInstructorRole(self.course.id)))
         self.assert_enrolled()
         # check that other user is unchanged
         user = User.objects.get(email=self.user.email)
-        self.assertTrue(auth.has_access(user, CourseInstructorRole(self.course.id)))
+        self.assertTrue(auth.user_has_role(user, CourseInstructorRole(self.course.id)))
         self.assertFalse(CourseStaffRole(self.course.id).has_user(user))
 
     def test_detail_post_instructor(self):
@@ -124,7 +124,7 @@ class UsersTestCase(CourseTestCase):
         self.assertEqual(resp.status_code, 204)
         # reload user from DB
         ext_user = User.objects.get(email=self.ext_user.email)
-        self.assertTrue(auth.has_access(ext_user, CourseInstructorRole(self.course.id)))
+        self.assertTrue(auth.user_has_role(ext_user, CourseInstructorRole(self.course.id)))
         self.assertFalse(CourseStaffRole(self.course.id).has_user(ext_user))
         self.assert_enrolled()
 
@@ -149,8 +149,8 @@ class UsersTestCase(CourseTestCase):
         self.assertEqual(resp.status_code, 204)
         # reload user from DB
         ext_user = User.objects.get(email=self.ext_user.email)
-        self.assertTrue(auth.has_access(ext_user, CourseStaffRole(self.course.id)))
-        self.assertFalse(auth.has_access(ext_user, CourseInstructorRole(self.course.id)))
+        self.assertTrue(auth.user_has_role(ext_user, CourseStaffRole(self.course.id)))
+        self.assertFalse(auth.user_has_role(ext_user, CourseInstructorRole(self.course.id)))
         self.assert_enrolled()
 
     def test_detail_delete_staff(self):
@@ -163,7 +163,7 @@ class UsersTestCase(CourseTestCase):
         self.assertEqual(resp.status_code, 204)
         # reload user from DB
         ext_user = User.objects.get(email=self.ext_user.email)
-        self.assertFalse(auth.has_access(ext_user, CourseStaffRole(self.course.id)))
+        self.assertFalse(auth.user_has_role(ext_user, CourseStaffRole(self.course.id)))
 
     def test_detail_delete_instructor(self):
         auth.add_users(self.user, CourseInstructorRole(self.course.id), self.ext_user, self.user)
@@ -175,7 +175,7 @@ class UsersTestCase(CourseTestCase):
         self.assertEqual(resp.status_code, 204)
         # reload user from DB
         ext_user = User.objects.get(email=self.ext_user.email)
-        self.assertFalse(auth.has_access(ext_user, CourseInstructorRole(self.course.id)))
+        self.assertFalse(auth.user_has_role(ext_user, CourseInstructorRole(self.course.id)))
 
     def test_delete_last_instructor(self):
         auth.add_users(self.user, CourseInstructorRole(self.course.id), self.ext_user)
@@ -189,7 +189,7 @@ class UsersTestCase(CourseTestCase):
         self.assertIn("error", result)
         # reload user from DB
         ext_user = User.objects.get(email=self.ext_user.email)
-        self.assertTrue(auth.has_access(ext_user, CourseInstructorRole(self.course.id)))
+        self.assertTrue(auth.user_has_role(ext_user, CourseInstructorRole(self.course.id)))
 
     def test_post_last_instructor(self):
         auth.add_users(self.user, CourseInstructorRole(self.course.id), self.ext_user)
@@ -204,7 +204,7 @@ class UsersTestCase(CourseTestCase):
         self.assertIn("error", result)
         # reload user from DB
         ext_user = User.objects.get(email=self.ext_user.email)
-        self.assertTrue(auth.has_access(ext_user, CourseInstructorRole(self.course.id)))
+        self.assertTrue(auth.user_has_role(ext_user, CourseInstructorRole(self.course.id)))
 
     def test_permission_denied_self(self):
         auth.add_users(self.user, CourseStaffRole(self.course.id), self.user)
@@ -247,7 +247,7 @@ class UsersTestCase(CourseTestCase):
         self.assertEqual(resp.status_code, 204)
         # reload user from DB
         user = User.objects.get(email=self.user.email)
-        self.assertFalse(auth.has_access(user, CourseStaffRole(self.course.id)))
+        self.assertFalse(auth.user_has_role(user, CourseStaffRole(self.course.id)))
 
     def test_staff_cannot_delete_other(self):
         auth.add_users(self.user, CourseStaffRole(self.course.id), self.user, self.ext_user)
@@ -260,7 +260,7 @@ class UsersTestCase(CourseTestCase):
         self.assertIn("error", result)
         # reload user from DB
         ext_user = User.objects.get(email=self.ext_user.email)
-        self.assertTrue(auth.has_access(ext_user, CourseStaffRole(self.course.id)))
+        self.assertTrue(auth.user_has_role(ext_user, CourseStaffRole(self.course.id)))
 
     def test_user_not_initially_enrolled(self):
         # Verify that ext_user is not enrolled in the new course before being added as a staff member.

cms/djangoapps/course_creators/tests/test_admin.py

@@ -56,7 +56,7 @@ class CourseCreatorAdminTest(TestCase):
         def change_state_and_verify_email(state, is_creator):
             """ Changes user state, verifies creator status, and verifies e-mail is sent based on transition """
             self._change_state(state)
-            self.assertEqual(is_creator, auth.has_access(self.user, CourseCreatorRole()))
+            self.assertEqual(is_creator, auth.user_has_role(self.user, CourseCreatorRole()))
 
             context = {'studio_request_email': self.studio_request_email}
             if state == CourseCreator.GRANTED:
@@ -74,7 +74,7 @@ class CourseCreatorAdminTest(TestCase):
         with mock.patch.dict('django.conf.settings.FEATURES', self.enable_creator_group_patch):
 
             # User is initially unrequested.
-            self.assertFalse(auth.has_access(self.user, CourseCreatorRole()))
+            self.assertFalse(auth.user_has_role(self.user, CourseCreatorRole()))
 
             change_state_and_verify_email(CourseCreator.GRANTED, True)
 

cms/djangoapps/course_creators/tests/test_views.py

@@ -50,7 +50,7 @@ class CourseCreatorView(TestCase):
     def test_add_granted(self):
         with mock.patch.dict('django.conf.settings.FEATURES', {"ENABLE_CREATOR_GROUP": True}):
             # Calling add_user_with_status_granted impacts is_user_in_course_group_role.
-            self.assertFalse(auth.has_access(self.user, CourseCreatorRole()))
+            self.assertFalse(auth.user_has_role(self.user, CourseCreatorRole()))
 
             add_user_with_status_granted(self.admin, self.user)
             self.assertEqual('granted', get_course_creator_status(self.user))
@@ -59,15 +59,15 @@ class CourseCreatorView(TestCase):
             add_user_with_status_unrequested(self.user)
             self.assertEqual('granted', get_course_creator_status(self.user))
 
-            self.assertTrue(auth.has_access(self.user, CourseCreatorRole()))
+            self.assertTrue(auth.user_has_role(self.user, CourseCreatorRole()))
 
     def test_update_creator_group(self):
         with mock.patch.dict('django.conf.settings.FEATURES', {"ENABLE_CREATOR_GROUP": True}):
-            self.assertFalse(auth.has_access(self.user, CourseCreatorRole()))
+            self.assertFalse(auth.user_has_role(self.user, CourseCreatorRole()))
             update_course_creator_group(self.admin, self.user, True)
-            self.assertTrue(auth.has_access(self.user, CourseCreatorRole()))
+            self.assertTrue(auth.user_has_role(self.user, CourseCreatorRole()))
             update_course_creator_group(self.admin, self.user, False)
-            self.assertFalse(auth.has_access(self.user, CourseCreatorRole()))
+            self.assertFalse(auth.user_has_role(self.user, CourseCreatorRole()))
 
     def test_user_requested_access(self):
         add_user_with_status_unrequested(self.user)

common/djangoapps/student/auth.py

@@ -20,7 +20,7 @@ STUDIO_VIEW_CONTENT = 1
 # In addition to the above, one is always allowed to "demote" oneself to a lower role within a course, or remove oneself
 
 
-def has_access(user, role):
+def user_has_role(user, role):
     """
     Check whether this user has access to this role (either direct or implied)
     :param user:
@@ -64,14 +64,14 @@ def get_user_permissions(user, course_key, org=None):
     # global staff, org instructors, and course instructors have all permissions:
     if GlobalStaff().has_user(user) or OrgInstructorRole(org=org).has_user(user):
         return all_perms
-    if course_key and has_access(user, CourseInstructorRole(course_key)):
+    if course_key and user_has_role(user, CourseInstructorRole(course_key)):
         return all_perms
     # Staff have all permissions except EDIT_ROLES:
-    if OrgStaffRole(org=org).has_user(user) or (course_key and has_access(user, CourseStaffRole(course_key))):
+    if OrgStaffRole(org=org).has_user(user) or (course_key and user_has_role(user, CourseStaffRole(course_key))):
         return STUDIO_VIEW_USERS | STUDIO_EDIT_CONTENT | STUDIO_VIEW_CONTENT
     # Otherwise, for libraries, users can view only:
     if course_key and isinstance(course_key, LibraryLocator):
-        if OrgLibraryUserRole(org=org).has_user(user) or has_access(user, LibraryUserRole(course_key)):
+        if OrgLibraryUserRole(org=org).has_user(user) or user_has_role(user, LibraryUserRole(course_key)):
             return STUDIO_VIEW_USERS | STUDIO_VIEW_CONTENT
     return 0
 
@@ -151,5 +151,5 @@ def _check_caller_authority(caller, role):
     if isinstance(role, (GlobalStaff, CourseCreatorRole)):
         raise PermissionDenied
     elif isinstance(role, CourseRole):  # instructors can change the roles w/in their course
-        if not has_access(caller, CourseInstructorRole(role.course_key)):
+        if not user_has_role(caller, CourseInstructorRole(role.course_key)):
             raise PermissionDenied

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 user_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(user_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(user_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(user_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(user_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(user_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(user_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(user_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(user_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(user_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(user_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(user_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(user_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(user_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(user_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(user_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(user_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(user_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(user_has_role(self.creator, CourseInstructorRole(self.course_key)))
 
     def test_remove_user_from_course_group_permission_denied(self):
         """

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)

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):

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
+        return bool(
+            settings.DEBUG
             or has_access(user, CourseStaffRole.ROLE, course)
-                or has_access(user, CourseInstructorRole.ROLE, course))
+            or has_access(user, CourseInstructorRole.ROLE, course)
+        )
 
     def check_course_permissions(self, user, course):
         """

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.
@@ -748,9 +849,9 @@ def is_mobile_available_for_user(user, descriptor):
         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.user_has_role(user, CourseBetaTesterRole(descriptor.id))
+        or _has_staff_access_to_descriptor(user, descriptor, descriptor.id)
+        or _is_descriptor_mobile_available(descriptor)
     )
 
 

lms/djangoapps/courseware/access_response.py

+"""
+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)

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'))
 

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):

lms/djangoapps/courseware/tests/test_access.py

+# -*- 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.
-        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.
+    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 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.

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
         )
 

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,9 +931,9 @@ 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
+    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)
 
@@ -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)

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)
     )

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)
             ),

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:

lms/djangoapps/instructor/views/api.py

@@ -274,7 +274,7 @@ def require_sales_admin(func):
             log.error(u"Unable to find course with course key %s", course_id)
             return HttpResponseNotFound()
 
-        access = auth.has_access(request.user, CourseSalesAdminRole(course_key))
+        access = auth.user_has_role(request.user, CourseSalesAdminRole(course_key))
 
         if access:
             return func(request, course_id)
@@ -299,7 +299,7 @@ def require_finance_admin(func):
             log.error(u"Unable to find course with course key %s", course_id)
             return HttpResponseNotFound()
 
-        access = auth.has_access(request.user, CourseFinanceAdminRole(course_key))
+        access = auth.user_has_role(request.user, CourseFinanceAdminRole(course_key))
 
         if access:
             return func(request, course_id)

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),
     }
 

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)
 

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"
 

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)

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)