Skip to content

Commit 155ece7

Browse files
yazzylazyclaude
andcommitted
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_edit/organization_view into edit/view directly, so every consumer of that queryset (not just the channel API) sees the combined permissions. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
1 parent b2379a2 commit 155ece7

4 files changed

Lines changed: 183 additions & 52 deletions

File tree

‎contentcuration/contentcuration/constants/organization_roles.py‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,14 @@
88
(ORGANIZATION_VIEWER, "Viewer"),
99
)
1010

11+
# Organization roles that grant edit/view access to the organization's channels.
12+
ORGANIZATION_CHANNEL_EDIT_ROLES = (ORGANIZATION_ADMIN, ORGANIZATION_EDITOR)
13+
ORGANIZATION_CHANNEL_VIEW_ROLES = (
14+
ORGANIZATION_ADMIN,
15+
ORGANIZATION_EDITOR,
16+
ORGANIZATION_VIEWER,
17+
)
18+
1119
ORGANIZATION_ROLE_STATUS_ACTIVE = "active"
1220
ORGANIZATION_ROLE_STATUS_INACTIVE = "inactive"
1321
ORGANIZATION_ROLE_STATUS_PENDING = "pending"

‎contentcuration/contentcuration/models.py‎

Lines changed: 88 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
from django.db import models
3030
from django.db.models import Count
3131
from django.db.models import Exists
32+
from django.db.models import ExpressionWrapper
3233
from django.db.models import F
3334
from django.db.models import Index
3435
from django.db.models import JSONField
@@ -80,6 +81,8 @@
8081
from contentcuration.constants import user_history
8182
from contentcuration.constants.contentnode import kind_activity_map
8283
from contentcuration.constants.organization_roles import ORGANIZATION_ADMIN
84+
from contentcuration.constants.organization_roles import ORGANIZATION_CHANNEL_EDIT_ROLES
85+
from contentcuration.constants.organization_roles import ORGANIZATION_CHANNEL_VIEW_ROLES
8386
from contentcuration.constants.organization_roles import ORGANIZATION_EDITOR
8487
from contentcuration.constants.organization_roles import organization_role_choices
8588
from contentcuration.constants.organization_roles import ORGANIZATION_ROLE_STATUS_ACTIVE
@@ -1006,29 +1009,63 @@ class PermissionCTE(With):
10061009
tree_id_fields = [
10071010
"channel__{}__tree_id".format(tree_name) for tree_name in CHANNEL_TREES
10081011
]
1012+
channel_tree_id_fields = [
1013+
"{}__tree_id".format(tree_name) for tree_name in CHANNEL_TREES
1014+
]
10091015

1010-
def __init__(self, model, user_id, **kwargs):
1011-
queryset = model.objects.filter(user_id=user_id).annotate(
1012-
tree_id=Unnest(
1013-
ArrayRemove(Array(*self.tree_id_fields), None),
1014-
output_field=models.IntegerField(),
1016+
@classmethod
1017+
def _personal_channels(cls, model, user_id):
1018+
return (
1019+
model.objects.filter(user_id=user_id)
1020+
.annotate(
1021+
tree_id=Unnest(
1022+
ArrayRemove(Array(*cls.tree_id_fields), None),
1023+
output_field=models.IntegerField(),
1024+
)
10151025
)
1026+
.values("user_id", "channel_id", "tree_id")
10161027
)
1017-
super(PermissionCTE, self).__init__(
1018-
queryset=queryset.values("user_id", "channel_id", "tree_id"), **kwargs
1028+
1029+
@classmethod
1030+
def _organization_channels(cls, user_id, roles):
1031+
# Organizations grant channel permissions to their members, on top of
1032+
# whatever personal editor/viewer access a user already has.
1033+
return (
1034+
Channel.objects.filter(
1035+
organization__user_roles__user_id=user_id,
1036+
organization__user_roles__role__in=roles,
1037+
organization__user_roles__status=ORGANIZATION_ROLE_STATUS_ACTIVE,
1038+
)
1039+
.annotate(
1040+
user_id=Value(user_id, output_field=models.IntegerField()),
1041+
channel_id=F("id"),
1042+
tree_id=Unnest(
1043+
ArrayRemove(Array(*cls.channel_tree_id_fields), None),
1044+
output_field=models.IntegerField(),
1045+
),
1046+
)
1047+
.values("user_id", "channel_id", "tree_id")
10191048
)
10201049

10211050
@classmethod
10221051
def editable_channels(cls, user_id):
1023-
return PermissionCTE(
1024-
User.editable_channels.through, user_id, name="editable_channels_cte"
1052+
queryset = cls._personal_channels(
1053+
User.editable_channels.through, user_id
1054+
).union(
1055+
cls._organization_channels(user_id, ORGANIZATION_CHANNEL_EDIT_ROLES),
1056+
all=True,
10251057
)
1058+
return PermissionCTE(queryset, name="editable_channels_cte")
10261059

10271060
@classmethod
10281061
def view_only_channels(cls, user_id):
1029-
return PermissionCTE(
1030-
User.view_only_channels.through, user_id, name="view_only_channels_cte"
1062+
queryset = cls._personal_channels(
1063+
User.view_only_channels.through, user_id
1064+
).union(
1065+
cls._organization_channels(user_id, ORGANIZATION_CHANNEL_VIEW_ROLES),
1066+
all=True,
10311067
)
1068+
return PermissionCTE(queryset, name="view_only_channels_cte")
10321069

10331070
def exists(self, *filters):
10341071
return Exists(self.queryset().filter(*filters).values("user_id"))
@@ -1218,6 +1255,17 @@ class Channel(models.Model):
12181255
def get_editable(cls, user, channel_id):
12191256
return cls.filter_edit_queryset(cls.objects.all(), user).get(id=channel_id)
12201257

1258+
@classmethod
1259+
def _organization_role_exists(cls, user_id, roles):
1260+
return Exists(
1261+
OrganizationRole.objects.filter(
1262+
user_id=user_id,
1263+
organization_id=OuterRef("organization_id"),
1264+
status=ORGANIZATION_ROLE_STATUS_ACTIVE,
1265+
role__in=roles,
1266+
)
1267+
)
1268+
12211269
@classmethod
12221270
def filter_edit_queryset(cls, queryset, user):
12231271
user_id = not user.is_anonymous and user.id
@@ -1231,16 +1279,8 @@ def filter_edit_queryset(cls, queryset, user):
12311279
user_id=user_id, channel_id=OuterRef("id")
12321280
)
12331281
)
1234-
organization_edit = Exists(
1235-
OrganizationRole.objects.filter(
1236-
user_id=user_id,
1237-
organization_id=OuterRef("organization_id"),
1238-
status=ORGANIZATION_ROLE_STATUS_ACTIVE,
1239-
role__in=(
1240-
ORGANIZATION_ADMIN,
1241-
ORGANIZATION_EDITOR,
1242-
),
1243-
)
1282+
organization_edit = cls._organization_role_exists(
1283+
user_id, ORGANIZATION_CHANNEL_EDIT_ROLES
12441284
)
12451285
queryset = queryset.annotate(
12461286
edit=edit,
@@ -1263,13 +1303,10 @@ def filter_delete_queryset(cls, queryset, user):
12631303
user_id=user_id, channel_id=OuterRef("id")
12641304
)
12651305
)
1266-
organization_delete = Exists(
1267-
OrganizationRole.objects.filter(
1268-
user_id=user_id,
1269-
organization_id=OuterRef("organization_id"),
1270-
status=ORGANIZATION_ROLE_STATUS_ACTIVE,
1271-
role=ORGANIZATION_ADMIN,
1272-
)
1306+
# Deleting a channel is restricted to organization admins, unlike
1307+
# editing it, which organization editors may also do.
1308+
organization_delete = cls._organization_role_exists(
1309+
user_id, (ORGANIZATION_ADMIN,)
12731310
)
12741311
queryset = queryset.annotate(
12751312
edit=edit,
@@ -1301,29 +1338,40 @@ def filter_view_queryset(cls, queryset, user):
13011338
)
13021339
)
13031340

1304-
organization_view = Exists(
1305-
OrganizationRole.objects.filter(
1306-
user_id=user_id,
1307-
organization_id=OuterRef("organization_id"),
1308-
status=ORGANIZATION_ROLE_STATUS_ACTIVE,
1309-
role__in=(
1310-
ORGANIZATION_ADMIN,
1311-
ORGANIZATION_EDITOR,
1312-
ORGANIZATION_VIEWER,
1313-
),
1314-
)
1341+
organization_edit = cls._organization_role_exists(
1342+
user_id, ORGANIZATION_CHANNEL_EDIT_ROLES
1343+
)
1344+
organization_view = cls._organization_role_exists(
1345+
user_id, ORGANIZATION_CHANNEL_VIEW_ROLES
13151346
)
13161347
else:
13171348
edit = boolean_val(False)
13181349
view = boolean_val(False)
1350+
organization_edit = boolean_val(False)
13191351
organization_view = boolean_val(False)
13201352

13211353
queryset = queryset.annotate(
13221354
edit=edit,
13231355
view=view,
1356+
organization_edit=organization_edit,
13241357
organization_view=organization_view,
13251358
)
13261359

1360+
# Fold organization grants into edit/view here, in the model layer,
1361+
# so every consumer of this queryset sees combined permissions
1362+
# rather than just the personal editor/viewer grants.
1363+
queryset = queryset.annotate(
1364+
edit=ExpressionWrapper(
1365+
Q(edit=True) | Q(organization_edit=True),
1366+
output_field=models.BooleanField(),
1367+
),
1368+
).annotate(
1369+
view=ExpressionWrapper(
1370+
Q(view=True) | Q(organization_view=True),
1371+
output_field=models.BooleanField(),
1372+
),
1373+
)
1374+
13271375
if user_id and user.is_admin:
13281376
return queryset
13291377

@@ -1338,10 +1386,7 @@ def filter_view_queryset(cls, queryset, user):
13381386
).values_list("channel_id", flat=True)
13391387

13401388
permission_filter = (
1341-
Q(view=True)
1342-
| Q(edit=True)
1343-
| Q(organization_view=True)
1344-
| Q(deleted=False, id__in=pending_channels)
1389+
Q(view=True) | Q(edit=True) | Q(deleted=False, id__in=pending_channels)
13451390
)
13461391

13471392
return queryset.filter(permission_filter | Q(deleted=False, public=True))

‎contentcuration/contentcuration/tests/viewsets/test_organization.py‎

Lines changed: 84 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,14 @@
1616
)
1717
from contentcuration.constants.organization_roles import ORGANIZATION_VIEWER
1818
from contentcuration.models import Channel
19+
from contentcuration.models import ContentNode
1920
from contentcuration.models import Organization
2021
from contentcuration.models import OrganizationRole
2122
from contentcuration.tests import testdata
2223
from contentcuration.tests.base import StudioAPITestCase
24+
from contentcuration.tests.viewsets.base import generate_update_event
25+
from contentcuration.tests.viewsets.base import SyncTestMixin
26+
from contentcuration.viewsets.sync.constants import CONTENTNODE
2327

2428

2529
class OrganizationAPITestCase(StudioAPITestCase):
@@ -602,7 +606,7 @@ def test_last_active_admin_cannot_be_removed(self):
602606
)
603607

604608

605-
class OrganizationChannelPermissionTestCase(OrganizationAPITestCase):
609+
class OrganizationChannelPermissionTestCase(SyncTestMixin, OrganizationAPITestCase):
606610
def setUp(self):
607611
super().setUp()
608612
self.channel = Channel.objects.create(
@@ -702,6 +706,85 @@ def test_pending_role_grants_no_channel_access(self):
702706
self.assertFalse(self._viewable_channel(self.pending_user).exists())
703707
self.assertFalse(self._editable_channel(self.pending_user).exists())
704708

709+
def test_org_editor_can_edit_channel_content_without_m2m_share(self):
710+
node = self.channel.main_tree
711+
self.assertTrue(
712+
ContentNode.filter_edit_queryset(
713+
ContentNode.objects.filter(pk=node.pk), self.editor_user
714+
).exists()
715+
)
716+
717+
def test_org_viewer_cannot_edit_channel_content(self):
718+
node = self.channel.main_tree
719+
self.assertFalse(
720+
ContentNode.filter_edit_queryset(
721+
ContentNode.objects.filter(pk=node.pk), self.viewer_user
722+
).exists()
723+
)
724+
725+
def test_org_editor_channel_api_reports_edit_permission(self):
726+
self.authenticate_as(self.editor_user)
727+
728+
response = self.client.get(
729+
reverse("channel-detail", kwargs={"pk": self.channel.id})
730+
)
731+
732+
self.assertEqual(response.status_code, status.HTTP_200_OK)
733+
self.assertTrue(response.data["edit"])
734+
735+
def test_org_viewer_channel_api_reports_view_not_edit_permission(self):
736+
self.authenticate_as(self.viewer_user)
737+
738+
response = self.client.get(
739+
reverse("channel-detail", kwargs={"pk": self.channel.id})
740+
)
741+
742+
self.assertEqual(response.status_code, status.HTTP_200_OK)
743+
self.assertFalse(response.data["edit"])
744+
self.assertTrue(response.data["view"])
745+
746+
def test_org_editor_channel_api_edit_filter_includes_channel(self):
747+
self.authenticate_as(self.editor_user)
748+
749+
response = self.client.get(reverse("channel-list"), {"edit": True})
750+
751+
self.assertEqual(response.status_code, status.HTTP_200_OK)
752+
self.assertIn(
753+
str(self.channel.id), [result["id"] for result in response.data["results"]]
754+
)
755+
756+
def test_personal_editor_without_org_role_excluded_from_view_only_filter(self):
757+
personal_editor = testdata.user(email="personal-editor@test.com")
758+
self.channel.editors.add(personal_editor)
759+
self.authenticate_as(personal_editor)
760+
761+
response = self.client.get(reverse("channel-list"), {"view": True})
762+
763+
self.assertEqual(response.status_code, status.HTTP_200_OK)
764+
self.assertNotIn(
765+
str(self.channel.id), [result["id"] for result in response.data["results"]]
766+
)
767+
768+
def test_org_editor_can_edit_channel_content_via_sync(self):
769+
self.authenticate_as(self.editor_user)
770+
node = self.channel.main_tree
771+
new_title = "Updated by org editor"
772+
773+
response = self.sync_changes(
774+
[
775+
generate_update_event(
776+
node.id,
777+
CONTENTNODE,
778+
{"title": new_title},
779+
channel_id=self.channel.id,
780+
)
781+
]
782+
)
783+
784+
self.assertEqual(response.status_code, 200, response.content)
785+
node.refresh_from_db()
786+
self.assertEqual(node.title, new_title)
787+
705788

706789
class OrganizationPaginationTestCase(OrganizationAPITestCase):
707790
def test_organization_list_is_paginated(self):

‎contentcuration/contentcuration/viewsets/channel.py‎

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -502,24 +502,19 @@ def perform_destroy(self, instance):
502502
instance.save(update_fields=["deleted"], actor_id=self.request.user.id)
503503

504504
def get_queryset(self):
505+
# edit/view are already annotated here with organization grants folded
506+
# in by Channel.filter_view_queryset.
505507
queryset = super(ChannelViewSet, self).get_queryset()
506-
user_id = not self.request.user.is_anonymous and self.request.user.id
507-
user_queryset = User.objects.filter(id=user_id)
508508
# Add the last modified node modified value as the channel last modified
509509
channel_main_tree_nodes = ContentNode.objects.filter(
510510
tree_id=OuterRef("main_tree__tree_id")
511511
)
512-
queryset = queryset.annotate(
512+
return queryset.annotate(
513513
modified=Subquery(
514514
channel_main_tree_nodes.values("modified").order_by("-modified")[:1]
515515
)
516516
)
517517

518-
return queryset.annotate(
519-
edit=Exists(user_queryset.filter(editable_channels=OuterRef("id"))),
520-
view=Exists(user_queryset.filter(view_only_channels=OuterRef("id"))),
521-
)
522-
523518
def _annotate_draft_token(self, queryset):
524519
draft_token_subquery = Subquery(
525520
ChannelVersion.objects.filter(channel=OuterRef("id"), version=None).values(

0 commit comments

Comments
 (0)