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

feat: add masquerading support to learning sequences

Browse Source 
Previously, it had some basic manual masquerading by calling the
endpoint with ?user=mytestuser. But this adds standard session
masquerading support to the endpoint as well.

This support is limited by LS's own partition group support. It
only looks at the enrollment track partition currently. Further
FBE and cohort partition support will come later.

But this commit opens up normal session masquerading for:
- Generic student
- Specific student
- Enrollment track

AA-1151
This commit is contained in:
Michael Terry
2021-12-14 14:54:42 -05:00
parent 2ef69d513f
commit 4418c66171
5 changed files with 103 additions and 48 deletions
Download Patch File Download Diff File Expand all files Collapse all files

10
lms/djangoapps/courseware/tests/helpers.py
View File

@@ -331,14 +331,16 @@ class MasqueradeMixin:
to pass in the course parameter below.
"""
def update_masquerade(self, course=None, role='student', group_id=None, username=None, user_partition_id=None):
def update_masquerade(self, course=None, course_id=None, role='student', group_id=None, username=None,
user_partition_id=None):
"""
Installs a masquerade for the specified user and course, to enable
the user to masquerade as belonging to the specific partition/group
combination.
Arguments:
course (object): a course or None for self.course
course (object): a course or None for self.course (or you can pass course_id instead)
course_id (str|CourseKey): a course id, useful if you don't happen to have a full course object handy
user_partition_id (int): the integer partition id, referring to partitions already
configured in the course.
group_id (int); the integer group id, within the specified partition.
@@ -347,11 +349,11 @@ class MasqueradeMixin:
Returns: the response object for the AJAX call to update the user's masquerade.
"""
course = course or self.course
course_id = str(course_id or (course and course.id) or self.course.id)
masquerade_url = reverse(
'masquerade_update',
kwargs={
'course_key_string': str(course.id),
'course_key_string': course_id,
}
)
response = self.client.post(

2
lms/djangoapps/courseware/tests/test_module_render.py
View File

@@ -2630,7 +2630,6 @@ class LmsModuleSystemShimTest(SharedModuleStoreTestCase):
self.descriptor,
self.course.id,
self.track_function,
self.xqueue_callback_url_prefix,
self.request_token,
course=self.course,
)
@@ -2703,7 +2702,6 @@ class LmsModuleSystemShimTest(SharedModuleStoreTestCase):
self.descriptor,
self.course.id,
self.track_function,
self.xqueue_callback_url_prefix,
self.request_token,
course=self.course,
)

25
openedx/core/djangoapps/content/learning_sequences/api/permissions.py
View File

@@ -7,11 +7,7 @@ to determine whether those processors even need to be run to filter the results.
"""
from opaque_keys.edx.keys import CourseKey
from common.djangoapps.student.roles import (
GlobalStaff,
CourseInstructorRole,
CourseStaffRole,
)
from lms.djangoapps.courseware.access import has_access
from openedx.core import types
from ..toggles import USE_FOR_OUTLINES
@@ -31,20 +27,5 @@ def can_see_all_content(requesting_user: types.User, course_key: CourseKey) -> b
There's no need to run processors to restrict results for these users.
"""
return (
GlobalStaff().has_user(requesting_user) or
CourseStaffRole(course_key).has_user(requesting_user) or
CourseInstructorRole(course_key).has_user(requesting_user)
)
def can_see_content_as_other_users(requesting_user: types.User, course_key: CourseKey) -> bool:
"""
Is this user allowed to view this content as other users?
For now, this is the same set of people who are allowed to see all content
(i.e. some kind of course or global staff). It's possible that we'll want to
make more granular distinctions between different kinds of staff roles in
the future (e.g. CourseDataResearcher).
"""
return can_see_all_content(requesting_user, course_key)
# has_access handles all possible staff cases, including checking for masquerading
return has_access(requesting_user, 'staff', course_key).has_access

67
openedx/core/djangoapps/content/learning_sequences/tests/test_views.py
View File

@@ -17,12 +17,15 @@ from datetime import datetime, timezone
import ddt
from edx_toggles.toggles.testutils import override_waffle_flag
from opaque_keys.edx.keys import CourseKey # lint-amnesty, pylint: disable=unused-import
from opaque_keys.edx.keys import CourseKey
from rest_framework.test import APITestCase, APIClient
from openedx.core.djangolib.testing.utils import CacheIsolationTestCase
from common.djangoapps.student.models import CourseEnrollment
from common.djangoapps.student.roles import CourseInstructorRole, CourseStaffRole
from common.djangoapps.student.tests.factories import UserFactory
from lms.djangoapps.courseware.tests.helpers import MasqueradeMixin
from openedx.core.djangoapps.content.course_overviews.tests.factories import CourseOverviewFactory
from openedx.core.djangolib.testing.utils import CacheIsolationTestCase, skip_unless_lms
from ..api import replace_course_outline
from ..api.tests.test_data import generate_sections
@@ -30,6 +33,7 @@ from ..data import CourseOutlineData, CourseVisibility
from ..toggles import USE_FOR_OUTLINES
@skip_unless_lms
@override_waffle_flag(USE_FOR_OUTLINES, active=True)
class CourseOutlineViewTest(CacheIsolationTestCase, APITestCase):
"""
@@ -137,9 +141,10 @@ class CourseOutlineViewTest(CacheIsolationTestCase, APITestCase):
@ddt.ddt
class CourseOutlineViewMasqueradingTest(CacheIsolationTestCase, APITestCase):
@skip_unless_lms
class CourseOutlineViewTargetUserTest(CacheIsolationTestCase, APITestCase):
"""
Tests permissions of masquerading.
Tests permissions of specifying a target user via url parameter.
"""
@classmethod
def setUpTestData(cls): # lint-amnesty, pylint: disable=super-method-not-called
@@ -261,6 +266,60 @@ class CourseOutlineViewMasqueradingTest(CacheIsolationTestCase, APITestCase):
assert masq_attempt_result.status_code == 403
@ddt.ddt
@skip_unless_lms
@override_waffle_flag(USE_FOR_OUTLINES, active=True)
class CourseOutlineViewMasqueradingTest(MasqueradeMixin, CacheIsolationTestCase):
"""
Tests permissions of session masquerading.
"""
@classmethod
def setUpTestData(cls):
"""Set up the basic course outline and our users."""
super().setUpTestData()
overview = CourseOverviewFactory()
cls.course_key = overview.id
outline = CourseOutlineData(
course_key=cls.course_key,
title="Views Test Course!",
published_at=datetime(2020, 5, 20, tzinfo=timezone.utc),
published_version="5ebece4b69dd593d82fe2020",
entrance_exam_id=None,
days_early_for_beta=None,
sections=generate_sections(cls.course_key, [2, 2]),
self_paced=False,
course_visibility=CourseVisibility.PUBLIC
)
replace_course_outline(outline)
# Users
cls.staff = UserFactory(is_staff=True, password='test')
cls.student = UserFactory(username='student')
UserFactory(username='student2')
CourseEnrollment.enroll(cls.student, cls.course_key)
def setUp(self):
super().setUp()
self.client.login(username=self.staff.username, password='test')
def test_masquerading_works(self):
"""Confirm that session masquerading works as expected."""
self.update_masquerade(course_id=self.course_key, username='student')
result = self.client.get(outline_url(self.course_key))
assert result.status_code == 200
assert result.data['username'] == 'student'
def test_target_user_takes_precedence(self):
"""Specifying a user should override any masquerading."""
self.update_masquerade(course_id=self.course_key, username='student')
result = self.client.get(outline_url(self.course_key), {'user': 'student2'})
assert result.status_code == 200
assert result.data['username'] == 'student2'
def outline_url(course_key):
"""Helper: Course outline URL for a given course key."""
return f'/api/learning_sequences/v1/course_outline/{course_key}'

47
openedx/core/djangoapps/content/learning_sequences/views.py
View File

@@ -16,11 +16,13 @@ from rest_framework.exceptions import NotAuthenticated, NotFound, PermissionDeni
from rest_framework.response import Response
from rest_framework.views import APIView
from lms.djangoapps.courseware.access import has_access
from lms.djangoapps.courseware.masquerade import setup_masquerade
from openedx.core import types
from openedx.core.lib.api.view_utils import validate_course_key
from .api import get_user_course_outline_details
from .api.permissions import can_call_public_api, can_see_content_as_other_users
from .api.permissions import can_call_public_api
from .data import CourseOutlineData
User = get_user_model()
@@ -162,6 +164,9 @@ class CourseOutlineView(APIView):
course_key = validate_course_key(course_key_str)
at_time = datetime.now(timezone.utc)
# Get target user (and override request user for the benefit of any waffle checks)
request.user = self._determine_user(request, course_key)
# We use can_call_public_api to slowly roll this feature out, and be
# able to turn it off for a course. But it's not really a permissions
# thing in that it doesn't give them elevated access. If I had it to do
@@ -177,13 +182,11 @@ class CourseOutlineView(APIView):
if (not force_on) and (not can_call_public_api(course_key)):
raise PermissionDenied()
outline_user = self._determine_user(request, course_key)
try:
# Grab the user's outline and send our response...
user_course_outline_details = get_user_course_outline_details(course_key, outline_user, at_time)
user_course_outline_details = get_user_course_outline_details(course_key, request.user, at_time)
except CourseOutlineData.DoesNotExist as does_not_exist_err:
if not outline_user.id:
if not request.user.id:
# Outline is private or doesn't exist. But don't leak whether a course exists or not to anonymous
# users with a 404 - give a 401 instead. This mostly prevents drive-by crawlers from creating a bunch
# of 404 errors in your error report dashboard.
@@ -197,28 +200,40 @@ class CourseOutlineView(APIView):
"""
For which user should we get an outline?
Uses a combination of the user on the request object and a manually
passed in "user" parameter. Ensures that the requesting user has
permission to view course outline of target user. Raise request-level
Uses a combination of the user on the request object, session masquerading
data, and a manually passed in "user" parameter. Ensures that the requesting
user has permission to view course outline of target user. Raise request-level
exceptions otherwise.
The "user" querystring param is expected to be a username, with a blank
value being interpreted as the anonymous user.
value being interpreted as the anonymous user. It will take priority over
session masquerading, if provided.
"""
has_staff_access = has_access(request.user, 'staff', course_key).has_access
target_username = request.GET.get("user")
if target_username is not None:
return self._get_target_user(request, course_key, has_staff_access, target_username)
# Sending no "user" querystring param at all defaults us to the user who
# is making the request.
if target_username is None:
return request.user
_course_masquerade, user = setup_masquerade(request, course_key, has_staff_access)
return user
@staticmethod
def _get_target_user(request, course_key: CourseKey, has_staff_access: bool, target_username: str) -> types.User:
"""
Load and return the requested user with permission checking.
A blank username will return an anonymous user.
This was designed for manual API testing and kept in, as it may be useful for future development.
"""
# Users can always see the outline as themselves.
if target_username == request.user.username:
return request.user
# Otherwise, do a permission check to see if they're allowed to view as
# other users.
if not can_see_content_as_other_users(request.user, course_key):
if not has_staff_access:
display_username = "the anonymous user" if target_username == "" else target_username
raise PermissionDenied(
f"User {request.user.username} does not have permission to "
@@ -226,14 +241,14 @@ class CourseOutlineView(APIView):
)
# If we've gotten this far, their permissions are fine. Now we handle
# the masquerade use case...
# the different-user use case...
# Having a "user" querystring that is a blank string is interpreted as
# "show me this outline as an anonymous user".
if target_username == "":
return AnonymousUser()
# Finally, the actual work of looking up a user to masquerade as.
# Finally, the actual work of looking up a user to target.
try:
target_user = User.objects.get(username=target_username)
except User.DoesNotExist as err: