diff --git a/lms/djangoapps/course_home_api/outline/v1/views.py b/lms/djangoapps/course_home_api/outline/v1/views.py index 8da16e2290..697461a001 100644 --- a/lms/djangoapps/course_home_api/outline/v1/views.py +++ b/lms/djangoapps/course_home_api/outline/v1/views.py @@ -296,7 +296,7 @@ class OutlineTabView(RetrieveAPIView): # # 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): + if course_blocks and learning_sequences_api_available(course_key): user_course_outline = get_user_course_outline( course_key, request.user, datetime.now(tz=timezone.utc) ) diff --git a/openedx/core/djangoapps/content/learning_sequences/api/outlines.py b/openedx/core/djangoapps/content/learning_sequences/api/outlines.py index fc7c3b3460..218fd7d504 100644 --- a/openedx/core/djangoapps/content/learning_sequences/api/outlines.py +++ b/openedx/core/djangoapps/content/learning_sequences/api/outlines.py @@ -85,7 +85,7 @@ def key_supports_outlines(opaque_key: OpaqueKey) -> bool: return False -def public_api_available(course_key: CourseKey, user: types.User) -> bool: +def public_api_available(course_key: CourseKey) -> bool: """ Is the Public API available for this Course to this User? @@ -96,7 +96,7 @@ def public_api_available(course_key: CourseKey, user: types.User) -> bool: return ( key_supports_outlines(course_key) and LearningContext.objects.filter(context_key=course_key).exists() and - can_call_public_api(user, course_key) + can_call_public_api(course_key) ) diff --git a/openedx/core/djangoapps/content/learning_sequences/api/permissions.py b/openedx/core/djangoapps/content/learning_sequences/api/permissions.py index 4315a0d12c..156e1d1678 100644 --- a/openedx/core/djangoapps/content/learning_sequences/api/permissions.py +++ b/openedx/core/djangoapps/content/learning_sequences/api/permissions.py @@ -17,14 +17,12 @@ from openedx.core import types from ..toggles import USE_FOR_OUTLINES -def can_call_public_api(requesting_user: types.User, course_key: CourseKey) -> bool: +def can_call_public_api(course_key: CourseKey) -> bool: """ - 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) + return USE_FOR_OUTLINES.is_enabled(course_key) def can_see_all_content(requesting_user: types.User, course_key: CourseKey) -> bool: 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 b0442dcc80..5a85499bc7 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 @@ -101,26 +101,20 @@ class PublicApiAvailableTestCase(django.test.TestCase): 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) + assert not public_api_available(self.fake_course_1) + assert not public_api_available(self.fake_course_2) - # 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) + # Waffle-flag controlled + assert not public_api_available(self.course_key) @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) + assert not public_api_available(self.fake_course_1) + assert not public_api_available(self.fake_course_2) - # 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) + # Waffle-flag controlled + assert public_api_available(self.course_key) class CourseOutlineTestCase(CacheIsolationTestCase): 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 6d269ae3d9..e8b0cdd0a4 100644 --- a/openedx/core/djangoapps/content/learning_sequences/tests/test_views.py +++ b/openedx/core/djangoapps/content/learning_sequences/tests/test_views.py @@ -30,6 +30,7 @@ from ..data import CourseOutlineData, CourseVisibility from ..toggles import USE_FOR_OUTLINES +@override_waffle_flag(USE_FOR_OUTLINES, active=True) class CourseOutlineViewTest(CacheIsolationTestCase, APITestCase): """ General tests for the CourseOutline. @@ -62,16 +63,6 @@ class CourseOutlineViewTest(CacheIsolationTestCase, APITestCase): super().setUp() self.client = APIClient() - def test_student_access_denied(self): - """ - For now, make sure you need staff access bits to use the API. - - This is a temporary safeguard until the API is more complete - """ - self.client.login(username='student', password='student_pass') - result = self.client.get(self.course_url) - assert result.status_code == 403 - def test_non_existent_course_404(self): """ We should 404, not 500, when asking for a course that isn't there. @@ -119,12 +110,22 @@ class CourseOutlineViewTest(CacheIsolationTestCase, APITestCase): assert len(data['outline']['sections'][1]['sequence_ids']) == 2 assert len(data['outline']['sequences']) == 4 - def test_query_for_other_user(self): + @override_waffle_flag(USE_FOR_OUTLINES, active=False) + def test_override_rollout(self): + """ + Test that we can still access the API by sending force_on + + This lets us manually test the outline rendering on live courses that + haven't been rolled out to yet. + + TODO: Remove this test entirely after rollout. + """ self.client.login(username='staff', password='staff_pass') - result = self.client.get(self.course_url + "?user=student") - data = result.data - assert data['username'] == 'student' - assert data['user_id'] == self.student.id + result = self.client.get(self.course_url) + assert result.status_code == 403 + + result = self.client.get(self.course_url, {'force_on': '1'}) + assert result.status_code == 200 @ddt.ddt diff --git a/openedx/core/djangoapps/content/learning_sequences/toggles.py b/openedx/core/djangoapps/content/learning_sequences/toggles.py index 746854496c..d40150b482 100644 --- a/openedx/core/djangoapps/content/learning_sequences/toggles.py +++ b/openedx/core/djangoapps/content/learning_sequences/toggles.py @@ -10,10 +10,11 @@ WAFFLE_NAMESPACE = 'learning_sequences' # .. 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 +# If this endpoint is not enabled for a given course, it will return a 403 +# error. The Courseware MFE should know how to detect this condition. To +# see the value of this API for a course before it has officially been rolled +# out for it, you can bypass this check by passing force_on=1 as a querystring +# parameter. 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). diff --git a/openedx/core/djangoapps/content/learning_sequences/views.py b/openedx/core/djangoapps/content/learning_sequences/views.py index 2648575613..26d3ec4955 100644 --- a/openedx/core/djangoapps/content/learning_sequences/views.py +++ b/openedx/core/djangoapps/content/learning_sequences/views.py @@ -160,7 +160,19 @@ class CourseOutlineView(APIView): course_key = validate_course_key(course_key_str) at_time = datetime.now(timezone.utc) - if not can_call_public_api(request.user, 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 + # over again, I'd call it something else, but all this code is supposed + # to go away when rollout is completed anyway. + # + # The force_on param just means, "Yeah, never mind whether you're turned + # on by default for the purposes of the MFE. I want to see production + # data using this API." The MFE should _never_ pass this parameter. It's + # just a way to peek at the API while it's techincally dark for rollout + # purposes. TODO: REMOVE THIS PARAM AFTER FULL ROLLOUT OF THIS FEATURE. + force_on = request.GET.get("force_on") + if (not force_on) and (not can_call_public_api(course_key)): raise PermissionDenied() try: