Commit 046e2953 by Brandon DeRosier

Change has_course_access definition and doc string

- has_course_access renamed to has_course_author_access for clarity
- Changed doc string to clearly state that it determines whether or not
  a user has write access to a course
parent bdc64a7c
......@@ -6,7 +6,7 @@ from opaque_keys.edx.locator import CourseLocator
from xmodule.modulestore import ModuleStoreEnum, EdxJSONEncoder
from contentstore.tests.utils import CourseTestCase
from contentstore.tasks import rerun_course
from student.auth import has_course_access
from student.auth import has_course_author_access
from course_action_state.models import CourseRerunState
from course_action_state.managers import CourseRerunUIStateManager
from mock import patch, Mock
......@@ -62,7 +62,7 @@ class CloneCourseTest(CourseTestCase):
result = rerun_course.delay(unicode(mongo_course1_id), unicode(split_course3_id), self.user.id,
json.dumps(fields, cls=EdxJSONEncoder))
self.assertEqual(result.get(), "succeeded")
self.assertTrue(has_course_access(self.user, split_course3_id), "Didn't grant access")
self.assertTrue(has_course_author_access(self.user, split_course3_id), "Didn't grant access")
rerun_state = CourseRerunState.objects.find_first(course_key=split_course3_id)
self.assertEqual(rerun_state.state, CourseRerunUIStateManager.State.SUCCEEDED)
......
......@@ -87,12 +87,12 @@ class TestCourseAccess(ModuleStoreTestCase):
else:
group = role(self.course_key)
# NOTE: this loop breaks the roles.py abstraction by purposely assigning
# users to one of each possible groupname in order to test that has_course_access
# users to one of each possible groupname in order to test that has_course_author_access
# and remove_user work
user = users.pop()
group.add_users(user)
user_by_role[role].append(user)
self.assertTrue(auth.has_course_access(user, self.course_key), "{} does not have access".format(user))
self.assertTrue(auth.has_course_author_access(user, self.course_key), "{} does not have access".format(user))
course_team_url = reverse_course_url('course_team_handler', self.course_key)
response = self.client.get_html(course_team_url)
......@@ -125,9 +125,9 @@ class TestCourseAccess(ModuleStoreTestCase):
if hasattr(user, '_roles'):
del user._roles
self.assertTrue(auth.has_course_access(user, copy_course_key), "{} no copy access".format(user))
self.assertTrue(auth.has_course_author_access(user, copy_course_key), "{} no copy access".format(user))
if (role is OrgStaffRole) or (role is OrgInstructorRole):
auth.remove_users(self.user, role(self.course_key.org), user)
else:
auth.remove_users(self.user, role(self.course_key), user)
self.assertFalse(auth.has_course_access(user, self.course_key), "{} remove didn't work".format(user))
self.assertFalse(auth.has_course_author_access(user, self.course_key), "{} remove didn't work".format(user))
......@@ -26,7 +26,7 @@ from util.json_request import JsonResponse
from django.http import HttpResponseNotFound
from django.utils.translation import ugettext as _
from pymongo import ASCENDING, DESCENDING
from student.auth import has_course_access
from student.auth import has_course_author_access
from xmodule.modulestore.exceptions import ItemNotFoundError
__all__ = ['assets_handler']
......@@ -57,7 +57,7 @@ def assets_handler(request, course_key_string=None, asset_key_string=None):
json: delete an asset
"""
course_key = CourseKey.from_string(course_key_string)
if not has_course_access(request.user, course_key):
if not has_course_author_access(request.user, course_key):
raise PermissionDenied()
response_format = request.REQUEST.get('format', 'html')
......
......@@ -13,7 +13,7 @@ from opaque_keys.edx.keys import CourseKey
from xmodule.modulestore.django import modulestore
from contentstore.utils import reverse_course_url
from student.auth import has_course_access
from student.auth import has_course_author_access
from xmodule.course_module import CourseDescriptor
from django.utils.translation import ugettext
......@@ -36,7 +36,7 @@ def checklists_handler(request, course_key_string, checklist_index=None):
json: updates the checked state for items within a particular checklist. checklist_index is required.
"""
course_key = CourseKey.from_string(course_key_string)
if not has_course_access(request.user, course_key):
if not has_course_author_access(request.user, course_key):
raise PermissionDenied()
course_module = modulestore().get_course(course_key)
......
......@@ -25,7 +25,7 @@ from contentstore.views.item import create_xblock_info
from opaque_keys.edx.keys import UsageKey
from student.auth import has_course_access
from student.auth import has_course_author_access
from django.utils.translation import ugettext as _
from models.settings.course_grading import CourseGradingModel
......@@ -349,7 +349,7 @@ def _get_item_in_course(request, usage_key):
course_key = usage_key.course_key
if not has_course_access(request.user, course_key):
if not has_course_author_access(request.user, course_key):
raise PermissionDenied()
course = modulestore().get_course(course_key)
......
......@@ -47,7 +47,7 @@ from models.settings.course_grading import CourseGradingModel
from models.settings.course_metadata import CourseMetadata
from util.json_request import expect_json
from util.string_utils import _has_non_ascii_characters
from student.auth import has_course_access
from student.auth import has_course_author_access
from .component import (
OPEN_ENDED_COMPONENT_TYPES,
NOTE_COMPONENT_TYPES,
......@@ -94,7 +94,7 @@ def _get_course_module(course_key, user, depth=0):
Internal method used to calculate and return the locator and course module
for the view functions in this file.
"""
if not has_course_access(user, course_key):
if not has_course_author_access(user, course_key):
raise PermissionDenied()
course_module = modulestore().get_course(course_key, depth=depth)
return course_module
......@@ -128,7 +128,7 @@ def course_notifications_handler(request, course_key_string=None, action_state_i
course_key = CourseKey.from_string(course_key_string)
if response_format == 'json' or 'application/json' in request.META.get('HTTP_ACCEPT', 'application/json'):
if not has_course_access(request.user, course_key):
if not has_course_author_access(request.user, course_key):
raise PermissionDenied()
if request.method == 'GET':
return _course_notifications_json_get(action_state_id)
......@@ -218,7 +218,7 @@ def course_handler(request, course_key_string=None):
return JsonResponse(_course_outline_json(request, course_module))
elif request.method == 'POST': # not sure if this is only post. If one will have ids, it goes after access
return _create_or_rerun_course(request)
elif not has_course_access(request.user, CourseKey.from_string(course_key_string)):
elif not has_course_author_access(request.user, CourseKey.from_string(course_key_string)):
raise PermissionDenied()
elif request.method == 'PUT':
raise NotImplementedError()
......@@ -290,7 +290,7 @@ def _accessible_courses_list(request):
if course.location.course == 'templates':
return False
return has_course_access(request.user, course.id)
return has_course_author_access(request.user, course.id)
courses = filter(course_filter, modulestore().get_courses())
in_process_course_actions = [
......@@ -298,7 +298,7 @@ def _accessible_courses_list(request):
CourseRerunState.objects.find_all(
exclude_args={'state': CourseRerunUIStateManager.State.SUCCEEDED}, should_display=True
)
if has_course_access(request.user, course.course_key)
if has_course_author_access(request.user, course.course_key)
]
return courses, in_process_course_actions
......@@ -621,7 +621,7 @@ def _rerun_course(request, org, number, run, fields):
source_course_key = CourseKey.from_string(request.json.get('source_course_key'))
# verify user has access to the original course
if not has_course_access(request.user, source_course_key):
if not has_course_author_access(request.user, source_course_key):
raise PermissionDenied()
# create destination course key
......@@ -702,7 +702,7 @@ def course_info_update_handler(request, course_key_string, provided_id=None):
provided_id = None
# check that logged in user has permissions to this item (GET shouldn't require this level?)
if not has_course_access(request.user, usage_key.course_key):
if not has_course_author_access(request.user, usage_key.course_key):
raise PermissionDenied()
if request.method == 'GET':
......
......@@ -10,7 +10,7 @@ from django.core.exceptions import PermissionDenied
from django_future.csrf import ensure_csrf_cookie
from django.utils.translation import ugettext as _
from student.auth import has_course_access
from student.auth import has_course_author_access
import contentstore.git_export_utils as git_export_utils
from edxmako.shortcuts import render_to_response
from xmodule.modulestore.django import modulestore
......@@ -26,7 +26,7 @@ def export_git(request, course_key_string):
This method serves up the 'Export to Git' page
"""
course_key = CourseKey.from_string(course_key_string)
if not has_course_access(request.user, course_key):
if not has_course_author_access(request.user, course_key):
raise PermissionDenied()
course_module = modulestore().get_course(course_key)
......
......@@ -28,7 +28,7 @@ from opaque_keys.edx.keys import CourseKey
from xmodule.modulestore.xml_importer import import_from_xml
from xmodule.modulestore.xml_exporter import export_to_xml
from student.auth import has_course_access
from student.auth import has_course_author_access
from extract_tar import safetar_extractall
from util.json_request import JsonResponse
......@@ -63,7 +63,7 @@ def import_handler(request, course_key_string):
json: import a course via the .tar.gz file specified in request.FILES
"""
course_key = CourseKey.from_string(course_key_string)
if not has_course_access(request.user, course_key):
if not has_course_author_access(request.user, course_key):
raise PermissionDenied()
if 'application/json' in request.META.get('HTTP_ACCEPT', 'application/json'):
......@@ -313,7 +313,7 @@ def import_status_handler(request, course_key_string, filename=None):
"""
course_key = CourseKey.from_string(course_key_string)
if not has_course_access(request.user, course_key):
if not has_course_author_access(request.user, course_key):
raise PermissionDenied()
try:
......@@ -346,7 +346,7 @@ def export_handler(request, course_key_string):
which describes the error.
"""
course_key = CourseKey.from_string(course_key_string)
if not has_course_access(request.user, course_key):
if not has_course_author_access(request.user, course_key):
raise PermissionDenied()
course_module = modulestore().get_course(course_key)
......
......@@ -37,7 +37,7 @@ from util.date_utils import get_default_time_display
from util.json_request import expect_json, JsonResponse
from student.auth import has_course_access
from student.auth import has_course_author_access
from contentstore.utils import find_release_date_source, find_staff_lock_source, is_currently_visible_to_students, \
ancestor_has_staff_lock
from contentstore.views.helpers import is_unit, xblock_studio_url, xblock_primary_child_category, \
......@@ -129,7 +129,7 @@ def xblock_handler(request, usage_key_string):
if usage_key_string:
usage_key = usage_key_with_run(usage_key_string)
if not has_course_access(request.user, usage_key.course_key):
if not has_course_author_access(request.user, usage_key.course_key):
raise PermissionDenied()
if request.method == 'GET':
......@@ -196,7 +196,7 @@ def xblock_view_handler(request, usage_key_string, view_name):
the second is the resource description
"""
usage_key = usage_key_with_run(usage_key_string)
if not has_course_access(request.user, usage_key.course_key):
if not has_course_author_access(request.user, usage_key.course_key):
raise PermissionDenied()
accept_header = request.META.get('HTTP_ACCEPT', 'application/json')
......@@ -283,7 +283,7 @@ def xblock_outline_handler(request, usage_key_string):
a course.
"""
usage_key = usage_key_with_run(usage_key_string)
if not has_course_access(request.user, usage_key.course_key):
if not has_course_author_access(request.user, usage_key.course_key):
raise PermissionDenied()
response_format = request.REQUEST.get('format', 'html')
......@@ -456,7 +456,7 @@ def _create_item(request):
display_name = request.json.get('display_name')
if not has_course_access(request.user, usage_key.course_key):
if not has_course_author_access(request.user, usage_key.course_key):
raise PermissionDenied()
store = modulestore()
......@@ -598,7 +598,7 @@ def orphan_handler(request, course_key_string):
"""
course_usage_key = CourseKey.from_string(course_key_string)
if request.method == 'GET':
if has_course_access(request.user, course_usage_key):
if has_course_author_access(request.user, course_usage_key):
return JsonResponse([unicode(item) for item in modulestore().get_orphans(course_usage_key)])
else:
raise PermissionDenied()
......
"""
Views related to course tabs
"""
from student.auth import has_course_access
from student.auth import has_course_author_access
from util.json_request import expect_json, JsonResponse
from django.http import HttpResponseNotFound
......@@ -40,7 +40,7 @@ def tabs_handler(request, course_key_string):
Instead use the general xblock URL (see item.xblock_handler).
"""
course_key = CourseKey.from_string(course_key_string)
if not has_course_access(request.user, course_key):
if not has_course_author_access(request.user, course_key):
raise PermissionDenied()
course_item = modulestore().get_course(course_key)
......
......@@ -7,7 +7,7 @@ import datetime
from contentstore.tests.utils import CourseTestCase
from contentstore.utils import reverse_course_url, add_instructor
from student.auth import has_course_access
from student.auth import has_course_author_access
from contentstore.views.course import course_outline_initial_state
from contentstore.views.item import create_xblock_info, VisibilityState
from course_action_state.models import CourseRerunState
......@@ -181,7 +181,7 @@ class TestCourseIndex(CourseTestCase):
# delete nofications that are dismissed
CourseRerunState.objects.get(id=rerun_state.id)
self.assertFalse(has_course_access(user2, rerun_course_key))
self.assertFalse(has_course_author_access(user2, rerun_course_key))
def assert_correct_json_response(self, json_response):
"""
......
......@@ -38,7 +38,7 @@ from xmodule.video_module.transcripts_utils import (
TranscriptsRequestValidationException
)
from student.auth import has_course_access
from student.auth import has_course_author_access
__all__ = [
'upload_transcripts',
......@@ -538,12 +538,12 @@ def _get_item(request, data):
Returns the item.
"""
usage_key = UsageKey.from_string(data.get('locator'))
# This is placed before has_course_access() to validate the location,
# because has_course_access() raises r if location is invalid.
# This is placed before has_course_author_access() to validate the location,
# because has_course_author_access() raises r if location is invalid.
item = modulestore().get_item(usage_key)
# use the item's course_key, because the usage_key might not have the run
if not has_course_access(request.user, item.location.course_key):
if not has_course_author_access(request.user, item.location.course_key):
raise PermissionDenied()
return item
......@@ -13,7 +13,7 @@ from util.json_request import JsonResponse, expect_json
from student.roles import CourseInstructorRole, CourseStaffRole
from course_creators.views import user_requested_access
from student.auth import has_course_access
from student.auth import has_course_author_access
from student.models import CourseEnrollment
from django.http import HttpResponseNotFound
......@@ -50,7 +50,7 @@ def course_team_handler(request, course_key_string=None, email=None):
json: remove a particular course team member from the course team (email is required).
"""
course_key = CourseKey.from_string(course_key_string) if course_key_string else None
if not has_course_access(request.user, course_key):
if not has_course_author_access(request.user, course_key):
raise PermissionDenied()
if 'application/json' in request.META.get('HTTP_ACCEPT', 'application/json'):
......@@ -66,7 +66,7 @@ def _manage_users(request, course_key):
This view will return all CMS users who are editors for the specified course
"""
# check that logged in user has permissions to this item
if not has_course_access(request.user, course_key):
if not has_course_author_access(request.user, course_key):
raise PermissionDenied()
course_module = modulestore().get_course(course_key)
......@@ -78,7 +78,7 @@ def _manage_users(request, course_key):
'context_course': course_module,
'staff': staff,
'instructors': instructors,
'allow_actions': has_course_access(request.user, course_key, role=CourseInstructorRole),
'allow_actions': has_course_author_access(request.user, course_key, role=CourseInstructorRole),
})
......@@ -88,10 +88,10 @@ def _course_team_user(request, course_key, email):
Handle the add, remove, promote, demote requests ensuring the requester has authority
"""
# check that logged in user has permissions to this item
if has_course_access(request.user, course_key, role=CourseInstructorRole):
if has_course_author_access(request.user, course_key, role=CourseInstructorRole):
# instructors have full permissions
pass
elif has_course_access(request.user, course_key, role=CourseStaffRole) and email == request.user.email:
elif has_course_author_access(request.user, course_key, role=CourseStaffRole) and email == request.user.email:
# staff can only affect themselves
pass
else:
......@@ -145,7 +145,7 @@ def _course_team_user(request, course_key, email):
return JsonResponse({"error": _("`role` is required")}, 400)
if role == "instructor":
if not has_course_access(request.user, course_key, role=CourseInstructorRole):
if not has_course_author_access(request.user, course_key, role=CourseInstructorRole):
msg = {
"error": _("Only instructors may create other instructors")
}
......
......@@ -40,18 +40,18 @@ def has_access(user, role):
return False
def has_course_access(user, course_key, role=CourseStaffRole):
def has_course_author_access(user, course_key, role=CourseStaffRole):
"""
Return True if user allowed to access this course_id
Note that the CMS permissions model is with respect to courses
There is a super-admin permissions if user.is_staff is set
Return True if user has studio (write) access to the given course.
Note that the CMS permissions model is with respect to courses.
There is a super-admin permissions if user.is_staff is set.
Also, since we're unifying the user database between LMS and CAS,
I'm presuming that the course instructor (formally known as admin)
will not be in both INSTRUCTOR and STAFF groups, so we have to cascade our
queries here as INSTRUCTOR has all the rights that STAFF do.
:param user:
:param course_key: A course key
:param course_key: a CourseKey
:param role: an AccessRole
"""
if GlobalStaff().has_user(user):
......
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