From 3d82ce18f9fc52d5359ca8bd63798662cde4d2e3 Mon Sep 17 00:00:00 2001 From: "Dave St.Germain" Date: Mon, 18 Nov 2019 12:47:41 -0500 Subject: [PATCH] Check for data download permission in report endpoints --- common/djangoapps/student/rules.py | 28 +++++++++++++++++++ .../lms/test_lms_instructor_dashboard.py | 4 ++- lms/djangoapps/instructor/tests/test_api.py | 9 +++++- lms/djangoapps/instructor/views/api.py | 23 +++++++-------- .../instructor/views/instructor_dashboard.py | 6 ++-- 5 files changed, 54 insertions(+), 16 deletions(-) create mode 100644 common/djangoapps/student/rules.py diff --git a/common/djangoapps/student/rules.py b/common/djangoapps/student/rules.py new file mode 100644 index 0000000000..c718d56af4 --- /dev/null +++ b/common/djangoapps/student/rules.py @@ -0,0 +1,28 @@ +""" +Django rules for student roles +""" +from __future__ import absolute_import + +import rules + +from lms.djangoapps.courseware.access import has_access +from openedx.core.djangoapps.waffle_utils import CourseWaffleFlag, WaffleFlag, WaffleFlagNamespace + +from .roles import CourseDataResearcherRole + +# Waffle flag to enable the separate course outline page and full width content. +RESEARCHER_ROLE = CourseWaffleFlag(WaffleFlagNamespace(name='instructor'), 'researcher') + + +@rules.predicate +def can_access_reports(user, course_id): + """ + Returns whether the user can access the course data downloads. + """ + is_staff = user.is_staff + if RESEARCHER_ROLE.is_enabled(course_id): + return is_staff or CourseDataResearcherRole(course_id).has_user(user) + else: + return is_staff or has_access(user, 'staff', course_id) + +rules.add_perm('student.can_research', can_access_reports) diff --git a/common/test/acceptance/tests/lms/test_lms_instructor_dashboard.py b/common/test/acceptance/tests/lms/test_lms_instructor_dashboard.py index 41a7e7a6e5..38ca243969 100644 --- a/common/test/acceptance/tests/lms/test_lms_instructor_dashboard.py +++ b/common/test/acceptance/tests/lms/test_lms_instructor_dashboard.py @@ -591,7 +591,9 @@ class DataDownloadsTest(BaseInstructorDashboardTest): def setUp(self): super(DataDownloadsTest, self).setUp() self.course_fixture = CourseFixture(**self.course_info).install() - self.instructor_username, self.instructor_id, __, __ = self.log_in_as_instructor() + self.instructor_username, self.instructor_id, __, __ = self.log_in_as_instructor( + course_access_roles=['data_researcher'] + ) instructor_dashboard_page = self.visit_instructor_dashboard() self.data_download_section = instructor_dashboard_page.select_data_download() diff --git a/lms/djangoapps/instructor/tests/test_api.py b/lms/djangoapps/instructor/tests/test_api.py index b1d56fd423..d73676470a 100644 --- a/lms/djangoapps/instructor/tests/test_api.py +++ b/lms/djangoapps/instructor/tests/test_api.py @@ -95,7 +95,10 @@ from student.models import ( get_retired_email_by_email, get_retired_username_by_username ) -from student.roles import CourseBetaTesterRole, CourseFinanceAdminRole, CourseInstructorRole, CourseSalesAdminRole +from student.roles import ( + CourseBetaTesterRole, CourseDataResearcherRole, CourseFinanceAdminRole, + CourseInstructorRole, CourseSalesAdminRole +) from student.tests.factories import AdminFactory, UserFactory from xmodule.fields import Date from xmodule.modulestore import ModuleStoreEnum @@ -555,6 +558,7 @@ class TestInstructorAPIDenyLevels(SharedModuleStoreTestCase, LoginEnrollmentTest staff_member = StaffFactory(course_key=self.course.id) CourseEnrollment.enroll(staff_member, self.course.id) CourseFinanceAdminRole(self.course.id).add_users(staff_member) + CourseDataResearcherRole(self.course.id).add_users(staff_member) self.client.login(username=staff_member.username, password='test') # Try to promote to forums admin - not working # update_forum_role(self.course.id, staff_member, FORUM_ROLE_ADMINISTRATOR, 'allow') @@ -593,6 +597,7 @@ class TestInstructorAPIDenyLevels(SharedModuleStoreTestCase, LoginEnrollmentTest CourseEnrollment.enroll(inst, self.course.id) CourseFinanceAdminRole(self.course.id).add_users(inst) + CourseDataResearcherRole(self.course.id).add_users(inst) self.client.login(username=inst.username, password='test') for endpoint, args in self.staff_level_endpoints: @@ -2602,6 +2607,7 @@ class TestInstructorAPILevelsDataDump(SharedModuleStoreTestCase, LoginEnrollment min_price=40) self.course_mode.save() self.instructor = InstructorFactory(course_key=self.course.id) + CourseDataResearcherRole(self.course.id).add_users(self.instructor) self.client.login(username=self.instructor.username, password='test') self.cart = Order.get_cart_for_user(self.instructor) self.coupon_code = 'abcde' @@ -3038,6 +3044,7 @@ class TestInstructorAPILevelsDataDump(SharedModuleStoreTestCase, LoginEnrollment 'max_size': 2, 'topics': [{'id': 'topic', 'name': 'Topic', 'description': 'A Topic'}] })) course_instructor = InstructorFactory(course_key=self.course.id) + CourseDataResearcherRole(self.course.id).add_users(course_instructor) self.client.login(username=course_instructor.username, password='test') url = reverse('get_students_features', kwargs={'course_id': text_type(self.course.id)}) diff --git a/lms/djangoapps/instructor/views/api.py b/lms/djangoapps/instructor/views/api.py index 457383e3f0..e876611eca 100644 --- a/lms/djangoapps/instructor/views/api.py +++ b/lms/djangoapps/instructor/views/api.py @@ -234,7 +234,7 @@ def require_post_params(*args, **kwargs): return decorator -def require_level(level): +def require_level(level, perm=None): """ Decorator with argument that requires an access level of the requesting user. If the requirement is not satisfied, returns an @@ -255,7 +255,8 @@ def require_level(level): request = args[0] course = get_course_by_id(CourseKey.from_string(kwargs['course_id'])) - if has_access(request.user, level, course): + if has_access(request.user, level, course) and \ + ((perm and request.user.has_perm(perm, course.id)) or not perm): return func(*args, **kwargs) else: return HttpResponseForbidden() @@ -1047,7 +1048,7 @@ def list_course_role_members(request, course_id): @require_POST @ensure_csrf_cookie @cache_control(no_cache=True, no_store=True, must_revalidate=True) -@require_level('staff') +@require_level('staff', perm='student.can_research') @common_exceptions_400 def get_problem_responses(request, course_id): """ @@ -1107,7 +1108,7 @@ def get_grading_config(request, course_id): @ensure_csrf_cookie @cache_control(no_cache=True, no_store=True, must_revalidate=True) -@require_level('staff') +@require_level('staff', perm='student.can_research') def get_sale_records(request, course_id, csv=False): # pylint: disable=unused-argument, redefined-outer-name """ return the summary of all sales records for a particular course @@ -1138,7 +1139,7 @@ def get_sale_records(request, course_id, csv=False): # pylint: disable=unused-a @ensure_csrf_cookie @cache_control(no_cache=True, no_store=True, must_revalidate=True) -@require_level('staff') +@require_level('staff', perm='student.can_research') def get_sale_order_records(request, course_id): # pylint: disable=unused-argument """ return the summary of all sales records for a particular course @@ -1286,7 +1287,7 @@ def get_issued_certificates(request, course_id): @require_POST @ensure_csrf_cookie @cache_control(no_cache=True, no_store=True, must_revalidate=True) -@require_level('staff') +@require_level('staff', perm='student.can_research') @common_exceptions_400 def get_students_features(request, course_id, csv=False): # pylint: disable=redefined-outer-name """ @@ -1933,7 +1934,7 @@ def spent_registration_codes(request, course_id): @ensure_csrf_cookie @cache_control(no_cache=True, no_store=True, must_revalidate=True) -@require_level('staff') +@require_level('staff', perm='student.can_research') def get_anon_ids(request, course_id): # pylint: disable=unused-argument """ Respond with 2-column CSV output of user-id, anonymized-user-id @@ -2564,7 +2565,7 @@ def list_report_downloads(request, course_id): @require_POST @ensure_csrf_cookie @cache_control(no_cache=True, no_store=True, must_revalidate=True) -@require_level('staff') +@require_level('staff', perm='student.can_research') @require_finance_admin def list_financial_report_downloads(_request, course_id): """ @@ -2586,7 +2587,7 @@ def list_financial_report_downloads(_request, course_id): @require_POST @ensure_csrf_cookie @cache_control(no_cache=True, no_store=True, must_revalidate=True) -@require_level('staff') +@require_level('staff', perm='student.can_research') @common_exceptions_400 def export_ora2_data(request, course_id): """ @@ -2604,7 +2605,7 @@ def export_ora2_data(request, course_id): @require_POST @ensure_csrf_cookie @cache_control(no_cache=True, no_store=True, must_revalidate=True) -@require_level('staff') +@require_level('staff', perm='student.can_research') @common_exceptions_400 def calculate_grades_csv(request, course_id): """ @@ -2622,7 +2623,7 @@ def calculate_grades_csv(request, course_id): @require_POST @ensure_csrf_cookie @cache_control(no_cache=True, no_store=True, must_revalidate=True) -@require_level('staff') +@require_level('staff', perm='student.can_research') @common_exceptions_400 def problem_grade_report(request, course_id): """ diff --git a/lms/djangoapps/instructor/views/instructor_dashboard.py b/lms/djangoapps/instructor/views/instructor_dashboard.py index dffcacc649..4171c3c3f0 100644 --- a/lms/djangoapps/instructor/views/instructor_dashboard.py +++ b/lms/djangoapps/instructor/views/instructor_dashboard.py @@ -57,7 +57,7 @@ from openedx.core.lib.xblock_utils import wrap_xblock from shoppingcart.models import Coupon, CourseRegCodeItem, PaidCourseRegistration from student.models import CourseEnrollment from student.roles import ( - CourseDataResearcherRole, CourseFinanceAdminRole, CourseInstructorRole, + CourseFinanceAdminRole, CourseInstructorRole, CourseSalesAdminRole, CourseStaffRole ) from util.json_request import JsonResponse @@ -121,7 +121,7 @@ def instructor_dashboard_2(request, course_id): 'sales_admin': CourseSalesAdminRole(course_key).has_user(request.user), 'staff': bool(has_access(request.user, 'staff', course)), 'forum_admin': has_forum_access(request.user, course_key, FORUM_ROLE_ADMINISTRATOR), - 'data_researcher': CourseDataResearcherRole(course_key).has_user(request.user), + 'data_researcher': request.user.has_perm('student.can_research', course_key), } if not access['staff']: @@ -713,7 +713,7 @@ def _section_data_download(course, access): ), 'export_ora2_data_url': reverse('export_ora2_data', kwargs={'course_id': six.text_type(course_key)}), } - if not (access.get('data_researcher') or access.get('admin')): + if not access.get('data_researcher'): section_data['is_hidden'] = True return section_data