From e2d94bf3a2f3fceab38cb9e6318e6b872fab773f Mon Sep 17 00:00:00 2001 From: Steve Cliff Date: Sun, 3 May 2026 23:09:09 +0100 Subject: [PATCH] server: populate audit UserID on credential mutations + slog prune push errors Switch handleSetHostCredentials, handleSetAdminCredentials, and handleDeleteAdminCredentials from authedUser (bool) to requireUser (*store.User) so AuditEntry.UserID and Actor are populated correctly. Add slog.Warn on the non-ErrNotFound pushAdminCredsToAgent path in handleRunRepoPrune so decrypt/send failures surface in the server log rather than appearing as a generic host_offline 503. --- internal/server/http/host_credentials.go | 12 +++- internal/server/http/host_credentials_test.go | 70 +++++++++++++++++-- internal/server/http/p2r01_test.go | 26 +++++++ internal/server/http/repo_ops.go | 2 + 4 files changed, 100 insertions(+), 10 deletions(-) diff --git a/internal/server/http/host_credentials.go b/internal/server/http/host_credentials.go index e87c99d..e4b9506 100644 --- a/internal/server/http/host_credentials.go +++ b/internal/server/http/host_credentials.go @@ -86,7 +86,8 @@ type hostRepoCredsRequest struct { // preserved. Re-encrypts under host_id and pushes a config.update // over the WS if the agent is connected. func (s *Server) handleSetHostCredentials(w stdhttp.ResponseWriter, r *stdhttp.Request) { - if !s.authedUser(r) { + user, ok := s.requireUser(r) + if !ok { writeJSONError(w, stdhttp.StatusUnauthorized, "unauthorized", "") return } @@ -147,6 +148,7 @@ func (s *Server) handleSetHostCredentials(w stdhttp.ResponseWriter, r *stdhttp.R _ = s.deps.Store.AppendAudit(r.Context(), store.AuditEntry{ ID: ulid.Make().String(), + UserID: &user.ID, Actor: "user", Action: "host.repo_credentials_set", TargetKind: ptr("host"), @@ -230,7 +232,8 @@ func (s *Server) handleGetAdminCredentials(w stdhttp.ResponseWriter, r *stdhttp. // persisting, pushes a config.update with Slot:"admin" over the WS if // the agent is connected. func (s *Server) handleSetAdminCredentials(w stdhttp.ResponseWriter, r *stdhttp.Request) { - if !s.authedUser(r) { + user, ok := s.requireUser(r) + if !ok { writeJSONError(w, stdhttp.StatusUnauthorized, "unauthorized", "") return } @@ -292,6 +295,7 @@ func (s *Server) handleSetAdminCredentials(w stdhttp.ResponseWriter, r *stdhttp. _ = s.deps.Store.AppendAudit(r.Context(), store.AuditEntry{ ID: ulid.Make().String(), + UserID: &user.ID, Actor: "user", Action: "host.admin_credentials_set", TargetKind: ptr("host"), @@ -313,7 +317,8 @@ func (s *Server) handleSetAdminCredentials(w stdhttp.ResponseWriter, r *stdhttp. // a deletion to the agent — the agent's local admin slot stays as-is // until the next deployment/reinstall. func (s *Server) handleDeleteAdminCredentials(w stdhttp.ResponseWriter, r *stdhttp.Request) { - if !s.authedUser(r) { + user, ok := s.requireUser(r) + if !ok { writeJSONError(w, stdhttp.StatusUnauthorized, "unauthorized", "") return } @@ -340,6 +345,7 @@ func (s *Server) handleDeleteAdminCredentials(w stdhttp.ResponseWriter, r *stdht _ = s.deps.Store.AppendAudit(r.Context(), store.AuditEntry{ ID: ulid.Make().String(), + UserID: &user.ID, Actor: "user", Action: "host.admin_credentials_deleted", TargetKind: ptr("host"), diff --git a/internal/server/http/host_credentials_test.go b/internal/server/http/host_credentials_test.go index bc7ebd0..c38e553 100644 --- a/internal/server/http/host_credentials_test.go +++ b/internal/server/http/host_credentials_test.go @@ -262,11 +262,12 @@ func TestAdminCredsPushOnSet(t *testing.T) { } // TestDeleteAdminCredentialsAuditLogged checks that DELETE appends an -// audit row with action='host.admin_credentials_deleted'. +// audit row with action='host.admin_credentials_deleted' and that the +// row carries the acting user's ID. func TestDeleteAdminCredentialsAuditLogged(t *testing.T) { t.Parallel() _, url, st := newTestServerWithHub(t) - cookie := loginAsAdmin(t, st) + cookie, userID := loginAsAdminWithID(t, st) hostID := makeHost(t, st, "audit-del-host") ctx := context.Background() @@ -287,9 +288,10 @@ func TestDeleteAdminCredentialsAuditLogged(t *testing.T) { t.Fatalf("delete: want 204, got %d", status) } - // Query audit_log for the host. + // Query audit_log for the delete row — action, user_id. rows, err := st.DB().QueryContext(ctx, - `SELECT action FROM audit_log WHERE target_id = ? AND target_kind = 'host'`, hostID) + `SELECT action, user_id FROM audit_log WHERE target_id = ? AND target_kind = 'host' AND action = 'host.admin_credentials_deleted'`, + hostID) if err != nil { t.Fatalf("query audit: %v", err) } @@ -298,11 +300,15 @@ func TestDeleteAdminCredentialsAuditLogged(t *testing.T) { found := false for rows.Next() { var action string - if err := rows.Scan(&action); err != nil { + var gotUserID *string + if err := rows.Scan(&action, &gotUserID); err != nil { t.Fatalf("scan: %v", err) } - if action == "host.admin_credentials_deleted" { - found = true + found = true + if gotUserID == nil { + t.Error("audit row: user_id is NULL, want non-nil") + } else if *gotUserID != userID { + t.Errorf("audit row: user_id=%q, want %q", *gotUserID, userID) } } if err := rows.Err(); err != nil { @@ -312,3 +318,53 @@ func TestDeleteAdminCredentialsAuditLogged(t *testing.T) { t.Error("audit row with action='host.admin_credentials_deleted' not found") } } + +// TestSetAdminCredentialsAuditCarriesUserID checks that PUT +// /api/hosts/{id}/admin-credentials appends an audit row with the +// correct action and a non-nil UserID matching the acting session. +func TestSetAdminCredentialsAuditCarriesUserID(t *testing.T) { + t.Parallel() + _, url, st := newTestServerWithHub(t) + cookie, userID := loginAsAdminWithID(t, st) + hostID := makeHost(t, st, "audit-set-admin-host") + + ctx := context.Background() + + status, body := doJSON(t, url, "PUT", "/api/hosts/"+hostID+"/admin-credentials", + map[string]any{ + "repo_url": "rest:http://admin.example/h", + "repo_password": "s3cr3t", + }, cookie) + if status != 204 { + t.Fatalf("set: want 204, got %d body=%+v", status, body) + } + + rows, err := st.DB().QueryContext(ctx, + `SELECT action, user_id FROM audit_log WHERE target_id = ? AND target_kind = 'host' AND action = 'host.admin_credentials_set'`, + hostID) + if err != nil { + t.Fatalf("query audit: %v", err) + } + defer rows.Close() + + found := false + for rows.Next() { + var action string + var gotUserID *string + if err := rows.Scan(&action, &gotUserID); err != nil { + t.Fatalf("scan: %v", err) + } + found = true + if gotUserID == nil { + t.Error("audit row: user_id is NULL, want non-nil") + } else if *gotUserID != userID { + t.Errorf("audit row: user_id=%q, want %q", *gotUserID, userID) + } + } + if err := rows.Err(); err != nil { + t.Fatalf("rows: %v", err) + } + if !found { + t.Error("audit row with action='host.admin_credentials_set' not found") + } +} diff --git a/internal/server/http/p2r01_test.go b/internal/server/http/p2r01_test.go index 6863e87..07451db 100644 --- a/internal/server/http/p2r01_test.go +++ b/internal/server/http/p2r01_test.go @@ -47,6 +47,32 @@ func loginAsAdmin(t *testing.T, st *store.Store) *stdhttp.Cookie { return &stdhttp.Cookie{Name: sessionCookieName, Value: tok} } +// loginAsAdminWithID is like loginAsAdmin but also returns the user ID. +// Use this when tests need to assert that the user ID was recorded +// (e.g. on audit entries). +func loginAsAdminWithID(t *testing.T, st *store.Store) (*stdhttp.Cookie, string) { + t.Helper() + ctx := context.Background() + uid := ulid.Make().String() + hash, _ := auth.HashPassword("very-long-test-password") + if err := st.CreateUser(ctx, store.User{ + ID: uid, Username: "tester-" + uid[:6], + PasswordHash: hash, Role: store.RoleAdmin, + CreatedAt: time.Now().UTC(), + }); err != nil { + t.Fatalf("create user: %v", err) + } + tok, _ := auth.NewToken() + if err := st.CreateSession(ctx, store.Session{ + UserID: uid, + CreatedAt: time.Now().UTC(), + ExpiresAt: time.Now().Add(time.Hour).UTC(), + }, auth.HashToken(tok)); err != nil { + t.Fatalf("create session: %v", err) + } + return &stdhttp.Cookie{Name: sessionCookieName, Value: tok}, uid +} + // makeHost inserts a minimal Host row directly via the store. Used by // HTTP-level tests that don't want to go through the full enrollment // path. Returns the host id. diff --git a/internal/server/http/repo_ops.go b/internal/server/http/repo_ops.go index 14c3812..920677d 100644 --- a/internal/server/http/repo_ops.go +++ b/internal/server/http/repo_ops.go @@ -6,6 +6,7 @@ package http import ( "errors" + "log/slog" stdhttp "net/http" "strconv" @@ -45,6 +46,7 @@ func (s *Server) handleRunRepoPrune(w stdhttp.ResponseWriter, r *stdhttp.Request // Hub.Send failure (offline) or decrypt failure — surface a // generic offline message so the operator retries when the // agent is back. + slog.Warn("prune: push admin creds failed", "host_id", hostID, "err", err) s.runOpError(w, r, stdhttp.StatusServiceUnavailable, "host_offline", "agent is not currently connected; try again when it reconnects") return