diff --git a/CHANGELOG.rst b/CHANGELOG.rst index d9cf23cf..0fcf508a 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -14,6 +14,14 @@ Change Log Unreleased ********** +1.21.0 - 2026-07-14 +******************* + +Added +===== + +* Introduced a new REST API endpoint and utility functions to fetch course authoring waffle flag states. (#358) + 1.20.1 - 2026-07-03 ******************* diff --git a/docs/decisions/0015-expose-course-authoring-waffle-flag-state-via-rest-api.rst b/docs/decisions/0015-expose-course-authoring-waffle-flag-state-via-rest-api.rst new file mode 100644 index 00000000..d870b218 --- /dev/null +++ b/docs/decisions/0015-expose-course-authoring-waffle-flag-state-via-rest-api.rst @@ -0,0 +1,71 @@ +0015: Expose Course-Authoring Waffle Flag State via REST API +############################################################## + +Status +****** + +**Draft** + +Context +******* + +``authz.enable_course_authoring`` is a three-tier flag (`ADR 0010`_), where a course override wins over an org override, which in turn wins over the platform default. + +`Issue #340`_ and `issue #341`_ report that the admin-console MFE keeps showing Authoring-related roles, scopes, and role assignments even when this flag is off, since nothing currently checks it. Both issues ask for a simpler rule than the full cascade. The Authoring UI should show if the flag is on at any level (platform, org, or course), and hide only if it's off at every level. `A review comment on frontend-app-admin-console#176`_ lays out the fuller course/org/platform truth table this problem could ideally follow. + +`PR #361`_ attempted to enforce that full truth table directly inside ``PermissionValidationMeView`` and other REST API endpoints, checking the flag per scope on every request. Per `PR #361's own comment thread`_, those endpoints are release-blocking for Verawood, so baking precise per-scope flag logic into them risked correctness and performance on critical paths without enough test coverage across the framework to be confident in time for the release. That approach was reverted, and the team pivoted to `issue #358`_ instead, exposing the flag's raw state through a dedicated endpoint and letting the admin-console MFE apply the simpler #340/#341 rule itself, deferring precise per-scope filtering to a later cycle. + +Neither edx-toggles nor edx-platform expose a suitable public API for this client-facing use case. ``/api/toggles/v0/state/`` (`edx_toggles source`_) can expose override data, but it requires Django staff/admin access and exposes flag state broadly (not just ``authz.enable_course_authoring``). ``WaffleFlagOrgOverrideModel.override_value(name, key)`` and its course-level counterpart (`waffle_utils models source`_) each answer for one specific org or course, not "which orgs/courses have an override." + +Decision +******** + +1. Add ``GET /api/authz/v1/waffle-flag-states/``, backed by ``openedx_authz.utils.get_waffle_flag_states()``, returning the flag's global state plus every org and course that currently has an active override, split into 'on' and 'off' lists. +2. The admin-console MFE decides what to show using this response, applying the #340/#341 rule for this release. +3. This supersedes PR #361's approach of enforcing the full cascade inside REST API endpoints themselves, for this release. PR #361's per-scope logic (``is_scope_visible``/``has_visible_scope``) stays documented on that branch for a future cycle. +4. Making the REST API endpoints themselves aware of the flag is still an open problem, and needs to be addressed on its own. Given the release timeline and the risk PR #361 surfaced, the team chose this more straightforward solution for now. + +Consequences +************ + +#. **Release-blocking endpoints stay untouched.** ``PermissionValidationMeView`` and the other endpoints named in PR #361 keep their existing behavior. This endpoint is additive, isolated, low-risk. +#. **One place answers "what's the flag's state right now."** ``get_waffle_flag_states()`` centralizes the lookup, reusing ``enable_authz_course_authoring()`` for the global tier and querying ``WaffleFlagOrgOverrideModel``/``WaffleFlagCourseOverrideModel`` directly for the org/course tiers, since no public API answers "which orgs/courses have an override." +#. **The MFE bears the filtering complexity.** Applying the #340/#341 "any tier on" rule, and any future precise per-course/per-org filtering, is MFE-side logic from here on. +#. **These override queries scan the whole table, unfiltered by any specific org/course.** For instances with many overrides, this is a full-table read on every call. Not a problem at current scale, but worth revisiting if usage grows (see `issue #360`_). +#. **``openedx_authz.utils`` now depends on** ``common.djangoapps.student.roles.enable_authz_course_authoring`` **and** ``openedx.core.djangoapps.waffle_utils.models``, guarded by the same standalone-import pattern already used elsewhere in this repo (``rest_api/utils.py``, ``handlers.py``). This is a temporary, direct edx-platform dependency, tracked as follow-up work under `issue #360`_ (moving the dependency direction so services depend on ``openedx_authz``). + +Rejected Alternatives +********************** + +**Enforcing the full per-scope truth table inside release-blocking REST API endpoints (PR #361)** + Correctness and performance across the whole framework weren't validated in time for a release-blocking change, per PR #361's own comment thread. The simpler #340/#341 rule doesn't need per-scope precision to ship. + +**Relying on** ``/api/toggles/v0/state/`` + This edx-toggles endpoint can expose override data, but it requires Django staff/admin access and is not suitable for this use case. It also exposes flag state broadly (not just ``authz.enable_course_authoring``), which is a security risk for this use case. + +References +********** + +* `ADR 0010`_ +* `Issue #340`_ +* `Issue #341`_ +* `Issue #358`_ +* `Issue #360`_ +* `PR #361`_ +* `PR #361's own comment thread`_ +* `A review comment on frontend-app-admin-console#176`_ + +.. _ADR 0010: 0010-course-authoring-flag.rst +.. _Issue #340: https://github.com/openedx/openedx-authz/issues/340 +.. _issue #340: https://github.com/openedx/openedx-authz/issues/340 +.. _Issue #341: https://github.com/openedx/openedx-authz/issues/341 +.. _issue #341: https://github.com/openedx/openedx-authz/issues/341 +.. _Issue #358: https://github.com/openedx/openedx-authz/issues/358 +.. _issue #358: https://github.com/openedx/openedx-authz/issues/358 +.. _Issue #360: https://github.com/openedx/openedx-authz/issues/360 +.. _issue #360: https://github.com/openedx/openedx-authz/issues/360 +.. _PR #361: https://github.com/openedx/openedx-authz/pull/361 +.. _PR #361's own comment thread: https://github.com/openedx/openedx-authz/pull/361#issuecomment-4967053225 +.. _A review comment on frontend-app-admin-console#176: https://github.com/openedx/frontend-app-admin-console/pull/176#issuecomment-4900922914 +.. _edx_toggles source: https://github.com/openedx/edx-toggles/blob/master/edx_toggles/toggles/state/internal/report.py +.. _waffle_utils models source: https://github.com/openedx/edx-platform/blob/master/openedx/core/djangoapps/waffle_utils/models.py diff --git a/openedx_authz/__init__.py b/openedx_authz/__init__.py index 469028cb..2c1e6d26 100644 --- a/openedx_authz/__init__.py +++ b/openedx_authz/__init__.py @@ -4,6 +4,6 @@ import os -__version__ = "1.20.1" +__version__ = "1.21.0" ROOT_DIRECTORY = os.path.dirname(os.path.abspath(__file__)) diff --git a/openedx_authz/rest_api/v1/urls.py b/openedx_authz/rest_api/v1/urls.py index 6c7def28..e31e5d79 100644 --- a/openedx_authz/rest_api/v1/urls.py +++ b/openedx_authz/rest_api/v1/urls.py @@ -20,4 +20,5 @@ ), path("assignments/", views.AssignmentsAPIView.as_view(), name="assignment-list"), path("scopes/", views.ScopesAPIView.as_view(), name="scope-list"), + path("waffle-flag-states/", views.WaffleFlagStatesAPIView.as_view(), name="waffle-flag-states"), ] diff --git a/openedx_authz/rest_api/v1/views.py b/openedx_authz/rest_api/v1/views.py index 96686338..eb528cd4 100644 --- a/openedx_authz/rest_api/v1/views.py +++ b/openedx_authz/rest_api/v1/views.py @@ -77,7 +77,7 @@ UserValidationAPIViewResponseSerializer, UserValidationAPIViewSerializer, ) -from openedx_authz.utils import get_user_by_username_or_email +from openedx_authz.utils import get_user_by_username_or_email, get_waffle_flag_states logger = logging.getLogger(__name__) @@ -1374,3 +1374,34 @@ def get(self, request: HttpRequest) -> Response: paginator = self.pagination_class() paginated_response_data = paginator.paginate_queryset(assignments, request) return paginator.get_paginated_response(paginated_response_data) + + +@view_auth_classes() +class WaffleFlagStatesAPIView(APIView): + """ + Simple API view that returns the waffle flag states from utils.get_waffle_flag_states. + + **Endpoints** + + - GET: Retrieve the enablement state of the course-authoring waffle flag across different scopes. + + **Response Format** + + * 'global' (bool): True if the global waffle flag is enabled. + * 'org_overrides' (dict): Orgs with an organization-level override, as 'on' + (forces the flag on) and 'off' (forces the flag off) lists. + * 'course_overrides' (dict): Courses with a course-level override, split the same way. + + **Example Request** + + GET /api/authz/v1/waffle-flag-states/ + """ + + def get(self, request: HttpRequest) -> Response: + """Retrieve the enablement state of the course-authoring waffle flag across different scopes.""" + try: + data = get_waffle_flag_states() + return Response(data, status=status.HTTP_200_OK) + except Exception as e: # pylint: disable=broad-exception-caught + logger.exception("Error getting waffle flag states: %s", e) + return Response({"message": "error"}, status=status.HTTP_500_INTERNAL_SERVER_ERROR) diff --git a/openedx_authz/tests/rest_api/test_views.py b/openedx_authz/tests/rest_api/test_views.py index e7632817..30bb173f 100644 --- a/openedx_authz/tests/rest_api/test_views.py +++ b/openedx_authz/tests/rest_api/test_views.py @@ -4338,3 +4338,54 @@ def test_scope_permission_vs_platform_permission(self, username, scopes, expecte response = self._put_lib(scopes, roles.LIBRARY_ADMIN.external_key) self.assertEqual(response.status_code, expected_status) + + +class TestWaffleFlagStatesAPIView(ViewTestMixin): + """Test suite for WaffleFlagStatesAPIView.""" + + def setUp(self): + """Set up test fixtures.""" + super().setUp() + self.url = reverse("openedx_authz:waffle-flag-states") + + def test_get_returns_the_waffle_flag_states(self): + """Test GET /waffle-flag-states/ with a successful lookup. + + Expected result: + - Returns 200 OK status. + - The response body is whatever get_waffle_flag_states returns, unchanged. + """ + flag_states = { + "global": True, + "org_overrides": {"on": ["Org1"], "off": []}, + "course_overrides": {"on": [], "off": ["course-v1:Org1+COURSE1+2024"]}, + } + with patch("openedx_authz.rest_api.v1.views.get_waffle_flag_states", return_value=flag_states): + response = self.client.get(self.url) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.data, flag_states) + + def test_get_handles_an_unexpected_error(self): + """Test GET /waffle-flag-states/ when get_waffle_flag_states raises. + + Expected result: + - Returns 500 INTERNAL SERVER ERROR status with a generic error message. + """ + with patch("openedx_authz.rest_api.v1.views.get_waffle_flag_states", side_effect=Exception("boom")): + response = self.client.get(self.url) + + self.assertEqual(response.status_code, status.HTTP_500_INTERNAL_SERVER_ERROR) + self.assertEqual(response.data, {"message": "error"}) + + def test_get_requires_authentication(self): + """Test GET /waffle-flag-states/ without authentication. + + Expected result: + - Returns 401 UNAUTHORIZED status. + """ + self.client.force_authenticate(user=None) + + response = self.client.get(self.url) + + self.assertEqual(response.status_code, status.HTTP_401_UNAUTHORIZED) diff --git a/openedx_authz/tests/test_utils.py b/openedx_authz/tests/test_utils.py index e3497143..7c68574b 100644 --- a/openedx_authz/tests/test_utils.py +++ b/openedx_authz/tests/test_utils.py @@ -1,5 +1,11 @@ """Test utilities for creating namespaced keys using class constants.""" +from types import SimpleNamespace +from unittest.mock import MagicMock, patch + +from ddt import data, ddt, unpack +from django.test import TestCase + from openedx_authz.api.data import ( GLOBAL_SCOPE_WILDCARD, ActionData, @@ -9,6 +15,9 @@ ScopeData, UserData, ) +from openedx_authz.utils import get_waffle_flag_states + +FLAG_NAME = "authz.enable_course_authoring" def make_policy(role_key: str, action_key: str, scope_key: str, effect: str = "allow") -> list[str]: @@ -187,3 +196,129 @@ def make_wildcard_key(namespace: str) -> str: str: Wildcard pattern (e.g., 'lib^*', 'org^*', 'course^*') """ return f"{namespace}{ScopeData.SEPARATOR}{GLOBAL_SCOPE_WILDCARD}" + + +@ddt +class TestGetWaffleFlagStates(TestCase): + """Test get_waffle_flag_states, which reports the course-authoring flag's state at each tier.""" + + def _mock_override_model(self, override_rows: list): + """Build a mock override model. override_rows is a list of (key, override_choice) tuples.""" + mock_model = MagicMock() + mock_model.objects.current_set.return_value.filter.return_value.values_list.return_value = override_rows + return mock_model + + @data(True, False) + def test_global_tier_follows_the_platform_flag(self, platform_enabled: bool): + """Test get_waffle_flag_states' global key. + + Expected result: + - Matches global waffle flag result. + """ + with patch( + "openedx_authz.utils.Flag", + MagicMock(objects=MagicMock( + filter=MagicMock(return_value=MagicMock(first=MagicMock( + return_value=SimpleNamespace(everyone=platform_enabled) + ))) + )), + ), patch( + "openedx_authz.utils.AUTHZ_COURSE_AUTHORING_FLAG", SimpleNamespace(name=FLAG_NAME) + ), patch( + "openedx_authz.utils.WaffleFlagOrgOverrideModel", self._mock_override_model([]) + ), patch( + "openedx_authz.utils.WaffleFlagCourseOverrideModel", self._mock_override_model([]) + ): + self.assertEqual(get_waffle_flag_states()["global"], platform_enabled) + + @data( + ([("Org1", "on")], {"on": ["Org1"], "off": []}), + ([("Org1", "off")], {"on": [], "off": ["Org1"]}), + ([("Org1", "on"), ("Org2", "off")], {"on": ["Org1"], "off": ["Org2"]}), + ([], {"on": [], "off": []}), + ) + @unpack + def test_org_tier_splits_active_overrides_by_choice(self, override_rows: list, expected: dict): + """Test get_waffle_flag_states' org key. + + Expected result: + - Orgs with an enabled override are split into 'on' and 'off' lists, + by the override's choice. + """ + with patch( + "openedx_authz.utils.Flag", + MagicMock(objects=MagicMock( + filter=MagicMock(return_value=MagicMock(first=MagicMock( + return_value=SimpleNamespace(everyone=False) + ))) + )), + ), patch( + "openedx_authz.utils.AUTHZ_COURSE_AUTHORING_FLAG", SimpleNamespace(name=FLAG_NAME) + ), patch( + "openedx_authz.utils.WaffleFlagOrgOverrideModel", self._mock_override_model(override_rows) + ), patch( + "openedx_authz.utils.WaffleFlagCourseOverrideModel", self._mock_override_model([]) + ): + self.assertEqual(get_waffle_flag_states()["org_overrides"], expected) + + @data( + ([("course-v1:Org1+COURSE1+2024", "on")], {"on": ["course-v1:Org1+COURSE1+2024"], "off": []}), + ([("course-v1:Org1+COURSE1+2024", "off")], {"on": [], "off": ["course-v1:Org1+COURSE1+2024"]}), + ( + [("course-v1:Org1+COURSE1+2024", "on"), ("course-v1:Org1+COURSE2+2024", "off")], + {"on": ["course-v1:Org1+COURSE1+2024"], "off": ["course-v1:Org1+COURSE2+2024"]}, + ), + ([], {"on": [], "off": []}), + ) + @unpack + def test_course_tier_splits_active_overrides_by_choice(self, override_rows: list, expected: dict): + """Test get_waffle_flag_states' course key. + + Expected result: + - Courses with an enabled override are split into 'on' and 'off' lists, + by the override's choice. Course keys are stringified. + """ + with patch( + "openedx_authz.utils.Flag", + MagicMock(objects=MagicMock( + filter=MagicMock(return_value=MagicMock(first=MagicMock( + return_value=SimpleNamespace(everyone=False) + ))) + )), + ), patch( + "openedx_authz.utils.AUTHZ_COURSE_AUTHORING_FLAG", SimpleNamespace(name=FLAG_NAME) + ), patch( + "openedx_authz.utils.WaffleFlagOrgOverrideModel", self._mock_override_model([]) + ), patch( + "openedx_authz.utils.WaffleFlagCourseOverrideModel", self._mock_override_model(override_rows) + ): + self.assertEqual(get_waffle_flag_states()["course_overrides"], expected) + + def test_all_three_tiers_are_independent(self): + """Test get_waffle_flag_states with each tier in a different state. + + Expected result: + - Each key reflects only its own tier, not a blend of the others. + """ + with patch( + "openedx_authz.utils.Flag", + MagicMock(objects=MagicMock( + filter=MagicMock(return_value=MagicMock(first=MagicMock( + return_value=SimpleNamespace(everyone=False) + ))) + )), + ), patch( + "openedx_authz.utils.AUTHZ_COURSE_AUTHORING_FLAG", SimpleNamespace(name=FLAG_NAME) + ), patch( + "openedx_authz.utils.WaffleFlagOrgOverrideModel", self._mock_override_model([("Org1", "on")]) + ), patch( + "openedx_authz.utils.WaffleFlagCourseOverrideModel", self._mock_override_model([]) + ): + self.assertEqual( + get_waffle_flag_states(), + { + "global": False, + "org_overrides": {"on": ["Org1"], "off": []}, + "course_overrides": {"on": [], "off": []}, + }, + ) diff --git a/openedx_authz/utils.py b/openedx_authz/utils.py index 12ec3c47..87f0e780 100644 --- a/openedx_authz/utils.py +++ b/openedx_authz/utils.py @@ -4,6 +4,23 @@ from django.db.models import Q from edx_django_utils.cache import RequestCache +try: + from openedx.core.djangoapps.waffle_utils.models import ( + WaffleFlagCourseOverrideModel, + WaffleFlagOrgOverrideModel, + ) + from openedx.core.toggles import AUTHZ_COURSE_AUTHORING_FLAG +except ImportError: + WaffleFlagCourseOverrideModel = None + WaffleFlagOrgOverrideModel = None + AUTHZ_COURSE_AUTHORING_FLAG = None + +from waffle.models import Flag + +# Match handlers.py semantics: an override forces ON when override_choice == "on" +WAFFLE_OVERRIDE_FORCE_ON = "on" +WAFFLE_OVERRIDE_FORCE_OFF = "off" + User = get_user_model() _STAFF_SUPERUSER_CACHE_NAMESPACE = "rbac_is_staff_or_superuser" @@ -50,3 +67,52 @@ def get_user_by_username_or_email(username_or_email: str) -> User: if hasattr(user, "userretirementrequest"): raise User.DoesNotExist return user + + +def get_waffle_flag_states() -> dict: + """ + Retrieve the enablement state of the course-authoring waffle flag across different scopes. + + Returns: + dict: A dictionary mapping scopes to their activation status: + * 'global' (bool): True if the global waffle flag is enabled for everyone. + * 'org_overrides' (dict): Orgs with an organization-level override, as 'on' + (forces the flag on) and 'off' (forces the flag off) lists. + * 'course_overrides' (dict): Courses with a course-level override, split the same way. + """ + # Global flag (falls back False if toggle not available) + global_enabled = False + if AUTHZ_COURSE_AUTHORING_FLAG is not None: + gf = Flag.objects.filter(name=AUTHZ_COURSE_AUTHORING_FLAG.name).first() + global_enabled = bool(gf and gf.everyone) + + # There's no public edx-platform API to get which orgs/courses have an override, only + # override_value(name, key) for one specific org/course at a time, so this queries the + # model directly. This is a temporary solution which should be addressed by + # https://github.com/openedx/openedx-authz/issues/360 + org_override_rows = list( + WaffleFlagOrgOverrideModel.objects.current_set() + .filter(waffle_flag=AUTHZ_COURSE_AUTHORING_FLAG.name, enabled=True) + .values_list("org", "override_choice") + ) + course_override_rows = list( + WaffleFlagCourseOverrideModel.objects.current_set() + .filter(waffle_flag=AUTHZ_COURSE_AUTHORING_FLAG.name, enabled=True) + .values_list("course_id", "override_choice") + ) + + return { + "global": global_enabled, + "org_overrides": { + "on": [org for org, choice in org_override_rows if choice == WAFFLE_OVERRIDE_FORCE_ON], + "off": [org for org, choice in org_override_rows if choice == WAFFLE_OVERRIDE_FORCE_OFF], + }, + "course_overrides": { + "on": [ + str(course_id) for course_id, choice in course_override_rows if choice == WAFFLE_OVERRIDE_FORCE_ON + ], + "off": [ + str(course_id) for course_id, choice in course_override_rows if choice == WAFFLE_OVERRIDE_FORCE_OFF + ], + }, + }