Fix organization channel edit/view permissions not reaching content and the channel API

PermissionCTE only read the personal editor/viewer m2m tables, so
organization editors/viewers could not edit or view content nodes,
assessment items, or files inside org-owned channels. Channel.filter_view_queryset
also only exposed the personal-only edit/view checks, so the channel API
reported edit: false for organization editors even though the channel
itself was already correctly org-aware.

PermissionCTE.editable_channels/view_only_channels now union the personal
m2m grants with channels reachable via an active OrganizationRole.
Channel.filter_view_queryset now folds organization grants directly into
edit/view, so every consumer of that queryset sees combined permissions.
Both layers treat organization viewing as viewer-only: an org editor/admin
is already admitted through the edit check wherever permissions are
checked, so including them in the view role set too would be redundant.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

Author: yazzylazy

contentcuration/contentcuration/constants/organization_roles.py

@@ -8,6 +8,9 @@
     (ORGANIZATION_VIEWER, "Viewer"),
 )
 
+# Viewing is ORGANIZATION_VIEWER alone; admins/editors are already admitted via the edit check.
+ORGANIZATION_CHANNEL_EDIT_ROLES = (ORGANIZATION_ADMIN, ORGANIZATION_EDITOR)
+
 ORGANIZATION_ROLE_STATUS_ACTIVE = "active"
 ORGANIZATION_ROLE_STATUS_INACTIVE = "inactive"
 ORGANIZATION_ROLE_STATUS_PENDING = "pending"

contentcuration/contentcuration/models.py

@@ -29,6 +29,7 @@
 from django.db import models
 from django.db.models import Count
 from django.db.models import Exists
+from django.db.models import ExpressionWrapper
 from django.db.models import F
 from django.db.models import Index
 from django.db.models import JSONField
@@ -80,6 +81,7 @@
 from contentcuration.constants import user_history
 from contentcuration.constants.contentnode import kind_activity_map
 from contentcuration.constants.organization_roles import ORGANIZATION_ADMIN
+from contentcuration.constants.organization_roles import ORGANIZATION_CHANNEL_EDIT_ROLES
 from contentcuration.constants.organization_roles import ORGANIZATION_EDITOR
 from contentcuration.constants.organization_roles import organization_role_choices
 from contentcuration.constants.organization_roles import ORGANIZATION_ROLE_STATUS_ACTIVE
@@ -1006,29 +1008,63 @@ class PermissionCTE(With):
     tree_id_fields = [
         "channel__{}__tree_id".format(tree_name) for tree_name in CHANNEL_TREES
     ]
+    channel_tree_id_fields = [
+        "{}__tree_id".format(tree_name) for tree_name in CHANNEL_TREES
+    ]
 
-    def __init__(self, model, user_id, **kwargs):
-        queryset = model.objects.filter(user_id=user_id).annotate(
-            tree_id=Unnest(
-                ArrayRemove(Array(*self.tree_id_fields), None),
-                output_field=models.IntegerField(),
+    @classmethod
+    def _personal_channels(cls, model, user_id):
+        return (
+            model.objects.filter(user_id=user_id)
+            .annotate(
+                tree_id=Unnest(
+                    ArrayRemove(Array(*cls.tree_id_fields), None),
+                    output_field=models.IntegerField(),
+                )
             )
+            .values("user_id", "channel_id", "tree_id")
         )
-        super(PermissionCTE, self).__init__(
-            queryset=queryset.values("user_id", "channel_id", "tree_id"), **kwargs
+
+    @classmethod
+    def _organization_channels(cls, user_id, roles):
+        # Organizations grant channel permissions to their members, on top of
+        # whatever personal editor/viewer access a user already has.
+        return (
+            Channel.objects.filter(
+                organization__user_roles__user_id=user_id,
+                organization__user_roles__role__in=roles,
+                organization__user_roles__status=ORGANIZATION_ROLE_STATUS_ACTIVE,
+            )
+            .annotate(
+                user_id=Value(user_id, output_field=models.IntegerField()),
+                channel_id=F("id"),
+                tree_id=Unnest(
+                    ArrayRemove(Array(*cls.channel_tree_id_fields), None),
+                    output_field=models.IntegerField(),
+                ),
+            )
+            .values("user_id", "channel_id", "tree_id")
         )
 
     @classmethod
     def editable_channels(cls, user_id):
-        return PermissionCTE(
-            User.editable_channels.through, user_id, name="editable_channels_cte"
+        queryset = cls._personal_channels(
+            User.editable_channels.through, user_id
+        ).union(
+            cls._organization_channels(user_id, ORGANIZATION_CHANNEL_EDIT_ROLES),
+            all=True,
         )
+        return PermissionCTE(queryset, name="editable_channels_cte")
 
     @classmethod
     def view_only_channels(cls, user_id):
-        return PermissionCTE(
-            User.view_only_channels.through, user_id, name="view_only_channels_cte"
+        queryset = cls._personal_channels(
+            User.view_only_channels.through, user_id
+        ).union(
+            cls._organization_channels(user_id, (ORGANIZATION_VIEWER,)),
+            all=True,
         )
+        return PermissionCTE(queryset, name="view_only_channels_cte")
 
     def exists(self, *filters):
         return Exists(self.queryset().filter(*filters).values("user_id"))
@@ -1218,6 +1254,17 @@ class Channel(models.Model):
     def get_editable(cls, user, channel_id):
         return cls.filter_edit_queryset(cls.objects.all(), user).get(id=channel_id)
 
+    @classmethod
+    def _organization_role_exists(cls, user_id, roles):
+        return Exists(
+            OrganizationRole.objects.filter(
+                user_id=user_id,
+                organization_id=OuterRef("organization_id"),
+                status=ORGANIZATION_ROLE_STATUS_ACTIVE,
+                role__in=roles,
+            )
+        )
+
     @classmethod
     def filter_edit_queryset(cls, queryset, user):
         user_id = not user.is_anonymous and user.id
@@ -1231,16 +1278,8 @@ def filter_edit_queryset(cls, queryset, user):
                 user_id=user_id, channel_id=OuterRef("id")
             )
         )
-        organization_edit = Exists(
-            OrganizationRole.objects.filter(
-                user_id=user_id,
-                organization_id=OuterRef("organization_id"),
-                status=ORGANIZATION_ROLE_STATUS_ACTIVE,
-                role__in=(
-                    ORGANIZATION_ADMIN,
-                    ORGANIZATION_EDITOR,
-                ),
-            )
+        organization_edit = cls._organization_role_exists(
+            user_id, ORGANIZATION_CHANNEL_EDIT_ROLES
         )
         queryset = queryset.annotate(
             edit=edit,
@@ -1263,13 +1302,10 @@ def filter_delete_queryset(cls, queryset, user):
                 user_id=user_id, channel_id=OuterRef("id")
             )
         )
-        organization_delete = Exists(
-            OrganizationRole.objects.filter(
-                user_id=user_id,
-                organization_id=OuterRef("organization_id"),
-                status=ORGANIZATION_ROLE_STATUS_ACTIVE,
-                role=ORGANIZATION_ADMIN,
-            )
+        # Deleting a channel is restricted to organization admins, unlike
+        # editing it, which organization editors may also do.
+        organization_delete = cls._organization_role_exists(
+            user_id, (ORGANIZATION_ADMIN,)
         )
         queryset = queryset.annotate(
             edit=edit,
@@ -1289,40 +1325,26 @@ def filter_view_queryset(cls, queryset, user):
         if user_id:
             filters = dict(user_id=user_id, channel_id=OuterRef("id"))
 
-            edit = Exists(
-                User.editable_channels.through.objects.filter(**filters).values(
-                    "user_id"
-                )
+            # Fold organization grants into edit/view (viewer-only for view; an org editor/admin is admitted via the edit check instead).
+            edit = ExpressionWrapper(
+                Q(Exists(User.editable_channels.through.objects.filter(**filters)))
+                | Q(
+                    cls._organization_role_exists(
+                        user_id, ORGANIZATION_CHANNEL_EDIT_ROLES
+                    )
+                ),
+                output_field=models.BooleanField(),
             )
-
-            view = Exists(
-                User.view_only_channels.through.objects.filter(**filters).values(
-                    "user_id"
-                )
-            )
-
-            organization_view = Exists(
-                OrganizationRole.objects.filter(
-                    user_id=user_id,
-                    organization_id=OuterRef("organization_id"),
-                    status=ORGANIZATION_ROLE_STATUS_ACTIVE,
-                    role__in=(
-                        ORGANIZATION_ADMIN,
-                        ORGANIZATION_EDITOR,
-                        ORGANIZATION_VIEWER,
-                    ),
-                )
+            view = ExpressionWrapper(
+                Q(Exists(User.view_only_channels.through.objects.filter(**filters)))
+                | Q(cls._organization_role_exists(user_id, (ORGANIZATION_VIEWER,))),
+                output_field=models.BooleanField(),
             )
         else:
             edit = boolean_val(False)
             view = boolean_val(False)
-            organization_view = boolean_val(False)
 
-        queryset = queryset.annotate(
-            edit=edit,
-            view=view,
-            organization_view=organization_view,
-        )
+        queryset = queryset.annotate(edit=edit, view=view)
 
         if user_id and user.is_admin:
             return queryset
@@ -1338,10 +1360,7 @@ def filter_view_queryset(cls, queryset, user):
             ).values_list("channel_id", flat=True)
 
             permission_filter = (
-                Q(view=True)
-                | Q(edit=True)
-                | Q(organization_view=True)
-                | Q(deleted=False, id__in=pending_channels)
+                Q(view=True) | Q(edit=True) | Q(deleted=False, id__in=pending_channels)
             )
 
         return queryset.filter(permission_filter | Q(deleted=False, public=True))

contentcuration/contentcuration/tests/viewsets/test_organization.py

@@ -16,10 +16,14 @@
 )
 from contentcuration.constants.organization_roles import ORGANIZATION_VIEWER
 from contentcuration.models import Channel
+from contentcuration.models import ContentNode
 from contentcuration.models import Organization
 from contentcuration.models import OrganizationRole
 from contentcuration.tests import testdata
 from contentcuration.tests.base import StudioAPITestCase
+from contentcuration.tests.viewsets.base import generate_update_event
+from contentcuration.tests.viewsets.base import SyncTestMixin
+from contentcuration.viewsets.sync.constants import CONTENTNODE
 
 
 class OrganizationAPITestCase(StudioAPITestCase):
@@ -602,7 +606,7 @@ def test_last_active_admin_cannot_be_removed(self):
         )
 
 
-class OrganizationChannelPermissionTestCase(OrganizationAPITestCase):
+class OrganizationChannelPermissionTestCase(SyncTestMixin, OrganizationAPITestCase):
     def setUp(self):
         super().setUp()
         self.channel = Channel.objects.create(
@@ -702,6 +706,71 @@ def test_pending_role_grants_no_channel_access(self):
         self.assertFalse(self._viewable_channel(self.pending_user).exists())
         self.assertFalse(self._editable_channel(self.pending_user).exists())
 
+    def test_org_role_channel_content_permissions(self):
+        node = self.channel.main_tree
+        self.assertTrue(
+            ContentNode.filter_edit_queryset(
+                ContentNode.objects.filter(pk=node.pk), self.editor_user
+            ).exists()
+        )
+        self.assertFalse(
+            ContentNode.filter_edit_queryset(
+                ContentNode.objects.filter(pk=node.pk), self.viewer_user
+            ).exists()
+        )
+        self.assertTrue(
+            ContentNode.filter_view_queryset(
+                ContentNode.objects.filter(pk=node.pk), self.viewer_user
+            ).exists()
+        )
+
+    def test_channel_detail_reports_edit_view_by_org_role(self):
+        detail_url = reverse("channel-detail", kwargs={"pk": self.channel.id})
+
+        self.authenticate_as(self.editor_user)
+        response = self.client.get(detail_url)
+        self.assertEqual(response.status_code, status.HTTP_200_OK)
+        self.assertTrue(response.data["edit"])
+
+        self.authenticate_as(self.viewer_user)
+        response = self.client.get(detail_url)
+        self.assertEqual(response.status_code, status.HTTP_200_OK)
+        self.assertFalse(response.data["edit"])
+        self.assertTrue(response.data["view"])
+
+    def test_org_editor_channel_list_filters(self):
+        self.authenticate_as(self.editor_user)
+
+        response = self.client.get(reverse("channel-list"), {"view": True})
+        self.assertEqual(response.status_code, status.HTTP_200_OK)
+        self.assertNotIn(
+            str(self.channel.id), [result["id"] for result in response.data]
+        )
+
+        response = self.client.get(reverse("channel-list"), {"edit": True})
+        self.assertEqual(response.status_code, status.HTTP_200_OK)
+        self.assertIn(str(self.channel.id), [result["id"] for result in response.data])
+
+    def test_org_editor_can_edit_channel_content_via_sync(self):
+        self.authenticate_as(self.editor_user)
+        node = self.channel.main_tree
+        new_title = "Updated by org editor"
+
+        response = self.sync_changes(
+            [
+                generate_update_event(
+                    node.id,
+                    CONTENTNODE,
+                    {"title": new_title},
+                    channel_id=self.channel.id,
+                )
+            ]
+        )
+
+        self.assertEqual(response.status_code, 200, response.content)
+        node.refresh_from_db()
+        self.assertEqual(node.title, new_title)
+
 
 class OrganizationPaginationTestCase(OrganizationAPITestCase):
     def test_organization_list_is_paginated(self):

contentcuration/contentcuration/viewsets/channel.py

@@ -502,24 +502,19 @@ def perform_destroy(self, instance):
         instance.save(update_fields=["deleted"], actor_id=self.request.user.id)
 
     def get_queryset(self):
+        # edit/view are already annotated here with organization grants folded
+        # in by Channel.filter_view_queryset.
         queryset = super(ChannelViewSet, self).get_queryset()
-        user_id = not self.request.user.is_anonymous and self.request.user.id
-        user_queryset = User.objects.filter(id=user_id)
         # Add the last modified node modified value as the channel last modified
         channel_main_tree_nodes = ContentNode.objects.filter(
             tree_id=OuterRef("main_tree__tree_id")
         )
-        queryset = queryset.annotate(
+        return queryset.annotate(
             modified=Subquery(
                 channel_main_tree_nodes.values("modified").order_by("-modified")[:1]
             )
         )
 
-        return queryset.annotate(
-            edit=Exists(user_queryset.filter(editable_channels=OuterRef("id"))),
-            view=Exists(user_queryset.filter(view_only_channels=OuterRef("id"))),
-        )
-
     def _annotate_draft_token(self, queryset):
         draft_token_subquery = Subquery(
             ChannelVersion.objects.filter(channel=OuterRef("id"), version=None).values(