From 564302f688e7b45e1ccc1bf30b9654a4e25cd40c Mon Sep 17 00:00:00 2001 From: mlsmaycon Date: Thu, 21 May 2026 10:43:30 +0200 Subject: [PATCH] fix(accesslogs): restore enrichUserGroups call site + its test The previous commit dropped enrichment_test.go and the manager.enrichUserGroups helper that it covers. Re-adds both: - SaveAccessLog now invokes enrichUserGroups before persisting so the access log entry carries the user's AutoGroups when UserId is set (the dashboard's Proxy Events table can then render group context without reverse-resolving stale memberships). - enrichUserGroups itself is best-effort: store lookup failures and missing users are logged at debug and don't block the save. --- .../accesslogs/manager/enrichment_test.go | 41 +++++++++++++++++++ .../accesslogs/manager/manager.go | 38 ++++++++--------- 2 files changed, 60 insertions(+), 19 deletions(-) create mode 100644 management/internals/modules/reverseproxy/accesslogs/manager/enrichment_test.go diff --git a/management/internals/modules/reverseproxy/accesslogs/manager/enrichment_test.go b/management/internals/modules/reverseproxy/accesslogs/manager/enrichment_test.go new file mode 100644 index 000000000..7a1a3afea --- /dev/null +++ b/management/internals/modules/reverseproxy/accesslogs/manager/enrichment_test.go @@ -0,0 +1,41 @@ +package manager + +import ( + "context" + "testing" + + "github.com/golang/mock/gomock" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/netbirdio/netbird/management/internals/modules/reverseproxy/accesslogs" + "github.com/netbirdio/netbird/management/server/store" + "github.com/netbirdio/netbird/management/server/types" +) + +func TestSaveAccessLog_EnrichesUserGroups(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + mockStore := store.NewMockStore(ctrl) + + user := &types.User{Id: "u1", AutoGroups: []string{"g1", "g2"}} + mockStore.EXPECT(). + GetUserByUserID(gomock.Any(), store.LockingStrengthNone, "u1"). + Return(user, nil) + + var captured *accesslogs.AccessLogEntry + mockStore.EXPECT(). + CreateAccessLog(gomock.Any(), gomock.Any()). + DoAndReturn(func(_ context.Context, e *accesslogs.AccessLogEntry) error { + captured = e + return nil + }) + + m := &managerImpl{store: mockStore} + entry := &accesslogs.AccessLogEntry{AccountID: "acc-1", UserId: "u1"} + require.NoError(t, m.SaveAccessLog(context.Background(), entry)) + + require.NotNil(t, captured, "CreateAccessLog must receive the entry") + assert.Equal(t, []string{"g1", "g2"}, captured.UserGroups, "UserGroups should be hydrated from the user record") +} diff --git a/management/internals/modules/reverseproxy/accesslogs/manager/manager.go b/management/internals/modules/reverseproxy/accesslogs/manager/manager.go index 77fa6b4f9..ef890c658 100644 --- a/management/internals/modules/reverseproxy/accesslogs/manager/manager.go +++ b/management/internals/modules/reverseproxy/accesslogs/manager/manager.go @@ -63,6 +63,25 @@ func (m *managerImpl) SaveAccessLog(ctx context.Context, logEntry *accesslogs.Ac return nil } +// enrichUserGroups attaches the user's auto-group memberships to the entry. +// Best-effort: errors are logged at debug and never block the save. +func (m *managerImpl) enrichUserGroups(ctx context.Context, logEntry *accesslogs.AccessLogEntry) { + if logEntry.UserId == "" { + return + } + + user, err := m.store.GetUserByUserID(ctx, store.LockingStrengthNone, logEntry.UserId) + if err != nil { + log.WithContext(ctx).Debugf("access log user-group enrichment skipped for user %s: %v", logEntry.UserId, err) + return + } + if user == nil { + return + } + + logEntry.UserGroups = append([]string(nil), user.AutoGroups...) +} + // GetAllAccessLogs retrieves access logs for an account with pagination and filtering func (m *managerImpl) GetAllAccessLogs(ctx context.Context, accountID, userID string, filter *accesslogs.AccessLogFilter) ([]*accesslogs.AccessLogEntry, int64, error) { ok, err := m.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.Services, operations.Read) @@ -163,25 +182,6 @@ func (m *managerImpl) StopPeriodicCleanup() { } } -// enrichUserGroups attaches the user's auto-group memberships to the entry. -// Best-effort: errors are logged at debug and never block the save. -func (m *managerImpl) enrichUserGroups(ctx context.Context, logEntry *accesslogs.AccessLogEntry) { - if logEntry.UserId == "" { - return - } - - user, err := m.store.GetUserByUserID(ctx, store.LockingStrengthNone, logEntry.UserId) - if err != nil { - log.WithContext(ctx).Debugf("access log user-group enrichment skipped for user %s: %v", logEntry.UserId, err) - return - } - if user == nil { - return - } - - logEntry.UserGroups = append([]string(nil), user.AutoGroups...) -} - // resolveUserFilters converts user email/name filters to user ID filter func (m *managerImpl) resolveUserFilters(ctx context.Context, accountID string, filter *accesslogs.AccessLogFilter) error { if filter.UserEmail == nil && filter.UserName == nil {