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 {