diff --git a/management/server/group.go b/management/server/group.go index dab891f2a..e6f748d02 100644 --- a/management/server/group.go +++ b/management/server/group.go @@ -6,6 +6,7 @@ import ( "fmt" "slices" + agentNetworkTypes "github.com/netbirdio/netbird/management/internals/modules/agentnetwork/types" "github.com/rs/xid" log "github.com/sirupsen/logrus" @@ -744,6 +745,10 @@ func validateDeleteGroup(ctx context.Context, transaction store.Store, group *ty return &GroupLinkError{"network router", linkedRouter.ID} } + if isLinked, linkedPolicy := isGroupLinkedToAgentNetworkPolicy(ctx, transaction, group.AccountID, group.ID); isLinked { + return &GroupLinkError{"agent network policy", linkedPolicy.Name} + } + return checkGroupLinkedToSettings(ctx, transaction, group) } @@ -875,6 +880,26 @@ func isGroupLinkedToNetworkRouter(ctx context.Context, transaction store.Store, return false, nil } +// isGroupLinkedToAgentNetworkPolicy checks if a group is used as a source group by any +// agent network policy in the account. +func isGroupLinkedToAgentNetworkPolicy(ctx context.Context, transaction store.Store, accountID string, groupID string) (bool, *agentNetworkTypes.Policy) { + policies, err := transaction.GetAccountAgentNetworkPolicies(ctx, store.LockingStrengthNone, accountID) + if err != nil { + log.WithContext(ctx).Errorf("error retrieving agent network policies while checking group linkage: %v", err) + return false, nil + } + + for _, policy := range policies { + if policy == nil { + continue + } + if slices.Contains(policy.SourceGroups, groupID) { + return true, policy + } + } + return false, nil +} + // areGroupChangesAffectPeers checks if any changes to the specified groups will affect peers. // It fetches each collection once and checks all groupIDs against them in memory. func areGroupChangesAffectPeers(ctx context.Context, transaction store.Store, accountID string, groupIDs []string) (bool, error) { diff --git a/management/server/group_test.go b/management/server/group_test.go index 22fda2671..17bb59d6a 100644 --- a/management/server/group_test.go +++ b/management/server/group_test.go @@ -18,6 +18,7 @@ import ( "golang.org/x/exp/maps" nbdns "github.com/netbirdio/netbird/dns" + agentNetworkTypes "github.com/netbirdio/netbird/management/internals/modules/agentnetwork/types" "github.com/netbirdio/netbird/management/server/groups" "github.com/netbirdio/netbird/management/server/networks" "github.com/netbirdio/netbird/management/server/networks/resources" @@ -125,6 +126,11 @@ func TestDefaultAccountManager_DeleteGroup(t *testing.T) { "grp-for-integration", "only service users with admin power can delete integration group", }, + { + "agent network policy", + "grp-for-agent-network-policy", + "agent network policy", + }, } for _, testCase := range testCases { @@ -218,6 +224,11 @@ func TestDefaultAccountManager_DeleteGroups(t *testing.T) { groupIDs: []string{"grp-for-integration"}, expectedReasons: []string{"only service users with admin power can delete integration group"}, }, + { + name: "agent network policy", + groupIDs: []string{"grp-for-agent-network-policy"}, + expectedReasons: []string{"agent network policy"}, + }, { name: "successfully delete multiple groups", groupIDs: []string{"group-1", "group-2"}, @@ -406,6 +417,14 @@ func initTestGroupAccount(am *DefaultAccountManager) (*DefaultAccountManager, *t Peers: make([]string, 0), } + groupForAgentNetworkPolicy := &types.Group{ + ID: "grp-for-agent-network-policy", + AccountID: "account-id", + Name: "Group for agent network policies", + Issued: types.GroupIssuedAPI, + Peers: make([]string, 0), + } + routeResource := &route.Route{ ID: "example route", Groups: []string{groupForRoute.ID}, @@ -461,6 +480,18 @@ func initTestGroupAccount(am *DefaultAccountManager) (*DefaultAccountManager, *t _ = am.CreateGroup(context.Background(), accountID, groupAdminUserID, groupForSetupKeys) _ = am.CreateGroup(context.Background(), accountID, groupAdminUserID, groupForUsers) _ = am.CreateGroup(context.Background(), accountID, groupAdminUserID, groupForIntegration) + _ = am.CreateGroup(context.Background(), accountID, groupAdminUserID, groupForAgentNetworkPolicy) + + agentNetworkPolicy := &agentNetworkTypes.Policy{ + ID: "example agent network policy", + AccountID: accountID, + Name: "Example agent network policy", + Enabled: true, + SourceGroups: []string{groupForAgentNetworkPolicy.ID}, + } + if err := am.Store.SaveAgentNetworkPolicy(context.Background(), agentNetworkPolicy); err != nil { + return nil, nil, err + } acc, err := am.Store.GetAccount(context.Background(), account.Id) if err != nil { diff --git a/management/server/types/account.go b/management/server/types/account.go index 1a3a30544..474825281 100644 --- a/management/server/types/account.go +++ b/management/server/types/account.go @@ -1707,14 +1707,34 @@ func (a *Account) injectPrivateServicePolicies(svc *service.Service, proxyPeers if len(proxyPeers) == 0 { return } + // A service's AccessGroups can name groups that no longer exist — persisted + // services and the agent-network synthesiser both carry the ids verbatim from + // their own state. An unresolvable source authorises nothing, so drop it here + // rather than let the network-map assembly resolve it to a nil group. + sources := a.existingGroupIDs(svc.AccessGroups) + if len(sources) == 0 { + return + } for _, proxyPeer := range proxyPeers { - a.Policies = append(a.Policies, a.createPrivateServicePolicy(svc, proxyPeer)) + a.Policies = append(a.Policies, a.createPrivateServicePolicy(svc, proxyPeer, sources)) } } -func (a *Account) createPrivateServicePolicy(svc *service.Service, proxyPeer *nbpeer.Peer) *Policy { +// existingGroupIDs returns the subset of groupIDs that resolve to a group in the account, +// preserving the input order. +func (a *Account) existingGroupIDs(groupIDs []string) []string { + out := make([]string, 0, len(groupIDs)) + for _, groupID := range groupIDs { + if _, ok := a.Groups[groupID]; ok { + out = append(out, groupID) + } + } + return out +} + +func (a *Account) createPrivateServicePolicy(svc *service.Service, proxyPeer *nbpeer.Peer, accessGroups []string) *Policy { policyID := fmt.Sprintf("private-access-%s-%s", svc.ID, proxyPeer.ID) - sources := append([]string(nil), svc.AccessGroups...) + sources := append([]string(nil), accessGroups...) return &Policy{ ID: policyID, Name: fmt.Sprintf("Private Access to %s", svc.Name),