Skip to content

Commit ac5018b

Browse files
Team scoping: close every unenumerated mutation path; API + form hardening
Review findings: scoping was enforced by enumerating dangerous views, and Wagtail has more object-level endpoints than were enumerated — anything not on the list defaulted to open. - Pages: register before_unpublish/copy/move_page and before_bulk_action (bulk delete/publish/unpublish fire ONLY the bulk hook) - Galleries: scoped Copy view (stock CopyView prefills via bare get_object_or_404 with an unrestricted map_group chooser) and the generic before_unpublish hook (UnpublishView checks only model-level publish) - Datastore: History/Usage views get the same 404 guard as Inspect - PlacePageForm no longer silently drops other teams' maps (or their curated order) from shared place pages - content/galleries APIs: negative limit no longer 500s (shared clamped pagination in core/api.py); available_languages deduped - Trim: TeamScopedViewSetMixin + instance_in_scope collapse the per-app scoping copies; 47-line Wagtail base.html copy replaced with a recursive same-name extends; dead Team.slug field removed; stale e2e endpoint removed - Regression tests for each closed path (212 cms tests green) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 46ce745 commit ac5018b

16 files changed

Lines changed: 310 additions & 175 deletions

File tree

app/e2e/utils/network-helpers.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ export const API_ENDPOINTS = {
1818
patchUnshatter: '**/api/unshatter/*',
1919
root: '**/', // Returns {"message":"Hello World"}
2020
districtrMaps: '**/api/districtr_maps',
21-
cmsContent: '**/api/cms/*',
2221
} as const;
2322

2423
function globToRegExp(glob: string): RegExp {
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
# Generated by Django 5.2.15 on 2026-08-04 21:16
2+
3+
from django.db import migrations
4+
5+
6+
class Migration(migrations.Migration):
7+
8+
dependencies = [
9+
('authapi', '0005_grant_admin_team_permissions'),
10+
]
11+
12+
operations = [
13+
migrations.RemoveField(
14+
model_name='team',
15+
name='slug',
16+
),
17+
]

cms/authapi/models.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,6 @@ class Team(ClusterableModel):
6767
"""
6868

6969
name = models.CharField(max_length=255, unique=True)
70-
slug = models.SlugField(max_length=255, unique=True)
7170

7271
class Meta:
7372
ordering = ["name"]

cms/authapi/teams.py

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,9 @@
1717
this module only answers "is this user scoped, and to which group slugs".
1818
"""
1919

20+
from functools import cached_property
21+
22+
from django.http import Http404
2023
from wagtail.permission_policies.base import ModelPermissionPolicy
2124

2225
from authapi.models import TeamMapGroup, TeamMembership
@@ -61,6 +64,14 @@ def districtr_map_slugs_for_user(user) -> set[str]:
6164
)
6265

6366

67+
def instance_in_scope(user, model, group_filter_field, pk) -> bool:
68+
"""False exactly when a team-scoped ``user`` may not act on ``model`` row
69+
``pk``. Unscoped users (admins, superusers, team-less) always pass."""
70+
if not user_is_team_scoped(user):
71+
return True
72+
return scoped_queryset(model, group_filter_field, user).filter(pk=pk).exists()
73+
74+
6475
def scoped_queryset(model, group_filter_field, user):
6576
"""``model`` rows whose MapGroup one of ``user``'s teams owns.
6677
@@ -137,3 +148,40 @@ def user_has_permission_for_instance(self, user, action, instance):
137148
.exists()
138149
)
139150
return super().user_has_permission_for_instance(user, action, instance)
151+
152+
153+
class TeamScopedViewSetMixin:
154+
"""SnippetViewSet mixin: index queryset and permission policy scoped to the
155+
user's teams. Set ``group_filter_field``; override
156+
``permission_policy_class`` for view-grant behaviour."""
157+
158+
group_filter_field: str
159+
permission_policy_class = TeamScopedModelPermissionPolicy
160+
161+
def get_queryset(self, request):
162+
if user_is_team_scoped(request.user):
163+
return scoped_queryset(self.model, self.group_filter_field, request.user)
164+
return None
165+
166+
@cached_property
167+
def permission_policy(self):
168+
return self.permission_policy_class(
169+
self.model, group_filter_field=self.group_filter_field
170+
)
171+
172+
173+
class TeamScopedGetObjectMixin:
174+
"""For snippet object views that fetch straight from the model with no
175+
instance permission check (Inspect/History/Usage/Copy): 404 when a
176+
team-scoped member addresses an out-of-scope object by URL. Set
177+
``group_filter_field`` on the view subclass."""
178+
179+
group_filter_field: str
180+
181+
def get_object(self, queryset=None):
182+
obj = super().get_object(queryset)
183+
if not instance_in_scope(
184+
self.request.user, self.model, self.group_filter_field, obj.pk
185+
):
186+
raise Http404
187+
return obj

cms/authapi/test_teams.py

Lines changed: 84 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -45,8 +45,8 @@ def make_user(group_name, email):
4545
return user
4646

4747

48-
def make_team(name, slug, *, members=(), group_slugs=()):
49-
team = Team.objects.create(name=name, slug=slug)
48+
def make_team(name, *, members=(), group_slugs=()):
49+
team = Team.objects.create(name=name)
5050
for user in members:
5151
TeamMembership.objects.create(team=team, user=user)
5252
for group_slug in group_slugs:
@@ -104,27 +104,27 @@ def test_superuser_never_scoped(self):
104104
root = get_user_model().objects.create_superuser(
105105
username="root@d.org", email="root@d.org", password=PASSWORD
106106
)
107-
make_team("Team", "team", members=[root], group_slugs=["ga"])
107+
make_team("Team", members=[root], group_slugs=["ga"])
108108
self.assertFalse(user_is_team_scoped(root))
109109

110110
def test_admin_group_never_scoped(self):
111111
admin = make_user("admin", "admin@d.org")
112-
make_team("Team", "team", members=[admin], group_slugs=["ga"])
112+
make_team("Team", members=[admin], group_slugs=["ga"])
113113
self.assertFalse(user_is_team_scoped(admin))
114114

115115
def test_editor_without_team_not_scoped(self):
116116
self.assertFalse(user_is_team_scoped(make_user("editor", "e@d.org")))
117117

118118
def test_editor_with_team_is_scoped(self):
119119
editor = make_user("editor", "e@d.org")
120-
make_team("Team A", "team-a", members=[editor], group_slugs=["ga", "gb"])
120+
make_team("Team A", members=[editor], group_slugs=["ga", "gb"])
121121
self.assertTrue(user_is_team_scoped(editor))
122122
self.assertEqual(map_group_slugs_for_user(editor), {"ga", "gb"})
123123

124124
def test_slugs_union_across_teams(self):
125125
editor = make_user("editor", "e@d.org")
126-
make_team("T1", "t1", members=[editor], group_slugs=["ga"])
127-
make_team("T2", "t2", members=[editor], group_slugs=["gb", "gc"])
126+
make_team("T1", members=[editor], group_slugs=["ga"])
127+
make_team("T2", members=[editor], group_slugs=["gb", "gc"])
128128
self.assertEqual(map_group_slugs_for_user(editor), {"ga", "gb", "gc"})
129129

130130

@@ -136,7 +136,7 @@ def setUpTestData(cls):
136136
Gallery, group_filter_field="map_group_id"
137137
)
138138
cls.member = make_user("editor", "member@d.org")
139-
make_team("Team A", "team-a", members=[cls.member], group_slugs=["ga"])
139+
make_team("Team A", members=[cls.member], group_slugs=["ga"])
140140
cls.mine = make_gallery("mine", group_slug="ga")
141141
cls.theirs = make_gallery("theirs", group_slug="gb")
142142

@@ -176,7 +176,7 @@ class GalleryAdminScopingViewTests(TestCase):
176176
def setUpTestData(cls):
177177
ensure_map_group_table()
178178
cls.member = make_user("editor", "member@d.org")
179-
make_team("Team A", "team-a", members=[cls.member], group_slugs=["ga"])
179+
make_team("Team A", members=[cls.member], group_slugs=["ga"])
180180
cls.mine = make_gallery("scoped-visible", group_slug="ga")
181181
cls.theirs = make_gallery("scoped-hidden", group_slug="gb")
182182

@@ -208,6 +208,29 @@ def test_create_view_restricts_map_group_to_team(self):
208208
self.assertEqual(set(field.queryset.values_list("slug", flat=True)), {"ga"})
209209
self.assertTrue(field.required)
210210

211+
def test_unpublish_out_of_scope_denied(self):
212+
# UnpublishView checks only the model-level publish permission; the
213+
# generic before_unpublish hook is the instance-level gate.
214+
url = reverse(
215+
"wagtailsnippets_galleries_gallery:unpublish", args=[self.theirs.pk]
216+
)
217+
self.client.post(url)
218+
self.theirs.refresh_from_db()
219+
self.assertTrue(self.theirs.live)
220+
221+
def test_copy_out_of_scope_404(self):
222+
# The stock CopyView prefills from a bare get_object_or_404 — the
223+
# scoped copy view must 404 out-of-scope sources.
224+
url = reverse("wagtailsnippets_galleries_gallery:copy", args=[self.theirs.pk])
225+
self.assertEqual(self.client.get(url).status_code, 404)
226+
227+
def test_copy_in_scope_restricts_map_group(self):
228+
url = reverse("wagtailsnippets_galleries_gallery:copy", args=[self.mine.pk])
229+
response = self.client.get(url)
230+
self.assertEqual(response.status_code, 200)
231+
field = response.context["form"].fields["map_group"]
232+
self.assertEqual(set(field.queryset.values_list("slug", flat=True)), {"ga"})
233+
211234

212235
class MapModuleScopingTests(TestCase):
213236
"""DistrictrMap modules: members get scoped, view-only access (admins keep
@@ -233,7 +256,7 @@ def setUpTestData(cls):
233256
DistrictrMapsToGroups.objects.create(districtrmap=cls.map_a, group=group_a)
234257
DistrictrMapsToGroups.objects.create(districtrmap=cls.map_b, group=group_b)
235258
cls.member = make_user("editor", "mm-member@d.org")
236-
make_team("Map Team A", "mm-team-a", members=[cls.member], group_slugs=["ga"])
259+
make_team("Map Team A", members=[cls.member], group_slugs=["ga"])
237260

238261
def test_member_view_instances_scoped(self):
239262
qs = self.policy.instances_user_has_permission_for(self.member, "view")
@@ -320,7 +343,7 @@ def setUpTestData(cls):
320343
cls.places_index.add_child(instance=cls.place_out)
321344

322345
cls.member = make_user("editor", "tp-member@d.org")
323-
make_team("Tag Team A", "tp-team-a", members=[cls.member], group_slugs=["ga"])
346+
make_team("Tag Team A", members=[cls.member], group_slugs=["ga"])
324347
cls.admin = make_user("admin", "tp-admin@d.org")
325348

326349
def _request(self, user):
@@ -382,6 +405,31 @@ def test_admin_never_blocked(self):
382405
_is_out_of_scope_page(self._request(self.admin), self.place_out)
383406
)
384407

408+
def test_all_page_mutation_hooks_registered(self):
409+
# Wagtail's unpublish/copy/move/bulk paths never fire the edit/delete
410+
# hooks — each needs its own registration or it defaults to open.
411+
from wagtail import hooks as wagtail_hooks
412+
413+
for name in (
414+
"before_edit_page",
415+
"before_delete_page",
416+
"before_unpublish_page",
417+
"before_copy_page",
418+
"before_move_page",
419+
"before_bulk_action",
420+
):
421+
modules = [fn.__module__ for fn in wagtail_hooks.get_hooks(name)]
422+
self.assertIn("content.wagtail_hooks", modules, name)
423+
424+
def test_member_cannot_unpublish_out_of_scope_page_via_admin(self):
425+
self.client.force_login(self.member)
426+
response = self.client.post(
427+
reverse("wagtailadmin_pages:unpublish", args=[self.tag_out.id])
428+
)
429+
self.assertNotEqual(response.status_code, 200)
430+
self.tag_out.refresh_from_db()
431+
self.assertTrue(self.tag_out.live)
432+
385433

386434
class ContentPageFormScopingTests(TestCase):
387435
"""The team-aware page forms only offer a member their own teams' map slugs
@@ -402,7 +450,7 @@ def setUpTestData(cls):
402450
)
403451
DistrictrMapsToGroups.objects.create(districtrmap=dmap, group=group)
404452
cls.member = make_user("editor", "form-member@d.org")
405-
make_team("Form Team", "form-team", members=[cls.member], group_slugs=["ga"])
453+
make_team("Form Team", members=[cls.member], group_slugs=["ga"])
406454
cls.admin = make_user("admin", "form-admin@d.org")
407455

408456
@staticmethod
@@ -451,3 +499,27 @@ def test_admin_form_unrestricted(self):
451499
# Admin keeps the plain free-text CharField (no scoped choices).
452500
form = self._bound(TagPage, user=self.admin)
453501
self.assertFalse(hasattr(form.fields["districtr_map_slug"], "choices"))
502+
503+
def test_placepage_form_preserves_other_teams_slugs_and_order(self):
504+
# A shared PlacePage carries another team's map; saving must keep it,
505+
# in its original position, even though the member can't select it.
506+
page = PlacePage(
507+
title="P", slug="p", districtr_map_slugs=["tx_other", "chi_wards"]
508+
)
509+
form = self._form_class(PlacePage)(
510+
data={
511+
"title": "P",
512+
"slug": "p",
513+
"districtr_map_slugs": ["chi_wards"],
514+
"body-count": "0",
515+
},
516+
instance=page,
517+
for_user=self.member,
518+
)
519+
# full_clean rather than is_valid: the bare instance lacks Wagtail's
520+
# tree fields (path/depth/...), which aren't what's under test here.
521+
form.full_clean()
522+
self.assertNotIn("districtr_map_slugs", form.errors)
523+
self.assertEqual(
524+
form.cleaned_data["districtr_map_slugs"], ["tx_other", "chi_wards"]
525+
)

cms/authapi/wagtail_hooks.py

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -113,13 +113,12 @@ class TeamViewSet(SnippetViewSet):
113113
menu_label = "Teams"
114114
menu_order = 260 # after "Review tag scopes" (250)
115115
add_to_admin_menu = True
116-
list_display = ["name", "slug"]
117-
search_fields = ["name", "slug"]
116+
list_display = ["name"]
117+
search_fields = ["name"]
118118
list_per_page = 50
119119

120120
panels = [
121121
FieldPanel("name"),
122-
FieldPanel("slug"),
123122
InlinePanel(
124123
"memberships",
125124
heading="Members",

cms/content/api.py

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -29,15 +29,14 @@
2929
from django.views.decorators.http import require_GET
3030

3131
from content.models import PlacePage, TagPage
32-
from core.api import _json
32+
from core.api import MAX_PAGE_SIZE, _json, pagination
3333

3434
CONTENT_TYPE_PAGES = {
3535
"tags": TagPage,
3636
"places": PlacePage,
3737
}
3838

3939
DEFAULT_LANGUAGE = "en"
40-
MAX_PAGE_SIZE = 100
4140

4241
# Stable ordering for available_languages / list endpoints.
4342
_LANGUAGE_ORDER = {
@@ -79,7 +78,7 @@ def content_detail(request, content_type, slug):
7978
# than loading every language's StreamField body just to pick one.
8079
live_pages = model.objects.live().filter(slug=slug)
8180
available_languages = sorted(
82-
live_pages.values_list("locale__language_code", flat=True),
81+
live_pages.values_list("locale__language_code", flat=True).distinct(),
8382
key=_language_sort_key,
8483
)
8584

@@ -124,8 +123,7 @@ def content_list(request, content_type):
124123
return _json({"detail": f"Unknown content type '{content_type}'"}, status=404)
125124

126125
try:
127-
offset = max(int(request.GET.get("offset", 0)), 0)
128-
limit = min(int(request.GET.get("limit", MAX_PAGE_SIZE)), MAX_PAGE_SIZE)
126+
offset, limit = pagination(request)
129127
except ValueError:
130128
return _json({"detail": "offset and limit must be integers"}, status=400)
131129

0 commit comments

Comments
 (0)