diff --git a/internal/api/admin_invites.go b/internal/api/admin_invites.go index 44cd28d0..555ab2dd 100644 --- a/internal/api/admin_invites.go +++ b/internal/api/admin_invites.go @@ -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) } diff --git a/internal/api/admin_users.go b/internal/api/admin_users.go index e27f0811..86bbb77b 100644 --- a/internal/api/admin_users.go +++ b/internal/api/admin_users.go @@ -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), diff --git a/internal/api/auth_forgot.go b/internal/api/auth_forgot.go index 0763837f..45aa26f0 100644 --- a/internal/api/auth_forgot.go +++ b/internal/api/auth_forgot.go @@ -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{}{}) } diff --git a/internal/api/auth_register.go b/internal/api/auth_register.go index ce0cac94..cc98ee2b 100644 --- a/internal/api/auth_register.go +++ b/internal/api/auth_register.go @@ -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{ diff --git a/internal/api/auth_reset.go b/internal/api/auth_reset.go index 23e85f86..48a7b070 100644 --- a/internal/api/auth_reset.go +++ b/internal/api/auth_reset.go @@ -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) } diff --git a/internal/api/me_password.go b/internal/api/me_password.go index 9031600f..c34993a4 100644 --- a/internal/api/me_password.go +++ b/internal/api/me_password.go @@ -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) } diff --git a/internal/api/me_token.go b/internal/api/me_token.go index 3a24533b..f596bf4f 100644 --- a/internal/api/me_token.go +++ b/internal/api/me_token.go @@ -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}) } diff --git a/internal/audit/audit.go b/internal/audit/audit.go index e4a43f77..e38b5ee4 100644 --- a/internal/audit/audit.go +++ b/internal/audit/audit.go @@ -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) + } + } +} diff --git a/internal/audit/audit_test.go b/internal/audit/audit_test.go index 09b493b2..8254f801 100644 --- a/internal/audit/audit_test.go +++ b/internal/audit/audit_test.go @@ -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