feat: enforce AuthZ permissions in the Instructor Dashboard API v2

lms/djangoapps/instructor/authz/README.rst

@@ -0,0 +1,125 @@
+Instructor Dashboard AuthZ
+##########################
+
+Integration of the Instructor Dashboard (``/api/instructor/v2``) with ``openedx-authz``.
+
+What it contains
+****************
+
+- ``constants.py``: the permission identifiers. Do not repeat the strings in the code. TEMPORARY: the permissions not defined by openedx-authz depend on a separate issue and are declared here only for testing.
+- ``helpers.py``: ``user_has_instructor_permission`` and ``get_instructor_authz_permissions``.
+- ``permissions.py``: ``InstructorAuthzPermission``, the DRF permission class for the views.
+- ``legacy.py``: the pre-RBAC check equivalent to each permission.
+- ``tests/mixins.py``: ``InstructorAuthzTestMixin`` to assign AuthZ roles in tests.
+
+Behavior
+********
+
+It is driven by the ``authz.enable_course_authoring`` waffle flag, evaluated per course, with the same semantics as Studio:
+
+- **Flag enabled:** only AuthZ is consulted. Legacy roles are ignored (openedx-authz already grants the global staff and superuser bypass).
+- **Flag disabled:** only the legacy check runs, so the previous behavior is unchanged.
+
+Every decision is logged with the path that resolved it (``authz`` or ``legacy``) in ``access_path``.
+
+Migrating a view
+****************
+
+Replace ``InstructorPermission`` by ``InstructorAuthzPermission`` and declare the AuthZ permission next to the legacy one. ``course_id`` must be a URL kwarg: the permission class reads it from ``view.kwargs`` and returns a ``404`` if it is not a valid course key.
+
+Single permission
+=================
+
+The same AuthZ permission for every HTTP method, with the legacy bridgekeeper permission the view already used:
+
+.. code-block:: python
+
+    from lms.djangoapps.instructor.authz import constants as authz_constants
+    from lms.djangoapps.instructor.authz.permissions import InstructorAuthzPermission
+
+    class MyView(APIView):
+        permission_classes = (IsAuthenticated, InstructorAuthzPermission)
+        authz_permission = authz_constants.COURSES_VIEW_ENROLLMENTS
+        # The legacy bridgekeeper permission.
+        permission_name = permissions.VIEW_ENROLLMENTS
+
+Different permission per HTTP method
+====================================
+
+Use a mapping when reading and changing need different permissions. A method missing from the mapping skips AuthZ and is checked with the legacy rule only:
+
+.. code-block:: python
+
+    class MyView(APIView):
+        permission_classes = (IsAuthenticated, InstructorAuthzPermission)
+        authz_permission = {
+            "GET": authz_constants.COURSES_VIEW_ENROLLMENTS,
+            "POST": authz_constants.COURSES_ENROLL_LEARNERS,
+        }
+        permission_name = permissions.CAN_ENROLL
+
+Any of several permissions
+==========================
+
+Declare a collection when holding any one of them is enough. With the flag disabled the legacy check of the view decides, evaluated once:
+
+.. code-block:: python
+
+    class MyView(APIView):
+        permission_classes = (IsAuthenticated, InstructorAuthzPermission)
+        authz_permission = authz_constants.INSTRUCTOR_DASHBOARD_PERMISSIONS
+        permission_name = permissions.VIEW_DASHBOARD
+
+Legacy rule that is not a bridgekeeper permission
+=================================================
+
+Define ``legacy_permission_check(user, course_key)`` on the view. It replaces ``permission_name`` and receives the same arguments for every method. Reuse the checks of ``legacy.py`` when one matches:
+
+.. code-block:: python
+
+    from lms.djangoapps.instructor.authz import legacy as authz_legacy
+
+    class MyView(APIView):
+        permission_classes = (IsAuthenticated, InstructorAuthzPermission)
+        authz_permission = {
+            "GET": authz_constants.COURSES_VIEW_COHORTS,
+            "POST": authz_constants.COURSES_MANAGE_COHORTS,
+        }
+
+        @staticmethod
+        def legacy_permission_check(user, course_key):
+            return authz_legacy.has_cohort_manage_access(user, course_key)
+
+Without ``@staticmethod`` the method would receive ``self`` as the first argument. The legacy check does not receive the request, so it cannot vary by HTTP method. If the legacy rule differs per method, call ``user_has_instructor_permission`` in the handler with the legacy check of each case.
+
+Checking a permission in other code
+===================================
+
+Use ``user_has_instructor_permission`` inside a handler, a serializer or any other code. The legacy check is optional and defaults to the one registered in ``legacy.LEGACY_PERMISSION_CHECKS``:
+
+.. code-block:: python
+
+    from opaque_keys.edx.keys import CourseKey
+    from lms.djangoapps.instructor.authz.helpers import user_has_instructor_permission
+
+    def post(self, request, course_id):
+        course_key = CourseKey.from_string(course_id)
+        if user_has_instructor_permission(request.user, authz_constants.COURSES_MANAGE_COHORTS, course_key):
+            ...
+
+        # With an explicit legacy check, used only when the flag is disabled.
+        user_has_instructor_permission(
+            request.user,
+            authz_constants.COURSES_VIEW_ENROLLMENTS,
+            course_key,
+            authz_legacy.has_instructor_access,
+        )
+
+To get the effective value of several permissions at once (as ``authz_permissions`` in ``CourseMetadataView``) use ``get_instructor_authz_permissions(user, course_key)``.
+
+Errors
+======
+
+- No ``authz_permission`` on the view: ``ImproperlyConfigured``.
+- Invalid ``course_id``: ``404``.
+- Denied by the active path (AuthZ with the flag enabled, legacy otherwise): ``403``.

lms/djangoapps/instructor/authz/__init__.py

@@ -0,0 +1,3 @@
+"""
+AuthZ integration for the Instructor Dashboard.
+"""

lms/djangoapps/instructor/authz/constants.py

@@ -0,0 +1,78 @@
+"""
+AuthZ permission identifiers used by the Instructor Dashboard.
+
+The ``courses.manage_course_team``, ``courses.view_course_team`` and
+``courses.view_grading_settings`` permissions are shared with Authoring and
+imported from openedx-authz.
+
+TEMPORARY: the rest depend on a separate issue that will define them in
+openedx-authz. They are declared here only for testing.
+"""
+
+from openedx_authz.constants import permissions as authz_permissions
+
+# Course info
+COURSES_VIEW_COURSE_INFO = "courses.view_course_info"
+
+# Enrollments
+COURSES_VIEW_ENROLLMENTS = "courses.view_enrollments"
+COURSES_ENROLL_LEARNERS = "courses.enroll_learners"
+COURSES_UNENROLL_LEARNERS = "courses.unenroll_learners"
+
+# Course team (shared with Authoring)
+COURSES_VIEW_COURSE_TEAM = authz_permissions.COURSES_VIEW_COURSE_TEAM.identifier
+COURSES_MANAGE_COURSE_TEAM = authz_permissions.COURSES_MANAGE_COURSE_TEAM.identifier
+
+# Grading
+COURSES_VIEW_GRADEBOOK = "courses.view_gradebook"
+COURSES_VIEW_GRADING_CONFIGURATION = "courses.view_grading_configuration"
+COURSES_VIEW_GRADING_SETTINGS = authz_permissions.COURSES_VIEW_GRADING_SETTINGS.identifier
+COURSES_MANAGE_LEARNER_GRADING = "courses.manage_learner_grading"
+
+# Date extensions
+COURSES_VIEW_LEARNER_EXTENSIONS = "courses.view_learner_extensions"
+COURSES_MANAGE_LEARNER_EXTENSIONS = "courses.manage_learner_extensions"
+
+# Cohorts
+COURSES_VIEW_COHORTS = "courses.view_cohorts"
+COURSES_MANAGE_COHORTS = "courses.manage_cohorts"
+
+# Every permission whose effective value is returned to the MFE in
+# ``authz_permissions``.
+INSTRUCTOR_DASHBOARD_PERMISSIONS = (
+    COURSES_VIEW_COURSE_INFO,
+    COURSES_VIEW_ENROLLMENTS,
+    COURSES_ENROLL_LEARNERS,
+    COURSES_UNENROLL_LEARNERS,
+    COURSES_VIEW_COURSE_TEAM,
+    COURSES_MANAGE_COURSE_TEAM,
+    COURSES_VIEW_GRADEBOOK,
+    COURSES_VIEW_GRADING_CONFIGURATION,
+    COURSES_VIEW_GRADING_SETTINGS,
+    COURSES_MANAGE_LEARNER_GRADING,
+    COURSES_VIEW_LEARNER_EXTENSIONS,
+    COURSES_MANAGE_LEARNER_EXTENSIONS,
+    COURSES_VIEW_COHORTS,
+    COURSES_MANAGE_COHORTS,
+)
+
+# Permissions that give access to each tab of the dashboard. A tab is shown when
+# the user holds at least one of them, not a specific one.
+TAB_PERMISSIONS = {
+    "course_info": (COURSES_VIEW_COURSE_INFO,),
+    "enrollments": (
+        COURSES_VIEW_ENROLLMENTS,
+        COURSES_ENROLL_LEARNERS,
+        COURSES_UNENROLL_LEARNERS,
+        COURSES_MANAGE_COURSE_TEAM,  # Its included because the tab manages the beta testers.
+    ),
+    "course_team": (COURSES_VIEW_COURSE_TEAM, COURSES_MANAGE_COURSE_TEAM),
+    "grading": (
+        COURSES_VIEW_GRADEBOOK,
+        COURSES_VIEW_GRADING_CONFIGURATION,
+        COURSES_VIEW_GRADING_SETTINGS,
+        COURSES_MANAGE_LEARNER_GRADING,
+    ),
+    "date_extensions": (COURSES_VIEW_LEARNER_EXTENSIONS, COURSES_MANAGE_LEARNER_EXTENSIONS),
+    "cohorts": (COURSES_VIEW_COHORTS, COURSES_MANAGE_COHORTS),
+}

lms/djangoapps/instructor/authz/helpers.py

@@ -0,0 +1,144 @@
+"""
+Helpers to enforce AuthZ permissions in the Instructor Dashboard.
+
+When the ``authz.enable_course_authoring`` flag is enabled for a course, the
+permission is resolved exclusively through AuthZ (same semantics as Studio, see
+``openedx.core.djangoapps.authz.decorators``). When it is disabled, the legacy
+check is used, so the pre-RBAC behavior is preserved.
+"""
+
+import logging
+from collections.abc import Callable, Iterable
+
+from django.contrib.auth.models import AbstractUser
+from opaque_keys.edx.keys import CourseKey
+from openedx_authz import api as authz_api
+
+from common.djangoapps.student.roles import enable_authz_course_authoring
+from lms.djangoapps.instructor.authz.constants import INSTRUCTOR_DASHBOARD_PERMISSIONS
+from lms.djangoapps.instructor.authz.legacy import LEGACY_PERMISSION_CHECKS
+
+log = logging.getLogger(__name__)
+
+# Signature of a legacy check: ``(user, course_key) -> bool``.
+LegacyCheck = Callable[[AbstractUser, CourseKey], bool]
+
+ACCESS_PATH_AUTHZ = "authz"
+ACCESS_PATH_LEGACY = "legacy"
+
+
+def user_has_instructor_permission(
+    user: AbstractUser,
+    authz_permission: str,
+    course_key: CourseKey,
+    legacy_check: LegacyCheck | None = None,
+) -> bool:
+    """
+    Return whether the user holds an AuthZ permission in the course.
+
+    When ``authz.enable_course_authoring`` is enabled for the course, only AuthZ
+    is consulted and legacy roles are ignored. When it is disabled, only the
+    legacy check is evaluated. The path that resolved the check (``authz`` or
+    ``legacy``) is logged.
+
+    Args:
+        user (AbstractUser): The user whose access is checked.
+        authz_permission (str): The AuthZ permission identifier, e.g.
+            ``courses.view_enrollments``.
+        course_key (CourseKey): The course where the permission is checked.
+        legacy_check (LegacyCheck | None): The pre-RBAC check, used only when
+            AuthZ is disabled for the course. If omitted, the default one
+            registered for ``authz_permission`` in ``LEGACY_PERMISSION_CHECKS``
+            is used, and a permission without any legacy check is denied.
+
+    Returns:
+        bool: True if the user holds the permission.
+    """
+    if enable_authz_course_authoring(course_key):
+        is_allowed = authz_api.is_user_allowed(user.username, authz_permission, str(course_key))
+        _log_decision(ACCESS_PATH_AUTHZ, is_allowed, user, authz_permission, course_key)
+        return is_allowed
+
+    legacy_check = legacy_check or LEGACY_PERMISSION_CHECKS.get(authz_permission)
+    is_allowed = bool(legacy_check and legacy_check(user, course_key))
+    _log_decision(ACCESS_PATH_LEGACY, is_allowed, user, authz_permission, course_key)
+    return is_allowed
+
+
+def user_has_any_instructor_permission(
+    user: AbstractUser,
+    authz_permissions: Iterable[str],
+    course_key: CourseKey,
+    legacy_check: LegacyCheck | None = None,
+) -> bool:
+    """
+    Return whether the user holds at least one of the AuthZ permissions.
+
+    It is meant for the visibility of a tab, which is shown when the user can do
+    anything in it, not only a specific action. With the flag enabled for the
+    course, only AuthZ is consulted. With it disabled, ``legacy_check`` is used
+    if given; otherwise the default legacy check of each permission is tried.
+
+    Args:
+        user (AbstractUser): The user whose access is checked.
+        authz_permissions (Iterable[str]): The AuthZ permission identifiers, any
+            of which grants access.
+        course_key (CourseKey): The course where the permissions are checked.
+        legacy_check (LegacyCheck | None): The pre-RBAC check of the whole set,
+            used only when AuthZ is disabled for the course.
+
+    Returns:
+        bool: True if the user holds at least one of the permissions.
+    """
+    if legacy_check is not None and not enable_authz_course_authoring(course_key):
+        # The legacy check covers the whole set; evaluate and log it once.
+        return user_has_instructor_permission(user, next(iter(authz_permissions)), course_key, legacy_check)
+    return any(
+        user_has_instructor_permission(user, permission, course_key, legacy_check) for permission in authz_permissions
+    )
+
+
+def get_instructor_authz_permissions(
+    user: AbstractUser, course_key: CourseKey, permissions: Iterable[str] = INSTRUCTOR_DASHBOARD_PERMISSIONS
+) -> dict[str, bool]:
+    """
+    Return the effective value of each permission for the user in the course.
+
+    Each value follows the same rules as ``user_has_instructor_permission``,
+    using the default legacy check of the permission.
+
+    Args:
+        user (AbstractUser): The user whose permissions are resolved.
+        course_key (CourseKey): The course where the permissions are resolved.
+        permissions (Iterable[str]): The permission identifiers to resolve.
+            Defaults to all the Instructor Dashboard ones.
+
+    Returns:
+        dict[str, bool]: Mapping of permission identifier to its value, e.g.
+        ``{"courses.enroll_learners": True}``.
+    """
+    return {permission: user_has_instructor_permission(user, permission, course_key) for permission in permissions}
+
+
+def _log_decision(path: str, is_allowed: bool, user: AbstractUser, authz_permission: str, course_key: CourseKey):
+    """
+    Log the path that resolved a permission check, and its outcome.
+
+    Args:
+        path (str): The path that resolved the check.
+        is_allowed (bool): Whether access was granted.
+        user (AbstractUser): The user checked.
+        authz_permission (str): The AuthZ permission identifier checked.
+        course_key (CourseKey): The course where it was checked.
+    """
+    log.debug(
+        "Instructor dashboard permission %s via %s",
+        "granted" if is_allowed else "denied",
+        path,
+        extra={
+            "user_id": user.id,
+            "authz_permission": authz_permission,
+            "course_key": str(course_key),
+            "access_path": path,
+        },
+    )