From fc2c23244a01f9fc3c04b695f8cc0e055dc6bcba Mon Sep 17 00:00:00 2001 From: Bo-Yi Wu Date: Mon, 1 Jun 2026 17:52:07 +0800 Subject: [PATCH 1/3] feat(audit): record and display actor full name in audit logs - Add an ActorFullName snapshot column to the audit log model and entry - Add a context helper to read the actor full name during enrichment - Fill the full name from the context user or a single backfill lookup - Show username with full name in the audit and dashboard actor columns - Match the audit search against the actor full name - Add a separate actor full name column to the CSV export --- internal/core/audit.go | 1 + internal/handlers/audit.go | 2 + internal/models/audit_log.go | 1 + internal/models/context.go | 9 +++++ internal/services/audit.go | 47 +++++++++++++++-------- internal/services/audit_test.go | 36 +++++++++++++++++ internal/store/audit_log.go | 7 +++- internal/store/audit_log_test.go | 16 ++++++++ internal/templates/admin_audit_logs.templ | 4 +- internal/templates/admin_dashboard.templ | 2 +- 10 files changed, 104 insertions(+), 21 deletions(-) diff --git a/internal/core/audit.go b/internal/core/audit.go index 874aa1d6..f280f333 100644 --- a/internal/core/audit.go +++ b/internal/core/audit.go @@ -14,6 +14,7 @@ type AuditLogEntry struct { Severity models.EventSeverity ActorUserID string ActorUsername string + ActorFullName string ActorIP string ResourceType models.ResourceType ResourceID string diff --git a/internal/handlers/audit.go b/internal/handlers/audit.go index 539f79c3..0d7cf378 100644 --- a/internal/handlers/audit.go +++ b/internal/handlers/audit.go @@ -225,6 +225,7 @@ func (h *AuditHandler) ExportAuditLogs(c *gin.Context) { "Event Type", "Severity", "Actor Username", + "Actor Full Name", "Actor IP", "Resource Type", "Resource Name", @@ -247,6 +248,7 @@ func (h *AuditHandler) ExportAuditLogs(c *gin.Context) { string(log.EventType), string(log.Severity), log.ActorUsername, + log.ActorFullName, log.ActorIP, string(log.ResourceType), log.ResourceName, diff --git a/internal/models/audit_log.go b/internal/models/audit_log.go index 551abd84..20d3e69c 100644 --- a/internal/models/audit_log.go +++ b/internal/models/audit_log.go @@ -144,6 +144,7 @@ type AuditLog struct { // Actor information ActorUserID string `gorm:"type:varchar(36);index" json:"actor_user_id"` ActorUsername string `gorm:"type:varchar(100)" json:"actor_username"` + ActorFullName string `gorm:"type:varchar(100)" json:"actor_full_name"` ActorIP string `gorm:"type:varchar(45);index" json:"actor_ip"` // Support IPv6 // Resource information diff --git a/internal/models/context.go b/internal/models/context.go index af01ea8a..1d4d31c4 100644 --- a/internal/models/context.go +++ b/internal/models/context.go @@ -28,6 +28,15 @@ func GetUsernameFromContext(ctx context.Context) string { return "" } +// GetFullNameFromContext extracts the full name from the user object in context. +// Returns empty string if user cannot be determined. +func GetFullNameFromContext(ctx context.Context) string { + if user, ok := ctx.Value(contextKeyUser).(*User); ok && user != nil { + return user.FullName + } + return "" +} + // GetUserIDFromContext extracts user ID from context func GetUserIDFromContext(ctx context.Context) string { if user, ok := ctx.Value(contextKeyUser).(*User); ok && user != nil { diff --git a/internal/services/audit.go b/internal/services/audit.go index bbcba77a..9c62286d 100644 --- a/internal/services/audit.go +++ b/internal/services/audit.go @@ -200,27 +200,41 @@ func (s *AuditService) buildAuditLog( if entry.ActorIP == "" { entry.ActorIP = util.GetIPFromContext(ctx) } - // Fill ActorUsername from context only when the entry's ActorUserID is - // empty or matches the context user — otherwise the username could be - // misattributed to a different principal than ActorUserID identifies. - if entry.ActorUsername == "" { - ctxUserID := models.GetUserIDFromContext(ctx) - if entry.ActorUserID == "" || entry.ActorUserID == ctxUserID { - entry.ActorUsername = models.GetUsernameFromContext(ctx) + // Fill the actor's username/full name from the context user, but only when + // the entry's ActorUserID is empty or matches that user — otherwise the + // values could be misattributed to a different principal than ActorUserID + // identifies. The context user is the authoritative, freshly-loaded row + // (see middleware.loadUserFromSession), so when it supplies the actor we + // skip the DB fallback entirely. + ctxUser := models.GetUserFromContext(ctx) + resolvedFromContext := ctxUser != nil && + (entry.ActorUserID == "" || entry.ActorUserID == ctxUser.ID) + if resolvedFromContext { + if entry.ActorUsername == "" { + entry.ActorUsername = ctxUser.Username + } + if entry.ActorFullName == "" { + entry.ActorFullName = ctxUser.FullName } } - // Fall back to a DB lookup when context did not provide a username and - // the actor is a real user (not a synthetic machine identity from the - // client_credentials grant, which uses a "client:" format and - // has no corresponding user row). - if entry.ActorUsername == "" && entry.ActorUserID != "" && - !IsMachineUserID(entry.ActorUserID) { + // Fall back to a DB lookup only when the actor was not resolved from the + // context user (e.g. token grants that set ActorUserID from request data + // without a logged-in session) and is a real user — not a synthetic machine + // identity from the client_credentials grant, which uses a "client:" + // format and has no corresponding user row. + if !resolvedFromContext && (entry.ActorUsername == "" || entry.ActorFullName == "") && + entry.ActorUserID != "" && !IsMachineUserID(entry.ActorUserID) { if user, err := s.store.GetUserByID(entry.ActorUserID); err == nil { - entry.ActorUsername = user.Username + if entry.ActorUsername == "" { + entry.ActorUsername = user.Username + } + if entry.ActorFullName == "" { + entry.ActorFullName = user.FullName + } } } - if entry.ActorUserID == "" { - entry.ActorUserID = models.GetUserIDFromContext(ctx) + if entry.ActorUserID == "" && ctxUser != nil { + entry.ActorUserID = ctxUser.ID } if entry.UserAgent == "" { entry.UserAgent = util.GetUserAgentFromContext(ctx) @@ -254,6 +268,7 @@ func (s *AuditService) buildAuditLog( Severity: entry.Severity, ActorUserID: entry.ActorUserID, ActorUsername: entry.ActorUsername, + ActorFullName: entry.ActorFullName, ActorIP: entry.ActorIP, ResourceType: entry.ResourceType, ResourceID: entry.ResourceID, diff --git a/internal/services/audit_test.go b/internal/services/audit_test.go index 9f14f568..c304faa9 100644 --- a/internal/services/audit_test.go +++ b/internal/services/audit_test.go @@ -133,6 +133,7 @@ func TestBuildAuditLog_EnrichesUserFromContext(t *testing.T) { user := &models.User{ ID: "user-123", Username: "testuser", + FullName: "Test User", } ctx := models.SetUserContext(context.Background(), user) @@ -147,6 +148,7 @@ func TestBuildAuditLog_EnrichesUserFromContext(t *testing.T) { assert.Equal(t, "user-123", result.ActorUserID) assert.Equal(t, "testuser", result.ActorUsername) + assert.Equal(t, "Test User", result.ActorFullName) } func TestBuildAuditLog_FillsActorUsernameFromDBFallback(t *testing.T) { @@ -156,6 +158,7 @@ func TestBuildAuditLog_FillsActorUsernameFromDBFallback(t *testing.T) { user := &models.User{ ID: "fallback-user-id", Username: "fallback-user", + FullName: "Fallback User", Email: "fallback@example.com", PasswordHash: "x", AuthSource: models.AuthSourceLocal, @@ -176,6 +179,39 @@ func TestBuildAuditLog_FillsActorUsernameFromDBFallback(t *testing.T) { assert.Equal(t, user.ID, result.ActorUserID) assert.Equal(t, "fallback-user", result.ActorUsername) + assert.Equal(t, "Fallback User", result.ActorFullName) +} + +func TestBuildAuditLog_FillsActorFullNameWhenUsernameAlreadySet(t *testing.T) { + // A call site that pre-sets ActorUserID + ActorUsername (no context user, + // e.g. a token grant with no logged-in session) must still backfill + // ActorFullName via the DB lookup even though ActorUsername is populated. + s := setupTestStore(t) + user := &models.User{ + ID: "view-user-id", + Username: "view-user", + FullName: "View User", + Email: "view@example.com", + PasswordHash: "x", + AuthSource: models.AuthSourceLocal, + } + require.NoError(t, s.CreateUser(user)) + + svc := &AuditService{store: s} + + entry := core.AuditLogEntry{ + EventType: models.EventTypeAuditLogView, + Severity: models.SeverityInfo, + ActorUserID: user.ID, + ActorUsername: "view-user", // already set, full name intentionally empty + Action: "Viewed audit logs", + Success: true, + } + + result := svc.buildAuditLog(context.Background(), entry) + + assert.Equal(t, "view-user", result.ActorUsername) + assert.Equal(t, "View User", result.ActorFullName) } func TestBuildAuditLog_SkipsDBLookupForMachineIdentity(t *testing.T) { diff --git a/internal/store/audit_log.go b/internal/store/audit_log.go index 3a6a1f6e..625dbd68 100644 --- a/internal/store/audit_log.go +++ b/internal/store/audit_log.go @@ -65,8 +65,11 @@ func (s *Store) GetAuditLogsPaginated( if filters.Search != "" { searchPattern := "%" + filters.Search + "%" query = query.Where( - "action LIKE ? OR resource_name LIKE ? OR actor_username LIKE ?", - searchPattern, searchPattern, searchPattern, + "action LIKE ? OR resource_name LIKE ? OR actor_username LIKE ? OR actor_full_name LIKE ?", + searchPattern, + searchPattern, + searchPattern, + searchPattern, ) } diff --git a/internal/store/audit_log_test.go b/internal/store/audit_log_test.go index 0778cb47..663f8c54 100644 --- a/internal/store/audit_log_test.go +++ b/internal/store/audit_log_test.go @@ -121,6 +121,7 @@ func TestGetAuditLogsPaginated(t *testing.T) { Severity: models.SeverityWarning, ActorUserID: uuid.New().String(), ActorUsername: "bob", + ActorFullName: "Wang Xiaoming", ActorIP: "10.0.0.2", ResourceType: models.ResourceUser, Action: "login_fail", @@ -189,6 +190,21 @@ func TestGetAuditLogsPaginated(t *testing.T) { require.NoError(t, err) assert.Len(t, result, 1) + // Filter by search (matches actor_full_name) + result, _, err = store.GetAuditLogsPaginated(params, AuditLogFilters{ + Search: "Xiaoming", + }) + require.NoError(t, err) + assert.Len(t, result, 1) + assert.Equal(t, "bob", result[0].ActorUsername) + + // Filter by search (no match) + result, _, err = store.GetAuditLogsPaginated(params, AuditLogFilters{ + Search: "nonexistent-actor", + }) + require.NoError(t, err) + assert.Empty(t, result) + // Filter by time range result, _, err = store.GetAuditLogsPaginated(params, AuditLogFilters{ StartTime: now.Add(-90 * time.Minute), diff --git a/internal/templates/admin_audit_logs.templ b/internal/templates/admin_audit_logs.templ index 8eaa1e5c..198f2878 100644 --- a/internal/templates/admin_audit_logs.templ +++ b/internal/templates/admin_audit_logs.templ @@ -42,7 +42,7 @@ templ AdminAuditLogs(props AuditLogsPageProps) { type="text" name="search" class="search-input" - placeholder="Search action, resource, username..." + placeholder="Search action, resource, username, full name..." value={ props.Search } data-debounce-search /> @@ -277,7 +277,7 @@ templ AuditLogRow(log *models.AuditLog) { if log.ActorUsername != "" { - { log.ActorUsername } + { UserDisplay(log.ActorUsername, log.ActorFullName) } } else { System } diff --git a/internal/templates/admin_dashboard.templ b/internal/templates/admin_dashboard.templ index 7fbc9ff9..7a697674 100644 --- a/internal/templates/admin_dashboard.templ +++ b/internal/templates/admin_dashboard.templ @@ -134,7 +134,7 @@ templ AdminDashboard(props DashboardPageProps) { if log.ActorUsername != "" { - { log.ActorUsername } + { UserDisplay(log.ActorUsername, log.ActorFullName) } } else { system } From 8e1fa395f2a23d8aa69cfdd4dd6995973fa2c74e Mon Sep 17 00:00:00 2001 From: Bo-Yi Wu Date: Mon, 1 Jun 2026 18:07:03 +0800 Subject: [PATCH 2/3] fix(audit): clamp actor names to the audit column width - Truncate actor username and full name to the varchar(100) audit columns so over-long names cannot error the INSERT on Postgres - Add a unit test for the full name context accessor - Cover actor name truncation in the audit log builder tests --- internal/models/context_test.go | 52 +++++++++++++++++++++++++++++++++ internal/services/audit.go | 5 ++++ internal/services/audit_test.go | 22 ++++++++++++++ 3 files changed, 79 insertions(+) diff --git a/internal/models/context_test.go b/internal/models/context_test.go index cbe3280f..fca61f8f 100644 --- a/internal/models/context_test.go +++ b/internal/models/context_test.go @@ -95,6 +95,58 @@ func TestGetUsernameFromContext(t *testing.T) { } } +func TestGetFullNameFromContext(t *testing.T) { + tests := []struct { + name string + user *User + expected string + }{ + { + name: "Valid user with full name", + user: &User{ + ID: "user-123", + Username: "testuser", + FullName: "Test User", + }, + expected: "Test User", + }, + { + name: "Valid user without full name", + user: &User{ + ID: "user-123", + Username: "testuser", + }, + expected: "", + }, + { + name: "Nil user", + user: nil, + expected: "", + }, + { + name: "Empty context", + user: nil, + expected: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var ctx context.Context + if tt.user != nil { + ctx = SetUserContext(context.Background(), tt.user) + } else { + ctx = context.Background() + } + + fullName := GetFullNameFromContext(ctx) + if fullName != tt.expected { + t.Errorf("Expected full name %q, got %q", tt.expected, fullName) + } + }) + } +} + func TestGetUserIDFromContext(t *testing.T) { tests := []struct { name string diff --git a/internal/services/audit.go b/internal/services/audit.go index 9c62286d..78576078 100644 --- a/internal/services/audit.go +++ b/internal/services/audit.go @@ -250,8 +250,13 @@ func (s *AuditService) buildAuditLog( // Truncate fields to match database column size limits. // TruncateString appends "..." (3 chars) when truncating, so subtract 3 // from the varchar limit to guarantee the final length fits the column. + // ActorUsername/ActorFullName mirror their unbounded User columns, so they + // must be clamped to the varchar(100) audit columns to avoid INSERT errors + // on stricter drivers (e.g. Postgres) for over-long names. entry.UserAgent = util.TruncateString(entry.UserAgent, 497) entry.RequestPath = util.TruncateString(entry.RequestPath, 497) + entry.ActorUsername = util.TruncateString(entry.ActorUsername, 97) + entry.ActorFullName = util.TruncateString(entry.ActorFullName, 97) // RequestMethod is stored in a varchar(10) column. Preserve values up to // the full column width and hard-truncate anything longer without adding diff --git a/internal/services/audit_test.go b/internal/services/audit_test.go index c304faa9..620b53b2 100644 --- a/internal/services/audit_test.go +++ b/internal/services/audit_test.go @@ -3,6 +3,7 @@ package services import ( "context" "fmt" + "strings" "testing" "time" @@ -100,6 +101,27 @@ func TestBuildAuditLog_EnrichesRequestMetadataFromContext(t *testing.T) { assert.Equal(t, "POST", result.RequestMethod) } +func TestBuildAuditLog_TruncatesActorNamesToColumnWidth(t *testing.T) { + // ActorUsername/ActorFullName mirror unbounded User columns but persist into + // varchar(100) audit columns, so over-long values must be clamped to avoid + // INSERT errors on stricter drivers (e.g. Postgres). + svc := &AuditService{} + + entry := core.AuditLogEntry{ + EventType: models.EventAccessTokenIssued, + Severity: models.SeverityInfo, + ActorUsername: strings.Repeat("u", 200), + ActorFullName: strings.Repeat("n", 200), + Action: "test", + Success: true, + } + + result := svc.buildAuditLog(context.Background(), entry) + + assert.LessOrEqual(t, len(result.ActorUsername), 100) + assert.LessOrEqual(t, len(result.ActorFullName), 100) +} + func TestBuildAuditLog_DoesNotOverrideExplicitValues(t *testing.T) { svc := &AuditService{} From 3a39eab5b26c679e003d0aa77f987e9a6f26d527 Mon Sep 17 00:00:00 2001 From: Bo-Yi Wu Date: Mon, 1 Jun 2026 18:23:37 +0800 Subject: [PATCH 3/3] fix(audit): preserve actor names that fit the column width - Clamp audit string fields only when they exceed the varchar width so values already within the limit are stored verbatim instead of being needlessly truncated with an ellipsis - Drop the now-unused GetFullNameFromContext helper and its test; the audit builder reads the full name from the context user directly --- internal/models/context.go | 9 ------ internal/models/context_test.go | 52 --------------------------------- internal/services/audit.go | 31 +++++++++++++------- internal/services/audit_test.go | 27 +++++++++++++++-- 4 files changed, 45 insertions(+), 74 deletions(-) diff --git a/internal/models/context.go b/internal/models/context.go index 1d4d31c4..af01ea8a 100644 --- a/internal/models/context.go +++ b/internal/models/context.go @@ -28,15 +28,6 @@ func GetUsernameFromContext(ctx context.Context) string { return "" } -// GetFullNameFromContext extracts the full name from the user object in context. -// Returns empty string if user cannot be determined. -func GetFullNameFromContext(ctx context.Context) string { - if user, ok := ctx.Value(contextKeyUser).(*User); ok && user != nil { - return user.FullName - } - return "" -} - // GetUserIDFromContext extracts user ID from context func GetUserIDFromContext(ctx context.Context) string { if user, ok := ctx.Value(contextKeyUser).(*User); ok && user != nil { diff --git a/internal/models/context_test.go b/internal/models/context_test.go index fca61f8f..cbe3280f 100644 --- a/internal/models/context_test.go +++ b/internal/models/context_test.go @@ -95,58 +95,6 @@ func TestGetUsernameFromContext(t *testing.T) { } } -func TestGetFullNameFromContext(t *testing.T) { - tests := []struct { - name string - user *User - expected string - }{ - { - name: "Valid user with full name", - user: &User{ - ID: "user-123", - Username: "testuser", - FullName: "Test User", - }, - expected: "Test User", - }, - { - name: "Valid user without full name", - user: &User{ - ID: "user-123", - Username: "testuser", - }, - expected: "", - }, - { - name: "Nil user", - user: nil, - expected: "", - }, - { - name: "Empty context", - user: nil, - expected: "", - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - var ctx context.Context - if tt.user != nil { - ctx = SetUserContext(context.Background(), tt.user) - } else { - ctx = context.Background() - } - - fullName := GetFullNameFromContext(ctx) - if fullName != tt.expected { - t.Errorf("Expected full name %q, got %q", tt.expected, fullName) - } - }) - } -} - func TestGetUserIDFromContext(t *testing.T) { tests := []struct { name string diff --git a/internal/services/audit.go b/internal/services/audit.go index 78576078..f0e32667 100644 --- a/internal/services/audit.go +++ b/internal/services/audit.go @@ -8,6 +8,7 @@ import ( "sync" "sync/atomic" "time" + "unicode/utf8" "github.com/go-authgate/authgate/internal/core" "github.com/go-authgate/authgate/internal/models" @@ -192,6 +193,17 @@ func (s *AuditService) flushBatchUnsafe() { } } +// clampToColumn returns s unchanged when it already fits within limit runes, +// otherwise it truncates to limit runes (the trailing 3 reserved for the "..." +// that TruncateString appends). This preserves values that are within the +// varchar column width verbatim instead of needlessly truncating them. +func clampToColumn(s string, limit int) string { + if utf8.RuneCountInString(s) <= limit { + return s + } + return util.TruncateString(s, limit-3) +} + // buildAuditLog enriches an AuditLogEntry from context and builds the database record. func (s *AuditService) buildAuditLog( ctx context.Context, @@ -247,16 +259,15 @@ func (s *AuditService) buildAuditLog( } entry.Details = maskSensitiveDetails(entry.Details) - // Truncate fields to match database column size limits. - // TruncateString appends "..." (3 chars) when truncating, so subtract 3 - // from the varchar limit to guarantee the final length fits the column. - // ActorUsername/ActorFullName mirror their unbounded User columns, so they - // must be clamped to the varchar(100) audit columns to avoid INSERT errors - // on stricter drivers (e.g. Postgres) for over-long names. - entry.UserAgent = util.TruncateString(entry.UserAgent, 497) - entry.RequestPath = util.TruncateString(entry.RequestPath, 497) - entry.ActorUsername = util.TruncateString(entry.ActorUsername, 97) - entry.ActorFullName = util.TruncateString(entry.ActorFullName, 97) + // Clamp string fields to their varchar column widths. Values already within + // the limit are preserved verbatim; only over-long values are truncated + // (with an ellipsis). ActorUsername/ActorFullName mirror their unbounded + // User columns, so clamping them avoids INSERT errors on stricter drivers + // (e.g. Postgres) for over-long names. + entry.UserAgent = clampToColumn(entry.UserAgent, 500) + entry.RequestPath = clampToColumn(entry.RequestPath, 500) + entry.ActorUsername = clampToColumn(entry.ActorUsername, 100) + entry.ActorFullName = clampToColumn(entry.ActorFullName, 100) // RequestMethod is stored in a varchar(10) column. Preserve values up to // the full column width and hard-truncate anything longer without adding diff --git a/internal/services/audit_test.go b/internal/services/audit_test.go index 620b53b2..f3b88609 100644 --- a/internal/services/audit_test.go +++ b/internal/services/audit_test.go @@ -101,7 +101,7 @@ func TestBuildAuditLog_EnrichesRequestMetadataFromContext(t *testing.T) { assert.Equal(t, "POST", result.RequestMethod) } -func TestBuildAuditLog_TruncatesActorNamesToColumnWidth(t *testing.T) { +func TestBuildAuditLog_TruncatesOverlongActorNames(t *testing.T) { // ActorUsername/ActorFullName mirror unbounded User columns but persist into // varchar(100) audit columns, so over-long values must be clamped to avoid // INSERT errors on stricter drivers (e.g. Postgres). @@ -118,8 +118,29 @@ func TestBuildAuditLog_TruncatesActorNamesToColumnWidth(t *testing.T) { result := svc.buildAuditLog(context.Background(), entry) - assert.LessOrEqual(t, len(result.ActorUsername), 100) - assert.LessOrEqual(t, len(result.ActorFullName), 100) + assert.LessOrEqual(t, len([]rune(result.ActorUsername)), 100) + assert.LessOrEqual(t, len([]rune(result.ActorFullName)), 100) +} + +func TestBuildAuditLog_PreservesActorNamesWithinColumnWidth(t *testing.T) { + // A name that already fits the varchar(100) column must be stored verbatim, + // not needlessly truncated by the ellipsis-appending clamp. + svc := &AuditService{} + + fullName := strings.Repeat("n", 100) // exactly the column width + entry := core.AuditLogEntry{ + EventType: models.EventAccessTokenIssued, + Severity: models.SeverityInfo, + ActorUsername: "alice", + ActorFullName: fullName, + Action: "test", + Success: true, + } + + result := svc.buildAuditLog(context.Background(), entry) + + assert.Equal(t, "alice", result.ActorUsername) + assert.Equal(t, fullName, result.ActorFullName) } func TestBuildAuditLog_DoesNotOverrideExplicitValues(t *testing.T) {