refactor(server): audit.WriteOrLog wrapper; migrate 13 sites (T5)
Add audit.WriteOrLog: a one-line wrapper around Write that logs at
Warn and swallows the error, matching the package contract that
audit failures must not break user-facing operations.
Migrate the 13 call sites across 7 files in internal/api/ from the
3-line "if err != nil { logger.Warn(...) }" shape to a single call.
audit.Write stays exported for tests + any future caller that
needs strict semantics.
Adds three tests: success (no log), failure-via-closed-pool (Warn
record with action+err keys), and nil-logger (no panic). Tests
skip when MINSTREL_TEST_DATABASE_URL is unset, matching the
existing harness convention.
This commit is contained in:
@@ -93,10 +93,8 @@ func (h *handlers) handleCreateInvite(w http.ResponseWriter, r *http.Request) {
|
||||
return
|
||||
}
|
||||
|
||||
if err := audit.Write(r.Context(), h.pool, user.ID, pgtype.UUID{}, audit.ActionInviteCreate,
|
||||
map[string]any{"token": token}); err != nil {
|
||||
h.logger.Warn("admin: invite-create audit failed", "err", err)
|
||||
}
|
||||
audit.WriteOrLog(r.Context(), h.pool, h.logger, user.ID, pgtype.UUID{}, audit.ActionInviteCreate,
|
||||
map[string]any{"token": token})
|
||||
|
||||
writeJSON(w, http.StatusOK, inviteFromRow(row))
|
||||
}
|
||||
@@ -127,10 +125,8 @@ func (h *handlers) handleDeleteInvite(w http.ResponseWriter, r *http.Request) {
|
||||
writeErrWithLog(w, h.logger, "admin: delete invite failed", apierror.Internal(err))
|
||||
return
|
||||
}
|
||||
if err := audit.Write(r.Context(), h.pool, user.ID, pgtype.UUID{}, audit.ActionInviteRevoke,
|
||||
map[string]any{"token": token}); err != nil {
|
||||
h.logger.Warn("admin: invite-revoke audit failed", "err", err)
|
||||
}
|
||||
audit.WriteOrLog(r.Context(), h.pool, h.logger, user.ID, pgtype.UUID{}, audit.ActionInviteRevoke,
|
||||
map[string]any{"token": token})
|
||||
w.WriteHeader(http.StatusNoContent)
|
||||
}
|
||||
|
||||
|
||||
@@ -117,9 +117,7 @@ func (h *handlers) handleUpdateUserAdmin(w http.ResponseWriter, r *http.Request)
|
||||
if !req.IsAdmin {
|
||||
action = audit.ActionDemoteAdmin
|
||||
}
|
||||
if err := audit.Write(r.Context(), h.pool, caller.ID, targetID, action, nil); err != nil {
|
||||
h.logger.Warn("admin: promote/demote audit failed", "err", err)
|
||||
}
|
||||
audit.WriteOrLog(r.Context(), h.pool, h.logger, caller.ID, targetID, action, nil)
|
||||
|
||||
writeJSON(w, http.StatusOK, adminUserView{
|
||||
ID: uuidToString(updated.ID),
|
||||
@@ -189,10 +187,8 @@ func (h *handlers) handleAdminCreateUser(w http.ResponseWriter, r *http.Request)
|
||||
return
|
||||
}
|
||||
|
||||
if err := audit.Write(r.Context(), h.pool, caller.ID, user.ID, audit.ActionCreateUserAdmin,
|
||||
map[string]any{"is_admin": req.IsAdmin}); err != nil {
|
||||
h.logger.Warn("admin create user: audit failed", "err", err)
|
||||
}
|
||||
audit.WriteOrLog(r.Context(), h.pool, h.logger, caller.ID, user.ID, audit.ActionCreateUserAdmin,
|
||||
map[string]any{"is_admin": req.IsAdmin})
|
||||
|
||||
writeJSON(w, http.StatusOK, adminUserView{
|
||||
ID: uuidToString(user.ID),
|
||||
@@ -239,10 +235,8 @@ func (h *handlers) handleAdminDeleteUser(w http.ResponseWriter, r *http.Request)
|
||||
return
|
||||
}
|
||||
|
||||
if err := audit.Write(r.Context(), h.pool, caller.ID, target.ID, audit.ActionDeleteUser,
|
||||
map[string]any{"username": target.Username, "was_admin": target.IsAdmin}); err != nil {
|
||||
h.logger.Warn("admin delete user: audit failed", "err", err)
|
||||
}
|
||||
audit.WriteOrLog(r.Context(), h.pool, h.logger, caller.ID, target.ID, audit.ActionDeleteUser,
|
||||
map[string]any{"username": target.Username, "was_admin": target.IsAdmin})
|
||||
|
||||
w.WriteHeader(http.StatusNoContent)
|
||||
}
|
||||
@@ -298,9 +292,7 @@ func (h *handlers) handleAdminResetPassword(w http.ResponseWriter, r *http.Reque
|
||||
return
|
||||
}
|
||||
|
||||
if err := audit.Write(r.Context(), h.pool, caller.ID, targetID, audit.ActionPasswordResetAdmin, nil); err != nil {
|
||||
h.logger.Warn("admin reset password: audit failed", "err", err)
|
||||
}
|
||||
audit.WriteOrLog(r.Context(), h.pool, h.logger, caller.ID, targetID, audit.ActionPasswordResetAdmin, nil)
|
||||
|
||||
w.WriteHeader(http.StatusNoContent)
|
||||
}
|
||||
@@ -342,10 +334,8 @@ func (h *handlers) handleAdminAutoApproveToggle(w http.ResponseWriter, r *http.R
|
||||
return
|
||||
}
|
||||
|
||||
if err := audit.Write(r.Context(), h.pool, caller.ID, targetID, audit.ActionAutoApproveToggle,
|
||||
map[string]any{"auto_approve": req.AutoApprove}); err != nil {
|
||||
h.logger.Warn("admin auto-approve: audit failed", "err", err)
|
||||
}
|
||||
audit.WriteOrLog(r.Context(), h.pool, h.logger, caller.ID, targetID, audit.ActionAutoApproveToggle,
|
||||
map[string]any{"auto_approve": req.AutoApprove})
|
||||
|
||||
writeJSON(w, http.StatusOK, adminUserView{
|
||||
ID: uuidToString(updated.ID),
|
||||
|
||||
@@ -66,10 +66,8 @@ func (h *handlers) handleForgotPassword(w http.ResponseWriter, r *http.Request)
|
||||
}
|
||||
|
||||
// Audit (best-effort).
|
||||
if err := audit.Write(r.Context(), h.pool, pgtype.UUID{}, auditTarget, audit.ActionForgotPasswordInit,
|
||||
map[string]any{"email_match": matched}); err != nil {
|
||||
h.logger.Warn("forgot-password: audit failed", "err", err)
|
||||
}
|
||||
audit.WriteOrLog(r.Context(), h.pool, h.logger, pgtype.UUID{}, auditTarget, audit.ActionForgotPasswordInit,
|
||||
map[string]any{"email_match": matched})
|
||||
|
||||
writeJSON(w, http.StatusOK, struct{}{})
|
||||
}
|
||||
|
||||
@@ -196,14 +196,10 @@ func (h *handlers) handleRegister(w http.ResponseWriter, r *http.Request) {
|
||||
if user.IsAdmin {
|
||||
auditMeta["first_admin"] = true
|
||||
}
|
||||
if err := audit.Write(r.Context(), h.pool, user.ID, user.ID, audit.ActionRegister, auditMeta); err != nil {
|
||||
h.logger.Warn("register: audit write failed", "err", err)
|
||||
}
|
||||
audit.WriteOrLog(r.Context(), h.pool, h.logger, user.ID, user.ID, audit.ActionRegister, auditMeta)
|
||||
if usedInviteToken != "" {
|
||||
if err := audit.Write(r.Context(), h.pool, user.ID, user.ID, audit.ActionInviteRedeem,
|
||||
map[string]any{"token": usedInviteToken}); err != nil {
|
||||
h.logger.Warn("register: audit invite-redeem write failed", "err", err)
|
||||
}
|
||||
audit.WriteOrLog(r.Context(), h.pool, h.logger, user.ID, user.ID, audit.ActionInviteRedeem,
|
||||
map[string]any{"token": usedInviteToken})
|
||||
}
|
||||
|
||||
writeJSON(w, http.StatusOK, LoginResponse{
|
||||
|
||||
@@ -92,9 +92,7 @@ func (h *handlers) handleResetPassword(w http.ResponseWriter, r *http.Request) {
|
||||
return
|
||||
}
|
||||
|
||||
if err := audit.Write(r.Context(), h.pool, reset.UserID, reset.UserID, audit.ActionPasswordResetByEmail, nil); err != nil {
|
||||
h.logger.Warn("reset password: audit failed", "err", err)
|
||||
}
|
||||
audit.WriteOrLog(r.Context(), h.pool, h.logger, reset.UserID, reset.UserID, audit.ActionPasswordResetByEmail, nil)
|
||||
|
||||
w.WriteHeader(http.StatusNoContent)
|
||||
}
|
||||
|
||||
@@ -63,9 +63,7 @@ func (h *handlers) handleChangePassword(w http.ResponseWriter, r *http.Request)
|
||||
return
|
||||
}
|
||||
|
||||
if err := audit.Write(r.Context(), h.pool, user.ID, user.ID, audit.ActionPasswordChangeSelf, nil); err != nil {
|
||||
h.logger.Warn("change password: audit failed", "err", err)
|
||||
}
|
||||
audit.WriteOrLog(r.Context(), h.pool, h.logger, user.ID, user.ID, audit.ActionPasswordChangeSelf, nil)
|
||||
|
||||
w.WriteHeader(http.StatusNoContent)
|
||||
}
|
||||
|
||||
@@ -54,9 +54,7 @@ func (h *handlers) handleRegenerateMyAPIToken(w http.ResponseWriter, r *http.Req
|
||||
return
|
||||
}
|
||||
|
||||
if err := audit.Write(r.Context(), h.pool, user.ID, user.ID, audit.ActionTokenRegenerate, nil); err != nil {
|
||||
h.logger.Warn("regenerate api token: audit failed", "err", err)
|
||||
}
|
||||
audit.WriteOrLog(r.Context(), h.pool, h.logger, user.ID, user.ID, audit.ActionTokenRegenerate, nil)
|
||||
|
||||
writeJSON(w, http.StatusOK, apiTokenResp{APIToken: updated.ApiToken})
|
||||
}
|
||||
|
||||
@@ -12,6 +12,7 @@ package audit
|
||||
import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"log/slog"
|
||||
|
||||
"github.com/jackc/pgx/v5/pgtype"
|
||||
"github.com/jackc/pgx/v5/pgxpool"
|
||||
@@ -75,3 +76,15 @@ func Write(ctx context.Context, pool *pgxpool.Pool, actorID, targetID pgtype.UUI
|
||||
Metadata: jsonMeta,
|
||||
})
|
||||
}
|
||||
|
||||
// WriteOrLog writes the audit row; on error, logs at Warn and swallows
|
||||
// (audit failures must not break user-facing operations — see package doc).
|
||||
// Use this when the audit is observability, not gating; use Write directly
|
||||
// when the caller needs strict semantics (e.g. tests).
|
||||
func WriteOrLog(ctx context.Context, pool *pgxpool.Pool, logger *slog.Logger, actorID, targetID pgtype.UUID, action Action, metadata map[string]any) {
|
||||
if err := Write(ctx, pool, actorID, targetID, action, metadata); err != nil {
|
||||
if logger != nil {
|
||||
logger.Warn("audit failed", "action", string(action), "err", err)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,8 +1,11 @@
|
||||
package audit_test
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"context"
|
||||
"log/slog"
|
||||
"os"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"github.com/jackc/pgx/v5/pgtype"
|
||||
@@ -78,6 +81,73 @@ func contains(s, sub string) bool {
|
||||
})()
|
||||
}
|
||||
|
||||
// TestWriteOrLog_Success exercises the happy path — when Write
|
||||
// succeeds, no log line is emitted. Skipped unless a test DB is
|
||||
// configured (no fixtures harness exists in this package beyond the
|
||||
// env-driven pool above).
|
||||
func TestWriteOrLog_Success(t *testing.T) {
|
||||
pool := newTestPool(t)
|
||||
var buf bytes.Buffer
|
||||
logger := slog.New(slog.NewTextHandler(&buf, &slog.HandlerOptions{Level: slog.LevelWarn}))
|
||||
var nilUUID pgtype.UUID
|
||||
audit.WriteOrLog(context.Background(), pool, logger, nilUUID, nilUUID, audit.ActionRegister, nil)
|
||||
if buf.Len() != 0 {
|
||||
t.Errorf("expected no log output on success, got %q", buf.String())
|
||||
}
|
||||
}
|
||||
|
||||
// TestWriteOrLog_DBFailure_Logs forces Write to fail by passing a
|
||||
// closed pool, and asserts the logger captured a Warn record with
|
||||
// action + err keys.
|
||||
func TestWriteOrLog_DBFailure_Logs(t *testing.T) {
|
||||
url := os.Getenv("MINSTREL_TEST_DATABASE_URL")
|
||||
if url == "" {
|
||||
t.Skip("MINSTREL_TEST_DATABASE_URL not set")
|
||||
}
|
||||
pool, err := pgxpool.New(context.Background(), url)
|
||||
if err != nil {
|
||||
t.Fatalf("connect: %v", err)
|
||||
}
|
||||
pool.Close() // force subsequent ops to fail
|
||||
|
||||
var buf bytes.Buffer
|
||||
logger := slog.New(slog.NewTextHandler(&buf, &slog.HandlerOptions{Level: slog.LevelWarn}))
|
||||
var nilUUID pgtype.UUID
|
||||
audit.WriteOrLog(context.Background(), pool, logger, nilUUID, nilUUID, audit.ActionRegister, nil)
|
||||
|
||||
out := buf.String()
|
||||
if out == "" {
|
||||
t.Fatal("expected a Warn log line on failure, got none")
|
||||
}
|
||||
if !strings.Contains(out, "level=WARN") {
|
||||
t.Errorf("expected level=WARN in log line, got %q", out)
|
||||
}
|
||||
if !strings.Contains(out, "action=register") {
|
||||
t.Errorf("expected action=register key in log line, got %q", out)
|
||||
}
|
||||
if !strings.Contains(out, "err=") {
|
||||
t.Errorf("expected err= key in log line, got %q", out)
|
||||
}
|
||||
}
|
||||
|
||||
// TestWriteOrLog_NilLogger ensures a nil logger + failing pool does
|
||||
// not panic. Uses a closed pool to force the failure branch.
|
||||
func TestWriteOrLog_NilLogger(t *testing.T) {
|
||||
url := os.Getenv("MINSTREL_TEST_DATABASE_URL")
|
||||
if url == "" {
|
||||
t.Skip("MINSTREL_TEST_DATABASE_URL not set")
|
||||
}
|
||||
pool, err := pgxpool.New(context.Background(), url)
|
||||
if err != nil {
|
||||
t.Fatalf("connect: %v", err)
|
||||
}
|
||||
pool.Close()
|
||||
|
||||
var nilUUID pgtype.UUID
|
||||
// Must not panic.
|
||||
audit.WriteOrLog(context.Background(), pool, nil, nilUUID, nilUUID, audit.ActionRegister, nil)
|
||||
}
|
||||
|
||||
func TestWrite_AllActionConstantsArePersisted(t *testing.T) {
|
||||
pool := newTestPool(t)
|
||||
var nilUUID pgtype.UUID
|
||||
|
||||
Reference in New Issue
Block a user