Commit e0840d2d by tlindaliu

MA-849: Change has_access return type

New classes for the return type, and changes to the has_access function and tests to make them compatible.
parent 4b4ce5f1
......@@ -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):
"""
......
......@@ -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)
......@@ -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):
......
......@@ -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):
"""
......
"""
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)
......@@ -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'))
......
......@@ -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):
......
......@@ -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
)
......
......@@ -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)
......@@ -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)
)
......
......@@ -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)
),
......
......@@ -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:
......
......@@ -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),
}
......
......@@ -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)
......
......@@ -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"
......
......@@ -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)
......
......@@ -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)
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment