diff --git a/openedx/core/djangoapps/content/search/api.py b/openedx/core/djangoapps/content/search/api.py index 6d6ce6148cd2..6281fba9ca6c 100644 --- a/openedx/core/djangoapps/content/search/api.py +++ b/openedx/core/djangoapps/content/search/api.py @@ -38,7 +38,12 @@ 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, + authz_has_platform_access, + 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 +1074,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, omit_orgs=list(orgs))) + return list(orgs) def _get_meili_access_filter(request: Request) -> dict: @@ -1084,6 +1096,11 @@ def _get_meili_access_filter(request: Request) -> dict: if GlobalStaff().has_user(request.user): return {} + # An authz platform-wide grant (course-v1:*) is the authz analogue of global + # staff -- it means "all courses across the platform", so no filters required. + if authz_has_platform_access(request.user): + return {} + # Everyone else is limited to their org staff roles... user_orgs = _get_user_orgs(request)[:MAX_ORGS_IN_FILTER] diff --git a/openedx/core/djangoapps/content/search/models.py b/openedx/core/djangoapps/content/search/models.py index d726f1ead057..ec7ba17b098c 100644 --- a/openedx/core/djangoapps/content/search/models.py +++ b/openedx/core/djangoapps/content/search/models.py @@ -2,14 +2,34 @@ from __future__ import annotations -from django.db import models +import logging + +from django.contrib.auth import get_user_model +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, + PlatformCourseOverviewGlobData, +) 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 common.djangoapps.student.roles import ( + CourseInstructorRole, + CourseStaffRole, + authz_get_all_course_assignments_for_user, +) +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__) + +User = get_user_model() class SearchAccess(models.Model): # noqa: DJ008 @@ -46,14 +66,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, 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 +95,165 @@ def get_access_ids_for_request(request: Request, omit_orgs: list[str] = None) -> ) +@request_cached(arg_map_function=lambda user: user.username) +def _get_cached_authz_course_assignments(user: User): + """ + Returns the user's course-relevant authz role assignments, cached per request. + + Delegates to ``authz_get_all_course_assignments_for_user``, the shared + course-role helper in ``student.roles`` (which also backs the legacy-compat + ``RoleCache`` path), so the set of scope types search cares about -- + ``CourseOverviewData`` (per-course), ``OrgCourseOverviewGlobData`` (org-wide), + and ``PlatformCourseOverviewGlobData`` (platform-wide) -- is defined in one + place rather than re-derived here. That single fetch is shared by all three + search helpers below (per-course keys, org keys, platform access), so each + filters the same materialized set by scope type without a second enforcer + round-trip within one request. + + Keyed on ``user.username`` via ``arg_map_function`` (a ``User`` object's + default repr would otherwise be an unstable cache key), so the helpers share + one 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 authz_get_all_course_assignments_for_user(user) + + +def _get_authz_course_keys(user: User, 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 shared per-request assignment set (see + ``_get_cached_authz_course_assignments``) 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_course_assignments(user) + 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", + user.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(user: User, 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 shared per-request assignment set (see + ``_get_cached_authz_course_assignments``) 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_course_assignments(user) + 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", + user.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 + + +def authz_has_platform_access(user: User) -> bool: + """ + Returns whether the user holds a platform-wide (``course-v1:*``) authz course + role grant that is active under the authz course authoring flag. + + A platform-level grant surfaces as a ``PlatformCourseOverviewGlobData`` scope + and means "all courses across the whole platform" -- the authz analogue of + legacy global staff. Such a user should see everything in search, so the + caller short-circuits the per-course/per-org filter entirely rather than + expanding the grant to one org entry per registered org (which + ``student.roles`` does for the legacy-compat path, but which would blow + ``MAX_ORGS_IN_FILTER`` here). The flag is resolved against the global switch + via ``is_enabled()`` because a platform-wide grant is tied to neither a course + nor an org against which a per-scope override could be evaluated. + + Filters the shared per-request assignment set (see + ``_get_cached_authz_course_assignments``). Fails open on a database error + (logged, returns ``False``) so search degrades to the per-scope + legacy/authz filter rather than 500-ing; any other exception propagates. + """ + try: + assignments = _get_cached_authz_course_assignments(user) + except DatabaseError as exc: + log.warning( + "Could not load authz role assignments for user %r; " + "falling back to per-scope search access. Error: %s", + user.username, + exc, + ) + return False + + if not core_toggles.AUTHZ_COURSE_AUTHORING_FLAG.is_enabled(): + return False + + return any( + isinstance(assignment.scope, PlatformCourseOverviewGlobData) + for assignment in assignments + ) + + 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..1950822b0361 100644 --- a/openedx/core/djangoapps/content/search/tests/test_models.py +++ b/openedx/core/djangoapps/content/search/tests/test_models.py @@ -1,26 +1,95 @@ """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, + authz_has_platform_access, + 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() + authz_has_platform_access = lambda username: False + +try: + # Real-policy authz test helpers. Guarded for the same reason as above (the + # openedx_authz engine is only wired up where content.search is installed). + from openedx_authz.api.users import assign_role_to_user_in_scope + from openedx_authz.constants.roles import COURSE_EDITOR + from openedx_authz.engine.enforcer import AuthzEnforcer + + from openedx.core.djangoapps.authz.tests.mixins import CourseAuthoringAuthzTestMixin + _HAS_AUTHZ_TEST_SUPPORT = True +except (RuntimeError, ImportError): + # Placeholder base when authz test support is unavailable (e.g. under lms, + # where content.search is not installed). Must be an empty *mixin* class, not + # ``object``: the real class lists it BEFORE SharedModuleStoreTestCase, and a + # bare ``object`` in that position is an inconsistent MRO (object must resolve + # last). The class is never instantiated -- the pytest.mark.skipif below skips + # the whole class on this path -- but it must still linearize cleanly so the + # module imports and pylint's MRO check passes. + class CourseAuthoringAuthzTestMixin: # pylint: disable=too-few-public-methods + """No-op placeholder used when openedx_authz test support is unavailable.""" + + assign_role_to_user_in_scope = None + COURSE_EDITOR = None + AuthzEnforcer = None + _HAS_AUTHZ_TEST_SUPPORT = False + + +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 +124,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 +321,380 @@ 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.authz_get_all_course_assignments_for_user' + + 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, 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.authz_get_all_course_assignments_for_user' + + def setUp(self): + super().setUp() + self.authz_org_user = UserFactory.create( + username='authz_org_editor', + email='authz_org_editor@example.com', + is_staff=False, + password='authz_org_editor_pass', + ) + + @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(self.authz_org_user, 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(self.authz_org_user, 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(self.authz_org_user, 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(self.authz_org_user, 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(self.authz_org_user, 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(self.authz_org_user, omit_orgs=[]) + + +@skip_unless_cms +@pytest.mark.skipif(not _HAS_AUTHZ_TEST_SUPPORT, reason="openedx_authz test support unavailable") +class StudioSearchAuthzRealPolicyTest(CourseAuthoringAuthzTestMixin, SharedModuleStoreTestCase): + """ + End-to-end authz coverage that seeds REAL role assignments into the enforcer + (via ``assign_role_to_user_in_scope``) instead of mocking + ``get_user_role_assignments``. + + The mocked tests above prove the helper logic in isolation, but they stub the + assignment set -- so they never exercise the real + ``RoleAssignmentData.scope.external_key`` -> ``CourseKey`` contract, nor that + a real per-course / org-glob / platform-glob grant lands in the right scope + class. These tests close that gap by driving the actual openedx-authz storage, + which is the concrete contract search depends on. + + Deliberately self-contained rather than reusing ``StudioSearchTestMixin``: + ``CourseAuthoringAuthzTestMixin`` patches ``AUTHZ_COURSE_AUTHORING_FLAG`` on + for the whole class, which routes the mixin's legacy org-role seeding through + the authz compatibility layer -- an unrelated path this test does not need. + So we build only the minimal course fixture the authz helpers read. The flag + being on for the class is exactly what these happy-path grants want, so no + per-method ``@override_waffle_flag`` is required. + """ + + def setUp(self): + super().setUp() + # Synthetic RequestFactory requests never cross RequestCacheMiddleware, + # so clear the per-request authz cache between tests explicitly. + RequestCache.clear_all_namespaces() + + self.course_keys = [] + for num in range(2): + course_location = self.store.make_course_key('org1', f'AuthzCourse{num}', 'Run') + CourseFactory.create( + org=course_location.org, + number=course_location.course, + run=course_location.run, + ) + course = CourseOverviewFactory.create(id=course_location, org=course_location.org) + SearchAccess.objects.create(context_key=course.id) + self.course_keys.append(course_location) + + 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) + ) + + def _assign_scope(self, user, scope_external_key, role=None): + """Seed a real authz grant for ``user`` at ``scope_external_key`` and reload policy.""" + assign_role_to_user_in_scope( + user.username, + (role or COURSE_EDITOR).external_key, + scope_external_key, + ) + AuthzEnforcer.get_enforcer().load_policy() + + def test_real_per_course_grant_surfaces_course(self): + """ + A real per-course authz grant (``course-v1:Org+C+R``) resolves through + the actual ``scope.external_key`` -> ``CourseKey`` path and yields that + course's access_id -- no legacy role involved. + """ + for course_key in self.course_keys: + self._assign_scope(self.authorized_user, str(course_key)) + + request = RequestFactory().get('/course') + request.user = self.authorized_user + access_ids = get_access_ids_for_request(request) + + assert set(access_ids) == self._course_access_ids(self.course_keys) + + def test_real_org_glob_grant_surfaces_org(self): + """A real org-wide grant (``course-v1:Org+*``) resolves to that org short_name.""" + # The org-glob path resolves the flag via ``is_enabled_for_org`` (org + # override, else global switch). The mixin patches ``is_enabled`` on but + # not ``is_enabled_for_org``, so seed a real per-org override to turn the + # org on -- exercising the actual org-tier resolution end to end. + WaffleFlagOrgOverrideModel.objects.create( + waffle_flag=core_toggles.AUTHZ_COURSE_AUTHORING_FLAG.name, + org='org1', + override_choice=WaffleFlagOrgOverrideModel.ALL_CHOICES.on, + enabled=True, + ) + self._assign_scope(self.authorized_user, 'course-v1:org1+*') + + orgs = get_authz_org_keys(self.authorized_user, omit_orgs=[]) + + assert orgs == {'org1'} + + def test_real_platform_glob_grant_grants_all_access(self): + """ + A real platform-wide grant (``course-v1:*``) is recognized as "see + everything" -- the authz analogue of global staff (openedx-authz#417 + follow-up: a ``course-v1:*`` user should see all orgs and all courses). + """ + self._assign_scope(self.authorized_user, 'course-v1:*') + + assert authz_has_platform_access(self.authorized_user) is True + + def test_no_grant_has_no_platform_access(self): + """A user with no platform-wide grant is not treated as see-everything.""" + assert authz_has_platform_access(self.unauthorized_user) is False diff --git a/openedx/core/djangoapps/content/search/tests/test_views.py b/openedx/core/djangoapps/content/search/tests/test_views.py index 3d056b2b4327..9c92d1fc2fa3 100644 --- a/openedx/core/djangoapps/content/search/tests/test_views.py +++ b/openedx/core/djangoapps/content/search/tests/test_views.py @@ -205,6 +205,61 @@ 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.authz_has_platform_access') + @patch('openedx.core.djangoapps.content.search.api.MeilisearchClient') + def test_studio_search_authz_platform_glob_access(self, mock_search_client, mock_platform_access): + """ + A user with a platform-wide (``course-v1:*``) authz course grant is the + authz analogue of global staff and can search any document, so the + access filter is empty -- no ``org``/``access_id`` restriction clause + (openedx/openedx-authz#417). The student has no legacy access, so the + empty filter here comes purely from the platform-access short-circuit. + """ + mock_platform_access.return_value = True + + 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 + mock_platform_access.assert_called_once() + mock_generate_tenant_token.assert_called_once_with( + api_key_uid=MOCK_API_KEY_UID, + search_rules={ + "studio_content": {} + }, + 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