refactor(audit): add NoopAuditService to eliminate nil checks (#137)

* refactor(audit): add NoopAuditService to eliminate nil checks

- Add AuditLogger interface and AuditLogEntry type in core package
- Add NoopAuditService with safe no-op methods for disabled audit
- Remove ~37 nil checks across 12 service, handler, and middleware files
- Remove enabled field and internal guards from AuditService
- Simplify NewAuditService constructor to 2 params (drop enabled)
- Bootstrap conditionally creates real vs noop audit service
- Update all test files to use NewNoopAuditService instead of nil

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* refactor(audit): apply NoopAuditService to new code and improve shutdown guard

- Remove remaining auditService nil checks in admin user operations
- Update DashboardService to accept core.AuditLogger interface
- Skip audit shutdown job registration when audit logging is disabled
- Document concurrency safety contract on AuditLogger interface

* fix(audit): guard against nil AuditLogger in service constructors

- Default auditService to NoopAuditService when nil in NewUserService,
  NewTokenService, NewClientService, and NewAuthorizationService
- Return error from NewRateLimiter when AuditService is nil in config
- Use safe type assertions for user_id/username in rate limit handler

* refactor(audit): use singleton NoopAuditService and add missing nil guards

- Return shared singleton from NewNoopAuditService to avoid repeated
  allocation of stateless struct across 120+ test call sites
- Add nil guard to NewDeviceService and NewDashboardService to match
  the pattern applied to the other six service constructors

* fix(audit): return meaningful pagination from NoopAuditService and clarify required field

- Use CalculatePagination(0, params) in GetAuditLogs so callers receive
  correct CurrentPage/PageSize metadata even when auditing is disabled
- Update noop test to assert against CalculatePagination result
- Mark RateLimitConfig.AuditService comment as Required to prevent nil

* fix(test): pass NoopAuditService in bootstrap rate limit test

- TestSetupRateLimitingMemory passed nil audit service which now errors;
  use NewNoopAuditService() consistent with all other test call sites
- Trim GetAuditLogs comment for consistency with other noop methods

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Author: appleboy

internal/bootstrap/bootstrap.go

@@ -38,7 +38,7 @@ type Application struct {
 	RateLimitRedisClient   *redis.Client
 
 	// Services
-	AuditService *services.AuditService
+	AuditService core.AuditLogger
 	services     serviceSet
 
 	// HTTP
@@ -134,11 +134,14 @@ func (app *Application) initializeInfrastructure(ctx context.Context) error {
 // initializeBusinessLayer sets up services
 func (app *Application) initializeBusinessLayer() {
 	// Audit service (required by other services)
-	app.AuditService = services.NewAuditService(
-		app.DB,
-		app.Config.EnableAuditLogging,
-		app.Config.AuditLogBufferSize,
-	)
+	if app.Config.EnableAuditLogging {
+		app.AuditService = services.NewAuditService(
+			app.DB,
+			app.Config.AuditLogBufferSize,
+		)
+	} else {
+		app.AuditService = services.NewNoopAuditService()
+	}
 
 	// Initialize token provider (stored for JWKS handler)
 	app.TokenProvider = initializeTokenProvider(app.Config)

internal/bootstrap/bootstrap_test.go

@@ -8,6 +8,7 @@ import (
 
 	"github.com/gin-gonic/gin"
 	"github.com/go-authgate/authgate/internal/config"
+	"github.com/go-authgate/authgate/internal/services"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
@@ -139,7 +140,7 @@ func TestSetupRateLimitingMemory(t *testing.T) {
 		TokenRateLimit:        20,
 		DeviceVerifyRateLimit: 10,
 	}
-	limiters := setupRateLimiting(cfg, nil, nil)
+	limiters := setupRateLimiting(cfg, services.NewNoopAuditService(), nil)
 	require.NotNil(t, limiters.login)
 	require.NotNil(t, limiters.deviceCode)
 	require.NotNil(t, limiters.token)

internal/bootstrap/handlers.go

@@ -37,7 +37,7 @@ type handlerSet struct {
 type handlerDeps struct {
 	cfg            *config.Config
 	services       serviceSet
-	auditService   *services.AuditService
+	auditService   core.AuditLogger
 	oauthProviders map[string]*auth.OAuthProvider
 	oauthClient    *http.Client
 	metrics        core.Recorder

internal/bootstrap/ratelimit.go

@@ -5,8 +5,8 @@ import (
 
 	"github.com/gin-gonic/gin"
 	"github.com/go-authgate/authgate/internal/config"
+	"github.com/go-authgate/authgate/internal/core"
 	"github.com/go-authgate/authgate/internal/middleware"
-	"github.com/go-authgate/authgate/internal/services"
 	"github.com/redis/go-redis/v9"
 )
 
@@ -24,7 +24,7 @@ type rateLimitMiddlewares struct {
 // Accepts an optional go-redis client
 func setupRateLimiting(
 	cfg *config.Config,
-	auditService *services.AuditService,
+	auditService core.AuditLogger,
 	redisClient *redis.Client,
 ) rateLimitMiddlewares {
 	// Return no-op middlewares when rate limiting is disabled
@@ -50,7 +50,7 @@ func setupRateLimiting(
 // Accepts an optional go-redis client
 func createRateLimiters(
 	cfg *config.Config,
-	auditService *services.AuditService,
+	auditService core.AuditLogger,
 	redisClient *redis.Client,
 ) rateLimitMiddlewares {
 	log.Printf("Rate limiting enabled (store: %s)", cfg.RateLimitStore)

internal/bootstrap/router.go

@@ -15,7 +15,6 @@ import (
 	"github.com/go-authgate/authgate/internal/handlers"
 	"github.com/go-authgate/authgate/internal/metrics"
 	"github.com/go-authgate/authgate/internal/middleware"
-	"github.com/go-authgate/authgate/internal/services"
 	"github.com/go-authgate/authgate/internal/store"
 
 	"github.com/gin-contrib/sessions"
@@ -33,7 +32,7 @@ func setupRouter(
 	db *store.Store,
 	h handlerSet,
 	prometheusMetrics core.Recorder,
-	auditService *services.AuditService,
+	auditService core.AuditLogger,
 	rateLimitRedisClient *redis.Client,
 	templatesFS embed.FS,
 	oauthProviders map[string]*auth.OAuthProvider,

internal/bootstrap/server.go

@@ -10,7 +10,6 @@ import (
 	"github.com/go-authgate/authgate/internal/core"
 	"github.com/go-authgate/authgate/internal/metrics"
 	"github.com/go-authgate/authgate/internal/models"
-	"github.com/go-authgate/authgate/internal/services"
 	"github.com/go-authgate/authgate/internal/store"
 
 	"github.com/appleboy/graceful"
@@ -98,9 +97,12 @@ func addRedisClientShutdownJob(m *graceful.Manager, redisClient *redis.Client, c
 // addAuditServiceShutdownJob adds audit service shutdown handler
 func addAuditServiceShutdownJob(
 	m *graceful.Manager,
-	auditService *services.AuditService,
+	auditService core.AuditLogger,
 	cfg *config.Config,
 ) {
+	if !cfg.EnableAuditLogging {
+		return
+	}
 	m.AddShutdownJob(func() error {
 		log.Println("Shutting down audit service...")
 
@@ -120,7 +122,7 @@ func addAuditServiceShutdownJob(
 func addAuditLogCleanupJob(
 	m *graceful.Manager,
 	cfg *config.Config,
-	auditService *services.AuditService,
+	auditService core.AuditLogger,
 ) {
 	if !cfg.EnableAuditLogging || cfg.AuditLogRetention <= 0 {
 		return

internal/bootstrap/services.go

@@ -23,7 +23,7 @@ type serviceSet struct {
 func initializeServices(
 	cfg *config.Config,
 	db *store.Store,
-	auditService *services.AuditService,
+	auditService core.AuditLogger,
 	prometheusMetrics core.Recorder,
 	userCache core.Cache[models.User],
 	clientCountCache core.Cache[int64],

internal/core/audit.go

@@ -0,0 +1,44 @@
+package core
+
+import (
+	"context"
+	"time"
+
+	"github.com/go-authgate/authgate/internal/models"
+	"github.com/go-authgate/authgate/internal/store/types"
+)
+
+// AuditLogEntry represents the data needed to create an audit log entry.
+type AuditLogEntry struct {
+	EventType     models.EventType
+	Severity      models.EventSeverity
+	ActorUserID   string
+	ActorUsername string
+	ActorIP       string
+	ResourceType  models.ResourceType
+	ResourceID    string
+	ResourceName  string
+	Action        string
+	Details       models.AuditDetails
+	Success       bool
+	ErrorMessage  string
+	UserAgent     string
+	RequestPath   string
+	RequestMethod string
+}
+
+// AuditLogger defines the contract for audit logging operations.
+// Implementations must be safe for concurrent use by multiple goroutines.
+// Implementations include the real AuditService (buffered, database-backed)
+// and NoopAuditService (silent no-op for when auditing is disabled).
+type AuditLogger interface {
+	Log(ctx context.Context, entry AuditLogEntry)
+	LogSync(ctx context.Context, entry AuditLogEntry) error
+	GetAuditLogs(
+		params types.PaginationParams,
+		filters types.AuditLogFilters,
+	) ([]models.AuditLog, types.PaginationResult, error)
+	CleanupOldLogs(retention time.Duration) (int64, error)
+	GetAuditLogStats(startTime, endTime time.Time) (types.AuditLogStats, error)
+	Shutdown(ctx context.Context) error
+}

internal/handlers/audit.go

@@ -8,20 +8,20 @@ import (
 	"time"
 
 	"github.com/gin-gonic/gin"
+	"github.com/go-authgate/authgate/internal/core"
 	"github.com/go-authgate/authgate/internal/middleware"
 	"github.com/go-authgate/authgate/internal/models"
-	"github.com/go-authgate/authgate/internal/services"
 	"github.com/go-authgate/authgate/internal/store"
 	"github.com/go-authgate/authgate/internal/templates"
 )
 
 // AuditHandler handles audit log operations
 type AuditHandler struct {
-	auditService *services.AuditService
+	auditService core.AuditLogger
 }
 
 // NewAuditHandler creates a new audit handler
-func NewAuditHandler(auditService *services.AuditService) *AuditHandler {
+func NewAuditHandler(auditService core.AuditLogger) *AuditHandler {
 	return &AuditHandler{
 		auditService: auditService,
 	}
@@ -121,7 +121,7 @@ func (h *AuditHandler) ListAuditLogs(c *gin.Context) {
 	// Log this action (viewing audit logs)
 	if userID, exists := c.Get("user_id"); exists {
 		if username, usernameExists := c.Get("username"); usernameExists {
-			h.auditService.Log(c.Request.Context(), services.AuditLogEntry{
+			h.auditService.Log(c.Request.Context(), core.AuditLogEntry{
 				EventType:     models.EventTypeAuditLogView,
 				Severity:      models.SeverityInfo,
 				ActorUserID:   userID.(string),
@@ -281,7 +281,7 @@ func (h *AuditHandler) ExportAuditLogs(c *gin.Context) {
 	// Log this action
 	if userID, exists := c.Get("user_id"); exists {
 		if username, usernameExists := c.Get("username"); usernameExists {
-			h.auditService.Log(c.Request.Context(), services.AuditLogEntry{
+			h.auditService.Log(c.Request.Context(), core.AuditLogEntry{
 				EventType:     models.EventTypeAuditLogExported,
 				Severity:      models.SeverityInfo,
 				ActorUserID:   userID.(string),

internal/handlers/device_test.go

@@ -36,7 +36,7 @@ func setupDeviceTestEnv(t *testing.T) (*gin.Engine, *store.Store) {
 	s, err := store.New(context.Background(), "sqlite", ":memory:", &config.Config{})
 	require.NoError(t, err)
 
-	auditSvc := services.NewAuditService(s, false, 0)
+	auditSvc := services.NewNoopAuditService()
 	deviceSvc := services.NewDeviceService(s, cfg, auditSvc, metrics.NewNoopMetrics())
 	userSvc := services.NewUserService(s, nil, nil, "local", false, auditSvc, nil, 0)
 	authzSvc := services.NewAuthorizationService(s, cfg, auditSvc, nil)