Andal.LND/edx-platform
6
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge pull request #21285 from cpennington/masquerade-permission

Browse Source 
Switch courseware.masquerade_as_student over to using a StaffAccessRu…
This commit is contained in:
Calen Pennington
2019-08-05 10:46:58 -04:00
committed by GitHub
parent 54269f3dfa 7bbf4ad15c
commit 0c01c6f056
10 changed files with 144 additions and 27 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
lms/djangoapps/ccx/tests/test_field_override_performance.py
View File

@@ -244,7 +244,7 @@ class TestFieldOverrideMongoPerformance(FieldOverridePerformanceTestCase):
__test__ = True
# TODO: decrease query count as part of REVO-28
QUERY_COUNT = 35
QUERY_COUNT = 36
TEST_DATA = {
# (providers, course_width, enable_ccx, view_as_ccx): (
# # of sql queries to default,
@@ -273,7 +273,7 @@ class TestFieldOverrideSplitPerformance(FieldOverridePerformanceTestCase):
__test__ = True
# TODO: decrease query count as part of REVO-28
QUERY_COUNT = 35
QUERY_COUNT = 36
TEST_DATA = {
('no_overrides', 1, True, False): (QUERY_COUNT, 3),

4
lms/djangoapps/courseware/permissions.py
View File

@@ -3,14 +3,16 @@ Permission definitions for the courseware djangoapp
"""
from bridgekeeper import perms
from .rules import HasAccessRule
from .rules import HasAccessRule, HasStaffAccessToContent
EDIT_BOOKMARK = 'courseware.edit_bookmark'
MASQUERADE_AS_STUDENT = 'courseware.masquerade_as_student'
VIEW_COURSE_HOME = 'courseware.view_course_home'
VIEW_COURSEWARE = 'courseware.view_courseware'
VIEW_XQA_INTERFACE = 'courseware.view_xqa_interface'
perms[EDIT_BOOKMARK] = HasAccessRule('staff')
perms[MASQUERADE_AS_STUDENT] = HasStaffAccessToContent()
perms[VIEW_COURSE_HOME] = HasAccessRule('load')
perms[VIEW_COURSEWARE] = HasAccessRule('load')
perms[VIEW_XQA_INTERFACE] = HasAccessRule('staff')

111
lms/djangoapps/courseware/rules.py
View File

@@ -3,17 +3,30 @@ django-rules and Bridgekeeper rules for courseware related features
"""
from __future__ import absolute_import
from bridgekeeper.rules import Rule
from django.db.models import Q
from opaque_keys.edx.keys import CourseKey
import logging
import laboratory
import rules
from bridgekeeper.rules import Rule, EMPTY
from course_modes.models import CourseMode
from student.models import CourseEnrollment
from django.conf import settings
from django.db.models import Q
from opaque_keys.edx.django.models import CourseKeyField
from opaque_keys.edx.keys import CourseKey, UsageKey
from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
from student.models import CourseEnrollment, CourseAccessRole
from xblock.core import XBlock
from xmodule.course_module import CourseDescriptor
from xmodule.error_module import ErrorDescriptor
from xmodule.x_module import XModule
from .access import has_access
LOG = logging.getLogger(__name__)
@rules.predicate
def is_track_ok_for_exam(user, exam):
"""
@@ -46,3 +59,93 @@ class HasAccessRule(Rule):
# that is used to determine if the rule should allow a user
# into django admin
return Q(pk__in=[])
class StaffAccessExperiment(laboratory.Experiment):
def compare(self, control, candidate):
return bool(control) == candidate
def publish(self, result):
if not result.match:
LOG.warning(
u"StaffAccessExperiment: control=%r, candidate=%r",
result.control,
result.candidates[0],
exc_info=True
)
class HasStaffAccessToContent(Rule):
"""
Check whether a user has `staff` access in a course.
Expects to be used to filter a CourseOverview queryset
"""
def check(self, user, instance=None):
"""
Return True if the supplied user has staff-level access to the supplied content.
"""
staff_sql_experiment = StaffAccessExperiment(
raise_on_mismatch=settings.DEBUG,
context={'userid': user.id, 'instance': repr(instance)}
)
staff_sql_experiment.control(self._check_with_has_access, args=(user, instance))
staff_sql_experiment.candidate(self._check_with_query, args=(user, instance))
return staff_sql_experiment.conduct()
def _check_with_has_access(self, user, instance=None):
return has_access(user, 'staff', instance)
def _check_with_query(self, user, instance=None):
"""
Use the query method to check whether a single user has access to the supplied object.
"""
# delegate the work to type-specific functions.
# (start with more specific types, then get more general)
if isinstance(instance, (CourseDescriptor, CourseOverview)):
course_key = instance.id
elif isinstance(instance, (ErrorDescriptor, XModule, XBlock)):
course_key = instance.scope_ids.usage_id.course_key
elif isinstance(instance, CourseKey):
course_key = instance
elif isinstance(instance, UsageKey):
course_key = instance.course_key
elif isinstance(instance, basestring):
course_key = CourseKey.from_string(instance)
return self.filter(user, CourseOverview.objects.filter(id=course_key)).exists()
def query(self, user):
"""
Returns a Q object that expects to be used to filter CourseOverview queries.
"""
if not user.is_authenticated:
return EMPTY
masq_settings = getattr(user, 'masquerade_settings', {})
masq_as_student = [
course_key for
(course_key, masq_setting) in masq_settings.items()
if masq_setting.role == 'student'
]
not_masquerading_as_student = ~Q(id__in=masq_as_student)
is_global_staff = user.is_staff
course_staff_or_instructor_courses = CourseAccessRole.objects.filter(
user=user,
role__in=('staff', 'instructor')
).exclude(
course_id=CourseKeyField.Empty,
).values('course_id')
org_staff_or_instructor_courses = CourseAccessRole.objects.filter(
user=user,
role__in=('staff', 'instructor'),
course_id=CourseKeyField.Empty,
org__isnull=False
).values('org')
query = not_masquerading_as_student
if not is_global_staff:
query &= Q(id__in=course_staff_or_instructor_courses) | Q(org__in=org_staff_or_instructor_courses)
return query

4
lms/djangoapps/courseware/tests/test_course_info.py
View File

@@ -434,8 +434,8 @@ class SelfPacedCourseInfoTestCase(LoginEnrollmentTestCase, SharedModuleStoreTest
def test_num_queries_instructor_paced(self):
# TODO: decrease query count as part of REVO-28
self.fetch_course_info_with_queries(self.instructor_paced_course, 44, 3)
self.fetch_course_info_with_queries(self.instructor_paced_course, 45, 3)
def test_num_queries_self_paced(self):
# TODO: decrease query count as part of REVO-28
self.fetch_course_info_with_queries(self.self_paced_course, 44, 3)
self.fetch_course_info_with_queries(self.self_paced_course, 45, 3)

12
lms/djangoapps/courseware/tests/test_views.py
View File

@@ -224,8 +224,8 @@ class IndexQueryTestCase(ModuleStoreTestCase):
NUM_PROBLEMS = 20
@ddt.data(
(ModuleStoreEnum.Type.mongo, 10, 181),
(ModuleStoreEnum.Type.split, 4, 175),
(ModuleStoreEnum.Type.mongo, 10, 182),
(ModuleStoreEnum.Type.split, 4, 176),
)
@ddt.unpack
def test_index_query_counts(self, store_type, expected_mongo_query_count, expected_mysql_query_count):
@@ -1466,8 +1466,8 @@ class ProgressPageTests(ProgressPageBaseTests):
self.assertContains(resp, u"Download Your Certificate")
@ddt.data(
(True, 55),
(False, 54)
(True, 56),
(False, 55)
)
@ddt.unpack
def test_progress_queries_paced_courses(self, self_paced, query_count):
@@ -1480,8 +1480,8 @@ class ProgressPageTests(ProgressPageBaseTests):
@patch.dict(settings.FEATURES, {'ASSUME_ZERO_GRADE_IF_ABSENT_FOR_ALL_TESTS': False})
@ddt.data(
(False, 63, 43),
(True, 54, 38)
(False, 64, 44),
(True, 55, 39)
)
@ddt.unpack
def test_progress_queries(self, enable_waffle, initial, subsequent):

7
lms/djangoapps/courseware/views/index.py
View File

@@ -66,8 +66,11 @@ from ..entrance_exams import (
from ..masquerade import check_content_start_date_for_masquerade_user, setup_masquerade
from ..model_data import FieldDataCache
from ..module_render import get_module_for_descriptor, toc_for_course
from ..permissions import MASQUERADE_AS_STUDENT
from .views import CourseTabView
log = logging.getLogger("edx.courseware.views.index")
TEMPLATE_IMPORTS = {'urllib': urllib}
@@ -152,6 +155,7 @@ class CoursewareIndex(View):
# If the user is considered enrolled show the default XBlock student_view.
pass
self.can_masquerade = request.user.has_perm(MASQUERADE_AS_STUDENT, self.course)
self.is_staff = has_access(request.user, 'staff', self.course)
self._setup_masquerade_for_effective_user()
@@ -168,7 +172,7 @@ class CoursewareIndex(View):
self.masquerade, self.effective_user = setup_masquerade(
self.request,
self.course_key,
self.is_staff,
self.can_masquerade,
reset_masquerade_data=True
)
# Set the user in the request to the effective user.
@@ -415,6 +419,7 @@ class CoursewareIndex(View):
'init': '',
'fragment': Fragment(),
'staff_access': self.is_staff,
'can_masquerade': self.can_masquerade,
'masquerade': self.masquerade,
'supports_preview_menu': True,
'studio_url': get_studio_url(self.course, 'course'),

23
lms/djangoapps/courseware/views/views.py
View File

@@ -63,7 +63,7 @@ from courseware.courses import (
from courseware.masquerade import setup_masquerade
from courseware.model_data import FieldDataCache
from courseware.models import BaseStudentModuleHistory, StudentModule
from courseware.permissions import VIEW_COURSE_HOME, VIEW_COURSEWARE, VIEW_XQA_INTERFACE
from courseware.permissions import MASQUERADE_AS_STUDENT, VIEW_COURSE_HOME, VIEW_COURSEWARE, VIEW_XQA_INTERFACE
from courseware.url_helpers import get_redirect_url
from courseware.user_state_client import DjangoXBlockUserStateClient
from edxmako.shortcuts import marketing_link, render_to_response, render_to_string
@@ -356,8 +356,8 @@ def course_info(request, course_id):
with modulestore().bulk_operations(course_key):
course = get_course_with_access(request.user, 'load', course_key)
staff_access = has_access(request.user, 'staff', course)
masquerade, user = setup_masquerade(request, course_key, staff_access, reset_masquerade_data=True)
can_masquerade = request.user.has_perm(MASQUERADE_AS_STUDENT, course)
masquerade, user = setup_masquerade(request, course_key, can_masquerade, reset_masquerade_data=True)
# LEARNER-612: CCX redirect handled by new Course Home (DONE)
# LEARNER-1697: Transition banner messages to new Course Home (DONE)
@@ -433,7 +433,7 @@ def course_info(request, course_id):
'course_subtitle': course_subtitle,
'show_subtitle': course_homepage_show_subtitle,
'show_org': course_homepage_show_org,
'staff_access': staff_access,
'can_masquerade': can_masquerade,
'masquerade': masquerade,
'supports_preview_menu': True,
'studio_url': get_studio_url(course, 'course_info'),
@@ -646,11 +646,16 @@ class CourseTabView(EdxFragmentView):
"""
Creates the context for the fragment's template.
"""
staff_access = has_access(request.user, 'staff', course)
can_masquerade = request.user.has_perm(MASQUERADE_AS_STUDENT, course)
supports_preview_menu = tab.get('supports_preview_menu', False)
uses_bootstrap = self.uses_bootstrap(request, course, tab=tab)
if supports_preview_menu:
masquerade, masquerade_user = setup_masquerade(request, course.id, staff_access, reset_masquerade_data=True)
masquerade, masquerade_user = setup_masquerade(
request,
course.id,
can_masquerade,
reset_masquerade_data=True,
)
request.user = masquerade_user
else:
masquerade = None
@@ -659,7 +664,7 @@ class CourseTabView(EdxFragmentView):
'course': course,
'tab': tab,
'active_page': tab.get('type', None),
'staff_access': staff_access,
'can_masquerade': can_masquerade,
'masquerade': masquerade,
'supports_preview_menu': supports_preview_menu,
'uses_bootstrap': uses_bootstrap,
@@ -989,11 +994,12 @@ def _progress(request, course_key, student_id):
course = get_course_with_access(request.user, 'load', course_key)
staff_access = bool(has_access(request.user, 'staff', course))
can_masquerade = request.user.has_perm(MASQUERADE_AS_STUDENT, course)
masquerade = None
if student_id is None or student_id == request.user.id:
# This will be a no-op for non-staff users, returning request.user
masquerade, student = setup_masquerade(request, course_key, staff_access, reset_masquerade_data=True)
masquerade, student = setup_masquerade(request, course_key, can_masquerade, reset_masquerade_data=True)
else:
try:
coach_access = has_ccx_coach_role(request.user, course_key)
@@ -1036,6 +1042,7 @@ def _progress(request, course_key, student_id):
'courseware_summary': courseware_summary,
'studio_url': studio_url,
'grade_summary': course_grade.summary,
'can_masquerade': can_masquerade,
'staff_access': staff_access,
'masquerade': masquerade,
'supports_preview_menu': True,

2
lms/templates/preview_menu.html
View File

@@ -11,7 +11,7 @@ from xmodule.partitions.partitions_service import get_all_partitions_for_course
%>
<%
show_preview_menu = course and staff_access and supports_preview_menu
show_preview_menu = course and can_masquerade and supports_preview_menu
%>
% if show_preview_menu:

2
openedx/features/course_experience/tests/views/test_course_home.py
View File

@@ -217,7 +217,7 @@ class TestCourseHomePage(CourseHomePageTestCase):
# Fetch the view and verify the query counts
# TODO: decrease query count as part of REVO-28
with self.assertNumQueries(94, table_blacklist=QUERY_COUNT_TABLE_BLACKLIST):
with self.assertNumQueries(95, table_blacklist=QUERY_COUNT_TABLE_BLACKLIST):
with check_mongo_calls(4):
url = course_home_url(self.course)
self.client.get(url)

2
openedx/features/course_experience/tests/views/test_course_updates.py
View File

@@ -134,7 +134,7 @@ class TestCourseUpdatesPage(SharedModuleStoreTestCase):
# Fetch the view and verify that the query counts haven't changed
# TODO: decrease query count as part of REVO-28
with self.assertNumQueries(55, table_blacklist=QUERY_COUNT_TABLE_BLACKLIST):
with self.assertNumQueries(56, table_blacklist=QUERY_COUNT_TABLE_BLACKLIST):
with check_mongo_calls(4):
url = course_updates_url(self.course)
self.client.get(url)