diff --git a/.github/workflows/pylint-checks.yml b/.github/workflows/pylint-checks.yml index 21d4421b0950..91141c0c29ed 100644 --- a/.github/workflows/pylint-checks.yml +++ b/.github/workflows/pylint-checks.yml @@ -21,7 +21,7 @@ jobs: - module-name: openedx-1 path: "openedx/core/types/ openedx/core/djangoapps/ace_common/ openedx/core/djangoapps/agreements/ openedx/core/djangoapps/api_admin/ openedx/core/djangoapps/auth_exchange/ openedx/core/djangoapps/bookmarks/ openedx/core/djangoapps/cache_toolbox/ openedx/core/djangoapps/catalog/ openedx/core/djangoapps/ccxcon/ openedx/core/djangoapps/commerce/ openedx/core/djangoapps/common_initialization/ openedx/core/djangoapps/common_views/ openedx/core/djangoapps/config_model_utils/ openedx/core/djangoapps/content/ openedx/core/djangoapps/content_libraries/ openedx/core/djangoapps/content_staging/ openedx/core/djangoapps/contentserver/ openedx/core/djangoapps/cookie_metadata/ openedx/core/djangoapps/cors_csrf/ openedx/core/djangoapps/course_apps/ openedx/core/djangoapps/course_date_signals/ openedx/core/djangoapps/course_groups/ openedx/core/djangoapps/courseware_api/ openedx/core/djangoapps/crawlers/ openedx/core/djangoapps/credentials/ openedx/core/djangoapps/credit/ openedx/core/djangoapps/dark_lang/ openedx/core/djangoapps/debug/ openedx/core/djangoapps/discussions/ openedx/core/djangoapps/django_comment_common/ openedx/core/djangoapps/embargo/ openedx/core/djangoapps/enrollments/ openedx/core/djangoapps/external_user_ids/ openedx/core/djangoapps/zendesk_proxy/ openedx/core/djangolib/ openedx/core/lib/ openedx/core/djangoapps/course_live/" - module-name: openedx-2 - path: "openedx/core/djangoapps/geoinfo/ openedx/core/djangoapps/header_control/ openedx/core/djangoapps/heartbeat/ openedx/core/djangoapps/lang_pref/ openedx/core/djangoapps/models/ openedx/core/djangoapps/monkey_patch/ openedx/core/djangoapps/oauth_dispatch/ openedx/core/djangoapps/olx_rest_api/ openedx/core/djangoapps/password_policy/ openedx/core/djangoapps/plugin_api/ openedx/core/djangoapps/plugins/ openedx/core/djangoapps/profile_images/ openedx/core/djangoapps/programs/ openedx/core/djangoapps/safe_sessions/ openedx/core/djangoapps/schedules/ openedx/core/djangoapps/service_status/ openedx/core/djangoapps/session_inactivity_timeout/ openedx/core/djangoapps/signals/ openedx/core/djangoapps/site_configuration/ openedx/core/djangoapps/system_wide_roles/ openedx/core/djangoapps/theming/ openedx/core/djangoapps/user_api/ openedx/core/djangoapps/user_authn/ openedx/core/djangoapps/util/ openedx/core/djangoapps/verified_track_content/ openedx/core/djangoapps/video_config/ openedx/core/djangoapps/video_pipeline/ openedx/core/djangoapps/waffle_utils/ openedx/core/djangoapps/xblock/ openedx/core/djangoapps/xmodule_django/ openedx/core/tests/ openedx/features/ openedx/testing/ openedx/tests/ openedx/envs/ openedx/core/djangoapps/notifications/ openedx/core/djangoapps/staticfiles/ openedx/core/djangoapps/content_tagging/" + path: "openedx/core/djangoapps/geoinfo/ openedx/core/djangoapps/header_control/ openedx/core/djangoapps/heartbeat/ openedx/core/djangoapps/lang_pref/ openedx/core/djangoapps/models/ openedx/core/djangoapps/monkey_patch/ openedx/core/djangoapps/oauth_dispatch/ openedx/core/djangoapps/olx_rest_api/ openedx/core/djangoapps/password_policy/ openedx/core/djangoapps/plugin_api/ openedx/core/djangoapps/plugins/ openedx/core/djangoapps/profile_images/ openedx/core/djangoapps/programs/ openedx/core/djangoapps/safe_sessions/ openedx/core/djangoapps/schedules/ openedx/core/djangoapps/service_status/ openedx/core/djangoapps/session_inactivity_timeout/ openedx/core/djangoapps/signals/ openedx/core/djangoapps/site_configuration/ openedx/core/djangoapps/system_wide_roles/ openedx/core/djangoapps/theming/ openedx/core/djangoapps/user_api/ openedx/core/djangoapps/user_authn/ openedx/core/djangoapps/util/ openedx/core/djangoapps/verified_track_content/ openedx/core/djangoapps/video_config/ openedx/core/djangoapps/video_pipeline/ openedx/core/djangoapps/waffle_utils/ openedx/core/djangoapps/xblock/ openedx/core/djangoapps/xmodule_django/ openedx/core/tests/ openedx/features/ openedx/testing/ openedx/tests/ openedx/envs/ openedx/core/djangoapps/notifications/ openedx/core/djangoapps/staticfiles/ openedx/core/djangoapps/content_tagging/ openedx/core/djangoapps/authz/" - module-name: common path: "common" - module-name: cms diff --git a/.github/workflows/unit-test-shards.json b/.github/workflows/unit-test-shards.json index cb9beeb3c6bf..2c1ffd4744b7 100644 --- a/.github/workflows/unit-test-shards.json +++ b/.github/workflows/unit-test-shards.json @@ -147,6 +147,7 @@ "openedx/core/djangoapps/xblock/", "openedx/core/djangoapps/xmodule_django/", "openedx/core/djangoapps/zendesk_proxy/", + "openedx/core/djangoapps/authz/", "openedx/core/djangolib/", "openedx/core/lib/", "openedx/core/tests/", @@ -227,6 +228,7 @@ "openedx/core/djangoapps/xblock/", "openedx/core/djangoapps/xmodule_django/", "openedx/core/djangoapps/zendesk_proxy/", + "openedx/core/djangoapps/authz/", "openedx/core/lib/", "openedx/tests/" ] diff --git a/cms/djangoapps/contentstore/api/tests/test_quality.py b/cms/djangoapps/contentstore/api/tests/test_quality.py index 8625cbeb55e5..aa6b71e054be 100644 --- a/cms/djangoapps/contentstore/api/tests/test_quality.py +++ b/cms/djangoapps/contentstore/api/tests/test_quality.py @@ -2,9 +2,12 @@ Tests for the course import API views """ - +from rest_framework.test import APIClient from rest_framework import status +from openedx_authz.constants.roles import COURSE_STAFF, COURSE_DATA_RESEARCHER +from common.djangoapps.student.tests.factories import UserFactory +from openedx.core.djangoapps.authz.tests.mixins import CourseAuthzTestMixin from .base import BaseCourseViewTest @@ -67,3 +70,63 @@ def test_student_fails(self): self.client.login(username=self.student.username, password=self.password) resp = self.client.get(self.get_url(self.course_key)) self.assertEqual(resp.status_code, status.HTTP_403_FORBIDDEN) + + +class CourseQualityAuthzTest(CourseAuthzTestMixin, BaseCourseViewTest): + """ + Tests Course Quality API authorization using openedx-authz. + The endpoint uses COURSES_VIEW_COURSE permission. + """ + + view_name = "courses_api:course_quality" + authz_roles_to_assign = [COURSE_STAFF.external_key] + + def test_authorized_user_can_access(self): + """User with COURSE_STAFF role can access.""" + resp = self.authorized_client.get(self.get_url(self.course_key)) + self.assertEqual(resp.status_code, status.HTTP_200_OK) + + def test_unauthorized_user_cannot_access(self): + """User without role cannot access.""" + resp = self.unauthorized_client.get(self.get_url(self.course_key)) + self.assertEqual(resp.status_code, status.HTTP_403_FORBIDDEN) + + def test_role_scoped_to_course(self): + """Authorization should only apply to the assigned course.""" + other_course = self.store.create_course("OtherOrg", "OtherCourse", "Run", self.staff.id) + + resp = self.authorized_client.get(self.get_url(other_course.id)) + self.assertEqual(resp.status_code, status.HTTP_403_FORBIDDEN) + + def test_staff_user_allowed_via_legacy(self): + """ + Staff users should still pass through legacy fallback. + """ + self.client.login(username=self.staff.username, password=self.password) + + resp = self.client.get(self.get_url(self.course_key)) + self.assertEqual(resp.status_code, status.HTTP_200_OK) + + def test_superuser_allowed(self): + """Superusers should always be allowed.""" + superuser = UserFactory(is_superuser=True) + + client = APIClient() + client.force_authenticate(user=superuser) + + resp = client.get(self.get_url(self.course_key)) + self.assertEqual(resp.status_code, status.HTTP_200_OK) + + def test_non_staff_user_cannot_access(self): + """ + User without permissions should be denied. + This case validates that a non-staff user cannot access even + if they have course author access to the course. + """ + non_staff_user = UserFactory() + non_staff_client = APIClient() + self.add_user_to_role(non_staff_user, COURSE_DATA_RESEARCHER.external_key) + non_staff_client.force_authenticate(user=non_staff_user) + + resp = non_staff_client.get(self.get_url(self.course_key)) + self.assertEqual(resp.status_code, status.HTTP_403_FORBIDDEN) diff --git a/cms/djangoapps/contentstore/api/tests/test_validation.py b/cms/djangoapps/contentstore/api/tests/test_validation.py index 8bf8e19b626c..a091eedcc577 100644 --- a/cms/djangoapps/contentstore/api/tests/test_validation.py +++ b/cms/djangoapps/contentstore/api/tests/test_validation.py @@ -7,18 +7,23 @@ import ddt import factory + from django.conf import settings from django.contrib.auth import get_user_model from django.test.utils import override_settings from django.urls import reverse from rest_framework import status from rest_framework.test import APITestCase +from rest_framework.test import APIClient +from openedx.core.djangoapps.authz.tests.mixins import CourseAuthzTestMixin +from openedx_authz.constants.roles import COURSE_STAFF, COURSE_DATA_RESEARCHER from common.djangoapps.course_modes.models import CourseMode from common.djangoapps.course_modes.tests.factories import CourseModeFactory from common.djangoapps.student.tests.factories import StaffFactory, UserFactory from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase from xmodule.modulestore.tests.factories import BlockFactory, CourseFactory +from cms.djangoapps.contentstore.api.tests.base import BaseCourseViewTest User = get_user_model() @@ -272,3 +277,81 @@ def test_list_ready_to_update_reference_success(self, mock_block, mock_auth): {'usage_key': str(self.block2.location)}, ]) mock_auth.assert_called_once() + + +class CourseValidationAuthzTest(CourseAuthzTestMixin, BaseCourseViewTest): + """ + Tests Course Validation API authorization using openedx-authz. + The endpoint uses COURSES_VIEW_COURSE permission. + """ + + view_name = "courses_api:course_validation" + authz_roles_to_assign = [COURSE_STAFF.external_key] + + def test_authorized_user_can_access(self): + """ + User with COURSE_STAFF role should be allowed via AuthZ. + """ + resp = self.authorized_client.get(self.get_url(self.course_key)) + + self.assertEqual(resp.status_code, status.HTTP_200_OK) + + def test_unauthorized_user_cannot_access(self): + """ + User without permissions should be denied. + """ + resp = self.unauthorized_client.get(self.get_url(self.course_key)) + + self.assertEqual(resp.status_code, status.HTTP_403_FORBIDDEN) + + def test_role_scoped_to_course(self): + """ + Authorization should only apply to the assigned course scope. + """ + other_course = self.store.create_course( + "OtherOrg", + "OtherCourse", + "Run", + self.staff.id, + ) + + resp = self.authorized_client.get(self.get_url(other_course.id)) + + self.assertEqual(resp.status_code, status.HTTP_403_FORBIDDEN) + + def test_staff_user_allowed_via_legacy(self): + """ + Course staff should pass through legacy fallback when AuthZ denies. + """ + self.client.login(username=self.staff.username, password=self.password) + + resp = self.client.get(self.get_url(self.course_key)) + + self.assertEqual(resp.status_code, status.HTTP_200_OK) + + def test_superuser_allowed(self): + """ + Superusers should always be allowed through legacy fallback. + """ + superuser = UserFactory(is_superuser=True) + + client = APIClient() + client.force_authenticate(user=superuser) + + resp = client.get(self.get_url(self.course_key)) + + self.assertEqual(resp.status_code, status.HTTP_200_OK) + + def test_non_staff_user_cannot_access(self): + """ + User without permissions should be denied. + This case validates that a non-staff user cannot access even + if they have course author access to the course. + """ + non_staff_user = UserFactory() + non_staff_client = APIClient() + self.add_user_to_role(non_staff_user, COURSE_DATA_RESEARCHER.external_key) + non_staff_client.force_authenticate(user=non_staff_user) + + resp = non_staff_client.get(self.get_url(self.course_key)) + self.assertEqual(resp.status_code, status.HTTP_403_FORBIDDEN) diff --git a/cms/djangoapps/contentstore/api/views/course_quality.py b/cms/djangoapps/contentstore/api/views/course_quality.py index b301f5ac1420..6ff2c02e2f6c 100644 --- a/cms/djangoapps/contentstore/api/views/course_quality.py +++ b/cms/djangoapps/contentstore/api/views/course_quality.py @@ -6,14 +6,17 @@ from edxval.api import get_course_videos_qset from rest_framework.generics import GenericAPIView from rest_framework.response import Response +from openedx.core.djangoapps.authz.constants import LegacyAuthoringPermission from scipy import stats +from openedx_authz.constants.permissions import COURSES_VIEW_COURSE from openedx.core.lib.api.view_utils import DeveloperErrorViewMixin, view_auth_classes from openedx.core.lib.cache_utils import request_cached from openedx.core.lib.graph_traversals import traverse_pre_order +from openedx.core.djangoapps.authz.decorators import authz_permission_required from xmodule.modulestore.django import modulestore # lint-amnesty, pylint: disable=wrong-import-order -from .utils import course_author_access_required, get_bool_param +from .utils import get_bool_param log = logging.getLogger(__name__) @@ -82,7 +85,7 @@ class CourseQualityView(DeveloperErrorViewMixin, GenericAPIView): # does not specify a serializer class. swagger_schema = None - @course_author_access_required + @authz_permission_required(COURSES_VIEW_COURSE.identifier, LegacyAuthoringPermission.READ) def get(self, request, course_key): """ Returns validation information for the given course. diff --git a/cms/djangoapps/contentstore/api/views/course_validation.py b/cms/djangoapps/contentstore/api/views/course_validation.py index d3565fc1688d..fd3e447b2746 100644 --- a/cms/djangoapps/contentstore/api/views/course_validation.py +++ b/cms/djangoapps/contentstore/api/views/course_validation.py @@ -9,8 +9,10 @@ from rest_framework import serializers, status from rest_framework.generics import GenericAPIView from rest_framework.response import Response +from openedx.core.djangoapps.authz.constants import LegacyAuthoringPermission from user_tasks.models import UserTaskStatus from user_tasks.views import StatusViewSet +from openedx_authz.constants.permissions import COURSES_VIEW_COURSE from cms.djangoapps.contentstore.course_info_model import get_course_updates from cms.djangoapps.contentstore.tasks import migrate_course_legacy_library_blocks_to_item_bank @@ -19,6 +21,7 @@ from openedx.core.lib.api.authentication import BearerAuthenticationAllowInactiveUser from openedx.core.lib.api.serializers import StatusSerializerWithUuid from openedx.core.lib.api.view_utils import DeveloperErrorViewMixin, view_auth_classes +from openedx.core.djangoapps.authz.decorators import authz_permission_required from xmodule.course_metadata_utils import DEFAULT_GRADING_POLICY # lint-amnesty, pylint: disable=wrong-import-order from xmodule.modulestore.django import modulestore # lint-amnesty, pylint: disable=wrong-import-order @@ -80,7 +83,7 @@ class CourseValidationView(DeveloperErrorViewMixin, GenericAPIView): # does not specify a serializer class. swagger_schema = None - @course_author_access_required + @authz_permission_required(COURSES_VIEW_COURSE.identifier, LegacyAuthoringPermission.READ) def get(self, request, course_key): """ Returns validation information for the given course. diff --git a/cms/djangoapps/contentstore/api/views/utils.py b/cms/djangoapps/contentstore/api/views/utils.py index 5d7dd8ff06d1..3426cc5156ba 100644 --- a/cms/djangoapps/contentstore/api/views/utils.py +++ b/cms/djangoapps/contentstore/api/views/utils.py @@ -120,7 +120,7 @@ def course_author_access_required(view): Usage:: @course_author_access_required def my_view(request, course_key): - # Some functionality ... + # Some functionality... """ def _wrapper_view(self, request, course_id, *args, **kwargs): """ diff --git a/cms/envs/common.py b/cms/envs/common.py index f7f690236c39..dc74d9f5e8d4 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -907,6 +907,9 @@ def make_lms_template_path(settings): # alternative swagger generator for CMS API 'drf_spectacular', + # Authz + 'openedx.core.djangoapps.authz', + 'openedx_events', # Core models to represent courses diff --git a/lms/envs/common.py b/lms/envs/common.py index ee41eab173e2..417b3ecb9c65 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -2088,6 +2088,9 @@ # Notifications 'openedx.core.djangoapps.notifications', + # Authz + 'openedx.core.djangoapps.authz', + 'openedx_events', # Core models to represent courses diff --git a/openedx/core/djangoapps/authz/README.rst b/openedx/core/djangoapps/authz/README.rst new file mode 100644 index 000000000000..a9f1112f2361 --- /dev/null +++ b/openedx/core/djangoapps/authz/README.rst @@ -0,0 +1,82 @@ +AuthZ Django Integration +######################## + +Overview +******** + +The ``openedx.core.djangoapps.authz`` app provides Django integrations for the +`openedx-authz` authorization framework within ``edx-platform``. + +The `openedx-authz` library implements a centralized authorization system based +on explicit permissions and policy evaluation. This Django app acts as a thin +integration layer between ``edx-platform`` and the external library, providing +utilities that make it easier to enforce authorization checks in Django views. + +Currently, the app provides a decorator used to enforce AuthZ permissions in +views. The app may also host additional Django-specific helpers and utilities +as the integration with the AuthZ framework evolves. + +Purpose +******* + +This app exists to: + +- Provide Django-specific integrations for the ``openedx-authz`` framework +- Offer reusable decorators for enforcing authorization checks in views +- Centralize AuthZ-related utilities used across LMS and Studio + +Keeping these integrations in a dedicated app avoids coupling authorization +logic with unrelated apps and provides a clear location for future extensions. + +Location in the Platform +************************ + +The app lives in ``openedx/core/djangoapps`` because the functionality it +provides is a **platform-level concern shared across LMS and Studio**, rather +than something specific to either service. + +Usage +***** + +The primary utility currently provided by this app is a decorator that enforces +authorization checks using the AuthZ framework. + +Example usage:: + + from openedx.core.djangoapps.authz.decorators import authz_permission_required + + + @authz_permission_required("course.read") + def my_view(request, course_key): + ... + +The decorator ensures that the requesting user has the required permission +before allowing the view to execute. + +Additional parameters may allow compatibility with legacy permission checks +during the transition to the new authorization framework. + +Contents +******** + +The app currently includes: + +- **Decorators** for enforcing AuthZ permissions in Django views +- **Constants** used by the AuthZ integration +- **Tests** validating decorator behavior + +Relationship with ``openedx-authz`` +*********************************** + +This app does not implement the authorization framework itself. Instead, it +provides Django-specific integrations that connect ``edx-platform`` with the +external ``openedx-authz`` library. + +Keeping these integrations in ``edx-platform`` ensures that the external +library remains framework-agnostic. + +References +********** + +- `openedx-authz repository `_ +- `openedx-authz documentation `_ diff --git a/openedx/core/djangoapps/authz/__init__.py b/openedx/core/djangoapps/authz/__init__.py new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/openedx/core/djangoapps/authz/apps.py b/openedx/core/djangoapps/authz/apps.py new file mode 100644 index 000000000000..3f247cc07f78 --- /dev/null +++ b/openedx/core/djangoapps/authz/apps.py @@ -0,0 +1,16 @@ +"""Django app configuration for authz app.""" + +from django.apps import AppConfig + + +class AuthzConfig(AppConfig): + """Django application configuration for the Open edX Authorization (AuthZ) app. + + This app provides a centralized location for integrations with the + openedx-authz library, including permission helpers, decorators, + and other utilities used to enforce RBAC-based authorization across + the platform.""" + + default_auto_field = 'django.db.models.BigAutoField' + name = 'openedx.core.djangoapps.authz' + verbose_name = "Open edX Authorization Framework" diff --git a/openedx/core/djangoapps/authz/constants.py b/openedx/core/djangoapps/authz/constants.py new file mode 100644 index 000000000000..ae20f37d6287 --- /dev/null +++ b/openedx/core/djangoapps/authz/constants.py @@ -0,0 +1,15 @@ +"""Constants used by the Open edX Authorization (AuthZ) framework.""" + +from common.djangoapps.student.auth import has_studio_read_access, has_studio_write_access +from enum import Enum + + +class LegacyAuthoringPermission(Enum): + READ = "read" + WRITE = "write" + + +LEGACY_PERMISSION_HANDLER_MAP = { + LegacyAuthoringPermission.READ: has_studio_read_access, + LegacyAuthoringPermission.WRITE: has_studio_write_access, +} diff --git a/openedx/core/djangoapps/authz/decorators.py b/openedx/core/djangoapps/authz/decorators.py new file mode 100644 index 000000000000..38b04e92d8e5 --- /dev/null +++ b/openedx/core/djangoapps/authz/decorators.py @@ -0,0 +1,119 @@ +"""Decorators for AuthZ-based permissions enforcement.""" +import logging +from functools import wraps +from collections.abc import Callable + +from django.contrib.auth.models import AbstractUser +from opaque_keys import InvalidKeyError +from opaque_keys.edx.keys import CourseKey, UsageKey +from openedx.core.djangoapps.authz.constants import LEGACY_PERMISSION_HANDLER_MAP, LegacyAuthoringPermission +from openedx_authz import api as authz_api +from rest_framework import status + +from openedx.core import toggles as core_toggles +from openedx.core.lib.api.view_utils import DeveloperErrorViewMixin + +log = logging.getLogger(__name__) + + +def authz_permission_required( + authz_permission: str, + legacy_permission: LegacyAuthoringPermission | None = None) -> Callable: + """ + Decorator enforcing course author permissions via AuthZ + with optional legacy fallback. + + This decorator checks if the requesting user has the specified AuthZ permission for the course. + If AuthZ is not enabled for the course, and a legacy_permission is provided, it falls back to checking + the legacy permission. + + Raises: + DeveloperErrorResponseException: If the user does not have the required permissions. + """ + + def decorator(view_func): + + @wraps(view_func) + def _wrapped_view(self, request, course_id, *args, **kwargs): + course_key = get_course_key(course_id) + + if not user_has_course_permission( + request.user, + authz_permission, + course_key, + legacy_permission + ): + raise DeveloperErrorViewMixin.api_error( + status_code=status.HTTP_403_FORBIDDEN, + developer_message="You do not have permission to perform this action.", + error_code="permission_denied", + ) + + return view_func(self, request, course_key, *args, **kwargs) + + return _wrapped_view + + return decorator + + +def user_has_course_permission( + user: AbstractUser, + authz_permission: str, + course_key: CourseKey, + legacy_permission: LegacyAuthoringPermission | None = None, +) -> bool: + """ + Checks if the user has the specified AuthZ permission for the course, + with optional fallback to legacy permissions. + """ + if core_toggles.enable_authz_course_authoring(course_key): + # If AuthZ is enabled for this course, check the permission via AuthZ only. + is_user_allowed = authz_api.is_user_allowed(user.username, authz_permission, str(course_key)) + log.info( + "AuthZ permission granted = {}".format(is_user_allowed), + extra={ + "user_id": user.id, + "authz_permission": authz_permission, + "course_key": str(course_key), + }, + ) + return is_user_allowed + + # If AuthZ is not enabled for this course, fall back to legacy course author + # access check if legacy_permission is provided. + has_legacy_permission: Callable | None = LEGACY_PERMISSION_HANDLER_MAP.get(legacy_permission) + if legacy_permission and has_legacy_permission and has_legacy_permission(user, course_key): + log.info( + "AuthZ fallback used", + extra={ + "user_id": user.id, + "authz_permission": authz_permission, + "legacy_permission": legacy_permission, + "course_key": str(course_key), + }, + ) + return True + + log.info( + "AuthZ permission denied", + extra={ + "user_id": user.id, + "authz_permission": authz_permission, + "course_key": str(course_key), + }, + ) + return False + + +def get_course_key(course_id: str) -> CourseKey: + """ + Given a course_id string, attempts to parse it as a CourseKey. + If that fails, attempts to parse it as a UsageKey and extract the course key from it. + """ + try: + return CourseKey.from_string(course_id) + except InvalidKeyError: + # If the course_id doesn't match the COURSE_KEY_PATTERN, it might be a usage key. + # Attempt to parse it as such and extract the course key. + usage_key = UsageKey.from_string(course_id) + return usage_key.course_key diff --git a/openedx/core/djangoapps/authz/docs/decisions/0001-authz-django-integration-app.rst b/openedx/core/djangoapps/authz/docs/decisions/0001-authz-django-integration-app.rst new file mode 100644 index 000000000000..4dd1e49f4bc7 --- /dev/null +++ b/openedx/core/djangoapps/authz/docs/decisions/0001-authz-django-integration-app.rst @@ -0,0 +1,74 @@ +0001 AUTHZ DJANGO INTEGRATION APP +#################### + +Status +****** + +Accepted + +Context +******* + +This ADR defines where Django integrations for the openedx-authz framework should live within edx-platform. +The openedx-authz library introduces a new authorization framework for Open edX based on explicit permissions and a centralized policy engine. Integrating this framework into edx-platform requires several Django-specific utilities. +One of the first integrations is a view decorator used to enforce authorization checks using the new AuthZ framework. This decorator is expected to be reused across multiple views in LMS and Studio. +During implementation, the question arose of where these Django integrations should live. + +Some options considered were: + +- common/djangoapps/student/auth.py +- openedx/core/authz.py (a new python module) +- openedx/core/djangoapps/authz (a new Django app) + +The student app contains legacy authentication and authorization logic tied to student functionality. Adding new platform-level authorization integrations there would introduce cross-cutting concerns into an unrelated app. + +Another option was creating a single module such as openedx/core/authz.py. However, the integration already includes multiple components (decorators, constants, tests) and is expected to grow over time. + +Because of this, a dedicated Django app provides a clearer and more scalable structure for these integrations. + +Decision +******** + +edx-platform will introduce a new lightweight Django app openedx.core.djangoapps.authz to host Django integrations for the openedx-authz framework. + +- The app will contain reusable decorators enforcing AuthZ permissions in Django views. +- Supporting modules such as constants and helper utilities will live in this app. +- The app will include tests validating these integrations. +- The app acts as a thin integration layer between edx-platform and the external openedx-authz library. +- The app will live in openedx/core/djangoapps because this functionality is a platform-level concern shared by LMS and Studio. + +Initial contents include: + +- An authorization decorator for Django views. +- A constants.py module for AuthZ-related constants. +- Tests validating the decorator behavior. + +Consequences +************ + +- Django integrations for the AuthZ framework have a centralized and discoverable location. +- Future integrations can be added without expanding unrelated modules. +- The separation clarifies the distinction between authentication (authn) and authorization (authz) responsibilities. + +However: + +- Introducing a new Django app slightly increases project structure complexity. +- Some authorization logic may remain elsewhere until future refactoring occurs. + +Rejected Alternatives +********************** + +- Add the decorator to common/djangoapps/student/auth.py +Rejected because the module belongs to the student app and already mixes authentication and authorization responsibilities. + +- Create a single module openedx/core/authz.py +Rejected because the integration already includes multiple components and is expected to grow. + +- Implement the decorator in the openedx-authz library +Rejected because the decorator is Django-specific and tied to how edx-platform integrates authorization checks into views. + +References +********** + +.. _openedx-authz repository: https://github.com/openedx/openedx-authz +.. _openedx-authz documentation: https://openedx-authz.readthedocs.io/ diff --git a/openedx/core/djangoapps/authz/migrations/__init__.py b/openedx/core/djangoapps/authz/migrations/__init__.py new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/openedx/core/djangoapps/authz/tests/__init__.py b/openedx/core/djangoapps/authz/tests/__init__.py new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/openedx/core/djangoapps/authz/tests/mixins.py b/openedx/core/djangoapps/authz/tests/mixins.py new file mode 100644 index 000000000000..c2dc32277b2a --- /dev/null +++ b/openedx/core/djangoapps/authz/tests/mixins.py @@ -0,0 +1,95 @@ +""" Mixins for testing course-scoped AuthZ endpoints. """ + +import casbin +import pkg_resources + +from unittest.mock import patch +from rest_framework.test import APIClient +from openedx_authz.api.users import assign_role_to_user_in_scope +from openedx_authz.constants.roles import COURSE_STAFF +from openedx_authz.engine.enforcer import AuthzEnforcer +from openedx_authz.engine.utils import migrate_policy_between_enforcers + +from openedx.core import toggles as core_toggles +from common.djangoapps.student.tests.factories import UserFactory + + +class CourseAuthzTestMixin: + """ + Reusable mixin for testing course-scoped AuthZ endpoints. + """ + + authz_roles_to_assign = [COURSE_STAFF.external_key] + + @classmethod + def setUpClass(cls): + cls.toggle_patcher = patch.object( + core_toggles.AUTHZ_COURSE_AUTHORING_FLAG, + "is_enabled", + return_value=True + ) + cls.toggle_patcher.start() + + super().setUpClass() + + @classmethod + def tearDownClass(cls): + cls.toggle_patcher.stop() + super().tearDownClass() + + def setUp(self): + super().setUp() + + self._seed_database_with_policies() + + self.authorized_user = UserFactory() + self.unauthorized_user = UserFactory() + + for role in self.authz_roles_to_assign: + assign_role_to_user_in_scope( + self.authorized_user.username, + role, + str(self.course_key) + ) + + AuthzEnforcer.get_enforcer().load_policy() + + self.authorized_client = APIClient() + self.authorized_client.force_authenticate(user=self.authorized_user) + + self.unauthorized_client = APIClient() + self.unauthorized_client.force_authenticate(user=self.unauthorized_user) + + def add_user_to_role(self, user, role): + """Helper method to add a user to a role for the course.""" + assign_role_to_user_in_scope( + user.username, + role, + str(self.course_key) + ) + AuthzEnforcer.get_enforcer().load_policy() + + def tearDown(self): + super().tearDown() + AuthzEnforcer.get_enforcer().clear_policy() + + @classmethod + def _seed_database_with_policies(cls): + """Seed the database with AuthZ policies.""" + global_enforcer = AuthzEnforcer.get_enforcer() + global_enforcer.load_policy() + + model_path = pkg_resources.resource_filename( + "openedx_authz.engine", + "config/model.conf" + ) + + policy_path = pkg_resources.resource_filename( + "openedx_authz.engine", + "config/authz.policy" + ) + + migrate_policy_between_enforcers( + source_enforcer=casbin.Enforcer(model_path, policy_path), + target_enforcer=global_enforcer, + ) diff --git a/openedx/core/djangoapps/authz/tests/test_decorators.py b/openedx/core/djangoapps/authz/tests/test_decorators.py new file mode 100644 index 000000000000..014932fbc315 --- /dev/null +++ b/openedx/core/djangoapps/authz/tests/test_decorators.py @@ -0,0 +1,156 @@ +"""Tests for authz decorators.""" +from unittest.mock import Mock, patch + +from django.test import RequestFactory, TestCase +from opaque_keys.edx.locator import BlockUsageLocator, CourseLocator + +from openedx.core.djangoapps.authz.constants import LegacyAuthoringPermission +from openedx.core.djangoapps.authz.decorators import authz_permission_required, get_course_key +from openedx.core.lib.api.view_utils import DeveloperErrorResponseException + + +class AuthzPermissionRequiredDecoratorTests(TestCase): + """ + Tests focused on the authz_permission_required decorator behavior. + """ + + def setUp(self): + self.factory = RequestFactory() + self.course_key = CourseLocator("TestX", "TST101", "2025") + + self.user = Mock() + self.user.username = "testuser" + self.user.id = 1 + + self.view_instance = Mock() + + def _build_request(self): + request = self.factory.get("/test") + request.user = self.user + return request + + def test_view_executes_when_permission_granted(self): + """Decorator allows execution when permission check passes.""" + request = self._build_request() + + mock_view = Mock(return_value="success") + + with patch( + "openedx.core.djangoapps.authz.decorators.user_has_course_permission", + return_value=True, + ): + decorated = authz_permission_required("courses.view")(mock_view) + + result = decorated(self.view_instance, request, str(self.course_key)) + + self.assertEqual(result, "success") + mock_view.assert_called_once_with( + self.view_instance, + request, + self.course_key, + ) + + def test_view_executes_when_legacy_fallback_read(self): + """Decorator allows execution when AuthZ denies but legacy permission succeeds.""" + request = self._build_request() + + mock_view = Mock(return_value="success") + + with patch( + "openedx.core.djangoapps.authz.decorators.core_toggles.enable_authz_course_authoring", + return_value=False, + ), patch( + "openedx.core.djangoapps.authz.decorators.authz_api.is_user_allowed", + return_value=True, # Should not be used when AuthZ is disabled, but set to True just in case + ), patch( + "openedx.core.djangoapps.authz.constants.has_studio_read_access", + return_value=True, + ): + decorated = authz_permission_required( + "courses.view", + legacy_permission=LegacyAuthoringPermission.READ + )(mock_view) + + result = decorated(self.view_instance, request, str(self.course_key)) + + self.assertEqual(result, "success") + mock_view.assert_called_once() + + def test_view_executes_when_legacy_fallback_write(self): + """Decorator allows execution when AuthZ denies but legacy write permission succeeds.""" + request = self._build_request() + + mock_view = Mock(return_value="success") + + with patch( + "openedx.core.djangoapps.authz.decorators.core_toggles.enable_authz_course_authoring", + return_value=False, + ), patch( + "openedx.core.djangoapps.authz.decorators.authz_api.is_user_allowed", + return_value=True, # Should not be used when AuthZ is disabled, but set to True just in case + ), patch( + "openedx.core.djangoapps.authz.constants.has_studio_write_access", + return_value=True, + ): + decorated = authz_permission_required( + "courses.edit", + legacy_permission=LegacyAuthoringPermission.WRITE + )(mock_view) + + result = decorated(self.view_instance, request, str(self.course_key)) + + self.assertEqual(result, "success") + mock_view.assert_called_once() + + def test_access_denied_when_permission_fails(self): + """Decorator raises API error when permission fails.""" + request = self._build_request() + + mock_view = Mock() + + with patch( + "openedx.core.djangoapps.authz.decorators.user_has_course_permission", + return_value=False, + ): + decorated = authz_permission_required("courses.view")(mock_view) + + with self.assertRaises(DeveloperErrorResponseException) as context: + decorated(self.view_instance, request, str(self.course_key)) + + self.assertEqual(context.exception.response.status_code, 403) + mock_view.assert_not_called() + + def test_decorator_preserves_function_name(self): + """Decorator preserves wrapped function metadata.""" + + def sample_view(self, request, course_key): + return "ok" + + decorated = authz_permission_required("courses.view")(sample_view) + + self.assertEqual(decorated.__name__, "sample_view") + + +class GetCourseKeyTests(TestCase): + """Tests for the get_course_key function used in the authz decorators.""" + + def setUp(self): + self.course_key = CourseLocator("TestX", "TST101", "2025") + + def test_course_key_string(self): + """Valid course key string returns CourseKey.""" + result = get_course_key(str(self.course_key)) + + self.assertEqual(result, self.course_key) + + def test_usage_key_string(self): + """UsageKey string resolves to course key.""" + usage_key = BlockUsageLocator( + self.course_key, + "html", + "block1" + ) + + result = get_course_key(str(usage_key)) + + self.assertEqual(result, self.course_key)