diff --git a/DB_SCHEMA_MAP.md b/DB_SCHEMA_MAP.md index ace7522..6960217 100644 --- a/DB_SCHEMA_MAP.md +++ b/DB_SCHEMA_MAP.md @@ -410,6 +410,7 @@ Default channels created by trigger on space INSERT: `general`, `announcements`, | 042 | Equipment double-booking P0 fix: `btree_gist` extension + `equipment_reservations_no_overlap` GiST EXCLUDE (equipment_id =, tstzrange(starts_at,ends_at,'[)') &&) WHERE status='reserved'. DB is the concurrency arbiter; reserveEquipment maps 23P01 to a friendly message. No new table/column | | 043 | `members_update` RLS gains a WITH CHECK mirroring its USING (defense-in-depth: a privileged caller can't mutate a member row into a space they don't administer). No new table/column | | 044 | `prevent_member_self_role_change` also blocks self-change of payment_status, payment_note, dues_paid_until, last_paid_at, last_payment_at, stripe_customer_id, joined_at (member could otherwise forge a dues-good signal via PostgREST). Service-client/privileged writes still bypass. No new table/column | +| 045 | `class_signup_tx` / `class_cancel_tx` SECURITY DEFINER functions: capacity-decision+insert and cancel+waitlist-promote run under a per-session `pg_advisory_xact_lock` (concurrent signups can't over-enroll; concurrent cancels can't double-promote). Mirrors the pure computeSignupStatus/pickPromotion rule; RPC is runtime authority. No new table/column | --- diff --git a/docs/DATABASE_SCHEMA.md b/docs/DATABASE_SCHEMA.md index 3ef7073..d1f31ce 100644 --- a/docs/DATABASE_SCHEMA.md +++ b/docs/DATABASE_SCHEMA.md @@ -725,6 +725,7 @@ migration is mirrored as a section in it. Tables/columns added: | 042 | Equipment double-booking P0 fix. `CREATE EXTENSION btree_gist` + `equipment_reservations_no_overlap` `EXCLUDE USING gist (equipment_id WITH =, tstzrange(starts_at,ends_at,'[)') WITH &&) WHERE status='reserved'`. Makes the DB the concurrency arbiter (the app-side check-then-insert was a TOCTOU); `reserveEquipment` maps the 23P01 exclusion violation to the friendly overlap message. No new table/column/policy/permission | | 043 | `members_update` RLS policy gains a `WITH CHECK` mirroring its `USING` (`user_id = auth.uid()` OR admin/board/treasurer in `space_id`). Defense-in-depth: the post-image is re-validated so a privileged caller cannot move a member row into a space they do not administer. No new table/column | | 044 | `prevent_member_self_role_change` extended to also block a non-privileged member changing their own `payment_status`, `payment_note`, `dues_paid_until`, `last_paid_at`, `last_payment_at`, `stripe_customer_id`, `joined_at` (RLS lets a member UPDATE their own row, so these were self-settable to forge a dues-good signal). Service-client (`auth.uid()` NULL) and privileged-on-another-member writes still bypass. No new table/column | +| 045 | `class_signup_tx(p_session_id,p_space_id,p_member_id)` + `class_cancel_tx(...)` SECURITY DEFINER, `search_path=public`. Each takes `pg_advisory_xact_lock(hashtext(session_id))` then does the capacity-decision+dup-check+insert (signup) or cancel+conditional waitlist-promote (cancel) atomically. Closes the concurrent over-enroll / double-promote races (dynamic capacity can't be a constraint). Mirrors `computeSignupStatus`/`pickPromotion`; the SQL is the runtime authority. No new table/column/policy | New enum: `comment_entity_type` = `forum_thread | proposal | incident | policy`. diff --git a/lib/actions/classes.ts b/lib/actions/classes.ts index ced43ba..29766f5 100644 --- a/lib/actions/classes.ts +++ b/lib/actions/classes.ts @@ -23,11 +23,8 @@ import { listSessionSignupsSchema, } from '@/lib/validations' import { - effectiveCapacity, - computeSignupStatus, canSignUp, signupFormEligibility, - pickPromotion, } from '@/lib/classes-logic' import { grantCertification } from './certifications' @@ -99,10 +96,6 @@ async function requirePermission( return { ok: true, supabase, member } } -function isUniqueViolation(message: string): boolean { - return /duplicate key value|already exists|unique constraint/i.test(message) -} - // ─── Class definitions (classes.manage) ────────────────────────────────────── export async function createClass(input: unknown) { @@ -515,52 +508,32 @@ export async function signUpForClass(input: unknown) { }) if (!formOk.ok) return { error: formOk.reason } - // Already signed up (non-cancelled)? - const { data: existing } = await admin - .from('class_signups') - .select('id, status') - .eq('session_id', v.data.sessionId) - .eq('member_id', targetMemberId) - .neq('status', 'cancelled') - .maybeSingle() - if (existing) { - return { error: targetMemberId === member.id ? 'You are already signed up for this session.' : 'That member is already signed up for this session.' } - } - - const cap = effectiveCapacity( - session.capacity as number | null, - cls?.capacity ?? null, - ) - // session was fetched scoped by member.space_id above, so counting by - // session_id alone is space-safe (do not loosen the session lookup). - const { count: registered } = await admin - .from('class_signups') - .select('id', { count: 'exact', head: true }) - .eq('session_id', v.data.sessionId) - .eq('status', 'registered') - const status = computeSignupStatus(cap, registered ?? 0) - - const { data, error } = await admin - .from('class_signups') - .insert({ - session_id: v.data.sessionId, - space_id: member.space_id, - member_id: targetMemberId, - status, - }) - .select('id') - .single() - if (error) { - if (isUniqueViolation(error.message)) { - return { error: 'You are already signed up for this session.' } + // Capacity decision + dup-check + insert run atomically inside + // class_signup_tx under a per-session advisory lock, so concurrent signups + // at the capacity boundary cannot over-enroll. computeSignupStatus stays + // the documented rule; the RPC is the runtime authority (045). + const { data: rpc, error } = await admin.rpc('class_signup_tx', { + p_session_id: v.data.sessionId, + p_space_id: member.space_id, + p_member_id: targetMemberId, + }) + if (error) return { error: error.message } + const row = (Array.isArray(rpc) ? rpc[0] : rpc) as + | { signup_id: string | null; signup_status: string | null; err: string | null } + | undefined + if (!row || row.err) { + if (row?.err === 'already') { + return { error: targetMemberId === member.id ? 'You are already signed up for this session.' : 'That member is already signed up for this session.' } } - return { error: error.message } + if (row?.err === 'no_session') return { error: 'Session not found' } + return { error: 'Could not complete signup. Please try again.' } } + const status = row.signup_status as string await logActivity(supabase, member, 'signed_up', 'class_session', v.data.sessionId) revalidatePath('/classes') revalidatePath('/me') - return { data: { id: data.id as string, status } } + return { data: { id: row.signup_id as string, status } } } export async function cancelMySignup(input: unknown) { @@ -573,58 +546,28 @@ export async function cancelMySignup(input: unknown) { if (!v.ok) return { error: v.error } const admin = createAdminClient() - // All admin-client queries below are additionally scoped by member.space_id: - // member.id can only hold a signup in its own space, but the writes must not - // be reachable cross-space even if a future edit loosens the member filter. - const { data: signup } = await admin - .from('class_signups') - .select('id, status') - .eq('space_id', member.space_id) - .eq('session_id', v.data.sessionId) - .eq('member_id', member.id) - .neq('status', 'cancelled') - .maybeSingle() - if (!signup) return { error: 'You are not signed up for this session.' } - - const wasRegistered = signup.status === 'registered' - const { error } = await admin - .from('class_signups') - .update({ status: 'cancelled' }) - .eq('id', signup.id) - .eq('space_id', member.space_id) + // Cancel + waitlist promotion run atomically inside class_cancel_tx under + // the same per-session advisory lock, so concurrent cancels cannot double- + // promote. pickPromotion stays the documented rule; the RPC is the runtime + // authority (045). p_space_id pins every write cross-space-safe. + const { data: rpc, error } = await admin.rpc('class_cancel_tx', { + p_session_id: v.data.sessionId, + p_space_id: member.space_id, + p_member_id: member.id, + }) if (error) return { error: error.message } - - // If a registered seat freed up, promote the earliest waitlisted member. - if (wasRegistered) { - const { data: session } = await admin - .from('class_sessions') - .select('capacity, classes(capacity)') - .eq('id', v.data.sessionId) - .eq('space_id', member.space_id) - .maybeSingle() - const cap = effectiveCapacity( - (session as { capacity: number | null } | null)?.capacity ?? null, - (session as { classes?: { capacity: number | null } | null } | null)?.classes?.capacity ?? null, - ) - const { data: all } = await admin - .from('class_signups') - .select('id, status, signed_up_at') - .eq('space_id', member.space_id) - .eq('session_id', v.data.sessionId) - .neq('status', 'cancelled') - const promoteId = pickPromotion( - (all ?? []) as Array<{ id: string; status: string; signed_up_at: string }>, - cap, - ) - if (promoteId) { - await admin.from('class_signups').update({ status: 'registered' }).eq('id', promoteId).eq('space_id', member.space_id) - } + const row = (Array.isArray(rpc) ? rpc[0] : rpc) as + | { cancelled_id: string | null; promoted_id: string | null; err: string | null } + | undefined + if (!row || row.err) { + if (row?.err === 'not_signed_up') return { error: 'You are not signed up for this session.' } + return { error: 'Could not cancel. Please try again.' } } await logActivity(supabase, member, 'cancelled_signup', 'class_session', v.data.sessionId) revalidatePath('/classes') revalidatePath('/me') - return { data: { id: signup.id } } + return { data: { id: row.cancelled_id as string } } } // ─── Instructor: attendees, attendance, completion ─────────────────────────── diff --git a/scripts/045_class_signup_concurrency.sql b/scripts/045_class_signup_concurrency.sql new file mode 100644 index 0000000..0d80d28 --- /dev/null +++ b/scripts/045_class_signup_concurrency.sql @@ -0,0 +1,95 @@ +-- ============================================================================= +-- 045: Class signup/cancel concurrency (P1, deeper-audit deferred) +-- ============================================================================= +-- signUpForClass did count-registered -> decide -> insert and cancelMySignup +-- did cancel -> recount -> promote, both non-atomic: concurrent signups at +-- the capacity boundary over-enrolled, and concurrent cancels double-promoted +-- waitlisters. Capacity is dynamic (session OR class), so a constraint can't +-- express it. These functions serialize all signup/cancel for a session with +-- a per-session advisory xact lock and do the decision + write atomically. +-- +-- The pure lib/classes-logic.ts (computeSignupStatus / pickPromotion) remains +-- the documented rule and is unit-tested; the SQL below MUST match it and is +-- the runtime authority. Idempotent: CREATE OR REPLACE. +-- ============================================================================= + +CREATE OR REPLACE FUNCTION public.class_signup_tx( + p_session_id uuid, p_space_id uuid, p_member_id uuid +) RETURNS TABLE(signup_id uuid, signup_status text, err text) + LANGUAGE plpgsql SECURITY DEFINER SET search_path = public AS $$ +DECLARE + v_cap int; v_reg int; v_status text; v_id uuid; +BEGIN + PERFORM pg_advisory_xact_lock(hashtext(p_session_id::text)); + + IF EXISTS ( + SELECT 1 FROM class_signups + WHERE session_id = p_session_id AND member_id = p_member_id + AND space_id = p_space_id AND status <> 'cancelled' + ) THEN + RETURN QUERY SELECT NULL::uuid, NULL::text, 'already'::text; RETURN; + END IF; + + SELECT COALESCE(s.capacity, c.capacity) INTO v_cap + FROM class_sessions s JOIN classes c ON c.id = s.class_id + WHERE s.id = p_session_id AND s.space_id = p_space_id; + IF NOT FOUND THEN + RETURN QUERY SELECT NULL::uuid, NULL::text, 'no_session'::text; RETURN; + END IF; + + SELECT count(*) INTO v_reg FROM class_signups + WHERE session_id = p_session_id AND status = 'registered'; + + v_status := CASE WHEN v_cap IS NULL OR v_reg < v_cap + THEN 'registered' ELSE 'waitlisted' END; + + INSERT INTO class_signups (session_id, space_id, member_id, status) + VALUES (p_session_id, p_space_id, p_member_id, v_status) + RETURNING id INTO v_id; + + RETURN QUERY SELECT v_id, v_status, NULL::text; +END; +$$; + +CREATE OR REPLACE FUNCTION public.class_cancel_tx( + p_session_id uuid, p_space_id uuid, p_member_id uuid +) RETURNS TABLE(cancelled_id uuid, promoted_id uuid, err text) + LANGUAGE plpgsql SECURITY DEFINER SET search_path = public AS $$ +DECLARE + v_sid uuid; v_status text; v_cap int; v_reg int; v_prom uuid; +BEGIN + PERFORM pg_advisory_xact_lock(hashtext(p_session_id::text)); + + SELECT id, status INTO v_sid, v_status FROM class_signups + WHERE session_id = p_session_id AND member_id = p_member_id + AND space_id = p_space_id AND status <> 'cancelled' + LIMIT 1; + IF NOT FOUND THEN + RETURN QUERY SELECT NULL::uuid, NULL::uuid, 'not_signed_up'::text; RETURN; + END IF; + + UPDATE class_signups SET status = 'cancelled' WHERE id = v_sid; + + v_prom := NULL; + IF v_status = 'registered' THEN + SELECT COALESCE(s.capacity, c.capacity) INTO v_cap + FROM class_sessions s JOIN classes c ON c.id = s.class_id + WHERE s.id = p_session_id AND s.space_id = p_space_id; + + SELECT count(*) INTO v_reg FROM class_signups + WHERE session_id = p_session_id AND status = 'registered'; + + IF v_cap IS NULL OR v_reg < v_cap THEN + SELECT id INTO v_prom FROM class_signups + WHERE session_id = p_session_id AND space_id = p_space_id + AND status = 'waitlisted' + ORDER BY signed_up_at ASC LIMIT 1; + IF v_prom IS NOT NULL THEN + UPDATE class_signups SET status = 'registered' WHERE id = v_prom; + END IF; + END IF; + END IF; + + RETURN QUERY SELECT v_sid, v_prom, NULL::text; +END; +$$; diff --git a/scripts/schema.sql b/scripts/schema.sql index 2c15693..ae66d3e 100644 --- a/scripts/schema.sql +++ b/scripts/schema.sql @@ -2243,6 +2243,79 @@ INSERT INTO public.space_role_permissions (space_id, subject, permission) SELECT id, 'board', 'classes.instruct' FROM public.spaces ON CONFLICT (space_id, subject, permission) DO NOTHING; +-- Atomic signup/cancel (equivalent to scripts/045). Per-session advisory +-- xact lock serializes the capacity decision + write so concurrent signups +-- can't over-enroll and concurrent cancels can't double-promote. Mirrors the +-- pure lib/classes-logic rule; this SQL is the runtime authority. +CREATE OR REPLACE FUNCTION public.class_signup_tx( + p_session_id uuid, p_space_id uuid, p_member_id uuid +) RETURNS TABLE(signup_id uuid, signup_status text, err text) + LANGUAGE plpgsql SECURITY DEFINER SET search_path = public AS $$ +DECLARE + v_cap int; v_reg int; v_status text; v_id uuid; +BEGIN + PERFORM pg_advisory_xact_lock(hashtext(p_session_id::text)); + IF EXISTS ( + SELECT 1 FROM class_signups + WHERE session_id = p_session_id AND member_id = p_member_id + AND space_id = p_space_id AND status <> 'cancelled' + ) THEN + RETURN QUERY SELECT NULL::uuid, NULL::text, 'already'::text; RETURN; + END IF; + SELECT COALESCE(s.capacity, c.capacity) INTO v_cap + FROM class_sessions s JOIN classes c ON c.id = s.class_id + WHERE s.id = p_session_id AND s.space_id = p_space_id; + IF NOT FOUND THEN + RETURN QUERY SELECT NULL::uuid, NULL::text, 'no_session'::text; RETURN; + END IF; + SELECT count(*) INTO v_reg FROM class_signups + WHERE session_id = p_session_id AND status = 'registered'; + v_status := CASE WHEN v_cap IS NULL OR v_reg < v_cap + THEN 'registered' ELSE 'waitlisted' END; + INSERT INTO class_signups (session_id, space_id, member_id, status) + VALUES (p_session_id, p_space_id, p_member_id, v_status) + RETURNING id INTO v_id; + RETURN QUERY SELECT v_id, v_status, NULL::text; +END; +$$; + +CREATE OR REPLACE FUNCTION public.class_cancel_tx( + p_session_id uuid, p_space_id uuid, p_member_id uuid +) RETURNS TABLE(cancelled_id uuid, promoted_id uuid, err text) + LANGUAGE plpgsql SECURITY DEFINER SET search_path = public AS $$ +DECLARE + v_sid uuid; v_status text; v_cap int; v_reg int; v_prom uuid; +BEGIN + PERFORM pg_advisory_xact_lock(hashtext(p_session_id::text)); + SELECT id, status INTO v_sid, v_status FROM class_signups + WHERE session_id = p_session_id AND member_id = p_member_id + AND space_id = p_space_id AND status <> 'cancelled' + LIMIT 1; + IF NOT FOUND THEN + RETURN QUERY SELECT NULL::uuid, NULL::uuid, 'not_signed_up'::text; RETURN; + END IF; + UPDATE class_signups SET status = 'cancelled' WHERE id = v_sid; + v_prom := NULL; + IF v_status = 'registered' THEN + SELECT COALESCE(s.capacity, c.capacity) INTO v_cap + FROM class_sessions s JOIN classes c ON c.id = s.class_id + WHERE s.id = p_session_id AND s.space_id = p_space_id; + SELECT count(*) INTO v_reg FROM class_signups + WHERE session_id = p_session_id AND status = 'registered'; + IF v_cap IS NULL OR v_reg < v_cap THEN + SELECT id INTO v_prom FROM class_signups + WHERE session_id = p_session_id AND space_id = p_space_id + AND status = 'waitlisted' + ORDER BY signed_up_at ASC LIMIT 1; + IF v_prom IS NOT NULL THEN + UPDATE class_signups SET status = 'registered' WHERE id = v_prom; + END IF; + END IF; + END IF; + RETURN QUERY SELECT v_sid, v_prom, NULL::text; +END; +$$; + -- ============================================================================= -- 20. Equipment registry + reservations