diff --git a/openedx/core/djangoapps/content/learning_sequences/api/permissions.py b/openedx/core/djangoapps/content/learning_sequences/api/permissions.py index ce1a646415705f45fa64b66f2330e4ded025e4e8..4315a0d12c247d640ce3ec10ab1f4b4724b6e3bd 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 19f41e88ff60bd21ae3dec794159143d07f38ade..6d269ae3d961d9dc75507d9412414075911a83cd 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 c187b55f586edf3ef051ebf5eb5978fbe706a3fa..2648575613a478c1207c368fe733abf6143f46b6 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. + 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 + exceptions otherwise. + + The "user" querystring param is expected to be a username, with a blank + value being interpreted as the anonymous user. """ - requested_username = request.GET.get("user") + target_username = request.GET.get("user") - # Simple case: No username passed in, so it's just the request.user - if not requested_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 - # 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 - - raise PermissionDenied( - "User {request.user.username} does not have permission to view course outline as {requested_username}" - ) + # 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): + 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