Skip to content

Commit b8cb5c0

Browse files
CMS review fixes: team-scope the data tools, safer commands, pinned deps
Review feedback on #714: - Overlay uploads: a team-scoped super partner's map choices (and the POST validation queryset) narrow to their teams' maps; previously any map in the database could be targeted. - Thumbnail regeneration 404s for out-of-scope maps instead of scheduling backend work with the service token. - custom_style must be a JSON object: valid-but-non-object JSON would fail OverlayPublic response validation and 500 affected maps. - migrate_tiptap wraps the standalone write pass in one transaction so the fidelity-check failure rolls back every page written this run (matching the atomic-migration behaviour); the JSON report still writes on failure. - provision_users resends the setup email for existing accounts that still have unusable passwords, so a failed send is recoverable by re-running the command. - requirements.lock (pip freeze of the built image) used as a pip constraints file: this service signs every auth token post-cutover, so transitive crypto/JWT versions must not drift between builds. - HSTS in production settings (the ALB already redirects HTTP→HTTPS). - review:review-all minted CMS-side: admins get the new explicit tag-scoping bypass; partners still never do (see #712). - Compose no longer sends the removed 'visible' flag (see #713). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent c39ad93 commit b8cb5c0

14 files changed

Lines changed: 356 additions & 98 deletions

File tree

cms/.env.docker

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
# Districtr CMS (Wagtail) — local dev via docker-compose
2+
DJANGO_SETTINGS_MODULE=config.settings.dev
3+
DJANGO_SECRET_KEY=django-insecure-dev-only-key
4+
5+
# Postgres (same database as the FastAPI backend; Django tables live in the
6+
# `admin` schema via search_path)
7+
POSTGRES_USER=postgres
8+
POSTGRES_PASSWORD=postgres
9+
POSTGRES_DB=districtr
10+
POSTGRES_SERVER=db
11+
POSTGRES_PORT=5432
12+
13+
WAGTAILADMIN_BASE_URL=http://localhost:8001
14+
DEFAULT_FROM_EMAIL=noreply@districtr.org
15+
16+
# FastAPI backend (GeoPackage import + thumbnail triggers; compose-internal)
17+
BACKEND_API_URL=http://backend:8000
18+
19+
# Object storage for GeoPackage and overlay uploads — mirrors the backend's
20+
# env contract (backend/app/core/config.py). Set ACCOUNT_ID for Cloudflare
21+
# R2, or AWS_S3_ENDPOINT for a custom S3 endpoint; leave both unset for
22+
# plain AWS S3.
23+
#AWS_ACCESS_KEY_ID=
24+
#AWS_SECRET_ACCESS_KEY=
25+
#R2_BUCKET_NAME=
26+
#ACCOUNT_ID=
27+
#AWS_S3_ENDPOINT=
28+
29+
# Public base URL stored as Overlay.source for uploaded overlays — the CDN
30+
# fronting the bucket (https://tilesets1.cdn.districtr.org in prod). When
31+
# unset, the raw s3://bucket/key path is stored instead.
32+
#OVERLAY_PUBLIC_URL_BASE=

cms/Dockerfile

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,8 @@ ENV PYTHONUNBUFFERED=1 \
77

88
WORKDIR /districtr-cms
99

10-
COPY requirements.txt .
11-
RUN pip install --no-cache-dir -r requirements.txt
10+
COPY requirements.txt requirements.lock ./
11+
RUN pip install --no-cache-dir -r requirements.txt -c requirements.lock
1212

1313
COPY . .
1414

cms/Dockerfile.dev

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,8 @@ ENV PYTHONUNBUFFERED=1 \
66

77
WORKDIR /districtr-cms
88

9-
COPY requirements.txt .
10-
RUN pip install --no-cache-dir -r requirements.txt
9+
COPY requirements.txt requirements.lock ./
10+
RUN pip install --no-cache-dir -r requirements.txt -c requirements.lock
1111

1212
COPY . .
1313

cms/authapi/management/commands/provision_users.py

Lines changed: 29 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,19 @@ def add_arguments(self, parser):
4040
help="Report what would happen without writing or emailing",
4141
)
4242

43+
def _send_setup_email(self, email):
44+
# No request here, so derive the email's domain/protocol from
45+
# WAGTAILADMIN_BASE_URL (django.contrib.sites isn't installed,
46+
# making domain_override mandatory).
47+
base_url = urlparse(settings.WAGTAILADMIN_BASE_URL)
48+
form = AccountSetupForm(data={"email": email})
49+
if form.is_valid():
50+
form.save(
51+
domain_override=base_url.netloc,
52+
use_https=base_url.scheme == "https",
53+
email_template_name="registration/password_reset_email.html",
54+
)
55+
4356
def handle(self, *args, **options):
4457
User = get_user_model()
4558
created, skipped = 0, 0
@@ -58,8 +71,21 @@ def handle(self, *args, **options):
5871
"one of admin/partner/super_partner"
5972
)
6073

61-
if User.objects.filter(username=email).exists():
62-
self.stdout.write(f"skip (exists): {email}")
74+
existing = User.objects.filter(username=email).first()
75+
if existing is not None:
76+
if existing.has_usable_password():
77+
self.stdout.write(f"skip (exists): {email}")
78+
skipped += 1
79+
continue
80+
# Account created by a prior run whose setup email failed
81+
# to send (the user was already committed): resend so the
82+
# command is safely re-runnable instead of stranding the
83+
# account with an unusable password and no link.
84+
if options["dry_run"]:
85+
self.stdout.write(f"would resend setup email: {email}")
86+
else:
87+
self._send_setup_email(email)
88+
self.stdout.write(f"resent setup email: {email}")
6389
skipped += 1
6490
continue
6591

@@ -82,17 +108,7 @@ def handle(self, *args, **options):
82108
user.save()
83109
user.groups.add(group)
84110

85-
# No request here, so derive the email's domain/protocol from
86-
# WAGTAILADMIN_BASE_URL (django.contrib.sites isn't installed,
87-
# making domain_override mandatory).
88-
base_url = urlparse(settings.WAGTAILADMIN_BASE_URL)
89-
form = AccountSetupForm(data={"email": email})
90-
if form.is_valid():
91-
form.save(
92-
domain_override=base_url.netloc,
93-
use_https=base_url.scheme == "https",
94-
email_template_name="registration/password_reset_email.html",
95-
)
111+
self._send_setup_email(email)
96112
self.stdout.write(f"created: {email} ({group_name})")
97113
created += 1
98114

cms/authapi/scopes.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,9 @@
2222
DELETE_ALL_CONTENT = "delete:delete-all"
2323

2424
REVIEW_CONTENT = "create:content_review"
25+
# Explicit bypass of per-reviewer tag scoping. Only admins/superusers get it;
26+
# see the PARTNER_SCOPES note below.
27+
REVIEW_ALL_CONTENT = "review:review-all"
2528

2629
ALL_SCOPES = [
2730
CREATE_DISTRICTR_MAPS,
@@ -37,11 +40,12 @@
3740
DELETE_CONTENT,
3841
DELETE_ALL_CONTENT,
3942
REVIEW_CONTENT,
43+
REVIEW_ALL_CONTENT,
4044
]
4145

4246
# Page editing, galleries, and the datastore tools are all Wagtail-side
4347
# permissions (or service-token calls); the only FastAPI scope a user token
44-
# needs is comment moderation. No read:read-all: the backend treats that
48+
# needs is comment moderation. No review:review-all: the backend treats that
4549
# scope as "unrestricted, ignore review_tags", and partner moderation is
4650
# always scoped by the portal-derived review_tags claim (serializers.py).
4751
# super_partner's extra powers are Django model permissions, not scopes.

cms/authapi/tests.py

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,9 @@ def fastapi_style_verify(token: str) -> dict:
4646

4747
class ScopeMappingTests(TestCase):
4848
def test_partner_scopes(self):
49-
# Comment moderation only — and no read:read-all, which would make
50-
# the backend treat the token as unrestricted and ignore review_tags.
49+
# Comment moderation only — and no review:review-all, which would
50+
# make the backend treat the token as unrestricted and ignore
51+
# review_tags.
5152
user = make_user("partner")
5253
self.assertEqual(scopes_for_user(user), "create:content_review")
5354

@@ -105,7 +106,7 @@ class ReviewScopingClaimTests(TestCase):
105106
the user's teams' portals (a portal's page slug is its comment tag slug).
106107
107108
The FastAPI backend (backend/app/comments/main.py::allowed_review_tags)
108-
treats an ABSENT claim — or a token carrying `read:read-all` — as
109+
treats an ABSENT claim — or a token carrying `review:review-all` — as
109110
unrestricted, and an EMPTY list as "allows nothing", so these tests pin:
110111
admins get no claim; team-scoped users get their portal slugs; team-less
111112
non-admins get [] (fail closed until they join a team).
@@ -147,7 +148,7 @@ def test_team_scoped_user_gets_portal_slugs(self):
147148

148149
payload = self._claim_for(user)
149150
self.assertEqual(payload["review_tags"], ["environment", "schools"])
150-
self.assertNotIn("read:read-all", payload["scope"].split())
151+
self.assertNotIn("review:review-all", payload["scope"].split())
151152

152153
def test_non_default_locale_portal_cannot_mint_scope(self):
153154
# Page slugs are unique only per parent and each locale has its own
@@ -192,7 +193,7 @@ def test_team_less_partner_fails_closed(self):
192193
def test_admin_gets_no_claim(self):
193194
payload = self._claim_for(make_user("admin"))
194195
self.assertNotIn("review_tags", payload)
195-
self.assertIn("read:read-all", payload["scope"].split())
196+
self.assertIn("review:review-all", payload["scope"].split())
196197

197198
def test_superuser_gets_no_claim(self):
198199
user = make_user(None)
@@ -201,9 +202,10 @@ def test_superuser_gets_no_claim(self):
201202
payload = self._claim_for(user)
202203
self.assertNotIn("review_tags", payload)
203204

204-
def test_partner_scope_has_no_read_all(self):
205-
# read:read-all would make the backend ignore review_tags entirely.
206-
self.assertNotIn("read:read-all", scopes_for_user(make_user("partner")))
205+
def test_partner_scope_has_no_review_all(self):
206+
# review:review-all would make the backend ignore review_tags
207+
# entirely.
208+
self.assertNotIn("review:review-all", scopes_for_user(make_user("partner")))
207209

208210

209211
class TokenEndpointTests(TestCase):

cms/config/settings/production.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,11 @@
88
SECURE_PROXY_SSL_HEADER = ("HTTP_X_FORWARDED_PROTO", "https")
99
SESSION_COOKIE_SECURE = True
1010
CSRF_COOKIE_SECURE = True
11+
# HSTS: the ALB already redirects HTTP→HTTPS (infra/alb.ts); this makes
12+
# browsers skip the insecure hop entirely. The CMS lives on its own
13+
# subdomain, so include-subdomains is safe for this host.
14+
SECURE_HSTS_SECONDS = 31536000
15+
SECURE_HSTS_INCLUDE_SUBDOMAINS = True
1116

1217
CSRF_TRUSTED_ORIGINS = [
1318
o for o in os.environ.get("DJANGO_CSRF_TRUSTED_ORIGINS", "").split(",") if o

cms/content/management/commands/migrate_tiptap.py

Lines changed: 73 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -149,66 +149,81 @@ def handle(self, *args, **options):
149149
pages_updated = 0
150150
pages_skipped = 0
151151

152-
for content_type, config in CONTENT_TYPES.items():
153-
rows = self._fetch_rows(config)
154-
by_slug = defaultdict(dict)
155-
for row in rows:
156-
by_slug[row["slug"]][row["language"]] = row
157-
158-
for slug in sorted(by_slug):
159-
lang_rows = by_slug[slug]
160-
canonical_language = (
161-
DEFAULT_LANGUAGE
162-
if DEFAULT_LANGUAGE in lang_rows
163-
else sorted(lang_rows)[0]
164-
)
165-
if canonical_language != DEFAULT_LANGUAGE:
166-
self.stderr.write(
167-
self.style.WARNING(
168-
f"{content_type}/{slug}: no English row; using "
169-
f"'{canonical_language}' as canonical"
152+
# One transaction around the whole pass: the fidelity-check
153+
# CommandError below must roll back every page written this run,
154+
# matching the all-or-nothing behaviour this command has when
155+
# executed inside the atomic Django migration. (In dry-run no
156+
# writes happen, so the wrapper is a no-op savepoint.)
157+
try:
158+
with transaction.atomic():
159+
for content_type, config in CONTENT_TYPES.items():
160+
rows = self._fetch_rows(config)
161+
by_slug = defaultdict(dict)
162+
for row in rows:
163+
by_slug[row["slug"]][row["language"]] = row
164+
165+
for slug in sorted(by_slug):
166+
lang_rows = by_slug[slug]
167+
canonical_language = (
168+
DEFAULT_LANGUAGE
169+
if DEFAULT_LANGUAGE in lang_rows
170+
else sorted(lang_rows)[0]
170171
)
172+
if canonical_language != DEFAULT_LANGUAGE:
173+
self.stderr.write(
174+
self.style.WARNING(
175+
f"{content_type}/{slug}: no English row; using "
176+
f"'{canonical_language}' as canonical"
177+
)
178+
)
179+
180+
canonical_page = None
181+
# Canonical language first; translations need it to exist.
182+
ordered = [canonical_language] + [
183+
language
184+
for language in sorted(lang_rows)
185+
if language != canonical_language
186+
]
187+
for language in ordered:
188+
row = lang_rows[language]
189+
entry = self._convert_row(content_type, row)
190+
report.append(entry)
191+
if not entry["text_ok"]:
192+
failed_rows += 1
193+
self._print_entry(entry)
194+
195+
if dry_run:
196+
continue
197+
198+
with transaction.atomic():
199+
page, action = self._upsert_page(
200+
config,
201+
row,
202+
entry,
203+
canonical_page=canonical_page,
204+
is_canonical=(language == canonical_language),
205+
)
206+
if language == canonical_language:
207+
canonical_page = page
208+
if action == "created":
209+
pages_created += 1
210+
elif action == "updated":
211+
pages_updated += 1
212+
else:
213+
pages_skipped += 1
214+
215+
if failed_rows:
216+
raise CommandError(
217+
f"{failed_rows} row(s) failed the plain-text fidelity check "
218+
"(see report lines marked TEXT-LOSS)."
171219
)
172-
173-
canonical_page = None
174-
# Canonical language first; translations need it to exist.
175-
ordered = [canonical_language] + [
176-
language
177-
for language in sorted(lang_rows)
178-
if language != canonical_language
179-
]
180-
for language in ordered:
181-
row = lang_rows[language]
182-
entry = self._convert_row(content_type, row)
183-
report.append(entry)
184-
if not entry["text_ok"]:
185-
failed_rows += 1
186-
self._print_entry(entry)
187-
188-
if dry_run:
189-
continue
190-
191-
with transaction.atomic():
192-
page, action = self._upsert_page(
193-
config,
194-
row,
195-
entry,
196-
canonical_page=canonical_page,
197-
is_canonical=(language == canonical_language),
198-
)
199-
if language == canonical_language:
200-
canonical_page = page
201-
if action == "created":
202-
pages_created += 1
203-
elif action == "updated":
204-
pages_updated += 1
205-
else:
206-
pages_skipped += 1
207-
208-
if options["json_report"]:
209-
with open(options["json_report"], "w") as f:
210-
json.dump(report, f, indent=2)
211-
self.stdout.write(f"Report written to {options['json_report']}")
220+
finally:
221+
# The JSON report is the diagnostic naming the failed rows —
222+
# write it even when the fidelity check rolls the pass back.
223+
if options["json_report"]:
224+
with open(options["json_report"], "w") as f:
225+
json.dump(report, f, indent=2)
226+
self.stdout.write(f"Report written to {options['json_report']}")
212227

213228
if not dry_run:
214229
self.stdout.write(
@@ -218,11 +233,6 @@ def handle(self, *args, **options):
218233
)
219234
)
220235

221-
if failed_rows:
222-
raise CommandError(
223-
f"{failed_rows} row(s) failed the plain-text fidelity check "
224-
"(see report lines marked TEXT-LOSS)."
225-
)
226236
if dry_run:
227237
self.stdout.write(
228238
self.style.SUCCESS(f"Dry run OK: {len(report)} row(s) converted.")

0 commit comments

Comments
 (0)