From 88d9c8e097c98aa4e1ed93574184a3e63c9b488a Mon Sep 17 00:00:00 2001 From: Bobban Rydh Date: Fri, 2 Oct 2026 22:40:46 +0200 Subject: [PATCH] Record sign-ins, new accounts, automatic log trimming and what settings changed A check of every write path found gaps in what the audit log captured: - Sign-ins and sign-outs are now recorded with the IP they came from (sign-out is recorded first and can't block signing out). - A new account is recorded when it's created on first sign-in, including when the very first user becomes admin - so the log shows who gained access, not only who changed things. - The automatic log purge, which deletes audit entries, now records itself, attributed to "system". recordAudit() takes an optional actor for this. It only records when something was actually deleted. - Settings updates record what changed (before and after) instead of only which sections were touched. The notification channels (Gotify, ntfy, SMTP, webhook) record field names only: they hold credentials, and webhook URLs and public ntfy topics act as secrets, while the audit log is readable by operators and Settings is admin-only. - Integration edits record renames, enabling/disabling, whether credentials were replaced (never the credentials), and which settings fields changed (names only). The Privacy page and README now say sign-ins store an IP in the audit log. Verified through the real routes against a scratch database: user creation, the logout route, the automatic purge, settings and integration edits - including that a secret token and a webhook URL appear nowhere in the stored entries. The sign-in callback itself needs a real identity provider and wasn't run. Co-Authored-By: Claude Sonnet 5.5 --- README.md | 2 + server/src/auth/router.ts | 47 +++++++++++++++++++- server/src/auth/users.ts | 6 +-- server/src/routes/integrations.ts | 16 ++++++- server/src/routes/settings.ts | 4 +- server/src/services/audit.ts | 11 +++-- server/src/services/logRetentionScheduler.ts | 7 +++ server/src/services/settingsDiff.ts | 41 +++++++++++++++++ web/src/pages/Privacy.tsx | 7 ++- 9 files changed, 129 insertions(+), 12 deletions(-) create mode 100644 server/src/services/settingsDiff.ts diff --git a/README.md b/README.md index 3470f27..f6ddca4 100644 --- a/README.md +++ b/README.md @@ -18,6 +18,8 @@ All modules from the original plan are built: - Monorepo scaffold, Tabler-themed app shell with a grouped sidebar (Infrastructure, Network, Automation, Operations, Administration; groups open on demand, the one holding the current page is always open, and what you leave open is remembered) - Authentik OIDC login, roles (first user to sign in becomes admin), audit log + (changes made in the app, sign-ins and sign-outs with the IP they came from, new accounts, and the log's own + automatic trimming — attributed to "system"; settings changes show what changed, but never credentials) - **Dashboard** — an overview of every system this app tracks, all sharing one widget-card design (label + status badge, a small stat row, then its own breakdown): DNS (domain/record counts per provider, cached records by diff --git a/server/src/auth/router.ts b/server/src/auth/router.ts index 8f19236..82bfae4 100644 --- a/server/src/auth/router.ts +++ b/server/src/auth/router.ts @@ -1,8 +1,12 @@ import { Router } from "express"; import * as client from "openid-client"; +import { eq } from "drizzle-orm"; import { getOidcConfig } from "./oidc.js"; import { upsertUserFromLogin } from "./users.js"; import { env } from "../env.js"; +import { db } from "../db/client.js"; +import { users } from "../db/schema.js"; +import { recordAudit } from "../services/audit.js"; export const authRouter = Router(); @@ -63,7 +67,28 @@ authRouter.get("/callback", async (req, res, next) => { // fall back to ID token claims already captured above } - await upsertUserFromLogin({ sub: claims.sub, email, name }); + const { user, created } = await upsertUserFromLogin({ sub: claims.sub, email, name }); + + // Access to the app is itself something worth being able to look back on: who got an account (and the very first + // one becomes admin), and every sign-in with where it came from. + if (created) { + await recordAudit({ + actor: user, + category: "user", + action: "create", + targetType: "user", + targetId: user.id, + detail: { name: user.name ?? user.email ?? user.oidcSub, role: user.role, firstUser: user.role === "admin" }, + }); + } + await recordAudit({ + actor: user, + category: "session", + action: "login", + targetType: "user", + targetId: user.id, + detail: { name: user.name ?? user.email ?? user.oidcSub, ip: req.ip }, + }); delete req.session.pendingAuth; req.session.user = { @@ -85,6 +110,26 @@ authRouter.get("/callback", async (req, res, next) => { authRouter.get("/logout", async (req, res, next) => { const idToken = req.session.user?.idToken; + + // Recorded first, and never allowed to get in the way of signing out. + if (req.session.user) { + try { + const [user] = await db.select().from(users).where(eq(users.oidcSub, req.session.user.sub)).limit(1); + if (user) { + await recordAudit({ + actor: user, + category: "session", + action: "logout", + targetType: "user", + targetId: user.id, + detail: { name: user.name ?? user.email ?? user.oidcSub, ip: req.ip }, + }); + } + } catch (err) { + console.error("[auth] couldn't record sign-out:", err); + } + } + try { const config = await getOidcConfig(); let endSessionUrl: URL | undefined; diff --git a/server/src/auth/users.ts b/server/src/auth/users.ts index 92a75ca..079eab7 100644 --- a/server/src/auth/users.ts +++ b/server/src/auth/users.ts @@ -11,7 +11,7 @@ export async function upsertUserFromLogin(params: { sub: string; email?: string; name?: string; -}) { +}): Promise<{ user: typeof users.$inferSelect; created: boolean }> { const [existing] = await db.select().from(users).where(eq(users.oidcSub, params.sub)).limit(1); const now = new Date().toISOString(); @@ -25,7 +25,7 @@ export async function upsertUserFromLogin(params: { }) .where(eq(users.id, existing.id)) .returning(); - return updated; + return { user: updated, created: false }; } const anyUser = await db.select({ id: users.id }).from(users).limit(1); @@ -41,5 +41,5 @@ export async function upsertUserFromLogin(params: { lastLoginAt: now, }) .returning(); - return created; + return { user: created, created: true }; } diff --git a/server/src/routes/integrations.ts b/server/src/routes/integrations.ts index b152d77..5a54824 100644 --- a/server/src/routes/integrations.ts +++ b/server/src/routes/integrations.ts @@ -139,10 +139,18 @@ integrationsRouter.patch("/:id", requireRole("admin"), asyncHandler(async (req, let credentialId = existing.credentialId; let configJson = existing.config; let baseUrl = existing.baseUrl; + // For the audit entry: what this edit actually changed. Names only for settings (operators can read the audit + // log but not an integration's config), and never anything about the credentials beyond "they were replaced". + let credentialsReplaced = false; + let fieldsChanged: string[] = []; if (parsed.data.config) { const loaded = await loadIntegrationConfig(id); const { secretFields, nonSecretFields } = splitIntegrationConfig(existing.type, parsed.data.config); + credentialsReplaced = Object.keys(secretFields).length > 0; + fieldsChanged = Object.keys(nonSecretFields).filter( + (key) => String(loaded?.config[key] ?? "") !== String(nonSecretFields[key] ?? ""), + ); const mergedNonSecret = { ...(loaded?.config ?? {}), ...nonSecretFields }; for (const field of INTEGRATION_FIELDS[existing.type] ?? []) { if (field.secret) delete (mergedNonSecret as Record)[field.key]; @@ -188,7 +196,13 @@ integrationsRouter.patch("/:id", requireRole("admin"), asyncHandler(async (req, action: "update", targetType: "integration", targetId: id, - detail: { name: updated.name }, + detail: { + name: updated.name, + ...(existing.name !== updated.name ? { renamedFrom: existing.name } : {}), + ...(existing.enabled !== updated.enabled ? { enabled: updated.enabled } : {}), + credentialsReplaced, + fieldsChanged, + }, }); res.json({ diff --git a/server/src/routes/settings.ts b/server/src/routes/settings.ts index 15abc11..9d89f5b 100644 --- a/server/src/routes/settings.ts +++ b/server/src/routes/settings.ts @@ -3,6 +3,7 @@ import { z } from "zod"; import { requireAuth, requireRole } from "../auth/middleware.js"; import { recordAudit } from "../services/audit.js"; import { getSettings, updateSettings } from "../services/settingsStore.js"; +import { describeSettingsChanges } from "../services/settingsDiff.js"; import { scheduleSecretExpiryCheck } from "../services/secretExpiryScheduler.js"; import { scheduleTailscaleKeyExpiryCheck } from "../services/tailscaleKeyExpiryScheduler.js"; import { scheduleDockerUpdateCheck } from "../services/dockerUpdateScheduler.js"; @@ -106,6 +107,7 @@ settingsRouter.put("/", requireRole("admin"), asyncHandler(async (req, res) => { return res.status(400).json({ error: "invalid_body", details: parsed.error.flatten() }); } + const before = await getSettings(); const updated = await updateSettings(parsed.data); if (parsed.data.notifications) { @@ -127,7 +129,7 @@ settingsRouter.put("/", requireRole("admin"), asyncHandler(async (req, res) => { category: "settings", action: "update", targetType: "settings", - detail: { sections: Object.keys(parsed.data) }, + detail: { sections: Object.keys(parsed.data), changes: describeSettingsChanges(before, parsed.data) }, }); res.json({ settings: updated }); diff --git a/server/src/services/audit.ts b/server/src/services/audit.ts index 39705c9..c78b805 100644 --- a/server/src/services/audit.ts +++ b/server/src/services/audit.ts @@ -3,9 +3,12 @@ import { auditLog, users } from "../db/schema.js"; type CurrentUser = typeof users.$inferSelect; -/** Records one audit-log entry. Call this from any route that mutates state or takes an action. */ +/** Who an automatic, no-one-clicked-anything entry is attributed to. */ +export const SYSTEM_ACTOR_LABEL = "system"; + +/** Records one audit-log entry. Call this from any route that mutates state or takes an action. Leave `actor` out for something the app did by itself. */ export async function recordAudit(params: { - actor: CurrentUser; + actor?: CurrentUser; category: string; action: string; targetType?: string; @@ -13,8 +16,8 @@ export async function recordAudit(params: { detail?: unknown; }) { await db.insert(auditLog).values({ - actorUserId: params.actor.id, - actorLabel: params.actor.name ?? params.actor.email ?? params.actor.oidcSub, + actorUserId: params.actor?.id, + actorLabel: params.actor ? (params.actor.name ?? params.actor.email ?? params.actor.oidcSub) : SYSTEM_ACTOR_LABEL, category: params.category, action: params.action, targetType: params.targetType, diff --git a/server/src/services/logRetentionScheduler.ts b/server/src/services/logRetentionScheduler.ts index e50386c..d96812e 100644 --- a/server/src/services/logRetentionScheduler.ts +++ b/server/src/services/logRetentionScheduler.ts @@ -1,5 +1,6 @@ import { getSettings, getInternalFlag, setInternalFlag } from "./settingsStore.js"; import { purgeOldLogs } from "./logRetention.js"; +import { recordAudit } from "./audit.js"; const LAST_RUN_FLAG = "logRetentionLastRunAt"; @@ -9,6 +10,12 @@ async function runPurge(): Promise { const result = await purgeOldLogs(logRetention.retentionDays); await setInternalFlag(LAST_RUN_FLAG, new Date().toISOString()); if (result.diagDeleted || result.auditDeleted) { + // Trimming the audit log is itself something to be able to look back on — attributed to the system, since no one asked for it. + await recordAudit({ + category: "settings", + action: "purge_logs", + detail: { automatic: true, retentionDays: logRetention.retentionDays, ...result }, + }); console.log( `[logRetention] purged ${result.diagDeleted} diagnostic log and ${result.auditDeleted} audit log entries older than ${logRetention.retentionDays} days`, ); diff --git a/server/src/services/settingsDiff.ts b/server/src/services/settingsDiff.ts new file mode 100644 index 0000000..b84662a --- /dev/null +++ b/server/src/services/settingsDiff.ts @@ -0,0 +1,41 @@ +/** + * What a settings update actually changed, in a form fit for the audit log. + * + * The audit log can be read by operators, but Settings can't — so values only go in for sections an operator could + * already see in the app. The notification channels (Gotify, ntfy, SMTP, webhook) hold credentials, and even their + * addresses can act as one (a webhook URL carries its own token; a public ntfy topic is the only thing protecting + * it), so for those only the names of the fields that changed are recorded, never what they changed to or from. + */ +const NAMES_ONLY_SECTIONS = new Set(["gotify", "ntfy", "smtp", "webhook"]); + +/** Anything longer than this isn't a useful thing to read in a table cell. */ +const MAX_VALUE_JSON = 300; + +export type SettingsChange = { from: unknown; to: unknown } | "(changed)"; + +function same(a: unknown, b: unknown): boolean { + return JSON.stringify(a) === JSON.stringify(b); +} + +function capture(value: unknown): unknown { + return value !== undefined && JSON.stringify(value)?.length > MAX_VALUE_JSON ? "(too long to show)" : value; +} + +/** + * Compares each section in `patch` with what was stored before. Only fields that really differ are listed, and a + * section where nothing differed is left out entirely. + */ +export function describeSettingsChanges(before: object, patch: object): Record> { + const out: Record> = {}; + for (const [section, incoming] of Object.entries(patch)) { + if (incoming === null || typeof incoming !== "object") continue; + const previous = ((before as Record)[section] ?? {}) as Record; + const changes: Record = {}; + for (const [key, value] of Object.entries(incoming as Record)) { + if (same(previous[key], value)) continue; + changes[key] = NAMES_ONLY_SECTIONS.has(section) ? "(changed)" : { from: capture(previous[key]), to: capture(value) }; + } + if (Object.keys(changes).length > 0) out[section] = changes; + } + return out; +} diff --git a/web/src/pages/Privacy.tsx b/web/src/pages/Privacy.tsx index 0f63d31..c0dd9f1 100644 --- a/web/src/pages/Privacy.tsx +++ b/web/src/pages/Privacy.tsx @@ -163,7 +163,10 @@ export default function Privacy() { Audit log - Who changed what: your name (or email) as it was at the time, the action, what it was done to, and details of the change. + + Who changed what: your name (or email) as it was at the time, the action, what it was done to, and details of the change. + Also each time you sign in or out — with the IP address you came from — and when your account was first created. + {retention?.enabled ? `Entries older than ${retention.retentionDays} days are deleted (checked every ${retention.intervalHours} h).` @@ -288,7 +291,7 @@ export default function Privacy() {
  • Everyone signed in: the inventory and status pages, including server, IP, domain and secret-name details.
    • -
  • Operators and admins: also the audit log — who did what.
    • +
  • Operators and admins: also the audit log — who did what, and who signed in from where.
  • Admins only: the user list, everyone's active sign-ins (with IP and browser), the diagnostic log and settings.