Skip to content

E
edx-platform
Commit 3e1e4a4b by Don Mitchell
Options

Merge pull request #6123 from mitocw/bdero/move-course-role-check-to-student 

Moved has_course_access from CMS to student.auth (common)
parents 699b3d08 046e2953
Showing with 73 additions and 70 deletions
cms/djangoapps/contentstore/tests/test_clone_course.py
......@@ -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 contentstore.views.access 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)
......
cms/djangoapps/contentstore/tests/test_permissions.py
......@@ -10,7 +10,6 @@ from contentstore.tests.utils import AjaxEnabledTestClient
from opaque_keys.edx.locations import SlashSeparatedCourseKey
from contentstore.utils import reverse_url, reverse_course_url
from student.roles import CourseInstructorRole, CourseStaffRole, OrgStaffRole, OrgInstructorRole
from contentstore.views.access import has_course_access
from student import auth
......@@ -88,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(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)
......@@ -126,9 +125,9 @@ class TestCourseAccess(ModuleStoreTestCase):
if hasattr(user, '_roles'):
del user._roles
self.assertTrue(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(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))
cms/djangoapps/contentstore/views/access.py
""" Helper methods for determining user access permissions in Studio """
from student.roles import CourseStaffRole, GlobalStaff, CourseInstructorRole, OrgStaffRole, OrgInstructorRole
from student.roles import CourseInstructorRole
from student import auth
def has_course_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
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
"""
if GlobalStaff().has_user(user):
return True
if OrgInstructorRole(org=course_key.org).has_user(user):
return True
if OrgStaffRole(org=course_key.org).has_user(user):
return True
# temporary to ensure we give universal access given a course until we impl branch specific perms
return auth.has_access(user, role(course_key.for_branch(None)))
def get_user_role(user, course_id):
"""
What type of access: staff or instructor does this user have in Studio?
......
cms/djangoapps/contentstore/views/assets.py
......@@ -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 .access 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')
......
cms/djangoapps/contentstore/views/checklist.py
......@@ -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 .access 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)
......
cms/djangoapps/contentstore/views/component.py
......@@ -25,7 +25,7 @@ from contentstore.views.item import create_xblock_info
from opaque_keys.edx.keys import UsageKey
from .access 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)
......
cms/djangoapps/contentstore/views/course.py
......@@ -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 .access 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':
......
cms/djangoapps/contentstore/views/export_git.py
......@@ -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 .access 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)
......
cms/djangoapps/contentstore/views/import_export.py
......@@ -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 .access 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)
......
cms/djangoapps/contentstore/views/item.py
......@@ -37,7 +37,7 @@ from util.date_utils import get_default_time_display
from util.json_request import expect_json, JsonResponse
from .access 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()
......
cms/djangoapps/contentstore/views/tabs.py
"""
Views related to course tabs
"""
from access 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)
......
cms/djangoapps/contentstore/views/tests/test_course_index.py
......@@ -7,7 +7,7 @@ import datetime
from contentstore.tests.utils import CourseTestCase
from contentstore.utils import reverse_course_url, add_instructor
from contentstore.views.access 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):
"""
......
cms/djangoapps/contentstore/views/transcripts_ajax.py
......@@ -38,7 +38,7 @@ from xmodule.video_module.transcripts_utils import (
TranscriptsRequestValidationException
)
from .access 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
cms/djangoapps/contentstore/views/user.py
......@@ -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 .access 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")
}
......
common/djangoapps/student/auth.py
......@@ -8,7 +8,7 @@ from django.core.exceptions import PermissionDenied
from django.conf import settings
from student.roles import GlobalStaff, CourseCreatorRole, CourseStaffRole, CourseInstructorRole, CourseRole, \
CourseBetaTesterRole
CourseBetaTesterRole, OrgInstructorRole, OrgStaffRole
def has_access(user, role):
......@@ -40,6 +40,30 @@ def has_access(user, role):
return False
def has_course_author_access(user, course_key, role=CourseStaffRole):
"""
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 CourseKey
:param role: an AccessRole
"""
if GlobalStaff().has_user(user):
return True
if OrgInstructorRole(org=course_key.org).has_user(user):
return True
if OrgStaffRole(org=course_key.org).has_user(user):
return True
# temporary to ensure we give universal access given a course until we impl branch specific perms
return has_access(user, role(course_key.for_branch(None)))
def add_users(caller, role, *users):
"""
The caller requests adding the given users to the role. Checks that the caller
......
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