From 046e2953224fcfa82f0725ac3c02b18ab7988f55 Mon Sep 17 00:00:00 2001 From: Brandon DeRosier Date: Tue, 2 Dec 2014 22:08:58 -0500 Subject: [PATCH] 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 --- .../contentstore/tests/test_clone_course.py | 4 ++-- .../contentstore/tests/test_permissions.py | 8 ++++---- cms/djangoapps/contentstore/views/assets.py | 4 ++-- cms/djangoapps/contentstore/views/checklist.py | 4 ++-- cms/djangoapps/contentstore/views/component.py | 4 ++-- cms/djangoapps/contentstore/views/course.py | 16 ++++++++-------- cms/djangoapps/contentstore/views/export_git.py | 4 ++-- .../contentstore/views/import_export.py | 8 ++++---- cms/djangoapps/contentstore/views/item.py | 12 ++++++------ cms/djangoapps/contentstore/views/tabs.py | 4 ++-- .../views/tests/test_course_index.py | 4 ++-- .../contentstore/views/transcripts_ajax.py | 8 ++++---- cms/djangoapps/contentstore/views/user.py | 14 +++++++------- common/djangoapps/student/auth.py | 10 +++++----- 14 files changed, 52 insertions(+), 52 deletions(-) diff --git a/cms/djangoapps/contentstore/tests/test_clone_course.py b/cms/djangoapps/contentstore/tests/test_clone_course.py index 7fe8e10bd2..b95b01c800 100644 --- a/cms/djangoapps/contentstore/tests/test_clone_course.py +++ b/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 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) diff --git a/cms/djangoapps/contentstore/tests/test_permissions.py b/cms/djangoapps/contentstore/tests/test_permissions.py index 29c8df9249..5a2274cf1c 100644 --- a/cms/djangoapps/contentstore/tests/test_permissions.py +++ b/cms/djangoapps/contentstore/tests/test_permissions.py @@ -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)) diff --git a/cms/djangoapps/contentstore/views/assets.py b/cms/djangoapps/contentstore/views/assets.py index 63e96046b5..5618807996 100644 --- a/cms/djangoapps/contentstore/views/assets.py +++ b/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 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') diff --git a/cms/djangoapps/contentstore/views/checklist.py b/cms/djangoapps/contentstore/views/checklist.py index 18d332bd67..d4bc570728 100644 --- a/cms/djangoapps/contentstore/views/checklist.py +++ b/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 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) diff --git a/cms/djangoapps/contentstore/views/component.py b/cms/djangoapps/contentstore/views/component.py index 8c9cbebe70..04e513b1cd 100644 --- a/cms/djangoapps/contentstore/views/component.py +++ b/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 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) diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index e0fbd6ffbc..e05d13fed8 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/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 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': diff --git a/cms/djangoapps/contentstore/views/export_git.py b/cms/djangoapps/contentstore/views/export_git.py index b7678305a8..f3cc5d04f3 100644 --- a/cms/djangoapps/contentstore/views/export_git.py +++ b/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 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) diff --git a/cms/djangoapps/contentstore/views/import_export.py b/cms/djangoapps/contentstore/views/import_export.py index 18010468af..b4078668d4 100644 --- a/cms/djangoapps/contentstore/views/import_export.py +++ b/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 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) diff --git a/cms/djangoapps/contentstore/views/item.py b/cms/djangoapps/contentstore/views/item.py index 60f9f59ae4..dcf44bb084 100644 --- a/cms/djangoapps/contentstore/views/item.py +++ b/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 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() diff --git a/cms/djangoapps/contentstore/views/tabs.py b/cms/djangoapps/contentstore/views/tabs.py index 7d15cd3b89..131be1d946 100644 --- a/cms/djangoapps/contentstore/views/tabs.py +++ b/cms/djangoapps/contentstore/views/tabs.py @@ -1,7 +1,7 @@ """ 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) diff --git a/cms/djangoapps/contentstore/views/tests/test_course_index.py b/cms/djangoapps/contentstore/views/tests/test_course_index.py index 9bc6e3aacc..b0ea997b4d 100644 --- a/cms/djangoapps/contentstore/views/tests/test_course_index.py +++ b/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 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): """ diff --git a/cms/djangoapps/contentstore/views/transcripts_ajax.py b/cms/djangoapps/contentstore/views/transcripts_ajax.py index ae68509b97..d957c8ff59 100644 --- a/cms/djangoapps/contentstore/views/transcripts_ajax.py +++ b/cms/djangoapps/contentstore/views/transcripts_ajax.py @@ -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 diff --git a/cms/djangoapps/contentstore/views/user.py b/cms/djangoapps/contentstore/views/user.py index b1e7029a12..c73ecc651f 100644 --- a/cms/djangoapps/contentstore/views/user.py +++ b/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 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") } diff --git a/common/djangoapps/student/auth.py b/common/djangoapps/student/auth.py index b68055696c..1a0caac948 100644 --- a/common/djangoapps/student/auth.py +++ b/common/djangoapps/student/auth.py @@ -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):