From bc1f11a9013bb7eef1e98cba800d957d83a26c6b Mon Sep 17 00:00:00 2001 From: Paperclip Date: Sun, 12 Apr 2026 02:47:17 +0000 Subject: [PATCH 1/2] feat(GRO-565): Better Auth Phase 3 - password change, OIDC discovery, session cleanup, email verification Co-Authored-By: Paperclip --- apps/api/src/lib/auth.ts | 71 ++++++++++++++++--- apps/api/src/services/reminders.ts | 14 ++++ apps/web/src/lib/auth-client.ts | 2 +- .../src/portal/sections/AccountSettings.tsx | 35 +++++++-- 4 files changed, 105 insertions(+), 17 deletions(-) diff --git a/apps/api/src/lib/auth.ts b/apps/api/src/lib/auth.ts index bb68b23..d88b91c 100644 --- a/apps/api/src/lib/auth.ts +++ b/apps/api/src/lib/auth.ts @@ -3,6 +3,7 @@ import { drizzleAdapter } from "better-auth/adapters/drizzle"; import { genericOAuth } from "better-auth/plugins"; import { getDb, authProviderConfig, eq } from "@groombook/db"; import { decryptSecret } from "@groombook/db"; +import { sendEmail } from "../services/email.js"; const BETTER_AUTH_SECRET = process.env.BETTER_AUTH_SECRET; const BETTER_AUTH_URL = process.env.BETTER_AUTH_URL ?? "http://localhost:3000"; @@ -176,6 +177,52 @@ export async function initAuth(): Promise { const hasGoogle = !!(process.env.GOOGLE_CLIENT_ID && process.env.GOOGLE_CLIENT_SECRET); const hasGitHub = !!(process.env.GITHUB_CLIENT_ID && process.env.GITHUB_CLIENT_SECRET); + // Fetch OIDC discovery document to derive canonical provider URLs. + // Replace the host of token/userinfo endpoints with internalBaseUrl when set, + // while keeping authorizationUrl public for browser redirects. + const discoveryUrlStr = `${providerConfig.issuerUrl}/.well-known/openid-configuration`; + let oidcConfig: Record = {}; + try { + const discoveryRes = await fetch(discoveryUrlStr); + if (discoveryRes.ok) { + const discovery = await discoveryRes.json() as { + authorization_endpoint?: string; + token_endpoint?: string; + userinfo_endpoint?: string; + }; + const replaceHost = (url: string, newHost: string) => { + try { + const parsed = new URL(url); + const newParsed = new URL(newHost); + return `${newParsed.origin}${parsed.pathname}${parsed.search}`; + } catch { + return url; + } + }; + const authzUrl = discovery.authorization_endpoint; + const tokenUrl = discovery.token_endpoint; + const userInfoUrl = discovery.userinfo_endpoint; + if (authzUrl && tokenUrl && userInfoUrl) { + oidcConfig = { + authorizationUrl: authzUrl, + tokenUrl: providerConfig.internalBaseUrl + ? replaceHost(tokenUrl, providerConfig.internalBaseUrl) + : tokenUrl, + userInfoUrl: providerConfig.internalBaseUrl + ? replaceHost(userInfoUrl, providerConfig.internalBaseUrl) + : userInfoUrl, + }; + console.log("[auth] OIDC discovery successful, provider:", providerConfig.providerId); + } else { + console.warn("[auth] OIDC discovery missing required endpoints, using discoveryUrl only"); + } + } else { + console.warn(`[auth] OIDC discovery failed (${discoveryRes.status}), using discoveryUrl only`); + } + } catch (err) { + console.warn(`[auth] OIDC discovery fetch failed: ${err}, using discoveryUrl only`); + } + // Build Better-Auth instance using resolved config authInstance = betterAuth({ database: drizzleAdapter(db, { @@ -192,6 +239,19 @@ export async function initAuth(): Promise { account: { storeStateStrategy: "cookie" as const, }, + emailAndPassword: { + enabled: true, + emailVerification: { + sendVerificationEmail: async ({ user, url }: { user: { email: string }; url: string }) => { + await sendEmail({ + to: user.email, + subject: "Verify your GroomBook email", + text: `Click the link to verify your email: ${url}`, + html: `

Click the link to verify your email:

${url}`, + }); + }, + }, + }, plugins: [ genericOAuth({ config: [ @@ -199,15 +259,8 @@ export async function initAuth(): Promise { providerId: providerConfig.providerId, clientId: providerConfig.clientId, clientSecret: providerConfig.clientSecret, - ...(providerConfig.internalBaseUrl - ? { - authorizationUrl: `${new URL(providerConfig.issuerUrl).origin}/application/o/authorize/`, - tokenUrl: `${providerConfig.internalBaseUrl}/application/o/token/`, - userInfoUrl: `${providerConfig.internalBaseUrl}/application/o/userinfo/`, - } - : { - discoveryUrl: `${providerConfig.issuerUrl}/.well-known/openid-configuration`, - }), + discoveryUrl: discoveryUrlStr, + ...(Object.keys(oidcConfig).length > 0 ? oidcConfig : {}), scopes: providerConfig.scopes.split(" ").filter(Boolean), }, ], diff --git a/apps/api/src/services/reminders.ts b/apps/api/src/services/reminders.ts index 3bbfae0..442b6c3 100644 --- a/apps/api/src/services/reminders.ts +++ b/apps/api/src/services/reminders.ts @@ -12,6 +12,7 @@ import { services, staff, reminderLogs, + session, } from "@groombook/db"; import { buildReminderEmail, @@ -155,6 +156,19 @@ export function startReminderScheduler(): void { runReminderCheck().catch((err) => { console.error("[reminders] Error during reminder check:", err); }); + runSessionCleanup().catch((err) => { + console.error("[reminders] Error during session cleanup:", err); + }); }); console.log("[reminders] Reminder scheduler started"); } + +// Deletes expired sessions from the database. +// Runs every minute alongside reminder checks. +export async function runSessionCleanup(): Promise { + const db = getDb(); + const now = new Date(); + await db + .delete(session) + .where(lt(session.expiresAt, now)); +} diff --git a/apps/web/src/lib/auth-client.ts b/apps/web/src/lib/auth-client.ts index 12ff8ed..6a9939a 100644 --- a/apps/web/src/lib/auth-client.ts +++ b/apps/web/src/lib/auth-client.ts @@ -4,4 +4,4 @@ export const authClient = createAuthClient({ baseURL: import.meta.env.VITE_API_URL ?? "", }); -export const { signIn, signOut, useSession } = authClient; \ No newline at end of file +export const { signIn, signOut, useSession, changePassword } = authClient; \ No newline at end of file diff --git a/apps/web/src/portal/sections/AccountSettings.tsx b/apps/web/src/portal/sections/AccountSettings.tsx index eb69ed0..7957c91 100644 --- a/apps/web/src/portal/sections/AccountSettings.tsx +++ b/apps/web/src/portal/sections/AccountSettings.tsx @@ -1,6 +1,7 @@ import React, { useState, useEffect } from "react"; import { User, Lock, PawPrint, FileCheck, Plus, Archive } from "lucide-react"; import { PetForm } from "./PetForm.js"; +import { authClient } from "../../lib/auth-client.js"; interface Props { sessionId: string | null; @@ -148,9 +149,11 @@ function PasswordChange({ readOnly }: { readOnly: boolean }) { const [newPassword, setNewPassword] = useState(""); const [confirmPassword, setConfirmPassword] = useState(""); const [error, setError] = useState(null); + const [success, setSuccess] = useState(false); + const [loading, setLoading] = useState(false); const passwordsMatch = newPassword === confirmPassword; - const canSubmit = currentPassword.length > 0 && newPassword.length > 0 && passwordsMatch; + const canSubmit = newPassword.length > 0 && passwordsMatch && !loading; if (readOnly) { return ( @@ -160,17 +163,34 @@ function PasswordChange({ readOnly }: { readOnly: boolean }) { ); } - function handleSubmit() { + async function handleSubmit() { if (!canSubmit) return; if (newPassword !== confirmPassword) { setError("Passwords do not match."); return; } - // TODO: Wire up to actual password-change API endpoint once backend support exists setError(null); - setCurrentPassword(""); - setNewPassword(""); - setConfirmPassword(""); + setLoading(true); + try { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const result = await (authClient as any).changePassword({ + currentPassword, + newPassword, + }); + if (result.error) { + setError(result.error.message ?? "Failed to change password."); + } else { + setSuccess(true); + setCurrentPassword(""); + setNewPassword(""); + setConfirmPassword(""); + setTimeout(() => setSuccess(false), 4000); + } + } catch { + setError("An unexpected error occurred."); + } finally { + setLoading(false); + } } return ( @@ -205,12 +225,13 @@ function PasswordChange({ readOnly }: { readOnly: boolean }) { /> {error &&

{error}

} + {success &&

Password updated successfully.

} From 15131b72f0388ae0c2bc63cff67f4875d52dff32 Mon Sep 17 00:00:00 2001 From: "groombook-cto[bot]" <269737991+groombook-cto[bot]@users.noreply.github.com> Date: Sun, 12 Apr 2026 03:30:45 +0000 Subject: [PATCH 2/2] fix(GRO-574): add rate_limit table migration for Better Auth Adds the missing rate_limit table that Better Auth v1.5.6 requires when rateLimit.storage is set to 'database'. Without this table, all auth endpoints return HTTP 500. Also includes GRO-566: SKIP_OOBE env var to bypass setup wizard in dev/test. cc @cpfarhood --- .env.example | 6 ++++ apps/api/src/__tests__/setup.test.ts | 42 ++++++++++++++++++++++ apps/api/src/routes/setup.ts | 11 ++++++ packages/db/migrations/0025_rate_limit.sql | 6 ++++ packages/db/migrations/meta/_journal.json | 7 ++++ 5 files changed, 72 insertions(+) create mode 100644 packages/db/migrations/0025_rate_limit.sql diff --git a/.env.example b/.env.example index 293f815..f91cd54 100644 --- a/.env.example +++ b/.env.example @@ -11,6 +11,12 @@ AUTH_DISABLED=false OIDC_ISSUER=https://authentik.example.com OIDC_AUDIENCE=groombook +# ── Setup Wizard ───────────────────────────────────────────────────────────── +# When SKIP_OOBE=true, the setup wizard is bypassed regardless of whether a +# super user exists in the database. Useful in dev/test environments where the +# database has data but the setup wizard would otherwise block access. +SKIP_OOBE=false + # ── API ─────────────────────────────────────────────────────────────────────── PORT=3000 CORS_ORIGIN=http://localhost:8080 diff --git a/apps/api/src/__tests__/setup.test.ts b/apps/api/src/__tests__/setup.test.ts index 8fc4ffd..9884e96 100644 --- a/apps/api/src/__tests__/setup.test.ts +++ b/apps/api/src/__tests__/setup.test.ts @@ -418,6 +418,48 @@ describe("GET /setup/status — OOBE bootstrap logic", () => { expect(body.showAuthProviderStep).toBe(false); // DB config already exists expect(body.authConfigExists).toBe(true); }); + + it("SKIP_OOBE=true bypasses setup check regardless of DB state", async () => { + dbStaffRows = []; // no super user + dbAuthConfigRows = []; + process.env.SKIP_OOBE = "true"; + + const app = makeApp(); + const { status, body } = await getStatus(app); + + expect(status).toBe(200); + expect(body.needsSetup).toBe(false); + expect(body.showAuthProviderStep).toBe(false); + expect(body.authConfigExists).toBe(false); + expect(body.authEnvVarsSet).toBe(false); + expect(body.skipped).toBe(true); + }); + + it("SKIP_OOBE=1 also bypasses setup check", async () => { + dbStaffRows = []; + dbAuthConfigRows = []; + process.env.SKIP_OOBE = "1"; + + const app = makeApp(); + const { status, body } = await getStatus(app); + + expect(status).toBe(200); + expect(body.needsSetup).toBe(false); + expect(body.skipped).toBe(true); + }); + + it("SKIP_OOBE=yes also bypasses setup check", async () => { + dbStaffRows = []; + dbAuthConfigRows = []; + process.env.SKIP_OOBE = "yes"; + + const app = makeApp(); + const { status, body } = await getStatus(app); + + expect(status).toBe(200); + expect(body.needsSetup).toBe(false); + expect(body.skipped).toBe(true); + }); }); describe("POST /setup/auth-provider — OOBE bootstrap", () => { diff --git a/apps/api/src/routes/setup.ts b/apps/api/src/routes/setup.ts index 079636a..a614b5c 100644 --- a/apps/api/src/routes/setup.ts +++ b/apps/api/src/routes/setup.ts @@ -9,6 +9,17 @@ export const setupRouter = new Hono(); // GET /api/setup/status — public (no auth), returns whether setup is needed // and whether the auth provider bootstrap step should be shown setupRouter.get("/status", async (c) => { + const skipOobe = ["true", "1", "yes"].includes((process.env.SKIP_OOBE || "").toLowerCase()); + if (skipOobe) { + return c.json({ + needsSetup: false, + showAuthProviderStep: false, + authConfigExists: false, + authEnvVarsSet: false, + skipped: true, + }); + } + const db = getDb(); // Check if any super user exists diff --git a/packages/db/migrations/0025_rate_limit.sql b/packages/db/migrations/0025_rate_limit.sql new file mode 100644 index 0000000..0a83e14 --- /dev/null +++ b/packages/db/migrations/0025_rate_limit.sql @@ -0,0 +1,6 @@ +-- Better-Auth rate limiting table (GRO-574) +CREATE TABLE "rate_limit" ( + key TEXT NOT NULL PRIMARY KEY, + count INTEGER NOT NULL, + last_request BIGINT NOT NULL +); diff --git a/packages/db/migrations/meta/_journal.json b/packages/db/migrations/meta/_journal.json index 3a1ec9c..7ca1dc5 100644 --- a/packages/db/migrations/meta/_journal.json +++ b/packages/db/migrations/meta/_journal.json @@ -176,6 +176,13 @@ "when": 1775396067192, "tag": "0024_invoice_indexes", "breakpoints": true + }, + { + "idx": 25, + "version": "7", + "when": 1775482467192, + "tag": "0025_rate_limit", + "breakpoints": true } ] } \ No newline at end of file