fix signature

Pre expand the peers from policy rules source and destination groups
to avoid extra allocation when calculating network map

.github/ISSUE_TEMPLATE/bug-issue-report.md

@@ -31,14 +31,9 @@ Please specify whether you use NetBird Cloud or self-host NetBird's control plan
 
 `netbird version`
 
-**NetBird status -dA output:**
+**NetBird status -d output:**
 
-If applicable, add the `netbird status -dA' command output.
-
-**Do you face any client issues on desktop?**
-
-Please provide the file created by `netbird debug for 1m -AS`.
-We advise reviewing the anonymized files for any remaining PII.
+If applicable, add the `netbird status -d' command output.
 
 **Screenshots**
 

.github/workflows/release.yml

@@ -6,7 +6,6 @@ on:
       - 'v*'
     branches:
       - main
-      - validate-icon
   pull_request:
 
 
@@ -94,28 +93,28 @@ jobs:
           UPLOAD_YUM_SECRET: ${{ secrets.PKG_UPLOAD_SECRET }}
       -
         name: upload non tags for debug purposes
-        uses: actions/upload-artifact@v4
+        uses: actions/upload-artifact@v3
         with:
           name: release
           path: dist/
           retention-days: 3
       -
         name: upload linux packages
-        uses: actions/upload-artifact@v4
+        uses: actions/upload-artifact@v3
         with:
           name: linux-packages
           path: dist/netbird_linux**
           retention-days: 3
       -
         name: upload windows packages
-        uses: actions/upload-artifact@v4
+        uses: actions/upload-artifact@v3
         with:
           name: windows-packages
           path: dist/netbird_windows**
           retention-days: 3
       -
         name: upload macos packages
-        uses: actions/upload-artifact@v4
+        uses: actions/upload-artifact@v3
         with:
           name: macos-packages
           path: dist/netbird_darwin**
@@ -177,7 +176,7 @@ jobs:
           UPLOAD_DEBIAN_SECRET: ${{ secrets.PKG_UPLOAD_SECRET }}
           UPLOAD_YUM_SECRET: ${{ secrets.PKG_UPLOAD_SECRET }}
       - name: upload non tags for debug purposes
-        uses: actions/upload-artifact@v4
+        uses: actions/upload-artifact@v3
         with:
           name: release-ui
           path: dist/
@@ -226,7 +225,7 @@ jobs:
           GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
       -
         name: upload non tags for debug purposes
-        uses: actions/upload-artifact@v4
+        uses: actions/upload-artifact@v3
         with:
           name: release-ui-darwin
           path: dist/

client/android/login.go

@@ -84,7 +84,7 @@ func (a *Auth) SaveConfigIfSSOSupported(listener SSOListener) {
 func (a *Auth) saveConfigIfSSOSupported() (bool, error) {
 	supportsSSO := true
 	err := a.withBackOff(a.ctx, func() (err error) {
-		_, err = internal.GetPKCEAuthorizationFlowInfo(a.ctx, a.config.PrivateKey, a.config.ManagementURL, nil)
+		_, err = internal.GetPKCEAuthorizationFlowInfo(a.ctx, a.config.PrivateKey, a.config.ManagementURL)
 		if s, ok := gstatus.FromError(err); ok && (s.Code() == codes.NotFound || s.Code() == codes.Unimplemented) {
 			_, err = internal.GetDeviceAuthorizationFlowInfo(a.ctx, a.config.PrivateKey, a.config.ManagementURL)
 			s, ok := gstatus.FromError(err)

client/cmd/login.go

@@ -39,11 +39,6 @@ var loginCmd = &cobra.Command{
 			ctx = context.WithValue(ctx, system.DeviceNameCtxKey, hostName)
 		}
 
-		providedSetupKey, err := getSetupKey()
-		if err != nil {
-			return err
-		}
-
 		// workaround to run without service
 		if logFile == "console" {
 			err = handleRebrand(cmd)
@@ -67,7 +62,7 @@ var loginCmd = &cobra.Command{
 
 			config, _ = internal.UpdateOldManagementURL(ctx, config, configPath)
 
-			err = foregroundLogin(ctx, cmd, config, providedSetupKey)
+			err = foregroundLogin(ctx, cmd, config, setupKey)
 			if err != nil {
 				return fmt.Errorf("foreground login failed: %v", err)
 			}
@@ -86,7 +81,7 @@ var loginCmd = &cobra.Command{
 		client := proto.NewDaemonServiceClient(conn)
 
 		loginRequest := proto.LoginRequest{
-			SetupKey:             providedSetupKey,
+			SetupKey:             setupKey,
 			ManagementUrl:        managementURL,
 			IsLinuxDesktopClient: isLinuxRunningDesktop(),
 			Hostname:             hostName,

client/cmd/root.go

@@ -56,7 +56,6 @@ var (
 	managementURL           string
 	adminURL                string
 	setupKey                string
-	setupKeyPath            string
 	hostName                string
 	preSharedKey            string
 	natExternalIPs          []string
@@ -129,8 +128,6 @@ func init() {
 	rootCmd.PersistentFlags().StringVarP(&logLevel, "log-level", "l", "info", "sets Netbird log level")
 	rootCmd.PersistentFlags().StringVar(&logFile, "log-file", defaultLogFile, "sets Netbird log path. If console is specified the log will be output to stdout. If syslog is specified the log will be sent to syslog daemon.")
 	rootCmd.PersistentFlags().StringVarP(&setupKey, "setup-key", "k", "", "Setup key obtained from the Management Service Dashboard (used to register peer)")
-	rootCmd.PersistentFlags().StringVar(&setupKeyPath, "setup-key-file", "", "The path to a setup key obtained from the Management Service Dashboard (used to register peer) This is ignored if the setup-key flag is provided.")
-	rootCmd.MarkFlagsMutuallyExclusive("setup-key", "setup-key-file")
 	rootCmd.PersistentFlags().StringVar(&preSharedKey, preSharedKeyFlag, "", "Sets Wireguard PreSharedKey property. If set, then only peers that have the same key can communicate.")
 	rootCmd.PersistentFlags().StringVarP(&hostName, "hostname", "n", "", "Sets a custom hostname for the device")
 	rootCmd.PersistentFlags().BoolVarP(&anonymizeFlag, "anonymize", "A", false, "anonymize IP addresses and non-netbird.io domains in logs and status output")
@@ -256,21 +253,6 @@ var CLIBackOffSettings = &backoff.ExponentialBackOff{
 	Clock:               backoff.SystemClock,
 }
 
-func getSetupKey() (string, error) {
-	if setupKeyPath != "" && setupKey == "" {
-		return getSetupKeyFromFile(setupKeyPath)
-	}
-	return setupKey, nil
-}
-
-func getSetupKeyFromFile(setupKeyPath string) (string, error) {
-	data, err := os.ReadFile(setupKeyPath)
-	if err != nil {
-		return "", fmt.Errorf("failed to read setup key file: %v", err)
-	}
-	return strings.TrimSpace(string(data)), nil
-}
-
 func handleRebrand(cmd *cobra.Command) error {
 	var err error
 	if logFile == defaultLogFile {

client/cmd/up.go

@@ -147,11 +147,6 @@ func runInForegroundMode(ctx context.Context, cmd *cobra.Command) error {
 		ic.DNSRouteInterval = &dnsRouteInterval
 	}
 
-	providedSetupKey, err := getSetupKey()
-	if err != nil {
-		return err
-	}
-
 	config, err := internal.UpdateOrCreateConfig(ic)
 	if err != nil {
 		return fmt.Errorf("get config file: %v", err)
@@ -159,7 +154,7 @@ func runInForegroundMode(ctx context.Context, cmd *cobra.Command) error {
 
 	config, _ = internal.UpdateOldManagementURL(ctx, config, configPath)
 
-	err = foregroundLogin(ctx, cmd, config, providedSetupKey)
+	err = foregroundLogin(ctx, cmd, config, setupKey)
 	if err != nil {
 		return fmt.Errorf("foreground login failed: %v", err)
 	}
@@ -204,13 +199,8 @@ func runInDaemonMode(ctx context.Context, cmd *cobra.Command) error {
 		return nil
 	}
 
-	providedSetupKey, err := getSetupKey()
-	if err != nil {
-		return err
-	}
-
 	loginRequest := proto.LoginRequest{
-		SetupKey:             providedSetupKey,
+		SetupKey:             setupKey,
 		ManagementUrl:        managementURL,
 		AdminURL:             adminURL,
 		NatExternalIPs:       natExternalIPs,

client/cmd/up_daemon_test.go

@@ -2,7 +2,6 @@ package cmd
 
 import (
 	"context"
-	"os"
 	"testing"
 	"time"
 
@@ -41,36 +40,6 @@ func TestUpDaemon(t *testing.T) {
 		return
 	}
 
-	// Test the setup-key-file flag.
-	tempFile, err := os.CreateTemp("", "setup-key")
-	if err != nil {
-		t.Errorf("could not create temp file, got error %v", err)
-		return
-	}
-	defer os.Remove(tempFile.Name())
-	if _, err := tempFile.Write([]byte("A2C8E62B-38F5-4553-B31E-DD66C696CEBB")); err != nil {
-		t.Errorf("could not write to temp file, got error %v", err)
-		return
-	}
-	if err := tempFile.Close(); err != nil {
-		t.Errorf("unable to close file, got error %v", err)
-	}
-	rootCmd.SetArgs([]string{
-		"login",
-		"--daemon-addr", "tcp://" + cliAddr,
-		"--setup-key-file", tempFile.Name(),
-		"--log-file", "",
-	})
-	if err := rootCmd.Execute(); err != nil {
-		t.Errorf("expected no error while running up command, got %v", err)
-		return
-	}
-	time.Sleep(time.Second * 3)
-	if status, err := state.Status(); err != nil && status != internal.StatusIdle {
-		t.Errorf("wrong status after login: %s, %v", internal.StatusIdle, err)
-		return
-	}
-
 	rootCmd.SetArgs([]string{
 		"up",
 		"--daemon-addr", "tcp://" + cliAddr,

client/internal/auth/oauth.go

@@ -86,7 +86,7 @@ func NewOAuthFlow(ctx context.Context, config *internal.Config, isLinuxDesktopCl
 
 // authenticateWithPKCEFlow initializes the Proof Key for Code Exchange flow auth flow
 func authenticateWithPKCEFlow(ctx context.Context, config *internal.Config) (OAuthFlow, error) {
-	pkceFlowInfo, err := internal.GetPKCEAuthorizationFlowInfo(ctx, config.PrivateKey, config.ManagementURL, config.ClientCertKeyPair)
+	pkceFlowInfo, err := internal.GetPKCEAuthorizationFlowInfo(ctx, config.PrivateKey, config.ManagementURL)
 	if err != nil {
 		return nil, fmt.Errorf("getting pkce authorization flow info failed with error: %v", err)
 	}

client/internal/auth/pkce_flow.go

@@ -4,7 +4,6 @@ import (
 	"context"
 	"crypto/sha256"
 	"crypto/subtle"
-	"crypto/tls"
 	"encoding/base64"
 	"errors"
 	"fmt"
@@ -144,18 +143,6 @@ func (p *PKCEAuthorizationFlow) WaitToken(ctx context.Context, _ AuthFlowInfo) (
 func (p *PKCEAuthorizationFlow) startServer(server *http.Server, tokenChan chan<- *oauth2.Token, errChan chan<- error) {
 	mux := http.NewServeMux()
 	mux.HandleFunc("/", func(w http.ResponseWriter, req *http.Request) {
-		cert := p.providerConfig.ClientCertPair
-		if cert != nil {
-			tr := &http.Transport{
-				TLSClientConfig: &tls.Config{
-					Certificates: []tls.Certificate{*cert},
-				},
-			}
-			sslClient := &http.Client{Transport: tr}
-			ctx := context.WithValue(req.Context(), oauth2.HTTPClient, sslClient)
-			req = req.WithContext(ctx)
-		}
-
 		token, err := p.handleRequest(req)
 		if err != nil {
 			renderPKCEFlowTmpl(w, err)

client/internal/config.go

@@ -2,7 +2,6 @@ package internal
 
 import (
 	"context"
-	"crypto/tls"
 	"fmt"
 	"net/url"
 	"os"
@@ -58,8 +57,6 @@ type ConfigInput struct {
 	DisableAutoConnect  *bool
 	ExtraIFaceBlackList []string
 	DNSRouteInterval    *time.Duration
-	ClientCertPath      string
-	ClientCertKeyPath   string
 }
 
 // Config Configuration type
@@ -105,13 +102,6 @@ type Config struct {
 
 	// DNSRouteInterval is the interval in which the DNS routes are updated
 	DNSRouteInterval time.Duration
-	//Path to a certificate used for mTLS authentication
-	ClientCertPath string
-
-	//Path to corresponding private key of ClientCertPath
-	ClientCertKeyPath string
-
-	ClientCertKeyPair *tls.Certificate `json:"-"`
 }
 
 // ReadConfig read config file and return with Config. If it is not exists create a new with default values
@@ -395,26 +385,6 @@ func (config *Config) apply(input ConfigInput) (updated bool, err error) {
 
 	}
 
-	if input.ClientCertKeyPath != "" {
-		config.ClientCertKeyPath = input.ClientCertKeyPath
-		updated = true
-	}
-
-	if input.ClientCertPath != "" {
-		config.ClientCertPath = input.ClientCertPath
-		updated = true
-	}
-
-	if config.ClientCertPath != "" && config.ClientCertKeyPath != "" {
-		cert, err := tls.LoadX509KeyPair(config.ClientCertPath, config.ClientCertKeyPath)
-		if err != nil {
-			log.Error("Failed to load mTLS cert/key pair: ", err)
-		} else {
-			config.ClientCertKeyPair = &cert
-			log.Info("Loaded client mTLS cert/key pair")
-		}
-	}
-
 	return updated, nil
 }
 

client/internal/pkce_auth.go

@@ -2,7 +2,6 @@ package internal
 
 import (
 	"context"
-	"crypto/tls"
 	"fmt"
 	"net/url"
 
@@ -37,12 +36,10 @@ type PKCEAuthProviderConfig struct {
 	RedirectURLs []string
 	// UseIDToken indicates if the id token should be used for authentication
 	UseIDToken bool
-	//ClientCertPair is used for mTLS authentication to the IDP
-	ClientCertPair *tls.Certificate
 }
 
 // GetPKCEAuthorizationFlowInfo initialize a PKCEAuthorizationFlow instance and return with it
-func GetPKCEAuthorizationFlowInfo(ctx context.Context, privateKey string, mgmURL *url.URL, clientCert *tls.Certificate) (PKCEAuthorizationFlow, error) {
+func GetPKCEAuthorizationFlowInfo(ctx context.Context, privateKey string, mgmURL *url.URL) (PKCEAuthorizationFlow, error) {
 	// validate our peer's Wireguard PRIVATE key
 	myPrivateKey, err := wgtypes.ParseKey(privateKey)
 	if err != nil {
@@ -96,7 +93,6 @@ func GetPKCEAuthorizationFlowInfo(ctx context.Context, privateKey string, mgmURL
 			Scope:                 protoPKCEAuthorizationFlow.GetProviderConfig().GetScope(),
 			RedirectURLs:          protoPKCEAuthorizationFlow.GetProviderConfig().GetRedirectURLs(),
 			UseIDToken:            protoPKCEAuthorizationFlow.GetProviderConfig().GetUseIDToken(),
-			ClientCertPair:        clientCert,
 		},
 	}
 

client/ios/NetBirdSDK/login.go

@@ -74,7 +74,7 @@ func (a *Auth) SaveConfigIfSSOSupported() (bool, error) {
 	err := a.withBackOff(a.ctx, func() (err error) {
 		_, err = internal.GetDeviceAuthorizationFlowInfo(a.ctx, a.config.PrivateKey, a.config.ManagementURL)
 		if s, ok := gstatus.FromError(err); ok && (s.Code() == codes.NotFound || s.Code() == codes.Unimplemented) {
-			_, err = internal.GetPKCEAuthorizationFlowInfo(a.ctx, a.config.PrivateKey, a.config.ManagementURL, nil)
+			_, err = internal.GetPKCEAuthorizationFlowInfo(a.ctx, a.config.PrivateKey, a.config.ManagementURL)
 			if s, ok := gstatus.FromError(err); ok && (s.Code() == codes.NotFound || s.Code() == codes.Unimplemented) {
 				supportsSSO = false
 				err = nil

go.mod

@@ -30,7 +30,7 @@ require (
 )
 
 require (
-	fyne.io/fyne/v2 v2.5.0
+	fyne.io/fyne/v2 v2.1.4
 	fyne.io/systray v1.11.0
 	github.com/TheJumpCloud/jcapi-go v3.0.0+incompatible
 	github.com/c-robinson/iplib v1.0.3
@@ -38,7 +38,7 @@ require (
 	github.com/coreos/go-iptables v0.7.0
 	github.com/creack/pty v1.1.18
 	github.com/eko/gocache/v3 v3.1.1
-	github.com/fsnotify/fsnotify v1.7.0
+	github.com/fsnotify/fsnotify v1.6.0
 	github.com/gliderlabs/ssh v0.3.4
 	github.com/godbus/dbus/v5 v5.1.0
 	github.com/golang/mock v1.6.0
@@ -82,7 +82,7 @@ require (
 	go.opentelemetry.io/otel/sdk/metric v1.26.0
 	goauthentik.io/api/v3 v3.2023051.3
 	golang.org/x/exp v0.0.0-20240506185415-9bf2ced13842
-	golang.org/x/mobile v0.0.0-20231127183840-76ac6878050a
+	golang.org/x/mobile v0.0.0-20190719004257-d2bd2a29d028
 	golang.org/x/net v0.26.0
 	golang.org/x/oauth2 v0.19.0
 	golang.org/x/sync v0.7.0
@@ -100,7 +100,7 @@ require (
 	cloud.google.com/go/compute/metadata v0.3.0 // indirect
 	dario.cat/mergo v1.0.0 // indirect
 	github.com/Azure/go-ansiterm v0.0.0-20230124172434-306776ec8161 // indirect
-	github.com/BurntSushi/toml v1.4.0 // indirect
+	github.com/BurntSushi/toml v1.3.2 // indirect
 	github.com/Microsoft/go-winio v0.6.2 // indirect
 	github.com/Microsoft/hcsshim v0.12.3 // indirect
 	github.com/XiaoMi/pegasus-go-client v0.0.0-20210427083443-f3b6b08bc4c2 // indirect
@@ -119,25 +119,20 @@ require (
 	github.com/docker/go-connections v0.5.0 // indirect
 	github.com/docker/go-units v0.5.0 // indirect
 	github.com/felixge/httpsnoop v1.0.4 // indirect
-	github.com/fredbi/uri v1.1.0 // indirect
-	github.com/fyne-io/gl-js v0.0.0-20220119005834-d2da28d9ccfe // indirect
-	github.com/fyne-io/glfw-js v0.0.0-20240101223322-6e1efdc71b7a // indirect
-	github.com/fyne-io/image v0.0.0-20220602074514-4956b0afb3d2 // indirect
-	github.com/go-gl/gl v0.0.0-20211210172815-726fda9656d6 // indirect
-	github.com/go-gl/glfw/v3.3/glfw v0.0.0-20240506104042-037f3cc74f2a // indirect
+	github.com/fredbi/uri v0.0.0-20181227131451-3dcfdacbaaf3 // indirect
+	github.com/go-gl/gl v0.0.0-20210813123233-e4099ee2221f // indirect
+	github.com/go-gl/glfw/v3.3/glfw v0.0.0-20211024062804-40e447a793be // indirect
 	github.com/go-logr/logr v1.4.1 // indirect
 	github.com/go-logr/stdr v1.2.2 // indirect
 	github.com/go-ole/go-ole v1.3.0 // indirect
 	github.com/go-redis/redis/v8 v8.11.5 // indirect
-	github.com/go-text/render v0.1.1-0.20240418202334-dd62631dae9b // indirect
-	github.com/go-text/typesetting v0.1.0 // indirect
 	github.com/gogo/protobuf v1.3.2 // indirect
+	github.com/goki/freetype v0.0.0-20181231101311-fa8a33aabaff // indirect
 	github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect
 	github.com/google/btree v1.0.1 // indirect
 	github.com/google/s2a-go v0.1.7 // indirect
 	github.com/googleapis/enterprise-certificate-proxy v0.3.2 // indirect
 	github.com/googleapis/gax-go/v2 v2.12.3 // indirect
-	github.com/gopherjs/gopherjs v1.17.2 // indirect
 	github.com/hashicorp/errwrap v1.1.0 // indirect
 	github.com/hashicorp/go-uuid v1.0.2 // indirect
 	github.com/inconshreveable/mousetrap v1.1.0 // indirect
@@ -145,11 +140,9 @@ require (
 	github.com/jackc/pgservicefile v0.0.0-20221227161230-091c0ba34f0a // indirect
 	github.com/jackc/pgx/v5 v5.5.5 // indirect
 	github.com/jackc/puddle/v2 v2.2.1 // indirect
-	github.com/jeandeaual/go-locale v0.0.0-20240223122105-ce5225dcaa49 // indirect
 	github.com/jinzhu/inflection v1.0.0 // indirect
 	github.com/jinzhu/now v1.1.5 // indirect
 	github.com/josharian/native v1.1.0 // indirect
-	github.com/jsummers/gobmp v0.0.0-20151104160322-e2ba15ffa76e // indirect
 	github.com/kelseyhightower/envconfig v1.4.0 // indirect
 	github.com/klauspost/compress v1.17.8 // indirect
 	github.com/lufia/plan9stats v0.0.0-20240513124658-fba389f38bae // indirect
@@ -161,7 +154,6 @@ require (
 	github.com/moby/sys/user v0.1.0 // indirect
 	github.com/moby/term v0.5.0 // indirect
 	github.com/morikuni/aec v1.0.0 // indirect
-	github.com/nicksnyder/go-i18n/v2 v2.4.0 // indirect
 	github.com/nxadm/tail v1.4.8 // indirect
 	github.com/opencontainers/go-digest v1.0.0 // indirect
 	github.com/opencontainers/image-spec v1.1.0 // indirect
@@ -176,24 +168,21 @@ require (
 	github.com/prometheus/client_model v0.6.1 // indirect
 	github.com/prometheus/common v0.53.0 // indirect
 	github.com/prometheus/procfs v0.15.0 // indirect
-	github.com/rymdport/portal v0.2.6 // indirect
 	github.com/shoenig/go-m1cpu v0.1.6 // indirect
 	github.com/spf13/cast v1.5.0 // indirect
-	github.com/srwiley/oksvg v0.0.0-20221011165216-be6e8873101c // indirect
-	github.com/srwiley/rasterx v0.0.0-20220730225603-2ab79fcdd4ef // indirect
+	github.com/srwiley/oksvg v0.0.0-20200311192757-870daf9aa564 // indirect
+	github.com/srwiley/rasterx v0.0.0-20200120212402-85cb7272f5e9 // indirect
 	github.com/tklauser/go-sysconf v0.3.14 // indirect
 	github.com/tklauser/numcpus v0.8.0 // indirect
 	github.com/vishvananda/netns v0.0.4 // indirect
-	github.com/yuin/goldmark v1.7.1 // indirect
+	github.com/yuin/goldmark v1.4.13 // indirect
 	go.opencensus.io v0.24.0 // indirect
 	go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.51.0 // indirect
 	go.opentelemetry.io/otel/sdk v1.26.0 // indirect
 	go.opentelemetry.io/otel/trace v1.26.0 // indirect
 	golang.org/x/image v0.18.0 // indirect
-	golang.org/x/mod v0.17.0 // indirect
 	golang.org/x/text v0.16.0 // indirect
 	golang.org/x/time v0.5.0 // indirect
-	golang.org/x/tools v0.21.1-0.20240508182429-e35e4ccd0d2d // indirect
 	golang.zx2c4.com/wintun v0.0.0-20230126152724-0fa3db229ce2 // indirect
 	google.golang.org/genproto/googleapis/api v0.0.0-20240509183442-62759503f434 // indirect
 	google.golang.org/genproto/googleapis/rpc v0.0.0-20240515191416-fc5f0ca64291 // indirect
@@ -215,5 +204,3 @@ replace github.com/cloudflare/circl => github.com/cunicu/circl v0.0.0-2023080111
 replace github.com/pion/ice/v3 => github.com/netbirdio/ice/v3 v3.0.0-20240315174635-e72a50fcb64e
 
 replace github.com/libp2p/go-netroute => github.com/netbirdio/go-netroute v0.0.0-20240611143515-f59b0e1d3944
-
-replace fyne.io/fyne/v2 => github.com/Jacalz/fyne/v2 v2.0.0-20240809153104-a1e60718db70

management/server/account.go

@@ -68,7 +68,6 @@ type AccountManager interface {
 	SaveSetupKey(ctx context.Context, accountID string, key *SetupKey, userID string) (*SetupKey, error)
 	CreateUser(ctx context.Context, accountID, initiatorUserID string, key *UserInfo) (*UserInfo, error)
 	DeleteUser(ctx context.Context, accountID, initiatorUserID string, targetUserID string) error
-	DeleteRegularUsers(ctx context.Context, accountID, initiatorUserID string, targetUserIDs []string) error
 	InviteUser(ctx context.Context, accountID string, initiatorUserID string, targetUserID string) error
 	ListSetupKeys(ctx context.Context, accountID, userID string) ([]*SetupKey, error)
 	SaveUser(ctx context.Context, accountID, initiatorUserID string, update *User) (*UserInfo, error)
@@ -102,7 +101,6 @@ type AccountManager interface {
 	SaveGroup(ctx context.Context, accountID, userID string, group *nbgroup.Group) error
 	SaveGroups(ctx context.Context, accountID, userID string, newGroups []*nbgroup.Group) error
 	DeleteGroup(ctx context.Context, accountId, userId, groupID string) error
-	DeleteGroups(ctx context.Context, accountId, userId string, groupIDs []string) error
 	ListGroups(ctx context.Context, accountId string) ([]*nbgroup.Group, error)
 	GroupAddPeer(ctx context.Context, accountId, groupID, peerID string) error
 	GroupDeletePeer(ctx context.Context, accountId, groupID, peerID string) error
@@ -415,6 +413,7 @@ func (a *Account) GetPeerNetworkMap(
 	peersCustomZone nbdns.CustomZone,
 	validatedPeersMap map[string]struct{},
 	metrics *telemetry.AccountManagerMetrics,
+	expandedPolicies PolicyRuleExpandedPeers,
 ) *NetworkMap {
 	start := time.Now()
 
@@ -431,7 +430,7 @@ func (a *Account) GetPeerNetworkMap(
 		}
 	}
 
-	aclPeers, firewallRules := a.getPeerConnectionResources(ctx, peerID, validatedPeersMap)
+	aclPeers, firewallRules := a.getPeerConnectionResources(ctx, peerID, validatedPeersMap, expandedPolicies)
 	// exclude expired peers
 	var peersToConnect []*nbpeer.Peer
 	var expiredPeers []*nbpeer.Peer
@@ -922,7 +921,7 @@ func (a *Account) UserGroupsAddToPeers(userID string, groups ...string) {
 func (a *Account) UserGroupsRemoveFromPeers(userID string, groups ...string) {
 	for _, gid := range groups {
 		group, ok := a.Groups[gid]
-		if !ok || group.Name == "All" {
+		if !ok {
 			continue
 		}
 		update := make([]string, 0, len(group.Peers))

management/server/account_test.go

@@ -412,7 +412,8 @@ func TestAccount_GetPeerNetworkMap(t *testing.T) {
 		}
 
 		customZone := account.GetPeersCustomZone(context.Background(), "netbird.io")
-		networkMap := account.GetPeerNetworkMap(context.Background(), testCase.peerID, customZone, validatedPeers, nil)
+		policyExpandedPeers := account.GetPolicyExpandedPeers()
+		networkMap := account.GetPeerNetworkMap(context.Background(), testCase.peerID, customZone, validatedPeers, nil, policyExpandedPeers)
 		assert.Len(t, networkMap.Peers, len(testCase.expectedPeers))
 		assert.Len(t, networkMap.OfflinePeers, len(testCase.expectedOfflinePeers))
 	}

management/server/group.go

@@ -2,12 +2,8 @@ package server
 
 import (
 	"context"
-	"errors"
 	"fmt"
-	"slices"
 
-	nbdns "github.com/netbirdio/netbird/dns"
-	"github.com/netbirdio/netbird/route"
 	"github.com/rs/xid"
 	log "github.com/sirupsen/logrus"
 
@@ -247,7 +243,7 @@ func difference(a, b []string) []string {
 	return diff
 }
 
-// DeleteGroup object of the peers.
+// DeleteGroup object of the peers
 func (am *DefaultAccountManager) DeleteGroup(ctx context.Context, accountId, userId, groupID string) error {
 	unlock := am.Store.AcquireWriteLockByUID(ctx, accountId)
 	defer unlock()
@@ -257,14 +253,96 @@ func (am *DefaultAccountManager) DeleteGroup(ctx context.Context, accountId, use
 		return err
 	}
 
-	group, ok := account.Groups[groupID]
+	g, ok := account.Groups[groupID]
 	if !ok {
 		return nil
 	}
 
-	if err = validateDeleteGroup(account, group, userId); err != nil {
-		return err
+	// disable a deleting integration group if the initiator is not an admin service user
+	if g.Issued == nbgroup.GroupIssuedIntegration {
+		executingUser := account.Users[userId]
+		if executingUser == nil {
+			return status.Errorf(status.NotFound, "user not found")
+		}
+		if executingUser.Role != UserRoleAdmin || !executingUser.IsServiceUser {
+			return status.Errorf(status.PermissionDenied, "only service users with admin power can delete integration group")
+		}
 	}
+
+	// check route links
+	for _, r := range account.Routes {
+		for _, g := range r.Groups {
+			if g == groupID {
+				return &GroupLinkError{"route", string(r.NetID)}
+			}
+		}
+		for _, g := range r.PeerGroups {
+			if g == groupID {
+				return &GroupLinkError{"route", string(r.NetID)}
+			}
+		}
+	}
+
+	// check DNS links
+	for _, dns := range account.NameServerGroups {
+		for _, g := range dns.Groups {
+			if g == groupID {
+				return &GroupLinkError{"name server groups", dns.Name}
+			}
+		}
+	}
+
+	// check ACL links
+	for _, policy := range account.Policies {
+		for _, rule := range policy.Rules {
+			for _, src := range rule.Sources {
+				if src == groupID {
+					return &GroupLinkError{"policy", policy.Name}
+				}
+			}
+
+			for _, dst := range rule.Destinations {
+				if dst == groupID {
+					return &GroupLinkError{"policy", policy.Name}
+				}
+			}
+		}
+	}
+
+	// check setup key links
+	for _, setupKey := range account.SetupKeys {
+		for _, grp := range setupKey.AutoGroups {
+			if grp == groupID {
+				return &GroupLinkError{"setup key", setupKey.Name}
+			}
+		}
+	}
+
+	// check user links
+	for _, user := range account.Users {
+		for _, grp := range user.AutoGroups {
+			if grp == groupID {
+				return &GroupLinkError{"user", user.Id}
+			}
+		}
+	}
+
+	// check DisabledManagementGroups
+	for _, disabledMgmGrp := range account.DNSSettings.DisabledManagementGroups {
+		if disabledMgmGrp == groupID {
+			return &GroupLinkError{"disabled DNS management groups", g.Name}
+		}
+	}
+
+	// check integrated peer validator groups
+	if account.Settings.Extra != nil {
+		for _, integratedPeerValidatorGroups := range account.Settings.Extra.IntegratedValidatorGroups {
+			if groupID == integratedPeerValidatorGroups {
+				return &GroupLinkError{"integrated validator", g.Name}
+			}
+		}
+	}
+
 	delete(account.Groups, groupID)
 
 	account.Network.IncSerial()
@@ -272,57 +350,13 @@ func (am *DefaultAccountManager) DeleteGroup(ctx context.Context, accountId, use
 		return err
 	}
 
-	am.StoreEvent(ctx, userId, groupID, accountId, activity.GroupDeleted, group.EventMeta())
+	am.StoreEvent(ctx, userId, groupID, accountId, activity.GroupDeleted, g.EventMeta())
 
 	am.updateAccountPeers(ctx, account)
 
 	return nil
 }
 
-// DeleteGroups deletes groups from an account.
-// Note: This function does not acquire the global lock.
-// It is the caller's responsibility to ensure proper locking is in place before invoking this method.
-//
-// If an error occurs while deleting a group, the function skips it and continues deleting other groups.
-// Errors are collected and returned at the end.
-func (am *DefaultAccountManager) DeleteGroups(ctx context.Context, accountId, userId string, groupIDs []string) error {
-	account, err := am.Store.GetAccount(ctx, accountId)
-	if err != nil {
-		return err
-	}
-
-	var allErrors error
-
-	deletedGroups := make([]*nbgroup.Group, 0, len(groupIDs))
-	for _, groupID := range groupIDs {
-		group, ok := account.Groups[groupID]
-		if !ok {
-			continue
-		}
-
-		if err := validateDeleteGroup(account, group, userId); err != nil {
-			allErrors = errors.Join(allErrors, fmt.Errorf("failed to delete group %s: %w", groupID, err))
-			continue
-		}
-
-		delete(account.Groups, groupID)
-		deletedGroups = append(deletedGroups, group)
-	}
-
-	account.Network.IncSerial()
-	if err = am.Store.SaveAccount(ctx, account); err != nil {
-		return err
-	}
-
-	for _, g := range deletedGroups {
-		am.StoreEvent(ctx, userId, g.ID, accountId, activity.GroupDeleted, g.EventMeta())
-	}
-
-	am.updateAccountPeers(ctx, account)
-
-	return allErrors
-}
-
 // ListGroups objects of the peers
 func (am *DefaultAccountManager) ListGroups(ctx context.Context, accountID string) ([]*nbgroup.Group, error) {
 	unlock := am.Store.AcquireWriteLockByUID(ctx, accountID)
@@ -406,102 +440,3 @@ func (am *DefaultAccountManager) GroupDeletePeer(ctx context.Context, accountID,
 
 	return nil
 }
-
-func validateDeleteGroup(account *Account, group *nbgroup.Group, userID string) error {
-	// disable a deleting integration group if the initiator is not an admin service user
-	if group.Issued == nbgroup.GroupIssuedIntegration {
-		executingUser := account.Users[userID]
-		if executingUser == nil {
-			return status.Errorf(status.NotFound, "user not found")
-		}
-		if executingUser.Role != UserRoleAdmin || !executingUser.IsServiceUser {
-			return status.Errorf(status.PermissionDenied, "only service users with admin power can delete integration group")
-		}
-	}
-
-	if isLinked, linkedRoute := isGroupLinkedToRoute(account.Routes, group.ID); isLinked {
-		return &GroupLinkError{"route", string(linkedRoute.NetID)}
-	}
-
-	if isLinked, linkedDns := isGroupLinkedToDns(account.NameServerGroups, group.ID); isLinked {
-		return &GroupLinkError{"name server groups", linkedDns.Name}
-	}
-
-	if isLinked, linkedPolicy := isGroupLinkedToPolicy(account.Policies, group.ID); isLinked {
-		return &GroupLinkError{"policy", linkedPolicy.Name}
-	}
-
-	if isLinked, linkedSetupKey := isGroupLinkedToSetupKey(account.SetupKeys, group.ID); isLinked {
-		return &GroupLinkError{"setup key", linkedSetupKey.Name}
-	}
-
-	if isLinked, linkedUser := isGroupLinkedToUser(account.Users, group.ID); isLinked {
-		return &GroupLinkError{"user", linkedUser.Id}
-	}
-
-	if slices.Contains(account.DNSSettings.DisabledManagementGroups, group.ID) {
-		return &GroupLinkError{"disabled DNS management groups", group.Name}
-	}
-
-	if account.Settings.Extra != nil {
-		if slices.Contains(account.Settings.Extra.IntegratedValidatorGroups, group.ID) {
-			return &GroupLinkError{"integrated validator", group.Name}
-		}
-	}
-
-	return nil
-}
-
-// isGroupLinkedToRoute checks if a group is linked to any route in the account.
-func isGroupLinkedToRoute(routes map[route.ID]*route.Route, groupID string) (bool, *route.Route) {
-	for _, r := range routes {
-		if slices.Contains(r.Groups, groupID) || slices.Contains(r.PeerGroups, groupID) {
-			return true, r
-		}
-	}
-	return false, nil
-}
-
-// isGroupLinkedToPolicy checks if a group is linked to any policy in the account.
-func isGroupLinkedToPolicy(policies []*Policy, groupID string) (bool, *Policy) {
-	for _, policy := range policies {
-		for _, rule := range policy.Rules {
-			if slices.Contains(rule.Sources, groupID) || slices.Contains(rule.Destinations, groupID) {
-				return true, policy
-			}
-		}
-	}
-	return false, nil
-}
-
-// isGroupLinkedToDns checks if a group is linked to any nameserver group in the account.
-func isGroupLinkedToDns(nameServerGroups map[string]*nbdns.NameServerGroup, groupID string) (bool, *nbdns.NameServerGroup) {
-	for _, dns := range nameServerGroups {
-		for _, g := range dns.Groups {
-			if g == groupID {
-				return true, dns
-			}
-		}
-	}
-	return false, nil
-}
-
-// isGroupLinkedToSetupKey checks if a group is linked to any setup key in the account.
-func isGroupLinkedToSetupKey(setupKeys map[string]*SetupKey, groupID string) (bool, *SetupKey) {
-	for _, setupKey := range setupKeys {
-		if slices.Contains(setupKey.AutoGroups, groupID) {
-			return true, setupKey
-		}
-	}
-	return false, nil
-}
-
-// isGroupLinkedToUser checks if a group is linked to any user in the account.
-func isGroupLinkedToUser(users map[string]*User, groupID string) (bool, *User) {
-	for _, user := range users {
-		if slices.Contains(user.AutoGroups, groupID) {
-			return true, user
-		}
-	}
-	return false, nil
-}

management/server/group_test.go

@@ -3,14 +3,12 @@ package server
 import (
 	"context"
 	"errors"
-	"fmt"
 	"testing"
 
 	nbdns "github.com/netbirdio/netbird/dns"
 	nbgroup "github.com/netbirdio/netbird/management/server/group"
 	"github.com/netbirdio/netbird/management/server/status"
 	"github.com/netbirdio/netbird/route"
-	"github.com/stretchr/testify/assert"
 )
 
 const (
@@ -23,7 +21,7 @@ func TestDefaultAccountManager_CreateGroup(t *testing.T) {
 		t.Error("failed to create account manager")
 	}
 
-	_, account, err := initTestGroupAccount(am)
+	account, err := initTestGroupAccount(am)
 	if err != nil {
 		t.Error("failed to init testing account")
 	}
@@ -58,7 +56,7 @@ func TestDefaultAccountManager_DeleteGroup(t *testing.T) {
 		t.Error("failed to create account manager")
 	}
 
-	_, account, err := initTestGroupAccount(am)
+	account, err := initTestGroupAccount(am)
 	if err != nil {
 		t.Error("failed to init testing account")
 	}
@@ -134,136 +132,7 @@ func TestDefaultAccountManager_DeleteGroup(t *testing.T) {
 	}
 }
 
-func TestDefaultAccountManager_DeleteGroups(t *testing.T) {
-	am, err := createManager(t)
-	assert.NoError(t, err, "Failed to create account manager")
-
-	manager, account, err := initTestGroupAccount(am)
-	assert.NoError(t, err, "Failed to init testing account")
-
-	groups := make([]*nbgroup.Group, 10)
-	for i := 0; i < 10; i++ {
-		groups[i] = &nbgroup.Group{
-			ID:        fmt.Sprintf("group-%d", i+1),
-			AccountID: account.Id,
-			Name:      fmt.Sprintf("group-%d", i+1),
-			Issued:    nbgroup.GroupIssuedAPI,
-		}
-	}
-
-	err = manager.SaveGroups(context.Background(), account.Id, groupAdminUserID, groups)
-	assert.NoError(t, err, "Failed to save test groups")
-
-	testCases := []struct {
-		name               string
-		groupIDs           []string
-		expectedReasons    []string
-		expectedDeleted    []string
-		expectedNotDeleted []string
-	}{
-		{
-			name:            "route",
-			groupIDs:        []string{"grp-for-route"},
-			expectedReasons: []string{"route"},
-		},
-		{
-			name:            "route with peer groups",
-			groupIDs:        []string{"grp-for-route2"},
-			expectedReasons: []string{"route"},
-		},
-		{
-			name:            "name server groups",
-			groupIDs:        []string{"grp-for-name-server-grp"},
-			expectedReasons: []string{"name server groups"},
-		},
-		{
-			name:            "policy",
-			groupIDs:        []string{"grp-for-policies"},
-			expectedReasons: []string{"policy"},
-		},
-		{
-			name:            "setup keys",
-			groupIDs:        []string{"grp-for-keys"},
-			expectedReasons: []string{"setup key"},
-		},
-		{
-			name:            "users",
-			groupIDs:        []string{"grp-for-users"},
-			expectedReasons: []string{"user"},
-		},
-		{
-			name:            "integration",
-			groupIDs:        []string{"grp-for-integration"},
-			expectedReasons: []string{"only service users with admin power can delete integration group"},
-		},
-		{
-			name:            "successfully delete multiple groups",
-			groupIDs:        []string{"group-1", "group-2"},
-			expectedDeleted: []string{"group-1", "group-2"},
-		},
-		{
-			name:            "delete non-existent group",
-			groupIDs:        []string{"non-existent-group"},
-			expectedDeleted: []string{"non-existent-group"},
-		},
-		{
-			name:               "delete multiple groups with mixed results",
-			groupIDs:           []string{"group-3", "grp-for-policies", "group-4", "grp-for-users"},
-			expectedReasons:    []string{"policy", "user"},
-			expectedDeleted:    []string{"group-3", "group-4"},
-			expectedNotDeleted: []string{"grp-for-policies", "grp-for-users"},
-		},
-		{
-			name:               "delete groups with multiple errors",
-			groupIDs:           []string{"grp-for-policies", "grp-for-users"},
-			expectedReasons:    []string{"policy", "user"},
-			expectedNotDeleted: []string{"grp-for-policies", "grp-for-users"},
-		},
-	}
-
-	for _, tc := range testCases {
-		t.Run(tc.name, func(t *testing.T) {
-			err = am.DeleteGroups(context.Background(), account.Id, groupAdminUserID, tc.groupIDs)
-			if len(tc.expectedReasons) > 0 {
-				assert.Error(t, err)
-				var foundExpectedErrors int
-
-				wrappedErr, ok := err.(interface{ Unwrap() []error })
-				assert.Equal(t, ok, true)
-
-				for _, e := range wrappedErr.Unwrap() {
-					var sErr *status.Error
-					if errors.As(e, &sErr) {
-						assert.Contains(t, tc.expectedReasons, sErr.Message, "unexpected error message")
-						foundExpectedErrors++
-					}
-
-					var gErr *GroupLinkError
-					if errors.As(e, &gErr) {
-						assert.Contains(t, tc.expectedReasons, gErr.Resource, "unexpected error resource")
-						foundExpectedErrors++
-					}
-				}
-				assert.Equal(t, len(tc.expectedReasons), foundExpectedErrors, "not all expected errors were found")
-			} else {
-				assert.NoError(t, err)
-			}
-
-			for _, groupID := range tc.expectedDeleted {
-				_, err := am.GetGroup(context.Background(), account.Id, groupID, groupAdminUserID)
-				assert.Error(t, err, "group should have been deleted: %s", groupID)
-			}
-
-			for _, groupID := range tc.expectedNotDeleted {
-				group, err := am.GetGroup(context.Background(), account.Id, groupID, groupAdminUserID)
-				assert.NoError(t, err, "group should not have been deleted: %s", groupID)
-				assert.NotNil(t, group, "group should exist: %s", groupID)
-			}
-		})
-	}
-}
-
-func initTestGroupAccount(am *DefaultAccountManager) (*DefaultAccountManager, *Account, error) {
+func initTestGroupAccount(am *DefaultAccountManager) (*Account, error) {
 	accountID := "testingAcc"
 	domain := "example.com"
 
@@ -367,7 +236,7 @@ func initTestGroupAccount(am *DefaultAccountManager) (*DefaultAccountManager, *A
 
 	err := am.Store.SaveAccount(context.Background(), account)
 	if err != nil {
-		return nil, nil, err
+		return nil, err
 	}
 
 	_ = am.SaveGroup(context.Background(), accountID, groupAdminUserID, groupForRoute)
@@ -378,9 +247,5 @@ func initTestGroupAccount(am *DefaultAccountManager) (*DefaultAccountManager, *A
 	_ = am.SaveGroup(context.Background(), accountID, groupAdminUserID, groupForUsers)
 	_ = am.SaveGroup(context.Background(), accountID, groupAdminUserID, groupForIntegration)
 
-	acc, err := am.Store.GetAccount(context.Background(), account.Id)
-	if err != nil {
-		return nil, nil, err
-	}
-	return am, acc, nil
+	return am.Store.GetAccount(context.Background(), account.Id)
 }

management/server/http/peers_handler.go

@@ -64,19 +64,28 @@ func (h *PeersHandler) getPeer(ctx context.Context, account *server.Account, pee
 
 	groupsInfo := toGroupsInfo(account.Groups, peer.ID)
 
-	validPeers, err := h.accountManager.GetValidatedPeers(account)
+	accessiblePeers, valid, err := h.getAccessibleAndValidStatus(ctx, account, peerID, dnsDomain, peer)
 	if err != nil {
-		log.WithContext(ctx).Errorf("failed to list appreoved peers: %v", err)
+		log.WithContext(ctx).Errorf("failed to list approved peers: %v", err)
 		util.WriteError(ctx, fmt.Errorf("internal error"), w)
 		return
 	}
+	util.WriteJSONObject(ctx, w, toSinglePeerResponse(peerToReturn, groupsInfo, dnsDomain, accessiblePeers, valid))
+}
+
+func (h *PeersHandler) getAccessibleAndValidStatus(ctx context.Context, account *server.Account, peerID string, dnsDomain string, peer *nbpeer.Peer) ([]api.AccessiblePeer, bool, error) {
+	validPeers, err := h.accountManager.GetValidatedPeers(account)
+	if err != nil {
+		return nil, false, err
+	}
 
 	customZone := account.GetPeersCustomZone(ctx, h.accountManager.GetDNSDomain())
-	netMap := account.GetPeerNetworkMap(ctx, peerID, customZone, validPeers, nil)
+	policyExpandedPeers := account.GetPolicyExpandedPeers()
+	netMap := account.GetPeerNetworkMap(ctx, peerID, customZone, validPeers, nil, policyExpandedPeers)
 	accessiblePeers := toAccessiblePeers(netMap, dnsDomain)
 
 	_, valid := validPeers[peer.ID]
-	util.WriteJSONObject(ctx, w, toSinglePeerResponse(peerToReturn, groupsInfo, dnsDomain, accessiblePeers, valid))
+	return accessiblePeers, valid, nil
 }
 
 func (h *PeersHandler) updatePeer(ctx context.Context, account *server.Account, user *server.User, peerID string, w http.ResponseWriter, r *http.Request) {
@@ -110,19 +119,13 @@ func (h *PeersHandler) updatePeer(ctx context.Context, account *server.Account,
 
 	groupMinimumInfo := toGroupsInfo(account.Groups, peer.ID)
 
-	validPeers, err := h.accountManager.GetValidatedPeers(account)
+	accessiblePeers, valid, err := h.getAccessibleAndValidStatus(ctx, account, peerID, dnsDomain, peer)
 	if err != nil {
-		log.WithContext(ctx).Errorf("failed to list appreoved peers: %v", err)
+		log.WithContext(ctx).Errorf("failed to list approved peers: %v", err)
 		util.WriteError(ctx, fmt.Errorf("internal error"), w)
 		return
 	}
 
-	customZone := account.GetPeersCustomZone(ctx, h.accountManager.GetDNSDomain())
-	netMap := account.GetPeerNetworkMap(ctx, peerID, customZone, validPeers, nil)
-	accessiblePeers := toAccessiblePeers(netMap, dnsDomain)
-
-	_, valid := validPeers[peer.ID]
-
 	util.WriteJSONObject(r.Context(), w, toSinglePeerResponse(peer, groupMinimumInfo, dnsDomain, accessiblePeers, valid))
 }
 

management/server/mock_server/account_mock.go

@@ -42,7 +42,6 @@ type MockAccountManager struct {
 	SaveGroupFunc                       func(ctx context.Context, accountID, userID string, group *group.Group) error
 	SaveGroupsFunc                      func(ctx context.Context, accountID, userID string, groups []*group.Group) error
 	DeleteGroupFunc                     func(ctx context.Context, accountID, userId, groupID string) error
-	DeleteGroupsFunc                    func(ctx context.Context, accountId, userId string, groupIDs []string) error
 	ListGroupsFunc                      func(ctx context.Context, accountID string) ([]*group.Group, error)
 	GroupAddPeerFunc                    func(ctx context.Context, accountID, groupID, peerID string) error
 	GroupDeletePeerFunc                 func(ctx context.Context, accountID, groupID, peerID string) error
@@ -68,7 +67,6 @@ type MockAccountManager struct {
 	SaveOrAddUserFunc                   func(ctx context.Context, accountID, userID string, user *server.User, addIfNotExists bool) (*server.UserInfo, error)
 	SaveOrAddUsersFunc                  func(ctx context.Context, accountID, initiatorUserID string, update []*server.User, addIfNotExists bool) ([]*server.UserInfo, error)
 	DeleteUserFunc                      func(ctx context.Context, accountID string, initiatorUserID string, targetUserID string) error
-	DeleteRegularUsersFunc              func(ctx context.Context, accountID, initiatorUserID string, targetUserIDs []string) error
 	CreatePATFunc                       func(ctx context.Context, accountID string, initiatorUserID string, targetUserId string, tokenName string, expiresIn int) (*server.PersonalAccessTokenGenerated, error)
 	DeletePATFunc                       func(ctx context.Context, accountID string, initiatorUserID string, targetUserId string, tokenID string) error
 	GetPATFunc                          func(ctx context.Context, accountID string, initiatorUserID string, targetUserId string, tokenID string) (*server.PersonalAccessToken, error)
@@ -328,14 +326,6 @@ func (am *MockAccountManager) DeleteGroup(ctx context.Context, accountId, userId
 	return status.Errorf(codes.Unimplemented, "method DeleteGroup is not implemented")
 }
 
-// DeleteGroups mock implementation of DeleteGroups from server.AccountManager interface
-func (am *MockAccountManager) DeleteGroups(ctx context.Context, accountId, userId string, groupIDs []string) error {
-	if am.DeleteGroupsFunc != nil {
-		return am.DeleteGroupsFunc(ctx, accountId, userId, groupIDs)
-	}
-	return status.Errorf(codes.Unimplemented, "method DeleteGroups is not implemented")
-}
-
 // ListGroups mock implementation of ListGroups from server.AccountManager interface
 func (am *MockAccountManager) ListGroups(ctx context.Context, accountID string) ([]*group.Group, error) {
 	if am.ListGroupsFunc != nil {
@@ -538,14 +528,6 @@ func (am *MockAccountManager) DeleteUser(ctx context.Context, accountID string,
 	return status.Errorf(codes.Unimplemented, "method DeleteUser is not implemented")
 }
 
-// DeleteRegularUsers mocks DeleteRegularUsers of the AccountManager interface
-func (am *MockAccountManager) DeleteRegularUsers(ctx context.Context, accountID string, initiatorUserID string, targetUserIDs []string) error {
-	if am.DeleteRegularUsersFunc != nil {
-		return am.DeleteRegularUsersFunc(ctx, accountID, initiatorUserID, targetUserIDs)
-	}
-	return status.Errorf(codes.Unimplemented, "method DeleteRegularUsers is not implemented")
-}
-
 func (am *MockAccountManager) InviteUser(ctx context.Context, accountID string, initiatorUserID string, targetUserID string) error {
 	if am.InviteUserFunc != nil {
 		return am.InviteUserFunc(ctx, accountID, initiatorUserID, targetUserID)

management/server/peer.go

@@ -87,8 +87,9 @@ func (am *DefaultAccountManager) GetPeers(ctx context.Context, accountID, userID
 	}
 
 	// fetch all the peers that have access to the user's peers
+	policyExpandedPeers := account.GetPolicyExpandedPeers()
 	for _, peer := range peers {
-		aclPeers, _ := account.getPeerConnectionResources(ctx, peer.ID, approvedPeersMap)
+		aclPeers, _ := account.getPeerConnectionResources(ctx, peer.ID, approvedPeersMap, policyExpandedPeers)
 		for _, p := range aclPeers {
 			peersMap[p.ID] = p
 		}
@@ -324,7 +325,8 @@ func (am *DefaultAccountManager) GetNetworkMap(ctx context.Context, peerID strin
 		return nil, err
 	}
 	customZone := account.GetPeersCustomZone(ctx, am.dnsDomain)
-	return account.GetPeerNetworkMap(ctx, peer.ID, customZone, validatedPeers, nil), nil
+	policyExpandedPeers := account.GetPolicyExpandedPeers()
+	return account.GetPeerNetworkMap(ctx, peer.ID, customZone, validatedPeers, nil, policyExpandedPeers), nil
 }
 
 // GetPeerNetwork returns the Network for a given peer
@@ -538,7 +540,8 @@ func (am *DefaultAccountManager) AddPeer(ctx context.Context, setupKey, userID s
 
 	postureChecks := am.getPeerPostureChecks(account, peer)
 	customZone := account.GetPeersCustomZone(ctx, am.dnsDomain)
-	networkMap := account.GetPeerNetworkMap(ctx, newPeer.ID, customZone, approvedPeersMap, am.metrics.AccountManagerMetrics())
+	policyExpandedPeers := account.GetPolicyExpandedPeers()
+	networkMap := account.GetPeerNetworkMap(ctx, newPeer.ID, customZone, approvedPeersMap, am.metrics.AccountManagerMetrics(), policyExpandedPeers)
 	return newPeer, networkMap, postureChecks, nil
 }
 
@@ -595,7 +598,8 @@ func (am *DefaultAccountManager) SyncPeer(ctx context.Context, sync PeerSync, ac
 	postureChecks = am.getPeerPostureChecks(account, peer)
 
 	customZone := account.GetPeersCustomZone(ctx, am.dnsDomain)
-	return peer, account.GetPeerNetworkMap(ctx, peer.ID, customZone, validPeersMap, am.metrics.AccountManagerMetrics()), postureChecks, nil
+	policyExpandedPeers := account.GetPolicyExpandedPeers()
+	return peer, account.GetPeerNetworkMap(ctx, peer.ID, customZone, validPeersMap, am.metrics.AccountManagerMetrics(), policyExpandedPeers), postureChecks, nil
 }
 
 // LoginPeer logs in or registers a peer.
@@ -743,7 +747,8 @@ func (am *DefaultAccountManager) getValidatedPeerWithMap(ctx context.Context, is
 	postureChecks = am.getPeerPostureChecks(account, peer)
 
 	customZone := account.GetPeersCustomZone(ctx, am.dnsDomain)
-	return peer, account.GetPeerNetworkMap(ctx, peer.ID, customZone, approvedPeersMap, am.metrics.AccountManagerMetrics()), postureChecks, nil
+	policyExpandedPeers := account.GetPolicyExpandedPeers()
+	return peer, account.GetPeerNetworkMap(ctx, peer.ID, customZone, approvedPeersMap, am.metrics.AccountManagerMetrics(), policyExpandedPeers), postureChecks, nil
 }
 
 func (am *DefaultAccountManager) handleExpiredPeer(ctx context.Context, login PeerLogin, account *Account, peer *nbpeer.Peer) error {
@@ -896,8 +901,9 @@ func (am *DefaultAccountManager) GetPeer(ctx context.Context, accountID, peerID,
 		return nil, err
 	}
 
+	policyExpandedPeers := account.GetPolicyExpandedPeers()
 	for _, p := range userPeers {
-		aclPeers, _ := account.getPeerConnectionResources(ctx, p.ID, approvedPeersMap)
+		aclPeers, _ := account.getPeerConnectionResources(ctx, p.ID, approvedPeersMap, policyExpandedPeers)
 		for _, aclPeer := range aclPeers {
 			if aclPeer.ID == peerID {
 				return peer, nil
@@ -939,7 +945,7 @@ func (am *DefaultAccountManager) updateAccountPeers(ctx context.Context, account
 
 	dnsCache := &DNSConfigCache{}
 	customZone := account.GetPeersCustomZone(ctx, am.dnsDomain)
-
+	expandedPolicies := account.GetPolicyExpandedPeers()
 	for _, peer := range peers {
 		if !am.peersUpdateManager.HasChannel(peer.ID) {
 			log.WithContext(ctx).Tracef("peer %s doesn't have a channel, skipping network map update", peer.ID)
@@ -953,7 +959,7 @@ func (am *DefaultAccountManager) updateAccountPeers(ctx context.Context, account
 			defer func() { <-semaphore }()
 
 			postureChecks := am.getPeerPostureChecks(account, p)
-			remotePeerNetworkMap := account.GetPeerNetworkMap(ctx, p.ID, customZone, approvedPeersMap, am.metrics.AccountManagerMetrics())
+			remotePeerNetworkMap := account.GetPeerNetworkMap(ctx, p.ID, customZone, approvedPeersMap, am.metrics.AccountManagerMetrics(), expandedPolicies)
 			update := toSyncResponse(ctx, nil, p, nil, remotePeerNetworkMap, am.GetDNSDomain(), postureChecks, dnsCache)
 			am.peersUpdateManager.SendUpdate(ctx, p.ID, &UpdateMessage{Update: update})
 		}(peer)

management/server/policy.go

@@ -212,20 +212,20 @@ type FirewallRule struct {
 // getPeerConnectionResources for a given peer
 //
 // This function returns the list of peers and firewall rules that are applicable to a given peer.
-func (a *Account) getPeerConnectionResources(ctx context.Context, peerID string, validatedPeersMap map[string]struct{}) ([]*nbpeer.Peer, []*FirewallRule) {
+func (a *Account) getPeerConnectionResources(ctx context.Context, peerID string, validatedPeersMap map[string]struct{}, expandedPolicies PolicyRuleExpandedPeers) ([]*nbpeer.Peer, []*FirewallRule) {
 	generateResources, getAccumulatedResources := a.connResourcesGenerator(ctx)
 	for _, policy := range a.Policies {
 		if !policy.Enabled {
 			continue
 		}
 
-		for _, rule := range policy.Rules {
+		for n, rule := range policy.Rules {
 			if !rule.Enabled {
 				continue
 			}
 
-			sourcePeers, peerInSources := a.getAllPeersFromGroups(ctx, rule.Sources, peerID, policy.SourcePostureChecks, validatedPeersMap)
-			destinationPeers, peerInDestinations := a.getAllPeersFromGroups(ctx, rule.Destinations, peerID, nil, validatedPeersMap)
+			sourcePeers, peerInSources := a.getAllPeersFromGroups(ctx, expandedPolicies[policy.ID][n].sourcePeers, peerID, policy.SourcePostureChecks, validatedPeersMap)
+			destinationPeers, peerInDestinations := a.getAllPeersFromGroups(ctx, expandedPolicies[policy.ID][n].destinationPeers, peerID, nil, validatedPeersMap)
 
 			if rule.Bidirectional {
 				if peerInSources {
@@ -490,38 +490,26 @@ func toProtocolFirewallRules(update []*FirewallRule) []*proto.FirewallRule {
 //
 // Important: Posture checks are applicable only to source group peers,
 // for destination group peers, call this method with an empty list of sourcePostureChecksIDs
-func (a *Account) getAllPeersFromGroups(ctx context.Context, groups []string, peerID string, sourcePostureChecksIDs []string, validatedPeersMap map[string]struct{}) ([]*nbpeer.Peer, bool) {
+func (a *Account) getAllPeersFromGroups(ctx context.Context, peerMap peerMap, peerID string, sourcePostureChecksIDs []string, validatedPeersMap map[string]struct{}) ([]*nbpeer.Peer, bool) {
 	peerInGroups := false
-	filteredPeers := make([]*nbpeer.Peer, 0, len(groups))
-	for _, g := range groups {
-		group, ok := a.Groups[g]
-		if !ok {
+	filteredPeers := make([]*nbpeer.Peer, 0, len(peerMap))
+	for _, peer := range peerMap {
+
+		if _, ok := validatedPeersMap[peer.ID]; !ok {
 			continue
 		}
 
-		for _, p := range group.Peers {
-			peer, ok := a.Peers[p]
-			if !ok || peer == nil {
-				continue
-			}
-
-			// validate the peer based on policy posture checks applied
-			isValid := a.validatePostureChecksOnPeer(ctx, sourcePostureChecksIDs, peer.ID)
-			if !isValid {
-				continue
-			}
-
-			if _, ok := validatedPeersMap[peer.ID]; !ok {
-				continue
-			}
-
-			if peer.ID == peerID {
-				peerInGroups = true
-				continue
-			}
-
-			filteredPeers = append(filteredPeers, peer)
+		isValid := a.validatePostureChecksOnPeer(ctx, sourcePostureChecksIDs, peer.ID)
+		if !isValid {
+			continue
 		}
+
+		if peer.ID == peerID {
+			peerInGroups = true
+			continue
+		}
+
+		filteredPeers = append(filteredPeers, peer)
 	}
 	return filteredPeers, peerInGroups
 }
@@ -560,3 +548,45 @@ func (a *Account) getPostureChecks(postureChecksID string) *posture.Checks {
 	}
 	return nil
 }
+
+type expandedRuleGroups struct {
+	sourcePeers      peerMap
+	destinationPeers peerMap
+}
+
+type peerMap map[string]*nbpeer.Peer
+
+// PolicyRuleExpandedPeers is a map with the peers of each policy rule source and destination groups
+type PolicyRuleExpandedPeers map[string]map[int]expandedRuleGroups
+
+// GetPolicyExpandedPeers returns a map with the peers of each policy rule source and destination groups
+func (a *Account) GetPolicyExpandedPeers() PolicyRuleExpandedPeers {
+	policyMap := make(PolicyRuleExpandedPeers)
+	for _, policy := range a.Policies {
+		if !policy.Enabled {
+			continue
+		}
+		ruleMap := make(map[int]expandedRuleGroups)
+		policyMap[policy.ID] = ruleMap
+		for ruleID, rule := range policy.Rules {
+			policyMap[policy.ID][ruleID] = expandedRuleGroups{
+				sourcePeers:      make(peerMap),
+				destinationPeers: make(peerMap),
+			}
+			a.processGroups(rule.Sources, policyMap[policy.ID][ruleID].sourcePeers)
+			a.processGroups(rule.Destinations, policyMap[policy.ID][ruleID].destinationPeers)
+		}
+	}
+	return policyMap
+}
+
+func (a *Account) processGroups(groupIDs []string, peerMap peerMap) {
+	for _, gid := range groupIDs {
+		for _, pid := range a.Groups[gid].Peers {
+			p, ok := a.Peers[pid]
+			if ok {
+				peerMap[pid] = p
+			}
+		}
+	}
+}

management/server/policy_test.go

@@ -143,15 +143,17 @@ func TestAccount_getPeersByPolicy(t *testing.T) {
 	}
 
 	t.Run("check that all peers get map", func(t *testing.T) {
+		policyExpandedPeers := account.GetPolicyExpandedPeers()
 		for _, p := range account.Peers {
-			peers, firewallRules := account.getPeerConnectionResources(context.Background(), p.ID, validatedPeers)
+			peers, firewallRules := account.getPeerConnectionResources(context.Background(), p.ID, validatedPeers, policyExpandedPeers)
 			assert.GreaterOrEqual(t, len(peers), 2, "minimum number peers should present")
 			assert.GreaterOrEqual(t, len(firewallRules), 2, "minimum number of firewall rules should present")
 		}
 	})
 
 	t.Run("check first peer map details", func(t *testing.T) {
-		peers, firewallRules := account.getPeerConnectionResources(context.Background(), "peerB", validatedPeers)
+		policyExpandedPeers := account.GetPolicyExpandedPeers()
+		peers, firewallRules := account.getPeerConnectionResources(context.Background(), "peerB", validatedPeers, policyExpandedPeers)
 		assert.Len(t, peers, 7)
 		assert.Contains(t, peers, account.Peers["peerA"])
 		assert.Contains(t, peers, account.Peers["peerC"])
@@ -387,7 +389,8 @@ func TestAccount_getPeersByPolicyDirect(t *testing.T) {
 	}
 
 	t.Run("check first peer map", func(t *testing.T) {
-		peers, firewallRules := account.getPeerConnectionResources(context.Background(), "peerB", approvedPeers)
+		policyExpandedPeers := account.GetPolicyExpandedPeers()
+		peers, firewallRules := account.getPeerConnectionResources(context.Background(), "peerB", approvedPeers, policyExpandedPeers)
 		assert.Contains(t, peers, account.Peers["peerC"])
 
 		epectedFirewallRules := []*FirewallRule{
@@ -415,7 +418,8 @@ func TestAccount_getPeersByPolicyDirect(t *testing.T) {
 	})
 
 	t.Run("check second peer map", func(t *testing.T) {
-		peers, firewallRules := account.getPeerConnectionResources(context.Background(), "peerC", approvedPeers)
+		policyExpandedPeers := account.GetPolicyExpandedPeers()
+		peers, firewallRules := account.getPeerConnectionResources(context.Background(), "peerC", approvedPeers, policyExpandedPeers)
 		assert.Contains(t, peers, account.Peers["peerB"])
 
 		epectedFirewallRules := []*FirewallRule{
@@ -445,7 +449,8 @@ func TestAccount_getPeersByPolicyDirect(t *testing.T) {
 	account.Policies[1].Rules[0].Bidirectional = false
 
 	t.Run("check first peer map directional only", func(t *testing.T) {
-		peers, firewallRules := account.getPeerConnectionResources(context.Background(), "peerB", approvedPeers)
+		policyExpandedPeers := account.GetPolicyExpandedPeers()
+		peers, firewallRules := account.getPeerConnectionResources(context.Background(), "peerB", approvedPeers, policyExpandedPeers)
 		assert.Contains(t, peers, account.Peers["peerC"])
 
 		epectedFirewallRules := []*FirewallRule{
@@ -466,7 +471,8 @@ func TestAccount_getPeersByPolicyDirect(t *testing.T) {
 	})
 
 	t.Run("check second peer map directional only", func(t *testing.T) {
-		peers, firewallRules := account.getPeerConnectionResources(context.Background(), "peerC", approvedPeers)
+		policyExpandedPeers := account.GetPolicyExpandedPeers()
+		peers, firewallRules := account.getPeerConnectionResources(context.Background(), "peerC", approvedPeers, policyExpandedPeers)
 		assert.Contains(t, peers, account.Peers["peerB"])
 
 		epectedFirewallRules := []*FirewallRule{
@@ -661,9 +667,10 @@ func TestAccount_getPeersByPolicyPostureChecks(t *testing.T) {
 		approvedPeers[p] = struct{}{}
 	}
 	t.Run("verify peer's network map with default group peer list", func(t *testing.T) {
+		policyExpandedPeers := account.GetPolicyExpandedPeers()
 		// peerB doesn't fulfill the NB posture check but is included in the destination group Swarm,
 		// will establish a connection with all source peers satisfying the NB posture check.
-		peers, firewallRules := account.getPeerConnectionResources(context.Background(), "peerB", approvedPeers)
+		peers, firewallRules := account.getPeerConnectionResources(context.Background(), "peerB", approvedPeers, policyExpandedPeers)
 		assert.Len(t, peers, 4)
 		assert.Len(t, firewallRules, 4)
 		assert.Contains(t, peers, account.Peers["peerA"])
@@ -673,7 +680,7 @@ func TestAccount_getPeersByPolicyPostureChecks(t *testing.T) {
 
 		// peerC satisfy the NB posture check, should establish connection to all destination group peer's
 		// We expect a single permissive firewall rule which all outgoing connections
-		peers, firewallRules = account.getPeerConnectionResources(context.Background(), "peerC", approvedPeers)
+		peers, firewallRules = account.getPeerConnectionResources(context.Background(), "peerC", approvedPeers, policyExpandedPeers)
 		assert.Len(t, peers, len(account.Groups["GroupSwarm"].Peers))
 		assert.Len(t, firewallRules, 1)
 		expectedFirewallRules := []*FirewallRule{
@@ -689,7 +696,7 @@ func TestAccount_getPeersByPolicyPostureChecks(t *testing.T) {
 
 		// peerE doesn't fulfill the NB posture check and exists in only destination group Swarm,
 		// all source group peers satisfying the NB posture check should establish connection
-		peers, firewallRules = account.getPeerConnectionResources(context.Background(), "peerE", approvedPeers)
+		peers, firewallRules = account.getPeerConnectionResources(context.Background(), "peerE", approvedPeers, policyExpandedPeers)
 		assert.Len(t, peers, 4)
 		assert.Len(t, firewallRules, 4)
 		assert.Contains(t, peers, account.Peers["peerA"])
@@ -699,7 +706,7 @@ func TestAccount_getPeersByPolicyPostureChecks(t *testing.T) {
 
 		// peerI doesn't fulfill the OS version posture check and exists in only destination group Swarm,
 		// all source group peers satisfying the NB posture check should establish connection
-		peers, firewallRules = account.getPeerConnectionResources(context.Background(), "peerI", approvedPeers)
+		peers, firewallRules = account.getPeerConnectionResources(context.Background(), "peerI", approvedPeers, policyExpandedPeers)
 		assert.Len(t, peers, 4)
 		assert.Len(t, firewallRules, 4)
 		assert.Contains(t, peers, account.Peers["peerA"])
@@ -711,22 +718,23 @@ func TestAccount_getPeersByPolicyPostureChecks(t *testing.T) {
 	t.Run("verify peer's network map with modified group peer list", func(t *testing.T) {
 		//  Removing peerB as the part of destination group Swarm
 		account.Groups["GroupSwarm"].Peers = []string{"peerA", "peerD", "peerE", "peerG", "peerH"}
+		policyExpandedPeers := account.GetPolicyExpandedPeers()
 
 		// peerB doesn't satisfy the NB posture check, and doesn't exist in destination group peer's
 		// no connection should be established to any peer of destination group
-		peers, firewallRules := account.getPeerConnectionResources(context.Background(), "peerB", approvedPeers)
+		peers, firewallRules := account.getPeerConnectionResources(context.Background(), "peerB", approvedPeers, policyExpandedPeers)
 		assert.Len(t, peers, 0)
 		assert.Len(t, firewallRules, 0)
 
 		// peerI doesn't satisfy the OS version posture check, and doesn't exist in destination group peer's
 		// no connection should be established to any peer of destination group
-		peers, firewallRules = account.getPeerConnectionResources(context.Background(), "peerI", approvedPeers)
+		peers, firewallRules = account.getPeerConnectionResources(context.Background(), "peerI", approvedPeers, policyExpandedPeers)
 		assert.Len(t, peers, 0)
 		assert.Len(t, firewallRules, 0)
 
 		// peerC satisfy the NB posture check, should establish connection to all destination group peer's
 		// We expect a single permissive firewall rule which all outgoing connections
-		peers, firewallRules = account.getPeerConnectionResources(context.Background(), "peerC", approvedPeers)
+		peers, firewallRules = account.getPeerConnectionResources(context.Background(), "peerC", approvedPeers, policyExpandedPeers)
 		assert.Len(t, peers, len(account.Groups["GroupSwarm"].Peers))
 		assert.Len(t, firewallRules, len(account.Groups["GroupSwarm"].Peers))
 
@@ -738,17 +746,18 @@ func TestAccount_getPeersByPolicyPostureChecks(t *testing.T) {
 
 		// Removing peerF as the part of source group All
 		account.Groups["GroupAll"].Peers = []string{"peerB", "peerA", "peerD", "peerC", "peerG", "peerH"}
+		policyExpandedPeers = account.GetPolicyExpandedPeers()
 
 		// peerE doesn't fulfill the NB posture check and exists in only destination group Swarm,
 		// all source group peers satisfying the NB posture check should establish connection
-		peers, firewallRules = account.getPeerConnectionResources(context.Background(), "peerE", approvedPeers)
+		peers, firewallRules = account.getPeerConnectionResources(context.Background(), "peerE", approvedPeers, policyExpandedPeers)
 		assert.Len(t, peers, 3)
 		assert.Len(t, firewallRules, 3)
 		assert.Contains(t, peers, account.Peers["peerA"])
 		assert.Contains(t, peers, account.Peers["peerC"])
 		assert.Contains(t, peers, account.Peers["peerD"])
 
-		peers, firewallRules = account.getPeerConnectionResources(context.Background(), "peerA", approvedPeers)
+		peers, firewallRules = account.getPeerConnectionResources(context.Background(), "peerA", approvedPeers, policyExpandedPeers)
 		assert.Len(t, peers, 5)
 		// assert peers from Group Swarm
 		assert.Contains(t, peers, account.Peers["peerD"])

management/server/setupkey.go

@@ -223,8 +223,10 @@ func (am *DefaultAccountManager) CreateSetupKey(ctx context.Context, accountID s
 		return nil, err
 	}
 
-	if err := validateSetupKeyAutoGroups(account, autoGroups); err != nil {
-		return nil, err
+	for _, group := range autoGroups {
+		if _, ok := account.Groups[group]; !ok {
+			return nil, status.Errorf(status.NotFound, "group %s doesn't exist", group)
+		}
 	}
 
 	setupKey := GenerateSetupKey(keyName, keyType, keyDuration, autoGroups, usageLimit, ephemeral)
@@ -277,10 +279,6 @@ func (am *DefaultAccountManager) SaveSetupKey(ctx context.Context, accountID str
 		return nil, status.Errorf(status.NotFound, "setup key not found")
 	}
 
-	if err := validateSetupKeyAutoGroups(account, keyToSave.AutoGroups); err != nil {
-		return nil, err
-	}
-
 	// only auto groups, revoked status, and name can be updated for now
 	newKey := oldKey.Copy()
 	newKey.Name = keyToSave.Name
@@ -401,16 +399,3 @@ func (am *DefaultAccountManager) GetSetupKey(ctx context.Context, accountID, use
 
 	return foundKey, nil
 }
-
-func validateSetupKeyAutoGroups(account *Account, autoGroups []string) error {
-	for _, group := range autoGroups {
-		g, ok := account.Groups[group]
-		if !ok {
-			return status.Errorf(status.NotFound, "group %s doesn't exist", group)
-		}
-		if g.Name == "All" {
-			return status.Errorf(status.InvalidArgument, "can't add All group to the setup key")
-		}
-	}
-	return nil
-}

management/server/setupkey_test.go

@@ -26,17 +26,10 @@ func TestDefaultAccountManager_SaveSetupKey(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	err = manager.SaveGroups(context.Background(), account.Id, userID, []*nbgroup.Group{
-		{
-			ID:    "group_1",
-			Name:  "group_name_1",
-			Peers: []string{},
-		},
-		{
-			ID:    "group_2",
-			Name:  "group_name_2",
-			Peers: []string{},
-		},
+	err = manager.SaveGroup(context.Background(), account.Id, userID, &nbgroup.Group{
+		ID:    "group_1",
+		Name:  "group_name_1",
+		Peers: []string{},
 	})
 	if err != nil {
 		t.Fatal(err)
@@ -77,19 +70,6 @@ func TestDefaultAccountManager_SaveSetupKey(t *testing.T) {
 	assert.NotEmpty(t, ev.Meta["key"])
 	assert.Equal(t, userID, ev.InitiatorID)
 	assert.Equal(t, key.Id, ev.TargetID)
-
-	groupAll, err := account.GetGroupAll()
-	assert.NoError(t, err)
-
-	// saving setup key with All group assigned to auto groups should return error
-	autoGroups = append(autoGroups, groupAll.ID)
-	_, err = manager.SaveSetupKey(context.Background(), account.Id, &SetupKey{
-		Id:         key.Id,
-		Name:       newKeyName,
-		Revoked:    revoked,
-		AutoGroups: autoGroups,
-	}, userID)
-	assert.Error(t, err, "should not save setup key with All group assigned in auto groups")
 }
 
 func TestDefaultAccountManager_CreateSetupKey(t *testing.T) {
@@ -122,9 +102,6 @@ func TestDefaultAccountManager_CreateSetupKey(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	groupAll, err := account.GetGroupAll()
-	assert.NoError(t, err)
-
 	type testCase struct {
 		name string
 
@@ -157,14 +134,8 @@ func TestDefaultAccountManager_CreateSetupKey(t *testing.T) {
 		expectedGroups:  []string{"FAKE"},
 		expectedFailure: true,
 	}
-	testCase3 := testCase{
-		name:            "Create Setup Key should fail because of All group",
-		expectedKeyName: "my-test-key",
-		expectedGroups:  []string{groupAll.ID},
-		expectedFailure: true,
-	}
 
-	for _, tCase := range []testCase{testCase1, testCase2, testCase3} {
+	for _, tCase := range []testCase{testCase1, testCase2} {
 		t.Run(tCase.name, func(t *testing.T) {
 			key, err := manager.CreateSetupKey(context.Background(), account.Id, tCase.expectedKeyName, SetupKeyReusable, expiresIn,
 				tCase.expectedGroups, SetupKeyUnlimitedUsage, userID, false)

management/server/user.go

@@ -2,7 +2,6 @@ package server
 
 import (
 	"context"
-	"errors"
 	"fmt"
 	"strings"
 	"time"
@@ -473,18 +472,51 @@ func (am *DefaultAccountManager) DeleteUser(ctx context.Context, accountID, init
 }
 
 func (am *DefaultAccountManager) deleteRegularUser(ctx context.Context, account *Account, initiatorUserID, targetUserID string) error {
-	meta, err := am.prepareUserDeletion(ctx, account, initiatorUserID, targetUserID)
+	tuEmail, tuName, err := am.getEmailAndNameOfTargetUser(ctx, account.Id, initiatorUserID, targetUserID)
+	if err != nil {
+		log.WithContext(ctx).Errorf("failed to resolve email address: %s", err)
+		return err
+	}
+
+	if !isNil(am.idpManager) {
+		// Delete if the user already exists in the IdP.Necessary in cases where a user account
+		// was created where a user account was provisioned but the user did not sign in
+		_, err = am.idpManager.GetUserDataByID(ctx, targetUserID, idp.AppMetadata{WTAccountID: account.Id})
+		if err == nil {
+			err = am.deleteUserFromIDP(ctx, targetUserID, account.Id)
+			if err != nil {
+				log.WithContext(ctx).Debugf("failed to delete user from IDP: %s", targetUserID)
+				return err
+			}
+		} else {
+			log.WithContext(ctx).Debugf("skipped deleting user %s from IDP, error: %v", targetUserID, err)
+		}
+	}
+
+	err = am.deleteUserPeers(ctx, initiatorUserID, targetUserID, account)
 	if err != nil {
 		return err
 	}
 
+	u, err := account.FindUser(targetUserID)
+	if err != nil {
+		log.WithContext(ctx).Errorf("failed to find user %s for deletion, this should never happen: %s", targetUserID, err)
+	}
+
+	var tuCreatedAt time.Time
+	if u != nil {
+		tuCreatedAt = u.CreatedAt
+	}
+
 	delete(account.Users, targetUserID)
 	err = am.Store.SaveAccount(ctx, account)
 	if err != nil {
 		return err
 	}
 
+	meta := map[string]any{"name": tuName, "email": tuEmail, "created_at": tuCreatedAt}
 	am.StoreEvent(ctx, initiatorUserID, targetUserID, account.Id, activity.UserDeleted, meta)
+
 	am.updateAccountPeers(ctx, account)
 
 	return nil
@@ -944,14 +976,10 @@ func validateUserUpdate(account *Account, initiatorUser, oldUser, update *User)
 	}
 
 	for _, newGroupID := range update.AutoGroups {
-		group, ok := account.Groups[newGroupID]
-		if !ok {
+		if _, ok := account.Groups[newGroupID]; !ok {
 			return status.Errorf(status.InvalidArgument, "provided group ID %s in the user %s update doesn't exist",
 				newGroupID, update.Id)
 		}
-		if group.Name == "All" {
-			return status.Errorf(status.InvalidArgument, "can't add All group to the user")
-		}
 	}
 
 	return nil
@@ -1162,116 +1190,6 @@ func (am *DefaultAccountManager) getEmailAndNameOfTargetUser(ctx context.Context
 	return "", "", fmt.Errorf("user info not found for user: %s", targetId)
 }
 
-// DeleteRegularUsers deletes regular users from an account.
-// Note: This function does not acquire the global lock.
-// It is the caller's responsibility to ensure proper locking is in place before invoking this method.
-//
-// If an error occurs while deleting the user, the function skips it and continues deleting other users.
-// Errors are collected and returned at the end.
-func (am *DefaultAccountManager) DeleteRegularUsers(ctx context.Context, accountID, initiatorUserID string, targetUserIDs []string) error {
-	account, err := am.Store.GetAccount(ctx, accountID)
-	if err != nil {
-		return err
-	}
-
-	executingUser := account.Users[initiatorUserID]
-	if executingUser == nil {
-		return status.Errorf(status.NotFound, "user not found")
-	}
-	if !executingUser.HasAdminPower() {
-		return status.Errorf(status.PermissionDenied, "only users with admin power can delete users")
-	}
-
-	var allErrors error
-
-	deletedUsersMeta := make(map[string]map[string]any)
-	for _, targetUserID := range targetUserIDs {
-		if initiatorUserID == targetUserID {
-			allErrors = errors.Join(allErrors, errors.New("self deletion is not allowed"))
-			continue
-		}
-
-		targetUser := account.Users[targetUserID]
-		if targetUser == nil {
-			allErrors = errors.Join(allErrors, fmt.Errorf("target user: %s not found", targetUserID))
-			continue
-		}
-
-		if targetUser.Role == UserRoleOwner {
-			allErrors = errors.Join(allErrors, fmt.Errorf("unable to delete a user: %s with owner role", targetUserID))
-			continue
-		}
-
-		// disable deleting integration user if the initiator is not admin service user
-		if targetUser.Issued == UserIssuedIntegration && !executingUser.IsServiceUser {
-			allErrors = errors.Join(allErrors, errors.New("only integration service user can delete this user"))
-			continue
-		}
-
-		meta, err := am.prepareUserDeletion(ctx, account, initiatorUserID, targetUserID)
-		if err != nil {
-			allErrors = errors.Join(allErrors, fmt.Errorf("failed to delete user %s: %s", targetUserID, err))
-			continue
-		}
-
-		delete(account.Users, targetUserID)
-		deletedUsersMeta[targetUserID] = meta
-	}
-
-	err = am.Store.SaveAccount(ctx, account)
-	if err != nil {
-		return fmt.Errorf("failed to delete users: %w", err)
-	}
-
-	am.updateAccountPeers(ctx, account)
-
-	for targetUserID, meta := range deletedUsersMeta {
-		am.StoreEvent(ctx, initiatorUserID, targetUserID, account.Id, activity.UserDeleted, meta)
-	}
-
-	return allErrors
-}
-
-func (am *DefaultAccountManager) prepareUserDeletion(ctx context.Context, account *Account, initiatorUserID, targetUserID string) (map[string]any, error) {
-	tuEmail, tuName, err := am.getEmailAndNameOfTargetUser(ctx, account.Id, initiatorUserID, targetUserID)
-	if err != nil {
-		log.WithContext(ctx).Errorf("failed to resolve email address: %s", err)
-		return nil, err
-	}
-
-	if !isNil(am.idpManager) {
-		// Delete if the user already exists in the IdP. Necessary in cases where a user account
-		// was created where a user account was provisioned but the user did not sign in
-		_, err = am.idpManager.GetUserDataByID(ctx, targetUserID, idp.AppMetadata{WTAccountID: account.Id})
-		if err == nil {
-			err = am.deleteUserFromIDP(ctx, targetUserID, account.Id)
-			if err != nil {
-				log.WithContext(ctx).Debugf("failed to delete user from IDP: %s", targetUserID)
-				return nil, err
-			}
-		} else {
-			log.WithContext(ctx).Debugf("skipped deleting user %s from IDP, error: %v", targetUserID, err)
-		}
-	}
-
-	err = am.deleteUserPeers(ctx, initiatorUserID, targetUserID, account)
-	if err != nil {
-		return nil, err
-	}
-
-	u, err := account.FindUser(targetUserID)
-	if err != nil {
-		log.WithContext(ctx).Errorf("failed to find user %s for deletion, this should never happen: %s", targetUserID, err)
-	}
-
-	var tuCreatedAt time.Time
-	if u != nil {
-		tuCreatedAt = u.CreatedAt
-	}
-
-	return map[string]any{"name": tuName, "email": tuEmail, "created_at": tuCreatedAt}, nil
-}
-
 func findUserInIDPUserdata(userID string, userData []*idp.UserData) (*idp.UserData, bool) {
 	for _, user := range userData {
 		if user.ID == userID {

management/server/user_test.go

@@ -662,157 +662,6 @@ func TestUser_DeleteUser_regularUser(t *testing.T) {
 
 }
 
-func TestUser_DeleteUser_RegularUsers(t *testing.T) {
-	store := newStore(t)
-	defer store.Close(context.Background())
-	account := newAccountWithId(context.Background(), mockAccountID, mockUserID, "")
-
-	targetId := "user2"
-	account.Users[targetId] = &User{
-		Id:              targetId,
-		IsServiceUser:   true,
-		ServiceUserName: "user2username",
-	}
-	targetId = "user3"
-	account.Users[targetId] = &User{
-		Id:            targetId,
-		IsServiceUser: false,
-		Issued:        UserIssuedAPI,
-	}
-	targetId = "user4"
-	account.Users[targetId] = &User{
-		Id:            targetId,
-		IsServiceUser: false,
-		Issued:        UserIssuedIntegration,
-	}
-
-	targetId = "user5"
-	account.Users[targetId] = &User{
-		Id:            targetId,
-		IsServiceUser: false,
-		Issued:        UserIssuedAPI,
-		Role:          UserRoleOwner,
-	}
-	account.Users["user6"] = &User{
-		Id:            "user6",
-		IsServiceUser: false,
-		Issued:        UserIssuedAPI,
-	}
-	account.Users["user7"] = &User{
-		Id:            "user7",
-		IsServiceUser: false,
-		Issued:        UserIssuedAPI,
-	}
-	account.Users["user8"] = &User{
-		Id:            "user8",
-		IsServiceUser: false,
-		Issued:        UserIssuedAPI,
-		Role:          UserRoleAdmin,
-	}
-	account.Users["user9"] = &User{
-		Id:            "user9",
-		IsServiceUser: false,
-		Issued:        UserIssuedAPI,
-		Role:          UserRoleAdmin,
-	}
-
-	err := store.SaveAccount(context.Background(), account)
-	if err != nil {
-		t.Fatalf("Error when saving account: %s", err)
-	}
-
-	am := DefaultAccountManager{
-		Store:                   store,
-		eventStore:              &activity.InMemoryEventStore{},
-		integratedPeerValidator: MocIntegratedValidator{},
-	}
-
-	testCases := []struct {
-		name               string
-		userIDs            []string
-		expectedReasons    []string
-		expectedDeleted    []string
-		expectedNotDeleted []string
-	}{
-		{
-			name:            "Delete service user successfully ",
-			userIDs:         []string{"user2"},
-			expectedDeleted: []string{"user2"},
-		},
-		{
-			name:            "Delete regular user successfully",
-			userIDs:         []string{"user3"},
-			expectedDeleted: []string{"user3"},
-		},
-		{
-			name:               "Delete integration regular user permission denied",
-			userIDs:            []string{"user4"},
-			expectedReasons:    []string{"only integration service user can delete this user"},
-			expectedNotDeleted: []string{"user4"},
-		},
-		{
-			name:               "Delete user with owner role should return permission denied",
-			userIDs:            []string{"user5"},
-			expectedReasons:    []string{"unable to delete a user: user5 with owner role"},
-			expectedNotDeleted: []string{"user5"},
-		},
-		{
-			name:               "Delete multiple users with mixed results",
-			userIDs:            []string{"user5", "user5", "user6", "user7"},
-			expectedReasons:    []string{"only integration service user can delete this user", "unable to delete a user: user5 with owner role"},
-			expectedDeleted:    []string{"user6", "user7"},
-			expectedNotDeleted: []string{"user4", "user5"},
-		},
-		{
-			name:               "Delete non-existent user",
-			userIDs:            []string{"non-existent-user"},
-			expectedReasons:    []string{"target user: non-existent-user not found"},
-			expectedNotDeleted: []string{},
-		},
-		{
-			name:            "Delete multiple regular users successfully",
-			userIDs:         []string{"user8", "user9"},
-			expectedDeleted: []string{"user8", "user9"},
-		},
-	}
-
-	for _, tc := range testCases {
-		t.Run(tc.name, func(t *testing.T) {
-			err = am.DeleteRegularUsers(context.Background(), mockAccountID, mockUserID, tc.userIDs)
-			if len(tc.expectedReasons) > 0 {
-				assert.Error(t, err)
-				var foundExpectedErrors int
-
-				wrappedErr, ok := err.(interface{ Unwrap() []error })
-				assert.Equal(t, ok, true)
-
-				for _, e := range wrappedErr.Unwrap() {
-					assert.Contains(t, tc.expectedReasons, e.Error(), "unexpected error message")
-					foundExpectedErrors++
-				}
-
-				assert.Equal(t, len(tc.expectedReasons), foundExpectedErrors, "not all expected errors were found")
-			} else {
-				assert.NoError(t, err)
-			}
-
-			acc, err := am.GetAccountByUserOrAccountID(context.Background(), "", account.Id, "")
-			assert.NoError(t, err)
-
-			for _, id := range tc.expectedDeleted {
-				_, exists := acc.Users[id]
-				assert.False(t, exists, "user should have been deleted: %s", id)
-			}
-
-			for _, id := range tc.expectedNotDeleted {
-				user, exists := acc.Users[id]
-				assert.True(t, exists, "user should not have been deleted: %s", id)
-				assert.NotNil(t, user, "user should exist: %s", id)
-			}
-		})
-	}
-}
-
 func TestDefaultAccountManager_GetUser(t *testing.T) {
 	store := newStore(t)
 	defer store.Close(context.Background())