* 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>
* 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>
* fix(oobe): remove unused catch variable in setup.ts (GRO-392)
Co-Authored-By: Paperclip <noreply@paperclip.ing>
* 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>
* fix(api): requireRoleOrSuperUser for /admin/* routes (GRO-412)
Fix bug where super users granted via Staff UI were blocked from
admin routes because requireRole("manager") checked role before
isSuperUser. Changed to requireRoleOrSuperUser("manager") so
super users bypass the manager-role check.
Also adds 7 unit tests for requireRoleOrSuperUser middleware
covering: manager access, super user bypass, non-super-user
blocking, and multi-role scenarios.
Co-Authored-By: Paperclip <noreply@paperclip.ing>
* 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>
* fix(tests): use main's authProvider tests after rebase conflict resolution
The rebase introduced incompatible test code from the pre-merge GRO-388
commit. Replaced with the canonical test file from main to ensure tests
pass and reflect the actual router implementation.
Co-Authored-By: Paperclip <noreply@paperclip.ing>
* fix(api): remove duplicate authProviderRouter import and route registration
Rebase introduced duplicate import from ./routes/admin/authProvider.js
and duplicate route registration. Removed duplicates since the correct
import is from ./routes/authProvider.js.
Co-Authored-By: Paperclip <noreply@paperclip.ing>
* 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>
* feat(web): add auth provider section to settings page (GRO-391)
Add Authentication Provider section to /admin/settings for super users.
Implements: provider ID, display name, issuer URL, internal base URL
(optional, collapsed), client ID, client secret (masked, only sent on
change), scopes fields; Test Connection button; Save and Reset to
Environment Defaults with confirmation dialog; warning banner about
service restart; env config info banner when no DB config is set.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* 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>
* 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>
---------
Co-authored-by: groombook-engineer[bot] <3141748+groombook-engineer[bot]@users.noreply.github.com>
Co-authored-by: Paperclip <noreply@paperclip.ing>
Co-authored-by: Barkley Trimsworth <noreply@groombook>
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Syntax error: `))` was closing the arrow function body prematurely.
Change `)),` to `}),` to properly close the values-returning object.
Co-Authored-By: Paperclip <noreply@paperclip.ing>
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>
Refactor auth initialization to support three config states:
1. DB config (auth_provider_config table) — primary source
2. OIDC_* env vars — fallback when DB config absent
3. Unconfigured — graceful handling when neither source available
Changes:
- auth.ts: Add initAuth() async factory, getAuth() getter, getAuthPromise()
- index.ts: Call initAuth() at startup before serve()
- middleware/auth.ts: Use getAuth() instead of direct auth import
- Add auth.test.ts covering all three config states
Preserves AUTH_DISABLED=true behavior and original hairpin NAT pattern.
Co-authored-by: groombook-engineer[bot] <3141748+groombook-engineer[bot]@users.noreply.github.com>
Co-authored-by: Paperclip <noreply@paperclip.ing>
Renumbered migration 0021 → 0023 to resolve conflict with pet_image and
logo_key migrations that landed on main after this branch was created.
Co-Authored-By: Paperclip <noreply@paperclip.ing>
* 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>
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>
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>
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>
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>
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>
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>
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>
## 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>
Add boolean is_super_user column (default false) to staff table.
Update Staff interface in shared types.
Mark first manager as super user in both seed modes.
Update test fixtures to include isSuperUser field.
Co-authored-by: groombook-ci[bot] <ci@groombook.bot>
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
* 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>
The DevLoginSelector stores the staff database id in localStorage and
sends it as X-Dev-User-Id. The resolveStaffMiddleware incorrectly
looked up staff by oidcSub instead of id, causing all API endpoints
to return 403 for every user in dev mode.
Co-Authored-By: Paperclip <noreply@paperclip.ing>
* 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>
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>
* 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>
Fixes ESLint @typescript-eslint/no-unused-vars error in CI.
baseUrl was declared but never used; confirmUrl/cancelUrl correctly
use apiUrl instead.
Co-Authored-By: Paperclip <noreply@paperclip.ing>
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>
- Remove unused makeSelectChain function from search.test.ts (lint blocker)
- Fix handleClientClick/handlePetClick to navigate to /admin/clients?highlight={id}
so the target client is identified in the URL rather than silently ignored
- Add console.warn for fetch errors in GlobalSearch instead of swallowing silently
Auth middleware verified: searchRouter is registered on the api Hono instance
which applies authMiddleware + resolveStaffMiddleware globally — no coverage gap.
Co-Authored-By: Paperclip <noreply@paperclip.ing>
Backend:
- GET /api/search?q={query} — returns up to 10 matching active clients and 10
matching pets in a single request; clients matched on name/email/phone,
pets matched on name/breed with owner name included
- Special chars (%, _, \) escaped before ILIKE to prevent injection/accidents
- Disabled clients excluded; pets from disabled client owners excluded via JOIN filter
- Route registered under protected API (auth + RBAC middleware applies automatically)
- Export `ilike` from @groombook/db alongside existing drizzle-orm helpers
Frontend:
- GlobalSearch component in sticky admin header: debounced input (300ms),
grouped dropdown (Clients / Pets sections), loading/empty states
- Client results show name + phone; pet results show name, breed, owner name
- Touch-friendly: 44px input height, 48px min row height, full-width dropdown
- Outside-click closes dropdown; selecting a result navigates to /admin/clients
Tests (apps/api/src/__tests__/search.test.ts):
- 400 on missing/empty/whitespace q
- Returns matching clients and pets
- Empty arrays on no match
- Response shape always has clients/pets keys
- Special character inputs handled without errors
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- DB migration 0012: add photo_key and photo_uploaded_at columns to pets table
- S3 client utility (apps/api/src/lib/s3.ts): presigned PUT/GET, delete via Rook-Ceph RGW
- API photo routes on petsRouter:
- POST /:petId/photo/upload-url — returns presigned PUT URL + object key
- POST /:petId/photo/confirm — records key in DB after successful upload
- DELETE /:petId/photo — deletes from storage and clears DB
- GET /:petId/photo — returns presigned GET URL
- RBAC: all staff roles (manager, receptionist, groomer) may upload/delete photos;
restructured index.ts guards so groomer-accessible photo paths don't overlap
with the manager/receptionist-only general pets write guard
- Frontend PetPhotoDisplay: responsive image with shimmer skeleton and paw placeholder
- Frontend PetPhotoUpload: client-side resize to max 1200px, XHR with progress,
presigned PUT flow — binary data never passes through the API server
- Wired both components into Clients.tsx staff portal pet cards
- Unit tests: 14 test cases covering all four routes (happy path + error cases)
Co-Authored-By: Paperclip <noreply@paperclip.ing>
Tests cover resetFactoryCounters(), counter determinism, override
merging, and compile-time enforcement of required fields on
buildAppointment. All 16 new tests pass (92 total).
Co-Authored-By: Paperclip <noreply@paperclip.ing>
Phase 1 — Seed Hardening:
- Replace all Math.random() calls in seed.ts with a Mulberry32 seeded PRNG
(seed 42) so the same data set is reproduced on every run
- Replace crypto.randomUUID() with a PRNG-based UUID v4 generator
- Add manager (Jordan Lee) and receptionist (Sam Rivera) staff members
to seed — previously all staff were groomers
- New packages/db/src/reset.ts drops all tables/enums and re-runs
migrate + seed; exposed as `pnpm db:reset` at root
- Generate migration 0010_impersonation_sessions.sql for the
impersonation_sessions and impersonation_audit_logs tables that were
already in schema.ts but had no corresponding migration
Phase 2 — Test Factories:
- New packages/db/src/factories.ts with buildStaff, buildClient, buildPet,
buildService, buildAppointment and resetFactoryCounters helpers
- Exported via @groombook/db/factories subpath (package.json + vitest alias)
- impersonation.test.ts updated to use buildStaff instead of hand-rolled
fixture objects
Closes#90 (Phases 1 + 2)
Co-Authored-By: Paperclip <noreply@paperclip.ing>
Use StaffRow type for all staff fixture objects so groomer/receptionist
variants don't cause type errors. Simplify buildApp/buildWithStaff helper
signatures to MiddlewareHandler<AppEnv> / Context<AppEnv> — no more
Parameters<...> inference gymnastics.
Co-Authored-By: Paperclip <noreply@paperclip.ing>
- New `apps/api/src/middleware/rbac.ts` with `resolveStaffMiddleware`
(resolves staff from DB by OIDC sub, supports AUTH_DISABLED dev mode)
and `requireRole(...roles)` factory for per-route role enforcement
- Wire `resolveStaffMiddleware` after `authMiddleware` on api basePath
- Route guards per permission matrix:
- Manager only: /staff/*, /admin/*, /reports/*, /invoices/*, /impersonation/*
- Manager + Receptionist only: /appointment-groups/*, /grooming-logs/*
- Groomers read-only on /clients/*, /pets/*, /appointments/* (write requires manager/receptionist)
- Services: all roles read, manager-only write
- Refactor impersonation router to use AppEnv and c.get("staff") instead
of inline staff resolution; role check delegated to requireRole middleware
- Unit tests in rbac.test.ts covering resolveStaffMiddleware and requireRole
- Update impersonation.test.ts to inject staff directly via context
Closes#88 (Phase 1)
Co-Authored-By: Paperclip <noreply@paperclip.ing>
- Import ImpersonationSession from @groombook/types (component was updated in #78)
- Remove stale tests: "shows customer name" and "returns null when inactive"
(component no longer renders customer name or checks session.active)
- Add isExtended prop to all render calls (component now takes isExtended as prop)
- Fix "does not show Extend button when already extended" to pass isExtended={true}
instead of session.extended (prop was extracted from session in #78)
- Fix clients.test.ts: selectRows typed as Record<string,unknown>[] to allow
spread in returning() callbacks (resolves TS2698)
Co-Authored-By: Paperclip <noreply@paperclip.ing>
* feat: implement Staff Impersonation backend and wire frontend
Add server-side impersonation session management with full audit
logging, replacing the frontend-only mock. Managers can start
time-limited sessions to view the app as a specific client.
Backend:
- Add impersonation_sessions and impersonation_audit_logs tables
(Drizzle schema) with proper FK constraints and status enum
- Add Hono API routes: start/get/extend/end session + audit logging
- Server-side session expiration, one-active-per-staff enforcement
- Staff role validation (manager-only)
Frontend:
- Add CustomerPortal wrapper with URL-param session init
- Add ImpersonationBanner with live countdown timer
- Add AuditLogViewer modal for session audit trail
- Add "View as Customer" button on Clients page
- Auto-log page visits during impersonation
Closes#74
Co-Authored-By: Paperclip <noreply@paperclip.ing>
* chore: remove unused useNavigate import from Clients.tsx
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* fix: add authorization + expiry checks to impersonation endpoints, add tests
Security: Add ownership verification (resolveStaff + staffId check) to
GET /sessions/:id, POST /sessions/:id/log, and GET /sessions/:id/audit-log
endpoints that were previously unprotected.
Bug: Add time-based expiry checks to extend, end, get-session, and log
endpoints via checkAndExpireSession() helper. Expired sessions are now
auto-marked as expired in the DB and cannot be extended or logged to.
Tests: Add 23 tests covering session creation (happy path, auth, conflict),
extend (active, expired, non-owner, ended), end (active, expired, non-owner),
audit logging (owner, non-owner, expired, ended), and audit-log retrieval
(owner, non-owner, not found).
Addresses QA review on PR #75 (GRO-66).
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* fix: resolve @groombook/db source in vitest config
Add resolve alias so vitest can resolve @groombook/db from source
TypeScript files without requiring a prior build step. Fixes CI
test failures when dist/ has not been compiled.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
---------
Co-authored-by: Groom Book CEO <ceo@groombook.dev>
Co-authored-by: Paperclip <noreply@paperclip.ing>
Co-authored-by: Groom Book CTO <cto@groombook.dev>
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Co-authored-by: Scrubs McBarkley <scrubs@groombook.app>
* feat: add client disable/deletion with soft-delete (#67)
Add soft-delete support for clients: disable is the default action
(hiding from client list and booking flow), with permanent deletion
requiring explicit type-to-confirm. Disabled clients remain in
reporting and can be re-enabled by staff.
- Add client_status enum (active/disabled) and disabled_at column
- API defaults GET /api/clients to active-only, ?includeDisabled=true shows all
- PATCH /api/clients/:id accepts status field for disable/enable
- DELETE requires ?confirm=true query param
- Booking flow skips disabled clients
- Frontend: show disabled toggle, disable/enable buttons, delete confirmation modal
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* fix: remove unused updateClientSchema (lint error)
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
---------
Co-authored-by: Groom Book CTO <cto@groombook.app>
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
* feat: add customizable business branding (name, logo, colors)
Add admin settings for business branding with name, logo upload, and
color scheme via CSS custom properties. Includes database migration,
API endpoints, admin settings page, and dynamic branding in both
admin nav and customer portal.
Closes#61
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* fix: address review feedback on branding PR
- Replace dynamic import with static import for @groombook/db in public branding endpoint
- Restore active nav item background highlight (bg-stone-100) in CustomerPortal
- Remove non-null assertion in settings route, add proper error handling
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* chore: trigger CI
* fix: resolve lint error and test failure for branding feature
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* fix: update E2E tests for branding changes
- Update navigation test to expect "GroomBook" (default branding) instead
of hardcoded "Paws & Reflect" since CustomerPortal now uses dynamic branding
- Add /api/branding mock to shared E2E fixtures so BrandingProvider resolves
immediately in all tests, preventing unhandled fetch interference
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
---------
Co-authored-by: GroomBook CTO <cto@groombook.dev>
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Co-authored-by: GroomBook CTO <cto@groombook.app>
* Add dev/demo login selector for quick user switching
When AUTH_DISABLED=true, the app now shows a login selector page that
lists staff members and clients from the database. Selecting a user
sets a localStorage-based session and sends X-Dev-User-Id header on
all API requests. A persistent bottom bar shows the active persona
with a "Switch user" link.
- API: /api/dev/config (public) and /api/dev/users (auth-disabled only)
- API: auth middleware reads X-Dev-User-Id header when auth is disabled
- Frontend: DevLoginSelector page, DevSessionIndicator bar
- Frontend: fetch interceptor injects X-Dev-User-Id on /api/* calls
- Tests: 7 passing (5 nav + 2 dev login)
Closes#60
Co-Authored-By: Paperclip <noreply@paperclip.ing>
* fix(e2e): seed dev user in localStorage to prevent login redirect
E2E tests were failing because the dev login selector redirects to
/login when AUTH_DISABLED=true and no dev user is in localStorage.
Added a shared Playwright fixture that pre-seeds localStorage with
a default dev user before each test.
Also rebased onto latest main to resolve merge conflict in App.test.tsx.
Co-Authored-By: Paperclip <noreply@paperclip.ing>
* fix(e2e): mock /api/dev/config to bypass auth redirect in tests
The fixture now also mocks /api/dev/config to return authDisabled: false,
preventing the app from entering the redirect flow during E2E tests.
Previously only seeded localStorage, but the async config fetch from the
real Docker API was still triggering the redirect check.
Co-Authored-By: Paperclip <noreply@paperclip.ing>
---------
Co-authored-by: Groom Book CTO <cto@groombook.app>
Co-authored-by: Paperclip <noreply@paperclip.ing>
The /api/reports/clients endpoint was crashing with a 500 on every request
because a raw JavaScript Date passed into a sql template literal in .having()
cannot be serialized by postgres-js. The fix serializes it as an ISO string
with an explicit ::timestamptz cast.
Also adds reportsRouter.onError() and improves the frontend error message
to surface which specific endpoint failed and why.
Fixes#49
Co-Authored-By: Paperclip <noreply@paperclip.ing>
The /api/reports/clients endpoint crashes with a 500 because
Drizzle's sql template literal in a HAVING clause cannot serialize
a JavaScript Date object — the postgres driver expects a string.
Convert the Date to an ISO string and add an explicit ::timestamptz
cast so PostgreSQL handles the comparison correctly.
Closesgroombook/groombook#49
Co-authored-by: Groom Book CEO <ceo@groombook.dev>
Co-authored-by: Paperclip <noreply@paperclip.ing>
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
pnpm install --prod creates workspace symlinks (node_modules/@groombook/db
→ packages/db/), but dist/ files didn't exist yet, causing Node.js to fall
back to resolving .ts source files at runtime (ERR_UNKNOWN_FILE_EXTENSION).
Copy compiled dist files and updated package.json from the builder stage
before running pnpm install so symlinks point to existing dist output.
Co-authored-by: Groom Book CEO <ceo@groombook.app>
Co-authored-by: Paperclip <noreply@paperclip.ing>
The API Docker image was crashing because Node.js ESM resolution
was finding TypeScript source files instead of compiled JS output.
Added explicit exports fields to workspace packages for deterministic
resolution and a cleanup step in the Dockerfile runner stage.
Co-authored-by: Groom Book CEO <ceo@groombook.dev>
Co-authored-by: Paperclip <noreply@paperclip.ing>
Extract slot generation from book.ts into pure utility for unit testing.
Add 8 API unit tests and 4 web component tests with coverage thresholds.
Closes#39
Co-Authored-By: Paperclip <noreply@paperclip.ing>
Fixes ERR_UNKNOWN_FILE_EXTENSION at container startup by compiling @groombook/types and @groombook/db to dist/ before the runtime stage, and copying only compiled JS instead of raw TypeScript source.
Co-Authored-By: Paperclip <noreply@paperclip.ing>
groombook-engineer[bot]
Scrubs McBarkley