diff --git a/src/__tests__/onlineEssaySubmissions.test.ts b/src/__tests__/onlineEssaySubmissions.test.ts new file mode 100644 index 00000000..3c981ed3 --- /dev/null +++ b/src/__tests__/onlineEssaySubmissions.test.ts @@ -0,0 +1,36 @@ +import { describe, expect, it } from 'vitest'; +import { keyOnlineEssaySubmissions } from '../utils/onlineEssaySubmissions'; + +describe('keyOnlineEssaySubmissions', () => { + it('groups student ids by assignment', () => { + const map = keyOnlineEssaySubmissions([ + { assignmentId: 'a1', studentId: 's1' }, + { assignmentId: 'a1', studentId: 's2' }, + { assignmentId: 'a2', studentId: 's3' }, + ]); + expect(map.get('a1')).toEqual(new Set(['s1', 's2'])); + expect(map.get('a2')).toEqual(new Set(['s3'])); + }); + + it('dedupes repeated submissions from the same student', () => { + const map = keyOnlineEssaySubmissions([ + { assignmentId: 'a1', studentId: 's1' }, + { assignmentId: 'a1', studentId: 's1' }, + ]); + expect(map.get('a1')).toEqual(new Set(['s1'])); + }); + + it('ignores rows without a student id', () => { + const map = keyOnlineEssaySubmissions([ + { assignmentId: 'a1', studentId: '' }, + { assignmentId: 'a1', studentId: 's1' }, + ]); + expect(map.get('a1')).toEqual(new Set(['s1'])); + }); + + it('returns an empty map for no rows and never for an existing key', () => { + expect(keyOnlineEssaySubmissions([])).toEqual(new Map()); + const map = keyOnlineEssaySubmissions([{ assignmentId: 'a1', studentId: 's1' }]); + expect(map.get('missing')).toBeUndefined(); + }); +}); diff --git a/src/__tests__/submitEssayEmail.test.ts b/src/__tests__/submitEssayEmail.test.ts new file mode 100644 index 00000000..b717e08a --- /dev/null +++ b/src/__tests__/submitEssayEmail.test.ts @@ -0,0 +1,51 @@ +import { describe, expect, it } from 'vitest'; +import { resolveStudentEmail } from '../../supabase/functions/submit-essay/email'; + +describe('submit-essay email resolution', () => { + it('falls back to the client email when the auth record has an EMPTY-STRING email (anonymous session regression)', () => { + // GoTrue anonymous sessions carry an empty-string email claim, not null. + // `authEmail ?? bodyEmail` would let the empty string win and the function + // would reject every anonymous submission as missing studentEmail. + const result = resolveStudentEmail('', 'student@school.nl'); + expect(result).toEqual({ email: 'student@school.nl', mismatch: false }); + }); + + it('uses the auth email when present and matching', () => { + expect(resolveStudentEmail('student@school.nl', 'student@school.nl')).toEqual({ + email: 'student@school.nl', + mismatch: false, + }); + }); + + it('treats emails case-insensitively', () => { + expect(resolveStudentEmail('Student@School.NL', 'student@school.nl')).toEqual({ + email: 'Student@School.NL', + mismatch: false, + }); + }); + + it('flags a mismatch when the auth and client emails differ (403 path)', () => { + const result = resolveStudentEmail('student@school.nl', 'other@school.nl'); + expect(result.email).toBe('student@school.nl'); + expect(result.mismatch).toBe(true); + }); + + it('uses the auth email when the client sends none', () => { + expect(resolveStudentEmail('student@school.nl', undefined)).toEqual({ + email: 'student@school.nl', + mismatch: false, + }); + }); + + it('uses the client email when there is no auth email at all', () => { + expect(resolveStudentEmail(undefined, 'student@school.nl')).toEqual({ + email: 'student@school.nl', + mismatch: false, + }); + }); + + it('resolves to null when neither source has an email', () => { + expect(resolveStudentEmail(undefined, undefined)).toEqual({ email: null, mismatch: false }); + expect(resolveStudentEmail('', null)).toEqual({ email: null, mismatch: false }); + }); +}); diff --git a/src/__tests__/submitTestValidation.test.ts b/src/__tests__/submitTestValidation.test.ts new file mode 100644 index 00000000..aec581b9 --- /dev/null +++ b/src/__tests__/submitTestValidation.test.ts @@ -0,0 +1,63 @@ +import { describe, expect, it } from 'vitest'; +import { + sanitizeAnswers, + isAssignmentExpired, + attemptPolicyFor, +} from '../../supabase/functions/submit-test/validation'; + +describe('submit-test sanitizeAnswers', () => { + it('strips forged fields like pointsEarned so they never reach storage', () => { + const input = [ + { questionId: 'q1', response: 'Paris', pointsEarned: 999 }, + { questionId: 'q2', response: '4', extra: { whatever: true } }, + ]; + expect(sanitizeAnswers(input)).toEqual([ + { questionId: 'q1', response: 'Paris' }, + { questionId: 'q2', response: '4' }, + ]); + }); + + it('preserves questionId/response and order', () => { + const input = [ + { questionId: 'b', response: '2' }, + { questionId: 'a', response: '1' }, + ]; + expect(sanitizeAnswers(input)).toEqual(input); + }); +}); + +describe('submit-test isAssignmentExpired', () => { + const now = new Date('2026-08-17T12:00:00Z'); + + it('rejects when the deadline is in the past', () => { + expect(isAssignmentExpired('2026-08-17T11:59:59Z', now)).toBe(true); + }); + + it('allows when the deadline is in the future', () => { + expect(isAssignmentExpired('2026-08-17T12:00:01Z', now)).toBe(false); + }); + + it('rejects a submission at the exact expiration instant', () => { + expect(isAssignmentExpired('2026-08-17T12:00:00Z', now)).toBe(true); + }); + + it('allows rows with no deadline', () => { + expect(isAssignmentExpired(null, now)).toBe(false); + expect(isAssignmentExpired(undefined, now)).toBe(false); + }); +}); + +describe('submit-test attemptPolicyFor', () => { + it('practice mode allows up to 5 attempts (retakes)', () => { + expect(attemptPolicyFor('practice')).toEqual({ isPractice: true, maxAttempts: 5 }); + }); + + it('assessment mode allows exactly one attempt', () => { + expect(attemptPolicyFor('assessment')).toEqual({ isPractice: false, maxAttempts: 1 }); + }); + + it('legacy rows with no mode keep the one-attempt guard', () => { + expect(attemptPolicyFor(undefined)).toEqual({ isPractice: false, maxAttempts: 1 }); + expect(attemptPolicyFor(null)).toEqual({ isPractice: false, maxAttempts: 1 }); + }); +}); diff --git a/src/hooks/useLiveSessionTelemetry.ts b/src/hooks/useLiveSessionTelemetry.ts index 84db1fc3..93a3fc35 100644 --- a/src/hooks/useLiveSessionTelemetry.ts +++ b/src/hooks/useLiveSessionTelemetry.ts @@ -50,6 +50,17 @@ export interface UseLiveSessionTelemetryReturn { flush: () => ProctorEvent[]; /** True once a Realtime broadcast channel is active (DB mode, enabled). */ isBroadcasting: boolean; + /** + * Sends a one-off broadcast on the active session channel and resolves with + * the server acknowledgement ('ok'), a timeout, or an error — or 'ok' + * immediately when no channel is live. Used for state transitions the + * channel owner needs to see — e.g. the student broadcasting 'submitted' + * just before `enabled` flips false and tears the channel down. Callers + * that gate UI on the broadcast (like the submitted confirmation) should + * await it; the channel is configured with `broadcast.ack: true` so the + * promise only resolves once the Realtime server confirmed receipt. + */ + broadcast: (event: string, payload?: unknown) => Promise<'ok' | 'timed out' | 'error'>; } function shallowEqualSnapshot(a: LiveSessionSnapshot | null, b: LiveSessionSnapshot): boolean { @@ -104,6 +115,13 @@ export function useLiveSessionTelemetry({ channelRef.current?.send({ type: 'broadcast', event: 'event', payload: event }); }, []); + const broadcast = useCallback((event: string, payload?: unknown): Promise<'ok' | 'timed out' | 'error'> => { + // send() is typed as a branded `string` in this realtime-js version; its runtime + // values are exactly 'ok' | 'timed out' | 'error' (see RealtimeChannel.send). + return (channelRef.current?.send({ type: 'broadcast', event, payload }) ?? + Promise.resolve('ok' as const)) as Promise<'ok' | 'timed out' | 'error'>; + }, []); + const flush = useCallback((): ProctorEvent[] => { const result = eventsRef.current; eventsRef.current = []; @@ -120,7 +138,12 @@ export function useLiveSessionTelemetry({ const client = createClient(supabaseUrl!, supabaseAnonKey!, { auth: { persistSession: false, autoRefreshToken: false, storageKey: 'rm_monitor_ephemeral' }, }); - const channel = client.channel(`monitor:${kind}:${assignmentKey}`); + // ack:true makes send() resolve only after the server confirms receipt — + // important for the 'submitted' broadcast, which is fire-and-forget today + // but must not be silently dropped right before the channel tears down. + const channel = client.channel(`monitor:${kind}:${assignmentKey}`, { + config: { broadcast: { ack: true, self: false } }, + }); channel.on('broadcast', { event: 'nudge' }, ({ payload }) => { onNudgeRef.current?.((payload as { message: string }).message); }); @@ -260,5 +283,5 @@ export function useLiveSessionTelemetry({ return () => clearInterval(interval); }, [enabled, getSnapshot, hasDb]); - return { events, flush, isBroadcasting }; + return { events, flush, isBroadcasting, broadcast }; } diff --git a/src/hooks/useOnlineEssaySubmissions.ts b/src/hooks/useOnlineEssaySubmissions.ts new file mode 100644 index 00000000..508d7280 --- /dev/null +++ b/src/hooks/useOnlineEssaySubmissions.ts @@ -0,0 +1,39 @@ +import { useEffect, useState } from 'react'; +import { useEssays } from '../context/AppContext'; +import { loadSupabaseConfig } from '../services/database'; +import { keyOnlineEssaySubmissions } from '../utils/onlineEssaySubmissions'; + +/** + * Online essay submissions (essay_submissions rows written by the submit-essay + * edge function), keyed teacherKey -> set of student ids that handed in. + * + * The store's hydrated `essaySubmissions` only tracks OFFLINE submissions + * (essay_offline_submissions — the pasted-code import path), so roster badges + * and submitted counts would never reflect a student who handed in through the + * online portal. This fetches the online table separately, purely for status + * derivation — the result is never persisted back. + */ +export function useOnlineEssaySubmissions(): Map> { + const { fetchAllEssaySubmissions } = useEssays(); + const config = loadSupabaseConfig(); + const hasDb = !!config?.supabaseUrl && !!config?.supabaseAnonKey; + const [byTeacherKey, setByTeacherKey] = useState>>(new Map()); + + useEffect(() => { + if (!hasDb) return; + let cancelled = false; + void fetchAllEssaySubmissions() + .then((rows) => { + if (cancelled) return; + setByTeacherKey(keyOnlineEssaySubmissions(rows)); + }) + .catch(() => { + // Non-fatal: offline submissions still drive the badges. + }); + return () => { + cancelled = true; + }; + }, [hasDb, fetchAllEssaySubmissions]); + + return byTeacherKey; +} diff --git a/src/main.tsx b/src/main.tsx index 753acd67..6f04a115 100644 --- a/src/main.tsx +++ b/src/main.tsx @@ -50,8 +50,25 @@ function isStudentRoute() { const router = createHashRouter([ { path: '/feedback/:code', element: }, { path: '/preview/:code', element: }, - { path: '/essay/:code', element: }, - { path: '/test/:code', element: }, + // Student pages need toasts (e.g. the live-monitor nudge check-in banner), so + // wrap them in ToastProvider like the main app route — without it, the + // ToastContext default makes showToast a silent no-op. + { + path: '/essay/:code', + element: ( + + + + ), + }, + { + path: '/test/:code', + element: ( + + + + ), + }, { path: '*', element: ( diff --git a/src/pages/EssayBuilderPage.tsx b/src/pages/EssayBuilderPage.tsx index e933fafd..8526619e 100644 --- a/src/pages/EssayBuilderPage.tsx +++ b/src/pages/EssayBuilderPage.tsx @@ -10,6 +10,7 @@ import Topbar from '../components/Layout/Topbar'; import Modal from '../components/ui/Modal'; import { useAuthoring, useClasses, useEssays, useStudents } from '../context/AppContext'; import { useToast } from '../hooks/useToast'; +import { useOnlineEssaySubmissions } from '../hooks/useOnlineEssaySubmissions'; import EssayAssignmentModal from '../components/Essay/EssayAssignmentModal'; import EssaySlipSheet from '../components/Essay/EssaySlipSheet'; import { encodeEssayAssignment } from '../utils/shareCode'; @@ -27,6 +28,10 @@ export default function EssayBuilderPage() { const { rubrics } = useAuthoring(); const { essayAssignments, essaySubmissions, addEssayAssignments, updateEssayGroup, addEssaySubmission } = useEssays(); + // Online hand-ins (essay_submissions via submit-essay) are NOT part of the + // hydrated `essaySubmissions` (which tracks only offline pasted-code imports), + // so the roster badge would stay Pending for a portal submission without this. + const onlineEssaySubmissions = useOnlineEssaySubmissions(); // The canonical group is every row saved under this exact teacherKey (the bulk // "Assign to Students" fan-out). `existing` is looked up directly rather than via @@ -372,9 +377,12 @@ export default function EssayBuilderPage() {
{rows.map((row) => { const student = students.find((s) => s.id === row.studentId); - const submitted = essaySubmissions.some( - (s) => s.teacherKey === row.teacherKey && s.assignmentStudentId === row.studentId - ); + const submitted = + essaySubmissions.some( + (s) => + s.teacherKey === row.teacherKey && s.assignmentStudentId === row.studentId + ) || + (onlineEssaySubmissions.get(row.teacherKey)?.has(row.studentId) ?? false); // A row can share this rubric without belonging to this page's // bulk group — e.g. assigned individually from GradeStudent. const isIndividual = row.teacherKey !== teacherKeyParam; diff --git a/src/pages/EssayListPage.tsx b/src/pages/EssayListPage.tsx index 77172cc9..c66da026 100644 --- a/src/pages/EssayListPage.tsx +++ b/src/pages/EssayListPage.tsx @@ -7,6 +7,7 @@ import Topbar from '../components/Layout/Topbar'; import { useAuthoring, useClasses, useEssays, useStudents } from '../context/AppContext'; import { ConfirmDialog } from '../components/ui/ConfirmDialog'; import { useConfirm } from '../hooks/useConfirm'; +import { useOnlineEssaySubmissions } from '../hooks/useOnlineEssaySubmissions'; import { sortByDisplayOrder, reorderDisplayOrder } from '../utils/displayOrder'; import { getCohortStudentIds, isAllCohorts, ALL_COHORTS } from '../utils/cohortAggregator'; import type { CohortFilter as CohortFilterValue } from '../types'; @@ -20,6 +21,7 @@ export default function EssayListPage() { const { rubrics } = useAuthoring(); const { essayAssignments, essaySubmissions, deleteEssayGroup, updateEssayGroup } = useEssays(); + const onlineEssaySubmissions = useOnlineEssaySubmissions(); const { confirm, dialogProps: confirmDialogProps } = useConfirm(); const [cohortFilter, setCohortFilter] = React.useState(ALL_COHORTS); @@ -104,11 +106,16 @@ export default function EssayListPage() { {groups.map(({ teacherKey, rows }, idx) => { const first = rows[0]; const rubric = rubrics.find((r) => r.id === first.rubricId); - const submittedCount = new Set( - essaySubmissions + // Offline (pasted-code) submissions come from the hydrated + // essaySubmissions; online hand-ins (essay_submissions via + // submit-essay) are added via useOnlineEssaySubmissions — + // otherwise the count would stay 0 for portal submissions. + const submittedCount = new Set([ + ...essaySubmissions .filter((s) => s.teacherKey === teacherKey) - .map((s) => s.assignmentStudentId) - ).size; + .map((s) => s.assignmentStudentId), + ...(onlineEssaySubmissions.get(teacherKey) ?? []), + ]).size; return ( >({}); - const { fetchEssayAssignmentByKey } = useEssays(); + const { fetchEssayAssignmentByKey, fetchEssaySubmissions } = useEssays(); const hasDb = dbStatus.isConnected && !!config?.supabaseUrl && !!config?.supabaseAnonKey; @@ -86,11 +88,34 @@ export default function LiveMonitorPage({ kind }: LiveMonitorPageProps) { return; } let cancelled = false; + const assignmentId = params.assignmentId; // narrowed: the guard above already checked it setEssayAssignmentLoading(true); - fetchEssayAssignmentByKey(params.assignmentId) + fetchEssayAssignmentByKey(assignmentId) .then((result) => { if (cancelled) return; setEssayAssignment(result ?? null); + if (!result) return; + // A submission may already be persisted (the student handed in before + // the monitor opened) — reflect it immediately instead of waiting for + // a live 'submitted' broadcast from a still-open student tab. + void fetchEssaySubmissions(assignmentId) + .then((rows) => { + if (cancelled || rows.length === 0) return; + setLiveStates((prev) => { + const current = prev[result.studentId] ?? emptyLiveState(result.studentId); + return { + ...prev, + [result.studentId]: { + ...current, + submitted: true, + lastUpdateAt: new Date().toISOString(), + }, + }; + }); + }) + .catch(() => { + // Non-fatal: the live broadcast path still covers an in-session submit. + }); }) .catch(() => { if (!cancelled) setEssayAssignment(null); @@ -101,7 +126,7 @@ export default function LiveMonitorPage({ kind }: LiveMonitorPageProps) { return () => { cancelled = true; }; - }, [kind, params.assignmentId, hasDb, fetchEssayAssignmentByKey]); + }, [kind, params.assignmentId, hasDb, fetchEssayAssignmentByKey, fetchEssaySubmissions]); // ── Test teacherKey lookup (Realtime channel name = the per-student teacherKey, // not a testId/studentId guess — see assignmentKeyFor below) ───────────────── @@ -231,6 +256,19 @@ export default function LiveMonitorPage({ kind }: LiveMonitorPageProps) { }; }); }) + .on('broadcast', { event: 'submitted' }, () => { + setLiveStates((prev) => { + const current = prev[row.studentId] ?? emptyLiveState(row.studentId); + return { + ...prev, + [row.studentId]: { + ...current, + submitted: true, + lastUpdateAt: new Date().toISOString(), + }, + }; + }); + }) .subscribe(); return channel; }); @@ -318,7 +356,17 @@ export default function LiveMonitorPage({ kind }: LiveMonitorPageProps) { ...snapshotAnswers, ] : row.persistedAnswers; - return { ...row, presence, flags, live, mergedAnswers }; + return { + ...row, + presence, + flags, + live, + mergedAnswers, + // Essays have no persisted student_tests row to derive a status from — + // the monitor learns about a hand-in via the student's 'submitted' + // broadcast or the essay_submissions check on mount. + status: row.status ?? (live.submitted ? 'submitted' : undefined), + }; }); // `tick` forces presence to be re-derived against the current time as heartbeats age. // eslint-disable-next-line react-hooks/exhaustive-deps diff --git a/src/pages/StudentEssayPage.tsx b/src/pages/StudentEssayPage.tsx index a1cd7d1e..c9b275c7 100644 --- a/src/pages/StudentEssayPage.tsx +++ b/src/pages/StudentEssayPage.tsx @@ -443,6 +443,16 @@ export default function StudentEssayPage() { } else { logEvent('action', 'essay_submitted', { teacherKey: assignment.teacherKey, wordCount }); adapter.clearStoredEmail(); + // Tell the teacher's live monitor the essay was handed in — the last + // broadcast before `setSubmitted(true)` below disables telemetry and + // tears the channel down. The monitor also re-checks essay_submissions + // on mount, so a reload after the fact still shows Submitted. Await + // the server ack so the live flip isn't silently dropped; on failure + // the persisted-row path still covers the monitor. + const ack = await telemetry.broadcast('submitted', { submittedAt: now, wordCount }); + if (ack !== 'ok') { + logEvent('error', 'submitted_broadcast_failed', { ack }, 'error'); + } } } diff --git a/src/pages/StudentTestPage.tsx b/src/pages/StudentTestPage.tsx index e7b9b411..5dcee2c9 100644 --- a/src/pages/StudentTestPage.tsx +++ b/src/pages/StudentTestPage.tsx @@ -497,6 +497,21 @@ export default function StudentTestPage() { testId: effectiveTestId, answerCount: testAnswers.length, }); + // Tell the teacher's live monitor the quiz was handed in — the last + // broadcast before `setSubmitted(true)` below disables telemetry and + // tears the channel down. Mirrors StudentEssayPage's 'submitted' + // broadcast; LiveMonitorPage listens for it on both kinds. The + // monitor also re-derives status from persisted student_tests rows, + // so a reload after the fact still shows Submitted. Await the + // server ack so the live flip isn't silently dropped; on failure the + // persisted-row path still covers the monitor. + const ack = await telemetry.broadcast('submitted', { + submittedAt, + answerCount: testAnswers.length, + }); + if (ack !== 'ok') { + logEvent('error', 'submitted_broadcast_failed', { ack }, 'error'); + } } } diff --git a/src/utils/onlineEssaySubmissions.ts b/src/utils/onlineEssaySubmissions.ts new file mode 100644 index 00000000..6fb94630 --- /dev/null +++ b/src/utils/onlineEssaySubmissions.ts @@ -0,0 +1,20 @@ +/** + * Pure mapping for online essay submissions (essay_submissions rows written by + * the submit-essay edge function): teacherKey (assignment id) -> set of student + * ids that handed in. Extracted from useOnlineEssaySubmissions so the keying + * logic is unit-testable (see src/__tests__/onlineEssaySubmissions.test.ts). + */ +export interface OnlineEssaySubmissionRow { + assignmentId: string; + studentId: string; +} + +export function keyOnlineEssaySubmissions(rows: OnlineEssaySubmissionRow[]): Map> { + const map = new Map>(); + for (const row of rows) { + const set = map.get(row.assignmentId) ?? new Set(); + if (row.studentId) set.add(row.studentId); + map.set(row.assignmentId, set); + } + return map; +} diff --git a/supabase/functions/submit-essay/email.ts b/supabase/functions/submit-essay/email.ts new file mode 100644 index 00000000..23256339 --- /dev/null +++ b/supabase/functions/submit-essay/email.ts @@ -0,0 +1,34 @@ +// Kept in its own pure module (no Deno imports) so the anonymous-session edge +// case below is unit-testable from the app's vitest suite — see +// src/__tests__/submitEssayEmail.test.ts. The edge function imports it with the +// explicit `.ts` extension (Deno convention); the vitest test imports it +// extensionless. + +export interface ResolvedStudentEmail { + email: string | null; + mismatch: boolean; +} + +/** + * Guard with a truthiness check, not `??`: a GoTrue anonymous session carries an + * EMPTY-STRING email claim (not null), and `authEmail ?? bodyEmail` would let that + * empty string win — silently rejecting every anonymous submission as "Missing + * required field: studentEmail". That regression is pinned by + * src/__tests__/submitEssayEmail.test.ts. + */ +export function resolveStudentEmail( + authEmail: string | null | undefined, + bodyEmail: string | null | undefined +): ResolvedStudentEmail { + // The auth record wins when it carries a real email (portal login sessions); + // otherwise the client-supplied value is used (anonymous sessions). A + // disagreement between the two is flagged so the caller can reject — one + // student must not be able to claim another's submission slot. + const normalizedAuth = authEmail ? authEmail : null; + const mismatch = + normalizedAuth !== null && + bodyEmail !== undefined && + bodyEmail !== null && + normalizedAuth.toLowerCase() !== bodyEmail.toLowerCase(); + return { email: normalizedAuth ?? bodyEmail ?? null, mismatch }; +} diff --git a/supabase/functions/submit-essay/index.ts b/supabase/functions/submit-essay/index.ts index a3d314b5..3abaede4 100644 --- a/supabase/functions/submit-essay/index.ts +++ b/supabase/functions/submit-essay/index.ts @@ -5,6 +5,7 @@ import { serve } from 'https://deno.land/std@0.168.0/http/server.ts'; import { createClient } from 'https://esm.sh/@supabase/supabase-js@2'; +import { resolveStudentEmail } from './email.ts'; const CORS = { 'Access-Control-Allow-Origin': '*', @@ -25,15 +26,13 @@ serve(async (req) => { if (!authHeader) return json({ error: 'Unauthorized' }, 401); // Service-role client for privileged DB/Storage operations - const admin = createClient( - Deno.env.get('SUPABASE_URL') ?? '', - Deno.env.get('SUPABASE_SERVICE_ROLE_KEY') ?? '', - ); + const admin = createClient(Deno.env.get('SUPABASE_URL') ?? '', Deno.env.get('SUPABASE_SERVICE_ROLE_KEY') ?? ''); // Verify the student's JWT - const { data: { user }, error: authErr } = await admin.auth.getUser( - authHeader.replace('Bearer ', ''), - ); + const { + data: { user }, + error: authErr, + } = await admin.auth.getUser(authHeader.replace('Bearer ', '')); if (authErr || !user) return json({ error: 'Invalid or expired token' }, 401); let body: { @@ -60,11 +59,13 @@ serve(async (req) => { // in their JWT, so fall back to the client-supplied value for those cases. // If the auth record has an email and the client sent a different one, reject the // request to prevent one student from claiming another's submission slot. - const authEmail = user.email ?? null; - if (authEmail && bodyEmail && authEmail.toLowerCase() !== bodyEmail.toLowerCase()) { + // The empty-string-anonymous-session guard lives in resolveStudentEmail (email.ts). + // `authEmail` is kept in scope: the anonymous-session roster check below needs it. + const authEmail = user.email ? user.email : null; + const { email: studentEmail, mismatch } = resolveStudentEmail(authEmail, bodyEmail); + if (mismatch) { return json({ error: 'Email mismatch: submitted email does not match your account' }, 403); } - const studentEmail = authEmail ?? bodyEmail ?? null; if (!studentEmail) { return json({ error: 'Missing required field: studentEmail' }, 400); @@ -108,7 +109,8 @@ serve(async (req) => { .select('data') .eq('id', assignment.student_id) .single(); - const rosterEmail: string | null = (studentRow?.data as Record | null)?.email as string ?? null; + const rosterEmail: string | null = + ((studentRow?.data as Record | null)?.email as string) ?? null; if (rosterEmail && rosterEmail.toLowerCase() !== studentEmail.toLowerCase()) { return json({ error: 'Email does not match the student record for this assignment' }, 403); } @@ -143,21 +145,22 @@ serve(async (req) => { if (uploadErr) return json({ error: `Storage upload failed: ${uploadErr.message}` }, 500); // Insert submission row (UNIQUE constraint is the final duplicate guard) - const { error: insertErr } = await admin - .from('essay_submissions') - .insert({ - id: submissionId, - assignment_id: assignmentId, - student_email: studentEmail ?? null, - student_user_id: user.id, - word_count: wordCount, - word_limit_status: wordLimitStatus, - submitted_at: new Date().toISOString(), - storage_path: storagePath, - }); + const { error: insertErr } = await admin.from('essay_submissions').insert({ + id: submissionId, + assignment_id: assignmentId, + student_email: studentEmail ?? null, + student_user_id: user.id, + word_count: wordCount, + word_limit_status: wordLimitStatus, + submitted_at: new Date().toISOString(), + storage_path: storagePath, + }); if (insertErr) { - await admin.storage.from('essays').remove([storagePath]).catch(() => {}); + await admin.storage + .from('essays') + .remove([storagePath]) + .catch(() => {}); if (insertErr.code === '23505') { return json({ error: 'You have already submitted this assignment' }, 409); } diff --git a/supabase/functions/submit-test/index.ts b/supabase/functions/submit-test/index.ts index ab56e18f..ffd54feb 100644 --- a/supabase/functions/submit-test/index.ts +++ b/supabase/functions/submit-test/index.ts @@ -19,6 +19,7 @@ import { serve } from 'https://deno.land/std@0.168.0/http/server.ts'; import { createClient } from 'https://esm.sh/@supabase/supabase-js@2'; +import { sanitizeAnswers, isAssignmentExpired, attemptPolicyFor } from './validation.ts'; const CORS = { 'Access-Control-Allow-Origin': '*', @@ -469,6 +470,12 @@ serve(async (req) => { } catch { return json({ error: 'Invalid request body' }, 400); } + // `req.json()` of the literal JSON `null` resolves to `null` (it only throws on + // malformed JSON), so destructuring it below would crash with a 500 instead of + // the 400 the validation block is meant to produce. + if (!body || typeof body !== 'object' || Array.isArray(body)) { + return json({ error: 'Invalid request body' }, 400); + } const { assignmentId, submissionId, answers, startedAt, submittedAt, events, sectionPath, levelPath } = body; if ( @@ -502,8 +509,8 @@ serve(async (req) => { // scoreAnswer() on the teacher-facing side (src/utils/testCalc.ts) treats as an // already-graded manual score and uses verbatim instead of auto-scoring. Reconstructing // the answer objects here (rather than trusting the spread) means a forged pointsEarned - // can never reach storage in the first place. - const sanitizedAnswers: MinimalAnswer[] = answers.map((a) => ({ questionId: a.questionId, response: a.response })); + // can never reach storage in the first place — see sanitizeAnswers in validation.ts. + const sanitizedAnswers: MinimalAnswer[] = sanitizeAnswers(answers); // Rate limit: at most 5 submissions per user per 60 seconds (mirrors submit-essay). const sixtySecondsAgo = new Date(Date.now() - 60_000).toISOString(); @@ -527,7 +534,7 @@ serve(async (req) => { if (assignErr || !assignment) return json({ error: 'Assignment not found' }, 404); - if (assignment.expires_at && new Date(assignment.expires_at) < new Date()) { + if (isAssignmentExpired(assignment.expires_at)) { return json({ error: 'Assignment deadline has passed' }, 403); } @@ -679,8 +686,7 @@ serve(async (req) => { // number of times on a 23505 conflict, re-counting each time, rather than failing a valid // retake outright — but only for practice mode; assessment-mode conflicts are always a // genuine duplicate submission and should fail immediately, as before. - const isPractice = assignment.mode === 'practice'; - const maxAttempts = isPractice ? 5 : 1; + const { isPractice, maxAttempts } = attemptPolicyFor(assignment.mode); for (let retry = 0; retry < maxAttempts; retry++) { let attemptNumber = 1; diff --git a/supabase/functions/submit-test/validation.ts b/supabase/functions/submit-test/validation.ts new file mode 100644 index 00000000..b1c9c88f --- /dev/null +++ b/supabase/functions/submit-test/validation.ts @@ -0,0 +1,43 @@ +// Shared pure validation helpers for submit-test. +// +// Kept in their own pure module (no Deno imports) so the guards below are +// unit-testable from the app's vitest suite — see +// src/__tests__/submitTestValidation.test.ts. The edge function imports it +// with the explicit `.ts` extension (Deno convention); the vitest test imports +// it extensionless. + +export interface MinimalAnswer { + questionId: string; + response: string; +} + +/** + * Rebuild answer objects as { questionId, response } only. + * + * Client input can carry arbitrary extra fields — most importantly + * pointsEarned, which scoreAnswer() on the teacher-facing side + * (src/utils/testCalc.ts) treats as an already-graded manual score and uses + * verbatim instead of auto-scoring. Reconstructing the answers here (rather + * than trusting the spread) means a forged pointsEarned can never reach + * storage in the first place. + */ +export function sanitizeAnswers(answers: MinimalAnswer[]): MinimalAnswer[] { + return answers.map((a) => ({ questionId: a.questionId, response: a.response })); +} + +/** Deadline guard — expired assignments (or rows with no deadline) reject/allow accordingly. */ +export function isAssignmentExpired(expiresAt: string | null | undefined, now: Date = new Date()): boolean { + // Inclusive: a submission at the exact expiration instant is already past the deadline. + return !!expiresAt && new Date(expiresAt) <= now; +} + +/** + * Attempt policy by assignment mode: practice-mode assignments allow retakes + * (up to 5 attempts, retried on 23505 conflicts), assessment-mode (or legacy + * rows with no mode) always allow exactly one — preserving the original + * one-submission-per-assignment guard. + */ +export function attemptPolicyFor(mode: string | null | undefined): { isPractice: boolean; maxAttempts: number } { + const isPractice = mode === 'practice'; + return { isPractice, maxAttempts: isPractice ? 5 : 1 }; +}