Commit Graph

74 Commits

Author SHA1 Message Date
Flea Flicker 5c2e13863e fix(GRO-653): remove unused sessionId variable and and import
Fix lint errors flagged by QA:
- Remove unused `sessionId` variable from PATCH waitlist handler
- Remove unused `and` import from portal.ts

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Paperclip <noreply@paperclip.ing>
2026-04-16 10:18:56 +00:00
Flea Flicker 5024cc4896 feat(GRO-653): add portal session middleware and server-side audit logging
- Add validatePortalSession middleware that reads X-Impersonation-Session-Id header,
  queries impersonationSessions, and sets portalClientId + portalSessionId on the context
- Add portalAudit middleware that logs all portal requests to impersonationAuditLogs table
- Apply both middlewares to the portalRouter
- Replace all getClientIdFromSession() calls with c.get("portalClientId")
- Remove getClientIdFromSession() helper and inline session checks in waitlist routes
- Consistent session.expiry > new Date() check across all routes

Co-Authored-By: Paperclip <noreply@paperclip.ing>
2026-04-16 04:23:08 +00:00
groombook-cto[bot] 1ef740c361 Merge branch 'main' into feature/gro-622-security-hardening 2026-04-15 06:53:50 +00:00
groombook-cto[bot] d433c902b4 fix(GRO-637): invoice status transitions, tip-split validation, refund idempotency, and tip-split response format
* Fix invoice status transitions, tip-split validation, refund idempotency, and tip-split response format

- Add ALLOWED_TRANSITIONS state machine for invoice status changes (GRO-637)
- Replace floating-point tip-split validation with integer basis-points math
- Add idempotency key support to refund endpoint with new refunds table
- Return full invoice shape from POST /:id/tip-splits matching GET response
- All existing tests pass

Co-Authored-By: Paperclip <noreply@paperclip.ing>

* fix(invoices): wrap refund flow in transaction for idempotency safety

- Wrap idempotency check + processRefund() + db.insert() in db.transaction()
- This prevents duplicate Stripe refunds if the DB insert fails after Stripe processes the refund
- Add migration 0027_refunds for the refunds table (was missing)
- Removes out-of-scope changes from PR #278 (csrf.ts, appointmentGroups, appointments, book, groomingLogs, services, stripe-webhooks)

Fixes GRO-637 per CTO review

Co-Authored-By: Paperclip <noreply@paperclip.ing>

* fix(api): wire up CSRF middleware for protected routes

Register csrfMiddleware in the protected API routes after authMiddleware
and resolveStaffMiddleware to protect against CSRF attacks on state-
changing operations (POST, PUT, PATCH, DELETE).

Addresses CTO review feedback on PR #278.

* fix(api): remove CSRF middleware that breaks POST/PUT/PATCH/DELETE

The CSRF middleware requires x-csrf-token header but the frontend never
sends it, which would break all mutating operations with 403 errors.

CSRF protection should be implemented in a separate coordinated PR with
frontend changes.

Co-Authored-By: Paperclip <noreply@paperclip.ing>

---------

Co-authored-by: Paperclip <noreply@paperclip.ing>
Co-authored-by: Flea Flicker <flea-flicker@groombook.ai>
2026-04-15 06:04:38 +00:00
groombook-cto[bot] e118607fd6 Merge branch 'main' into fix/gro-627-scheduling-correctness 2026-04-15 05:00:12 +00:00
groombook-cto[bot] e1e13d5091 fix(GRO-636): input validation fixes for 5 API routes
Adds Zod validation across 5 API routes:

1. invoices GET / — query param validation (uuid, enum, int bounds)
2. book POST / — future-time refinement on startTime
3. appointments — recurrence series capped at 1 year
4. services — durationMinutes capped at 480 (8 hours)
5. stripe-webhooks — UUID validation on invoice IDs before DB lookup

Closes GRO-636

Co-Authored-By: Paperclip <noreply@paperclip.ing>
2026-04-15 04:26:20 +00:00
Flea Flicker 0ed87f9ed8 fix(api): add server-side pagination to churn risk query (GRO-641)
- Add SQL-level LIMIT/OFFSET pagination to churn risk query
- Add separate COUNT(*) subquery for total without fetching all rows
- Accept page and limit query params with sensible defaults and bounds
- Return page, limit, and churnRiskTotal in response

Co-Authored-By: Paperclip <noreply@paperclip.ing>
2026-04-15 00:12:01 +00:00
Flea Flicker f7b8b7e668 fix(GRO-634): atomic confirmation token in book.ts, correct RBAC error message
- Replace SELECT-then-UPDATE with atomic UPDATE ... WHERE token=? AND status='pending' RETURNING *
  to prevent confirmation token replay attacks (TOCTOU race condition)
- Fix requireRoleOrSuperUser() error message: swap the conditional branches so
  'Forbidden: super user privileges required' is returned when user lacks role,
  and 'Forbidden: role X is not permitted' when user is not superuser
- Add 'and' mock export to confirmation.test.ts and rbac.test.ts for new query patterns
- Update test expectations to match corrected error message semantics
2026-04-14 23:23:48 +00:00
Flea Flicker 1cce354413 fix(GRO-622): security hardening for auth, authorization, and token handling
- Remove placeholder secret fallback in AUTH_DISABLED mode (auth.ts)
- Make auth-provider setup atomic via DB transaction (setup.ts)
- Fix confirmation token replay with atomic UPDATE...WHERE (book.ts)
- Add strict CORS origin allowlist validation (index.ts)
- Validate OIDC discovery URL hostname matches issuer (auth.ts)
- Use timingSafeEqual for iCal token comparison (calendar.ts)
- Add in-memory rate limiting to setup endpoints (setup.ts)
- Keep RBAC error message correct (rbac.ts - already correct in main)

Co-Authored-By: Paperclip <noreply@paperclip.ing>
2026-04-14 23:23:48 +00:00
groombook-cto[bot] df07f2d6dc fix(GRO-635): implement groomer data isolation in appointmentGroups, groomingLogs + batherStaffId conflict check
- appointmentGroups: Hono<AppEnv>() + groomer isolation on all 5 endpoints
- groomingLogs: Hono<AppEnv>() + groomer isolation on GET, POST, DELETE with appointmentId preserved
- appointments: batherStaffId conflict checks in POST and PATCH handlers
- Non-groomer roles retain full access

Co-Authored-By: Paperclip <noreply@paperclip.ing>
2026-04-14 18:15:05 +00:00
Paperclip ab4b9fe6fc fix(GRO-638): appointment scheduling correctness and client deletion integrity
- Recurrence conflict checking: check ALL occurrences in recurrence loop
- Cascade update transaction safety: add conflict checking for shifted appointments
- Client deletion integrity: check for existing appointments before delete
- Email notification error handling: add retry wrapper (max 2 retries, 1s delay)
- Null guards on recurrence result: validate inserted after each insert

Co-Authored-By: Paperclip <noreply@paperclip.ing>
2026-04-14 14:31:52 +00:00
groombook-cto[bot] c438f5772c feat(GRO-607): Stripe Elements payment UI replacing mock flow
* GRO-605: Stripe SDK integration + payment service

Co-Authored-By: Paperclip <noreply@paperclip.ing>

* GRO-606: Add payment API endpoints (pay invoice, payment methods, refunds)

Co-Authored-By: Paperclip <noreply@paperclip.ing>

* feat(GRO-597): Stripe payment backend — schema, service, API, webhooks

Consolidates GRO-605, GRO-606, GRO-608 into a single clean PR:
- GRO-605: Stripe SDK integration + payment service
- GRO-606: Payment API endpoints (pay invoice, payment methods, refunds)
- GRO-608: Stripe webhook handler

Migration consolidation:
- Single 0026_stripe_payment.sql migration adds stripeCustomerId to clients
  and stripe_payment_intent_id, stripe_refund_id, payment_failure_reason to invoices
- Removed duplicate 0027_stripe_identifiers.sql

Co-Authored-By: Paperclip <noreply@paperclip.ing>

* GRO-607: Install Stripe frontend packages

Co-Authored-By: Paperclip <noreply@paperclip.ing>

* GRO-607: Add /portal/config endpoint + rename date field

Co-Authored-By: Paperclip <noreply@paperclip.ing>

* GRO-607: Replace mock payment flow with real Stripe Elements

Co-Authored-By: Paperclip <noreply@paperclip.ing>

* fix(GRO-607): Stripe Elements payment UI - lint/type fixes

Co-Authored-By: Paperclip <noreply@paperclip.ing>

* fix(GRO-607): remove unused eslint-disable directive in CustomerPortal

Co-Authored-By: Paperclip <noreply@paperclip.ing>

* fix(GRO-607): CTO review fixes — payment security and correctness

- Fix multi-invoice total calculation: use inArray() instead of eq()
  on single ID, sum all invoices not just first
- Add ownership check to payment method deletion: verify the payment
  method belongs to the authenticated Stripe customer before detaching
- Remove duplicate /config endpoint in portal.ts
- Fix webhook Stripe client: use getStripeClient() from payment service
  instead of constructing with WEBHOOK_SECRET
- Remove unnecessary body validator on /invoices/:id/pay route
- Export getStripeClient() for use by stripe-webhooks.ts
- Add inArray import to payment.ts

Co-Authored-By: Paperclip <noreply@paperclip.ing>

---------

Co-authored-by: Paperclip <noreply@paperclip.ing>
2026-04-14 08:27:03 +00:00
groombook-cto[bot] 15131b72f0 fix(GRO-574): add rate_limit table migration for Better Auth
Adds the missing rate_limit table that Better Auth v1.5.6 requires when rateLimit.storage is set to 'database'. Without this table, all auth endpoints return HTTP 500.

Also includes GRO-566: SKIP_OOBE env var to bypass setup wizard in dev/test.

cc @cpfarhood
2026-04-12 03:30:45 +00:00
Paperclip 1380d5a9d3 feat(GRO-564): Better Auth Phase 2 security hardening
- Add logout button to admin layout header (signOut from better-auth)
- AUTH_DISABLED production guard already present in auth.ts middleware
- Remove automatic email-based staff-user linking (security fix)
- Add PATCH /api/staff/:id/link-user endpoint for manual linking by admins
- Add rate limiting to Better Auth (10 req/min, database storage)

Co-Authored-By: Paperclip <noreply@paperclip.ing>
2026-04-11 22:53:00 +00:00
Flea Flicker 0fe10434e1 feat(invoices): add indexes, pagination, and client name enrichment
- Add database migration 0024 with indexes on invoices, invoice_line_items, and invoice_tip_splits
- Update Drizzle schema with index definitions for sync
- Add pagination (limit/offset) to GET /api/invoices with max 200 limit
- Add LEFT JOIN to include clientName in invoice list response
- Return { data: [...], total: N } response shape for pagination

Co-Authored-By: Paperclip <noreply@paperclip.ing>
2026-04-07 19:23:03 +00:00
Flea Flicker 8348f1c152 fix(api): resolve CI typecheck failures in GRO-485 fix
Fix type errors that caused CI Lint & Typecheck job to fail:
- setup.ts: replace unavailable isNull import with sql template tag
  (isNull not exported from @groombook/db; sql IS exported)
- setup.ts: add non-null assertion on newStaff after insert.returning()
- setup.test.ts: add sql mock template tag to @groombook/db mock
- setup.test.ts: fix evaluateCond to handle sql template tag type
- setup.test.ts: add type assertions for body.staff in OOBE regression tests
- setup.test.ts: fix dbStaffRows type casts in mock insert function

All 18 tests pass, full typecheck clean.

Co-Authored-By: Paperclip <noreply@paperclip.ing>
2026-04-05 20:05:53 +00:00
Flea Flicker fa18c41677 fix(api): exempt OOBE setup from staff middleware and auto-create staff (GRO-485)
Exempt POST /api/setup from resolveStaffMiddleware so OOBE users (with no
pre-existing staff record) can complete the out-of-box experience without
getting blocked by the "no staff record found" 403 error.

Changes:
- rbac.ts: add /api/setup to path exemption alongside /api/auth/
- setup.ts POST /: add find-or-create logic that:
  - Looks up existing staff by userId from JWT
  - Auto-links legacy staff records by email if userId is null
  - Creates a new staff record if none exists (OOBE case)
  - Returns 400 if JWT has no email and no staff record found
- setup.test.ts: add regression tests for all scenarios

Fixes GRO-485 (OOBE regression introduced by GRO-480).

Co-Authored-By: Paperclip <noreply@paperclip.ing>
2026-04-05 19:37:23 +00:00
Paperclip 6819bff2bf fix(api): use correct schema in POST /admin/auth-provider/test (GRO-454)
Switch the test endpoint from putAuthProviderSchema.omit({ clientSecret })
(which requires providerId, displayName, clientId, scopes) to the
minimal authProviderTestSchema (issuerUrl, internalBaseUrl?) that matches
what the Settings.tsx frontend actually sends.

Co-Authored-By: Paperclip <noreply@paperclip.ing>
2026-04-04 13:34:14 +00:00
Flea Flicker 1c502bb165 fix(api): wrap encryptSecret in try/catch to return proper JSON error
PUT /api/admin/auth-provider was returning HTTP 500 with an HTML error page
when BETTER_AUTH_SECRET was missing, because encryptSecret() throws an
unhandled error. This change wraps both the encryption step and the DB
transaction in try/catch blocks to return a proper JSON error response.

Also adds the missing authProviderConfig schema and encryptSecret crypto
helpers from the feat/gro-392-oobe-auth-provider-bootstrap branch.

Fixes: GRO-441

Co-Authored-By: Paperclip <noreply@paperclip.ing>
2026-04-04 00:02:11 +00:00
groombook-engineer[bot] 1f2a73cb44 fix(GRO-424): add try/catch around reinitAuth() calls
reinitAuth() can throw if BETTER_AUTH_SECRET is missing, causing
an unhandled rejection that returns an HTML error page instead of
JSON. Wrap both PUT and DELETE handlers in try/catch to return a
proper JSON error response.

Co-Authored-By: Paperclip <noreply@paperclip.ing>
2026-04-03 13:02:13 +00:00
groombook-engineer[bot] ae920aa347 fix(GRO-424): move reinitAuth to active router, add SSRF timeout, fix trailing slash
- Add reinitAuth() import and calls to routes/authProvider.ts (active router)
  instead of routes/admin/authProvider.ts (dead code, not imported)
- Add AbortSignal.timeout(10_000) to fetch in setup auth-provider/test endpoint
- Add .replace(/\/$/, "") to strip trailing slash from internalBaseUrl
- Delete dead routes/admin/authProvider.ts

Co-Authored-By: Paperclip <noreply@paperclip.ing>
2026-04-03 13:02:13 +00:00
groombook-engineer[bot] 38ce40ca39 feat(GRO-390): restart-on-save for auth provider config
Adds reinitAuth() for in-process auth re-init after PUT/DELETE on /api/admin/auth-provider. Sessions survive (DB-backed). Co-Authored-By: Paperclip <noreply@paperclip.ing>
2026-04-03 08:48:28 +00:00
groombook-engineer[bot] 624bb14ccb fix(GRO-391): remove clientSecret from test schema; use internalBaseUrl
Test connection was always 400 because testAuthProviderSchema required
clientSecret, but OIDC discovery only needs issuer/internal URLs.
Aligned admin test endpoint with setup.ts behavior:
- Drop providerId, clientId, clientSecret from schema
- Add optional internalBaseUrl; use it for discovery URL when set
- Frontend now sends issuerUrl + internalBaseUrl (when populated)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-04-03 07:43:44 +00:00
groombook-engineer[bot] 032ce584df fix(api): replace c.req.valid("json") with await c.req.json()
Replace zValidator-orphaned c.req.valid("json") calls with await c.req.json()
in the auth provider bootstrap and test endpoints per CTO review.

Co-Authored-By: Paperclip <noreply@paperclip.ing>
2026-04-03 07:04:00 +00:00
groombook-engineer[bot] 0953d6cb32 fix(api): move needsSetup guard before Zod parsing in setup endpoints
POST /api/setup/auth-provider and POST /api/setup/auth-provider/test
were returning 400 (Zod validation) instead of 403 when needsSetup
was false, because zValidator middleware ran before the route handler
body. Now manually parse the body after the needsSetup guard so 403
fires immediately for post-setup requests.

Co-Authored-By: Paperclip <noreply@paperclip.ing>
2026-04-03 02:36:29 +00:00
Barkley Trimsworth 075fd68cde fix(e2e): use lean schema for OIDC test endpoint; add trailing newline
Fix CTO review comments on GRO-392:

- POST /api/setup/auth-provider/test now uses authProviderTestSchema
  (only issuerUrl + internalBaseUrl) instead of full
  authProviderBootstrapSchema — clientSecret is not needed for OIDC
  discovery and was not being sent by the frontend handler
- POST /api/admin/auth-provider/test already uses omit() correctly;
  no change needed
- apps/api/src/routes/admin/authProvider.ts: added trailing newline

Co-Authored-By: Paperclip <noreply@paperclip.ing>
2026-04-03 02:08:52 +00:00
groombook-engineer[bot] 847d250c73 fix(api): remove unused decryptSecret import and eslint-disable directives
Fixes lint error exposed by merge with main (GRO-392 PR #214)

Co-Authored-By: Paperclip <noreply@paperclip.ing>
2026-04-03 01:35:51 +00:00
groombook-engineer[bot] 6307ce8bdc feat(api): auth provider CRUD endpoints + test-connection (GRO-388)
Implement admin API endpoints for managing auth provider configuration:

- GET  /api/admin/auth-provider         — get current config (secret redacted)
- PUT  /api/admin/auth-provider         — create or update provider config
- POST /api/admin/auth-provider/test    — validate via OIDC discovery endpoint
- DELETE /api/admin/auth-provider       — remove DB config (falls back to env vars)

All endpoints are gated by requireSuperUser(). The clientSecret is
AES-256-GCM encrypted before DB write and always redacted on return.
Test-connection fetches /.well-known/openid-configuration and returns
metadata on success or error detail on failure.

Includes 16 unit tests covering all endpoints and error paths.

Co-Authored-By: Paperclip <noreply@paperclip.ing>
2026-04-03 01:34:20 +00:00
groombook-engineer[bot] 802d12e885 fix(oobe): remove unused catch variable in setup.ts (GRO-392)
Co-Authored-By: Paperclip <noreply@paperclip.ing>
2026-04-03 01:32:56 +00:00
groombook-engineer[bot] 98508af01f fix(oobe): add test connection endpoint and fix EOF newline (GRO-392)
- Add POST /api/setup/auth-provider/test endpoint for OOBE test connection
- Guard with same !superUser check as bootstrap endpoint
- Update SetupWizard to call /api/setup/auth-provider/test instead of
  /api/admin/auth-provider/test (which requires auth session)
- Add trailing newline at EOF in setup.ts

Co-Authored-By: Paperclip <noreply@paperclip.ing>
2026-04-03 01:32:56 +00:00
groombook-engineer[bot] cd1b979747 feat(oobe): add conditional auth provider bootstrap step (GRO-392)
Backend:
- GET /api/setup/status now returns showAuthProviderStep, authConfigExists,
  and authEnvVarsSet to inform the frontend whether to show the step
- POST /api/setup/auth-provider: unauthenticated endpoint for first-time
  auth provider configuration during OOBE; guarded by needsSetup check
  (returns 403 after setup completes); encrypts clientSecret before storing

Frontend:
- SetupWizard fetches /api/setup/status on mount to determine if the
  auth provider step is needed (fresh install with no DB config and no
  OIDC env vars)
- When needed, inserts the Auth Provider step after Welcome, before
  Business Name; includes full form with Test Connection button
- Endpoint is POST /api/admin/auth-provider/test for connection testing

Co-Authored-By: Paperclip <noreply@paperclip.ing>
2026-04-03 01:32:56 +00:00
groombook-engineer[bot] 63c829bfd3 feat(api): auth provider CRUD endpoints + test-connection (GRO-388)
Implements admin API endpoints for managing auth provider configuration.
All gated by requireSuperUser().

Endpoints:
- GET /api/admin/auth-provider - returns config with clientSecret=redacted
- PUT /api/admin/auth-provider - encrypts clientSecret before DB write
- POST /api/admin/auth-provider/test - validates OIDC discovery endpoint
- DELETE /api/admin/auth-provider - removes DB config

Fixes CTO review findings:
- PUT uses db.transaction() for atomic upsert (was non-atomic delete+insert)
- Rebased on latest main (drops stale GRO-404/406 commits)
- Added EOF newlines to authProvider.ts and authProvider.test.ts

Unit tests with 9 passing test cases covering all endpoints and RBAC.

Co-Authored-By: Paperclip <noreply@paperclip.ing>
2026-04-02 21:50:40 +00:00
groombook-engineer[bot] 7b208bbedb Merge main into feat/gro-395-demo-assets
Resolve conflict in settings.ts: keep S3 logo migration imports.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-04-02 13:32:14 +00:00
groombook-engineer[bot] 004e23f8bc fix(api): enforce requireSuperUser on settings PATCH and fix dev-mode auth bypass (#206)
* fix(api): enforce requireSuperUser on settings PATCH and fix dev-mode auth bypass

- Add requireSuperUser() middleware to PATCH /api/admin/settings route
  to ensure only super users can modify business settings

- Fix dev-mode (AUTH_DISABLED=true) force-set of isSuperUser:true
  for all staff records in resolveStaffMiddleware. Now preserves
  actual database value with isSuperUser ?? false fallback.
  This prevents non-super-users (e.g., receptionists) from
  bypassing RBAC checks in dev mode.

- Fix test data: RECEPTIONIST and GROOMER now correctly have
  isSuperUser: false (was incorrectly inheriting true from MANAGER)

- Add 7 new tests for requireSuperUser middleware covering:
  - Super user access allowed
  - Non-super-user receptionist blocked with 403
  - Non-super-user groomer blocked with 403
  - Unresolved staff record returns 403
  - Receptionist cannot grant super user via PATCH
  - JSON error response format

Co-Authored-By: Paperclip <noreply@paperclip.ing>

* fix(api): remove dead code in rbac test

Remove unused `app` variable from 'returns 403 when staff record is
not resolved' test - the test uses `testApp` instead.

Co-Authored-By: Paperclip <noreply@paperclip.ing>

---------

Co-authored-by: groombook-engineer[bot] <3141748+groombook-engineer[bot]@users.noreply.github.com>
Co-authored-by: Paperclip <noreply@paperclip.ing>
2026-04-02 12:57:56 +00:00
groombook-engineer[bot] 28ed09b33d fix(api): add 404 guard when logo confirm returns no rows
The returning() on the update query can produce undefined when zero
rows match. Added explicit 404 if updated is falsy.

Co-Authored-By: Paperclip <noreply@paperclip.ing>
2026-04-02 12:44:14 +00:00
groombook-engineer[bot] 74571d9f2b feat(demo): expand demo pet images and seed data with diverse breed showcase
Generated 16 diverse pet images for demo site using MiniMax image generation:
- Multiple dog breeds (Golden Retriever, Poodle, Labrador, Shih Tzu, Cocker Spaniel, Schnauzer, Maltese, Dachshund, Pomeranian)
- Professional grooming styles and poses
- Studio lighting for quality showcase

Updated seed.ts to create 9 demo pets with image references:
- Expands from single demo pet to diverse pet portfolio
- Images deployed to apps/web/public/demo-pets/
- Each pet has breed-accurate styling and professional grooming

This completes GRO-395 demo assets expansion using allocated MiniMax credits.

Co-Authored-By: Paperclip <noreply@paperclip.ing>
2026-04-02 12:15:21 +00:00
groombook-engineer[bot] dff11d02be fix(db): seed staff_id FK fix (GRO-369)
Fixes staff_id FK violation on re-seed: move TRUNCATE before staff upsert and add id to onConflictDoUpdate set clause.

Co-Authored-By: Paperclip <noreply@paperclip.ing>
2026-04-01 14:19:49 +00:00
groombook-engineer[bot] 2fd86d0636 fix(api): use UTC in reports date helpers — reports show no data (GRO-302)
Fixes GRO-302. Reports page showed no data because date range helpers used local time instead of UTC for boundary calculations.
2026-03-31 19:47:30 +00:00
Barkley Trimsworth 4fb0c7b14d fix(api): use valid staff ID for dev-session impersonation
The hardcoded DEV_STAFF_ID (all zeros) did not exist in the staff
table, causing a foreign-key violation and 500 error. Now falls back
to the demo-manager (KNOWN_STAFF_ID from seed) or any active staff
record instead.

Co-Authored-By: Paperclip <noreply@paperclip.ing>
2026-03-30 18:09:09 +00:00
Barkley Trimsworth 51431c7bc1 fix(portal): wire dev client login to portal session
When a client user selects their account from the dev login selector,
the portal previously had no way to establish an authenticated session —
it only checked for a ?sessionId= URL param (used by the real staff
impersonation flow). This caused the portal to always show "Hi, Guest".

Changes:
- POST /api/portal/dev-session: new endpoint (auth-disabled only) that
  creates an impersonation session for a given clientId, using a fixed
  dev staff ID to avoid conflicts with the one-active-session-per-staff
  rule in the real impersonation flow. Sessions are long-lived (24h).
- CustomerPortal: on mount, after checking for ?sessionId=, also check
  for a dev client user in localStorage and call /api/portal/dev-session
  to obtain a session. This mirrors the real impersonation flow so all
  existing portal API calls (which require X-Impersonation-Session-Id)
  work without modification.

cc @cpfarhood

Co-Authored-By: Paperclip <noreply@paperclip.ing>
2026-03-30 17:07:49 +00:00
Barkley Trimsworth 853c55fd04 fix(staff): count only active super users in last-super-user guardrail
Add active=true filter to all 3 superUserCount queries in staff.ts
(revoke, deactivate, delete) so inactive super users aren't counted,
preventing false positives when checking the last-super-user guardrail.

Co-Authored-By: Paperclip <noreply@paperclip.ing>
2026-03-30 14:50:50 +00:00
groombook-ci[bot] 0e1c36a407 feat(staff): super user grant/revoke UI + last-super-user guardrail (GRO-206)
Backend:
- PATCH /api/staff/:id now accepts optional isSuperUser field
- Only super users can change isSuperUser (403 otherwise)
- Revoke (isSuperUser=false) blocked if target is last super user (400)
- Deactivate (active=false) blocked if target is last super user (400)
- DELETE /:id blocked if target is last super user (400)
- New GET /api/staff/me returns current authenticated staff record

Frontend (Staff.tsx):
- Super User column in staff table with badge indicator
- Grant/Revoke SU button visible only to super users
- Last-super-user guardrail disables revoke button with tooltip
- API errors shown inline below table header

Co-Authored-By: Paperclip <noreply@paperclip.ing>
2026-03-30 13:33:10 +00:00
groombook-engineer[bot] 4746a63292 feat(portal): replace mock data with real session-driven API calls (#152)
Closes GRO-205. Reviewed and approved by CTO (The Dogfather) and QA (Lint Roller). cc @cpfarhood
2026-03-29 07:08:35 +00:00
groombook-engineer[bot] 6872342d8f fix(auth): resolve redirect loop and mount Better-Auth as sub-app (#144)
## Changes
- Replace toNodeHandler with auth.handler(c.req.raw) sub-app mount for Hono compatibility
- Add /api/auth/ path skip in authMiddleware and resolveStaffMiddleware
- Add OIDC_INTERNAL_BASE env var for split-horizon (hairpin NAT) URL resolution
- Replace render-time signIn.social() with LoginPage component (fixes redirect loop)
- Change auth-client baseURL to relative (empty string) for deployed environments
- Add POST /api/portal/appointments/:id/reschedule endpoint with session auth
- Add RescheduleFlow modal, PetForm component, and wire Dashboard/Appointments UI

## CTO Note
Auth fix is P0-critical. Portal mock data (UAT blocker) predates this PR and is tracked separately in GRO-218.

Co-Authored-By: Paperclip <noreply@paperclip.ing>
2026-03-28 22:10:50 +00:00
groombook-engineer[bot] ad1f32eb8f feat(auth): replace OIDC/jose with Better-Auth (#136)
* feat(db): add Better-Auth schema tables (GRO-118)

Add user, session, account, and verification tables required by
Better-Auth's Drizzle adapter. Add nullable userId FK on staff to
link business identity to auth identity. Fix test fixtures and
factory to include the new column.

Co-Authored-By: Paperclip <noreply@paperclip.ing>

* feat(api): mount Better-Auth handler at /api/auth/** (GRO-118)

- Import toNodeHandler from better-auth/node and auth from ./lib/auth.js
- Mount Better-Auth HTTP handler before auth middleware block
- Handles OAuth callbacks, sign-in/sign-out, session management
- Supports GET/POST/PUT/PATCH/DELETE/OPTIONS methods

Co-Authored-By: Paperclip <noreply@paperclip.ing>

* feat(api): replace JWT auth with Better-Auth session validation (GRO-118)

- Replace jose/jwtVerify with auth.api.getSession()
- Session token validated via cookie/header, DB-backed
- jwtPayload.sub now = Better-Auth user ID (not OIDC sub)
- Dev mode bypass preserved; production guard against AUTH_DISABLED preserved
- rbac.ts and tests updated in subsequent tasks

Co-Authored-By: Paperclip <noreply@paperclip.ing>

* feat(api): update resolveStaffMiddleware for Better-Auth userId (GRO-118)

- Remove JwtPayload import; use inline type in AppEnv
- Production and dev mode lookups now use staff.userId (not oidcSub)
- Backward compat: jwtPayload.sub now = Better-Auth user ID

Co-Authored-By: Paperclip <noreply@paperclip.ing>

* chore(api): remove jose and openid-client deps (GRO-118)

- Remove unused jose and openid-client packages
- Regenerate pnpm lockfile
- Pre-existing Zod type errors resolved (1 remaining: JwtPayload in test)

Co-Authored-By: Paperclip <noreply@paperclip.ing>

* fix(api): remove stale JwtPayload import from impersonation test (GRO-118)

auth.ts no longer exports JwtPayload — replace with inline type.

Co-Authored-By: Paperclip <noreply@paperclip.ing>

* test(api): update RBAC tests for Better-Auth userId (GRO-128)

- Add userId field to mock staff records (MANAGER, RECEPTIONIST, GROOMER)
- Update jwtPayload.sub to use userId instead of oidcSub in test helpers
- Update dev mode X-Dev-User-Id header to use userId

Co-Authored-By: Paperclip <noreply@paperclip.ing>

* chore(api): upgrade zod to v4 with v3 compat layer (GRO-131)

- Bump zod from ^3.24.1 to ^4.3.6
- Bump @hono/zod-validator from ^0.4.3 to ^0.7.6
- Update all 12 route files to import from "zod/v3" compat layer

Co-Authored-By: Paperclip <noreply@paperclip.ing>

* feat(api): add Better-Auth configuration (GRO-118)

Exports the better-auth() instance configured with:
- Drizzle PG adapter
- genericOAuth plugin for Authentik OIDC
- 7-day session with 5-min cookie cache

Co-Authored-By: Paperclip <noreply@paperclip.ing>

* feat(web): install Better-Auth client and create config (GRO-118)

- Add better-auth to apps/web/package.json dependencies
- Create apps/web/src/lib/auth-client.ts with createAuthClient config
- Export signIn, signOut, useSession from the client
- Add vite-env.d.ts for Vite client types

Co-Authored-By: Paperclip <noreply@paperclip.ing>

* feat(web): use Better-Auth session state in App.tsx (GRO-126)

Add useSession hook to check Better-Auth session for production auth.
Redirect to Authentik sign-in when no session in production mode.
Dev mode flow (DevLoginSelector) preserved.

Co-Authored-By: Paperclip <noreply@paperclip.ing>

* fix(web): scope devFetch interceptor to dev mode only (GRO-127)

* fix(api): validate BETTER_AUTH_SECRET and fix lockfile specifier (GRO-118)

- Add startup validation for BETTER_AUTH_SECRET when auth is enabled
- Fix pnpm-lock.yaml typescript specifier mismatch (^5.9.3 → ^5.7.3)

Co-Authored-By: Paperclip <noreply@paperclip.ing>

* fix(web): mock authDisabled=true in App.test.tsx to fix CI failures

App.test.tsx "App navigation" tests were failing because the beforeEach
set authDisabled=false (production mode), which triggers the Better Auth
useSession() path. Since useSession() was not mocked in tests, the
component rendered null instead of the admin nav.

Now uses authDisabled=true + dev user in localStorage for those tests,
bypassing the Better Auth dependency while still testing the nav render.

Also removes duplicate App.test.js (compiled artifact).

Co-Authored-By: Paperclip <noreply@paperclip.ing>

* fix(e2e): set authDisabled=true in fixtures to bypass Better Auth

The App.tsx production auth path calls signIn.social() when
authDisabled=false, causing E2E tests to render blank. The fixtures
must mock authDisabled=true so the dev login selector is used instead.

Co-Authored-By: Paperclip <noreply@paperclip.ing>

* fix(e2e): add dev/config, dev/users, and branding mocks to navigation.spec.ts

Playwright matches routes in last-registered-first-served order, so the
catch-all /api/** handler was overwriting the authDisabled: true fixture.
Added specific handlers before the catch-all to ensure auth config,
user list, and branding responses are properly shaped.

Co-Authored-By: Paperclip <noreply@paperclip.ing>

* fix(web): gate DevLoginSelector on API authDisabled, not import.meta.env.DEV

Move the DevLoginSelector rendering check from import.meta.env.DEV to the
API-driven authDisabled state, after the loading guard. Simplify the redirect
condition to remove the now-redundant pathname exception.

Fixes E2E login tests that were failing because DevLoginSelector was never
rendered in Docker production builds where import.meta.env.DEV is false.

Co-Authored-By: Paperclip <noreply@paperclip.ing>

* fix(db): add missing migration journal entries 0012-0017

Co-Authored-By: Paperclip <noreply@paperclip.ing>

* fix(web): import App.tsx (not App.js) in App.test.tsx (#137)

* fix(web): mock /api/auth/get-session in Dev login selector test

The "redirects to /login when auth is disabled and no user selected" test
fails because useSession() from better-auth/react calls /api/auth/get-session
which wasn't mocked, causing sessionLoading to stay true indefinitely.

Co-Authored-By: Paperclip <noreply@paperclip.ing>

* fix(web): import App.tsx (not App.js) in test to get authDisabled bypass

The Dev login selector test was importing the compiled App.js instead of
the source App.tsx. App.js has different logic (uses import.meta.env.DEV
instead of API-based authDisabled) and doesn't implement the
sessionLoading bypass needed for tests to pass.

Also applied the rawSession/rawSessionLoading refactor in App.tsx that
bypasses useSession result when authDisabled=true.

Co-Authored-By: Paperclip <noreply@paperclip.ing>

* fix(web): use extensionless import for App in test

The `.tsx` extension in the import path is not allowed without
`allowImportingTsExtensions` (TS5097). Use extensionless `../App`
which resolves correctly via moduleResolution: "bundler".

Co-Authored-By: Paperclip <noreply@paperclip.ing>

---------

Co-authored-by: Paperclip <noreply@paperclip.ing>

* fix(auth): dev login resolve staff by id, not userId

Co-Authored-By: Paperclip <noreply@paperclip.ing>

* fix(rbac): fallback lookup for staff records predating Better-Auth userId (#140)

GRO-153: /api/staff returned 403 for all staff because resolveStaffMiddleware
looked up by staff.userId (Better-Auth ID) but dev login sent staff.id (PK),
and existing staff records had userId=NULL.

Changes:
- resolveStaffMiddleware: try userId first, fall back to staff.id (dev mode)
- resolveStaffMiddleware: try userId first, fall back to oidcSub (production)
- GET /api/dev/users: include userId field for DevLoginSelector
- DevLoginSelector: send userId (not staff.id) as X-Dev-User-Id
- Migration 0018: backfill userId for known demo staff

Co-authored-by: groombook-engineer[bot] <groombook-engineer@users.noreply.github.com>
Co-authored-by: Paperclip <noreply@paperclip.ing>
Co-authored-by: Barkley Trimsworth <barkley@groombook.farh.net>

* fix(rbac): allow all staff roles to READ /api/staff

GRO-156 follow-up: RBAC middleware was blocking groomer/receptionist
from GET /api/staff. The QA review found 403 with "role groomer is not
permitted" after PR #140 deployment.

Fix: split the /staff/* guard — GET requests allow all roles
(groomer, receptionist, manager); write operations remain manager-only.

Co-Authored-By: Paperclip <noreply@paperclip.ing>

---------

Co-authored-by: Paperclip <noreply@paperclip.ing>
Co-authored-by: groombook-engineer[bot] <269742240+groombook-engineer[bot]@users.noreply.github.com>
Co-authored-by: Flea Flicker <flea-flicker@paperclip.ing>
Co-authored-by: groombook-engineer[bot] <groombook-engineer@users.noreply.github.com>
Co-authored-by: Barkley Trimsworth <barkley@groombook.farh.net>
2026-03-28 03:50:45 +00:00
groombook-engineer[bot] 9eb0c3d151 fix(gro66): E2E selector fix + groomer isolation + portal confirm/cancel
* Implement confirm/cancel in customer portal (GRO-50)

Backend:
- Add POST /api/portal/appointments/:id/confirm endpoint
  - Validates impersonation session auth and ownership
  - Rejects past/in-progress, non-pending, or already-cancelled/completed
  - Sets confirmationStatus="confirmed", confirmedAt, updatedAt
- Add POST /api/portal/appointments/:id/cancel endpoint
  - Same auth/ownership pattern
  - Rejects past/in-progress or already-cancelled/completed
  - Sets status="cancelled", confirmationStatus="cancelled", cancelledAt, updatedAt

Frontend (Appointments.tsx):
- Add confirmationStatus field to Appointment type and mock data
- Add ConfirmationSection component: shows status badge + confirm button
- Add CancelAppointmentButton: wires to cancel API with loading/error state
- Wire existing Cancel button to CancelAppointmentButton
- Show confirmation status badge in expanded view for upcoming appointments

Co-Authored-By: Paperclip <noreply@paperclip.ing>

* feat(gro-48): row-level data scoping for groomer role (RBAC Phase 2)

Filter query results at the route handler level when staff role is groomer:

- GET /api/appointments: WHERE staffId = groomer OR batherStaffId = groomer
- GET /api/appointments/🆔 403 if not assigned to groomer (as staff or bather)
- GET /api/clients: Clients with ≥1 appointment for this groomer (via exists subquery)
- GET /api/clients/🆔 403 if no appointment linkage
- GET /api/pets: Pets owned by groomer-linked clients (via exists subquery)
- GET /api/pets/:petId: 403 if no appointment linkage

Managers and receptionists: no change.

Added exists to @groombook/db exports (was missing from re-export).
Added groomerIsolation unit tests for role guard and filter logic.

Co-Authored-By: Paperclip <noreply@paperclip.ing>

* fix(gro-50): add portal confirm/cancel tests and fix ConfirmationSection state

- Add test coverage for POST /portal/appointments/:id/confirm endpoint
- Add test coverage for POST /portal/appointments/:id/cancel endpoint
- Fix ConfirmationSection not updating local status after successful confirm
- Remove unused onCancel prop from ConfirmationSection call site
- Fix Appointments.test.tsx missing confirmationStatus field

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* test(gro-50): add ConfirmationSection UI component tests

Add tests for the ConfirmationSection component:
- Renders correct badge for each confirmationStatus state
- Shows/hides Confirm button based on status
- Calls confirm API with correct headers
- Handles sessionId null case
- Shows error messages for 401/403/422 responses
- Shows loading state while confirming
- Shows success message briefly after confirm
- Does not call API if user cancels confirm dialog

Co-Authored-By: Paperclip <noreply@paperclip.ing>

* fix(gro-48): address QA review feedback — staffRow?.role and portal TS guards

- appointments.ts: use staffRow?.role (consistent with clients.ts/pets.ts)
  to handle undefined staff context safely
- portal.ts: add null guards on .returning() results for confirm and cancel
  endpoints (TS18048: 'updated' is possibly undefined)
- All 188 tests passing; TypeScript typecheck clean

Co-Authored-By: Paperclip <noreply@paperclip.ing>

* fix(gro66): use specific selector for banner visibility assertion

Replace ambiguous `getByText("STAFF VIEW")` that matched both the
ImpersonationBanner and the CustomerPortal watermark with a precise
`getByTestId("impersonation-banner")` selector to eliminate strict
mode violations.

Co-Authored-By: Paperclip <noreply@paperclip.ing>

* fix(gro-66): add missing afterEach to vitest import

Co-Authored-By: Paperclip <noreply@paperclip.ing>

* fix(gro-48): add icalToken to MANAGER mock after rebase

After rebasing onto origin/main (which added icalToken to the staff
schema via GRO-107), the MANAGER mock in groomerIsolation.test.ts was
missing the new required field. Added icalToken: null to the MANAGER
constant. factories.ts is clean (no duplicate icalToken after rebase).

Co-Authored-By: Paperclip <noreply@paperclip.ing>

* fix(gro-47): add non-null assertions on Drizzle RETURNING results

Drizzle's update().returning() types the array element as T | undefined.
After the if (!appt) guard, updated is still typed as possibly undefined
because RETURNING can succeed with no rows. Add ! assertions since
we already guard with the existence check.

Co-Authored-By: Paperclip <noreply@paperclip.ing>

---------

Co-authored-by: Flea Flicker <fleaflicker@groombook.ai>
Co-authored-by: Paperclip <noreply@paperclip.ing>
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Co-authored-by: Flea Flicker <flea-flicker@paperclip.ing>
2026-03-27 14:23:19 +00:00
groombook-engineer[bot] 6539eb4554 feat: iCal calendar feed (GRO-107)
feat: iCal calendar feed (GRO-107)

Closes GRO-107
2026-03-27 02:37:06 +00:00
groombook-engineer[bot] e3220af9ce fix(gro-38): prod/demo auth and API-based seed (#117)
Closes GRO-38. Adds POST /api/admin/seed (manager-only, gated by SEED_KNOWN_USERS_ONLY) and separates dev vs prod seeding paths. Reviewed and approved by CTO and QA.

Co-Authored-By: Paperclip <noreply@paperclip.ing>
2026-03-26 20:51:08 +00:00
groombook-engineer[bot] d0b4baf5aa feat: customer-facing appointment notes (GRO-106) (#109)
* feat: add customer-facing appointment notes (GRO-106)

- Migration 0014: add customer_notes column to appointments
- Schema update: add customerNotes field to appointments table
- Factory update: include customerNotes in buildAppointment
- Portal route: PATCH /api/portal/appointments/:id/notes
  - Ownership validation via impersonation session
  - Future-only validation (no edits after start)
  - 500 character limit
- Register portal router in index.ts

Co-Authored-By: Paperclip <noreply@paperclip.ing>

* Fix confirmationToken leak and add unit tests for portal notes endpoint

- Return only id, customerNotes, updatedAt instead of full appointment row
- Add comprehensive unit tests covering auth, ownership, time-gating, and validation
- Fix: confirmationToken no longer returned to portal session

Co-Authored-By: Paperclip <noreply@paperclip.ing>

* feat: add customer notes UI to portal and staff views (GRO-178)

- Add customerNotes field to Appointment type
- Add read-only customer notes display in staff appointment detail modal
- Add customer notes textarea with save, char counter (500 max), and disabled state
- Wire up PATCH /api/portal/appointments/:id/notes in portal UI
- Update mockData with customerNotes field

Co-Authored-By: Paperclip <noreply@paperclip.ing>

* fix: address QA review feedback - null check and portal route auth

- Add null check after db.update().returning() in portal notes endpoint
- Move portal router registration before auth middleware so clients can access it
- Remove unused ENDED_SESSION variable from test file

Co-Authored-By: Paperclip <noreply@paperclip.ing>

* fix(portal): address QA review - isUpcoming time parsing and session header

- Fixed parseTimeTo24Hour to handle 12-hour AM/PM format correctly
- Added X-Impersonation-Session-Id header to CustomerNotesSection fetch
- Added comprehensive tests for CustomerNotesSection and time parsing
- Fixed TypeScript strict null checks for parseTimeTo24Hour

Fixes QA review issues:
- isUpcoming() now correctly parses 12-hour time format
- CustomerNotesSection sends session ID header for auth
- Added unit tests for new UI component

Co-Authored-By: Paperclip <noreply@paperclip.ing>

* fix: thread sessionId as prop instead of sessionStorage

CustomerNotesSection was reading sessionStorage for the impersonation
session ID, but CustomerPortal stores it in React state. Pass sessionId
as a prop through AppointmentsSection and AppointmentCard instead.

Also update tests to pass sessionId prop and add test for null sessionId
case.

Co-Authored-By: Paperclip <noreply@paperclip.ing>

---------

Co-authored-by: Scrubs McBarkley <scrubs@groombook.app>
Co-authored-by: Paperclip <noreply@paperclip.ing>
Co-authored-by: groombook-cto[bot] <269737991+groombook-cto[bot]@users.noreply.github.com>
2026-03-26 08:24:21 +00:00
Scrubs McBarkley d1ab91adfa feat: appointment confirmation and cancellation (GH #98, GRO-153)
Add customer confirmation/cancellation flow for appointments:

- DB migration (0013): add confirmation_status, confirmed_at, cancelled_at,
  confirmation_token to appointments table with index on token column
- schema.ts + factories.ts + types: expose new columns and ConfirmationStatus type
- GET /api/book/confirm/:token — tokenized confirm via email link (redirects)
- GET /api/book/cancel/:token — tokenized cancel via email link, single-use token
- POST /api/appointments/:id/confirm — portal/staff confirm endpoint
- POST /api/appointments/:id/cancel — portal/staff cancel endpoint
- Reminder emails now include Confirm/Cancel CTA buttons with tokenized links
- Reminder service generates confirmation token if missing before sending
- Staff calendar shows confirmation status indicator on appointment cards
  and in the detail modal (confirmed ✓ / customer cancelled ✗)
- /booking/confirmed, /booking/cancelled, /booking/error redirect pages
- 23 new unit tests covering all new endpoints and edge cases

Co-Authored-By: Paperclip <noreply@paperclip.ing>
2026-03-24 16:02:58 +00:00