diff --git a/openedx/core/djangoapps/content_tagging/api.py b/openedx/core/djangoapps/content_tagging/api.py index 18cf2650e0..a8630e29e5 100644 --- a/openedx/core/djangoapps/content_tagging/api.py +++ b/openedx/core/djangoapps/content_tagging/api.py @@ -20,11 +20,12 @@ def create_taxonomy( enabled=True, allow_multiple=False, allow_free_text=False, + orgs: list[Organization] | None = None, ) -> Taxonomy: """ Creates, saves, and returns a new Taxonomy with the given attributes. """ - return oel_tagging.create_taxonomy( + taxonomy = oel_tagging.create_taxonomy( name=name, description=description, enabled=enabled, @@ -32,6 +33,11 @@ def create_taxonomy( allow_free_text=allow_free_text, ) + if orgs is not None: + set_taxonomy_orgs(taxonomy=taxonomy, all_orgs=False, orgs=orgs) + + return taxonomy + def set_taxonomy_orgs( taxonomy: Taxonomy, diff --git a/openedx/core/djangoapps/content_tagging/rest_api/v1/filters.py b/openedx/core/djangoapps/content_tagging/rest_api/v1/filters.py index 9ad192f545..723a90d877 100644 --- a/openedx/core/djangoapps/content_tagging/rest_api/v1/filters.py +++ b/openedx/core/djangoapps/content_tagging/rest_api/v1/filters.py @@ -2,20 +2,92 @@ API Filters for content tagging org """ +from django.db.models import Exists, OuterRef, Q from rest_framework.filters import BaseFilterBackend import openedx_tagging.core.tagging.rules as oel_tagging +from organizations.models import Organization + +from ...rules import get_admin_orgs, get_user_orgs +from ...models import TaxonomyOrg class UserOrgFilterBackend(BaseFilterBackend): """ - Taxonomy admin can see all taxonomies - Everyone else can see only enabled taxonomies + Filter taxonomies based on user's orgs roles + Taxonomy admin can see all taxonomies + Org staff can see all taxonomies from their orgs + Content creators and instructors can see enabled taxonomies avaliable to their orgs """ def filter_queryset(self, request, queryset, _): if oel_tagging.is_taxonomy_admin(request.user): return queryset - return queryset.filter(enabled=True) + orgs = list(Organization.objects.all()) + user_admin_orgs = get_admin_orgs(request.user, orgs) + user_orgs = get_user_orgs(request.user, orgs) # Orgs that the user is a content creator or instructor + + if len(user_orgs) == 0 and len(user_admin_orgs) == 0: + return queryset.none() + + return queryset.filter( + # Get enabled taxonomies available to all orgs, or from orgs that the user is + # a content creator or instructor + Q( + Exists( + TaxonomyOrg.objects + .filter( + taxonomy=OuterRef("pk"), + rel_type=TaxonomyOrg.RelType.OWNER, + ) + .filter( + Q(org=None) | + Q(org__in=user_orgs) + ) + ), + enabled=True, + ) | + # Get all taxonomies from orgs that the user is OrgStaff + Q( + Exists( + TaxonomyOrg.objects + .filter(taxonomy=OuterRef("pk"), rel_type=TaxonomyOrg.RelType.OWNER) + .filter(org__in=user_admin_orgs) + ) + ) + ) + + +class ObjectTagTaxonomyOrgFilterBackend(BaseFilterBackend): + """ + Filter for ObjectTagViewSet to only show taxonomies that the user can view. + """ + + def filter_queryset(self, request, queryset, _): + if oel_tagging.is_taxonomy_admin(request.user): + return queryset + + orgs = list(Organization.objects.all()) + user_admin_orgs = get_admin_orgs(request.user, orgs) + user_orgs = get_user_orgs(request.user, orgs) + user_or_admin_orgs = list(set(user_orgs) | set(user_admin_orgs)) + + return queryset.filter(taxonomy__enabled=True).filter( + # Get ObjectTags from taxonomies available to all orgs, or from orgs that the user is + # a OrgStaff, content creator or instructor + Q( + Exists( + TaxonomyOrg.objects + .filter( + taxonomy=OuterRef("taxonomy_id"), + rel_type=TaxonomyOrg.RelType.OWNER, + ) + .filter( + Q(org=None) | + Q(org__in=user_or_admin_orgs) + ) + ) + ) + ) diff --git a/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py b/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py index 1389c2762e..d37b5df26c 100644 --- a/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py +++ b/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py @@ -2,11 +2,13 @@ Tests tagging rest api views """ +from __future__ import annotations + from urllib.parse import parse_qs, urlparse +import abc import ddt from django.contrib.auth import get_user_model -from django.test import override_settings from opaque_keys.edx.locator import BlockUsageLocator, CourseLocator from openedx_tagging.core.tagging.models import Tag, Taxonomy from openedx_tagging.core.tagging.models.system_defined import SystemDefinedTaxonomy @@ -16,9 +18,23 @@ from rest_framework import status from rest_framework.test import APITestCase from common.djangoapps.student.auth import add_users, update_org_role -from common.djangoapps.student.roles import CourseStaffRole, OrgContentCreatorRole +from common.djangoapps.student.roles import ( + CourseInstructorRole, + CourseStaffRole, + OrgContentCreatorRole, + OrgInstructorRole, + OrgLibraryUserRole, + OrgStaffRole, +) +from openedx.core.djangoapps.content_libraries.api import ( + AccessLevel, + create_library, + COMPLEX, + set_library_user_permissions, +) from openedx.core.djangoapps.content_tagging.models import TaxonomyOrg from openedx.core.djangolib.testing.utils import skip_unless_cms +from openedx.core.lib import blockstore_api User = get_user_model() @@ -57,29 +73,102 @@ class TestTaxonomyObjectsMixin: """ Sets up data for testing Content Taxonomies. """ + def _setUp_orgs(self): + """ + Create orgs for testing + """ + self.orgA = Organization.objects.create(name="Organization A", short_name="orgA") + self.orgB = Organization.objects.create(name="Organization B", short_name="orgB") + self.orgX = Organization.objects.create(name="Organization X", short_name="orgX") - def setUp(self): - super().setUp() + def _setUp_courses(self): + """ + Create courses for testing + """ + self.courseA = CourseLocator("orgA", "101", "test") + self.courseB = CourseLocator("orgB", "101", "test") + + def _setUp_library(self): + """ + Create library for testing + """ + self.collection = blockstore_api.create_collection("Test library collection") + self.content_libraryA = create_library( + collection_uuid=self.collection.uuid, + org=self.orgA, + slug="lib_a", + library_type=COMPLEX, + title="Library Org A", + description="This is a library from Org A", + allow_public_learning=False, + allow_public_read=False, + library_license="", + ) + + def _setUp_users(self): + """ + Create users for testing + """ self.user = User.objects.create( username="user", email="user@example.com", ) - self.userS = User.objects.create( + self.staff = User.objects.create( username="staff", email="staff@example.com", is_staff=True, ) - self.orgA = Organization.objects.create(name="Organization A", short_name="orgA") - self.orgB = Organization.objects.create(name="Organization B", short_name="orgB") - self.orgX = Organization.objects.create(name="Organization X", short_name="orgX") - - self.userA = User.objects.create( - username="userA", + self.staffA = User.objects.create( + username="staffA", email="userA@example.com", ) - update_org_role(self.userS, OrgContentCreatorRole, self.userA, [self.orgA.short_name]) + update_org_role(self.staff, OrgStaffRole, self.staffA, [self.orgA.short_name]) + self.content_creatorA = User.objects.create( + username="content_creatorA", + email="content_creatorA@example.com", + ) + update_org_role(self.staff, OrgContentCreatorRole, self.content_creatorA, [self.orgA.short_name]) + + self.instructorA = User.objects.create( + username="instructorA", + email="instructorA@example.com", + ) + update_org_role(self.staff, OrgInstructorRole, self.instructorA, [self.orgA.short_name]) + + self.library_staffA = User.objects.create( + username="library_staffA", + email="library_staffA@example.com", + ) + update_org_role(self.staff, OrgLibraryUserRole, self.library_staffA, [self.orgA.short_name]) + + self.course_instructorA = User.objects.create( + username="course_instructorA", + email="course_instructorA@example.com", + ) + add_users(self.staff, CourseInstructorRole(self.courseA), self.course_instructorA) + + self.course_staffA = User.objects.create( + username="course_staffA", + email="course_staffA@example.com", + ) + add_users(self.staff, CourseStaffRole(self.courseA), self.course_staffA) + + self.library_userA = User.objects.create( + username="library_userA", + email="library_userA@example.com", + ) + set_library_user_permissions( + self.content_libraryA.key, + self.library_userA, + AccessLevel.READ_LEVEL + ) + + def _setUp_taxonomies(self): + """ + Create taxonomies for testing + """ # Orphaned taxonomy self.ot1 = Taxonomy.objects.create(name="ot1", enabled=True) self.ot2 = Taxonomy.objects.create(name="ot2", enabled=False) @@ -117,9 +206,7 @@ class TestTaxonomyObjectsMixin: self.tA1 = Taxonomy.objects.create(name="tA1", enabled=True) TaxonomyOrg.objects.create( taxonomy=self.tA1, - org=self.orgA, - rel_type=TaxonomyOrg.RelType.OWNER, - ) + org=self.orgA, rel_type=TaxonomyOrg.RelType.OWNER,) self.tA2 = Taxonomy.objects.create(name="tA2", enabled=False) TaxonomyOrg.objects.create( taxonomy=self.tA2, @@ -142,92 +229,139 @@ class TestTaxonomyObjectsMixin: ) # OrgA and OrgB taxonomy - self.tC1 = Taxonomy.objects.create(name="tC1", enabled=True) + self.tBA1 = Taxonomy.objects.create(name="tBA1", enabled=True) TaxonomyOrg.objects.create( - taxonomy=self.tC1, + taxonomy=self.tBA1, org=self.orgA, rel_type=TaxonomyOrg.RelType.OWNER, ) TaxonomyOrg.objects.create( - taxonomy=self.tC1, + taxonomy=self.tBA1, org=self.orgB, rel_type=TaxonomyOrg.RelType.OWNER, ) - self.tC2 = Taxonomy.objects.create(name="tC2", enabled=False) + self.tBA2 = Taxonomy.objects.create(name="tBA2", enabled=False) TaxonomyOrg.objects.create( - taxonomy=self.tC2, + taxonomy=self.tBA2, org=self.orgA, rel_type=TaxonomyOrg.RelType.OWNER, ) TaxonomyOrg.objects.create( - taxonomy=self.tC2, + taxonomy=self.tBA2, org=self.orgB, rel_type=TaxonomyOrg.RelType.OWNER, ) + def setUp(self): + + super().setUp() + + self._setUp_orgs() + self._setUp_courses() + self._setUp_library() + self._setUp_users() + self._setUp_taxonomies() + @skip_unless_cms @ddt.ddt -@override_settings(FEATURES={"ENABLE_CREATOR_GROUP": True}) -class TestTaxonomyViewSet(TestTaxonomyObjectsMixin, APITestCase): +class TestTaxonomyListCreateViewSet(TestTaxonomyObjectsMixin, APITestCase): """ - Test cases for TaxonomyViewSet when ENABLE_CREATOR_GROUP is True + Test cases for TaxonomyViewSet for list and create actions """ - @ddt.data( - ("user", None, None, ("ot1", "st1", "t1", "tA1", "tB1", "tC1")), - ("userA", None, None, ("ot1", "st1", "t1", "tA1", "tB1", "tC1")), - ("userS", None, None, ("ot1", "ot2", "st1", "st2", "t1", "t2", "tA1", "tA2", "tB1", "tB2")), - # Default page_size=10, and so "tC1" and "tC2" appear on the second page - ("user", True, None, ("ot1", "st1", "t1", "tA1", "tB1", "tC1")), - ("userA", True, None, ("ot1", "st1", "t1", "tA1", "tB1", "tC1")), - ("userS", True, None, ("ot1", "st1", "t1", "tA1", "tB1", "tC1")), - ("user", False, None, ()), - ("userA", False, None, ()), - ("userS", False, None, ("ot2", "st2", "t2", "tA2", "tB2", "tC2")), - ("user", None, "orgA", ("st1", "t1", "tA1", "tC1")), - ("userA", None, "orgA", ("st1", "t1", "tA1", "tC1")), - ("userS", None, "orgA", ("st1", "st2", "t1", "t2", "tA1", "tA2", "tC1", "tC2")), - ("user", True, "orgA", ("st1", "t1", "tA1", "tC1")), - ("userA", True, "orgA", ("st1", "t1", "tA1", "tC1")), - ("userS", True, "orgA", ("st1", "t1", "tA1", "tC1")), - ("user", False, "orgA", ()), - ("userA", False, "orgA", ()), - ("userS", False, "orgA", ("st2", "t2", "tA2", "tC2")), - ("user", None, "orgX", ("st1", "t1")), - ("userA", None, "orgX", ("st1", "t1")), - ("userS", None, "orgX", ("st1", "st2", "t1", "t2")), - ("user", True, "orgX", ("st1", "t1")), - ("userA", True, "orgX", ("st1", "t1")), - ("userS", True, "orgX", ("st1", "t1")), - ("user", False, "orgX", ()), - ("userA", False, "orgX", ()), - ("userS", False, "orgX", ("st2", "t2")), - ) - @ddt.unpack - def test_list_taxonomy(self, user_attr, enabled_parameter, org_name, expected_taxonomies): + def _test_list_taxonomy( + self, + user_attr: str, + expected_taxonomies: list[str], + enabled_parameter: bool | None = None, + org_parameter: str | None = None + ) -> None: + """ + Helper function to call the list endpoint and check the response + """ url = TAXONOMY_ORG_LIST_URL - if user_attr: - user = getattr(self, user_attr) - self.client.force_authenticate(user=user) + user = getattr(self, user_attr) + self.client.force_authenticate(user=user) # Set parameters cleaning empty values - query_params = {k: v for k, v in {"enabled": enabled_parameter, "org": org_name}.items() if v is not None} + query_params = {k: v for k, v in {"enabled": enabled_parameter, "org": org_parameter}.items() if v is not None} response = self.client.get(url, query_params, format="json") assert response.status_code == status.HTTP_200_OK self.assertEqual(set(t["name"] for t in response.data["results"]), set(expected_taxonomies)) - def test_list_taxonomy_invalid_org( - self, - ): + def test_list_taxonomy_staff(self) -> None: + """ + Tests that staff users see all taxonomies + """ + # Default page_size=10, and so "tBA1" and "tBA2" appear on the second page + expected_taxonomies = ["ot1", "ot2", "st1", "st2", "t1", "t2", "tA1", "tA2", "tB1", "tB2"] + self._test_list_taxonomy( + user_attr="staff", + expected_taxonomies=expected_taxonomies, + ) + + @ddt.data( + "content_creatorA", + "instructorA", + "library_staffA", + "course_instructorA", + "course_staffA", + "library_userA", + ) + def test_list_taxonomy_orgA(self, user_attr: str) -> None: + """ + Tests that non staff users from orgA can see only enabled taxonomies from orgA and global taxonomies + """ + expected_taxonomies = ["st1", "t1", "tA1", "tBA1"] + self._test_list_taxonomy( + user_attr=user_attr, + enabled_parameter=True, + expected_taxonomies=expected_taxonomies, + ) + + @ddt.data( + (True, ["ot1", "st1", "t1", "tA1", "tB1", "tBA1"]), + (False, ["ot2", "st2", "t2", "tA2", "tB2", "tBA2"]), + ) + @ddt.unpack + def test_list_taxonomy_enabled_filter(self, enabled_parameter: bool, expected_taxonomies: list[str]) -> None: + """ + Tests that the enabled filter works as expected + """ + self._test_list_taxonomy( + user_attr="staff", + enabled_parameter=enabled_parameter, + expected_taxonomies=expected_taxonomies + ) + + @ddt.data( + ("orgA", ["st1", "st2", "t1", "t2", "tA1", "tA2", "tBA1", "tBA2"]), + ("orgB", ["st1", "st2", "t1", "t2", "tB1", "tB2", "tBA1", "tBA2"]), + ("orgX", ["st1", "st2", "t1", "t2"]), + ) + @ddt.unpack + def test_list_taxonomy_org_filter(self, org_parameter: str, expected_taxonomies: list[str]) -> None: + """ + Tests that the org filter works as expected + """ + self._test_list_taxonomy( + user_attr="staff", + org_parameter=org_parameter, + expected_taxonomies=expected_taxonomies, + ) + + def test_list_taxonomy_invalid_org(self) -> None: + """ + Tests that using an invalid org in the filter will raise BAD_REQUEST + """ url = TAXONOMY_ORG_LIST_URL - self.client.force_authenticate(user=self.userS) + self.client.force_authenticate(user=self.staff) - # Set parameters cleaning empty values query_params = {"org": "invalidOrg"} response = self.client.get(url, query_params, format="json") @@ -235,30 +369,38 @@ class TestTaxonomyViewSet(TestTaxonomyObjectsMixin, APITestCase): assert response.status_code == status.HTTP_400_BAD_REQUEST @ddt.data( - ("user", ("tA1", "tB1", "tC1"), None), - ("userA", ("tA1", "tB1", "tC1"), None), - ("userS", ("st2", "t1", "t2"), "3"), + ("user", (), None), + ("staffA", ["tA2", "tBA1", "tBA2"], None), + ("staff", ["st2", "t1", "t2"], "3"), ) @ddt.unpack - def test_list_taxonomy_pagination(self, user_attr, expected_taxonomies, expected_next_page): + def test_list_taxonomy_pagination( + self, user_attr: str, expected_taxonomies: list[str], expected_next_page: str | None + ) -> None: + """ + Tests that the pagination works as expected + """ url = TAXONOMY_ORG_LIST_URL - if user_attr: - user = getattr(self, user_attr) - self.client.force_authenticate(user=user) + user = getattr(self, user_attr) + self.client.force_authenticate(user=user) query_params = {"page_size": 3, "page": 2} response = self.client.get(url, query_params, format="json") - assert response.status_code == status.HTTP_200_OK - self.assertEqual(set(t["name"] for t in response.data["results"]), set(expected_taxonomies)) - parsed_url = urlparse(response.data["next"]) + assert response.status_code == status.HTTP_200_OK if len(expected_taxonomies) > 0 else status.HTTP_404_NOT_FOUND + if status.is_success(response.status_code): + self.assertEqual(set(t["name"] for t in response.data["results"]), set(expected_taxonomies)) + parsed_url = urlparse(response.data["next"]) - next_page = parse_qs(parsed_url.query).get("page", [None])[0] - assert next_page == expected_next_page + next_page = parse_qs(parsed_url.query).get("page", [None])[0] + assert next_page == expected_next_page - def test_list_invalid_page(self): + def test_list_invalid_page(self) -> None: + """ + Tests that using an invalid page will raise NOT_FOUND + """ url = TAXONOMY_ORG_LIST_URL self.client.force_authenticate(user=self.user) @@ -269,80 +411,23 @@ class TestTaxonomyViewSet(TestTaxonomyObjectsMixin, APITestCase): assert response.status_code == status.HTTP_404_NOT_FOUND - @ddt.data( - (None, "ot1", status.HTTP_401_UNAUTHORIZED), - (None, "ot2", status.HTTP_401_UNAUTHORIZED), - (None, "st1", status.HTTP_401_UNAUTHORIZED), - (None, "st2", status.HTTP_401_UNAUTHORIZED), - (None, "t1", status.HTTP_401_UNAUTHORIZED), - (None, "t2", status.HTTP_401_UNAUTHORIZED), - (None, "tA1", status.HTTP_401_UNAUTHORIZED), - (None, "tA2", status.HTTP_401_UNAUTHORIZED), - (None, "tB1", status.HTTP_401_UNAUTHORIZED), - (None, "tB2", status.HTTP_401_UNAUTHORIZED), - (None, "tC1", status.HTTP_401_UNAUTHORIZED), - (None, "tC2", status.HTTP_401_UNAUTHORIZED), - ("user", "ot1", status.HTTP_200_OK), - ("user", "ot2", status.HTTP_404_NOT_FOUND), - ("user", "st1", status.HTTP_200_OK), - ("user", "st2", status.HTTP_404_NOT_FOUND), - ("user", "t1", status.HTTP_200_OK), - ("user", "t2", status.HTTP_404_NOT_FOUND), - ("user", "tA1", status.HTTP_200_OK), - ("user", "tA2", status.HTTP_404_NOT_FOUND), - ("user", "tB1", status.HTTP_200_OK), - ("user", "tB2", status.HTTP_404_NOT_FOUND), - ("user", "tC1", status.HTTP_200_OK), - ("user", "tC2", status.HTTP_404_NOT_FOUND), - ("userA", "ot1", status.HTTP_200_OK), - ("userA", "ot2", status.HTTP_404_NOT_FOUND), - ("userA", "st1", status.HTTP_200_OK), - ("userA", "st2", status.HTTP_404_NOT_FOUND), - ("userA", "t1", status.HTTP_200_OK), - ("userA", "t2", status.HTTP_404_NOT_FOUND), - ("userA", "tA1", status.HTTP_200_OK), - ("userA", "tA2", status.HTTP_404_NOT_FOUND), - ("userA", "tB1", status.HTTP_200_OK), - ("userA", "tB2", status.HTTP_404_NOT_FOUND), - ("userA", "tC1", status.HTTP_200_OK), - ("userA", "tC2", status.HTTP_404_NOT_FOUND), - ("userS", "ot1", status.HTTP_200_OK), - ("userS", "ot2", status.HTTP_200_OK), - ("userS", "st1", status.HTTP_200_OK), - ("userS", "st2", status.HTTP_200_OK), - ("userS", "t1", status.HTTP_200_OK), - ("userS", "t2", status.HTTP_200_OK), - ("userS", "tA1", status.HTTP_200_OK), - ("userS", "tA2", status.HTTP_200_OK), - ("userS", "tB1", status.HTTP_200_OK), - ("userS", "tB2", status.HTTP_200_OK), - ("userS", "tC1", status.HTTP_200_OK), - ("userS", "tC2", status.HTTP_200_OK), - ) - @ddt.unpack - def test_detail_taxonomy(self, user_attr, taxonomy_attr, expected_status): - taxonomy = getattr(self, taxonomy_attr) - - url = TAXONOMY_ORG_DETAIL_URL.format(pk=taxonomy.pk) - - if user_attr: - user = getattr(self, user_attr) - self.client.force_authenticate(user=user) - - response = self.client.get(url) - assert response.status_code == expected_status - - if status.is_success(expected_status): - check_taxonomy(response.data, taxonomy.pk, **(TaxonomySerializer(taxonomy.cast()).data)) - @ddt.data( (None, status.HTTP_401_UNAUTHORIZED), ("user", status.HTTP_403_FORBIDDEN), - ("userA", status.HTTP_403_FORBIDDEN), - ("userS", status.HTTP_201_CREATED), + ("content_creatorA", status.HTTP_403_FORBIDDEN), + ("instructorA", status.HTTP_403_FORBIDDEN), + ("library_staffA", status.HTTP_403_FORBIDDEN), + ("course_instructorA", status.HTTP_403_FORBIDDEN), + ("course_staffA", status.HTTP_403_FORBIDDEN), + ("library_userA", status.HTTP_403_FORBIDDEN), + ("staffA", status.HTTP_201_CREATED), + ("staff", status.HTTP_201_CREATED), ) @ddt.unpack - def test_create_taxonomy(self, user_attr, expected_status): + def test_create_taxonomy(self, user_attr: str, expected_status: int) -> None: + """ + Tests that only Taxonomy admins and org level admins can create taxonomies + """ url = TAXONOMY_ORG_LIST_URL create_data = { @@ -367,73 +452,514 @@ class TestTaxonomyViewSet(TestTaxonomyObjectsMixin, APITestCase): response = self.client.get(url) check_taxonomy(response.data, response.data["id"], **create_data) + # Also checks if the taxonomy was associated with the org + if user_attr == "staffA": + assert TaxonomyOrg.objects.filter(taxonomy=response.data["id"], org=self.orgA).exists() + + +@ddt.ddt +class TestTaxonomyDetailExportMixin(TestTaxonomyObjectsMixin): + """ + Test cases to be used with detail and export actions + """ + + @abc.abstractmethod + def _test_api_call(self, **_kwargs) -> None: + """ + Helper function to call the detail/export endpoint and check the response + """ + @ddt.data( - (None, "ot1", status.HTTP_401_UNAUTHORIZED), - (None, "ot2", status.HTTP_401_UNAUTHORIZED), - (None, "st1", status.HTTP_401_UNAUTHORIZED), - (None, "st2", status.HTTP_401_UNAUTHORIZED), - (None, "t1", status.HTTP_401_UNAUTHORIZED), - (None, "t2", status.HTTP_401_UNAUTHORIZED), - (None, "tA1", status.HTTP_401_UNAUTHORIZED), - (None, "tA2", status.HTTP_401_UNAUTHORIZED), - (None, "tB1", status.HTTP_401_UNAUTHORIZED), - (None, "tB2", status.HTTP_401_UNAUTHORIZED), - (None, "tC1", status.HTTP_401_UNAUTHORIZED), - (None, "tC2", status.HTTP_401_UNAUTHORIZED), - ("user", "ot1", status.HTTP_403_FORBIDDEN), - ("user", "ot2", status.HTTP_403_FORBIDDEN), - ("user", "st1", status.HTTP_403_FORBIDDEN), - ("user", "st2", status.HTTP_403_FORBIDDEN), - ("user", "t1", status.HTTP_403_FORBIDDEN), - ("user", "t2", status.HTTP_403_FORBIDDEN), - ("user", "tA1", status.HTTP_403_FORBIDDEN), - ("user", "tA2", status.HTTP_403_FORBIDDEN), - ("user", "tB1", status.HTTP_403_FORBIDDEN), - ("user", "tB2", status.HTTP_403_FORBIDDEN), - ("user", "tC1", status.HTTP_403_FORBIDDEN), - ("user", "tC2", status.HTTP_403_FORBIDDEN), - ("userA", "ot1", status.HTTP_403_FORBIDDEN), - ("userA", "ot2", status.HTTP_403_FORBIDDEN), - ("userA", "st1", status.HTTP_403_FORBIDDEN), - ("userA", "st2", status.HTTP_403_FORBIDDEN), - ("userA", "t1", status.HTTP_403_FORBIDDEN), - ("userA", "t2", status.HTTP_403_FORBIDDEN), - ("userA", "tA1", status.HTTP_403_FORBIDDEN), - ("userA", "tA2", status.HTTP_403_FORBIDDEN), - ("userA", "tB1", status.HTTP_403_FORBIDDEN), - ("userA", "tB2", status.HTTP_403_FORBIDDEN), - ("userA", "tC1", status.HTTP_403_FORBIDDEN), - ("userA", "tC2", status.HTTP_403_FORBIDDEN), - ("userS", "ot1", status.HTTP_200_OK), - ("userS", "ot2", status.HTTP_200_OK), - ("userS", "st1", status.HTTP_403_FORBIDDEN), - ("userS", "st2", status.HTTP_403_FORBIDDEN), - ("userS", "t1", status.HTTP_200_OK), - ("userS", "t2", status.HTTP_200_OK), - ("userS", "t1", status.HTTP_200_OK), - ("userS", "t2", status.HTTP_200_OK), - ("userS", "tA1", status.HTTP_200_OK), - ("userS", "tA2", status.HTTP_200_OK), - ("userS", "tB1", status.HTTP_200_OK), - ("userS", "tB2", status.HTTP_200_OK), - ("userS", "tC1", status.HTTP_200_OK), - ("userS", "tC2", status.HTTP_200_OK), + "user", + "content_creatorA", + "instructorA", + "library_staffA", + "course_instructorA", + "course_staffA", + "library_userA", + ) + def test_detail_taxonomy_all_org_enabled(self, user_attr: str) -> None: + """ + Tests that everyone can see enabled global taxonomies + """ + self._test_api_call( + user_attr=user_attr, + taxonomy_attr="t1", + expected_status=status.HTTP_200_OK, + reason="Everyone should see enabled global taxonomies", + ) + + @ddt.data( + ("content_creatorA", "tA1", "User with OrgContentCreatorRole(orgA) should see an enabled taxonomy from orgA"), + ("content_creatorA", "tBA1", "User with OrgContentCreatorRole(orgA) should see an enabled taxonomy from orgA"), + ("content_creatorA", "t1", "User with OrgContentCreatorRole(orgA) should see an enabled global taxonomy"), + ("instructorA", "tA1", "User with OrgInstructorRole(orgA) should see an enabled taxonomy from orgA"), + ("instructorA", "tBA1", "User with OrgInstructorRole(orgA) should see an enabled taxonomy from orgA"), + ("instructorA", "t1", "User with OrgInstructorRole(orgA) should see an enabled global taxonomy"), + ("library_staffA", "tA1", "User with OrgLibraryUserRole(orgA) should see an enabled taxonomy from orgA"), + ("library_staffA", "tBA1", "User with OrgLibraryUserRole(orgA) should see an enabled taxonomy from orgA"), + ("library_staffA", "t1", "User with OrgInstructorRole(orgA) should see an enabled global taxonomy"), + ( + "course_instructorA", + "tA1", + "User with CourseInstructorRole in a course from orgA should see an enabled taxonomy from orgA" + ), + ( + "course_instructorA", + "tBA1", + "User with CourseInstructorRole in a course from orgA should see an enabled taxonomy from orgA" + ), + ( + "course_instructorA", + "t1", + "User with CourseInstructorRole in a course from orgA should see an enabled global taxonomy" + ), + ( + "course_staffA", + "tA1", + "User with CourseStaffRole in a course from orgA should see an enabled taxonomy from orgA" + ), + ( + "course_staffA", + "tBA1", + "User with CourseStaffRole in a course from orgA should see an enabled taxonomy from orgA" + ), + ( + "course_staffA", + "t1", + "User with CourseStaffRole in a course from orgA should see an enabled global taxonomy" + ), + ( + "library_userA", + "tA1", + "User with permission on a library from orgA should see an enabled taxonomy from orgA" + ), + ( + "library_userA", + "tBA1", + "User with permission on a library from orgA should see an enabled taxonomy from orgA" + ), + ( + "library_userA", + "t1", + "User with permission on a library from orgA should see an enabled global taxonomy" + ), ) @ddt.unpack - def test_update_taxonomy(self, user_attr, taxonomy_attr, expected_status): + def test_detail_taxonomy_org_user_see_enabled(self, user_attr: str, taxonomy_attr: str, reason: str) -> None: + """ + Tests that org users (content creators and instructors) can see enabled global taxonomies and taxonomies + from their orgs + """ + self._test_api_call( + user_attr=user_attr, + taxonomy_attr=taxonomy_attr, + expected_status=status.HTTP_200_OK, + reason=reason, + ) + + @ddt.data( + "tA2", + "tBA2", + ) + def test_detail_taxonomy_org_admin_see_disabled(self, taxonomy_attr: str) -> None: + """ + Tests that org admins can see disabled taxonomies from their orgs + """ + self._test_api_call( + user_attr="staffA", + taxonomy_attr=taxonomy_attr, + expected_status=status.HTTP_200_OK, + reason="User with OrgContentCreatorRole(orgA) should see a disabled taxonomy from orgA", + ) + + @ddt.data( + "st2", + "t2", + ) + def test_detail_taxonomy_org_admin_dont_see_disabled_global(self, taxonomy_attr: str) -> None: + """ + Tests that org admins can't see disabled global taxonomies + """ + self._test_api_call( + user_attr="staffA", + taxonomy_attr=taxonomy_attr, + expected_status=status.HTTP_404_NOT_FOUND, + reason="User with OrgContentCreatorRole(orgA) shouldn't see a disabled global taxonomy", + ) + + @ddt.data( + ("content_creatorA", "t2", "User with OrgContentCreatorRole(orgA) shouldn't see a disabled global taxonomy"), + ("instructorA", "tA2", "User with OrgInstructorRole(orgA) shouldn't see a disabled taxonomy from orgA"), + ("instructorA", "tBA2", "User with OrgInstructorRole(orgA) shouldn't see a disabled taxonomy from orgA"), + ("instructorA", "t2", "User with OrgInstructorRole(orgA) shouldn't see a disabled global taxonomy"), + ("library_staffA", "tA2", "User with OrgLibraryUserRole(orgA) shouldn't see a disabled taxonomy from orgA"), + ("library_staffA", "tBA2", "User with OrgLibraryUserRole(orgA) shouldn't see a disabled taxonomy from orgA"), + ("library_staffA", "t2", "User with OrgInstructorRole(orgA) shouldn't see a disabled global taxonomy"), + ( + "course_instructorA", + "tA2", + "User with CourseInstructorRole in a course from orgA shouldn't see a disabled taxonomy from orgA" + ), + ( + "course_instructorA", + "tBA2", + "User with CourseInstructorRole in a course from orgA shouldn't see a disabled taxonomy from orgA" + ), + ( + "course_instructorA", + "t2", + "User with CourseInstructorRole in a course from orgA shouldn't see a disabled global taxonomy" + ), + ( + "course_staffA", + "tA2", + "User with CourseStaffRole in a course from orgA shouldn't see a disabled taxonomy from orgA" + ), + ( + "course_staffA", + "tBA2", + "User with CourseStaffRole in a course from orgA shouldn't see a disabled taxonomy from orgA" + ), + ( + "course_staffA", + "t2", + "User with CourseStaffRole in a course from orgA should't see a disabled global taxonomy" + ), + ( + "library_userA", + "tA2", + "User with permission on a library from orgA shouldn't see an disabled taxonomy from orgA" + ), + ( + "library_userA", + "tBA2", + "User with permission on a library from orgA shouldn't see an disabled taxonomy from orgA" + ), + ( + "library_userA", + "t2", + "User with permission on a library from orgA shouldn't see an disabled global taxonomy" + ), + ) + @ddt.unpack + def test_detail_taxonomy_org_user_dont_see_disabled(self, user_attr: str, taxonomy_attr: str, reason: str) -> None: + """ + Tests that org users (content creators and instructors) can't see disabled global taxonomies and taxonomies + from their orgs + """ + self._test_api_call( + user_attr=user_attr, + taxonomy_attr=taxonomy_attr, + expected_status=status.HTTP_404_NOT_FOUND, + reason=reason, + ) + + @ddt.data( + ("staff", "ot1", "Staff should see an enabled no org taxonomy"), + ("staff", "ot2", "Staff should see a disabled no org taxonomy"), + ) + @ddt.unpack + def test_detail_taxonomy_staff_see_no_org(self, user_attr: str, taxonomy_attr: str, reason: str) -> None: + """ + Tests that staff can see taxonomies with no org + """ + self._test_api_call( + user_attr=user_attr, + taxonomy_attr=taxonomy_attr, + expected_status=status.HTTP_200_OK, + reason=reason, + ) + + @ddt.data( + "staffA", + "content_creatorA", + "instructorA", + "library_staffA", + "course_instructorA", + "course_staffA", + "library_userA" + ) + def test_detail_taxonomy_other_dont_see_no_org(self, user_attr: str) -> None: + """ + Tests that org users can't see taxonomies with no org + """ + self._test_api_call( + user_attr=user_attr, + taxonomy_attr="ot1", + expected_status=status.HTTP_404_NOT_FOUND, + reason="Only staff should see taxonomies with no org", + ) + + @ddt.data( + "staffA", + "content_creatorA", + "instructorA", + "library_staffA", + "course_instructorA", + "course_staffA", + "library_userA" + ) + def test_detail_taxonomy_dont_see_other_org(self, user_attr: str) -> None: + """ + Tests that org users can't see taxonomies from other orgs + """ + self._test_api_call( + user_attr=user_attr, + taxonomy_attr="tB1", + expected_status=status.HTTP_404_NOT_FOUND, + reason="Users shouldn't see taxonomies from other orgs", + ) + + @ddt.data( + "ot1", + "ot2", + "st1", + "st2", + "t1", + "t2", + "tA1", + "tA2", + "tB1", + "tB2", + "tBA1", + "tBA2", + ) + def test_detail_taxonomy_staff_see_all(self, taxonomy_attr: str) -> None: + """ + Tests that staff can see all taxonomies + """ + self._test_api_call( + user_attr="staff", + taxonomy_attr=taxonomy_attr, + expected_status=status.HTTP_200_OK, + reason="Staff should see all taxonomies", + ) + + +@skip_unless_cms +class TestTaxonomyDetailViewSet(TestTaxonomyDetailExportMixin, APITestCase): + """ + Test cases for TaxonomyViewSet with detail action + """ + + def _test_api_call(self, **kwargs) -> None: + """ + Helper function to call the retrieve endpoint and check the response + """ + user_attr = kwargs.get("user_attr") + taxonomy_attr = kwargs.get("taxonomy_attr") + expected_status = kwargs.get("expected_status") + reason = kwargs.get("reason", "Unexpected response status") + + assert taxonomy_attr is not None, "taxonomy_attr is required" + assert user_attr is not None, "user_attr is required" + assert expected_status is not None, "expected_status is required" + taxonomy = getattr(self, taxonomy_attr) url = TAXONOMY_ORG_DETAIL_URL.format(pk=taxonomy.pk) - if user_attr: - user = getattr(self, user_attr) - self.client.force_authenticate(user=user) + user = getattr(self, user_attr) + self.client.force_authenticate(user=user) + + response = self.client.get(url) + assert response.status_code == expected_status, reason + + if status.is_success(expected_status): + check_taxonomy(response.data, taxonomy.pk, **(TaxonomySerializer(taxonomy.cast()).data)) + + +@skip_unless_cms +class TestTaxonomyExportViewSet(TestTaxonomyDetailExportMixin, APITestCase): + """ + Test cases for TaxonomyViewSet with export action + """ + + def _test_api_call(self, **kwargs) -> None: + """ + Helper function to call the export endpoint and check the response + """ + user_attr = kwargs.get("user_attr") + taxonomy_attr = kwargs.get("taxonomy_attr") + expected_status = kwargs.get("expected_status") + reason = kwargs.get("reason", "Unexpected response status") + + assert taxonomy_attr is not None, "taxonomy_attr is required" + assert user_attr is not None, "user_attr is required" + assert expected_status is not None, "expected_status is required" + + taxonomy = getattr(self, taxonomy_attr) + + url = TAXONOMY_ORG_DETAIL_URL.format(pk=taxonomy.pk) + + user = getattr(self, user_attr) + self.client.force_authenticate(user=user) + + response = self.client.get(url) + assert response.status_code == expected_status, reason + assert len(response.data) > 0 + + +@ddt.ddt +class TestTaxonomyChangeMixin(TestTaxonomyObjectsMixin): + """ + Test cases to be used with update, patch and delete actions + """ + + @abc.abstractmethod + def _test_api_call(self, **_kwargs) -> None: + """ + Helper function to call the update/patch/delete endpoint and check the response + """ + + @ddt.data( + "ot1", + "ot2", + "st1", + "st2", + "t1", + "t2", + "tA1", + "tA2", + "tB1", + "tB2", + "tBA1", + "tBA2", + ) + def test_regular_user_cant_edit_taxonomies(self, taxonomy_attr: str) -> None: + """ + Tests that regular users can't edit taxonomies + """ + self._test_api_call( + user_attr="user", + taxonomy_attr=taxonomy_attr, + expected_status=[status.HTTP_403_FORBIDDEN, status.HTTP_404_NOT_FOUND], + reason="Regular users shouldn't be able to edit taxonomies", + ) + + @ddt.data( + "content_creatorA", + "instructorA", + "library_staffA", + "course_instructorA", + "course_staffA", + "library_userA", + ) + def test_org_user_cant_edit_org_taxonomies(self, user_attr: str) -> None: + """ + Tests that content creators and instructors from orgA can't edit taxonomies from orgA + """ + self._test_api_call( + user_attr=user_attr, + taxonomy_attr="tA1", + expected_status=[status.HTTP_403_FORBIDDEN], + reason="Content creators and instructors shouldn't be able to edit taxonomies", + ) + + @ddt.data( + "tA1", + "tA2", + "tBA1", + "tBA2", + ) + def test_org_staff_can_edit_org_taxonomies(self, taxonomy_attr: str) -> None: + """ + Tests that org staff can edit taxonomies from their orgs + """ + self._test_api_call( + user_attr="staffA", + taxonomy_attr=taxonomy_attr, + # Check both status: 200 for update and 204 for delete + expected_status=[status.HTTP_200_OK, status.HTTP_204_NO_CONTENT], + reason="Org staff should be able to edit taxonomies from their orgs", + ) + + @ddt.data( + "tB1", + "tB2", + ) + def test_org_staff_cant_edit_other_org_taxonomies(self, taxonomy_attr: str) -> None: + """ + Tests that org staff can't edit taxonomies from other orgs + """ + self._test_api_call( + user_attr="staffA", + taxonomy_attr=taxonomy_attr, + expected_status=[status.HTTP_403_FORBIDDEN, status.HTTP_404_NOT_FOUND], + reason="Org staff shouldn't be able to edit taxonomies from other orgs", + ) + + @ddt.data( + "ot1", + "ot2", + "t1", + "t2", + "tA1", + "tA2", + "tB1", + "tB2", + "tBA1", + "tBA2", + + ) + def test_staff_can_edit_almost_all_taxonomies(self, taxonomy_attr: str) -> None: + """ + Tests that staff can edit all but system defined taxonomies + """ + self._test_api_call( + user_attr="staff", + taxonomy_attr=taxonomy_attr, + # Check both status: 200 for update and 204 for delete + expected_status=[status.HTTP_200_OK, status.HTTP_204_NO_CONTENT], + reason="Staff should be able to edit all but system defined taxonomies", + ) + + @ddt.data( + "st1", + "st2", + ) + def test_staff_cant_edit_system_defined_taxonomies(self, taxonomy_attr: str) -> None: + """ + Tests that staff can't edit system defined taxonomies + """ + self._test_api_call( + user_attr="staff", + taxonomy_attr=taxonomy_attr, + # Check both status: 200 for update and 204 for delete + expected_status=[status.HTTP_403_FORBIDDEN], + reason="Staff shouldn't be able to edit system defined ", + ) + + +@skip_unless_cms +class TestTaxonomyUpdateViewSet(TestTaxonomyChangeMixin, APITestCase): + """ + Test cases for TaxonomyViewSet with PUT method + """ + + def _test_api_call(self, **kwargs) -> None: + user_attr = kwargs.get("user_attr") + taxonomy_attr = kwargs.get("taxonomy_attr") + expected_status = kwargs.get("expected_status") + reason = kwargs.get("reason", "Unexpected response status") + + assert taxonomy_attr is not None, "taxonomy_attr is required" + assert user_attr is not None, "user_attr is required" + assert expected_status is not None, "expected_status is required" + + taxonomy = getattr(self, taxonomy_attr) + + url = TAXONOMY_ORG_DETAIL_URL.format(pk=taxonomy.pk) + + user = getattr(self, user_attr) + self.client.force_authenticate(user=user) response = self.client.put(url, {"name": "new name"}, format="json") - assert response.status_code == expected_status + assert response.status_code in expected_status, reason # If we were able to update the taxonomy, check if the name changed - if status.is_success(expected_status): + if status.is_success(response.status_code): response = self.client.get(url) check_taxonomy( response.data, @@ -445,90 +971,35 @@ class TestTaxonomyViewSet(TestTaxonomyObjectsMixin, APITestCase): }, ) - @ddt.data( - (False, status.HTTP_403_FORBIDDEN), - (True, status.HTTP_403_FORBIDDEN), - ) - @ddt.unpack - def test_update_taxonomy_system_defined(self, update_value, expected_status): - """ - Test that we can't update system_defined field - """ - url = TAXONOMY_ORG_DETAIL_URL.format(pk=self.st1.pk) - self.client.force_authenticate(user=self.userS) - response = self.client.put(url, {"name": "new name", "system_defined": update_value}, format="json") - assert response.status_code == expected_status +@skip_unless_cms +class TestTaxonomyPatchViewSet(TestTaxonomyChangeMixin, APITestCase): + """ + Test cases for TaxonomyViewSet with PATCH method + """ - # Verify that system_defined has not changed - response = self.client.get(url) - assert response.data["system_defined"] is True + def _test_api_call(self, **kwargs) -> None: + user_attr = kwargs.get("user_attr") + taxonomy_attr = kwargs.get("taxonomy_attr") + expected_status = kwargs.get("expected_status") + reason = kwargs.get("reason", "Unexpected response status") + + assert taxonomy_attr is not None, "taxonomy_attr is required" + assert user_attr is not None, "user_attr is required" + assert expected_status is not None, "expected_status is required" - @ddt.data( - (None, "ot1", status.HTTP_401_UNAUTHORIZED), - (None, "ot2", status.HTTP_401_UNAUTHORIZED), - (None, "st1", status.HTTP_401_UNAUTHORIZED), - (None, "st2", status.HTTP_401_UNAUTHORIZED), - (None, "t1", status.HTTP_401_UNAUTHORIZED), - (None, "t2", status.HTTP_401_UNAUTHORIZED), - (None, "tA1", status.HTTP_401_UNAUTHORIZED), - (None, "tA2", status.HTTP_401_UNAUTHORIZED), - (None, "tB1", status.HTTP_401_UNAUTHORIZED), - (None, "tB2", status.HTTP_401_UNAUTHORIZED), - (None, "tC1", status.HTTP_401_UNAUTHORIZED), - (None, "tC2", status.HTTP_401_UNAUTHORIZED), - ("user", "ot1", status.HTTP_403_FORBIDDEN), - ("user", "ot2", status.HTTP_403_FORBIDDEN), - ("user", "st1", status.HTTP_403_FORBIDDEN), - ("user", "st2", status.HTTP_403_FORBIDDEN), - ("user", "t1", status.HTTP_403_FORBIDDEN), - ("user", "t2", status.HTTP_403_FORBIDDEN), - ("user", "tA1", status.HTTP_403_FORBIDDEN), - ("user", "tA2", status.HTTP_403_FORBIDDEN), - ("user", "tB1", status.HTTP_403_FORBIDDEN), - ("user", "tB2", status.HTTP_403_FORBIDDEN), - ("user", "tC1", status.HTTP_403_FORBIDDEN), - ("user", "tC2", status.HTTP_403_FORBIDDEN), - ("userA", "ot1", status.HTTP_403_FORBIDDEN), - ("userA", "ot2", status.HTTP_403_FORBIDDEN), - ("userA", "st1", status.HTTP_403_FORBIDDEN), - ("userA", "st2", status.HTTP_403_FORBIDDEN), - ("userA", "t1", status.HTTP_403_FORBIDDEN), - ("userA", "t2", status.HTTP_403_FORBIDDEN), - ("userA", "tA1", status.HTTP_403_FORBIDDEN), - ("userA", "tA2", status.HTTP_403_FORBIDDEN), - ("userA", "tB1", status.HTTP_403_FORBIDDEN), - ("userA", "tB2", status.HTTP_403_FORBIDDEN), - ("userA", "tC1", status.HTTP_403_FORBIDDEN), - ("userA", "tC2", status.HTTP_403_FORBIDDEN), - ("userS", "ot1", status.HTTP_200_OK), - ("userS", "ot2", status.HTTP_200_OK), - ("userS", "st1", status.HTTP_403_FORBIDDEN), - ("userS", "st2", status.HTTP_403_FORBIDDEN), - ("userS", "t1", status.HTTP_200_OK), - ("userS", "t2", status.HTTP_200_OK), - ("userS", "tA1", status.HTTP_200_OK), - ("userS", "tA2", status.HTTP_200_OK), - ("userS", "tB1", status.HTTP_200_OK), - ("userS", "tB2", status.HTTP_200_OK), - ("userS", "tC1", status.HTTP_200_OK), - ("userS", "tC2", status.HTTP_200_OK), - ) - @ddt.unpack - def test_patch_taxonomy(self, user_attr, taxonomy_attr, expected_status): taxonomy = getattr(self, taxonomy_attr) url = TAXONOMY_ORG_DETAIL_URL.format(pk=taxonomy.pk) - if user_attr: - user = getattr(self, user_attr) - self.client.force_authenticate(user=user) + user = getattr(self, user_attr) + self.client.force_authenticate(user=user) response = self.client.patch(url, {"name": "new name"}, format="json") - assert response.status_code == expected_status + assert response.status_code in expected_status, reason # If we were able to patch the taxonomy, check if the name changed - if status.is_success(expected_status): + if status.is_success(response.status_code): response = self.client.get(url) check_taxonomy( response.data, @@ -540,123 +1011,53 @@ class TestTaxonomyViewSet(TestTaxonomyObjectsMixin, APITestCase): }, ) - @ddt.data( - (False, status.HTTP_403_FORBIDDEN), - (True, status.HTTP_403_FORBIDDEN), - ) - @ddt.unpack - def test_patch_taxonomy_system_defined(self, update_value, expected_status): - """ - Test that we can't patch system_defined field - """ - url = TAXONOMY_ORG_DETAIL_URL.format(pk=self.st1.pk) - self.client.force_authenticate(user=self.userS) - response = self.client.patch(url, {"name": "new name", "system_defined": update_value}, format="json") - assert response.status_code == expected_status +@skip_unless_cms +class TestTaxonomyDeleteViewSet(TestTaxonomyChangeMixin, APITestCase): + """ + Test cases for TaxonomyViewSet with DELETE method + """ - # Verify that system_defined has not changed - response = self.client.get(url) - assert response.data["system_defined"] is True + def _test_api_call(self, **kwargs) -> None: + user_attr = kwargs.get("user_attr") + taxonomy_attr = kwargs.get("taxonomy_attr") + expected_status = kwargs.get("expected_status") + reason = kwargs.get("reason", "Unexpected response status") + + assert taxonomy_attr is not None, "taxonomy_attr is required" + assert user_attr is not None, "user_attr is required" + assert expected_status is not None, "expected_status is required" - @ddt.data( - (None, "ot1", status.HTTP_401_UNAUTHORIZED), - (None, "ot2", status.HTTP_401_UNAUTHORIZED), - (None, "st1", status.HTTP_401_UNAUTHORIZED), - (None, "st2", status.HTTP_401_UNAUTHORIZED), - (None, "t1", status.HTTP_401_UNAUTHORIZED), - (None, "t2", status.HTTP_401_UNAUTHORIZED), - (None, "tA1", status.HTTP_401_UNAUTHORIZED), - (None, "tA2", status.HTTP_401_UNAUTHORIZED), - (None, "tB1", status.HTTP_401_UNAUTHORIZED), - (None, "tB2", status.HTTP_401_UNAUTHORIZED), - (None, "tC1", status.HTTP_401_UNAUTHORIZED), - (None, "tC2", status.HTTP_401_UNAUTHORIZED), - ("user", "ot1", status.HTTP_403_FORBIDDEN), - ("user", "ot2", status.HTTP_403_FORBIDDEN), - ("user", "st1", status.HTTP_403_FORBIDDEN), - ("user", "st2", status.HTTP_403_FORBIDDEN), - ("user", "t1", status.HTTP_403_FORBIDDEN), - ("user", "t2", status.HTTP_403_FORBIDDEN), - ("user", "tA1", status.HTTP_403_FORBIDDEN), - ("user", "tA2", status.HTTP_403_FORBIDDEN), - ("user", "tB1", status.HTTP_403_FORBIDDEN), - ("user", "tB2", status.HTTP_403_FORBIDDEN), - ("user", "tC1", status.HTTP_403_FORBIDDEN), - ("user", "tC2", status.HTTP_403_FORBIDDEN), - ("userA", "ot1", status.HTTP_403_FORBIDDEN), - ("userA", "ot2", status.HTTP_403_FORBIDDEN), - ("userA", "st1", status.HTTP_403_FORBIDDEN), - ("userA", "st2", status.HTTP_403_FORBIDDEN), - ("userA", "t1", status.HTTP_403_FORBIDDEN), - ("userA", "t2", status.HTTP_403_FORBIDDEN), - ("userA", "tA1", status.HTTP_403_FORBIDDEN), - ("userA", "tA2", status.HTTP_403_FORBIDDEN), - ("userA", "tB1", status.HTTP_403_FORBIDDEN), - ("userA", "tB2", status.HTTP_403_FORBIDDEN), - ("userA", "tC1", status.HTTP_403_FORBIDDEN), - ("userA", "tC2", status.HTTP_403_FORBIDDEN), - ("userS", "ot1", status.HTTP_204_NO_CONTENT), - ("userS", "ot2", status.HTTP_204_NO_CONTENT), - ("userS", "st1", status.HTTP_403_FORBIDDEN), - ("userS", "st2", status.HTTP_403_FORBIDDEN), - ("userS", "t1", status.HTTP_204_NO_CONTENT), - ("userS", "t2", status.HTTP_204_NO_CONTENT), - ("userS", "tA1", status.HTTP_204_NO_CONTENT), - ("userS", "tA2", status.HTTP_204_NO_CONTENT), - ("userS", "tB1", status.HTTP_204_NO_CONTENT), - ("userS", "tB2", status.HTTP_204_NO_CONTENT), - ("userS", "tC1", status.HTTP_204_NO_CONTENT), - ("userS", "tC2", status.HTTP_204_NO_CONTENT), - ) - @ddt.unpack - def test_delete_taxonomy(self, user_attr, taxonomy_attr, expected_status): taxonomy = getattr(self, taxonomy_attr) url = TAXONOMY_ORG_DETAIL_URL.format(pk=taxonomy.pk) - if user_attr: - user = getattr(self, user_attr) - self.client.force_authenticate(user=user) + user = getattr(self, user_attr) + self.client.force_authenticate(user=user) response = self.client.delete(url) - assert response.status_code == expected_status + assert response.status_code in expected_status, reason # If we were able to delete the taxonomy, check that it's really gone - if status.is_success(expected_status): + if status.is_success(response.status_code): response = self.client.get(url) assert response.status_code == status.HTTP_404_NOT_FOUND -@skip_unless_cms -@ddt.ddt -@override_settings(FEATURES={"ENABLE_CREATOR_GROUP": False}) -class TestTaxonomyViewSetNoCreatorGroup(TestTaxonomyViewSet): # pylint: disable=test-inherits-tests +class TestObjectTagMixin(TestTaxonomyObjectsMixin): """ - Test cases for TaxonomyViewSet when ENABLE_CREATOR_GROUP is False - - The permissions are the same for when ENABLED_CREATOR_GRUP is True - """ - - -@skip_unless_cms -@ddt.ddt -class TestObjectTagViewSet(TestTaxonomyObjectsMixin, APITestCase): - """ - Testing various cases for the ObjectTagView. + Sets up data for testing ObjectTags. """ def setUp(self): """ Setup the test cases """ super().setUp() - self.courseA = CourseLocator("orgA", "101", "test") self.xblockA = BlockUsageLocator( course_key=self.courseA, block_type='problem', block_id='block_id' ) - self.courseB = CourseLocator("orgB", "101", "test") self.xblockB = BlockUsageLocator( course_key=self.courseB, block_type='problem', @@ -691,37 +1092,41 @@ class TestObjectTagViewSet(TestTaxonomyObjectsMixin, APITestCase): rel_type=TaxonomyOrg.RelType.OWNER, ) - add_users(self.userS, CourseStaffRole(self.courseA), self.userA) + add_users(self.staff, CourseStaffRole(self.courseA), self.staffA) + + +@skip_unless_cms +@ddt.ddt +class TestObjectTagViewSet(TestObjectTagMixin, APITestCase): + """ + Testing various cases for the ObjectTagView. + """ + + def test_get_tags(self): + pass @ddt.data( # userA and userS are staff in courseA and can tag using enabled taxonomies - (None, "tA1", ["Tag 1"], status.HTTP_401_UNAUTHORIZED), ("user", "tA1", ["Tag 1"], status.HTTP_403_FORBIDDEN), - ("userA", "tA1", ["Tag 1"], status.HTTP_200_OK), - ("userS", "tA1", ["Tag 1"], status.HTTP_200_OK), - (None, "tA1", [], status.HTTP_401_UNAUTHORIZED), + ("staffA", "tA1", ["Tag 1"], status.HTTP_200_OK), + ("staff", "tA1", ["Tag 1"], status.HTTP_200_OK), ("user", "tA1", [], status.HTTP_403_FORBIDDEN), - ("userA", "tA1", [], status.HTTP_200_OK), - ("userS", "tA1", [], status.HTTP_200_OK), - (None, "multiple_taxonomy", ["Tag 1", "Tag 2"], status.HTTP_401_UNAUTHORIZED), + ("staffA", "tA1", [], status.HTTP_200_OK), + ("staff", "tA1", [], status.HTTP_200_OK), ("user", "multiple_taxonomy", ["Tag 1", "Tag 2"], status.HTTP_403_FORBIDDEN), - ("userA", "multiple_taxonomy", ["Tag 1", "Tag 2"], status.HTTP_200_OK), - ("userS", "multiple_taxonomy", ["Tag 1", "Tag 2"], status.HTTP_200_OK), - (None, "open_taxonomy", ["tag1"], status.HTTP_401_UNAUTHORIZED), + ("staffA", "multiple_taxonomy", ["Tag 1", "Tag 2"], status.HTTP_200_OK), + ("staff", "multiple_taxonomy", ["Tag 1", "Tag 2"], status.HTTP_200_OK), ("user", "open_taxonomy", ["tag1"], status.HTTP_403_FORBIDDEN), - ("userA", "open_taxonomy", ["tag1"], status.HTTP_200_OK), - ("userS", "open_taxonomy", ["tag1"], status.HTTP_200_OK), - # Only userS is Tagging Admin and can tag objects using disabled taxonomies - (None, "tA2", ["Tag 1"], status.HTTP_401_UNAUTHORIZED), - ("user", "tA2", ["Tag 1"], status.HTTP_403_FORBIDDEN), - ("userA", "tA2", ["Tag 1"], status.HTTP_403_FORBIDDEN), - ("userS", "tA2", ["Tag 1"], status.HTTP_200_OK), + ("staffA", "open_taxonomy", ["tag1"], status.HTTP_200_OK), + ("staff", "open_taxonomy", ["tag1"], status.HTTP_200_OK), ) @ddt.unpack def test_tag_course(self, user_attr, taxonomy_attr, tag_values, expected_status): - if user_attr: - user = getattr(self, user_attr) - self.client.force_authenticate(user=user) + """ + Tests that only staff and org level users can tag courses + """ + user = getattr(self, user_attr) + self.client.force_authenticate(user=user) taxonomy = getattr(self, taxonomy_attr) @@ -734,62 +1139,72 @@ class TestObjectTagViewSet(TestTaxonomyObjectsMixin, APITestCase): assert len(response.data) == len(tag_values) assert set(t["value"] for t in response.data) == set(tag_values) + # Check that re-fetching the tags returns what we set + response = self.client.get(url, format="json") + assert status.is_success(response.status_code) + assert set(t["value"] for t in response.data) == set(tag_values) + @ddt.data( - # Can't add invalid tags to a object using a closed taxonomy - (None, "tA1", ["invalid"], status.HTTP_401_UNAUTHORIZED), - ("user", "tA1", ["invalid"], status.HTTP_403_FORBIDDEN), - ("userA", "tA1", ["invalid"], status.HTTP_400_BAD_REQUEST), - ("userS", "tA1", ["invalid"], status.HTTP_400_BAD_REQUEST), - (None, "multiple_taxonomy", ["invalid"], status.HTTP_401_UNAUTHORIZED), - ("user", "multiple_taxonomy", ["invalid"], status.HTTP_403_FORBIDDEN), - ("userA", "multiple_taxonomy", ["invalid"], status.HTTP_400_BAD_REQUEST), - ("userS", "multiple_taxonomy", ["invalid"], status.HTTP_400_BAD_REQUEST), - # Staff can't add invalid tags to a object using a closed taxonomy - ("userS", "tA2", ["invalid"], status.HTTP_400_BAD_REQUEST), + "staffA", + "staff", + ) + def test_tag_course_disabled_taxonomy(self, user_attr): + """ + Nobody can use disable taxonomies to tag objects + """ + user = getattr(self, user_attr) + self.client.force_authenticate(user=user) + + disabled_taxonomy = self.tA2 + assert disabled_taxonomy.enabled is False + + url = OBJECT_TAG_UPDATE_URL.format(object_id=self.courseA, taxonomy_id=disabled_taxonomy.pk) + response = self.client.put(url, {"tags": ["Tag 1"]}, format="json") + + assert response.status_code == status.HTTP_403_FORBIDDEN + + @ddt.data( + ("staffA", "tA1"), + ("staff", "tA1"), + ("staffA", "multiple_taxonomy"), + ("staff", "multiple_taxonomy"), ) @ddt.unpack - def test_tag_course_invalid(self, user_attr, taxonomy_attr, tag_values, expected_status): - if user_attr: - user = getattr(self, user_attr) - self.client.force_authenticate(user=user) + def test_tag_course_invalid(self, user_attr, taxonomy_attr): + """ + Tests that nobody can add invalid tags to a course using a closed taxonomy + """ + user = getattr(self, user_attr) + self.client.force_authenticate(user=user) taxonomy = getattr(self, taxonomy_attr) url = OBJECT_TAG_UPDATE_URL.format(object_id=self.courseA, taxonomy_id=taxonomy.pk) - response = self.client.put(url, {"tags": tag_values}, format="json") - assert response.status_code == expected_status - assert not status.is_success(expected_status) # No success cases here + response = self.client.put(url, {"tags": ["invalid"]}, format="json") + assert response.status_code == status.HTTP_400_BAD_REQUEST @ddt.data( - # userA and userS are staff in courseA (owner of xblockA) and can tag using enabled taxonomies - (None, "tA1", ["Tag 1"], status.HTTP_401_UNAUTHORIZED), + # userA and userS are staff in courseA (owner of xblockA) and can tag using any taxonomies ("user", "tA1", ["Tag 1"], status.HTTP_403_FORBIDDEN), - ("userA", "tA1", ["Tag 1"], status.HTTP_200_OK), - ("userS", "tA1", ["Tag 1"], status.HTTP_200_OK), - (None, "tA1", [], status.HTTP_401_UNAUTHORIZED), - ("user", "tA1", [], status.HTTP_403_FORBIDDEN), - ("userA", "tA1", [], status.HTTP_200_OK), - ("userS", "tA1", [], status.HTTP_200_OK), - (None, "multiple_taxonomy", ["Tag 1", "Tag 2"], status.HTTP_401_UNAUTHORIZED), + ("staffA", "tA1", ["Tag 1"], status.HTTP_200_OK), + ("staff", "tA1", ["Tag 1"], status.HTTP_200_OK), ("user", "multiple_taxonomy", ["Tag 1", "Tag 2"], status.HTTP_403_FORBIDDEN), - ("userA", "multiple_taxonomy", ["Tag 1", "Tag 2"], status.HTTP_200_OK), - ("userS", "multiple_taxonomy", ["Tag 1", "Tag 2"], status.HTTP_200_OK), - (None, "open_taxonomy", ["tag1"], status.HTTP_401_UNAUTHORIZED), + ("staffA", "tA1", [], status.HTTP_200_OK), + ("staff", "tA1", [], status.HTTP_200_OK), + ("staffA", "multiple_taxonomy", ["Tag 1", "Tag 2"], status.HTTP_200_OK), + ("staff", "multiple_taxonomy", ["Tag 1", "Tag 2"], status.HTTP_200_OK), ("user", "open_taxonomy", ["tag1"], status.HTTP_403_FORBIDDEN), - ("userA", "open_taxonomy", ["tag1"], status.HTTP_200_OK), - ("userS", "open_taxonomy", ["tag1"], status.HTTP_200_OK), - # Only userS is Tagging Admin and can tag objects using disabled taxonomies - (None, "tA2", ["Tag 1"], status.HTTP_401_UNAUTHORIZED), - ("user", "tA2", ["Tag 1"], status.HTTP_403_FORBIDDEN), - ("userA", "tA2", ["Tag 1"], status.HTTP_403_FORBIDDEN), - ("userS", "tA2", ["Tag 1"], status.HTTP_200_OK), + ("staffA", "open_taxonomy", ["tag1"], status.HTTP_200_OK), + ("staff", "open_taxonomy", ["tag1"], status.HTTP_200_OK), ) @ddt.unpack def test_tag_xblock(self, user_attr, taxonomy_attr, tag_values, expected_status): - if user_attr: - user = getattr(self, user_attr) - self.client.force_authenticate(user=user) + """ + Tests that only staff and org level users can tag xblocks + """ + user = getattr(self, user_attr) + self.client.force_authenticate(user=user) taxonomy = getattr(self, taxonomy_attr) @@ -802,32 +1217,67 @@ class TestObjectTagViewSet(TestTaxonomyObjectsMixin, APITestCase): assert len(response.data) == len(tag_values) assert set(t["value"] for t in response.data) == set(tag_values) + # Check that re-fetching the tags returns what we set + response = self.client.get(url, format="json") + assert status.is_success(response.status_code) + assert set(t["value"] for t in response.data) == set(tag_values) + @ddt.data( - # Can't add invalid tags to a object using a closed taxonomy - (None, "tA1", ["invalid"], status.HTTP_401_UNAUTHORIZED), - ("user", "tA1", ["invalid"], status.HTTP_403_FORBIDDEN), - ("userA", "tA1", ["invalid"], status.HTTP_400_BAD_REQUEST), - ("userS", "tA1", ["invalid"], status.HTTP_400_BAD_REQUEST), - (None, "multiple_taxonomy", ["invalid"], status.HTTP_401_UNAUTHORIZED), - ("user", "multiple_taxonomy", ["invalid"], status.HTTP_403_FORBIDDEN), - ("userA", "multiple_taxonomy", ["invalid"], status.HTTP_400_BAD_REQUEST), - ("userS", "multiple_taxonomy", ["invalid"], status.HTTP_400_BAD_REQUEST), - # Staff can't add invalid tags to a object using a closed taxonomy - ("userS", "tA2", ["invalid"], status.HTTP_400_BAD_REQUEST), + "staffA", + "staff", + ) + def test_tag_xblock_disabled_taxonomy(self, user_attr): + """ + Tests that nobody can use disabled taxonomies to tag xblocks + """ + user = getattr(self, user_attr) + self.client.force_authenticate(user=user) + + disabled_taxonomy = self.tA2 + assert disabled_taxonomy.enabled is False + + url = OBJECT_TAG_UPDATE_URL.format(object_id=self.xblockA, taxonomy_id=disabled_taxonomy.pk) + response = self.client.put(url, {"tags": ["Tag 1"]}, format="json") + + assert response.status_code == status.HTTP_403_FORBIDDEN + + @ddt.data( + ("staffA", "tA1"), + ("staff", "tA1"), + ("staffA", "multiple_taxonomy"), + ("staff", "multiple_taxonomy"), ) @ddt.unpack - def test_tag_xblock_invalid(self, user_attr, taxonomy_attr, tag_values, expected_status): - if user_attr: - user = getattr(self, user_attr) - self.client.force_authenticate(user=user) + def test_tag_xblock_invalid(self, user_attr, taxonomy_attr): + """ + Tests that staff can't add invalid tags to a xblock using a closed taxonomy + """ + user = getattr(self, user_attr) + self.client.force_authenticate(user=user) taxonomy = getattr(self, taxonomy_attr) url = OBJECT_TAG_UPDATE_URL.format(object_id=self.xblockA, taxonomy_id=taxonomy.pk) - response = self.client.put(url, {"tags": tag_values}, format="json") - assert response.status_code == expected_status - assert not status.is_success(expected_status) # No success cases here + response = self.client.put(url, {"tags": ["invalid"]}, format="json") + assert response.status_code == status.HTTP_400_BAD_REQUEST + + @ddt.data( + "courseB", + "xblockB", + ) + def test_tag_no_permission(self, objectid_attr): + """ + Test that a user without access to courseB can't apply tags to it + """ + self.client.force_authenticate(user=self.staffA) + object_id = getattr(self, objectid_attr) + + url = OBJECT_TAG_UPDATE_URL.format(object_id=object_id, taxonomy_id=self.tA1.pk) + + response = self.client.put(url, {"tags": ["Tag 1"]}, format="json") + + assert response.status_code == status.HTTP_403_FORBIDDEN @ddt.data( "courseB", @@ -837,14 +1287,13 @@ class TestObjectTagViewSet(TestTaxonomyObjectsMixin, APITestCase): """ Test that a user without access to courseB can't apply tags to it """ - self.client.force_authenticate(user=self.userA) object_id = getattr(self, objectid_attr) url = OBJECT_TAG_UPDATE_URL.format(object_id=object_id, taxonomy_id=self.tA1.pk) response = self.client.put(url, {"tags": ["Tag 1"]}, format="json") - assert response.status_code == status.HTTP_403_FORBIDDEN + assert response.status_code == status.HTTP_401_UNAUTHORIZED @skip_unless_cms diff --git a/openedx/core/djangoapps/content_tagging/rest_api/v1/urls.py b/openedx/core/djangoapps/content_tagging/rest_api/v1/urls.py index d6fc047e52..38bb0a9ac1 100644 --- a/openedx/core/djangoapps/content_tagging/rest_api/v1/urls.py +++ b/openedx/core/djangoapps/content_tagging/rest_api/v1/urls.py @@ -7,7 +7,6 @@ from rest_framework.routers import DefaultRouter from django.urls.conf import path, include from openedx_tagging.core.tagging.rest_api.v1 import ( - views as oel_tagging_views, views_import as oel_tagging_views_import, ) @@ -15,7 +14,7 @@ from . import views router = DefaultRouter() router.register("taxonomies", views.TaxonomyOrgView, basename="taxonomy") -router.register("object_tags", oel_tagging_views.ObjectTagView, basename="object_tag") +router.register("object_tags", views.ObjectTagOrgView, basename="object_tag") urlpatterns = [ path( diff --git a/openedx/core/djangoapps/content_tagging/rest_api/v1/views.py b/openedx/core/djangoapps/content_tagging/rest_api/v1/views.py index 7bdfe7cd39..6f1f7dd73a 100644 --- a/openedx/core/djangoapps/content_tagging/rest_api/v1/views.py +++ b/openedx/core/djangoapps/content_tagging/rest_api/v1/views.py @@ -2,7 +2,7 @@ Tagging Org API Views """ -from openedx_tagging.core.tagging.rest_api.v1.views import TaxonomyView +from openedx_tagging.core.tagging.rest_api.v1.views import ObjectTagView, TaxonomyView from ...api import ( @@ -10,8 +10,9 @@ from ...api import ( get_taxonomies, get_taxonomies_for_org, ) +from ...rules import get_admin_orgs from .serializers import TaxonomyOrgListQueryParamsSerializer -from .filters import UserOrgFilterBackend +from .filters import ObjectTagTaxonomyOrgFilterBackend, UserOrgFilterBackend class TaxonomyOrgView(TaxonomyView): @@ -57,4 +58,15 @@ class TaxonomyOrgView(TaxonomyView): """ Create a new taxonomy. """ - serializer.instance = create_taxonomy(**serializer.validated_data) + user_admin_orgs = get_admin_orgs(self.request.user) + serializer.instance = create_taxonomy(**serializer.validated_data, orgs=user_admin_orgs) + + +class ObjectTagOrgView(ObjectTagView): + """ + View to create and retrieve ObjectTags for a provided Object ID (object_id). + This view extends the ObjectTagView to add Organization filters for the results. + + Refer to ObjectTagView docstring for usage details. + """ + filter_backends = [ObjectTagTaxonomyOrgFilterBackend] diff --git a/openedx/core/djangoapps/content_tagging/rules.py b/openedx/core/djangoapps/content_tagging/rules.py index b046fe5173..5206f62045 100644 --- a/openedx/core/djangoapps/content_tagging/rules.py +++ b/openedx/core/djangoapps/content_tagging/rules.py @@ -6,35 +6,210 @@ from typing import Union import django.contrib.auth.models import openedx_tagging.core.tagging.rules as oel_tagging +from organizations.models import Organization import rules from opaque_keys import InvalidKeyError from opaque_keys.edx.keys import CourseKey, UsageKey -from common.djangoapps.student.auth import is_content_creator, has_studio_write_access +from common.djangoapps.student.auth import has_studio_read_access, has_studio_write_access +from common.djangoapps.student.models import CourseAccessRole +from common.djangoapps.student.roles import ( + CourseInstructorRole, + CourseStaffRole, + OrgContentCreatorRole, + OrgInstructorRole, + OrgLibraryUserRole, + OrgStaffRole +) +from openedx.core.djangoapps.content_libraries.api import get_libraries_for_user from .models import TaxonomyOrg UserType = Union[django.contrib.auth.models.User, django.contrib.auth.models.AnonymousUser] -def is_taxonomy_user(user: UserType, taxonomy: oel_tagging.Taxonomy) -> bool: +def is_org_admin(user: UserType, orgs: list[Organization] | None = None) -> bool: + """ + Return True if the given user is an admin for any of the given orgs. """ - Returns True if the given user is a Taxonomy User for the given content taxonomy. - Taxonomy users include global staff and superusers, plus course creators who can create courses for any org. - Otherwise, we need to check taxonomy provided to determine if the user is an org-level course creator for one of - the orgs allowed to use this taxonomy. Only global staff and superusers can use disabled system taxonomies. + return len(get_admin_orgs(user, orgs)) > 0 + + +def is_org_user(user: UserType, orgs: list[Organization]) -> bool: """ + Return True if the given user is a member of any of the given orgs. + """ + return len(get_user_orgs(user, orgs)) > 0 + + +def get_admin_orgs(user: UserType, orgs: list[Organization] | None = None) -> list[Organization]: + """ + Returns a list of orgs that the given user is an admin, from the given list of orgs. + + If no orgs are provided, check all orgs + """ + org_list = Organization.objects.all() if orgs is None else orgs + return [ + org for org in org_list if OrgStaffRole(org=org.short_name).has_user(user) + ] + + +def get_content_creator_orgs(user: UserType, orgs: list[Organization]) -> list[Organization]: + """ + Returns a list of orgs that the given user is a content creator, the given list of orgs. + """ + return [ + org for org in orgs if ( + OrgLibraryUserRole(org=org.short_name).has_user(user) or + OrgInstructorRole(org=org.short_name).has_user(user) or + OrgContentCreatorRole(org=org.short_name).has_user(user) + ) + ] + + +def get_instructor_orgs(user: UserType, orgs: list[Organization]) -> list[Organization]: + """ + Returns a list of orgs that the given user is an instructor, from the given list of orgs. + """ + instructor_roles = CourseAccessRole.objects.filter( + org__in=(org.short_name for org in orgs), + user=user, + role__in=(CourseStaffRole.ROLE, CourseInstructorRole.ROLE), + ) + instructor_orgs = [role.org for role in instructor_roles] + return [org for org in orgs if org.short_name in instructor_orgs] + + +def get_library_user_orgs(user: UserType, orgs: list[Organization]) -> list[Organization]: + """ + Returns a list of orgs that the given user has explicity permission, from the given list of orgs. + """ + return [ + org for org in orgs if ( + len(get_libraries_for_user(user, org=org.short_name)) > 0 + ) + ] + + +def get_user_orgs(user: UserType, orgs: list[Organization]) -> list[Organization]: + """ + Return a list of orgs that the given user is a member of (instructor or content creator), + from the given list of orgs. + """ + content_creator_orgs = get_content_creator_orgs(user, orgs) + instructor_orgs = get_instructor_orgs(user, orgs) + library_user_orgs = get_library_user_orgs(user, orgs) + user_orgs = list(set(content_creator_orgs) | set(instructor_orgs) | set(library_user_orgs)) + + return user_orgs + + +def can_create_taxonomy(user: UserType) -> bool: + """ + Returns True if the given user can create a taxonomy. + + Taxonomy admins and org-level staff can create taxonomies. + """ + # Taxonomy admins can view any taxonomy if oel_tagging.is_taxonomy_admin(user): return True + # Org-level staff can create taxonomies associated with one of their orgs. + if is_org_admin(user): + return True + + return False + + +@rules.predicate +def can_view_taxonomy(user: UserType, taxonomy: oel_tagging.Taxonomy) -> bool: + """ + Returns True if the given user can view the given taxonomy. + + Taxonomy admins can view any taxonomy. + Org-level staff can view any taxonomy that is associated with one of their orgs. + Org-level course creators and instructors can view any enabled taxonomy that is owned by one of their orgs. + """ + # The following code allows METHOD permission (GET) in the viewset for everyone + if not taxonomy: + return True + + taxonomy = taxonomy.cast() + + # Taxonomy admins can view any taxonomy + if oel_tagging.is_taxonomy_admin(user): + return True + + is_all_org = TaxonomyOrg.objects.filter( + taxonomy=taxonomy, + org=None, + rel_type=TaxonomyOrg.RelType.OWNER, + ).exists() + + # Enabled all-org taxonomies can be viewed by any registred user + if is_all_org: + return taxonomy.enabled + taxonomy_orgs = TaxonomyOrg.get_organizations( taxonomy=taxonomy, rel_type=TaxonomyOrg.RelType.OWNER, ) - for org in taxonomy_orgs: - if is_content_creator(user, org.short_name): - return True + + # Org-level staff can view any taxonomy that is associated with one of their orgs. + if is_org_admin(user, taxonomy_orgs): + return True + + # Org-level course creators and instructors can view any enabled taxonomy that is owned by one of their orgs. + if is_org_user(user, taxonomy_orgs): + return taxonomy.enabled + + return False + + +@rules.predicate +def can_change_taxonomy(user: UserType, taxonomy: oel_tagging.Taxonomy) -> bool: + """ + Returns True if the given user can edit the given taxonomy. + + System definied taxonomies cannot be edited + Taxonomy admins can edit any non system defined taxonomies + Only taxonomy admins can edit all org taxonomies + Org-level staff can edit any taxonomy that is associated with one of their orgs. + """ + # The following code allows METHOD permission (PUT, PATCH) in the viewset for everyone + if not taxonomy: + return True + + taxonomy = taxonomy.cast() + + # System definied taxonomies cannot be edited + if taxonomy.system_defined: + return False + + # Taxonomy admins can edit any non system defined taxonomies + if oel_tagging.is_taxonomy_admin(user): + return True + + is_all_org = TaxonomyOrg.objects.filter( + taxonomy=taxonomy, + org=None, + rel_type=TaxonomyOrg.RelType.OWNER, + ).exists() + + # Only taxonomy admins can edit all org taxonomies + if is_all_org: + return False + + taxonomy_orgs = TaxonomyOrg.get_organizations( + taxonomy=taxonomy, + rel_type=TaxonomyOrg.RelType.OWNER, + ) + + # Org-level staff can edit any taxonomy that is associated with one of their orgs. + if is_org_admin(user, taxonomy_orgs): + return True + return False @@ -57,12 +232,32 @@ def can_change_object_tag_objectid(user: UserType, object_id: str) -> bool: @rules.predicate -def can_change_object_tag_taxonomy(user: UserType, taxonomy: oel_tagging.Taxonomy) -> bool: +def can_view_object_tag_taxonomy(user: UserType, taxonomy: oel_tagging.Taxonomy) -> bool: """ - Taxonomy users can tag objects using tags from any taxonomy that they have permission to view. Only taxonomy admins - can tag objects using tags from disabled taxonomies. + Only enabled taxonomy and users with permission to view this taxonomy can view object tags + from that taxonomy. + + This rule is different from can_view_taxonomy because it checks if the taxonomy is enabled. """ - return oel_tagging.is_taxonomy_admin(user) or (taxonomy.cast().enabled and is_taxonomy_user(user, taxonomy)) + return taxonomy.cast().enabled and can_view_taxonomy(user, taxonomy) + + +@rules.predicate +def can_view_object_tag_objectid(user: UserType, object_id: str) -> bool: + """ + Everyone that has permission to view the object should be able to tag it. + """ + if not object_id: + raise ValueError("object_id must be provided") + try: + usage_key = UsageKey.from_string(object_id) + if not usage_key.course_key.is_course: + raise ValueError("object_id must be from a block or a course") + course_key = usage_key.course_key + except InvalidKeyError: + course_key = CourseKey.from_string(object_id) + + return has_studio_read_access(user, course_key) @rules.predicate @@ -82,11 +277,11 @@ def can_change_taxonomy_tag(user: UserType, tag: oel_tagging.Tag | None = None) # Taxonomy -rules.set_perm("oel_tagging.add_taxonomy", oel_tagging.is_taxonomy_admin) -rules.set_perm("oel_tagging.change_taxonomy", oel_tagging.can_change_taxonomy) -rules.set_perm("oel_tagging.delete_taxonomy", oel_tagging.can_change_taxonomy) -rules.set_perm("oel_tagging.view_taxonomy", oel_tagging.can_view_taxonomy) -rules.set_perm("oel_tagging.export_taxonomy", oel_tagging.can_view_taxonomy) +rules.set_perm("oel_tagging.add_taxonomy", can_create_taxonomy) +rules.set_perm("oel_tagging.change_taxonomy", can_change_taxonomy) +rules.set_perm("oel_tagging.delete_taxonomy", can_change_taxonomy) +rules.set_perm("oel_tagging.view_taxonomy", can_view_taxonomy) +rules.set_perm("oel_tagging.export_taxonomy", can_view_taxonomy) # Tag rules.set_perm("oel_tagging.add_tag", can_change_taxonomy_tag) @@ -96,11 +291,13 @@ rules.set_perm("oel_tagging.view_tag", rules.always_allow) # ObjectTag rules.set_perm("oel_tagging.add_object_tag", oel_tagging.can_change_object_tag) -rules.set_perm("oel_tagging.change_object_tag", oel_tagging.can_change_object_tag) -rules.set_perm("oel_tagging.delete_object_tag", oel_tagging.can_change_object_tag) -rules.set_perm("oel_tagging.view_object_tag", rules.always_allow) +rules.set_perm("oel_tagging.change_objecttag", oel_tagging.can_change_object_tag) +rules.set_perm("oel_tagging.delete_objecttag", oel_tagging.can_change_object_tag) +rules.set_perm("oel_tagging.view_objecttag", oel_tagging.can_view_object_tag) # This perms are used in the tagging rest api from openedx_tagging that is exposed in the CMS. They are overridden here # to include Organization and objects permissions. -rules.set_perm("oel_tagging.change_objecttag_taxonomy", can_change_object_tag_taxonomy) +rules.set_perm("oel_tagging.view_objecttag_taxonomy", can_view_object_tag_taxonomy) +rules.set_perm("oel_tagging.view_objecttag_objectid", can_view_object_tag_objectid) +rules.set_perm("oel_tagging.change_objecttag_taxonomy", can_view_object_tag_taxonomy) rules.set_perm("oel_tagging.change_objecttag_objectid", can_change_object_tag_objectid) diff --git a/openedx/core/djangoapps/content_tagging/tests/test_api.py b/openedx/core/djangoapps/content_tagging/tests/test_api.py index c0595dffb6..985199322d 100644 --- a/openedx/core/djangoapps/content_tagging/tests/test_api.py +++ b/openedx/core/djangoapps/content_tagging/tests/test_api.py @@ -22,7 +22,7 @@ class TestTaxonomyMixin: name="Learning Objectives", enabled=False, ) - api.set_taxonomy_orgs(self.taxonomy_disabled, all_orgs=True) + api.set_taxonomy_orgs(self.taxonomy_disabled, orgs=[self.org1, self.org2]) self.taxonomy_all_orgs = api.create_taxonomy( name="Content Types", enabled=True, @@ -121,8 +121,8 @@ class TestAPITaxonomy(TestTaxonomyMixin, TestCase): @ddt.data( # All orgs (None, True, ["taxonomy_all_orgs"]), - (None, False, ["taxonomy_disabled"]), - (None, None, ["taxonomy_all_orgs", "taxonomy_disabled"]), + (None, False, []), + (None, None, ["taxonomy_all_orgs"]), # Org 1 ("org1", True, ["taxonomy_all_orgs", "taxonomy_one_org", "taxonomy_both_orgs"]), ("org1", False, ["taxonomy_disabled"]), diff --git a/openedx/core/djangoapps/content_tagging/tests/test_rules.py b/openedx/core/djangoapps/content_tagging/tests/test_rules.py index 4edb43b28b..9c1187ab91 100644 --- a/openedx/core/djangoapps/content_tagging/tests/test_rules.py +++ b/openedx/core/djangoapps/content_tagging/tests/test_rules.py @@ -2,17 +2,16 @@ import ddt from django.contrib.auth import get_user_model -from django.test import TestCase, override_settings +from django.test import TestCase from opaque_keys.edx.locator import BlockUsageLocator, CourseLocator from openedx_tagging.core.tagging.models import ( Tag, UserSystemDefinedTaxonomy, ) -from openedx_tagging.core.tagging.rules import ChangeObjectTagPermissionItem -from organizations.models import Organization +from openedx_tagging.core.tagging.rules import ObjectTagPermissionItem from common.djangoapps.student.auth import add_users, update_org_role -from common.djangoapps.student.roles import CourseCreatorRole, CourseStaffRole, OrgContentCreatorRole +from common.djangoapps.student.roles import CourseStaffRole, OrgStaffRole from .. import api from .test_api import TestTaxonomyMixin @@ -21,7 +20,6 @@ User = get_user_model() @ddt.ddt -@override_settings(FEATURES={"ENABLE_CREATOR_GROUP": True}) class TestRulesTaxonomy(TestTaxonomyMixin, TestCase): """ Tests that the expected rules have been applied to the Taxonomy models. @@ -41,12 +39,6 @@ class TestRulesTaxonomy(TestTaxonomyMixin, TestCase): email="staff@example.com", is_staff=True, ) - # Normal user: grant course creator role (for all orgs) - self.user_all_orgs = User.objects.create( - username="user_all_orgs", - email="staff+all@example.com", - ) - add_users(self.staff, CourseCreatorRole(), self.user_all_orgs) # Normal user: grant course creator access to both org1 and org2 self.user_both_orgs = User.objects.create( @@ -55,7 +47,7 @@ class TestRulesTaxonomy(TestTaxonomyMixin, TestCase): ) update_org_role( self.staff, - OrgContentCreatorRole, + OrgStaffRole, self.user_both_orgs, [self.org1.short_name, self.org2.short_name], ) @@ -66,7 +58,7 @@ class TestRulesTaxonomy(TestTaxonomyMixin, TestCase): email="staff+org2@example.com", ) update_org_role( - self.staff, OrgContentCreatorRole, self.user_org2, [self.org2.short_name] + self.staff, OrgStaffRole, self.user_org2, [self.org2.short_name] ) # Normal user: no course creator access @@ -95,67 +87,65 @@ class TestRulesTaxonomy(TestTaxonomyMixin, TestCase): block_id='block_id' ) - add_users(self.staff, CourseStaffRole(self.course1), self.user_all_orgs) add_users(self.staff, CourseStaffRole(self.course1), self.user_both_orgs) - add_users(self.staff, CourseStaffRole(self.course2), self.user_all_orgs) add_users(self.staff, CourseStaffRole(self.course2), self.user_both_orgs) add_users(self.staff, CourseStaffRole(self.course2), self.user_org2) add_users(self.staff, CourseStaffRole(self.course2), self.user_org2) - self.tax_all_course1 = ChangeObjectTagPermissionItem( + self.tax_all_course1 = ObjectTagPermissionItem( taxonomy=self.taxonomy_all_orgs, object_id=str(self.course1), ) - self.tax_all_course2 = ChangeObjectTagPermissionItem( + self.tax_all_course2 = ObjectTagPermissionItem( taxonomy=self.taxonomy_all_orgs, object_id=str(self.course2), ) - self.tax_all_xblock1 = ChangeObjectTagPermissionItem( + self.tax_all_xblock1 = ObjectTagPermissionItem( taxonomy=self.taxonomy_all_orgs, object_id=str(self.xblock1), ) - self.tax_all_xblock2 = ChangeObjectTagPermissionItem( + self.tax_all_xblock2 = ObjectTagPermissionItem( taxonomy=self.taxonomy_all_orgs, object_id=str(self.xblock2), ) - self.tax_both_course1 = ChangeObjectTagPermissionItem( + self.tax_both_course1 = ObjectTagPermissionItem( taxonomy=self.taxonomy_both_orgs, object_id=str(self.course1), ) - self.tax_both_course2 = ChangeObjectTagPermissionItem( + self.tax_both_course2 = ObjectTagPermissionItem( taxonomy=self.taxonomy_both_orgs, object_id=str(self.course2), ) - self.tax_both_xblock1 = ChangeObjectTagPermissionItem( + self.tax_both_xblock1 = ObjectTagPermissionItem( taxonomy=self.taxonomy_both_orgs, object_id=str(self.xblock1), ) - self.tax_both_xblock2 = ChangeObjectTagPermissionItem( + self.tax_both_xblock2 = ObjectTagPermissionItem( taxonomy=self.taxonomy_both_orgs, object_id=str(self.xblock2), ) - self.tax1_course1 = ChangeObjectTagPermissionItem( + self.tax1_course1 = ObjectTagPermissionItem( taxonomy=self.taxonomy_one_org, object_id=str(self.course1), ) - self.tax1_xblock1 = ChangeObjectTagPermissionItem( + self.tax1_xblock1 = ObjectTagPermissionItem( taxonomy=self.taxonomy_one_org, object_id=str(self.xblock1), ) - self.tax_no_org_course1 = ChangeObjectTagPermissionItem( + self.tax_no_org_course1 = ObjectTagPermissionItem( taxonomy=self.taxonomy_no_orgs, object_id=str(self.course1), ) - self.tax_no_org_xblock1 = ChangeObjectTagPermissionItem( + self.tax_no_org_xblock1 = ObjectTagPermissionItem( taxonomy=self.taxonomy_no_orgs, object_id=str(self.xblock1), ) - self.disabled_course_tag_perm = ChangeObjectTagPermissionItem( + self.disabled_course2_tag_perm = ObjectTagPermissionItem( taxonomy=self.taxonomy_disabled, object_id=str(self.course2), ) @@ -184,8 +174,6 @@ class TestRulesTaxonomy(TestTaxonomyMixin, TestCase): assert self.superuser.has_perm(perm, obj) assert self.staff.has_perm(perm) assert self.staff.has_perm(perm, obj) - assert self.user_all_orgs.has_perm(perm) - assert self.user_all_orgs.has_perm(perm, obj) # Org content creators are bound by a taxonomy's org restrictions assert self.user_both_orgs.has_perm(perm) == learner_perm @@ -199,58 +187,78 @@ class TestRulesTaxonomy(TestTaxonomyMixin, TestCase): assert self.learner.has_perm(perm, obj) == learner_obj # Taxonomy + def test_taxonomy_base_add_permissions(self): + """ + Test that staff, superuser and org admins can call POST on taxonomies. + """ + perm = "oel_tagging.add_taxonomy" + assert self.superuser.has_perm(perm) + assert self.staff.has_perm(perm) + assert self.user_both_orgs.has_perm(perm) + assert self.user_org2.has_perm(perm) + assert not self.learner.has_perm(perm) @ddt.data( - "oel_tagging.add_taxonomy", "oel_tagging.change_taxonomy", "oel_tagging.delete_taxonomy", ) def test_taxonomy_base_edit_permissions(self, perm): """ - Test that only Staff & Superuser can call add/edit/delete taxonomies. + Test that everyone can call PUT, PATCH and DELETE on taxonomies. """ assert self.superuser.has_perm(perm) assert self.staff.has_perm(perm) - assert not self.user_all_orgs.has_perm(perm) - assert not self.user_both_orgs.has_perm(perm) - assert not self.user_org2.has_perm(perm) - assert not self.learner.has_perm(perm) + assert self.user_both_orgs.has_perm(perm) + assert self.user_org2.has_perm(perm) + assert self.learner.has_perm(perm) @ddt.data( "oel_tagging.view_taxonomy", ) def test_taxonomy_base_view_permissions(self, perm): """ - Test that everyone can call view taxonomy. + Test that everyone can call GET on taxonomies. """ assert self.superuser.has_perm(perm) assert self.staff.has_perm(perm) - assert self.user_all_orgs.has_perm(perm) assert self.user_both_orgs.has_perm(perm) assert self.user_org2.has_perm(perm) assert self.learner.has_perm(perm) @ddt.data( - ("oel_tagging.change_taxonomy", "taxonomy_all_orgs"), ("oel_tagging.change_taxonomy", "taxonomy_disabled"), ("oel_tagging.change_taxonomy", "taxonomy_both_orgs"), ("oel_tagging.change_taxonomy", "taxonomy_one_org"), - ("oel_tagging.change_taxonomy", "taxonomy_no_orgs"), - ("oel_tagging.delete_taxonomy", "taxonomy_all_orgs"), ("oel_tagging.delete_taxonomy", "taxonomy_disabled"), ("oel_tagging.delete_taxonomy", "taxonomy_both_orgs"), ("oel_tagging.delete_taxonomy", "taxonomy_one_org"), - ("oel_tagging.delete_taxonomy", "taxonomy_no_orgs"), ) @ddt.unpack def test_change_taxonomy(self, perm, taxonomy_attr): """ - Test that only Staff & Superuser can edit/delete taxonomies. + Test that only instance level and org level admins can edit/delete taxonomies from their orgs. + """ + taxonomy = getattr(self, taxonomy_attr) + assert self.superuser.has_perm(perm, taxonomy) + assert self.staff.has_perm(perm, taxonomy) + assert self.user_both_orgs.has_perm(perm, taxonomy) + assert self.user_org2.has_perm(perm, taxonomy) == (taxonomy_attr != "taxonomy_one_org") + assert not self.learner.has_perm(perm, taxonomy) + + @ddt.data( + ("oel_tagging.change_taxonomy", "taxonomy_all_orgs"), + ("oel_tagging.change_taxonomy", "taxonomy_no_orgs"), + ("oel_tagging.delete_taxonomy", "taxonomy_all_orgs"), + ("oel_tagging.delete_taxonomy", "taxonomy_no_orgs"), + ) + @ddt.unpack + def test_change_taxonomy_all_no_org(self, perm, taxonomy_attr): + """ + Test that only Staff & Superuser can edit/delete taxonomies from all or no org. """ taxonomy = getattr(self, taxonomy_attr) assert self.superuser.has_perm(perm, taxonomy) assert self.staff.has_perm(perm, taxonomy) - assert not self.user_all_orgs.has_perm(perm, taxonomy) assert not self.user_both_orgs.has_perm(perm, taxonomy) assert not self.user_org2.has_perm(perm, taxonomy) assert not self.learner.has_perm(perm, taxonomy) @@ -270,20 +278,31 @@ class TestRulesTaxonomy(TestTaxonomyMixin, TestCase): system_taxonomy = system_taxonomy.cast() assert self.superuser.has_perm(perm, system_taxonomy) assert not self.staff.has_perm(perm, system_taxonomy) - assert not self.user_all_orgs.has_perm(perm, system_taxonomy) assert not self.user_both_orgs.has_perm(perm, system_taxonomy) assert not self.user_org2.has_perm(perm, system_taxonomy) assert not self.learner.has_perm(perm, system_taxonomy) + def test_view_taxonomy_no_orgs(self): + """ + Test that only Staff & Superuser can view taxonomies with no orgs. + """ + taxonomy = self.taxonomy_no_orgs + taxonomy.enabled = True + perm = "oel_tagging.view_taxonomy" + + assert self.superuser.has_perm(perm, taxonomy) + assert self.staff.has_perm(perm, taxonomy) + assert not self.user_both_orgs.has_perm(perm, taxonomy) + assert not self.user_org2.has_perm(perm, taxonomy) + assert not self.learner.has_perm(perm, taxonomy) + @ddt.data( - "taxonomy_all_orgs", "taxonomy_both_orgs", "taxonomy_one_org", - "taxonomy_no_orgs", ) def test_view_taxonomy_enabled(self, taxonomy_attr): """ - Test that anyone can view enabled taxonomies. + Test that anyone can view enabled taxonomies from their org. """ taxonomy = getattr(self, taxonomy_attr) taxonomy.enabled = True @@ -291,20 +310,31 @@ class TestRulesTaxonomy(TestTaxonomyMixin, TestCase): assert self.superuser.has_perm(perm, taxonomy) assert self.staff.has_perm(perm, taxonomy) - assert self.user_all_orgs.has_perm(perm, taxonomy) + assert self.user_both_orgs.has_perm(perm, taxonomy) + assert self.user_org2.has_perm(perm, taxonomy) == (taxonomy_attr != "taxonomy_one_org") + assert not self.learner.has_perm(perm, taxonomy) + + def test_view_taxonomy_enabled_all_orgs(self): + """ + Test that anyone can view enabled global taxonomies. + """ + taxonomy = self.taxonomy_all_orgs + taxonomy.enabled = True + perm = "oel_tagging.view_taxonomy" + + assert self.superuser.has_perm(perm, taxonomy) + assert self.staff.has_perm(perm, taxonomy) assert self.user_both_orgs.has_perm(perm, taxonomy) assert self.user_org2.has_perm(perm, taxonomy) assert self.learner.has_perm(perm, taxonomy) @ddt.data( - "taxonomy_all_orgs", "taxonomy_both_orgs", "taxonomy_one_org", - "taxonomy_no_orgs", ) def test_view_taxonomy_disabled(self, taxonomy_attr): """ - Test that only Staff & Superuser can view disabled taxonomies. + Test that only instance level and org level admins can view disabled taxonomies. """ taxonomy = getattr(self, taxonomy_attr) taxonomy.enabled = False @@ -312,7 +342,34 @@ class TestRulesTaxonomy(TestTaxonomyMixin, TestCase): assert self.superuser.has_perm(perm, taxonomy) assert self.staff.has_perm(perm, taxonomy) - assert not self.user_all_orgs.has_perm(perm, taxonomy) + assert self.user_both_orgs.has_perm(perm, taxonomy) + assert self.user_org2.has_perm(perm, taxonomy) == (taxonomy_attr != "taxonomy_one_org") + assert not self.learner.has_perm(perm, taxonomy) + + def test_view_taxonomy_all_orgs_disabled(self): + """ + Test that only instance level admins can view disabled all org taxonomies. + """ + taxonomy = self.taxonomy_all_orgs + taxonomy.enabled = False + perm = "oel_tagging.view_taxonomy" + + assert self.superuser.has_perm(perm, taxonomy) + assert self.staff.has_perm(perm, taxonomy) + assert not self.user_both_orgs.has_perm(perm, taxonomy) + assert not self.user_org2.has_perm(perm, taxonomy) + assert not self.learner.has_perm(perm, taxonomy) + + def test_view_taxonomy_disabled_no_org(self): + """ + Test that only Staff & Superuser can view disabled taxonomies with no orgs. + """ + taxonomy = self.taxonomy_no_orgs + taxonomy.enabled = False + perm = "oel_tagging.view_taxonomy" + + assert self.superuser.has_perm(perm, taxonomy) + assert self.staff.has_perm(perm, taxonomy) assert not self.user_both_orgs.has_perm(perm, taxonomy) assert not self.user_org2.has_perm(perm, taxonomy) assert not self.learner.has_perm(perm, taxonomy) @@ -330,21 +387,17 @@ class TestRulesTaxonomy(TestTaxonomyMixin, TestCase): """ assert self.superuser.has_perm(perm) assert self.staff.has_perm(perm) - assert not self.user_all_orgs.has_perm(perm) assert not self.user_both_orgs.has_perm(perm) assert not self.user_org2.has_perm(perm) assert not self.learner.has_perm(perm) - @ddt.data( - "oel_tagging.view_tag", - ) - def test_tag_base_view_permissions(self, perm): + def test_tag_base_view_permissions(self): """ Test that everyone can call view tag. """ + perm = "oel_tagging.view_tag" assert self.superuser.has_perm(perm) assert self.staff.has_perm(perm) - assert self.user_all_orgs.has_perm(perm) assert self.user_both_orgs.has_perm(perm) assert self.user_org2.has_perm(perm) assert self.learner.has_perm(perm) @@ -369,7 +422,6 @@ class TestRulesTaxonomy(TestTaxonomyMixin, TestCase): tag = getattr(self, tag_attr) assert self.superuser.has_perm(perm, tag) assert self.staff.has_perm(perm, tag) - assert not self.user_all_orgs.has_perm(perm, tag) assert not self.user_both_orgs.has_perm(perm, tag) assert not self.user_org2.has_perm(perm, tag) assert not self.learner.has_perm(perm, tag) @@ -394,7 +446,6 @@ class TestRulesTaxonomy(TestTaxonomyMixin, TestCase): assert self.superuser.has_perm(perm, tag_system_taxonomy) assert not self.staff.has_perm(perm, tag_system_taxonomy) - assert not self.user_all_orgs.has_perm(perm, tag_system_taxonomy) assert not self.user_both_orgs.has_perm(perm, tag_system_taxonomy) assert not self.user_org2.has_perm(perm, tag_system_taxonomy) assert not self.learner.has_perm(perm, tag_system_taxonomy) @@ -419,7 +470,6 @@ class TestRulesTaxonomy(TestTaxonomyMixin, TestCase): assert self.superuser.has_perm(perm, tag_free_text_taxonomy) assert not self.staff.has_perm(perm, tag_free_text_taxonomy) - assert not self.user_all_orgs.has_perm(perm, tag_free_text_taxonomy) assert not self.user_both_orgs.has_perm(perm, tag_free_text_taxonomy) assert not self.user_org2.has_perm(perm, tag_free_text_taxonomy) assert not self.learner.has_perm(perm, tag_free_text_taxonomy) @@ -437,7 +487,6 @@ class TestRulesTaxonomy(TestTaxonomyMixin, TestCase): assert self.staff.has_perm(perm, tag) # Everyone else can't do anything - assert not self.user_all_orgs.has_perm(perm, tag) assert not self.user_both_orgs.has_perm(perm, tag) assert not self.user_org2.has_perm(perm, tag) assert not self.learner.has_perm(perm, tag) @@ -459,28 +508,29 @@ class TestRulesTaxonomy(TestTaxonomyMixin, TestCase): # ObjectTag @ddt.data( - ("oel_tagging.add_object_tag", "disabled_course_tag_perm"), - ("oel_tagging.change_object_tag", "disabled_course_tag_perm"), - ("oel_tagging.delete_object_tag", "disabled_course_tag_perm"), + ("oel_tagging.add_objecttag", "disabled_course2_tag_perm"), + ("oel_tagging.change_objecttag", "disabled_course2_tag_perm"), + ("oel_tagging.delete_objecttag", "disabled_course2_tag_perm"), ) @ddt.unpack def test_object_tag_disabled_taxonomy(self, perm, tag_attr): - """Only taxonomy administrators can create/edit an ObjectTag using a disabled Taxonomy""" + """ + Only superuser create/edit an ObjectTag using a disabled Taxonomy + """ object_tag_perm = getattr(self, tag_attr) assert self.superuser.has_perm(perm, object_tag_perm) - assert self.staff.has_perm(perm, object_tag_perm) - assert not self.user_all_orgs.has_perm(perm, object_tag_perm) + assert not self.staff.has_perm(perm, object_tag_perm) assert not self.user_both_orgs.has_perm(perm, object_tag_perm) assert not self.user_org2.has_perm(perm, object_tag_perm) assert not self.learner.has_perm(perm, object_tag_perm) @ddt.data( - ("oel_tagging.add_object_tag", "tax_no_org_course1"), - ("oel_tagging.add_object_tag", "tax_no_org_xblock1"), - ("oel_tagging.change_object_tag", "tax_no_org_course1"), - ("oel_tagging.change_object_tag", "tax_no_org_xblock1"), - ("oel_tagging.delete_object_tag", "tax_no_org_xblock1"), - ("oel_tagging.delete_object_tag", "tax_no_org_course1"), + ("oel_tagging.add_objecttag", "tax_no_org_course1"), + ("oel_tagging.add_objecttag", "tax_no_org_xblock1"), + ("oel_tagging.change_objecttag", "tax_no_org_course1"), + ("oel_tagging.change_objecttag", "tax_no_org_xblock1"), + ("oel_tagging.delete_objecttag", "tax_no_org_xblock1"), + ("oel_tagging.delete_objecttag", "tax_no_org_course1"), ) @ddt.unpack def test_object_tag_no_orgs(self, perm, tag_attr): @@ -488,15 +538,14 @@ class TestRulesTaxonomy(TestTaxonomyMixin, TestCase): object_tag = getattr(self, tag_attr) assert self.superuser.has_perm(perm, object_tag) assert self.staff.has_perm(perm, object_tag) - assert not self.user_all_orgs.has_perm(perm, object_tag) assert not self.user_both_orgs.has_perm(perm, object_tag) assert not self.user_org2.has_perm(perm, object_tag) assert not self.learner.has_perm(perm, object_tag) @ddt.data( - "oel_tagging.add_object_tag", - "oel_tagging.change_object_tag", - "oel_tagging.delete_object_tag", + "oel_tagging.add_objecttag", + "oel_tagging.change_objecttag", + "oel_tagging.delete_objecttag", ) def test_change_object_tag_all_orgs(self, perm): """ @@ -506,18 +555,17 @@ class TestRulesTaxonomy(TestTaxonomyMixin, TestCase): for perm_item in self.all_org_perms: assert self.superuser.has_perm(perm, perm_item) assert self.staff.has_perm(perm, perm_item) - assert self.user_all_orgs.has_perm(perm, perm_item) assert self.user_both_orgs.has_perm(perm, perm_item) assert self.user_org2.has_perm(perm, perm_item) == (self.org2.short_name in perm_item.object_id) assert not self.learner.has_perm(perm, perm_item) @ddt.data( - ("oel_tagging.add_object_tag", "tax1_course1"), - ("oel_tagging.add_object_tag", "tax1_xblock1"), - ("oel_tagging.change_object_tag", "tax1_course1"), - ("oel_tagging.change_object_tag", "tax1_xblock1"), - ("oel_tagging.delete_object_tag", "tax1_course1"), - ("oel_tagging.delete_object_tag", "tax1_xblock1"), + ("oel_tagging.add_objecttag", "tax1_course1"), + ("oel_tagging.add_objecttag", "tax1_xblock1"), + ("oel_tagging.change_objecttag", "tax1_course1"), + ("oel_tagging.change_objecttag", "tax1_xblock1"), + ("oel_tagging.delete_objecttag", "tax1_course1"), + ("oel_tagging.delete_objecttag", "tax1_xblock1"), ) @ddt.unpack def test_change_object_tag_org1(self, perm, tag_attr): @@ -525,83 +573,37 @@ class TestRulesTaxonomy(TestTaxonomyMixin, TestCase): perm_item = getattr(self, tag_attr) assert self.superuser.has_perm(perm, perm_item) assert self.staff.has_perm(perm, perm_item) - assert self.user_all_orgs.has_perm(perm, perm_item) assert self.user_both_orgs.has_perm(perm, perm_item) assert not self.user_org2.has_perm(perm, perm_item) assert not self.learner.has_perm(perm, perm_item) @ddt.data( - "all_orgs_course_tag", - "all_orgs_block_tag", - "both_orgs_course_tag", - "both_orgs_block_tag", - "one_org_block_tag", - "disabled_course_tag", + "tax_all_course1", + "tax_all_course2", + "tax_all_xblock1", + "tax_all_xblock2", + "tax_both_course1", + "tax_both_course2", + "tax_both_xblock1", + "tax_both_xblock2", ) def test_view_object_tag(self, tag_attr): """Anyone can view any ObjectTag""" - object_tag = getattr(self, tag_attr) - self._expected_users_have_perm( - "oel_tagging.view_object_tag", - object_tag, - learner_perm=True, - learner_obj=True, - ) + perm = "oel_tagging.view_objecttag" + perm_item = getattr(self, tag_attr) + assert self.superuser.has_perm(perm, perm_item) + assert self.staff.has_perm(perm, perm_item) + assert self.user_both_orgs.has_perm(perm, perm_item) + assert self.user_org2.has_perm(perm, perm_item) == tag_attr.endswith("2") + assert not self.learner.has_perm(perm, perm_item) - -@ddt.ddt -@override_settings(FEATURES={"ENABLE_CREATOR_GROUP": False}) -class TestRulesTaxonomyNoCreatorGroup( - TestRulesTaxonomy -): # pylint: disable=test-inherits-tests - """ - Run the above tests with ENABLE_CREATOR_GROUP unset, to demonstrate that all users have course creator access for - all orgs, and therefore everyone is a Taxonomy Administrator. - - However, if there are no Organizations in the database, then nobody has access to the Tagging models. - """ - - def _expected_users_have_perm( - self, perm, obj, learner_perm=False, learner_obj=False, user_org2=True - ): + def test_view_object_tag_diabled(self): """ - When ENABLE_CREATOR_GROUP is disabled, all users have all permissions. + Noboty can view a ObjectTag from a disable taxonomy """ - super()._expected_users_have_perm( - perm=perm, - obj=obj, - learner_perm=learner_perm, - learner_obj=learner_obj, - user_org2=user_org2, - ) - - # Taxonomy - - @ddt.data( - ("oel_tagging.change_taxonomy", "taxonomy_all_orgs"), - ("oel_tagging.change_taxonomy", "taxonomy_both_orgs"), - ("oel_tagging.change_taxonomy", "taxonomy_disabled"), - ("oel_tagging.change_taxonomy", "taxonomy_one_org"), - ("oel_tagging.change_taxonomy", "taxonomy_no_orgs"), - ("oel_tagging.delete_taxonomy", "taxonomy_all_orgs"), - ("oel_tagging.delete_taxonomy", "taxonomy_both_orgs"), - ("oel_tagging.delete_taxonomy", "taxonomy_disabled"), - ("oel_tagging.delete_taxonomy", "taxonomy_one_org"), - ("oel_tagging.delete_taxonomy", "taxonomy_no_orgs"), - ) - @ddt.unpack - def test_no_orgs_no_perms(self, perm, taxonomy_attr): - """ - Org-level permissions are revoked when there are no orgs. - """ - Organization.objects.all().delete() - taxonomy = getattr(self, taxonomy_attr) - # Superusers & Staff always have access - assert self.superuser.has_perm(perm, taxonomy) - assert self.staff.has_perm(perm, taxonomy) - - # But everyone else's object-level access is removed - assert not self.user_all_orgs.has_perm(perm, taxonomy) - assert not self.user_both_orgs.has_perm(perm, taxonomy) - assert not self.user_org2.has_perm(perm, taxonomy) - assert not self.learner.has_perm(perm, taxonomy) + perm = "oel_tagging.view_objecttag" + assert self.superuser.has_perm(perm, self.disabled_course_tag) + assert not self.staff.has_perm(perm, self.disabled_course_tag) + assert not self.user_both_orgs.has_perm(perm, self.disabled_course_tag) + assert not self.user_org2.has_perm(perm, self.disabled_course_tag) + assert not self.learner.has_perm(perm, self.disabled_course_tag) diff --git a/requirements/constraints.txt b/requirements/constraints.txt index d94b8a19df..9a13b2eccd 100644 --- a/requirements/constraints.txt +++ b/requirements/constraints.txt @@ -121,7 +121,7 @@ libsass==0.10.0 click==8.1.6 # pinning this version to avoid updates while the library is being developed -openedx-learning==0.2.4 +openedx-learning==0.2.5 # lti-consumer-xblock 9.6.2 contains a breaking change that makes # existing custom parameter configurations unusable. diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index c56eb4e07e..86d81da81f 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -785,7 +785,7 @@ openedx-filters==1.6.0 # via # -r requirements/edx/kernel.in # lti-consumer-xblock -openedx-learning==0.2.4 +openedx-learning==0.2.5 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/kernel.in diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index 309dc47829..aade9f40f3 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -1318,7 +1318,7 @@ openedx-filters==1.6.0 # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt # lti-consumer-xblock -openedx-learning==0.2.4 +openedx-learning==0.2.5 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/doc.txt diff --git a/requirements/edx/doc.txt b/requirements/edx/doc.txt index b9680674b7..45b7995928 100644 --- a/requirements/edx/doc.txt +++ b/requirements/edx/doc.txt @@ -925,7 +925,7 @@ openedx-filters==1.6.0 # via # -r requirements/edx/base.txt # lti-consumer-xblock -openedx-learning==0.2.4 +openedx-learning==0.2.5 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/base.txt diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index 604ea2c026..9ad5b53954 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -992,7 +992,7 @@ openedx-filters==1.6.0 # via # -r requirements/edx/base.txt # lti-consumer-xblock -openedx-learning==0.2.4 +openedx-learning==0.2.5 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/base.txt