diff --git a/lms/djangoapps/course_home_api/outline/v1/tests/test_views.py b/lms/djangoapps/course_home_api/outline/v1/tests/test_views.py index a5ab954c35..bc8436f72e 100644 --- a/lms/djangoapps/course_home_api/outline/v1/tests/test_views.py +++ b/lms/djangoapps/course_home_api/outline/v1/tests/test_views.py @@ -3,7 +3,7 @@ Tests for Outline Tab API in the Course Home API """ import itertools -from datetime import datetime +from datetime import datetime, timezone from unittest.mock import Mock, patch import ddt @@ -18,6 +18,9 @@ from common.djangoapps.student.tests.factories import UserFactory from lms.djangoapps.course_home_api.tests.utils import BaseCourseHomeTests from lms.djangoapps.course_home_api.toggles import COURSE_HOME_MICROFRONTEND from lms.djangoapps.experiments.testutils import override_experiment_waffle_flag +from openedx.core.djangoapps.content.learning_sequences.api import replace_course_outline +from openedx.core.djangoapps.content.learning_sequences.data import CourseOutlineData, CourseVisibility +from openedx.core.djangoapps.content.learning_sequences.toggles import USE_FOR_OUTLINES from openedx.core.djangoapps.course_date_signals.utils import MIN_DURATION from openedx.core.djangoapps.user_api.preferences.api import set_user_preference from openedx.core.djangoapps.user_api.tests.factories import UserCourseTagFactory @@ -366,3 +369,39 @@ class OutlineTabTestViews(BaseCourseHomeTests): response = self.client.get(self.url) assert response.data['verified_mode'] == {'access_expiration_date': (enrollment.created + MIN_DURATION), 'currency': 'USD', 'currency_symbol': '$', 'price': 149, 'sku': 'ABCD1234', 'upgrade_url': '/dashboard'} + + @override_experiment_waffle_flag(COURSE_HOME_MICROFRONTEND, active=True) + def test_hide_learning_sequences(self): + """ + Check that Learning Sequences filters out sequences. + """ + CourseEnrollment.enroll(self.user, self.course.id, CourseMode.VERIFIED) + + # Normal behavior: the sequence exists + response = self.client.get(self.url) + assert response.status_code == 200 + blocks = response.data['course_blocks']['blocks'] + seq_block_id = next( + block_id + for block_id, block in blocks.items() + if block['type'] == 'sequential' + ) + + # With Learning Sequences active and a course outline loaded, the same + # sequence is removed. + with override_waffle_flag(USE_FOR_OUTLINES, active=True): + new_learning_seq_outline = CourseOutlineData( + course_key=self.course.id, + title="Test Course Outline!", + published_at=datetime(2021, 6, 14, tzinfo=timezone.utc), + published_version="5ebece4b69dd593d82fe2022", + entrance_exam_id=None, + days_early_for_beta=None, + sections=[], + self_paced=False, + course_visibility=CourseVisibility.PRIVATE + ) + replace_course_outline(new_learning_seq_outline) + response = self.client.get(self.url) + blocks = response.data['course_blocks']['blocks'] + assert seq_block_id not in blocks diff --git a/lms/djangoapps/course_home_api/outline/v1/views.py b/lms/djangoapps/course_home_api/outline/v1/views.py index 877d4d1ade..2c7c40d3b1 100644 --- a/lms/djangoapps/course_home_api/outline/v1/views.py +++ b/lms/djangoapps/course_home_api/outline/v1/views.py @@ -1,6 +1,7 @@ """ Outline Tab Views """ +from datetime import datetime, timezone from completion.exceptions import UnavailableCompletionData from completion.utilities import get_key_to_last_completed_block @@ -37,6 +38,10 @@ from lms.djangoapps.courseware.courses import get_course_date_blocks, get_course from lms.djangoapps.courseware.date_summary import TodaysDate from lms.djangoapps.courseware.masquerade import is_masquerading, setup_masquerade from lms.djangoapps.courseware.views.views import get_cert_data +from openedx.core.djangoapps.content.learning_sequences.api import ( + get_user_course_outline, + public_api_available as learning_sequences_api_available, +) from openedx.core.djangoapps.content.course_overviews.models import CourseOverview from openedx.core.lib.api.authentication import BearerAuthenticationAllowInactiveUser from openedx.features.course_duration_limits.access import get_access_expiration_data @@ -277,6 +282,35 @@ class OutlineTabView(RetrieveAPIView): elif course.invitation_only: enroll_alert['can_enroll'] = False + # Sometimes there are sequences returned by Course Blocks that we + # don't actually want to show to the user, such as when a sequence is + # composed entirely of units that the user can't access. The Learning + # Sequences API knows how to roll this up, so we use it determine which + # sequences we should remove from course_blocks. + # + # The long term goal is to remove the Course Blocks API call entirely, + # so this is a tiny first step in that migration. + if course_blocks and learning_sequences_api_available(course_key, request.user): + user_course_outline = get_user_course_outline( + course_key, request.user, datetime.now(tz=timezone.utc) + ) + available_seq_ids = {str(usage_key) for usage_key in user_course_outline.sequences} + + # course_blocks is a reference to the root of the course, so we go + # through the chapters (sections) to look for sequences to remove. + for chapter_data in course_blocks['children']: + chapter_data['children'] = [ + seq_data + for seq_data in chapter_data['children'] + if ( + seq_data['id'] in available_seq_ids or + # Edge case: Sometimes we have weird course structures. + # We expect only sequentials here, but if there is + # another type, just skip it (don't filter it out). + seq_data['type'] != 'sequential' + ) + ] + data = { 'access_expiration': access_expiration, 'cert_data': cert_data, diff --git a/openedx/core/djangoapps/content/learning_sequences/api/__init__.py b/openedx/core/djangoapps/content/learning_sequences/api/__init__.py index 2ab8283a6e..bbede54893 100644 --- a/openedx/core/djangoapps/content/learning_sequences/api/__init__.py +++ b/openedx/core/djangoapps/content/learning_sequences/api/__init__.py @@ -6,5 +6,6 @@ from .outlines import ( get_user_course_outline, get_user_course_outline_details, key_supports_outlines, + public_api_available, replace_course_outline, ) diff --git a/openedx/core/djangoapps/content/learning_sequences/api/outlines.py b/openedx/core/djangoapps/content/learning_sequences/api/outlines.py index c08796e707..ffe2ac68e4 100644 --- a/openedx/core/djangoapps/content/learning_sequences/api/outlines.py +++ b/openedx/core/djangoapps/content/learning_sequences/api/outlines.py @@ -39,7 +39,7 @@ from ..models import ( PublishReport, UserPartitionGroup ) -from .permissions import can_see_all_content +from .permissions import can_call_public_api, can_see_all_content from .processors.content_gating import ContentGatingOutlineProcessor from .processors.enrollment import EnrollmentOutlineProcessor from .processors.enrollment_track_partition_groups import EnrollmentTrackPartitionGroupsOutlineProcessor @@ -58,6 +58,7 @@ __all__ = [ 'get_user_course_outline', 'get_user_course_outline_details', 'key_supports_outlines', + 'public_api_available', 'replace_course_outline', ] @@ -84,6 +85,21 @@ def key_supports_outlines(opaque_key: OpaqueKey) -> bool: return False +def public_api_available(course_key: CourseKey, user: types.User) -> bool: + """ + Is the Public API available for this Course to this User? + + This only really exists while we do the waffle-flag rollout of this feature, + so that in-process callers from other apps can determine whether they should + trust Learning Sequences API data for a particular user/course. + """ + return ( + key_supports_outlines(course_key) and + LearningContext.objects.filter(context_key=course_key).exists() and + can_call_public_api(user, course_key) + ) + + @function_trace('learning_sequences.api.get_course_keys_with_outlines') def get_course_keys_with_outlines() -> QuerySet: """ diff --git a/openedx/core/djangoapps/content/learning_sequences/api/permissions.py b/openedx/core/djangoapps/content/learning_sequences/api/permissions.py index a6874903a0..ce1a646415 100644 --- a/openedx/core/djangoapps/content/learning_sequences/api/permissions.py +++ b/openedx/core/djangoapps/content/learning_sequences/api/permissions.py @@ -1,8 +1,37 @@ """ -This is a dummy placeholder for now, as access to this endpoint is currently -limited to global staff. +Simple permissions for Learning Sequences. + +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 common.djangoapps.student.roles import ( + GlobalStaff, + CourseInstructorRole, + CourseStaffRole, +) + +from ..toggles import USE_FOR_OUTLINES -def can_see_all_content(requesting_user, _course_key): - return requesting_user.is_staff +def can_call_public_api(requesting_user, course_key): + """ + Global staff can always call the public API. Otherwise, check waffle flag. + + This is only intended for rollout purposes, and eventually everyone will be + able to call the public API for all courses. + """ + return GlobalStaff().has_user(requesting_user) or USE_FOR_OUTLINES.is_enabled(course_key) + + +def can_see_all_content(requesting_user, course_key): + """ + Global staff, course staff, and instructors can see everything. + + 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) + ) diff --git a/openedx/core/djangoapps/content/learning_sequences/api/tests/test_outlines.py b/openedx/core/djangoapps/content/learning_sequences/api/tests/test_outlines.py index e4dd66d614..b0442dcc80 100644 --- a/openedx/core/djangoapps/content/learning_sequences/api/tests/test_outlines.py +++ b/openedx/core/djangoapps/content/learning_sequences/api/tests/test_outlines.py @@ -3,20 +3,22 @@ Top level API tests. Tests API public contracts only. Do not import/create/mock models for this app. """ from datetime import datetime, timezone -from unittest import TestCase from unittest.mock import patch +import unittest -import ddt -import pytest -import attr from django.conf import settings from django.contrib.auth.models import AnonymousUser, User # lint-amnesty, pylint: disable=imported-auth-user from django.db.models import signals from edx_proctoring.exceptions import ProctoredExamNotFoundException +from edx_toggles.toggles.testutils import override_waffle_flag from edx_when.api import set_dates_for_course from opaque_keys.edx.keys import CourseKey from opaque_keys.edx.locator import LibraryLocator -from edx_toggles.toggles.testutils import override_waffle_flag +import attr +import ddt +import django.test +import pytest + from openedx.core.djangolib.testing.utils import CacheIsolationTestCase from openedx.features.course_experience import COURSE_ENABLE_UNENROLLED_ACCESS_FLAG from common.djangoapps.course_modes.models import CourseMode @@ -38,19 +40,21 @@ from ...data import ( VisibilityData, ) +from ...toggles import USE_FOR_OUTLINES from ..outlines import ( get_content_errors, get_course_outline, get_user_course_outline, get_user_course_outline_details, key_supports_outlines, + public_api_available, replace_course_outline, ) from ..processors.enrollment_track_partition_groups import EnrollmentTrackPartitionGroupsOutlineProcessor from .test_data import generate_sections -class OutlineSupportTestCase(TestCase): +class OutlineSupportTestCase(unittest.TestCase): """ Make sure we know what kinds of course-like keys we support for outlines. """ @@ -63,6 +67,62 @@ class OutlineSupportTestCase(TestCase): assert not key_supports_outlines(LibraryLocator(org="edX", library="100")) +class PublicApiAvailableTestCase(django.test.TestCase): + """ + Test API availability checks. + """ + @classmethod + def setUpTestData(cls): # lint-amnesty, pylint: disable=super-method-not-called + """ + Create an empty course outline. + """ + cls.course_key = CourseKey.from_string("course-v1:OpenEdX+Learn+Roundtrip") + cls.course_outline = CourseOutlineData( + course_key=cls.course_key, + title="PublicApiAvailableTestCase Test Course!", + published_at=datetime(2021, 6, 14, tzinfo=timezone.utc), + published_version="5ebece4b69dd593d82fe2021", + entrance_exam_id=None, + days_early_for_beta=None, + sections=[], + self_paced=False, + course_visibility=CourseVisibility.PRIVATE + ) + replace_course_outline(cls.course_outline) + + cls.global_staff = User.objects.create_user( + 'global_staff', email='gstaff@example.com', is_staff=True + ) + cls.student = User.objects.create_user( + 'student', email='student@example.com', is_staff=False + ) + cls.fake_course_1 = CourseKey.from_string("course-v1:Not+Really+Here") + cls.fake_course_2 = CourseKey.from_string("Also/Not/Here") + + def test_flag_inactive(self): + # Old Mongo and non-existent courses are always unavailable + for user in [self.global_staff, self.student]: + assert not public_api_available(self.fake_course_1, user) + assert not public_api_available(self.fake_course_2, user) + + # Since the waffle flag is off, only global staff can use the Learning + # Sequences API. + assert public_api_available(self.course_key, self.global_staff) + assert not public_api_available(self.course_key, self.student) + + @override_waffle_flag(USE_FOR_OUTLINES, active=True) + def test_flag_active(self): + # Old Mongo and non-existent courses are always unavailable + for user in [self.global_staff, self.student]: + assert not public_api_available(self.fake_course_1, user) + assert not public_api_available(self.fake_course_2, user) + + # Since the waffle flag is on, both global staff and students can use + # the Learning Sequences API. + assert public_api_available(self.course_key, self.global_staff) + assert public_api_available(self.course_key, self.student) + + class CourseOutlineTestCase(CacheIsolationTestCase): """ Simple tests around reading and writing CourseOutlineData. No user info. diff --git a/openedx/core/djangoapps/content/learning_sequences/toggles.py b/openedx/core/djangoapps/content/learning_sequences/toggles.py new file mode 100644 index 0000000000..746854496c --- /dev/null +++ b/openedx/core/djangoapps/content/learning_sequences/toggles.py @@ -0,0 +1,24 @@ +""" +Rollout waffle flags for the learning_sequences API. +""" +from openedx.core.djangoapps.waffle_utils import CourseWaffleFlag + + +WAFFLE_NAMESPACE = 'learning_sequences' + +# .. toggle_name: learning_sequences.use_for_outlines +# .. toggle_implementation: CourseWaffleFlag +# .. toggle_description: Waffle flag to enable the use of the Learning Sequences +# Course Outline API (/api/learning_sequences/v1/course_outline/{course_key}). +# Staff can always use this endpoint. If you are a student and this endpoint +# is not enabled, it will return a 403 error. The Courseware MFE should know +# how to detect this condition. +# This flag is also used to determine what is returned by the +# public_api_available learning_sequences API function, though other apps +# calling this API are always able to ignore this result and call any +# learning_sequences API directly (e.g. get_course_outline). +# .. toggle_default: False +# .. toggle_use_cases: temporary, open_edx +# .. toggle_creation_date: 2021-06-07 +# .. toggle_target_removal_date: 2020-08-01 +USE_FOR_OUTLINES = CourseWaffleFlag(WAFFLE_NAMESPACE, 'use_for_outlines', __name__) diff --git a/openedx/core/djangoapps/content/learning_sequences/views.py b/openedx/core/djangoapps/content/learning_sequences/views.py index 5af03aaa21..69abe46985 100644 --- a/openedx/core/djangoapps/content/learning_sequences/views.py +++ b/openedx/core/djangoapps/content/learning_sequences/views.py @@ -12,12 +12,12 @@ from edx_rest_framework_extensions.auth.session.authentication import SessionAut from opaque_keys import InvalidKeyError from opaque_keys.edx.keys import CourseKey from rest_framework import serializers -from rest_framework.exceptions import NotFound +from rest_framework.exceptions import NotFound, PermissionDenied from rest_framework.response import Response from rest_framework.views import APIView -from openedx.core.lib.api.permissions import IsStaff from .api import get_user_course_outline_details +from .api.permissions import can_call_public_api from .data import CourseOutlineData User = get_user_model() @@ -31,9 +31,6 @@ class CourseOutlineView(APIView): # We want to eventually allow unauthenticated users to use this as well... authentication_classes = (JwtAuthentication, SessionAuthenticationAllowInactiveUser) - # For early testing, restrict this to only global staff... - permission_classes = (IsStaff,) - class UserCourseOutlineDataSerializer(serializers.BaseSerializer): # lint-amnesty, pylint: disable=abstract-method """ Read-only serializer for CourseOutlineData for this endpoint. @@ -154,36 +151,35 @@ class CourseOutlineView(APIView): """ The CourseOutline, customized for a given user. - Currently restricted to global staff. - TODO: Swagger docs of API. For an exemplar to imitate, see: https://github.com/edx/edx-platform/blob/master/lms/djangoapps/program_enrollments/rest_api/v1/views.py#L792-L820 """ - # Translate input params and do any substitutions... + # Translate input params and do course key validation (will cause HTTP + # 400 error if an invalid CourseKey was entered, instead of 404). course_key = self._validate_course_key(course_key_str) at_time = datetime.now(timezone.utc) - user = self._determine_user(request) - # Grab the user's outline and send our response... + if not can_call_public_api(request.user, course_key): + raise PermissionDenied() + try: - user_course_outline_details = get_user_course_outline_details(course_key, user, at_time) + # Grab the user's outline and send our response... + outline_user = self._determine_user(request) + 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 serializer = self.UserCourseOutlineDataSerializer(user_course_outline_details) return Response(serializer.data) - def _validate_course_key(self, course_key_str): # lint-amnesty, pylint: disable=missing-function-docstring + def _validate_course_key(self, course_key_str): + """Validate the Course Key and raise a ValidationError if it fails.""" try: course_key = CourseKey.from_string(course_key_str) - except InvalidKeyError: - raise serializers.ValidationError( # lint-amnesty, pylint: disable=raise-missing-from - f"{course_key_str} is not a valid CourseKey" - ) + except InvalidKeyError as err: + raise serializers.ValidationError(f"{course_key_str} is not a valid CourseKey") from err if course_key.deprecated: - raise serializers.ValidationError( - "Deprecated CourseKeys (Org/Course/Run) are not supported." - ) + raise serializers.ValidationError("Deprecated CourseKeys (Org/Course/Run) are not supported.") return course_key def _determine_user(self, request): @@ -193,7 +189,20 @@ class CourseOutlineView(APIView): until we have more full fledged permissions/masquerading. """ requested_username = request.GET.get("user") - if request.user.is_staff and requested_username: - return User.objects.get(username=requested_username) - return request.user + # Simple case: No username passed in, so it's just the request.user + if not requested_username: + 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}" + )