* 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>
Auto-link staff records by email when userId is NULL on first authenticated request.
Resolves GRO-667 UAT 403 blocker.
Co-Authored-By: Flea Flicker <noreply@anthropic.com>
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>
- 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>
Prevents ENOENT crash in migrate and seed jobs.
Root cause: corepack tries to mkdir /home/node/.cache/node/corepack/v1
but the directory does not exist in the builder stage. This was a
regression in c438f57 where the cache directory was not pre-created.
Co-Authored-By: Paperclip <noreply@paperclip.ing>
- 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>
* feat(GRO-566): add SKIP_OOBE env var to bypass setup wizard
Co-Authored-By: Paperclip <noreply@paperclip.ing>
* Add rate_limit table migration for Better Auth (GRO-574)
Co-Authored-By: Paperclip <noreply@paperclip.ing>
* fix(GRO-574): switch rate limit to memory storage to unblock UAT
Better Auth rate_limit table migration exists on branch but hasn't
been deployed to UAT. Switching to memory storage bypasses the
missing table entirely, restoring auth functionality immediately.
Memory storage is per-instance (not shared) — rate limiting still
functions but won't be distributed across pods. This is acceptable
for UAT while the migration is being promoted through the pipeline.
Co-Authored-By: Paperclip <noreply@paperclip.ing>
---------
Co-authored-by: Paperclip <noreply@paperclip.ing>
Co-authored-by: groombook-qa[bot] <269744346+groombook-qa[bot]@users.noreply.github.com>
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
- apps/web: upgrade better-auth from ^1.0.0 to ^1.5.6 (matches API)
- apps/web/vite.config.ts: exclude /api/auth/* from service worker caching
- apps/api/index.ts: return 503 when auth not configured
- apps/api/middleware/auth.ts: return 503 when auth not initialized
Co-Authored-By: Paperclip <noreply@paperclip.ing>
The OAuth callback was failing with "please_restart_the_process" because
Better-Auth's default DB-backed state (verification table) was unreliable —
the UAT hourly reset wipes all tables including verification records. Switch
to cookie-based state storage so the encrypted state survives in the browser
cookie across the redirect flow.
Also removes explicit redirectURI from socialProviders (Better-Auth derives
it from baseURL) and adds visible error feedback on the login page when
OAuth callbacks fail.
Co-Authored-By: Paperclip <noreply@paperclip.ing>
Explicitly set redirectURI in social provider configs to ensure
Better-Auth uses the correct callback URL for OAuth providers.
Co-Authored-By: Paperclip <noreply@paperclip.ing>
- auth.ts: add google/github social providers from better-auth/social-providers
- auth.ts: add getActiveProviders() to enumerate configured OAuth/social providers
- index.ts: add /api/auth/providers public endpoint for frontend
- App.tsx: update LoginPage to show Google/GitHub buttons based on /api/auth/providers response
Co-Authored-By: Paperclip <noreply@paperclip.ing>
- Add ALLOW_RESET env var override to reset.ts safety guard
- Add reset Docker build target to Dockerfile
- Add reset image build step to CI docker job
- Add reset image tag update to CD job dev overlay update
Co-Authored-By: Paperclip <noreply@paperclip.ing>
- 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>
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>
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>
drizzle-orm is not a direct dependency of @groombook/api, causing
TS2307 at typecheck time. Re-export isNull from @groombook/db and
update the import in rbac.ts.
Co-Authored-By: Paperclip <noreply@paperclip.ing>
When a staff record exists with a matching email but no userId (e.g. seed data
or admin UI-created records), resolveStaffMiddleware now auto-links it to the
Better-Auth user record on first SSO login instead of returning 403.
Safety: only links when userId IS NULL, never overwrites an existing link.
Email matching is safe since it comes from the trusted SSO provider (Authentik).
Staff emails are unique by schema.
Co-Authored-By: Paperclip <noreply@paperclip.ing>
The authProviderRouter was registered twice at /admin/auth-provider in
apps/api/src/index.ts. The second registration is a no-op but creates
confusion. Remove the duplicate line.
Co-authored-by: Paperclip <noreply@paperclip.ing>
Use a 16-byte random salt per encryption instead of the fixed
"groombook-auth-provider-config" salt. This prevents identical
plaintexts from producing identical ciphertexts, closing the
timing/anagram security gap identified in GRO-452.
New format: salt:iv:ciphertext:authTag (all base64).
Legacy format (iv:ciphertext:authTag) is still accepted for
backward-compatible decryption of existing stored values.
Co-Authored-By: Paperclip <noreply@paperclip.ing>
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>
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>
Generate a unique 16-byte random salt for each encryptSecret() call
and store it as a prefix in the ciphertext. Format changed from
iv:ciphertext:authTag → salt:iv:ciphertext:authTag
decryptSecret() detects legacy 3-part format and uses the fixed
package salt for backward compatibility with existing encrypted rows.
Co-Authored-By: Paperclip <noreply@paperclip.ing>
reinitAuth was imported by authProvider.ts but never defined.
Added a stub implementation that resolves immediately — proper
restart mechanism is tracked in GRO-390.
Co-Authored-By: Paperclip <noreply@paperclip.ing>
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>
vi.mock the auth module so reinitAuth() is a no-op in tests.
This decouples the tests from the BETTER_AUTH_SECRET env var.
Co-Authored-By: Paperclip <noreply@paperclip.ing>
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>
- 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>
- GET /api/setup/status: verify showAuthProviderStep logic for all cases
(fresh install, env vars present, setup complete, DB config exists)
- POST /api/setup/auth-provider: 403 after complete, 409 if already configured,
creates config with encrypted secret, Zod validation
- POST /api/setup/auth-provider/test: 403 after complete, unreachable issuer,
valid issuer, invalid issuer (non-200)
Co-Authored-By: Paperclip <noreply@paperclip.ing>
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>
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>
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 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>
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>
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>
groombook-cto[bot]