From caf75dc0d985935159074b303fb5c899de6d0ece Mon Sep 17 00:00:00 2001 From: Kyle McCormick Date: Mon, 28 Jun 2021 15:51:36 -0400 Subject: [PATCH] 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). --- .../learning_sequences/api/permissions.py | 19 ++- .../learning_sequences/tests/test_views.py | 152 ++++++++++++++++-- .../content/learning_sequences/views.py | 69 +++++--- 3 files changed, 208 insertions(+), 32 deletions(-) diff --git a/openedx/core/djangoapps/content/learning_sequences/api/permissions.py b/openedx/core/djangoapps/content/learning_sequences/api/permissions.py index ce1a646415..4315a0d12c 100644 --- a/openedx/core/djangoapps/content/learning_sequences/api/permissions.py +++ b/openedx/core/djangoapps/content/learning_sequences/api/permissions.py @@ -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) diff --git a/openedx/core/djangoapps/content/learning_sequences/tests/test_views.py b/openedx/core/djangoapps/content/learning_sequences/tests/test_views.py index 19f41e88ff..6d269ae3d9 100644 --- a/openedx/core/djangoapps/content/learning_sequences/tests/test_views.py +++ b/openedx/core/djangoapps/content/learning_sequences/tests/test_views.py @@ -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}' diff --git a/openedx/core/djangoapps/content/learning_sequences/views.py b/openedx/core/djangoapps/content/learning_sequences/views.py index c187b55f58..2648575613 100644 --- a/openedx/core/djangoapps/content/learning_sequences/views.py +++ b/openedx/core/djangoapps/content/learning_sequences/views.py @@ -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