mirror of
https://github.com/netbirdio/netbird.git
synced 2026-05-29 12:09:59 +00:00
chore(accesslogs): drop UserGroups field — nothing populates it
UserGroups on AccessLogEntry was a server-side enrichment artefact: the proto AccessLog message never carried it, so the only writer was manager.enrichUserGroups at save time. Without that writer the field stays nil forever and the dashboard's user_groups column is always empty — better to remove the dead surface than ship an unused field. The dashboard can still reverse-resolve groups from UserId when it needs them, accepting the tradeoff that memberships are resolved at display time rather than captured at write time. - AccessLogEntry.UserGroups field removed (no GORM column either). - ToAPIResponse stops emitting the user_groups key. - openapi.yml user_groups field removed; types.gen.go regenerated. - enrichUserGroups + its test removed.
This commit is contained in:
@@ -39,10 +39,6 @@ type AccessLogEntry struct {
|
||||
BytesDownload int64 `gorm:"index"`
|
||||
Protocol AccessLogProtocol `gorm:"index"`
|
||||
Metadata map[string]string `gorm:"serializer:json"`
|
||||
// UserGroups captures the group IDs the user (UserId) belonged to at
|
||||
// the time the entry was written, so the dashboard can render group
|
||||
// context without reverse-resolving stale memberships.
|
||||
UserGroups []string `gorm:"serializer:json"`
|
||||
}
|
||||
|
||||
// FromProto creates an AccessLogEntry from a proto.AccessLog
|
||||
@@ -129,12 +125,6 @@ func (a *AccessLogEntry) ToAPIResponse() *api.ProxyAccessLog {
|
||||
metadata = &a.Metadata
|
||||
}
|
||||
|
||||
var userGroups *[]string
|
||||
if len(a.UserGroups) > 0 {
|
||||
groups := append([]string(nil), a.UserGroups...)
|
||||
userGroups = &groups
|
||||
}
|
||||
|
||||
return &api.ProxyAccessLog{
|
||||
Id: a.ID,
|
||||
ServiceId: a.ServiceID,
|
||||
@@ -155,6 +145,5 @@ func (a *AccessLogEntry) ToAPIResponse() *api.ProxyAccessLog {
|
||||
BytesDownload: a.BytesDownload,
|
||||
Protocol: protocol,
|
||||
Metadata: metadata,
|
||||
UserGroups: userGroups,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,41 +0,0 @@
|
||||
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")
|
||||
}
|
||||
@@ -47,8 +47,6 @@ func (m *managerImpl) SaveAccessLog(ctx context.Context, logEntry *accesslogs.Ac
|
||||
}
|
||||
}
|
||||
|
||||
m.enrichUserGroups(ctx, logEntry)
|
||||
|
||||
if err := m.store.CreateAccessLog(ctx, logEntry); err != nil {
|
||||
log.WithContext(ctx).WithFields(log.Fields{
|
||||
"service_id": logEntry.ServiceID,
|
||||
@@ -63,25 +61,6 @@ 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)
|
||||
|
||||
@@ -2896,11 +2896,6 @@ components:
|
||||
additionalProperties:
|
||||
type: string
|
||||
description: "Extra context about the request (e.g. crowdsec_verdict)"
|
||||
user_groups:
|
||||
type: array
|
||||
items:
|
||||
type: string
|
||||
description: "Group IDs the user belonged to when the entry was written"
|
||||
required:
|
||||
- id
|
||||
- service_id
|
||||
|
||||
@@ -3786,9 +3786,6 @@ type ProxyAccessLog struct {
|
||||
// Timestamp Timestamp when the request was made
|
||||
Timestamp time.Time `json:"timestamp"`
|
||||
|
||||
// UserGroups Group IDs the user belonged to when the entry was written
|
||||
UserGroups *[]string `json:"user_groups,omitempty"`
|
||||
|
||||
// UserId ID of the authenticated user, if applicable
|
||||
UserId *string `json:"user_id,omitempty"`
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user