fix: close auth bypass in staff RPCs and tighten data-access boundaries

- set_staff_role/remove_staff guards were NULL-unsafe: an authenticated but
  unenrolled principal (possible on the shared auth pool) skipped the RAISE
  and could promote or delete staff; rewritten with the null-safe form.
- Report/prescription PDFs now require a clinical role and the invoice PDF a
  staff role, matching the screens that link to them; the role sets live in
  lib/db/roles.js and are shared by layouts, actions, and route handlers.
- Storage policies on bsk-checkup-media scoped to clinical roles.
- Billing RPCs serialize on a per-checkup advisory lock so a line rewrite can
  never commit after payment; mark_order_paid rejects deleted checkups and
  zero-line invoices.
- PDF customer lookups skip soft-deleted patients; invite rate limiter fails
  closed (SMTP spend is irrecoverable); patient search escapes LIKE wildcards.
This commit is contained in:
2026-08-18 20:41:45 +07:00
parent deec3d69c0
commit 8331ad5087
14 changed files with 323 additions and 17 deletions
+6 -3
View File
@@ -46,15 +46,18 @@ export async function inviteUserAction(_prevState, formData) {
}
// ── Rate limit (bound shared-project SMTP spend) ─────────────────────────────
// Keyed by admin id (server-derived, not spoofable). Fail OPEN on Redis
// outage — an invite is admin-gated already, so availability wins.
// Keyed by admin id (server-derived, not spoofable). Fail CLOSED on Redis
// outage: unlike a login retry, SMTP spend on the shared project cannot be
// undone, and an admin can simply retry once Redis is back. (The login
// limiter stays fail-open — availability wins there.)
try {
const { success: withinLimit } = await inviteLimiter.limit(session.user.id);
if (!withinLimit) {
return { status: "error", fieldErrors: {}, formError: t("tooManyRequests") };
}
} catch (err) {
console.warn("[invite] rate limiter unavailable, failing open:", err);
console.warn("[invite] rate limiter unavailable, failing closed:", err);
return { status: "error", fieldErrors: {}, formError: t("tooManyRequests") };
}
// ── Input validation ───────────────────────────────────────────────────────
@@ -6,6 +6,7 @@
import { getTranslations } from "next-intl/server";
import { getServerSession } from "@/lib/auth/get-server-session";
import { billingRoles } from "@/lib/db/roles";
import { createSupabaseServerClient } from "@/lib/supabase/server";
import { renderInvoicePdf } from "@/lib/pdf/invoice-document";
import { sumLineTotals } from "@/lib/billing/totals";
@@ -24,8 +25,11 @@ export async function GET(_req, { params }) {
const checkupId = Number(id);
if (!Number.isFinite(checkupId)) return new Response("Not found", { status: 404 });
// Billing gate: every staff role may print an invoice, patient may not.
const session = await getServerSession();
if (!session?.role) return new Response("Forbidden", { status: 403 });
if (!session?.role || !billingRoles.includes(session.role)) {
return new Response("Forbidden", { status: 403 });
}
const supabase = await createSupabaseServerClient();
@@ -45,6 +49,7 @@ export async function GET(_req, { params }) {
.from("customers")
.select("last_name, first_name")
.eq("id", c.customer_id)
.eq("deleted", false)
.maybeSingle(),
supabase
.from("order_items")
@@ -16,6 +16,7 @@ import { revalidatePath } from "next/cache";
import { redirect } from "@/i18n/navigation";
import { getServerSession } from "@/lib/auth/get-server-session";
import { createSupabaseServerClient } from "@/lib/supabase/server";
import { clinicalRoles } from "@/lib/db/roles";
import {
MedicineLinesSchema,
ServiceLinesSchema,
@@ -26,8 +27,8 @@ import {
/** @typedef {import('@/lib/billing/prescription-schema').PrescriptionSaveState} PrescriptionSaveState */
/** @typedef {import('@/lib/billing/prescription-schema').MarkPaidState} MarkPaidState */
/** @type {AppRole[]} */
const CLINICAL = ["admin", "receptionist", "doctor", "nurse"];
/** @type {readonly AppRole[]} */
const CLINICAL = clinicalRoles;
/** @type {AppRole[]} */
const BILLING = ["admin", "cashier"];
/** @param {AppRole | null | undefined} r */
@@ -7,6 +7,7 @@
import { getTranslations } from "next-intl/server";
import { getServerSession } from "@/lib/auth/get-server-session";
import { clinicalRoles } from "@/lib/db/roles";
import { createSupabaseServerClient } from "@/lib/supabase/server";
import { computeAge } from "@/lib/pdf/patient-info";
import { renderPrescriptionPdf } from "@/lib/pdf/prescription-document";
@@ -25,8 +26,12 @@ export async function GET(_req, { params }) {
const checkupId = Number(id);
if (!Number.isFinite(checkupId)) return new Response("Not found", { status: 404 });
// Clinical gate, matching checkups/layout.jsx — this PDF embeds diagnosis,
// medicines, and the patient's address.
const session = await getServerSession();
if (!session?.role) return new Response("Forbidden", { status: 403 });
if (!session?.role || !clinicalRoles.includes(session.role)) {
return new Response("Forbidden", { status: 403 });
}
const supabase = await createSupabaseServerClient();
@@ -46,6 +51,7 @@ export async function GET(_req, { params }) {
.from("customers")
.select("last_name, first_name, dob, gender, address_detail, province_code, ward_code")
.eq("id", c.customer_id)
.eq("deleted", false)
.maybeSingle(),
supabase
.from("order_items")
@@ -11,6 +11,7 @@
import { getTranslations } from "next-intl/server";
import { getServerSession } from "@/lib/auth/get-server-session";
import { clinicalRoles } from "@/lib/db/roles";
import { createSupabaseServerClient } from "@/lib/supabase/server";
import { CHECKUP_MEDIA_BUCKET, SIGNED_URL_TTL_SECONDS } from "@/lib/imaging/image-schema";
import { computeAge } from "@/lib/pdf/patient-info";
@@ -32,8 +33,12 @@ export async function GET(_req, { params }) {
const checkupId = Number(id);
if (!Number.isFinite(checkupId)) return new Response("Not found", { status: 404 });
// Clinical gate, matching checkups/layout.jsx — route handlers do not
// inherit layout gates, and this PDF embeds diagnosis/vitals/demographics.
const session = await getServerSession();
if (!session?.role) return new Response("Forbidden", { status: 403 });
if (!session?.role || !clinicalRoles.includes(session.role)) {
return new Response("Forbidden", { status: 403 });
}
const supabase = await createSupabaseServerClient();
@@ -54,6 +59,7 @@ export async function GET(_req, { params }) {
.from("customers")
.select("last_name, first_name, dob, gender")
.eq("id", c.customer_id)
.eq("deleted", false)
.maybeSingle(),
c.doctor_id
? supabase
+2 -1
View File
@@ -1,6 +1,7 @@
// WARNING: Do NOT add `'use cache'` — requireRole() reads cookies().
import { requireRole } from "@/lib/auth/require-role";
import { clinicalRoles } from "@/lib/db/roles";
/**
* @param {{ children: import("react").ReactNode, params: Promise<{ locale: string }> }} props
@@ -8,6 +9,6 @@ import { requireRole } from "@/lib/auth/require-role";
*/
export default async function CheckupsLayout({ children, params }) {
const { locale } = await params;
await requireRole(["admin", "receptionist", "doctor", "nurse"], locale);
await requireRole(clinicalRoles, locale);
return <>{children}</>;
}
+3 -2
View File
@@ -14,13 +14,14 @@ import { redirect } from "@/i18n/navigation";
import { getServerSession } from "@/lib/auth/get-server-session";
import { createSupabaseServerClient } from "@/lib/supabase/server";
import { CustomerSchema } from "@/lib/customers/customer-schema";
import { clinicalRoles } from "@/lib/db/roles";
/** @typedef {import('@/lib/db/roles').AppRole} AppRole */
/** @typedef {import('@/lib/customers/customer-schema').CustomerFormState} CustomerFormState */
/** @typedef {import('@/lib/customers/customer-schema').CustomerInput} CustomerInput */
/** @type {AppRole[]} */
const CLINICAL_ROLES = ["admin", "receptionist", "doctor", "nurse"];
/** @type {readonly AppRole[]} */
const CLINICAL_ROLES = clinicalRoles;
/** @param {AppRole | null | undefined} role */
const isClinical = (role) => !!role && CLINICAL_ROLES.includes(role);
/** @param {string} s */
+2 -1
View File
@@ -7,6 +7,7 @@
*/
import { requireRole } from "@/lib/auth/require-role";
import { clinicalRoles } from "@/lib/db/roles";
/**
* @param {{ children: import("react").ReactNode, params: Promise<{ locale: string }> }} props
@@ -14,6 +15,6 @@ import { requireRole } from "@/lib/auth/require-role";
*/
export default async function PatientsLayout({ children, params }) {
const { locale } = await params;
await requireRole(["admin", "receptionist", "doctor", "nurse"], locale);
await requireRole(clinicalRoles, locale);
return <>{children}</>;
}
+3 -2
View File
@@ -14,13 +14,14 @@ import { redirect } from "@/i18n/navigation";
import { getServerSession } from "@/lib/auth/get-server-session";
import { createSupabaseServerClient } from "@/lib/supabase/server";
import { RegisterCheckupSchema, SetQueueCounterSchema } from "@/lib/checkups/checkup-schema";
import { clinicalRoles } from "@/lib/db/roles";
/** @typedef {import('@/lib/db/roles').AppRole} AppRole */
/** @typedef {import('@/lib/checkups/checkup-schema').RegisterCheckupState} RegisterCheckupState */
/** @typedef {import('@/lib/checkups/checkup-schema').SetQueueCounterState} SetQueueCounterState */
/** @type {AppRole[]} */
const CLINICAL = ["admin", "receptionist", "doctor", "nurse"];
/** @type {readonly AppRole[]} */
const CLINICAL = clinicalRoles;
/** @param {AppRole | null | undefined} r */
const isClinical = (r) => !!r && CLINICAL.includes(r);
+2 -1
View File
@@ -1,6 +1,7 @@
// WARNING: Do NOT add `'use cache'` — requireRole() reads cookies().
import { requireRole } from "@/lib/auth/require-role";
import { clinicalRoles } from "@/lib/db/roles";
/**
* @param {{ children: import("react").ReactNode, params: Promise<{ locale: string }> }} props
@@ -8,6 +9,6 @@ import { requireRole } from "@/lib/auth/require-role";
*/
export default async function QueueLayout({ children, params }) {
const { locale } = await params;
await requireRole(["admin", "receptionist", "doctor", "nurse"], locale);
await requireRole(clinicalRoles, locale);
return <>{children}</>;
}
+2 -1
View File
@@ -1,6 +1,7 @@
// WARNING: Do NOT add `'use cache'` — requireRole() reads cookies().
import { requireRole } from "@/lib/auth/require-role";
import { clinicalRoles } from "@/lib/db/roles";
/**
* @param {{ children: import("react").ReactNode, params: Promise<{ locale: string }> }} props
@@ -8,6 +9,6 @@ import { requireRole } from "@/lib/auth/require-role";
*/
export default async function RemindersLayout({ children, params }) {
const { locale } = await params;
await requireRole(["admin", "receptionist", "doctor", "nurse"], locale);
await requireRole(clinicalRoles, locale);
return <>{children}</>;
}
+1 -1
View File
@@ -23,7 +23,7 @@ import { getServerSession } from "@/lib/auth/get-server-session";
/** @typedef {import("@/lib/auth/get-server-session").ServerSession} ServerSession */
/**
* @param {AppRole[]} allowed
* @param {readonly AppRole[]} allowed
* @param {string} locale
* @returns {Promise<ServerSession & { role: AppRole }>}
*/
+21
View File
@@ -27,6 +27,27 @@ export const appRoles = /** @type {const} */ ([
const _roleGuard = [...appRoles];
void _roleGuard; // prevent unused-variable lint warning
/**
* Roles allowed to see clinical data (diagnoses, vitals, imaging, printed
* medical reports). Shared by layouts AND route handlers so the PDF endpoints
* can never drift broader than the screens that link to them.
* @type {readonly AppRole[]}
*/
export const clinicalRoles = /** @type {const} */ (["admin", "receptionist", "doctor", "nurse"]);
/**
* Roles allowed to render billing documents (invoice PDF): every staff role.
* The `patient` role is excluded — invoices carry other patients' ids/names.
* @type {readonly AppRole[]}
*/
export const billingRoles = /** @type {const} */ ([
"admin",
"receptionist",
"doctor",
"nurse",
"cashier",
]);
/**
* Returns true if `s` is a valid `AppRole` value.
* Use as a type-narrowing guard when validating external input.
@@ -0,0 +1,258 @@
-- BSK — security hardening.
--
-- (a) set_staff_role / remove_staff used the NULL-unsafe guard
-- `IF bsk.current_role() <> 'admin'`: for an authenticated principal with
-- no bsk.app_users row, current_role() is NULL, `NULL <> 'admin'` is NULL,
-- the RAISE is skipped, and the mutation runs. On the shared auth pool an
-- unenrolled sibling-app user could promote an accomplice account to admin
-- or delete staff. Rewritten with the null-safe form used by every other
-- RPC in the schema.
-- (b) Paid-invoice lock was check-then-act with no lock: a cashier marking an
-- order paid concurrently with a clinical re-save of the lines could
-- commit a line rewrite after payment. save_prescription /
-- save_checkup_services / mark_order_paid now serialize on a per-checkup
-- advisory lock.
-- (c) mark_order_paid accepted soft-deleted checkups and zero-line invoices;
-- both now raise.
-- (d) Storage read policy on bsk-checkup-media admitted ANY enrolled role
-- (cashier/patient could enumerate clinical images) while the imaging UI
-- and the checkup_images table are clinical-gated. All four storage
-- policies now use the clinical role set.
-- (e) search_customers: escape LIKE wildcards in the query so a literal
-- '%'/'_' in a patient search does not act as a wildcard.
-- ─── (a) Null-safe admin guards on staff mutations ───────────────────────────
CREATE OR REPLACE FUNCTION bsk.set_staff_role(p_user_id uuid, p_role bsk.app_role)
RETURNS void
LANGUAGE plpgsql
VOLATILE
SECURITY DEFINER
SET search_path = bsk, pg_catalog
AS $$
DECLARE
v_role bsk.app_role := bsk.current_role();
v_target_role bsk.app_role;
BEGIN
IF v_role IS NULL OR v_role <> 'admin' THEN
RAISE EXCEPTION 'not authorized to change staff roles';
END IF;
IF p_user_id = auth.uid() THEN
RAISE EXCEPTION 'cannot change your own role';
END IF;
PERFORM pg_advisory_xact_lock(hashtext('bsk:staff')::bigint);
SELECT role INTO v_target_role FROM bsk.app_users WHERE user_id = p_user_id;
IF v_target_role IS NULL THEN
RAISE EXCEPTION 'user not enrolled';
END IF;
-- Demoting the last admin is forbidden.
IF v_target_role = 'admin' AND p_role <> 'admin'
AND (SELECT count(*) FROM bsk.app_users WHERE role = 'admin') <= 1 THEN
RAISE EXCEPTION 'cannot demote the last admin';
END IF;
UPDATE bsk.app_users SET role = p_role WHERE user_id = p_user_id;
END
$$;
CREATE OR REPLACE FUNCTION bsk.remove_staff(p_user_id uuid)
RETURNS void
LANGUAGE plpgsql
VOLATILE
SECURITY DEFINER
SET search_path = bsk, pg_catalog
AS $$
DECLARE
v_role bsk.app_role := bsk.current_role();
v_target_role bsk.app_role;
BEGIN
IF v_role IS NULL OR v_role <> 'admin' THEN
RAISE EXCEPTION 'not authorized to remove staff';
END IF;
IF p_user_id = auth.uid() THEN
RAISE EXCEPTION 'cannot remove yourself';
END IF;
PERFORM pg_advisory_xact_lock(hashtext('bsk:staff')::bigint);
SELECT role INTO v_target_role FROM bsk.app_users WHERE user_id = p_user_id;
IF v_target_role IS NULL THEN
RETURN; -- already gone
END IF;
IF v_target_role = 'admin'
AND (SELECT count(*) FROM bsk.app_users WHERE role = 'admin') <= 1 THEN
RAISE EXCEPTION 'cannot remove the last admin';
END IF;
DELETE FROM bsk.app_users WHERE user_id = p_user_id;
END
$$;
-- ─── (b)+(c) Serialized billing writes ───────────────────────────────────────
CREATE OR REPLACE FUNCTION bsk.save_prescription(p_checkup_id bigint, p_items jsonb)
RETURNS void
LANGUAGE plpgsql
VOLATILE
SECURITY DEFINER
SET search_path = bsk, pg_catalog
AS $$
DECLARE
v_role bsk.app_role := bsk.current_role();
BEGIN
IF v_role IS NULL OR v_role NOT IN ('admin', 'receptionist', 'doctor', 'nurse') THEN
RAISE EXCEPTION 'not authorized to save a prescription';
END IF;
-- Serialize with mark_order_paid so a line rewrite can never commit after
-- the invoice was paid (check-then-act below is safe under the lock).
PERFORM pg_advisory_xact_lock(hashtext('bsk:invoice:' || p_checkup_id::text)::bigint);
IF EXISTS (SELECT 1 FROM bsk.medicine_orders WHERE checkup_id = p_checkup_id AND payment_status = 'paid') THEN
RAISE EXCEPTION 'cannot modify a paid invoice';
END IF;
DELETE FROM bsk.order_items WHERE checkup_id = p_checkup_id;
INSERT INTO bsk.order_items (checkup_id, medicine_id, quantity, dosage, unit_price, line_total, notes)
SELECT
p_checkup_id,
(elem->>'medicine_id')::bigint,
(elem->>'quantity')::integer,
NULLIF(elem->>'dosage', ''),
m.sale_price,
m.sale_price * (elem->>'quantity')::integer,
NULLIF(elem->>'notes', '')
FROM jsonb_array_elements(COALESCE(p_items, '[]'::jsonb)) AS elem
JOIN bsk.medicines m ON m.id = (elem->>'medicine_id')::bigint;
INSERT INTO bsk.medicine_orders (checkup_id) VALUES (p_checkup_id)
ON CONFLICT (checkup_id) DO NOTHING;
END
$$;
CREATE OR REPLACE FUNCTION bsk.save_checkup_services(p_checkup_id bigint, p_items jsonb)
RETURNS void
LANGUAGE plpgsql
VOLATILE
SECURITY DEFINER
SET search_path = bsk, pg_catalog
AS $$
DECLARE
v_role bsk.app_role := bsk.current_role();
BEGIN
IF v_role IS NULL OR v_role NOT IN ('admin', 'receptionist', 'doctor', 'nurse') THEN
RAISE EXCEPTION 'not authorized to save checkup services';
END IF;
PERFORM pg_advisory_xact_lock(hashtext('bsk:invoice:' || p_checkup_id::text)::bigint);
IF EXISTS (SELECT 1 FROM bsk.medicine_orders WHERE checkup_id = p_checkup_id AND payment_status = 'paid') THEN
RAISE EXCEPTION 'cannot modify a paid invoice';
END IF;
DELETE FROM bsk.checkup_services WHERE checkup_id = p_checkup_id;
INSERT INTO bsk.checkup_services (checkup_id, service_id, quantity, unit_price, line_total)
SELECT
p_checkup_id,
(elem->>'service_id')::bigint,
(elem->>'quantity')::integer,
s.price,
s.price * (elem->>'quantity')::integer
FROM jsonb_array_elements(COALESCE(p_items, '[]'::jsonb)) AS elem
JOIN bsk.services s ON s.id = (elem->>'service_id')::bigint;
END
$$;
CREATE OR REPLACE FUNCTION bsk.mark_order_paid(p_checkup_id bigint, p_method text)
RETURNS void
LANGUAGE plpgsql
VOLATILE
SECURITY DEFINER
SET search_path = bsk, pg_catalog
AS $$
DECLARE
v_role bsk.app_role := bsk.current_role();
BEGIN
IF v_role IS NULL OR v_role NOT IN ('admin', 'cashier') THEN
RAISE EXCEPTION 'not authorized to mark an order paid';
END IF;
PERFORM pg_advisory_xact_lock(hashtext('bsk:invoice:' || p_checkup_id::text)::bigint);
IF NOT EXISTS (SELECT 1 FROM bsk.checkups WHERE id = p_checkup_id AND NOT deleted) THEN
RAISE EXCEPTION 'checkup does not exist or is deleted';
END IF;
-- A payment needs something to pay for; a zero-line invoice would only
-- surface later as reconciliation noise in the revenue export.
IF NOT EXISTS (SELECT 1 FROM bsk.order_items WHERE checkup_id = p_checkup_id)
AND NOT EXISTS (SELECT 1 FROM bsk.checkup_services WHERE checkup_id = p_checkup_id) THEN
RAISE EXCEPTION 'invoice has no line items';
END IF;
INSERT INTO bsk.medicine_orders (checkup_id, payment_status, payment_method, processed_by, paid_at)
VALUES (p_checkup_id, 'paid', p_method, auth.uid(), now())
ON CONFLICT (checkup_id) DO UPDATE
SET payment_status = 'paid',
payment_method = EXCLUDED.payment_method,
processed_by = EXCLUDED.processed_by,
paid_at = EXCLUDED.paid_at;
END
$$;
-- ─── (d) Clinical-only storage policies on bsk-checkup-media ────────────────
-- The table-level policies (checkup_images) and the imaging UI are already
-- clinical-gated; the object policies must not be broader.
DROP POLICY IF EXISTS bsk_checkup_media_select ON storage.objects;
DROP POLICY IF EXISTS bsk_checkup_media_insert ON storage.objects;
DROP POLICY IF EXISTS bsk_checkup_media_update ON storage.objects;
DROP POLICY IF EXISTS bsk_checkup_media_delete ON storage.objects;
CREATE POLICY bsk_checkup_media_select ON storage.objects FOR SELECT
USING (bucket_id = 'bsk-checkup-media'
AND bsk.current_role() IN ('admin', 'receptionist', 'doctor', 'nurse'));
CREATE POLICY bsk_checkup_media_insert ON storage.objects FOR INSERT
WITH CHECK (bucket_id = 'bsk-checkup-media'
AND bsk.current_role() IN ('admin', 'receptionist', 'doctor', 'nurse'));
CREATE POLICY bsk_checkup_media_update ON storage.objects FOR UPDATE
USING (bucket_id = 'bsk-checkup-media'
AND bsk.current_role() IN ('admin', 'receptionist', 'doctor', 'nurse'))
WITH CHECK (bucket_id = 'bsk-checkup-media'
AND bsk.current_role() IN ('admin', 'receptionist', 'doctor', 'nurse'));
CREATE POLICY bsk_checkup_media_delete ON storage.objects FOR DELETE
USING (bucket_id = 'bsk-checkup-media'
AND bsk.current_role() IN ('admin', 'receptionist', 'doctor', 'nurse'));
-- ─── (e) Escape LIKE wildcards in patient search ─────────────────────────────
CREATE OR REPLACE FUNCTION bsk.search_customers(q text)
RETURNS SETOF bsk.customers
LANGUAGE sql
STABLE
SET search_path = bsk, pg_catalog
AS $$
SELECT *
FROM bsk.customers
WHERE NOT deleted
AND (
COALESCE(q, '') = ''
OR bsk.immutable_unaccent(lower(last_name || ' ' || first_name))
LIKE '%' || replace(replace(replace(bsk.immutable_unaccent(lower(q)),
'\', '\\'), '%', '\%'), '_', '\_') || '%'
OR COALESCE(phone, '') LIKE '%' || replace(replace(replace(q,
'\', '\\'), '%', '\%'), '_', '\_') || '%'
)
ORDER BY last_name, first_name
LIMIT 50
$$;
COMMENT ON FUNCTION bsk.search_customers(text) IS
'Accent-insensitive patient search by name (or phone substring). SECURITY '
'INVOKER: runs under the caller RLS. Empty q returns the first 50 patients. '
'LIKE wildcards in q are escaped — a literal % or _ matches literally.';