From fd33a16bdce34e2c4cbeb772b70db56503947d54 Mon Sep 17 00:00:00 2001 From: Alex Luong Date: Mon, 20 Apr 2026 21:55:39 +0700 Subject: [PATCH] feat: add audit logging to tenant and destination CRUD operations Closes #235 Co-Authored-By: Claude Opus 4.6 (1M context) --- internal/apirouter/audit_log_test.go | 180 +++++++++++++++++++++ internal/apirouter/destination_handlers.go | 24 +++ internal/apirouter/router_test.go | 12 +- internal/apirouter/tenant_handlers.go | 10 ++ 4 files changed, 225 insertions(+), 1 deletion(-) create mode 100644 internal/apirouter/audit_log_test.go diff --git a/internal/apirouter/audit_log_test.go b/internal/apirouter/audit_log_test.go new file mode 100644 index 00000000..62a37e7d --- /dev/null +++ b/internal/apirouter/audit_log_test.go @@ -0,0 +1,180 @@ +package apirouter_test + +import ( + "net/http" + "net/http/httptest" + "testing" + "time" + + "github.com/hookdeck/outpost/internal/logging" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/uptrace/opentelemetry-go-extra/otelzap" + "go.uber.org/zap" + "go.uber.org/zap/zaptest/observer" +) + +func newAuditTest(t *testing.T, opts ...apiTestOption) (*apiTest, *observer.ObservedLogs) { + t.Helper() + core, logs := observer.New(zap.InfoLevel) + // When auditLogger is nil, Audit() falls through to the main logger. + logger := &logging.Logger{Logger: otelzap.New(zap.New(core))} + opts = append(opts, withLogger(logger)) + return newAPITest(t, opts...), logs +} + +func findAuditLog(logs *observer.ObservedLogs, msg string) *observer.LoggedEntry { + for _, entry := range logs.All() { + if entry.Message == msg { + return &entry + } + } + return nil +} + +func assertAuditField(t *testing.T, entry *observer.LoggedEntry, key, expected string) { + t.Helper() + for _, f := range entry.Context { + if f.Key == key { + assert.Equal(t, expected, f.String, "audit field %s", key) + return + } + } + t.Errorf("audit field %q not found in log entry", key) +} + +func TestAuditLog_Tenant(t *testing.T) { + t.Run("tenant created", func(t *testing.T) { + h, logs := newAuditTest(t) + + req := httptest.NewRequest(http.MethodPut, "/api/v1/tenants/t1", nil) + resp := h.do(h.withAPIKey(req)) + + require.Equal(t, http.StatusCreated, resp.Code) + entry := findAuditLog(logs, "tenant created") + require.NotNil(t, entry, "expected 'tenant created' audit log") + assertAuditField(t, entry, "tenant_id", "t1") + }) + + t.Run("tenant updated", func(t *testing.T) { + h, logs := newAuditTest(t) + h.tenantStore.UpsertTenant(t.Context(), tf.Any(tf.WithID("t1"))) + + req := h.jsonReq(http.MethodPut, "/api/v1/tenants/t1", map[string]any{ + "metadata": map[string]string{"env": "prod"}, + }) + resp := h.do(h.withAPIKey(req)) + + require.Equal(t, http.StatusOK, resp.Code) + entry := findAuditLog(logs, "tenant updated") + require.NotNil(t, entry, "expected 'tenant updated' audit log") + assertAuditField(t, entry, "tenant_id", "t1") + }) + + t.Run("tenant deleted", func(t *testing.T) { + h, logs := newAuditTest(t) + h.tenantStore.UpsertTenant(t.Context(), tf.Any(tf.WithID("t1"))) + + req := httptest.NewRequest(http.MethodDelete, "/api/v1/tenants/t1", nil) + resp := h.do(h.withAPIKey(req)) + + require.Equal(t, http.StatusOK, resp.Code) + entry := findAuditLog(logs, "tenant deleted") + require.NotNil(t, entry, "expected 'tenant deleted' audit log") + assertAuditField(t, entry, "tenant_id", "t1") + }) +} + +func TestAuditLog_Destination(t *testing.T) { + t.Run("destination created", func(t *testing.T) { + h, logs := newAuditTest(t) + h.tenantStore.UpsertTenant(t.Context(), tf.Any(tf.WithID("t1"))) + + req := h.jsonReq(http.MethodPost, "/api/v1/tenants/t1/destinations", validDestination()) + resp := h.do(h.withAPIKey(req)) + + require.Equal(t, http.StatusCreated, resp.Code) + entry := findAuditLog(logs, "destination created") + require.NotNil(t, entry, "expected 'destination created' audit log") + assertAuditField(t, entry, "tenant_id", "t1") + assertAuditField(t, entry, "destination_type", "webhook") + }) + + t.Run("destination updated", func(t *testing.T) { + h, logs := newAuditTest(t) + h.tenantStore.UpsertTenant(t.Context(), tf.Any(tf.WithID("t1"))) + h.tenantStore.CreateDestination(t.Context(), df.Any( + df.WithTenantID("t1"), df.WithID("d1"), df.WithTopics([]string{"user.created"}), + )) + + req := h.jsonReq(http.MethodPatch, "/api/v1/tenants/t1/destinations/d1", map[string]any{ + "topics": []string{"user.deleted"}, + }) + resp := h.do(h.withAPIKey(req)) + + require.Equal(t, http.StatusOK, resp.Code) + entry := findAuditLog(logs, "destination updated") + require.NotNil(t, entry, "expected 'destination updated' audit log") + assertAuditField(t, entry, "tenant_id", "t1") + assertAuditField(t, entry, "destination_id", "d1") + }) + + t.Run("destination deleted", func(t *testing.T) { + h, logs := newAuditTest(t) + h.tenantStore.UpsertTenant(t.Context(), tf.Any(tf.WithID("t1"))) + h.tenantStore.CreateDestination(t.Context(), df.Any(df.WithTenantID("t1"), df.WithID("d1"))) + + req := httptest.NewRequest(http.MethodDelete, "/api/v1/tenants/t1/destinations/d1", nil) + resp := h.do(h.withAPIKey(req)) + + require.Equal(t, http.StatusOK, resp.Code) + entry := findAuditLog(logs, "destination deleted") + require.NotNil(t, entry, "expected 'destination deleted' audit log") + assertAuditField(t, entry, "tenant_id", "t1") + assertAuditField(t, entry, "destination_id", "d1") + }) + + t.Run("destination disabled", func(t *testing.T) { + h, logs := newAuditTest(t) + h.tenantStore.UpsertTenant(t.Context(), tf.Any(tf.WithID("t1"))) + h.tenantStore.CreateDestination(t.Context(), df.Any(df.WithTenantID("t1"), df.WithID("d1"))) + + req := httptest.NewRequest(http.MethodPut, "/api/v1/tenants/t1/destinations/d1/disable", nil) + resp := h.do(h.withAPIKey(req)) + + require.Equal(t, http.StatusOK, resp.Code) + entry := findAuditLog(logs, "destination disabled") + require.NotNil(t, entry, "expected 'destination disabled' audit log") + assertAuditField(t, entry, "tenant_id", "t1") + assertAuditField(t, entry, "destination_id", "d1") + }) + + t.Run("destination enabled", func(t *testing.T) { + h, logs := newAuditTest(t) + h.tenantStore.UpsertTenant(t.Context(), tf.Any(tf.WithID("t1"))) + h.tenantStore.CreateDestination(t.Context(), df.Any(df.WithTenantID("t1"), df.WithID("d1"), df.WithDisabledAt(time.Now()))) + + req := httptest.NewRequest(http.MethodPut, "/api/v1/tenants/t1/destinations/d1/enable", nil) + resp := h.do(h.withAPIKey(req)) + + require.Equal(t, http.StatusOK, resp.Code) + entry := findAuditLog(logs, "destination enabled") + require.NotNil(t, entry, "expected 'destination enabled' audit log") + assertAuditField(t, entry, "tenant_id", "t1") + assertAuditField(t, entry, "destination_id", "d1") + }) + + t.Run("no audit log when disable is no-op", func(t *testing.T) { + h, logs := newAuditTest(t) + h.tenantStore.UpsertTenant(t.Context(), tf.Any(tf.WithID("t1"))) + h.tenantStore.CreateDestination(t.Context(), df.Any(df.WithTenantID("t1"), df.WithID("d1"), df.WithDisabledAt(time.Now()))) + + // Disabling an already-disabled destination should not emit audit log + req := httptest.NewRequest(http.MethodPut, "/api/v1/tenants/t1/destinations/d1/disable", nil) + resp := h.do(h.withAPIKey(req)) + + require.Equal(t, http.StatusOK, resp.Code) + entry := findAuditLog(logs, "destination disabled") + assert.Nil(t, entry, "should not emit audit log for no-op disable") + }) +} diff --git a/internal/apirouter/destination_handlers.go b/internal/apirouter/destination_handlers.go index fc045c63..5725efc7 100644 --- a/internal/apirouter/destination_handlers.go +++ b/internal/apirouter/destination_handlers.go @@ -109,6 +109,11 @@ func (h *DestinationHandlers) Create(c *gin.Context) { } h.telemetry.DestinationCreated(c.Request.Context(), destination.Type) h.emitSubscriptionUpdateIfChanged(c.Request.Context(), tenant.ID, prev) + h.logger.Ctx(c.Request.Context()).Audit("destination created", + zap.String("tenant_id", tenant.ID), + zap.String("destination_id", destination.ID), + zap.String("destination_type", destination.Type), + ) display, err := h.displayer.Display(&destination) if err != nil { @@ -204,6 +209,11 @@ func (h *DestinationHandlers) Update(c *gin.Context) { return } h.emitSubscriptionUpdateIfChanged(c.Request.Context(), tenant.ID, prev) + h.logger.Ctx(c.Request.Context()).Audit("destination updated", + zap.String("tenant_id", tenant.ID), + zap.String("destination_id", updatedDestination.ID), + zap.String("destination_type", updatedDestination.Type), + ) display, err := h.displayer.Display(&updatedDestination) if err != nil { @@ -225,6 +235,11 @@ func (h *DestinationHandlers) Delete(c *gin.Context) { return } h.emitSubscriptionUpdateIfChanged(c.Request.Context(), tenant.ID, prev) + h.logger.Ctx(c.Request.Context()).Audit("destination deleted", + zap.String("tenant_id", tenant.ID), + zap.String("destination_id", destination.ID), + zap.String("destination_type", destination.Type), + ) c.JSON(http.StatusOK, gin.H{"success": true}) } @@ -275,6 +290,15 @@ func (h *DestinationHandlers) setDisabilityHandler(c *gin.Context, disabled bool return } h.emitSubscriptionUpdateIfChanged(c.Request.Context(), tenant.ID, prev) + action := "destination enabled" + if disabled { + action = "destination disabled" + } + h.logger.Ctx(c.Request.Context()).Audit(action, + zap.String("tenant_id", tenant.ID), + zap.String("destination_id", destination.ID), + zap.String("destination_type", destination.Type), + ) } display, err := h.displayer.Display(destination) diff --git a/internal/apirouter/router_test.go b/internal/apirouter/router_test.go index 22cf808d..16a3e0f1 100644 --- a/internal/apirouter/router_test.go +++ b/internal/apirouter/router_test.go @@ -61,6 +61,7 @@ type apiTestConfig struct { tenantStore tenantstore.TenantStore destRegistry destregistry.Registry subscriptionEmitter apirouter.SubscriptionEmitter + logger *logging.Logger } func withTenantStore(ts tenantstore.TenantStore) apiTestOption { @@ -81,6 +82,12 @@ func withSubscriptionEmitter(e apirouter.SubscriptionEmitter) apiTestOption { } } +func withLogger(l *logging.Logger) apiTestOption { + return func(cfg *apiTestConfig) { + cfg.logger = l + } +} + func newAPITest(t *testing.T, opts ...apiTestOption) *apiTest { t.Helper() @@ -91,7 +98,10 @@ func newAPITest(t *testing.T, opts ...apiTestOption) *apiTest { o(&cfg) } - logger := &logging.Logger{Logger: otelzap.New(zap.NewNop())} + logger := cfg.logger + if logger == nil { + logger = &logging.Logger{Logger: otelzap.New(zap.NewNop())} + } ts := cfg.tenantStore ls := logstore.NewMemLogStore() dp := &mockDeliveryPublisher{} diff --git a/internal/apirouter/tenant_handlers.go b/internal/apirouter/tenant_handlers.go index cd2f8b7e..8646dfd7 100644 --- a/internal/apirouter/tenant_handlers.go +++ b/internal/apirouter/tenant_handlers.go @@ -11,6 +11,7 @@ import ( "github.com/hookdeck/outpost/internal/models" "github.com/hookdeck/outpost/internal/telemetry" "github.com/hookdeck/outpost/internal/tenantstore" + "go.uber.org/zap" ) type TenantHandlers struct { @@ -67,6 +68,9 @@ func (h *TenantHandlers) Upsert(c *gin.Context) { AbortWithError(c, http.StatusInternalServerError, NewErrInternalServer(err)) return } + h.logger.Ctx(c.Request.Context()).Audit("tenant updated", + zap.String("tenant_id", tenantID), + ) c.JSON(http.StatusOK, existingTenant) return } @@ -85,6 +89,9 @@ func (h *TenantHandlers) Upsert(c *gin.Context) { return } h.telemetry.TenantCreated(c.Request.Context()) + h.logger.Ctx(c.Request.Context()).Audit("tenant created", + zap.String("tenant_id", tenantID), + ) c.JSON(http.StatusCreated, tenant) } @@ -185,6 +192,9 @@ func (h *TenantHandlers) Delete(c *gin.Context) { AbortWithError(c, http.StatusInternalServerError, NewErrInternalServer(err)) return } + h.logger.Ctx(c.Request.Context()).Audit("tenant deleted", + zap.String("tenant_id", tenant.ID), + ) c.JSON(http.StatusOK, gin.H{"success": true}) }