Merge branch 'main' into fix/gro-640-n-plus-one-email

fix(GRO-689): only validate authorizationUrl hostname, add OIDC_INTERNAL_BASE in dev

apps/api/src/__tests__/clients.test.ts

@@ -195,10 +195,11 @@ describe("POST /clients", () => {
     expect(insertedValues[0]!.name).toBe("Charlie");
   });
 
-  it("creates a client with required fields", async () => {
+  it("creates a client with name and email", async () => {
     const res = await jsonRequest("POST", "/clients", { name: "Dana", email: "dana@example.com" });
     expect(res.status).toBe(201);
     expect(insertedValues[0]!.name).toBe("Dana");
+    expect(insertedValues[0]!.email).toBe("dana@example.com");
   });
 
   it("rejects empty name", async () => {

apps/api/src/lib/auth.ts

@@ -204,15 +204,11 @@ export async function initAuth(): Promise<void> {
         const userInfoUrl = discovery.userinfo_endpoint;
         if (authzUrl && tokenUrl && userInfoUrl) {
           const authzUrlObj = new URL(authzUrl);
-          const tokenUrlObj = new URL(tokenUrl);
-          const userInfoUrlObj = new URL(userInfoUrl);
-          if (
-            authzUrlObj.hostname !== issuerHostname ||
-            tokenUrlObj.hostname !== issuerHostname ||
-            userInfoUrlObj.hostname !== issuerHostname
-          ) {
+          // Only validate authorizationUrl hostname against issuer — token/userinfo
+          // may legitimately use internal hostnames (OIDC_INTERNAL_BASE) for server-to-server calls.
+          if (authzUrlObj.hostname !== issuerHostname) {
             throw new Error(
-              `[FATAL] OIDC discovery URL hostname mismatch: expected '${issuerHostname}' but got '${authzUrlObj.hostname}', '${tokenUrlObj.hostname}', or '${userInfoUrlObj.hostname}'. This may indicate a man-in-the-middle attack.`
+              `[FATAL] OIDC discovery URL hostname mismatch: expected '${issuerHostname}' but got '${authzUrlObj.hostname}'. This may indicate a man-in-the-middle attack.`
             );
           }
           oidcConfig = {

apps/api/src/routes/appointments.ts

@@ -338,44 +338,35 @@ async function sendConfirmationEmail(
   db: ReturnType<typeof getDb>,
   appt: typeof appointments.$inferSelect
 ): Promise<void> {
-  const [client] = await db
-    .select({ name: clients.name, email: clients.email, emailOptOut: clients.emailOptOut })
-    .from(clients)
-    .where(eq(clients.id, appt.clientId))
+  const [row] = await db
+    .select({
+      clientName: clients.name,
+      clientEmail: clients.email,
+      clientEmailOptOut: clients.emailOptOut,
+      petName: pets.name,
+      serviceName: services.name,
+      groomerName: staff.name,
+    })
+    .from(appointments)
+    .innerJoin(clients, eq(clients.id, appointments.clientId))
+    .innerJoin(pets, eq(pets.id, appointments.petId))
+    .innerJoin(services, eq(services.id, appointments.serviceId))
+    .leftJoin(staff, eq(staff.id, appointments.staffId))
+    .where(eq(appointments.id, appt.id))
     .limit(1);
 
-  if (!client || !client.email || client.emailOptOut) return;
+  if (!row) return;
+  const { clientName, clientEmail, clientEmailOptOut, petName, serviceName, groomerName } = row;
 
-  const [pet] = await db
-    .select({ name: pets.name })
-    .from(pets)
-    .where(eq(pets.id, appt.petId))
-    .limit(1);
-
-  const [service] = await db
-    .select({ name: services.name })
-    .from(services)
-    .where(eq(services.id, appt.serviceId))
-    .limit(1);
-
-  let groomerName: string | null = null;
-  if (appt.staffId) {
-    const [groomer] = await db
-      .select({ name: staff.name })
-      .from(staff)
-      .where(eq(staff.id, appt.staffId))
-      .limit(1);
-    groomerName = groomer?.name ?? null;
-  }
-
-  if (!pet || !service) return;
+  if (!clientEmail || clientEmailOptOut) return;
+  if (!petName || !serviceName) return;
 
   const sent = await sendEmail(
-    buildConfirmationEmail(client.email, {
-      clientName: client.name,
-      petName: pet.name,
-      serviceName: service.name,
-      groomerName,
+    buildConfirmationEmail(clientEmail, {
+      clientName,
+      petName,
+      serviceName,
+      groomerName: groomerName ?? null,
       startTime: appt.startTime,
     })
   );

apps/api/src/routes/clients.ts

@@ -8,7 +8,7 @@ export const clientsRouter = new Hono<AppEnv>();
 
 const createClientSchema = z.object({
   name: z.string().min(1).max(200),
-  email: z.string().email().optional(),
+  email: z.string().email(),
   phone: z.string().max(50).optional(),
   address: z.string().max(500).optional(),
   notes: z.string().max(2000).optional(),

packages/db/migrations/0028_sms_reminders.sql

@@ -1,12 +1,15 @@
--- SMS opt-in fields for clients
-ALTER TABLE "clients" ADD COLUMN "sms_opt_in" boolean NOT NULL DEFAULT false;
-ALTER TABLE "clients" ADD COLUMN "sms_consent_date" timestamp;
-ALTER TABLE "clients" ADD COLUMN "sms_opt_out_date" timestamp;
-ALTER TABLE "clients" ADD COLUMN "sms_consent_text" text;
+-- SMS opt-in fields for clients (idempotent)
+ALTER TABLE "clients" ADD COLUMN IF NOT EXISTS "sms_opt_in" boolean NOT NULL DEFAULT false;
+ALTER TABLE "clients" ADD COLUMN IF NOT EXISTS "sms_consent_date" timestamp;
+ALTER TABLE "clients" ADD COLUMN IF NOT EXISTS "sms_opt_out_date" timestamp;
+ALTER TABLE "clients" ADD COLUMN IF NOT EXISTS "sms_consent_text" text;
 
--- Add channel column to reminder_logs with default 'email'
-ALTER TABLE "reminder_logs" ADD COLUMN "channel" text NOT NULL DEFAULT 'email';
+-- Add channel column to reminder_logs with default 'email' (idempotent)
+ALTER TABLE "reminder_logs" ADD COLUMN IF NOT EXISTS "channel" text NOT NULL DEFAULT 'email';
 
--- Drop the old unique constraint and recreate with channel
-ALTER TABLE "reminder_logs" DROP CONSTRAINT "reminder_logs_appointment_id_reminder_type_unique";
+-- Drop old unique constraints if they exist (idempotent)
+ALTER TABLE "reminder_logs" DROP CONSTRAINT IF EXISTS "reminder_logs_appointment_id_reminder_type_key";
+ALTER TABLE "reminder_logs" DROP CONSTRAINT IF EXISTS "reminder_logs_appointment_id_reminder_type_unique";
+
+-- Add new unique constraint with channel
 ALTER TABLE "reminder_logs" ADD CONSTRAINT "reminder_logs_appointment_id_reminder_type_channel_unique" UNIQUE ("appointment_id", "reminder_type", "channel");

packages/db/src/seed.ts

@@ -462,6 +462,37 @@ async function seedKnownUsers() {
     }
   }
 
+  // ── Staff: UAT Groomer Personas (SEED_UAT_GROOMER_EMAILS + SEED_UAT_GROOMER_NAMES) ──
+  const groomerEmails = process.env.SEED_UAT_GROOMER_EMAILS?.split(",").map((e) => e.trim()).filter(Boolean) ?? [];
+  const groomerNames = process.env.SEED_UAT_GROOMER_NAMES?.split(",").map((n) => n.trim()).filter(Boolean) ?? [];
+  const groomerCount = Math.min(groomerEmails.length, groomerNames.length);
+  for (let i = 0; i < groomerCount; i++) {
+    const email = groomerEmails[i]!;
+    const name = groomerNames[i]!;
+    // Use deterministic IDs in the 00000000-0000-0000-0000-000000000005+ range
+    const staffId = `00000000-0000-0000-0000-${String(5 + i).padStart(12, "0")}`;
+    const [existingGroomer] = await db
+      .select()
+      .from(schema.staff)
+      .where(eq(schema.staff.email, email))
+      .limit(1);
+
+    if (existingGroomer) {
+      console.log(`✓ Staff groomer '${existingGroomer.name}' already exists — skipping`);
+    } else {
+      await db.insert(schema.staff).values({
+        id: staffId,
+        name,
+        email,
+        oidcSub: email,
+        role: "groomer",
+        isSuperUser: false,
+        active: true,
+      });
+      console.log(`✓ Created staff groomer '${name}' (${email})`);
+    }
+  }
+
   // ── Services: idempotent upsert using name as unique key ─────────────────────
   // UNIQUE constraint on services.name (migration 0020) must exist first.
   // Uses b0000001-... IDs to match main seed servicesDef for same-named services.
@@ -629,6 +660,31 @@ async function seed() {
     console.log(`✓ Upserted admin staff '${adminName}' (${adminEmail})`);
   }
 
+  // ── UAT Groomer Personas (SEED_UAT_GROOMER_EMAILS + SEED_UAT_GROOMER_NAMES) ──
+  const groomerEmails = process.env.SEED_UAT_GROOMER_EMAILS?.split(",").map((e) => e.trim()).filter(Boolean) ?? [];
+  const groomerNames = process.env.SEED_UAT_GROOMER_NAMES?.split(",").map((n) => n.trim()).filter(Boolean) ?? [];
+  const groomerCount = Math.min(groomerEmails.length, groomerNames.length);
+  for (let i = 0; i < groomerCount; i++) {
+    const email = groomerEmails[i]!;
+    const name = groomerNames[i]!;
+    const staffId = `00000000-0000-0000-0000-${String(5 + i).padStart(12, "0")}`;
+    await db.insert(schema.staff)
+      .values({
+        id: staffId,
+        name,
+        email,
+        oidcSub: email,
+        role: "groomer",
+        isSuperUser: false,
+        active: true,
+      })
+      .onConflictDoUpdate({
+        target: schema.staff.email,
+        set: { id: staffId, name, role: "groomer", isSuperUser: false, active: true },
+      });
+    console.log(`✓ Upserted groomer '${name}' (${email})`);
+  }
+
   // ── Services ──
   // Upsert services using name as unique key. With deterministic IDs in
   // servicesDef and TRUNCATE clearing downstream tables first, this is