Merge pull request #29608 from edx/mikix/ls-masquerading
feat: add masquerading support to learning sequences
This commit is contained in:
@@ -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(
|
||||
|
||||
@@ -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,
|
||||
)
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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}'
|
||||
|
||||
@@ -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:
|
||||
|
||||
Reference in New Issue
Block a user