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>
This commit was merged in pull request #136.
This commit is contained in:
groombook-engineer[bot]
2026-03-28 03:50:45 +00:00
committed by GitHub
parent dc67b2bf44
commit ad1f32eb8f
36 changed files with 695 additions and 131 deletions
@@ -15,6 +15,7 @@ import type { StaffRow } from "../middleware/rbac.js";
const MANAGER: StaffRow = {
id: "staff-manager-id",
oidcSub: "oidc-manager-sub",
userId: null,
role: "manager",
name: "Manager McManager",
email: "manager@example.com",
+1 -2
View File
@@ -1,6 +1,5 @@
import { describe, it, expect, vi, beforeEach } from "vitest";
import { Hono } from "hono";
import type { JwtPayload } from "../middleware/auth.js";
import type { AppEnv, StaffRow } from "../middleware/rbac.js";
import { buildStaff } from "@groombook/db/factories";
@@ -167,7 +166,7 @@ function createApp(
if (!staffRow) {
return c.json({ error: "Forbidden: no staff record found for authenticated user" }, 403);
}
c.set("jwtPayload", { sub: staffRow.oidcSub } as JwtPayload);
c.set("jwtPayload", { sub: staffRow.oidcSub } as { sub: string; email?: string; name?: string });
c.set("staff", staffRow as unknown as StaffRow);
await next();
});
+1
View File
@@ -7,6 +7,7 @@ import type { AppEnv, StaffRow } from "../middleware/rbac.js";
const MANAGER: StaffRow = {
id: "staff-manager-id",
oidcSub: "oidc-manager-sub",
userId: null,
role: "manager",
name: "Manager McManager",
email: "manager@example.com",
+5 -2
View File
@@ -8,6 +8,7 @@ import type { AppEnv, StaffRow } from "../middleware/rbac.js";
const MANAGER: StaffRow = {
id: "staff-manager-id",
oidcSub: "oidc-manager-sub",
userId: "ba-user-manager",
role: "manager",
name: "Manager McManager",
email: "manager@example.com",
@@ -21,6 +22,7 @@ const RECEPTIONIST: StaffRow = {
...MANAGER,
id: "staff-receptionist-id",
oidcSub: "oidc-receptionist-sub",
userId: "ba-user-receptionist",
role: "receptionist",
name: "Receptionist Rita",
email: "receptionist@example.com",
@@ -30,6 +32,7 @@ const GROOMER: StaffRow = {
...MANAGER,
id: "staff-groomer-id",
oidcSub: "oidc-groomer-sub",
userId: "ba-user-groomer",
role: "groomer",
name: "Groomer Gary",
email: "groomer@example.com",
@@ -89,7 +92,7 @@ function buildApp(
) {
const app = new Hono<AppEnv>();
app.use("*", async (c, next) => {
c.set("jwtPayload", { sub: staffLookupResult?.oidcSub ?? "unknown-sub" });
c.set("jwtPayload", { sub: staffLookupResult?.userId ?? "unknown-sub" });
await next();
});
app.use("*", middleware);
@@ -106,7 +109,7 @@ function buildWithStaff(
) {
const app = new Hono<AppEnv>();
app.use("*", async (c, next) => {
c.set("jwtPayload", { sub: staffRow.oidcSub ?? "" });
c.set("jwtPayload", { sub: staffRow.userId ?? "" });
c.set("staff", staffRow);
await next();
});
+14 -1
View File
@@ -2,6 +2,8 @@ import { serve } from "@hono/node-server";
import { Hono } from "hono";
import { logger } from "hono/logger";
import { cors } from "hono/cors";
import { toNodeHandler } from "better-auth/node";
import { auth } from "./lib/auth.js";
import { clientsRouter } from "./routes/clients.js";
import { petsRouter } from "./routes/pets.js";
import { servicesRouter } from "./routes/services.js";
@@ -65,13 +67,24 @@ app.get("/api/branding", async (c) => {
// Public iCal calendar feed — token auth in URL, no auth middleware required
app.route("/api/calendar", calendarRouter);
// Better-Auth handler — public, handles OAuth callbacks, session management
// Mounted BEFORE auth middleware so it's accessible without authentication
app.on(["GET", "POST", "PUT", "PATCH", "DELETE", "OPTIONS"], "/api/auth/**", (c) => {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const { incoming, outgoing } = c.env as any;
return toNodeHandler(auth)(incoming, outgoing);
});
// Protected API routes
const api = app.basePath("/api");
api.use("*", authMiddleware);
api.use("*", resolveStaffMiddleware);
// ── Role guards ────────────────────────────────────────────────────────────────
// Manager-only: staff, admin settings, reports, invoices, impersonation
// Manager-only: admin settings, reports, invoices, impersonation
// Staff CRUD: all roles may READ; manager-only for CREATE/UPDATE/DELETE
api.on(["GET"], "/staff/*", requireRole("manager", "receptionist", "groomer"));
api.use("/staff/*", requireRole("manager"));
api.use("/admin/*", requireRole("manager"));
api.use("/reports/*", requireRole("manager"));
+48
View File
@@ -0,0 +1,48 @@
import { betterAuth } from "better-auth";
import { drizzleAdapter } from "better-auth/adapters/drizzle";
import { genericOAuth } from "better-auth/plugins";
import { getDb } from "@groombook/db";
const OIDC_ISSUER = process.env.OIDC_ISSUER;
const OIDC_CLIENT_ID = process.env.OIDC_CLIENT_ID;
const OIDC_CLIENT_SECRET = process.env.OIDC_CLIENT_SECRET;
const BETTER_AUTH_SECRET = process.env.BETTER_AUTH_SECRET;
const BETTER_AUTH_URL = process.env.BETTER_AUTH_URL ?? "http://localhost:3000";
if (!BETTER_AUTH_SECRET && process.env.AUTH_DISABLED !== "true") {
throw new Error(
"[FATAL] BETTER_AUTH_SECRET environment variable is required when auth is enabled"
);
}
export const auth = betterAuth({
database: drizzleAdapter(getDb(), {
provider: "pg",
}),
secret: BETTER_AUTH_SECRET,
baseURL: BETTER_AUTH_URL,
plugins: [
genericOAuth({
config: [
{
providerId: "authentik",
clientId: OIDC_CLIENT_ID ?? "",
clientSecret: OIDC_CLIENT_SECRET ?? "",
discoveryUrl: OIDC_ISSUER
? `${OIDC_ISSUER}/.well-known/openid-configuration`
: undefined,
scopes: ["openid", "profile", "email"],
},
],
}),
],
session: {
expiresIn: 60 * 60 * 24 * 7, // 7 days
updateAge: 60 * 60 * 24, // 1 day
cookieCache: {
enabled: true,
maxAge: 5 * 60, // 5 minutes
},
},
trustedOrigins: [process.env.CORS_ORIGIN ?? "http://localhost:5173"],
});
+20 -39
View File
@@ -1,34 +1,18 @@
import type { MiddlewareHandler } from "hono";
import { createRemoteJWKSet, jwtVerify } from "jose";
import { auth } from "../lib/auth.js";
// Authentik OIDC configuration — loaded from env at startup
const OIDC_ISSUER = process.env.OIDC_ISSUER;
const OIDC_AUDIENCE = process.env.OIDC_AUDIENCE;
let jwks: ReturnType<typeof createRemoteJWKSet> | null = null;
function getJwks() {
if (!OIDC_ISSUER) throw new Error("OIDC_ISSUER is not set");
if (!jwks) {
jwks = createRemoteJWKSet(
new URL(`${OIDC_ISSUER}/application/o/groombook/jwks/`)
);
}
return jwks;
export interface AuthUser {
id: string;
email: string;
name: string;
}
export interface JwtPayload {
sub: string;
email?: string;
name?: string;
}
// Guard: refuse to start with AUTH_DISABLED in production (fixes #22).
// Guard: refuse to start with AUTH_DISABLED in production.
if (process.env.AUTH_DISABLED === "true") {
if (process.env.NODE_ENV === "production") {
console.error(
"[FATAL] AUTH_DISABLED=true is not allowed in production. " +
"Remove AUTH_DISABLED from your environment and configure OIDC_ISSUER."
"Remove AUTH_DISABLED from your environment and configure Better-Auth."
);
process.exit(1);
}
@@ -42,27 +26,24 @@ export const authMiddleware: MiddlewareHandler = async (c, next) => {
if (process.env.AUTH_DISABLED === "true") {
const devUserId = c.req.header("X-Dev-User-Id");
const sub = devUserId ?? "dev-user";
c.set("jwtPayload", { sub } as JwtPayload);
c.set("jwtPayload", { sub } as { sub: string });
await next();
return;
}
const authorization = c.req.header("Authorization");
if (!authorization?.startsWith("Bearer ")) {
const session = await auth.api.getSession({
headers: c.req.raw.headers,
});
if (!session) {
return c.json({ error: "Unauthorized" }, 401);
}
const token = authorization.slice(7);
try {
const { payload } = await jwtVerify(token, getJwks(), {
issuer: OIDC_ISSUER,
audience: OIDC_AUDIENCE,
});
c.set("jwtPayload", payload as JwtPayload);
await next();
} catch {
return c.json({ error: "Invalid or expired token" }, 401);
}
// Set jwtPayload with sub = Better-Auth user ID for backward compat with resolveStaffMiddleware
c.set("jwtPayload", {
sub: session.user.id,
email: session.user.email,
name: session.user.name,
});
await next();
};
+29 -9
View File
@@ -1,13 +1,12 @@
import type { MiddlewareHandler } from "hono";
import { eq, getDb, staff } from "@groombook/db";
import type { JwtPayload } from "./auth.js";
export type StaffRole = "groomer" | "receptionist" | "manager";
export type StaffRow = typeof staff.$inferSelect;
export interface AppEnv {
Variables: {
jwtPayload: JwtPayload;
jwtPayload: { sub: string; email?: string; name?: string };
staff: StaffRow;
};
}
@@ -16,8 +15,8 @@ export interface AppEnv {
* Resolves the authenticated staff record from the DB and stores it in context.
* Must be applied after authMiddleware on all protected routes.
*
* Dev mode (AUTH_DISABLED=true): resolves staff by X-Dev-User-Id header (treated
* as oidcSub), or falls back to the first manager in the DB.
* Dev mode (AUTH_DISABLED=true): resolves staff by X-Dev-User-Id header (Better-Auth
* user ID), or falls back to the first manager in the DB.
*/
export const resolveStaffMiddleware: MiddlewareHandler<AppEnv> = async (
c,
@@ -41,34 +40,55 @@ export const resolveStaffMiddleware: MiddlewareHandler<AppEnv> = async (
await next();
return;
}
// Treat X-Dev-User-Id as the staff database id (the frontend stores staff.id)
// Treat X-Dev-User-Id as the Better-Auth user ID first
const [row] = await db
.select()
.from(staff)
.where(eq(staff.userId, devUserId));
if (row) {
c.set("staff", row);
await next();
return;
}
// Fallback: if userId is null, treat X-Dev-User-Id as staff.id (dev login
// may send the primary key for staff records that predate the userId field)
const [fallbackRow] = await db
.select()
.from(staff)
.where(eq(staff.id, devUserId));
if (!row) {
if (!fallbackRow) {
return c.json(
{ error: "Forbidden: no staff record found for X-Dev-User-Id" },
403
);
}
c.set("staff", row);
c.set("staff", fallbackRow);
await next();
return;
}
const jwt = c.get("jwtPayload");
const [row] = await db
.select()
.from(staff)
.where(eq(staff.userId, jwt.sub));
if (row) {
c.set("staff", row);
await next();
return;
}
// Fallback: staff records that predate the userId field may still have oidcSub
const [fallbackRow] = await db
.select()
.from(staff)
.where(eq(staff.oidcSub, jwt.sub));
if (!row) {
if (!fallbackRow) {
return c.json(
{ error: "Forbidden: no staff record found for authenticated user" },
403
);
}
c.set("staff", row);
c.set("staff", fallbackRow);
await next();
};
+1 -1
View File
@@ -1,6 +1,6 @@
import { Hono } from "hono";
import { zValidator } from "@hono/zod-validator";
import { z } from "zod";
import { z } from "zod/v3";
import {
and,
eq,
+1 -1
View File
@@ -1,6 +1,6 @@
import { Hono } from "hono";
import { zValidator } from "@hono/zod-validator";
import { z } from "zod";
import { z } from "zod/v3";
import { randomBytes } from "node:crypto";
import {
and,
+1 -1
View File
@@ -1,6 +1,6 @@
import { Hono } from "hono";
import { zValidator } from "@hono/zod-validator";
import { z } from "zod";
import { z } from "zod/v3";
import {
and,
eq,
+1 -1
View File
@@ -1,6 +1,6 @@
import { Hono } from "hono";
import { zValidator } from "@hono/zod-validator";
import { z } from "zod";
import { z } from "zod/v3";
import { and, eq, exists, getDb, or, clients, appointments } from "@groombook/db";
import type { AppEnv } from "../middleware/rbac.js";
+1
View File
@@ -20,6 +20,7 @@ devRouter.get("/users", async (c) => {
const staffList = await db
.select({
id: staff.id,
userId: staff.userId,
name: staff.name,
email: staff.email,
role: staff.role,
+1 -1
View File
@@ -1,6 +1,6 @@
import { Hono } from "hono";
import { zValidator } from "@hono/zod-validator";
import { z } from "zod";
import { z } from "zod/v3";
import { desc, eq, getDb, groomingVisitLogs } from "@groombook/db";
export const groomingLogsRouter = new Hono();
+1 -1
View File
@@ -1,6 +1,6 @@
import { Hono } from "hono";
import { zValidator } from "@hono/zod-validator";
import { z } from "zod";
import { z } from "zod/v3";
import {
and,
eq,
+1 -1
View File
@@ -1,6 +1,6 @@
import { Hono } from "hono";
import { zValidator } from "@hono/zod-validator";
import { z } from "zod";
import { z } from "zod/v3";
import {
and,
eq,
+1 -1
View File
@@ -1,6 +1,6 @@
import { Hono } from "hono";
import { zValidator } from "@hono/zod-validator";
import { z } from "zod";
import { z } from "zod/v3";
import { and, eq, exists, getDb, or, pets, appointments } from "@groombook/db";
import type { AppEnv } from "../middleware/rbac.js";
import {
+1 -1
View File
@@ -1,6 +1,6 @@
import { Hono } from "hono";
import { zValidator } from "@hono/zod-validator";
import { z } from "zod";
import { z } from "zod/v3";
import { and, eq, getDb, appointments, impersonationSessions, waitlistEntries } from "@groombook/db";
import type { AppEnv } from "../middleware/rbac.js";
+1 -1
View File
@@ -1,6 +1,6 @@
import { Hono } from "hono";
import { zValidator } from "@hono/zod-validator";
import { z } from "zod";
import { z } from "zod/v3";
import { eq, getDb, services } from "@groombook/db";
export const servicesRouter = new Hono();
+1 -1
View File
@@ -1,6 +1,6 @@
import { Hono } from "hono";
import { zValidator } from "@hono/zod-validator";
import { z } from "zod";
import { z } from "zod/v3";
import { eq, getDb, businessSettings } from "@groombook/db";
export const settingsRouter = new Hono();
+1 -1
View File
@@ -1,6 +1,6 @@
import { Hono } from "hono";
import { zValidator } from "@hono/zod-validator";
import { z } from "zod";
import { z } from "zod/v3";
import { randomBytes } from "node:crypto";
import { and, eq, getDb, ne, staff, appointments } from "@groombook/db";
import type { AppEnv } from "../middleware/rbac.js";