feat: allow course staff to view outlines as students
This adds support for course staff masquerading as any other user when
viewing their own course, via user={username} querystring parameter.
Rules:
* Anonymous users are repesented by a blank "user" value.
* If there is no "user" parameter at all, render for the user making the
request.
* Global staff can view any course as any user.
* Course staff and instructors can view their own course as any user
(including other staff, the anonymous user, or users not enrolled in
their course).
This commit supports TNL-8330 (switching the frontend-app-learning MFE
to use the Learning Sequences API instead if Course Blocks).
This commit is contained in:
committed by
David Ormsbee
parent
e72119ddeb
commit
caf75dc0d9
@@ -5,16 +5,19 @@ Most access rules determining what a user will see are determined within the
|
||||
outline processors themselves. This is where we'd put permissions that are used
|
||||
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 openedx.core import types
|
||||
|
||||
from ..toggles import USE_FOR_OUTLINES
|
||||
|
||||
|
||||
def can_call_public_api(requesting_user, course_key):
|
||||
def can_call_public_api(requesting_user: types.User, course_key: CourseKey) -> bool:
|
||||
"""
|
||||
Global staff can always call the public API. Otherwise, check waffle flag.
|
||||
|
||||
@@ -24,7 +27,7 @@ def can_call_public_api(requesting_user, course_key):
|
||||
return GlobalStaff().has_user(requesting_user) or USE_FOR_OUTLINES.is_enabled(course_key)
|
||||
|
||||
|
||||
def can_see_all_content(requesting_user, course_key):
|
||||
def can_see_all_content(requesting_user: types.User, course_key: CourseKey) -> bool:
|
||||
"""
|
||||
Global staff, course staff, and instructors can see everything.
|
||||
|
||||
@@ -35,3 +38,15 @@ def can_see_all_content(requesting_user, course_key):
|
||||
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)
|
||||
|
||||
@@ -14,18 +14,26 @@ Where possible, seed data using public API methods (e.g. replace_course_outline
|
||||
from this app, edx-when's set_dates_for_course).
|
||||
"""
|
||||
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 rest_framework.test import APITestCase, APIClient
|
||||
|
||||
from openedx.core.djangolib.testing.utils import CacheIsolationTestCase
|
||||
from common.djangoapps.student.roles import CourseInstructorRole, CourseStaffRole
|
||||
from common.djangoapps.student.tests.factories import UserFactory
|
||||
|
||||
from ..api import replace_course_outline
|
||||
from ..data import CourseOutlineData, CourseVisibility
|
||||
from ..api.tests.test_data import generate_sections
|
||||
from ..data import CourseOutlineData, CourseVisibility
|
||||
from ..toggles import USE_FOR_OUTLINES
|
||||
|
||||
|
||||
class CourseOutlineViewTest(CacheIsolationTestCase, APITestCase): # lint-amnesty, pylint: disable=missing-class-docstring
|
||||
class CourseOutlineViewTest(CacheIsolationTestCase, APITestCase):
|
||||
"""
|
||||
General tests for the CourseOutline.
|
||||
"""
|
||||
|
||||
@classmethod
|
||||
def setUpTestData(cls): # lint-amnesty, pylint: disable=super-method-not-called
|
||||
@@ -36,7 +44,7 @@ class CourseOutlineViewTest(CacheIsolationTestCase, APITestCase): # lint-amnest
|
||||
username='student', email='student@example.com', is_staff=False, password='student_pass'
|
||||
)
|
||||
cls.course_key = CourseKey.from_string("course-v1:OpenEdX+Seq+View")
|
||||
cls.course_url = cls.url_for(cls.course_key)
|
||||
cls.course_url = outline_url(cls.course_key)
|
||||
cls.outline = CourseOutlineData(
|
||||
course_key=cls.course_key,
|
||||
title="Views Test Course!",
|
||||
@@ -54,10 +62,6 @@ class CourseOutlineViewTest(CacheIsolationTestCase, APITestCase): # lint-amnest
|
||||
super().setUp()
|
||||
self.client = APIClient()
|
||||
|
||||
@classmethod
|
||||
def url_for(cls, course_key):
|
||||
return f'/api/learning_sequences/v1/course_outline/{course_key}'
|
||||
|
||||
def test_student_access_denied(self):
|
||||
"""
|
||||
For now, make sure you need staff access bits to use the API.
|
||||
@@ -74,7 +78,7 @@ class CourseOutlineViewTest(CacheIsolationTestCase, APITestCase): # lint-amnest
|
||||
"""
|
||||
self.client.login(username='staff', password='staff_pass')
|
||||
fake_course_key = self.course_key.replace(run="not_real")
|
||||
result = self.client.get(self.url_for(fake_course_key))
|
||||
result = self.client.get(outline_url(fake_course_key))
|
||||
assert result.status_code == 404
|
||||
|
||||
def test_deprecated_course_key(self):
|
||||
@@ -85,7 +89,7 @@ class CourseOutlineViewTest(CacheIsolationTestCase, APITestCase): # lint-amnest
|
||||
"""
|
||||
self.client.login(username='staff', password='staff_pass')
|
||||
old_course_key = CourseKey.from_string("OldOrg/OldCourse/OldRun")
|
||||
result = self.client.get(self.url_for(old_course_key))
|
||||
result = self.client.get(outline_url(old_course_key))
|
||||
assert result.status_code == 400
|
||||
|
||||
def test_outline_as_staff(self):
|
||||
@@ -121,3 +125,133 @@ class CourseOutlineViewTest(CacheIsolationTestCase, APITestCase): # lint-amnest
|
||||
data = result.data
|
||||
assert data['username'] == 'student'
|
||||
assert data['user_id'] == self.student.id
|
||||
|
||||
|
||||
@ddt.ddt
|
||||
class CourseOutlineViewMasqueradingTest(CacheIsolationTestCase, APITestCase):
|
||||
"""
|
||||
Tests permissions of masquerading.
|
||||
"""
|
||||
@classmethod
|
||||
def setUpTestData(cls): # lint-amnesty, pylint: disable=super-method-not-called
|
||||
"""Set up the basic course outline and our users."""
|
||||
# This is the course that we're creating staff for.
|
||||
cls.course_key = CourseKey.from_string("course-v1:OpenEdX+Masq+StaffAccess")
|
||||
|
||||
# This is the course that our users are not course staff for
|
||||
cls.not_staff_course_key = CourseKey.from_string("course-v1:OpenEdX+Masq+NoStaffAccess")
|
||||
|
||||
for course_key in [cls.course_key, cls.not_staff_course_key]:
|
||||
outline = CourseOutlineData(
|
||||
course_key=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(course_key, [2, 2]),
|
||||
self_paced=False,
|
||||
course_visibility=CourseVisibility.PUBLIC
|
||||
)
|
||||
replace_course_outline(outline)
|
||||
|
||||
# Users
|
||||
cls.global_staff = UserFactory.create(
|
||||
username='global_staff',
|
||||
email='global_staff@example.com',
|
||||
is_staff=True,
|
||||
password='global_staff_pass',
|
||||
)
|
||||
cls.course_instructor = UserFactory.create(
|
||||
username='course_instructor',
|
||||
email='instructor@example.com',
|
||||
is_staff=False,
|
||||
password='course_instructor_pass',
|
||||
)
|
||||
cls.course_staff = UserFactory.create(
|
||||
username='course_staff',
|
||||
email='course_staff@example.com',
|
||||
is_staff=False,
|
||||
password='course_staff_pass',
|
||||
)
|
||||
cls.student = UserFactory.create(
|
||||
username='student',
|
||||
email='student@example.com',
|
||||
is_staff=False,
|
||||
password='student_pass',
|
||||
)
|
||||
|
||||
# Roles
|
||||
CourseInstructorRole(cls.course_key).add_users(cls.course_instructor)
|
||||
CourseStaffRole(cls.course_key).add_users(cls.course_staff)
|
||||
|
||||
def setUp(self):
|
||||
super().setUp()
|
||||
self.client = APIClient()
|
||||
|
||||
@override_waffle_flag(USE_FOR_OUTLINES, active=True)
|
||||
def test_global_staff(self):
|
||||
"""Global staff can successfuly masquerade in both courses."""
|
||||
self.client.login(username='global_staff', password='global_staff_pass')
|
||||
for course_key in [self.course_key, self.not_staff_course_key]:
|
||||
result = self.client.get(outline_url(course_key), {'user': 'student'})
|
||||
assert result.status_code == 200
|
||||
assert result.data['username'] == 'student'
|
||||
|
||||
@override_waffle_flag(USE_FOR_OUTLINES, active=True)
|
||||
@ddt.data(
|
||||
('course_instructor', 'course_instructor_pass'),
|
||||
('course_staff', 'course_staff_pass'),
|
||||
)
|
||||
@ddt.unpack
|
||||
def test_course_staff(self, username, password):
|
||||
"""Course Instructors/Staff can only masquerade for their own course."""
|
||||
self.client.login(username=username, password=password)
|
||||
our_course_url = outline_url(self.course_key)
|
||||
our_course_as_student = self.client.get(our_course_url, {'user': 'student'})
|
||||
assert our_course_as_student.status_code == 200
|
||||
assert our_course_as_student.data['username'] == 'student'
|
||||
|
||||
our_course_as_self = self.client.get(our_course_url)
|
||||
assert our_course_as_self.status_code == 200
|
||||
assert our_course_as_self.data['username'] == username
|
||||
|
||||
our_course_as_anonymous = self.client.get(our_course_url, {'user': ''})
|
||||
assert our_course_as_anonymous.status_code == 200
|
||||
assert our_course_as_anonymous.data['username'] == ''
|
||||
|
||||
our_course_as_missing_user = self.client.get(our_course_url, {'user': 'idontexist'})
|
||||
assert our_course_as_missing_user.status_code == 404
|
||||
|
||||
# No permission to masquerade in the course we're not a staff member of.
|
||||
other_course_as_student = self.client.get(
|
||||
outline_url(self.not_staff_course_key), {'user': 'student'}
|
||||
)
|
||||
assert other_course_as_student.status_code == 403
|
||||
|
||||
@override_waffle_flag(USE_FOR_OUTLINES, active=True)
|
||||
def test_student(self):
|
||||
"""Students have no ability to masquerade."""
|
||||
self.client.login(username='student', password='student_pass')
|
||||
course_url = outline_url(self.course_key)
|
||||
|
||||
# No query param should net us the result as ourself
|
||||
course_as_self_implicit = self.client.get(course_url)
|
||||
assert course_as_self_implicit.status_code == 200
|
||||
assert course_as_self_implicit.data['username'] == 'student'
|
||||
|
||||
# We should be allowed to explicitly put ourselves here...
|
||||
course_as_self_implicit = self.client.get(course_url, {'user': 'student'})
|
||||
assert course_as_self_implicit.status_code == 200
|
||||
assert course_as_self_implicit.data['username'] == 'student'
|
||||
|
||||
# Any attempt to masquerade as anyone else (including anonymous users or
|
||||
# non-existent users) results in a 403.
|
||||
for username in ['idontexist', '', 'course_staff', 'global_staff']:
|
||||
masq_attempt_result = self.client.get(course_url, {'user': username})
|
||||
assert masq_attempt_result.status_code == 403
|
||||
|
||||
|
||||
def outline_url(course_key):
|
||||
"""Helper: Course outline URL for a given course key."""
|
||||
return f'/api/learning_sequences/v1/course_outline/{course_key}'
|
||||
|
||||
@@ -7,8 +7,10 @@ import logging
|
||||
|
||||
from django.conf import settings
|
||||
from django.contrib.auth import get_user_model
|
||||
from django.contrib.auth.models import AnonymousUser
|
||||
from edx_rest_framework_extensions.auth.jwt.authentication import JwtAuthentication
|
||||
from edx_rest_framework_extensions.auth.session.authentication import SessionAuthenticationAllowInactiveUser
|
||||
from opaque_keys.edx.keys import CourseKey
|
||||
from rest_framework import serializers
|
||||
from rest_framework.exceptions import NotFound, PermissionDenied
|
||||
from rest_framework.response import Response
|
||||
@@ -16,7 +18,7 @@ from rest_framework.views import APIView
|
||||
|
||||
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
|
||||
from .api.permissions import can_call_public_api, can_see_content_as_other_users
|
||||
from .data import CourseOutlineData
|
||||
|
||||
User = get_user_model()
|
||||
@@ -163,7 +165,7 @@ class CourseOutlineView(APIView):
|
||||
|
||||
try:
|
||||
# Grab the user's outline and send our response...
|
||||
outline_user = self._determine_user(request)
|
||||
outline_user = self._determine_user(request, course_key)
|
||||
user_course_outline_details = get_user_course_outline_details(course_key, outline_user, at_time)
|
||||
except CourseOutlineData.DoesNotExist as does_not_exist_err:
|
||||
raise NotFound() from does_not_exist_err
|
||||
@@ -171,27 +173,52 @@ class CourseOutlineView(APIView):
|
||||
serializer = self.UserCourseOutlineDataSerializer(user_course_outline_details)
|
||||
return Response(serializer.data)
|
||||
|
||||
def _determine_user(self, request):
|
||||
def _determine_user(self, request, course_key: CourseKey) -> User:
|
||||
"""
|
||||
Requesting for a different user (easiest way to test for students)
|
||||
while restricting access to only global staff. This is a placeholder
|
||||
until we have more full fledged permissions/masquerading.
|
||||
"""
|
||||
requested_username = request.GET.get("user")
|
||||
For which user should we get an outline?
|
||||
|
||||
# Simple case: No username passed in, so it's just the request.user
|
||||
if not requested_username:
|
||||
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
|
||||
exceptions otherwise.
|
||||
|
||||
The "user" querystring param is expected to be a username, with a blank
|
||||
value being interpreted as the anonymous user.
|
||||
"""
|
||||
target_username = request.GET.get("user")
|
||||
|
||||
# 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
|
||||
|
||||
# If we're here, then the requesting user is asking for someone else's
|
||||
# course outline. Right now, only global staff is allowed to do that.
|
||||
if request.user.is_staff:
|
||||
try:
|
||||
user = User.objects.get(username=requested_username)
|
||||
return user
|
||||
except User.DoesNotExist as err:
|
||||
raise NotFound("User {requested_username} does not exist.") from err
|
||||
# Users can always see the outline as themselves.
|
||||
if target_username == request.user.username:
|
||||
return request.user
|
||||
|
||||
raise PermissionDenied(
|
||||
"User {request.user.username} does not have permission to view course outline as {requested_username}"
|
||||
)
|
||||
# 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):
|
||||
display_username = "the anonymous user" if target_username == "" else target_username
|
||||
raise PermissionDenied(
|
||||
f"User {request.user.username} does not have permission to "
|
||||
f"view course outline for {course_key} as {display_username}."
|
||||
)
|
||||
|
||||
# If we've gotten this far, their permissions are fine. Now we handle
|
||||
# the masquerade 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.
|
||||
try:
|
||||
target_user = User.objects.get(username=target_username)
|
||||
except User.DoesNotExist as err:
|
||||
# We throw this only after we've passed the permission check, to
|
||||
# make it harder for crawlers to get a list of our usernames.
|
||||
raise NotFound(f"User {target_username} does not exist.") from err
|
||||
|
||||
return target_user
|
||||
|
||||
Reference in New Issue
Block a user