diff --git a/openedx/core/djangoapps/content/search/api.py b/openedx/core/djangoapps/content/search/api.py index 6d6ce6148cd2..dbd71fa6a947 100644 --- a/openedx/core/djangoapps/content/search/api.py +++ b/openedx/core/djangoapps/content/search/api.py @@ -38,7 +38,11 @@ INDEX_SEARCHABLE_ATTRIBUTES, INDEX_SORTABLE_ATTRIBUTES, ) -from openedx.core.djangoapps.content.search.models import IncrementalIndexCompleted, get_access_ids_for_request +from openedx.core.djangoapps.content.search.models import ( + IncrementalIndexCompleted, + get_access_ids_for_request, + get_authz_org_keys, +) from openedx.core.djangoapps.content_libraries import api as lib_api from xmodule.modulestore.django import modulestore from xmodule.modulestore.exceptions import ItemNotFoundError @@ -1069,11 +1073,18 @@ def _get_user_orgs(request: Request) -> list[str]: Get the org.short_names for the organizations that the requesting user has OrgStaffRole or OrgInstructorRole. Note: org-level roles have course_id=None to distinguish them from course-level roles. + + Also includes orgs where the user holds an org-wide (glob) authz course role, so that + authz-only users granted at the org level (e.g. ``course-v1:Org+*``) are covered by the + ``org IN [...]`` search filter clause rather than being dropped. """ course_roles = get_course_roles(request.user) - return list( - set(role.org for role in course_roles if role.course_id is None and role.role in ["staff", "instructor"]) + orgs = set( + role.org for role in course_roles if role.course_id is None and role.role in ["staff", "instructor"] ) + # Union in org-level authz grants (flag-gated per org inside the helper). + orgs.update(get_authz_org_keys(request.user.username, omit_orgs=list(orgs))) + return list(orgs) def _get_meili_access_filter(request: Request) -> dict: diff --git a/openedx/core/djangoapps/content/search/models.py b/openedx/core/djangoapps/content/search/models.py index d726f1ead057..ac9edcbb22bd 100644 --- a/openedx/core/djangoapps/content/search/models.py +++ b/openedx/core/djangoapps/content/search/models.py @@ -2,14 +2,24 @@ from __future__ import annotations -from django.db import models +import logging + +from django.db import DatabaseError, models from django.utils.translation import gettext_lazy as _ +from opaque_keys import InvalidKeyError from opaque_keys.edx.django.models import LearningContextKeyField +from opaque_keys.edx.keys import CourseKey +from openedx_authz.api.data import CourseOverviewData, OrgCourseOverviewGlobData +from openedx_authz.api.users import get_user_role_assignments from rest_framework.request import Request from common.djangoapps.student.role_helpers import get_course_roles from common.djangoapps.student.roles import CourseInstructorRole, CourseStaffRole +from openedx.core import toggles as core_toggles from openedx.core.djangoapps.content_libraries.api import get_libraries_for_user +from openedx.core.lib.cache_utils import request_cached + +log = logging.getLogger(__name__) class SearchAccess(models.Model): # noqa: DJ008 @@ -46,14 +56,20 @@ def get_access_ids_for_request(request: Request, omit_orgs: list[str] = None) -> omit_orgs = omit_orgs or [] course_roles = get_course_roles(request.user) - course_clause = models.Q(context_key__in=[ + course_keys = set( role.course_id for role in course_roles if ( role.role in [CourseInstructorRole.ROLE, CourseStaffRole.ROLE] and role.org not in omit_orgs ) - ]) + ) + + # When authz is enabled, also include courses where the user has an authz role assignment. + # This ensures authz-only users (editor/auditor without legacy roles) can search their courses. + course_keys.update(_get_authz_course_keys(request.user.username, omit_orgs)) + + course_clause = models.Q(context_key__in=list(course_keys)) libraries = get_libraries_for_user(user=request.user) library_clause = models.Q(context_key__in=[ @@ -69,6 +85,119 @@ def get_access_ids_for_request(request: Request, omit_orgs: list[str] = None) -> ) +@request_cached() +def _get_cached_authz_assignments(username: str): + """ + Returns the user's full authz role-assignment set, cached per request. + + ``get_user_role_assignments_per_scope_type`` fetches the user's entire + assignment set from the enforcer regardless of the scope types requested, + then filters in Python. Both search helpers below need a different slice of + that same set (per-course vs org-glob scopes), so caching the single + underlying whole-set fetch lets them each filter by scope type without + paying for a second enforcer round-trip within one request. + + Keyed on ``username`` only (the sole argument), so the two helpers share the + cache entry. Not wrapped in a try/except here: callers own the fail-open + decision, and ``request_cached`` never caches a raised exception, so a + transient ``DatabaseError`` on the first call is re-attempted on the second. + """ + return get_user_role_assignments(user_external_key=username) + + +def _get_authz_course_keys(username: str, omit_orgs: list[str]) -> set[str]: + """ + Returns serialized course keys from the user's authz role assignments where + the authz course authoring flag is enabled. + + Filters the per-request-cached whole assignment set to the per-course + (``CourseOverviewData``) scopes, then to only courses where the flag is + active (supporting both global enablement and per-course overrides). + + Keys are returned as strings (not ``CourseKey`` objects) to match the legacy + ``get_course_roles`` branch, whose ``course_id`` is already a string. This + keeps the unioned ``course_keys`` set type-homogeneous so a course held via + both a legacy and an authz role de-duplicates to a single entry. + + Fails open: if the authz lookup hits a database error, it is logged and an + empty set is returned so that search degrades to legacy-role access rather + than returning a 500. Any other (unexpected) exception propagates. + """ + try: + assignments = _get_cached_authz_assignments(username) + except DatabaseError as exc: + log.warning( + "Could not load authz role assignments for user %r; " + "falling back to legacy course roles for search access. Error: %s", + username, + exc, + ) + return set() + + course_keys = set() + for assignment in assignments: + if not isinstance(assignment.scope, CourseOverviewData): + continue + try: + course_key = CourseKey.from_string(assignment.scope.external_key) + except InvalidKeyError: + # A non-course scope (e.g. a library) can legitimately appear here; skip it. + continue + if course_key.org not in omit_orgs and core_toggles.AUTHZ_COURSE_AUTHORING_FLAG.is_enabled(course_key): + # Store the serialized form to match the legacy branch's string + # course_id, so the unioned set de-duplicates across both paths. + course_keys.add(str(course_key)) + return course_keys + + +def get_authz_org_keys(username: str, omit_orgs: list[str]) -> set[str]: + """ + Returns org short_names from the user's org-level (glob) authz course role + assignments where the authz course authoring flag is enabled for that org. + + An authz role can be granted at an org-wide scope (e.g. ``course-v1:Org+*``), + which surfaces as an ``OrgCourseOverviewGlobData`` scope rather than a + per-course ``CourseOverviewData`` scope. Such a grant means "all courses in + this org", so it belongs in the Meilisearch ``org IN [...]`` clause -- one + entry per org rather than fanning out to every course id (which would burn + ``MAX_ACCESS_IDS_IN_FILTER`` slots) and mirrors how legacy org staff roles + are handled. + + Filters the per-request-cached whole assignment set to the org-glob scopes, + so this and ``_get_authz_course_keys`` share a single enforcer fetch within + one request. The flag is resolved at the org tier via + ``CourseWaffleFlag.is_enabled_for_org`` (org override takes precedence, else + the global switch), since an org-wide grant is not tied to a single course. + + Fails open on a database error (logged, returns an empty set) so search + degrades to legacy access rather than 500-ing; any other exception + propagates. Orgs already covered by ``omit_orgs`` are skipped. + """ + try: + assignments = _get_cached_authz_assignments(username) + except DatabaseError as exc: + log.warning( + "Could not load authz org role assignments for user %r; " + "falling back to legacy roles for search access. Error: %s", + username, + exc, + ) + return set() + + org_keys = set() + for assignment in assignments: + if not isinstance(assignment.scope, OrgCourseOverviewGlobData): + continue + org = assignment.scope.org + if ( + org + and org not in omit_orgs + and core_toggles.AUTHZ_COURSE_AUTHORING_FLAG.is_enabled_for_org(org) + ): + org_keys.add(org) + return org_keys + + class IncrementalIndexCompleted(models.Model): # noqa: DJ008 """ Stores the contex keys of aleady indexed courses and libraries for incremental indexing. diff --git a/openedx/core/djangoapps/content/search/tests/test_models.py b/openedx/core/djangoapps/content/search/tests/test_models.py index ef42a1c04879..0498528daa58 100644 --- a/openedx/core/djangoapps/content/search/tests/test_models.py +++ b/openedx/core/djangoapps/content/search/tests/test_models.py @@ -1,26 +1,68 @@ """Content search model tests""" from __future__ import annotations +from unittest import mock + import ddt +import pytest +from django.db import OperationalError from django.test import RequestFactory from django.utils.crypto import get_random_string +from edx_django_utils.cache import RequestCache +from edx_toggles.toggles.testutils import override_waffle_flag from organizations.models import Organization from common.djangoapps.student.auth import update_org_role from common.djangoapps.student.roles import CourseInstructorRole, CourseStaffRole, OrgInstructorRole, OrgStaffRole from common.djangoapps.student.tests.factories import UserFactory +from openedx.core import toggles as core_toggles from openedx.core.djangoapps.content.course_overviews.tests.factories import CourseOverviewFactory from openedx.core.djangoapps.content_libraries import api as library_api +from openedx.core.djangoapps.waffle_utils.models import WaffleFlagOrgOverrideModel from openedx.core.djangolib.testing.utils import skip_unless_cms from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory try: # This import errors in the lms because content.search is not an installed app there. - from openedx.core.djangoapps.content.search.models import SearchAccess, get_access_ids_for_request + from openedx_authz.api.data import CourseOverviewData, OrgCourseOverviewGlobData + + from openedx.core.djangoapps.content.search.models import ( + SearchAccess, + _get_authz_course_keys, + get_access_ids_for_request, + get_authz_org_keys, + ) except RuntimeError: SearchAccess = {} + CourseOverviewData = OrgCourseOverviewGlobData = None + _get_authz_course_keys = lambda username, omit_orgs: set() get_access_ids_for_request = lambda request: [] + get_authz_org_keys = lambda username, omit_orgs: set() + + +def _fake_authz_assignment(course_key): + """ + Build a stand-in for openedx_authz's RoleAssignmentData whose scope is a + real ``CourseOverviewData`` (so it passes the ``isinstance`` scope-type + filter in ``_get_authz_course_keys``) exposing the ``external_key`` the + helper reads. + + We stub the assignment rather than provision real authz role rows because + the model helper only cares about the scope's type and external key; the + shape of the authz storage is exercised by openedx-authz's own test suite. + """ + return mock.Mock(scope=CourseOverviewData(external_key=str(course_key))) + + +def _fake_authz_org_glob_assignment(org): + """ + Build a stand-in for openedx_authz's RoleAssignmentData whose scope is a + real org-wide (glob) ``OrgCourseOverviewGlobData`` (so it passes the + ``isinstance`` scope-type filter in ``get_authz_org_keys``), exposing the + ``org`` property the helper reads (e.g. ``course-v1:Org+*`` -> ``'Org'``). + """ + return mock.Mock(scope=OrgCourseOverviewGlobData(external_key=f'course-v1:{org}+*')) class StudioSearchTestMixin: @@ -55,6 +97,13 @@ def setUp(self): """ super().setUp() + # The authz assignment fetch is memoized per request via + # @request_cached. These tests exercise the helpers with synthetic + # RequestFactory requests that never cross RequestCacheMiddleware (which + # normally clears the cache at request boundaries), so clear it here to + # keep each test's mocked fetch from leaking into the next. + RequestCache.clear_all_namespaces() + self.course_user_keys = [] self.staff_user_keys = [] @@ -245,3 +294,266 @@ def test_no_access_ids_for_request(self): request.user = self.student access_ids = get_access_ids_for_request(request) assert not access_ids + + +@ddt.ddt +@skip_unless_cms +class StudioSearchAuthzAccessTest(StudioSearchTestMixin, SharedModuleStoreTestCase): + """ + Tests that ``get_access_ids_for_request`` includes courses granted through + openedx-authz role assignments, not just legacy CourseStaffRole / + CourseInstructorRole (openedx/openedx-authz#417). + """ + + AUTHZ_PATH = 'openedx.core.djangoapps.content.search.models.get_user_role_assignments' + + def _create_course(self, course_location): + """Create a SearchAccess row per course so access_ids can resolve.""" + course = super()._create_course(course_location) + SearchAccess.objects.create(context_key=course.id) + return course + + def _create_library(self, org, num): + """Create a SearchAccess row per library so access_ids can resolve.""" + library = super()._create_library(org, num) + SearchAccess.objects.create(context_key=library.key) + return library + + def _authz_only_user(self): + """A user with no legacy course role — access can only come from authz.""" + return UserFactory.create( + username='authz_editor', + email='authz_editor@example.com', + is_staff=False, + password='authz_editor_pass', + ) + + def _course_access_ids(self, course_keys): + """Resolve the SearchAccess ids for the given course keys.""" + return set( + SearchAccess.objects.filter(context_key__in=course_keys).values_list('id', flat=True) + ) + + @override_waffle_flag(core_toggles.AUTHZ_COURSE_AUTHORING_FLAG, active=True) + def test_authz_only_user_sees_authz_courses(self): + """ + A user with an authz role assignment but no legacy role gets the + matching course access_ids when the flag is enabled. + """ + user = self._authz_only_user() + granted = self.course_user_keys[:2] # first two are CourseKeys (libraries come later) + request = RequestFactory().get('/course') + request.user = user + + with mock.patch( + self.AUTHZ_PATH, + return_value=[_fake_authz_assignment(key) for key in granted], + ): + access_ids = get_access_ids_for_request(request) + + assert set(access_ids) == self._course_access_ids(granted) + + @override_waffle_flag(core_toggles.AUTHZ_COURSE_AUTHORING_FLAG, active=True) + def test_authz_courses_respect_omit_orgs(self): + """Authz-granted courses in an omitted org are excluded.""" + user = self._authz_only_user() + granted = self.course_user_keys[:2] + request = RequestFactory().get('/course') + request.user = user + + with mock.patch( + self.AUTHZ_PATH, + return_value=[_fake_authz_assignment(key) for key in granted], + ): + access_ids = get_access_ids_for_request(request, omit_orgs=['Org']) + + assert not access_ids + + @override_waffle_flag(core_toggles.AUTHZ_COURSE_AUTHORING_FLAG, active=False) + def test_authz_courses_excluded_when_flag_off(self): + """With the flag disabled, authz assignments contribute no access_ids.""" + user = self._authz_only_user() + granted = self.course_user_keys[:2] + request = RequestFactory().get('/course') + request.user = user + + with mock.patch( + self.AUTHZ_PATH, + return_value=[_fake_authz_assignment(key) for key in granted], + ): + access_ids = get_access_ids_for_request(request) + + assert not access_ids + + @override_waffle_flag(core_toggles.AUTHZ_COURSE_AUTHORING_FLAG, active=True) + def test_authz_db_failure_is_swallowed(self): + """ + A database failure in the authz query must not break search; the + request falls back to legacy roles only (here: none, so no access_ids). + """ + user = self._authz_only_user() + request = RequestFactory().get('/course') + request.user = user + + with mock.patch(self.AUTHZ_PATH, side_effect=OperationalError('authz db down')): + access_ids = get_access_ids_for_request(request) + + assert not access_ids + + @override_waffle_flag(core_toggles.AUTHZ_COURSE_AUTHORING_FLAG, active=True) + def test_authz_unexpected_error_propagates(self): + """ + Only known operational (DatabaseError) failures are swallowed. An + unexpected error is NOT masked — it propagates so real bugs surface. + """ + user = self._authz_only_user() + request = RequestFactory().get('/course') + request.user = user + + with mock.patch(self.AUTHZ_PATH, side_effect=RuntimeError('unexpected')): + with pytest.raises(RuntimeError): + get_access_ids_for_request(request) + + @override_waffle_flag(core_toggles.AUTHZ_COURSE_AUTHORING_FLAG, active=True) + def test_authz_ignores_unparseable_scope(self): + """A scope external_key that is not a course key is skipped, not fatal.""" + user = self._authz_only_user() + granted = self.course_user_keys[:1] + request = RequestFactory().get('/course') + request.user = user + assignments = [ + _fake_authz_assignment(granted[0]), + _fake_authz_assignment('not-a-course-key'), + ] + + with mock.patch(self.AUTHZ_PATH, return_value=assignments): + access_ids = get_access_ids_for_request(request) + + assert set(access_ids) == self._course_access_ids(granted) + + @override_waffle_flag(core_toggles.AUTHZ_COURSE_AUTHORING_FLAG, active=True) + def test_authz_union_with_legacy_roles_no_duplicates(self): + """ + When a course is granted through BOTH a legacy role and authz, its + access_id appears exactly once. + """ + request = RequestFactory().get('/course') + request.user = self.course_staff # already has legacy CourseStaffRole on course_user_keys + legacy_courses = self.course_user_keys[:2] + + with mock.patch( + self.AUTHZ_PATH, + return_value=[_fake_authz_assignment(key) for key in legacy_courses], + ): + access_ids = get_access_ids_for_request(request) + + assert len(access_ids) == len(set(access_ids)) + assert self._course_access_ids(legacy_courses).issubset(set(access_ids)) + + @override_waffle_flag(core_toggles.AUTHZ_COURSE_AUTHORING_FLAG, active=True) + def test_authz_course_keys_are_strings_matching_legacy(self): + """ + _get_authz_course_keys returns serialized (str) keys, matching the + legacy get_course_roles branch (whose course_id is a str). This keeps + the unioned course_keys set type-homogeneous: a course held via both a + legacy role and an authz role collapses to ONE set member rather than + two (a str and a CourseKey object hash/compare unequal). The downstream + SQL ``IN`` coerces both forms, so the query works either way -- this + guards the set-union de-dup invariant itself, at the source. + """ + user = self._authz_only_user() + granted = self.course_user_keys[:1] + + with mock.patch( + self.AUTHZ_PATH, + return_value=[_fake_authz_assignment(key) for key in granted], + ): + keys = _get_authz_course_keys(user.username, omit_orgs=[]) + + assert keys == {str(key) for key in granted} + assert all(isinstance(key, str) for key in keys) + + +@ddt.ddt +@skip_unless_cms +class StudioSearchAuthzOrgAccessTest(StudioSearchTestMixin, SharedModuleStoreTestCase): + """ + Tests that ``get_authz_org_keys`` surfaces org-wide (glob) authz course + role assignments -- e.g. a ``course_editor`` granted at ``course-v1:Org+*`` + -- so that org-level authz-only users are covered by the ``org IN [...]`` + search filter clause (openedx/openedx-authz#417). + + An org glob returns an ``OrgCourseOverviewGlobData`` scope, which the + per-course path (``get_access_ids_for_request`` -> ``CourseOverviewData``) + deliberately skips; it must be resolved to an org short_name here instead. + """ + + AUTHZ_PATH = 'openedx.core.djangoapps.content.search.models.get_user_role_assignments' + + @override_waffle_flag(core_toggles.AUTHZ_COURSE_AUTHORING_FLAG, active=True) + def test_org_glob_grant_returns_org(self): + """An org-wide authz grant yields that org's short_name when the flag is on.""" + with mock.patch( + self.AUTHZ_PATH, + return_value=[_fake_authz_org_glob_assignment('org1')], + ): + orgs = get_authz_org_keys('authz_org_editor', omit_orgs=[]) + + assert orgs == {'org1'} + + @override_waffle_flag(core_toggles.AUTHZ_COURSE_AUTHORING_FLAG, active=True) + def test_org_glob_respects_omit_orgs(self): + """An org already covered by the legacy org clause is not duplicated.""" + with mock.patch( + self.AUTHZ_PATH, + return_value=[_fake_authz_org_glob_assignment('org1')], + ): + orgs = get_authz_org_keys('authz_org_editor', omit_orgs=['org1']) + + assert orgs == set() + + @override_waffle_flag(core_toggles.AUTHZ_COURSE_AUTHORING_FLAG, active=False) + def test_org_glob_excluded_when_flag_off(self): + """With the flag globally off (and no org override), org globs contribute nothing.""" + with mock.patch( + self.AUTHZ_PATH, + return_value=[_fake_authz_org_glob_assignment('org1')], + ): + orgs = get_authz_org_keys('authz_org_editor', omit_orgs=[]) + + assert orgs == set() + + def test_org_glob_enabled_by_org_override_when_flag_off_globally(self): + """ + A per-org waffle override forces the org on even when the global flag is + off -- the same resolution CourseWaffleFlag applies at the org tier. + """ + WaffleFlagOrgOverrideModel.objects.create( + waffle_flag=core_toggles.AUTHZ_COURSE_AUTHORING_FLAG.name, + org='org1', + override_choice=WaffleFlagOrgOverrideModel.ALL_CHOICES.on, + enabled=True, + ) + with override_waffle_flag(core_toggles.AUTHZ_COURSE_AUTHORING_FLAG, active=False): + with mock.patch( + self.AUTHZ_PATH, + return_value=[_fake_authz_org_glob_assignment('org1')], + ): + orgs = get_authz_org_keys('authz_org_editor', omit_orgs=[]) + + assert orgs == {'org1'} + + @override_waffle_flag(core_toggles.AUTHZ_COURSE_AUTHORING_FLAG, active=True) + def test_org_glob_db_failure_is_swallowed(self): + """A DatabaseError in the authz org query fails open (empty set, no raise).""" + with mock.patch(self.AUTHZ_PATH, side_effect=OperationalError('authz db down')): + orgs = get_authz_org_keys('authz_org_editor', omit_orgs=[]) + + assert orgs == set() + + @override_waffle_flag(core_toggles.AUTHZ_COURSE_AUTHORING_FLAG, active=True) + def test_org_glob_unexpected_error_propagates(self): + """An unexpected error is not masked -- it propagates so real bugs surface.""" + with mock.patch(self.AUTHZ_PATH, side_effect=RuntimeError('unexpected')): + with pytest.raises(RuntimeError): + get_authz_org_keys('authz_org_editor', omit_orgs=[]) diff --git a/openedx/core/djangoapps/content/search/tests/test_views.py b/openedx/core/djangoapps/content/search/tests/test_views.py index 3d056b2b4327..3a726363728d 100644 --- a/openedx/core/djangoapps/content/search/tests/test_views.py +++ b/openedx/core/djangoapps/content/search/tests/test_views.py @@ -205,6 +205,35 @@ def test_studio_search_org_access(self, username, mock_search_client): expires_at=ANY, ) + @mock_meilisearch(enabled=True) + @patch('openedx.core.djangoapps.content.search.api.get_authz_org_keys') + @patch('openedx.core.djangoapps.content.search.api.MeilisearchClient') + def test_studio_search_authz_org_glob_access(self, mock_search_client, mock_authz_orgs): + """ + A user with only an org-wide (glob) authz course grant -- and no legacy + role -- is covered by the org clause (openedx/openedx-authz#417). The + student has no legacy access, so 'org1' here comes purely from the authz + org union wired into ``_get_user_orgs``. + """ + mock_authz_orgs.return_value = {'org1'} + + self.client.login(username='student', password='student_pass') + mock_generate_tenant_token = self._mock_generate_tenant_token(mock_search_client) + result = self.client.get(STUDIO_SEARCH_ENDPOINT_URL) + assert result.status_code == 200 + # The authz org union is fed into _get_user_orgs, which is passed as omit_orgs + # to the access_ids query, so org1's courses are covered by the org clause. + mock_authz_orgs.assert_called_once() + mock_generate_tenant_token.assert_called_once_with( + api_key_uid=MOCK_API_KEY_UID, + search_rules={ + "studio_content": { + "filter": "org IN ['org1'] OR access_id IN []", + } + }, + expires_at=ANY, + ) + @mock_meilisearch(enabled=True) @patch('openedx.core.djangoapps.content.search.api.MeilisearchClient') def test_studio_search_omit_orgs(self, mock_search_client): diff --git a/openedx/core/djangoapps/waffle_utils/__init__.py b/openedx/core/djangoapps/waffle_utils/__init__.py index 95fa360d5e84..52df524a1234 100644 --- a/openedx/core/djangoapps/waffle_utils/__init__.py +++ b/openedx/core/djangoapps/waffle_utils/__init__.py @@ -62,7 +62,7 @@ def _get_course_override_value(self, course_key): course_key (CourseKey): The course to check for override before checking waffle. """ # Import is placed here to avoid model import at project startup. - from .models import WaffleFlagCourseOverrideModel, WaffleFlagOrgOverrideModel + from .models import WaffleFlagCourseOverrideModel course_cache_key = f"{self.name}.cwaffle.{str(course_key)}" course_override = self.cached_flags().get(course_cache_key) @@ -80,20 +80,34 @@ def _get_course_override_value(self, course_key): # Since no course-specific override was found, fall back to checking at the org-level. if course_key: - org = course_key.org - org_cache_key = f"{self.name}.owaffle.{org}" - org_override = self.cached_flags().get(org_cache_key) + return self._get_org_override_value(course_key.org) - if org_override is None: - org_override = WaffleFlagOrgOverrideModel.override_value( - self.name, org - ) - self.cached_flags()[org_cache_key] = org_override + return None + + def _get_org_override_value(self, org): + """ + Check whether the flag was overridden for an entire org. + + Returns True/False if the flag was forced on or off for the provided org. + Returns None if the flag was not overridden at the org level. + + Arguments: + org (str): The org short_name to check for an override. + """ + # Import is placed here to avoid model import at project startup. + from .models import WaffleFlagOrgOverrideModel + + org_cache_key = f"{self.name}.owaffle.{org}" + org_override = self.cached_flags().get(org_cache_key) + + if org_override is None: + org_override = WaffleFlagOrgOverrideModel.override_value(self.name, org) + self.cached_flags()[org_cache_key] = org_override - if org_override == WaffleFlagOrgOverrideModel.ALL_CHOICES.on: - return True - if org_override == WaffleFlagOrgOverrideModel.ALL_CHOICES.off: - return False + if org_override == WaffleFlagOrgOverrideModel.ALL_CHOICES.on: + return True + if org_override == WaffleFlagOrgOverrideModel.ALL_CHOICES.off: + return False return None @@ -120,3 +134,24 @@ def is_enabled(self, course_key=None): # pylint: disable=arguments-differ # act like a normal waffle flag. We currently don't support library-specific overrides. assert isinstance(course_key, LearningContextKey), "expected a course key or other learning context key" return super().is_enabled() + + def is_enabled_for_org(self, org): + """ + Returns whether the flag is enabled for an entire org. + + Resolves the flag at the org tier: an org override (force-on / force-off) + takes precedence, otherwise falls back to the global waffle switch. Unlike + :meth:`is_enabled`, this takes an org short_name rather than a course key, + for grants that are not tied to a single course (e.g. an org-wide role). + + A result here does not guarantee the flag's state for any specific course + in the org -- that course may have its own override. Use :meth:`is_enabled` + to check a specific course. + + Arguments: + org (str): The org short_name to check. + """ + org_override = self._get_org_override_value(org) + if org_override is not None: + return org_override + return super().is_enabled() diff --git a/openedx/core/djangoapps/waffle_utils/tests/test_init.py b/openedx/core/djangoapps/waffle_utils/tests/test_init.py index 57f31767373a..00ad1d4c7782 100644 --- a/openedx/core/djangoapps/waffle_utils/tests/test_init.py +++ b/openedx/core/djangoapps/waffle_utils/tests/test_init.py @@ -225,3 +225,39 @@ def test_without_request_and_everyone_active_waffle(self): test_course_flag = CourseWaffleFlag(self.NAMESPACED_FLAG_NAME, __name__) with override_waffle_flag(self.TEST_COURSE_FLAG, active=True): assert test_course_flag.is_enabled(self.TEST_COURSE_KEY) is True + + @ddt.data( + (False, WaffleFlagOrgOverrideModel.ALL_CHOICES.unset, False), + (True, WaffleFlagOrgOverrideModel.ALL_CHOICES.unset, True), + (False, WaffleFlagOrgOverrideModel.ALL_CHOICES.on, True), + (True, WaffleFlagOrgOverrideModel.ALL_CHOICES.on, True), + (False, WaffleFlagOrgOverrideModel.ALL_CHOICES.off, False), + (True, WaffleFlagOrgOverrideModel.ALL_CHOICES.off, False), + ) + @ddt.unpack + def test_is_enabled_for_org(self, waffle_enabled, org_override_choice, is_enabled): + """ + Tests is_enabled_for_org: an org override (on/off) takes precedence, otherwise + the base waffle switch decides. Takes an org short_name, not a course key. + + on = active (enabled) + off = inactive (disabled) + unset = mirror the base waffle flag's activity + """ + WaffleFlagOrgOverrideModel.objects.create( + waffle_flag=self.NAMESPACED_FLAG_NAME, + org=self.TEST_ORG, + override_choice=org_override_choice, + note='', + enabled=True + ) + with override_waffle_flag(self.TEST_COURSE_FLAG, active=waffle_enabled): + assert self.TEST_COURSE_FLAG.is_enabled_for_org(self.TEST_ORG) == is_enabled + + def test_is_enabled_for_org_no_override_uses_global_switch(self): + """ + With no org override at all, is_enabled_for_org falls back to the global switch. + """ + with override_waffle_flag(self.TEST_COURSE_FLAG, active=True): + assert self.TEST_COURSE_FLAG.is_enabled_for_org("SomeUnoverriddenOrg") is True + assert self.TEST_COURSE_FLAG.is_enabled_for_org("SomeUnoverriddenOrg") is False