Andal.LND/edx-platform
6
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Don't fetch the course just to find out if the user has access to it.

Browse Source
This commit is contained in:
Don Mitchell
2014-05-06 17:23:51 -04:00
parent d466132a1f
commit 6834376b8a
2 changed files with 57 additions and 51 deletions
Download Patch File Download Diff File Expand all files Collapse all files

61
lms/djangoapps/courseware/access.py
View File

@@ -23,6 +23,7 @@ from student.roles import (
GlobalStaff, CourseStaffRole, CourseInstructorRole,
OrgStaffRole, OrgInstructorRole, CourseBetaTesterRole
)
from xmodule.modulestore.keys import CourseKey
DEBUG_ACCESS = False
log = logging.getLogger(__name__)
@@ -81,6 +82,9 @@ def has_access(user, action, obj, course_key=None):
if isinstance(obj, XBlock):
return _has_access_descriptor(user, action, obj, course_key)
if isinstance(obj, CourseKey):
return _has_access_course_key(user, action, obj)
if isinstance(obj, Location):
return _has_access_location(user, action, obj, course_key)
@@ -297,8 +301,6 @@ def _has_access_location(user, action, location, course_key):
NOTE: if you add other actions, make sure that
has_access(user, location, action) == has_access(user, get_item(location), action)
And in general, prefer checking access on loaded items, rather than locations.
"""
checkers = {
'staff': lambda: _has_staff_access_to_location(user, location, course_key)
@@ -307,6 +309,22 @@ def _has_access_location(user, action, location, course_key):
return _dispatch(checkers, action, user, location)
def _has_access_course_key(user, action, course_key):
"""
Check if user has access to the course with this course_key
Valid actions:
'staff' : True if the user has staff access to this location
'instructor' : True if the user has staff access to this location
"""
checkers = {
'staff': lambda: _has_staff_access_to_location(user, None, course_key),
'instructor': lambda: _has_instructor_access_to_location(user, None, course_key),
}
return _dispatch(checkers, action, user, course_key)
def _has_access_string(user, action, perm, course_key):
"""
Check if user has certain special access, specified as string. Valid strings:
@@ -388,27 +406,25 @@ def _adjust_start_date_for_beta_testers(user, descriptor, course_key=None): # p
return descriptor.start
def _has_instructor_access_to_location(user, location, course_key=None): # pylint: disable=invalid-name
return _has_access_to_location(user, 'instructor', location, course_key)
def _has_instructor_access_to_location(user, location, course_key=None):
if course_key is None:
course_key = location.course_key
return _has_access_to_course(user, 'instructor', course_key)
def _has_staff_access_to_location(user, location, course_key=None):
return _has_access_to_location(user, 'staff', location, course_key)
if course_key is None:
course_key = location.course_key
return _has_access_to_course(user, 'staff', course_key)
def _has_access_to_location(user, access_level, location, course_key):
def _has_access_to_course(user, access_level, course_key):
'''
Returns True if the given user has access_level (= staff or
instructor) access to a location. For now this is equivalent to
having staff / instructor access to the course location.course.
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.
This means that user is in the staff_* group or instructor_* group, or is an overall admin.
TODO (vshnayder): this needs to be changed to allow per-course_key permissions, not per-course
(e.g. staff in 2012 is different from 2013, but maybe some people always have access)
course is a string: the course field of the location being accessed.
location = location
access_level = string, either "staff" or "instructor"
'''
if user is None or (not user.is_authenticated()):
@@ -423,7 +439,7 @@ def _has_access_to_location(user, access_level, location, course_key):
return True
if access_level not in ('staff', 'instructor'):
log.debug("Error in access._has_access_to_location access_level=%s unknown", access_level)
log.debug("Error in access._has_access_to_course access_level=%s unknown", access_level)
debug("Deny: unknown access level")
return False
@@ -449,13 +465,6 @@ def _has_access_to_location(user, access_level, location, course_key):
return False
# TODO Please change this function signature to _has_staff_access_to_course_key at next opportunity!
def _has_staff_access_to_course_id(user, course_key):
"""Helper method that takes a course_key instead of a course name"""
loc = CourseDescriptor.id_to_location(course_key)
return _has_staff_access_to_location(user, loc, course_key)
def _has_instructor_access_to_descriptor(user, descriptor, course_key): # pylint: disable=invalid-name
"""Helper method that checks whether the user has staff access to
the course of the location.
@@ -479,13 +488,11 @@ def get_user_role(user, course_key):
Return corresponding string if user has staff, instructor or student
course role in LMS.
"""
from courseware.courses import get_course
course = get_course(course_key)
if is_masquerading_as_student(user):
return 'student'
elif has_access(user, 'instructor', course):
elif has_access(user, 'instructor', course_key):
return 'instructor'
elif has_access(user, 'staff', course):
elif has_access(user, 'staff', course_key):
return 'staff'
else:
return 'student'

47
lms/djangoapps/courseware/tests/test_access.py
View File

@@ -8,7 +8,6 @@ from django.test.utils import override_settings
from courseware.tests.factories import UserFactory, StaffFactory, InstructorFactory
from student.tests.factories import AnonymousUserFactory, CourseEnrollmentAllowedFactory
from xmodule.modulestore import Location
from courseware.tests.tests import TEST_DATA_MIXED_MODULESTORE
import pytz
from xmodule.modulestore.locations import SlashSeparatedCourseKey
@@ -31,48 +30,48 @@ class AccessTestCase(TestCase):
self.course_staff = StaffFactory(course=self.course.course_key)
self.course_instructor = InstructorFactory(course=self.course.course_key)
def test__has_access_to_location(self):
self.assertFalse(access._has_access_to_location(
None, 'staff', self.course, self.course.course_key
def test_has_access_to_course(self):
self.assertFalse(access._has_access_to_course(
None, 'staff', self.course.course_key
))
self.assertFalse(access._has_access_to_location(
self.anonymous_user, 'staff', self.course, self.course.course_key
self.assertFalse(access._has_access_to_course(
self.anonymous_user, 'staff', self.course.course_key
))
self.assertFalse(access._has_access_to_location(
self.anonymous_user, 'instructor', self.course, self.course.course_key
self.assertFalse(access._has_access_to_course(
self.anonymous_user, 'instructor', self.course.course_key
))
self.assertTrue(access._has_access_to_location(
self.global_staff, 'staff', self.course, self.course.course_key
self.assertTrue(access._has_access_to_course(
self.global_staff, 'staff', self.course.course_key
))
self.assertTrue(access._has_access_to_location(
self.global_staff, 'instructor', self.course, self.course.course_key
self.assertTrue(access._has_access_to_course(
self.global_staff, 'instructor', self.course.course_key
))
# A user has staff access if they are in the staff group
self.assertTrue(access._has_access_to_location(
self.course_staff, 'staff', self.course, self.course.course_key
self.assertTrue(access._has_access_to_course(
self.course_staff, 'staff', self.course.course_key
))
self.assertFalse(access._has_access_to_location(
self.course_staff, 'instructor', self.course, self.course.course_key
self.assertFalse(access._has_access_to_course(
self.course_staff, 'instructor', self.course.course_key
))
# A user has staff and instructor access if they are in the instructor group
self.assertTrue(access._has_access_to_location(
self.course_instructor, 'staff', self.course, self.course.course_key
self.assertTrue(access._has_access_to_course(
self.course_instructor, 'staff', self.course.course_key
))
self.assertTrue(access._has_access_to_location(
self.course_instructor, 'instructor', self.course, self.course.course_key
self.assertTrue(access._has_access_to_course(
self.course_instructor, 'instructor', self.course.course_key
))
# A user does not have staff or instructor access if they are
# not in either the staff or the the instructor group
self.assertFalse(access._has_access_to_location(
self.student, 'staff', self.course, self.course.course_key
self.assertFalse(access._has_access_to_course(
self.student, 'staff', self.course.course_key
))
self.assertFalse(access._has_access_to_location(
self.student, 'instructor', self.course, self.course.course_key
self.assertFalse(access._has_access_to_course(
self.student, 'instructor', self.course.course_key
))
def test__has_access_string(self):