diff --git a/cms/djangoapps/contentstore/rest_api/v1/urls.py b/cms/djangoapps/contentstore/rest_api/v1/urls.py index 685a81d778ce..da97bcab6a7a 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/urls.py +++ b/cms/djangoapps/contentstore/rest_api/v1/urls.py @@ -2,6 +2,7 @@ from django.conf import settings from django.urls import path, re_path +from rest_framework.routers import DefaultRouter from openedx.core.constants import COURSE_ID_PATTERN @@ -23,6 +24,7 @@ HomePageCoursesView, HomePageLibrariesView, HomePageView, + HomeViewSet, ProctoredExamSettingsView, ProctoringErrorsView, VideoDownloadView, @@ -34,7 +36,13 @@ VIDEO_ID_PATTERN = r'(?P[-\w]+)' -urlpatterns = [ +# ADR 0028: ViewSets registered via DefaultRouter. +router = DefaultRouter() +router.register(r'home', HomeViewSet, basename='home') + +urlpatterns = router.urls + [ + # DEPRECATED (ADR 0028): Use HomeViewSet instead (GET home/, home/courses/, home/libraries/). + # Kept as backward-compatible aliases. Remove after one named release. path( 'home', HomePageView.as_view(), diff --git a/cms/djangoapps/contentstore/rest_api/v1/views/__init__.py b/cms/djangoapps/contentstore/rest_api/v1/views/__init__.py index 7654c9e0befc..4bc061c1f6a4 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/views/__init__.py +++ b/cms/djangoapps/contentstore/rest_api/v1/views/__init__.py @@ -1,6 +1,7 @@ """ Views for v1 contentstore API. """ + from .certificates import CourseCertificatesView # noqa: F401 from .course_details import CourseDetailsView # noqa: F401 from .course_index import ContainerChildrenView, CourseIndexView # noqa: F401 @@ -10,7 +11,7 @@ from .grading import CourseGradingView # noqa: F401 from .group_configurations import CourseGroupConfigurationsView # noqa: F401 from .help_urls import HelpUrlsView # noqa: F401 -from .home import HomePageCoursesView, HomePageLibrariesView, HomePageView # noqa: F401 +from .home import HomePageCoursesView, HomePageLibrariesView, HomePageView, HomeViewSet # noqa: F401 from .proctoring import ProctoredExamSettingsView, ProctoringErrorsView # noqa: F401 from .settings import CourseSettingsView # noqa: F401 from .textbooks import CourseTextbooksView # noqa: F401 diff --git a/cms/djangoapps/contentstore/rest_api/v1/views/home.py b/cms/djangoapps/contentstore/rest_api/v1/views/home.py index 95723020c11f..9dc34744a7fa 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/views/home.py +++ b/cms/djangoapps/contentstore/rest_api/v1/views/home.py @@ -2,7 +2,12 @@ import edx_api_doc_tools as apidocs from django.conf import settings +from edx_rest_framework_extensions.auth.jwt.authentication import JwtAuthentication +from edx_rest_framework_extensions.auth.session.authentication import SessionAuthenticationAllowInactiveUser from organizations import api as org_api +from rest_framework import viewsets +from rest_framework.decorators import action +from rest_framework.permissions import IsAuthenticated from rest_framework.request import Request from rest_framework.response import Response from rest_framework.views import APIView @@ -13,6 +18,145 @@ from ..serializers import CourseHomeTabSerializer, LibraryTabSerializer, StudioHomeSerializer +# ADR 0028 – consolidated from HomePageView, HomePageCoursesView, HomePageLibrariesView +class HomeViewSet(viewsets.ViewSet): + """ + ViewSet for the Studio home page. Registered via DefaultRouter (basename ``home``). + + Router-generated URLs: + GET /api/contentstore/v1/home/ → list (aggregated home context) + GET /api/contentstore/v1/home/courses/ → courses (course list only) + GET /api/contentstore/v1/home/libraries/→ libraries (library list only) + + ADR 0025 compliance notes: + - Three different serializers are returned by ``get_serializer_class()`` depending + on the action. ``serializer_class`` is set to the default (``StudioHomeSerializer``). + - All response formatting is handled by the respective serializer class. + """ + + authentication_classes = (JwtAuthentication, SessionAuthenticationAllowInactiveUser) + permission_classes = (IsAuthenticated,) # ADR 0026 + serializer_class = StudioHomeSerializer # default; overridden by get_serializer_class() + + def get_serializer_class(self): + """Return the appropriate serializer class for the current action.""" + if self.action == 'courses': + return CourseHomeTabSerializer + if self.action == 'libraries': + return LibraryTabSerializer + return StudioHomeSerializer + + def get_serializer(self, *args, **kwargs): + """Return a serializer instance using the action-appropriate class.""" + return self.get_serializer_class()(*args, **kwargs) + + @apidocs.schema( + parameters=[ + apidocs.string_parameter( + "org", + apidocs.ParameterLocation.QUERY, + description="Query param to filter by course org", + )], + responses={ + 200: StudioHomeSerializer, + 401: "The requester is not authenticated.", + }, + ) + def list(self, request: Request): + """ + Get an object containing all courses and libraries on home page. + + **Example Request** + + GET /api/contentstore/v1/home/ + """ + home_context = get_home_context(request, True) + home_context.update({ + # 'allow_to_create_new_org' is actually about auto-creating organizations + # (e.g. when creating a course or library), so we add an additional test. + 'allow_to_create_new_org': ( + home_context['can_create_organizations'] and + org_api.is_autocreate_enabled() + ), + 'studio_name': settings.STUDIO_NAME, + 'studio_short_name': settings.STUDIO_SHORT_NAME, + 'studio_request_email': settings.FEATURES.get('STUDIO_REQUEST_EMAIL', ''), + 'tech_support_email': settings.TECH_SUPPORT_EMAIL, + 'platform_name': settings.PLATFORM_NAME, + 'user_is_active': request.user.is_active, + }) + serializer = self.get_serializer(home_context) + return Response(serializer.data) + + @apidocs.schema( + parameters=[ + apidocs.string_parameter( + "org", + apidocs.ParameterLocation.QUERY, + description="Query param to filter by course org", + )], + responses={ + 200: CourseHomeTabSerializer, + 401: "The requester is not authenticated.", + }, + ) + @action(detail=False, methods=['get'], url_path='courses', url_name='courses') + def courses(self, request: Request): + """ + Get an object containing all courses. + + **Example Request** + + GET /api/contentstore/v1/home/courses/ + """ + active_courses, archived_courses, in_process_course_actions = get_course_context(request) + courses_context = { + "courses": active_courses, + "archived_courses": archived_courses, + "in_process_course_actions": in_process_course_actions, + } + serializer = self.get_serializer(courses_context) + return Response(serializer.data) + + @apidocs.schema( + parameters=[ + apidocs.string_parameter( + "org", + apidocs.ParameterLocation.QUERY, + description="Query param to filter by course org", + ), + apidocs.query_parameter( + "is_migrated", + bool, + description=( + "Query param to filter by migrated status of library." + " If present (true or false), it will filter by migration status" + " else it will return all legacy libraries." + ), + ) + ], + responses={ + 200: LibraryTabSerializer, + 401: "The requester is not authenticated.", + }, + ) + @action(detail=False, methods=['get'], url_path='libraries', url_name='libraries') + def libraries(self, request: Request): + """ + Get an object containing all libraries on home page. + + **Example Request** + + GET /api/contentstore/v1/home/libraries/ + """ + library_context = get_library_context(request) + serializer = self.get_serializer(library_context) + return Response(serializer.data) + + +# DEPRECATED (ADR 0028): Use HomeViewSet instead. +# Will be removed after one named release. +# Use GET home/, home/courses/, home/libraries/ instead. @view_auth_classes(is_authenticated=True) class HomePageView(APIView): """ @@ -99,11 +243,13 @@ def get(self, request: Request): return Response(serializer.data) -@view_auth_classes(is_authenticated=True) class HomePageCoursesView(APIView): """ View for getting all courses and libraries available to the logged in user. """ + authentication_classes = (JwtAuthentication, SessionAuthenticationAllowInactiveUser) + permission_classes = (IsAuthenticated,) + serializer_class = CourseHomeTabSerializer @apidocs.schema( parameters=[ apidocs.string_parameter( @@ -170,7 +316,7 @@ def get(self, request: Request): "archived_courses": archived_courses, "in_process_course_actions": in_process_course_actions, } - serializer = CourseHomeTabSerializer(courses_context) + serializer = self.serializer_class(courses_context) return Response(serializer.data) diff --git a/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_home.py b/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_home.py index ccdd906609a4..d0a16c095eb2 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_home.py +++ b/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_home.py @@ -7,17 +7,19 @@ import ddt import pytz from django.conf import settings -from django.test import override_settings +from django.test import TestCase, override_settings from django.urls import reverse from opaque_keys.edx.locator import LibraryLocatorV2 from openedx_content import api as content_api from organizations.tests.factories import OrganizationFactory from rest_framework import status +from rest_framework.test import APIClient from cms.djangoapps.contentstore.tests.test_libraries import LibraryTestCase from cms.djangoapps.contentstore.tests.utils import CourseTestCase from cms.djangoapps.modulestore_migrator import api as migrator_api from cms.djangoapps.modulestore_migrator.data import CompositionLevel, RepeatHandlingStrategy +from common.djangoapps.student.tests.factories import UserFactory from openedx.core.djangoapps.content.course_overviews.tests.factories import CourseOverviewFactory from openedx.core.djangoapps.content_libraries import api as lib_api @@ -400,5 +402,54 @@ def test_home_page_libraries_response(self): ], } - self.assertEqual(response.status_code, status.HTTP_200_OK) # noqa: PT009 - self.assertDictEqual(expected_response, response.json()) # noqa: PT009 + assert response.status_code == status.HTTP_200_OK + assert response.json() == expected_response + + +class HomePageCoursesViewPermissionsTest(TestCase): + """ + ADR 0026 – permission regression tests for HomePageCoursesView. + + Verifies that the explicit permission_classes = (IsAuthenticated,) enforces + the same access rules previously set by the @view_auth_classes(is_authenticated=True) + decorator. + """ + + def setUp(self): + super().setUp() + self.client = APIClient() + self.url = reverse("cms.djangoapps.contentstore:v1:courses") + self.user = UserFactory.create() + self.staff_user = UserFactory.create(is_staff=True) + + def test_unauthenticated_request_returns_401(self): + """ + Unauthenticated request (no credentials) must be rejected with 401. + + Before ADR 0026: enforced by @view_auth_classes(is_authenticated=True). + After ADR 0026: enforced by permission_classes = (IsAuthenticated,). + """ + response = self.client.get(self.url) + assert response.status_code == status.HTTP_401_UNAUTHORIZED + + def test_authenticated_user_gets_200(self): + """ + Any authenticated user (not necessarily staff) must receive 200. + + HomePageCoursesView only requires authentication — no staff role needed. + The view returns an empty course list for users with no assigned courses. + """ + self.client.force_authenticate(user=self.user) + response = self.client.get(self.url) + assert response.status_code == status.HTTP_200_OK + + def test_staff_user_gets_200(self): + """Staff user must also receive 200 (staff is a superset of authenticated).""" + self.client.force_authenticate(user=self.staff_user) + response = self.client.get(self.url) + assert response.status_code == status.HTTP_200_OK + + def test_post_by_unauthenticated_returns_401(self): + """Non-GET methods also enforce authentication — POST without credentials is 401.""" + response = self.client.post(self.url, data={}) + assert response.status_code == status.HTTP_401_UNAUTHORIZED diff --git a/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_home_viewset.py b/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_home_viewset.py new file mode 100644 index 000000000000..d0e309f5377a --- /dev/null +++ b/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_home_viewset.py @@ -0,0 +1,128 @@ +""" +Unit tests for HomeViewSet (ADR 0028). + +MongoDB-free: all service-layer calls are mocked. + +patch.object is used for the ViewSet's get_serializer() method because: + - get_serializer_class() returns a *different* serializer per action. + - Each serializer (StudioHomeSerializer, CourseHomeTabSerializer, + LibraryTabSerializer) has many required fields that would be painful to + satisfy with synthetic data. + - Patching get_serializer() lets us focus on routing + service-call assertions + without re-testing serializer logic (covered in serializer unit tests). +""" +from unittest.mock import patch + +from django.urls import reverse +from rest_framework import status +from rest_framework.test import APITestCase + +from cms.djangoapps.contentstore.rest_api.v1.views.home import HomeViewSet +from common.djangoapps.student.tests.factories import UserFactory + +# ------------------------------------------------------------------ +# Mock target paths +# ------------------------------------------------------------------ +MOCK_GET_HOME_CONTEXT = ( + 'cms.djangoapps.contentstore.rest_api.v1.views.home.get_home_context' +) +MOCK_GET_COURSE_CONTEXT = ( + 'cms.djangoapps.contentstore.rest_api.v1.views.home.get_course_context' +) +MOCK_GET_LIBRARY_CONTEXT = ( + 'cms.djangoapps.contentstore.rest_api.v1.views.home.get_library_context' +) +MOCK_ORG_API = ( + 'cms.djangoapps.contentstore.rest_api.v1.views.home.org_api' +) + + +class TestHomeViewSetPermissions(APITestCase): + """ + ADR 0028 – permission regression tests for HomeViewSet. + + Verifies that IsAuthenticated enforces the same access rules previously + provided by @view_auth_classes(is_authenticated=True) on the deprecated views. + """ + + def test_unauthenticated_list_returns_401(self): + """Unauthenticated GET /home/ must return 401.""" + url = reverse('cms.djangoapps.contentstore:v1:home-list') + response = self.client.get(url) + assert response.status_code == status.HTTP_401_UNAUTHORIZED + + def test_unauthenticated_courses_returns_401(self): + """Unauthenticated GET /home/courses/ must return 401.""" + url = reverse('cms.djangoapps.contentstore:v1:home-courses') + response = self.client.get(url) + assert response.status_code == status.HTTP_401_UNAUTHORIZED + + def test_unauthenticated_libraries_returns_401(self): + """Unauthenticated GET /home/libraries/ must return 401.""" + url = reverse('cms.djangoapps.contentstore:v1:home-libraries') + response = self.client.get(url) + assert response.status_code == status.HTTP_401_UNAUTHORIZED + + +class TestHomeViewSetActions(APITestCase): + """ + Action tests for HomeViewSet (list, courses, libraries). + + Any authenticated user can access these endpoints — no course-staff role + is required — so a plain (non-staff) factory user is sufficient. + """ + + def setUp(self): + super().setUp() + self.user = UserFactory.create() + self.client.force_authenticate(user=self.user) + + # ------------------------------------------------------------------ + # list → GET /home/ + # ------------------------------------------------------------------ + + @patch.object(HomeViewSet, 'get_serializer') + @patch(MOCK_ORG_API) + @patch(MOCK_GET_HOME_CONTEXT) + def test_list_calls_get_home_context(self, mock_home, mock_org, mock_get_ser): + """GET /home/ calls get_home_context() and returns 200.""" + mock_home.return_value = {'can_create_organizations': True} + mock_org.is_autocreate_enabled.return_value = True + mock_get_ser.return_value.data = {'studio_name': 'Studio'} + + response = self.client.get(reverse('cms.djangoapps.contentstore:v1:home-list')) + + assert response.status_code == status.HTTP_200_OK + mock_home.assert_called_once() + + # ------------------------------------------------------------------ + # courses → GET /home/courses/ + # ------------------------------------------------------------------ + + @patch.object(HomeViewSet, 'get_serializer') + @patch(MOCK_GET_COURSE_CONTEXT) + def test_courses_calls_get_course_context(self, mock_courses, mock_get_ser): + """GET /home/courses/ calls get_course_context() and returns 200.""" + mock_courses.return_value = ([], [], []) + mock_get_ser.return_value.data = {'courses': [], 'archived_courses': [], 'in_process_course_actions': []} + + response = self.client.get(reverse('cms.djangoapps.contentstore:v1:home-courses')) + + assert response.status_code == status.HTTP_200_OK + mock_courses.assert_called_once() + + # ------------------------------------------------------------------ + # libraries → GET /home/libraries/ + # ------------------------------------------------------------------ + + @patch.object(HomeViewSet, 'get_serializer') + @patch(MOCK_GET_LIBRARY_CONTEXT) + def test_libraries_calls_get_library_context(self, mock_libs, mock_get_ser): + """GET /home/libraries/ calls get_library_context() and returns 200.""" + mock_libs.return_value = {'libraries': []} + mock_get_ser.return_value.data = {'libraries': []} + + response = self.client.get(reverse('cms.djangoapps.contentstore:v1:home-libraries')) + + assert response.status_code == status.HTTP_200_OK + mock_libs.assert_called_once()