From 3f057cad4cb02e947cdf363f6547de5eb89b6e9e Mon Sep 17 00:00:00 2001 From: Taylor Payne Date: Fri, 14 Aug 2026 14:57:33 -0600 Subject: [PATCH 1/2] feat: split read vs write authz checks for group configurations Use COURSES_VIEW_GROUP_CONFIGURATIONS for GET access and COURSES_MANAGE_GROUP_CONFIGURATIONS for write access. Add can_manage flag to the response so the frontend knows whether to render edit controls. Course Editors and Auditors can now view group configurations in read-only mode. Updates both the REST API v1 view and the legacy view handler. ENG45-715 --- .../v1/serializers/group_configurations.py | 1 + .../rest_api/v1/views/group_configurations.py | 12 ++++++-- .../views/tests/test_group_configurations.py | 30 ++++++++++++++++++- cms/djangoapps/contentstore/views/course.py | 21 ++++++++++++- 4 files changed, 59 insertions(+), 5 deletions(-) diff --git a/cms/djangoapps/contentstore/rest_api/v1/serializers/group_configurations.py b/cms/djangoapps/contentstore/rest_api/v1/serializers/group_configurations.py index fefeac8e748e..7b7fe132beca 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/serializers/group_configurations.py +++ b/cms/djangoapps/contentstore/rest_api/v1/serializers/group_configurations.py @@ -57,3 +57,4 @@ class CourseGroupConfigurationsSerializer(serializers.Serializer): ) should_show_enrollment_track = serializers.BooleanField() should_show_experiment_groups = serializers.BooleanField() + can_manage = serializers.BooleanField(default=False, read_only=True) diff --git a/cms/djangoapps/contentstore/rest_api/v1/views/group_configurations.py b/cms/djangoapps/contentstore/rest_api/v1/views/group_configurations.py index 4de2dd8a16b7..f6373d3febaa 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/views/group_configurations.py +++ b/cms/djangoapps/contentstore/rest_api/v1/views/group_configurations.py @@ -2,7 +2,7 @@ import edx_api_doc_tools as apidocs from opaque_keys.edx.keys import CourseKey -from openedx_authz.constants.permissions import COURSES_MANAGE_GROUP_CONFIGURATIONS +from openedx_authz.constants.permissions import COURSES_MANAGE_GROUP_CONFIGURATIONS, COURSES_VIEW_GROUP_CONFIGURATIONS from rest_framework.request import Request from rest_framework.response import Response from rest_framework.views import APIView @@ -10,7 +10,7 @@ from cms.djangoapps.contentstore.rest_api.v1.serializers import CourseGroupConfigurationsSerializer from cms.djangoapps.contentstore.utils import get_group_configurations_context from openedx.core.djangoapps.authz.constants import LegacyAuthoringPermission -from openedx.core.djangoapps.authz.decorators import authz_permission_required +from openedx.core.djangoapps.authz.decorators import authz_permission_required, user_has_course_permission from openedx.core.lib.api.view_utils import DeveloperErrorViewMixin, verify_course_exists, view_auth_classes from xmodule.modulestore.django import modulestore @@ -36,7 +36,7 @@ class CourseGroupConfigurationsView(DeveloperErrorViewMixin, APIView): ) @verify_course_exists() @authz_permission_required( - authz_permission=COURSES_MANAGE_GROUP_CONFIGURATIONS.identifier, + authz_permission=COURSES_VIEW_GROUP_CONFIGURATIONS.identifier, legacy_permission=LegacyAuthoringPermission.READ ) def get(self, request: Request, course_key: CourseKey): @@ -144,5 +144,11 @@ def get(self, request: Request, course_key: CourseKey): with store.bulk_operations(course_key): course = modulestore().get_course(course_key) group_configurations_context = get_group_configurations_context(course, store) + group_configurations_context['can_manage'] = user_has_course_permission( + request.user, + COURSES_MANAGE_GROUP_CONFIGURATIONS.identifier, + course_key, + LegacyAuthoringPermission.WRITE + ) serializer = CourseGroupConfigurationsSerializer(group_configurations_context) return Response(serializer.data) diff --git a/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_group_configurations.py b/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_group_configurations.py index 3db602dc42f3..21152920d7c1 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_group_configurations.py +++ b/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_group_configurations.py @@ -2,7 +2,7 @@ Unit tests for the course's setting group configuration. """ from django.urls import reverse -from openedx_authz.constants.roles import COURSE_DATA_RESEARCHER, COURSE_STAFF +from openedx_authz.constants.roles import COURSE_AUDITOR, COURSE_DATA_RESEARCHER, COURSE_EDITOR, COURSE_STAFF from rest_framework import status from rest_framework.test import APIClient @@ -111,3 +111,31 @@ def test_non_staff_user_cannot_access(self): resp = non_staff_client.get(self.get_url(self.course_key)) self.assertEqual(resp.status_code, status.HTTP_403_FORBIDDEN) # noqa: PT009 + + def test_staff_has_can_manage_true(self): + """User with COURSE_STAFF role gets can_manage=True in response.""" + resp = self.authorized_client.get(self.get_url(self.course_key)) + assert resp.status_code == status.HTTP_200_OK + assert resp.data["can_manage"] is True + + def test_editor_can_view_group_configurations(self): + """User with COURSE_EDITOR role can view group configurations (has view_group_configurations).""" + editor_user = UserFactory() + editor_client = APIClient() + self.add_user_to_role(editor_user, COURSE_EDITOR.external_key) + editor_client.force_authenticate(user=editor_user) + + resp = editor_client.get(self.get_url(self.course_key)) + assert resp.status_code == status.HTTP_200_OK + assert resp.data["can_manage"] is True # editor has manage_group_configurations + + def test_auditor_can_view_group_configurations(self): + """User with COURSE_AUDITOR role can view group configurations (has view_group_configurations).""" + auditor_user = UserFactory() + auditor_client = APIClient() + self.add_user_to_role(auditor_user, COURSE_AUDITOR.external_key) + auditor_client.force_authenticate(user=auditor_user) + + resp = auditor_client.get(self.get_url(self.course_key)) + assert resp.status_code == status.HTTP_200_OK + assert resp.data["can_manage"] is False # auditor has view only diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index 65bdeb0bc883..b4c8a24fe898 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/cms/djangoapps/contentstore/views/course.py @@ -42,6 +42,7 @@ COURSES_PUBLISH_COURSE_CONTENT, COURSES_VIEW_COURSE, COURSES_VIEW_COURSE_UPDATES, + COURSES_VIEW_GROUP_CONFIGURATIONS, COURSES_VIEW_PAGES_AND_RESOURCES, ) from organizations.api import add_organization_course, ensure_organization @@ -185,6 +186,21 @@ def get_course_and_check_manage_group_configurations_access(course_key, user, de return _get_course_block(course_key, depth) +def get_course_and_check_view_group_configurations_access(course_key, user, depth=0): + """ + Function used to validate read permission and return a course block + for group configurations list/detail GET requests. + """ + if not user_has_course_permission( + user=user, + authz_permission=COURSES_VIEW_GROUP_CONFIGURATIONS.identifier, + course_key=course_key, + legacy_permission=LegacyAuthoringPermission.READ + ): + raise PermissionDenied() + return _get_course_block(course_key, depth) + + def reindex_course_and_check_access(course_key, user): """ Internal method used to restart indexing on a course. @@ -1915,7 +1931,10 @@ def group_configurations_list_handler(request, course_key_string): course_key = CourseKey.from_string(course_key_string) store = modulestore() with store.bulk_operations(course_key): - course = get_course_and_check_manage_group_configurations_access(course_key, request.user) + if request.method == 'GET': + course = get_course_and_check_view_group_configurations_access(course_key, request.user) + else: + course = get_course_and_check_manage_group_configurations_access(course_key, request.user) if 'text/html' in request.META.get('HTTP_ACCEPT', 'text/html'): return redirect(get_group_configurations_url(course_key)) From f909598470560d61f4a2e6d9cc2561c0f56a9236 Mon Sep 17 00:00:00 2001 From: Taylor Payne Date: Wed, 26 Aug 2026 15:21:07 -0600 Subject: [PATCH 2/2] test: cover GET (view) authz branch of group_configurations_list_handler Adds GetGroupConfigurationsListHandlerAuthzTest to verify the read path introduced by the view/manage permission split: - staff, editor, and auditor (view_group_configurations) can GET - users without a role or with a non-viewing role are denied The POST (write) path was already covered by PostGroupConfigurationsListHandlerAuthzTest. --- .../views/tests/test_group_configurations.py | 62 ++++++++++++++++++- 1 file changed, 61 insertions(+), 1 deletion(-) diff --git a/cms/djangoapps/contentstore/views/tests/test_group_configurations.py b/cms/djangoapps/contentstore/views/tests/test_group_configurations.py index dcf9e18a52d3..03e84d35d88d 100644 --- a/cms/djangoapps/contentstore/views/tests/test_group_configurations.py +++ b/cms/djangoapps/contentstore/views/tests/test_group_configurations.py @@ -9,7 +9,7 @@ import ddt from django.test import Client -from openedx_authz.constants.roles import COURSE_DATA_RESEARCHER, COURSE_STAFF +from openedx_authz.constants.roles import COURSE_AUDITOR, COURSE_DATA_RESEARCHER, COURSE_EDITOR, COURSE_STAFF from rest_framework import status from cms.djangoapps.contentstore.api.tests.base import BaseCourseViewTest @@ -1235,6 +1235,66 @@ def test_update_usage_info_no_message(self): """ self.verify_validation_update_usage_info(None, None) # pylint: disable=no-value-for-parameter +@ddt.ddt +class GetGroupConfigurationsListHandlerAuthzTest(CourseAuthzTestMixin, BaseCourseViewTest): + """ + Tests the GET (read) path of group_configurations_list_handler authorization + using openedx-authz. The GET path uses the COURSES_VIEW_GROUP_CONFIGURATIONS + permission, so view-only roles (editor/auditor) are allowed while roles without + any view access are denied. + """ + + view_name = "group_configurations_list_handler" + authz_roles_to_assign = [COURSE_STAFF.external_key] + course_key_arg_name = 'course_key_string' + + def setUp(self): + super().setUp() + # Function-based views require session auth, not DRF force_authenticate. + self.authorized_client = Client() + self.authorized_client.login( + username=self.authorized_user.username, password=self.password + ) + self.unauthorized_client = Client() + self.unauthorized_client.login( + username=self.unauthorized_user.username, password=self.password + ) + + def _get(self, client, course_key): + """GET the list handler as an HTML request (redirects to the MFE on success).""" + return client.get( + self.get_url(course_key), + HTTP_ACCEPT='text/html', + ) + + @ddt.data(COURSE_STAFF, COURSE_EDITOR, COURSE_AUDITOR) + def test_view_permission_roles_can_read(self, role): + """Staff, editor, and auditor all have view_group_configurations and can GET.""" + user = UserFactory(password=self.password) + self.add_user_to_role(user, role.external_key) + client = Client() + client.login(username=user.username, password=self.password) + + resp = self._get(client, self.course_key) + # HTML GET redirects to the authoring MFE when access is granted. + assert resp.status_code == status.HTTP_302_FOUND + + def test_unauthorized_user_cannot_read(self): + """User without any role cannot GET.""" + resp = self._get(self.unauthorized_client, self.course_key) + assert resp.status_code == status.HTTP_403_FORBIDDEN + + def test_role_without_view_permission_cannot_read(self): + """A role lacking view_group_configurations (data researcher) cannot GET.""" + non_staff_user = UserFactory(password=self.password) + self.add_user_to_role(non_staff_user, COURSE_DATA_RESEARCHER.external_key) + client = Client() + client.login(username=non_staff_user.username, password=self.password) + + resp = self._get(client, self.course_key) + assert resp.status_code == status.HTTP_403_FORBIDDEN + + class PostGroupConfigurationsListHandlerAuthzTest(CourseAuthzTestMixin, BaseCourseViewTest): """ Tests endpoint used to create new Course Group Configurations