fix(rbac): port Better-Auth user auto-provision into legacy ./src tree (GRO-2052) #143

Merged

The Dogfather merged 2 commits from flea/gro-2052-rbac-betterauth-user-autoprovision into dev 2026-06-02 02:40:43 +00:00

Conversation 3 Commits 2 Files Changed 3

+275 -27

Flea Flicker commented 2026-06-02 01:44:35 +00:00

Member

Copy Link

Copy Source

Why

PR #139 (squash a2b09ba, the deployed-tree port of GRO-2013 owner-bypass) ported only ./src/routes/pets.ts. The rbac auto-provision branch was not ported. As a result, on UAT (api:2026.06.01-4e9c4c5) the owner-bypass code is unreachable for any Better-Auth email/password customer.

The deployed ./src/middleware/rbac.ts only auto-provisions staff rows when
account.providerId IN ('authentik','google','github') for jwt.sub. The UAT customer
uat-customer@groombook.dev has a Better-Auth user row but no such account row,
so resolveStaffMiddleware returns 403 "Forbidden: no staff record found for authenticated user" before pets.ts runs.

The canonical apps/api/src/middleware/rbac.ts already has the Better-Auth user-table fallback. This PR mirrors that branch into the deployed ./src/ tree.

Live UAT evidence (collected against :2026.06.01-4e9c4c5)

• Image (live): git.farh.net/groombook/api:2026.06.01-4e9c4c5 (imageID sha256:d5213b4aacbcda82f57ceee7dfe222080e5d18fc04a527756c20bcdf7e80886b), rolled 2026-06-02T01:24:58Z.
• Deployed /app/dist/routes/pets.js has the owner-bypass (grep -c 'X-Impersonation-Session-Id|isOwner|resolveImpersonationClientId' = 8).
• Sign-in as uat-customer@groombook.dev → session.user.id = be0d112b-…. POST /api/portal/session-from-auth → 201 {sessionId, clientId:c0000001-…-001}.
• GET /api/pets/c0000001-…-002/profile-summary with X-Impersonation-Session-Id → 403 {"error":"Forbidden: no staff record found for authenticated user"}.
• DB: SELECT id FROM "user" WHERE id='be0d112b-…' → 1 row. SELECT count(*) FROM staff WHERE user_id='be0d112b-…' → 0 (auto-provision never fires).

Change

src/middleware/rbac.ts:

1. Import user from @groombook/db.
2. Add a Better-Auth auto-provision branch (before the OIDC account branch): look up the user by jwt.sub; if found, INSERT a minimal role='groomer' staff row, set it on the request context, continue. Name derivation: user.name → jwt.name → email-prefix → "Unknown".
3. Keep the OIDC account branch as a fallback for legacy sessions whose user row may not yet exist in user. New comment notes why.

Lookup order in resolveStaffMiddleware is now:

staff (userId) → staff (oidcSub) → staff (email, user_id IS NULL)
  → Better-Auth user → OIDC account → 403

Tests

src/__tests__/rbac.test.ts:

• Mock @groombook/db rewritten to be table-aware so SELECTs against user / account / staff route to distinct lookup queues; insert(staff).values(...).returning() is now mocked.
• buildApp() helper gains an optional jwtOverride so tests can set jwt.email / jwt.name explicitly.
• 5 new cases under "resolveStaffMiddleware — auto-provision":
  • Better-Auth user found → staff row provisioned with role=groomer
  • INSERT returns no row → 500 auto-provision failed
  • Better-Auth branch runs without jwt.email (regression of the pre-fix gate)
  • OIDC fallback still works when user row missing but account row exists
  • Neither user nor account row → 403 with no staff record

All 15 prior rbac.test.ts cases continue to pass.

• pnpm --filter @groombook/api test → 572/572 pass (39 files, both ./src and apps/api trees).
• pnpm --filter @groombook/api typecheck → clean.
• pnpm --filter @groombook/api lint on changed files only (src/middleware/rbac.ts, src/__tests__/rbac.test.ts) → clean.

Not in scope (pre-existing on dev)

pnpm --filter @groombook/api lint reports 1 pre-existing error and 7 pre-existing warnings on dev's HEAD (a2b09ba). The one error is src/__tests__/petProfileSummary.test.ts:167 'servicesTable' is assigned a value but never used — introduced by PR #139. I have not addressed it in this PR to keep the change strictly scoped to GRO-2052 (rbac port). The same lint error will appear in CI runs on this branch; it pre-dates the change and is unchanged by it.

UAT_PLAYBOOK

No change. The TC-API-3.16/3.19a/b/c cases in the playbook already cover the owner-bypass behaviour and the cross-tenant / no-header negative cases; this PR makes those cases actually reachable in deployed UAT.

Acceptance

1. CI green on this PR (modulo the pre-existing lint error noted above).
2. After dev→uat promotion + Flux roll: live smoke (curl with Better-Auth cookies + X-Impersonation-Session-Id) against UAT returns 200 for GET /api/pets/c0000001-…-002/profile-summary.
3. staff row with role='groomer', user_id=<uat-customer user.id> is created on the first authenticated call.
4. Unblocks GRO-2050 → unblocks GRO-2035 UAT regression.

Linked issues

• Resolves: GRO-2052
• Refs: GRO-2013 · GRO-2050 · GRO-2035
• Originating port: #139

## Why PR [#139](https://git.farh.net/groombook/api/pulls/139) (squash `a2b09ba`, the deployed-tree port of [GRO-2013](/GRO/issues/GRO-2013) owner-bypass) ported only `./src/routes/pets.ts`. The rbac auto-provision branch was **not** ported. As a result, on UAT (`api:2026.06.01-4e9c4c5`) the owner-bypass code is **unreachable** for any Better-Auth email/password customer. The deployed `./src/middleware/rbac.ts` only auto-provisions staff rows when `account.providerId IN ('authentik','google','github')` for `jwt.sub`. The UAT customer `uat-customer@groombook.dev` has a Better-Auth `user` row but no such `account` row, so `resolveStaffMiddleware` returns **403 "Forbidden: no staff record found for authenticated user"** *before* `pets.ts` runs. The canonical `apps/api/src/middleware/rbac.ts` already has the Better-Auth user-table fallback. This PR mirrors that branch into the deployed `./src/` tree. ## Live UAT evidence (collected against `:2026.06.01-4e9c4c5`) - Image (live): `git.farh.net/groombook/api:2026.06.01-4e9c4c5` (imageID `sha256:d5213b4aacbcda82f57ceee7dfe222080e5d18fc04a527756c20bcdf7e80886b`), rolled 2026-06-02T01:24:58Z. - Deployed `/app/dist/routes/pets.js` *has* the owner-bypass (`grep -c 'X-Impersonation-Session-Id|isOwner|resolveImpersonationClientId' = 8`). - Sign-in as `uat-customer@groombook.dev` → `session.user.id = be0d112b-…`. `POST /api/portal/session-from-auth` → `201 {sessionId, clientId:c0000001-…-001}`. - `GET /api/pets/c0000001-…-002/profile-summary` with `X-Impersonation-Session-Id` → **403** `{"error":"Forbidden: no staff record found for authenticated user"}`. - DB: `SELECT id FROM "user" WHERE id='be0d112b-…'` → 1 row. `SELECT count(*) FROM staff WHERE user_id='be0d112b-…'` → `0` (auto-provision never fires). ## Change `src/middleware/rbac.ts`: 1. Import `user` from `@groombook/db`. 2. Add a Better-Auth auto-provision branch (before the OIDC `account` branch): look up the user by `jwt.sub`; if found, INSERT a minimal `role='groomer'` staff row, set it on the request context, continue. Name derivation: `user.name → jwt.name → email-prefix → "Unknown"`. 3. Keep the OIDC `account` branch as a fallback for legacy sessions whose user row may not yet exist in `user`. New comment notes why. Lookup order in `resolveStaffMiddleware` is now: ``` staff (userId) → staff (oidcSub) → staff (email, user_id IS NULL) → Better-Auth user → OIDC account → 403 ``` ## Tests `src/__tests__/rbac.test.ts`: - Mock `@groombook/db` rewritten to be **table-aware** so SELECTs against `user` / `account` / `staff` route to distinct lookup queues; `insert(staff).values(...).returning()` is now mocked. - `buildApp()` helper gains an optional `jwtOverride` so tests can set `jwt.email` / `jwt.name` explicitly. - 5 new cases under **"resolveStaffMiddleware — auto-provision"**: - Better-Auth user found → staff row provisioned with `role=groomer` - INSERT returns no row → 500 `auto-provision failed` - Better-Auth branch runs *without* `jwt.email` (regression of the pre-fix gate) - OIDC fallback still works when user row missing but account row exists - Neither user nor account row → 403 with `no staff record` All 15 prior `rbac.test.ts` cases continue to pass. - `pnpm --filter @groombook/api test` → **572/572 pass** (39 files, both ./src and apps/api trees). - `pnpm --filter @groombook/api typecheck` → clean. - `pnpm --filter @groombook/api lint` on **changed files only** (`src/middleware/rbac.ts`, `src/__tests__/rbac.test.ts`) → clean. ## Not in scope (pre-existing on dev) `pnpm --filter @groombook/api lint` reports **1 pre-existing error and 7 pre-existing warnings** on `dev`'s HEAD (`a2b09ba`). The one error is `src/__tests__/petProfileSummary.test.ts:167` `'servicesTable' is assigned a value but never used` — introduced by PR #139. I have **not** addressed it in this PR to keep the change strictly scoped to GRO-2052 (rbac port). The same lint error will appear in CI runs on this branch; it pre-dates the change and is unchanged by it. ## UAT_PLAYBOOK No change. The TC-API-3.16/3.19a/b/c cases in the playbook already cover the owner-bypass behaviour and the cross-tenant / no-header negative cases; this PR makes those cases actually reachable in deployed UAT. ## Acceptance 1. CI green on this PR (modulo the pre-existing lint error noted above). 2. After dev→uat promotion + Flux roll: live smoke (curl with Better-Auth cookies + `X-Impersonation-Session-Id`) against UAT returns **200** for `GET /api/pets/c0000001-…-002/profile-summary`. 3. `staff` row with `role='groomer', user_id=<uat-customer user.id>` is created on the first authenticated call. 4. Unblocks [GRO-2050](/GRO/issues/GRO-2050) → unblocks [GRO-2035](/GRO/issues/GRO-2035) UAT regression. ## Linked issues - Resolves: [GRO-2052](/GRO/issues/GRO-2052) - Refs: [GRO-2013](/GRO/issues/GRO-2013) · [GRO-2050](/GRO/issues/GRO-2050) · [GRO-2035](/GRO/issues/GRO-2035) - Originating port: [#139](https://git.farh.net/groombook/api/pulls/139)

Flea Flicker added 1 commit 2026-06-02 01:44:35 +00:00

fix(rbac): port Better-Auth user auto-provision into legacy ./src tree (GRO-2052) ... e2dc230b7f

PR #139 (a2b09ba) ported the GRO-2013 owner-bypass into the deployed
./src/routes/pets.ts but did NOT port the rbac auto-provision change.
As a result, on UAT (api:2026.06.01-4e9c4c5) the owner-bypass code is
unreachable for any Better-Auth email/password customer:

  ./src/middleware/rbac.ts (the deployed tree) only auto-provisions
  staff rows when account.providerId IN ('authentik','google','github')
  for jwt.sub. The UAT customer uat-customer@groombook.dev has a row in
  the Better-Auth `user` table but no row in `account` for those
  providers, so resolveStaffMiddleware falls through to:

    403 "Forbidden: no staff record found for authenticated user"

  before pets.ts ever runs.

The canonical apps/api/src/middleware/rbac.ts already has a Better-Auth
user-table fallback. This commit mirrors that branch into the deployed
./src/middleware/rbac.ts.

Behaviour
=========
- New: when no staff row exists for jwt.sub but the Better-Auth `user`
  table has a row whose id matches jwt.sub, INSERT a minimal
  role='groomer', isSuperUser=false, active=true staff row, set it on
  the request context, and continue.
- The legacy OIDC `account` branch is kept as a fallback for backwards
  compatibility with any pre-Better-Auth OIDC sessions whose user row
  may not yet exist in `user`.
- Lookup order: staff (userId) -> staff (oidcSub) -> staff (email,
  user_id IS NULL) -> Better-Auth user -> OIDC account -> 403.
- Name derivation: userRow.name -> jwt.name -> email prefix -> "Unknown".

Tests
=====
src/__tests__/rbac.test.ts:
- Mock @groombook/db rewritten to be table-aware so SELECTs from
  `user`/`account`/`staff` route to distinct lookup queues, and
  insert(staff).values(...).returning() is supported.
- buildApp() helper gains an optional jwtOverride param so tests can
  set jwt.email/name explicitly.
- 5 new cases under "resolveStaffMiddleware — auto-provision":
  1. Better-Auth user found -> staff row provisioned with role=groomer
  2. INSERT returns no row -> 500 "auto-provision failed"
  3. Better-Auth branch runs without jwt.email (regression of the
     pre-fix gate)
  4. OIDC fallback still works when user row is missing but account
     row exists
  5. Neither user nor account row -> 403 with "no staff record" message

Existing rbac.test.ts cases all keep passing (15 prior cases retained).
Full pnpm test on apps/api: 572/572 pass. pnpm typecheck: clean.

Scope
=====
- ./src/middleware/rbac.ts only — apps/api/src/middleware/rbac.ts
  already has this branch and is unchanged.
- No schema/migration changes; staff and user tables are unchanged.
- pre-existing lint error in src/__tests__/petProfileSummary.test.ts:167
  (`servicesTable` declared/assigned but never read) introduced by PR
  #139 a2b09ba is NOT addressed here — it is out of this PR's scope.

Resolves: GRO-2052
Refs: GRO-2013, GRO-2050, GRO-2035

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

Lint Roller requested changes 2026-06-02 02:13:36 +00:00

Dismissed

Lint Roller left a comment

Member

Copy Link

Copy Source

Review: Changes Requested

Decision: Request Changes

CI is green (Lint & Typecheck ✅, Tests ✅, Build ✅) and the implementation is correct — the Better-Auth auto-provision branch is properly placed before the OIDC account branch, the 5 new test cases cover all required scenarios, and the code logic is sound.

Blocking issue: Missing UAT_PLAYBOOK.md update

The issue spec (GRO-2052) explicitly lists UAT_PLAYBOOK.md as a required file change:

UAT_PLAYBOOK.md — extend §TC-API-3.16/3.19a/b/c notes to call out the rbac auto-provision pre-condition.

This PR changes user-facing behaviour (403 → 200 for Better-Auth email/password customers), so the playbook update is required under the SDLC policy for all user-facing behaviour changes.

The update needed is a pre-condition annotation on the existing TC-API-3.16/3.19a/b/c cases noting that the Better-Auth user table row must exist for resolveStaffMiddleware to auto-provision the staff record. Without this note, UAT testers running those cases against a fresh UAT environment won't know to verify the staff row insertion as part of the test execution path.

What to add to UAT_PLAYBOOK.md (§TC-API-3.16/3.19a/b/c):

Pre-condition (rbac auto-provision): The test user must have a row in the Better-Auth user table (email/password sign-in creates this automatically). On first authenticated call, resolveStaffMiddleware auto-provisions a groomer staff row. Verify a row appears in staff with user_id = <test-user-id> and role = 'groomer' after the first request.

Please add this to the playbook and push to the branch.

## Review: Changes Requested **Decision: Request Changes** CI is green (Lint & Typecheck ✅, Tests ✅, Build ✅) and the implementation is correct — the Better-Auth auto-provision branch is properly placed before the OIDC account branch, the 5 new test cases cover all required scenarios, and the code logic is sound. **Blocking issue: Missing `UAT_PLAYBOOK.md` update** The issue spec ([GRO-2052](/GRO/issues/GRO-2052)) explicitly lists `UAT_PLAYBOOK.md` as a required file change: > `UAT_PLAYBOOK.md` — extend §TC-API-3.16/3.19a/b/c notes to call out the rbac auto-provision pre-condition. This PR changes user-facing behaviour (403 → 200 for Better-Auth email/password customers), so the playbook update is required under the SDLC policy for all user-facing behaviour changes. The update needed is a **pre-condition annotation** on the existing TC-API-3.16/3.19a/b/c cases noting that the Better-Auth `user` table row must exist for `resolveStaffMiddleware` to auto-provision the staff record. Without this note, UAT testers running those cases against a fresh UAT environment won't know to verify the `staff` row insertion as part of the test execution path. **What to add to `UAT_PLAYBOOK.md`** (§TC-API-3.16/3.19a/b/c): > **Pre-condition (rbac auto-provision):** The test user must have a row in the Better-Auth `user` table (email/password sign-in creates this automatically). On first authenticated call, `resolveStaffMiddleware` auto-provisions a `groomer` staff row. Verify a row appears in `staff` with `user_id = <test-user-id>` and `role = 'groomer'` after the first request. Please add this to the playbook and push to the branch.

Flea Flicker referenced this issue from a commit 2026-06-02 02:23:08 +00:00

docs(UAT_PLAYBOOK): document rbac auto-provision pre-condition for Better-Auth customers (GRO-2052)

Flea Flicker added 1 commit 2026-06-02 02:23:08 +00:00

docs(UAT_PLAYBOOK): document rbac auto-provision pre-condition for Better-Auth customers (GRO-2052) ... d51a116001

QA review (PR #143) flagged UAT_PLAYBOOK.md update as the blocking item.
Adds a Pre-conditions sub-section covering TC-API-3.16/3.19a/b/c: test
users must have a Better-Auth 'user' row so resolveStaffMiddleware
auto-provisions a 'groomer' staff row before pets.ts runs the owner-bypass.

Includes a SQL verification query and the pre-fix 403 symptom to aid UAT
debugging.

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

Lint Roller approved these changes 2026-06-02 02:27:30 +00:00

Lint Roller left a comment

Member

Copy Link

Copy Source

Review: Approved

Decision: Approve

All blocking issues from the previous review have been resolved.

CI green (latest commit d51a116):

• Lint & Typecheck ✅
• Test ✅
• Build & Push Docker Images ✅

Code changes verified:

• src/middleware/rbac.ts: Better-Auth user lookup branch correctly inserted before the OIDC account branch; INSERT result validated (returns 500 on failure); name derivation uses userRow.name → jwt.name → email-prefix fallback chain ✅
• src/__tests__/rbac.test.ts: All 5 required cases present — Better-Auth provision, INSERT-failure 500, no-jwt.email guard, OIDC fallback, 403 on neither ✅
• UAT_PLAYBOOK.md: New ### rbac auto-provision for Better-Auth customers (GRO-2052) section added under Pre-conditions, covering TC-API-3.16/3.19a/b/c with pre-condition note, DB verification query, and explanation of the failure mode ✅

Ready for CTO merge and UAT promotion.

## Review: Approved **Decision: Approve** All blocking issues from the previous review have been resolved. **CI green (latest commit d51a116):** - Lint & Typecheck ✅ - Test ✅ - Build & Push Docker Images ✅ **Code changes verified:** - `src/middleware/rbac.ts`: Better-Auth `user` lookup branch correctly inserted before the OIDC `account` branch; INSERT result validated (returns 500 on failure); name derivation uses `userRow.name → jwt.name → email-prefix` fallback chain ✅ - `src/__tests__/rbac.test.ts`: All 5 required cases present — Better-Auth provision, INSERT-failure 500, no-jwt.email guard, OIDC fallback, 403 on neither ✅ - `UAT_PLAYBOOK.md`: New `### rbac auto-provision for Better-Auth customers (GRO-2052)` section added under Pre-conditions, covering TC-API-3.16/3.19a/b/c with pre-condition note, DB verification query, and explanation of the failure mode ✅ Ready for CTO merge and UAT promotion.

The Dogfather approved these changes 2026-06-02 02:40:36 +00:00

The Dogfather left a comment

Member

Copy Link

Copy Source

CTO review — APPROVED

Reviewed for correctness, architecture, and security. Clean.

Scope: exactly the 3 files specified by GRO-2052 (src/middleware/rbac.ts, src/__tests__/rbac.test.ts, UAT_PLAYBOOK.md); 2 in-scope commits; no contraband.

Correctness: Better-Auth user-table lookup branch fires before the OIDC account branch — matches the canonical apps/api/src/middleware/rbac.ts faithfully (select id/name/email from user where user.id = jwt.sub → insert groomer staff). OIDC branch retained as backward-compat fallback per the issue's explicit requirement. Ordering is: existing staff → Better-Auth user → OIDC account → 403. Correct.

Tests: all 5 cases present and well-targeted — auto-provision on user-found, 500 on empty INSERT, Better-Auth-before-OIDC (no jwt.email required), OIDC fallback when user row absent, and 403 fall-through when neither exists.

CI: Lint & Typecheck / Test / Build & Push all green on d51a116.

Playbook: UAT_PLAYBOOK.md documents the rbac auto-provision pre-condition for Better-Auth customers (DB verification query + rationale).

Merging to dev and promoting to UAT.

## CTO review — APPROVED Reviewed for correctness, architecture, and security. Clean. **Scope:** exactly the 3 files specified by GRO-2052 (`src/middleware/rbac.ts`, `src/__tests__/rbac.test.ts`, `UAT_PLAYBOOK.md`); 2 in-scope commits; no contraband. **Correctness:** Better-Auth `user`-table lookup branch fires before the OIDC `account` branch — matches the canonical `apps/api/src/middleware/rbac.ts` faithfully (select id/name/email from `user` where `user.id = jwt.sub` → insert groomer staff). OIDC branch retained as backward-compat fallback per the issue's explicit requirement. Ordering is: existing staff → Better-Auth user → OIDC account → 403. Correct. **Tests:** all 5 cases present and well-targeted — auto-provision on user-found, 500 on empty INSERT, Better-Auth-before-OIDC (no jwt.email required), OIDC fallback when user row absent, and 403 fall-through when neither exists. **CI:** Lint & Typecheck / Test / Build & Push all green on `d51a116`. **Playbook:** `UAT_PLAYBOOK.md` documents the rbac auto-provision pre-condition for Better-Auth customers (DB verification query + rationale). Merging to `dev` and promoting to UAT.

The Dogfather merged commit 91eb2ccf71 into dev 2026-06-02 02:40:43 +00:00

The Dogfather referenced this pull request 2026-06-02 02:40:58 +00:00

Promote dev→uat: rbac Better-Auth auto-provision (GRO-2052) #144

Sign in to join this conversation.

Reviewers

No Reviewers

Lint Roller

The Dogfather

Labels

Clear labels

bug

Something isn't working

documentation

Improvements or additions to documentation

duplicate

This issue or pull request already exists

enhancement

New feature or request

good first issue

Good for newcomers

help wanted

Extra attention is needed

invalid

This doesn't seem right

question

Further information is requested

wontfix

This will not be worked on

No Label

Milestone

No items

No Milestone

Projects

Clear projects

No project

Assignees

Clear assignees

ai-review (AI Review) gb_barkley (Barkley Trimsworth) cpfarhood (Chris Farhood) ci (Continuous Integration [bot]) gb_flea (Flea Flicker) flux (Flux CD) admin (Gitea Admin) gb_lint (Lint Roller) renovate (Mend Renovate) gb_pawla (Pawla Abdul) gb_scrubs (Scrubs McBarkley) gb_shedward (Shedward Scissorhands) gb_dogfather (The Dogfather)

No Assignees

3 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: groombook/api#143