Skip to content

Commit 4b2cb16

Browse files
fix(security): gate checkpoint attempts on entitlements, not enrollments (#532) (#535)
The lesson-checkpoint attempt route reads through the service-role client, so the RLS backstop added in #509 never applied to it, and the only gate it had was an active `enrollments` row. That row has not meant "has access" since migration 20260516150000: nothing revokes it (refunds revoke entitlements, and tenants.access_cutoff_at is read only inside has_course_access), and its RLS INSERT policy let any tenant member issue one for any course in their tenant. Three populations could therefore spend the school's AI grading budget and write progress records for content they had no right to: students past their tenant's access cutoff (#494), students whose entitlement was revoked by a refund (#498/#515), and anyone who self-inserted an enrollment. - Route now calls resolveCourseAccessState() and returns a 403 that tells a tenant suspension apart from a missing entitlement, so the checkpoint UI can explain which one it is (new translated copy, en + es). - Same check added to the two sibling admin-client media routes (analyze, signed-url), which were ownership-checked only. Not live holes — both sit behind the gated upload path — but both spend AI credits or mint storage URLs, and an entitlement can be revoked after the upload was authorized. - Migration 20260725160000 stops enrollments being an access grant at the database level: its INSERT policy and the checkpoint-attempt INSERT policy now both require has_course_access(). Every legitimate writer (enroll_user, self_enroll_subscription_course, grant_free_entitlement, issue_certificate_if_eligible) is SECURITY DEFINER, so none are affected. - docs/MONETIZATION.md now describes the API-route layer accurately instead of claiming page gates cover it, and both docs state that an enrollment row is not an access grant. New E2E spec pins the three states (entitled 200, revoked 403 accessDenied, cutoff 403 accessSuspended); its two refusal cases fail against the old code. The four pre-existing `no-explicit-any` errors in the media analyze route are fixed rather than bypassed, since lint-staged lints the whole file. Claude-Session: https://claude.ai/code/session_019QSWA8GwmjsYHUS7ov5NG3 Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 86d60fa commit 4b2cb16

10 files changed

Lines changed: 369 additions & 24 deletions

File tree

app/api/exercises/media/analyze/route.ts

Lines changed: 31 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { createAdminClient } from '@/lib/supabase/admin'
22
import { getApiAuthContext } from '@/lib/supabase/api-auth'
3+
import { hasCourseAccess } from '@/lib/services/course-access'
34
import { runSpeechPipeline } from '@/lib/speech/pipeline'
45
import { getPipeline } from '@/lib/speech/registry'
56
import type { ExerciseContext } from '@/lib/speech/types'
@@ -8,6 +9,22 @@ export const maxDuration = 120
89

910
const STORAGE_BUCKET = 'exercise-media'
1011

12+
/** The `exercises(...)` join on the submission row. */
13+
type JoinedExercise = {
14+
id: number
15+
title: string | null
16+
instructions: string | null
17+
course_id: number
18+
tenant_id: string
19+
exercise_config: {
20+
passing_score?: number
21+
topic_prompt?: string
22+
rubric?: ExerciseContext['rubric']
23+
stt_provider?: string
24+
ai_coach?: string
25+
} | null
26+
}
27+
1128
export async function POST(req: Request) {
1229
// 1. Auth — cookie session (web) or Bearer token (mobile), server-verified
1330
const auth = await getApiAuthContext(req)
@@ -44,8 +61,10 @@ export async function POST(req: Request) {
4461
return new Response('Submission not found', { status: 404 })
4562
}
4663

64+
const exercise = submission.exercises as unknown as JoinedExercise | null
65+
4766
// 5. Status guard — only pending submissions can be analyzed (prevents re-triggering)
48-
const passingScore = ((submission.exercises as any)?.exercise_config as any)?.passing_score ?? 70
67+
const passingScore = exercise?.exercise_config?.passing_score ?? 70
4968
if (submission.status === 'completed') {
5069
return Response.json({ already_completed: true, evaluation: submission.ai_evaluation, passed: true, passingScore })
5170
}
@@ -57,11 +76,18 @@ export async function POST(req: Request) {
5776
}
5877

5978
// 6. Verify exercise tenant matches (defense in depth)
60-
const exercise = submission.exercises as any
6179
if (!exercise || exercise.tenant_id !== tenantId) {
6280
return new Response('Exercise not found', { status: 404 })
6381
}
6482

83+
// 6b. Course access (issue #532). Ownership of the submission is not access:
84+
// this route runs on the admin client and spends AI credits, and an
85+
// entitlement can be revoked — or the tenant's access cutoff can pass —
86+
// between the gated upload and this call.
87+
if (!(await hasCourseAccess(adminClient, user.id, exercise.course_id))) {
88+
return new Response('You do not have access to this course', { status: 403 })
89+
}
90+
6591
// 7. Atomically set processing (prevents concurrent analysis of same submission)
6692
const { data: updated, error: updateError } = await adminClient
6793
.from('exercise_media_submissions')
@@ -149,14 +175,15 @@ export async function POST(req: Request) {
149175
}
150176

151177
return Response.json({ evaluation, passed, passingScore })
152-
} catch (err: any) {
178+
} catch (err) {
153179
console.error('Speech pipeline error:', err)
154180

155181
await adminClient
156182
.from('exercise_media_submissions')
157183
.update({ status: 'failed' })
158184
.eq('id', submissionId)
159185

160-
return new Response(err.message ?? 'Analysis failed', { status: 500 })
186+
const message = err instanceof Error ? err.message : 'Analysis failed'
187+
return new Response(message, { status: 500 })
161188
}
162189
}

app/api/exercises/media/signed-url/route.ts

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { createAdminClient } from '@/lib/supabase/admin'
22
import { getApiAuthContext } from '@/lib/supabase/api-auth'
3+
import { hasCourseAccess } from '@/lib/services/course-access'
34

45
const STORAGE_BUCKET = 'exercise-media'
56

@@ -21,7 +22,7 @@ export async function POST(req: Request) {
2122

2223
const { data: submission, error } = await adminClient
2324
.from('exercise_media_submissions')
24-
.select('media_url, user_id, tenant_id')
25+
.select('media_url, user_id, tenant_id, exercises(course_id, tenant_id)')
2526
.eq('id', submissionId)
2627
.single()
2728

@@ -33,6 +34,21 @@ export async function POST(req: Request) {
3334
return new Response('Submission not found', { status: 404 })
3435
}
3536

37+
// Course access (issue #532). Owning the recording is not the same as being
38+
// entitled to the course it belongs to: this handler mints a storage URL off
39+
// the admin client, and entitlements can be revoked (or the tenant's access
40+
// cutoff can pass) after the submission was created.
41+
const exercise = submission.exercises as unknown as {
42+
course_id: number
43+
tenant_id: string
44+
} | null
45+
if (!exercise || exercise.tenant_id !== tenantId) {
46+
return new Response('Submission not found', { status: 404 })
47+
}
48+
if (!(await hasCourseAccess(adminClient, user.id, exercise.course_id))) {
49+
return new Response('You do not have access to this course', { status: 403 })
50+
}
51+
3652
const { data: urlData } = await adminClient
3753
.storage
3854
.from(STORAGE_BUCKET)

app/api/lesson-checkpoints/[checkpointId]/attempt/route.ts

Lines changed: 23 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { z } from 'zod'
22
import { generateObject } from 'ai'
33
import { getApiAuthContext } from '@/lib/supabase/api-auth'
44
import { createAdminClient } from '@/lib/supabase/admin'
5+
import { resolveCourseAccessState } from '@/lib/services/course-access'
56
import { AI_MODELS } from '@/lib/ai/config'
67
import { gradeCheckpointQuestions } from '@/lib/checkpoints/grading'
78
import {
@@ -92,18 +93,28 @@ export async function POST(
9293
return Response.json({ error: 'Checkpoint not found' }, { status: 404 })
9394
}
9495

95-
// Active enrollment required — mirrors the RLS insert policy so failures
96-
// surface as a clear 403 instead of an opaque insert error.
97-
const { data: enrollment } = await adminClient
98-
.from('enrollments')
99-
.select('enrollment_id')
100-
.eq('user_id', user.id)
101-
.eq('course_id', exercise.course_id)
102-
.eq('status', 'active')
103-
.limit(1)
104-
.maybeSingle()
105-
if (!enrollment) {
106-
return Response.json({ error: 'Not enrolled in this course' }, { status: 403 })
96+
// Course access required (issue #532). This route reads through the admin
97+
// client, so #509's RLS backstop on content never applies here — the gate
98+
// has to be explicit. It must be `entitlements`, not `enrollments`: an
99+
// enrollment row is a progress record that nothing revokes and that a
100+
// tenant member can issue for themselves, so it grants no access on its own.
101+
const accessState = await resolveCourseAccessState(
102+
adminClient,
103+
user.id,
104+
exercise.course_id
105+
)
106+
if (accessState !== 'granted') {
107+
return Response.json(
108+
{
109+
error:
110+
accessState === 'suspended'
111+
? "Your school's access is currently suspended"
112+
: 'You do not have access to this course',
113+
accessDenied: true,
114+
accessSuspended: accessState === 'suspended',
115+
},
116+
{ status: 403 }
117+
)
107118
}
108119

109120
const config = exercise.exercise_config ?? {}

components/lesson/checkpoints/checkpoint-exercise-renderer.tsx

Lines changed: 30 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,28 @@ interface CheckpointExerciseRendererProps {
2121
checkpoint: LessonCheckpointClientData
2222
}
2323

24-
type AttemptResponse =
25-
| { ok: true; data: CheckpointAttemptResult }
26-
| { ok: false; status: number; notCompleted?: boolean; error?: string }
24+
type AttemptFailure = {
25+
ok: false
26+
status: number
27+
notCompleted?: boolean
28+
accessDenied?: boolean
29+
accessSuspended?: boolean
30+
error?: string
31+
}
32+
33+
type AttemptResponse = { ok: true; data: CheckpointAttemptResult } | AttemptFailure
34+
35+
/**
36+
* The route's access refusals (issue #532) carry flags rather than only a
37+
* message, so the student reads translated copy instead of the server's raw
38+
* English — and a school-wide suspension reads differently from "you never
39+
* bought this course", which is the distinction #494 exists to make.
40+
*/
41+
function attemptErrorKey(res: AttemptFailure): 'accessSuspended' | 'accessDenied' | null {
42+
if (res.accessSuspended) return 'accessSuspended'
43+
if (res.accessDenied) return 'accessDenied'
44+
return null
45+
}
2746

2847
async function postAttempt(checkpointId: number, body: unknown): Promise<AttemptResponse> {
2948
try {
@@ -38,6 +57,8 @@ async function postAttempt(checkpointId: number, body: unknown): Promise<Attempt
3857
ok: false,
3958
status: res.status,
4059
notCompleted: payload?.notCompleted === true,
60+
accessDenied: payload?.accessDenied === true,
61+
accessSuspended: payload?.accessSuspended === true,
4162
error: typeof payload?.error === 'string' ? payload.error : undefined,
4263
}
4364
}
@@ -120,7 +141,8 @@ function ClosedCheckpointForm({ checkpoint }: CheckpointExerciseRendererProps) {
120141
const res = await postAttempt(checkpoint.id, { kind: 'answers', answers: payload })
121142
setSubmitting(false)
122143
if (!res.ok) {
123-
setError(res.error ?? t('genericError'))
144+
const errorKey = attemptErrorKey(res)
145+
setError(errorKey ? t(errorKey) : (res.error ?? t('genericError')))
124146
return
125147
}
126148
setResult(res.data)
@@ -257,7 +279,8 @@ function TextCheckpointForm({ checkpoint }: CheckpointExerciseRendererProps) {
257279
const res = await postAttempt(checkpoint.id, { kind: 'text', text })
258280
setSubmitting(false)
259281
if (!res.ok) {
260-
setError(res.error ?? t('genericError'))
282+
const errorKey = attemptErrorKey(res)
283+
setError(errorKey ? t(errorKey) : (res.error ?? t('genericError')))
261284
return
262285
}
263286
setResult(res.data)
@@ -346,7 +369,8 @@ function ExternalCheckpointForm({ checkpoint }: CheckpointExerciseRendererProps)
346369
setNotCompleted(true)
347370
return
348371
}
349-
setError(res.error ?? t('genericError'))
372+
const errorKey = attemptErrorKey(res)
373+
setError(errorKey ? t(errorKey) : (res.error ?? t('genericError')))
350374
return
351375
}
352376
setResult(res.data)

docs/DATABASE_SCHEMA.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -456,6 +456,8 @@ Written by `enroll_user()` and `handle_new_subscription()`. Read via `has_course
456456

457457
> The old `product_id` / `subscription_id` columns and their "one or the other" CHECK constraint were **dropped** in the entitlements migration. If you find code or docs referencing them, they are out of date. Students on a subscription self-enroll from `/dashboard/student/browse` (`useEnrollment()` hook); subscriptions grant access but do not auto-enroll.
458458
459+
> **Never gate on the presence of an enrollment row** (issue #532). Nothing revokes one — refunds revoke *entitlements* and `tenants.access_cutoff_at` is read only inside `has_course_access()` — so a row survives every revocation path, and until migration `20260725160000` any tenant member could insert one for any course in their tenant. The INSERT policy now also requires `has_course_access()`; legitimate writers (`enroll_user()`, `self_enroll_subscription_course()`, `grant_free_entitlement()`, `issue_certificate_if_eligible()`) are all `SECURITY DEFINER` and unaffected.
460+
459461
#### `products`
460462
Purchasable course bundles. Tenant-scoped.
461463

docs/MONETIZATION.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -243,7 +243,9 @@ The two sections above are **creation-time soft caps only** — they block a *ne
243243
- Enforcement itself is in the database: once `access_cutoff_at` passes while the tenant is still over its plan's limits, `has_course_access()` (SQL function, migration `supabase/migrations/20260724130000_access_cutoff_enforcement.sql`) starts returning `false` for every student in that tenant. Access is restored automatically as soon as usage is back under the limit or the tenant upgrades.
244244
- That function is enforced in two layers (issue #509 — before it, the cutoff was a no-op on exactly the pages that serve content):
245245
- **RLS** (`supabase/migrations/20260724150000_content_entitlement_rls.sql`): the `authenticated` SELECT policies on `lessons`, `exercises`, `exams` and `lesson_resources` all require `has_course_access()`, with escape hatches for the course's author and for active teachers/admins of the tenant (`is_tenant_staff()`). Published preview lessons (`is_preview`) stay readable to everyone. This is the backstop — it holds even where a page uses the service-role client.
246-
- **Page/route gates**: `requireCourseAccess()` (`lib/services/course-access-guard.ts`) on every student course route — course detail, lessons, exercises (list + detail), exams (list, taker, result, review), community — plus `hasCourseAccess()` in the certificate, AI tutor and exercise-submission APIs. A refusal caused by the tenant cutoff (rather than by a missing entitlement) sends the student to `/dashboard/student/access-suspended`, which explains that the school is over its limits and that their enrollment is intact.
246+
- **Page gates**: `requireCourseAccess()` (`lib/services/course-access-guard.ts`) on every student course route — course detail, lessons, exercises (list + detail), exams (list, taker, result, review), community. A refusal caused by the tenant cutoff (rather than by a missing entitlement) sends the student to `/dashboard/student/access-suspended`, which explains that the school is over its limits and that their enrollment is intact.
247+
- **API-route gates**: an API route that reads through `createAdminClient()` is outside the RLS backstop by construction, so it has to carry the check itself. The routes that serve or produce paid content all call `hasCourseAccess()` / `resolveCourseAccessState()` (`lib/services/course-access.ts` — the API-safe module; `course-access-guard.ts` imports `next/navigation` and is for pages only): `api/certificates/generate`, `api/chat/aristotle`, `api/exercises/artifact/evaluate`, `api/exercises/media/upload-url`, `api/exercises/media/analyze`, `api/exercises/media/signed-url`, and `api/lesson-checkpoints/[checkpointId]/attempt`. The checkpoint route returns a 403 that distinguishes a suspension from a missing entitlement, so the checkpoint UI can say which one it is.
248+
- **An `enrollments` row is not an access grant** (issue #532). Since migration `20260516150000` it is a learning-progress record: nothing revokes it (refunds revoke *entitlements*, and `access_cutoff_at` is read only inside `has_course_access()`), and its RLS INSERT policy now requires `has_course_access()` precisely because it used to be self-issuable by any tenant member. Gate on entitlements — never on an enrollment row.
247249
- This is a deliberate decision, not an oversight: with no production users yet, real enforcement was shipped directly rather than grandfathering pre-existing over-limit usage.
248250

249251
#### Blast radius — the cutoff is tenant-wide and all-or-nothing (issue #517)

messages/en.json

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3890,6 +3890,8 @@
38903890
"retry": "Try again",
38913891
"review": "Edit and resubmit",
38923892
"genericError": "Something went wrong. Please try again.",
3893+
"accessDenied": "You don't have access to this course.",
3894+
"accessSuspended": "Your school's access is currently suspended. Your progress is safe — ask your school administrator to restore access.",
38933895
"aiUnavailable": "Your answer was recorded, but AI feedback is unavailable right now.",
38943896
"externalPrompt": "This checkpoint is completed through its exercise page. Open the exercise, finish it there, then sync your result here.",
38953897
"externalLinkLabel": "Open exercise",

messages/es.json

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3735,6 +3735,8 @@
37353735
"retry": "Intentar de nuevo",
37363736
"review": "Editar y reenviar",
37373737
"genericError": "Algo salió mal. Inténtalo de nuevo.",
3738+
"accessDenied": "No tienes acceso a este curso.",
3739+
"accessSuspended": "El acceso de tu escuela está suspendido en este momento. Tu progreso está a salvo: pide a la administración de tu escuela que lo restablezca.",
37383740
"aiUnavailable": "Tu respuesta fue registrada, pero la retroalimentación con IA no está disponible en este momento.",
37393741
"externalPrompt": "Este checkpoint se completa en su página de ejercicio. Abre el ejercicio, termínalo allí y luego sincroniza tu resultado aquí.",
37403742
"externalLinkLabel": "Abrir ejercicio",
Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
-- Issue #532 — an `enrollments` row is not an access grant.
2+
--
3+
-- Since migration 20260516150000, `enrollments` is a learning-progress record
4+
-- and `entitlements` is the source of truth for course access. The INSERT
5+
-- policy below never caught up: its WITH CHECK was
6+
--
7+
-- (auth.uid() = user_id) AND (tenant_id = get_tenant_id())
8+
--
9+
-- so any tenant member could POST /rest/v1/enrollments for any course in
10+
-- their tenant and manufacture a row that *looks* like proof of access.
11+
-- Nothing revokes an enrollment either — refunds revoke entitlements
12+
-- (lib/payments/webhook-dispatch.ts) and `tenants.access_cutoff_at` (#494) is
13+
-- read only inside has_course_access() — so the row also survives every
14+
-- revocation path. Code that reads it as an access check (the lesson-checkpoint
15+
-- attempt API route, and the checkpoint-attempt INSERT policy below) was
16+
-- therefore gate-less in three cases: past the tenant access cutoff, after a
17+
-- refund, and for a self-inserted row.
18+
--
19+
-- Both policies now gate on has_course_access(), the same function the page
20+
-- guards and the content RLS from #509 use.
21+
--
22+
-- Safe for every legitimate writer: enroll_user(), self_enroll_subscription_course(),
23+
-- grant_free_entitlement() and issue_certificate_if_eligible() are all
24+
-- SECURITY DEFINER (RLS does not apply to them), and admin/seed paths use the
25+
-- service-role client. No client-side `enrollments` INSERT exists in the app.
26+
-- The policy is tightened rather than dropped so any caller that genuinely
27+
-- holds access keeps working.
28+
29+
DROP POLICY IF EXISTS "Users can create own enrollments" ON public.enrollments;
30+
31+
CREATE POLICY "Users can create own enrollments"
32+
ON public.enrollments FOR INSERT TO authenticated
33+
WITH CHECK (
34+
(SELECT auth.uid()) = user_id
35+
AND tenant_id = (SELECT get_tenant_id())
36+
AND public.has_course_access((SELECT auth.uid()), course_id)
37+
);
38+
39+
-- The checkpoint-attempt insert policy leaned on the same self-issuable row.
40+
-- Everything else about it is unchanged: same user, same enabled checkpoint,
41+
-- same (checkpoint, lesson, exercise, tenant) consistency requirement.
42+
DROP POLICY IF EXISTS lesson_checkpoint_attempts_students_insert_own_rows
43+
ON public.lesson_checkpoint_attempts;
44+
45+
CREATE POLICY lesson_checkpoint_attempts_students_insert_own_rows
46+
ON public.lesson_checkpoint_attempts FOR INSERT TO authenticated
47+
WITH CHECK (
48+
user_id = (SELECT auth.uid())
49+
AND EXISTS (
50+
SELECT 1
51+
FROM lesson_checkpoints c
52+
WHERE c.id = lesson_checkpoint_attempts.checkpoint_id
53+
AND c.tenant_id = lesson_checkpoint_attempts.tenant_id
54+
AND c.lesson_id = lesson_checkpoint_attempts.lesson_id
55+
AND c.exercise_id = lesson_checkpoint_attempts.exercise_id
56+
AND c.is_enabled
57+
)
58+
AND public.has_course_access((SELECT auth.uid()), lesson_checkpoint_attempts.course_id)
59+
);
60+
61+
COMMENT ON TABLE public.enrollments IS
62+
'Learning-progress record only (since 20260516150000). NOT an access grant: '
63+
'course access lives in `entitlements` and is checked with has_course_access(). '
64+
'Never gate content, APIs or RLS on the presence of an enrollment row.';

0 commit comments

Comments
 (0)