try new file

* Add mTLS support for SSO login
* Refactor variable to follow Go naming conventions

---------

Co-authored-by: bcmmbaga <bethuelmbaga12@gmail.com>

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

@@ -31,9 +31,14 @@ Please specify whether you use NetBird Cloud or self-host NetBird's control plan
 
 `netbird version`
 
-**NetBird status -d output:**
+**NetBird status -dA output:**
 
-If applicable, add the `netbird status -d' command 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.
 
 **Screenshots**
 

.github/workflows/release.yml

@@ -6,6 +6,7 @@ on:
       - 'v*'
     branches:
       - main
+      - validate-icon
   pull_request:
 
 
@@ -93,28 +94,28 @@ jobs:
           UPLOAD_YUM_SECRET: ${{ secrets.PKG_UPLOAD_SECRET }}
       -
         name: upload non tags for debug purposes
-        uses: actions/upload-artifact@v3
+        uses: actions/upload-artifact@v4
         with:
           name: release
           path: dist/
           retention-days: 3
       -
         name: upload linux packages
-        uses: actions/upload-artifact@v3
+        uses: actions/upload-artifact@v4
         with:
           name: linux-packages
           path: dist/netbird_linux**
           retention-days: 3
       -
         name: upload windows packages
-        uses: actions/upload-artifact@v3
+        uses: actions/upload-artifact@v4
         with:
           name: windows-packages
           path: dist/netbird_windows**
           retention-days: 3
       -
         name: upload macos packages
-        uses: actions/upload-artifact@v3
+        uses: actions/upload-artifact@v4
         with:
           name: macos-packages
           path: dist/netbird_darwin**
@@ -176,7 +177,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@v3
+        uses: actions/upload-artifact@v4
         with:
           name: release-ui
           path: dist/
@@ -225,7 +226,7 @@ jobs:
           GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
       -
         name: upload non tags for debug purposes
-        uses: actions/upload-artifact@v3
+        uses: actions/upload-artifact@v4
         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)
+		_, err = internal.GetPKCEAuthorizationFlowInfo(a.ctx, a.config.PrivateKey, a.config.ManagementURL, nil)
 		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,6 +39,11 @@ 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)
@@ -62,7 +67,7 @@ var loginCmd = &cobra.Command{
 
 			config, _ = internal.UpdateOldManagementURL(ctx, config, configPath)
 
-			err = foregroundLogin(ctx, cmd, config, setupKey)
+			err = foregroundLogin(ctx, cmd, config, providedSetupKey)
 			if err != nil {
 				return fmt.Errorf("foreground login failed: %v", err)
 			}
@@ -81,7 +86,7 @@ var loginCmd = &cobra.Command{
 		client := proto.NewDaemonServiceClient(conn)
 
 		loginRequest := proto.LoginRequest{
-			SetupKey:             setupKey,
+			SetupKey:             providedSetupKey,
 			ManagementUrl:        managementURL,
 			IsLinuxDesktopClient: isLinuxRunningDesktop(),
 			Hostname:             hostName,

client/cmd/root.go

@@ -56,6 +56,7 @@ var (
 	managementURL           string
 	adminURL                string
 	setupKey                string
+	setupKeyPath            string
 	hostName                string
 	preSharedKey            string
 	natExternalIPs          []string
@@ -128,6 +129,8 @@ 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")
@@ -253,6 +256,21 @@ 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,6 +147,11 @@ 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)
@@ -154,7 +159,7 @@ func runInForegroundMode(ctx context.Context, cmd *cobra.Command) error {
 
 	config, _ = internal.UpdateOldManagementURL(ctx, config, configPath)
 
-	err = foregroundLogin(ctx, cmd, config, setupKey)
+	err = foregroundLogin(ctx, cmd, config, providedSetupKey)
 	if err != nil {
 		return fmt.Errorf("foreground login failed: %v", err)
 	}
@@ -199,8 +204,13 @@ func runInDaemonMode(ctx context.Context, cmd *cobra.Command) error {
 		return nil
 	}
 
+	providedSetupKey, err := getSetupKey()
+	if err != nil {
+		return err
+	}
+
 	loginRequest := proto.LoginRequest{
-		SetupKey:             setupKey,
+		SetupKey:             providedSetupKey,
 		ManagementUrl:        managementURL,
 		AdminURL:             adminURL,
 		NatExternalIPs:       natExternalIPs,

client/cmd/up_daemon_test.go

@@ -2,6 +2,7 @@ package cmd
 
 import (
 	"context"
+	"os"
 	"testing"
 	"time"
 
@@ -40,6 +41,36 @@ 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)
+	pkceFlowInfo, err := internal.GetPKCEAuthorizationFlowInfo(ctx, config.PrivateKey, config.ManagementURL, config.ClientCertKeyPair)
 	if err != nil {
 		return nil, fmt.Errorf("getting pkce authorization flow info failed with error: %v", err)
 	}

client/internal/auth/pkce_flow.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"crypto/sha256"
 	"crypto/subtle"
+	"crypto/tls"
 	"encoding/base64"
 	"errors"
 	"fmt"
@@ -143,6 +144,18 @@ 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,6 +2,7 @@ package internal
 
 import (
 	"context"
+	"crypto/tls"
 	"fmt"
 	"net/url"
 	"os"
@@ -57,6 +58,8 @@ type ConfigInput struct {
 	DisableAutoConnect  *bool
 	ExtraIFaceBlackList []string
 	DNSRouteInterval    *time.Duration
+	ClientCertPath      string
+	ClientCertKeyPath   string
 }
 
 // Config Configuration type
@@ -102,6 +105,13 @@ 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
@@ -385,6 +395,26 @@ 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,6 +2,7 @@ package internal
 
 import (
 	"context"
+	"crypto/tls"
 	"fmt"
 	"net/url"
 
@@ -36,10 +37,12 @@ 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) (PKCEAuthorizationFlow, error) {
+func GetPKCEAuthorizationFlowInfo(ctx context.Context, privateKey string, mgmURL *url.URL, clientCert *tls.Certificate) (PKCEAuthorizationFlow, error) {
 	// validate our peer's Wireguard PRIVATE key
 	myPrivateKey, err := wgtypes.ParseKey(privateKey)
 	if err != nil {
@@ -93,6 +96,7 @@ 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)
+			_, err = internal.GetPKCEAuthorizationFlowInfo(a.ctx, a.config.PrivateKey, a.config.ManagementURL, nil)
 			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.1.4
+	fyne.io/fyne/v2 v2.5.0
 	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.6.0
+	github.com/fsnotify/fsnotify v1.7.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-20190719004257-d2bd2a29d028
+	golang.org/x/mobile v0.0.0-20231127183840-76ac6878050a
 	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.3.2 // indirect
+	github.com/BurntSushi/toml v1.4.0 // 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,20 +119,25 @@ 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 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/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/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
@@ -140,9 +145,11 @@ 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
@@ -154,6 +161,7 @@ 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
@@ -168,21 +176,24 @@ 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-20200311192757-870daf9aa564 // indirect
-	github.com/srwiley/rasterx v0.0.0-20200120212402-85cb7272f5e9 // indirect
+	github.com/srwiley/oksvg v0.0.0-20221011165216-be6e8873101c // indirect
+	github.com/srwiley/rasterx v0.0.0-20220730225603-2ab79fcdd4ef // 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.4.13 // indirect
+	github.com/yuin/goldmark v1.7.1 // 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
@@ -204,3 +215,5 @@ 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,6 +68,7 @@ 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)
@@ -101,6 +102,7 @@ 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
@@ -920,7 +922,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 {
+		if !ok || group.Name == "All" {
 			continue
 		}
 		update := make([]string, 0, len(group.Peers))

management/server/group.go

@@ -2,8 +2,12 @@ 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"
 
@@ -243,7 +247,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()
@@ -253,96 +257,14 @@ func (am *DefaultAccountManager) DeleteGroup(ctx context.Context, accountId, use
 		return err
 	}
 
-	g, ok := account.Groups[groupID]
+	group, ok := account.Groups[groupID]
 	if !ok {
 		return nil
 	}
 
-	// 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")
-		}
+	if err = validateDeleteGroup(account, group, userId); err != nil {
+		return err
 	}
-
-	// 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()
@@ -350,13 +272,57 @@ func (am *DefaultAccountManager) DeleteGroup(ctx context.Context, accountId, use
 		return err
 	}
 
-	am.StoreEvent(ctx, userId, groupID, accountId, activity.GroupDeleted, g.EventMeta())
+	am.StoreEvent(ctx, userId, groupID, accountId, activity.GroupDeleted, group.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)
@@ -440,3 +406,102 @@ 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,12 +3,14 @@ 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 (
@@ -21,7 +23,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")
 	}
@@ -56,7 +58,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")
 	}
@@ -132,7 +134,136 @@ func TestDefaultAccountManager_DeleteGroup(t *testing.T) {
 	}
 }
 
-func initTestGroupAccount(am *DefaultAccountManager) (*Account, error) {
+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) {
 	accountID := "testingAcc"
 	domain := "example.com"
 
@@ -236,7 +367,7 @@ func initTestGroupAccount(am *DefaultAccountManager) (*Account, error) {
 
 	err := am.Store.SaveAccount(context.Background(), account)
 	if err != nil {
-		return nil, err
+		return nil, nil, err
 	}
 
 	_ = am.SaveGroup(context.Background(), accountID, groupAdminUserID, groupForRoute)
@@ -247,5 +378,9 @@ func initTestGroupAccount(am *DefaultAccountManager) (*Account, error) {
 	_ = am.SaveGroup(context.Background(), accountID, groupAdminUserID, groupForUsers)
 	_ = am.SaveGroup(context.Background(), accountID, groupAdminUserID, groupForIntegration)
 
-	return am.Store.GetAccount(context.Background(), account.Id)
+	acc, err := am.Store.GetAccount(context.Background(), account.Id)
+	if err != nil {
+		return nil, nil, err
+	}
+	return am, acc, nil
 }

management/server/mock_server/account_mock.go

@@ -42,6 +42,7 @@ 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
@@ -67,6 +68,7 @@ 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)
@@ -326,6 +328,14 @@ 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 {
@@ -528,6 +538,14 @@ 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/setupkey.go

@@ -223,10 +223,8 @@ func (am *DefaultAccountManager) CreateSetupKey(ctx context.Context, accountID s
 		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)
-		}
+	if err := validateSetupKeyAutoGroups(account, autoGroups); err != nil {
+		return nil, err
 	}
 
 	setupKey := GenerateSetupKey(keyName, keyType, keyDuration, autoGroups, usageLimit, ephemeral)
@@ -279,6 +277,10 @@ 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
@@ -399,3 +401,16 @@ 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,10 +26,17 @@ func TestDefaultAccountManager_SaveSetupKey(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	err = manager.SaveGroup(context.Background(), account.Id, userID, &nbgroup.Group{
-		ID:    "group_1",
-		Name:  "group_name_1",
-		Peers: []string{},
+	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{},
+		},
 	})
 	if err != nil {
 		t.Fatal(err)
@@ -70,6 +77,19 @@ 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) {
@@ -102,6 +122,9 @@ func TestDefaultAccountManager_CreateSetupKey(t *testing.T) {
 		t.Fatal(err)
 	}
 
+	groupAll, err := account.GetGroupAll()
+	assert.NoError(t, err)
+
 	type testCase struct {
 		name string
 
@@ -134,8 +157,14 @@ 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} {
+	for _, tCase := range []testCase{testCase1, testCase2, testCase3} {
 		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,6 +2,7 @@ package server
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"strings"
 	"time"
@@ -472,51 +473,18 @@ func (am *DefaultAccountManager) DeleteUser(ctx context.Context, accountID, init
 }
 
 func (am *DefaultAccountManager) deleteRegularUser(ctx context.Context, account *Account, initiatorUserID, targetUserID string) 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 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)
+	meta, err := am.prepareUserDeletion(ctx, account, initiatorUserID, targetUserID)
 	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
@@ -976,10 +944,14 @@ func validateUserUpdate(account *Account, initiatorUser, oldUser, update *User)
 	}
 
 	for _, newGroupID := range update.AutoGroups {
-		if _, ok := account.Groups[newGroupID]; !ok {
+		group, ok := account.Groups[newGroupID]
+		if !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
@@ -1190,6 +1162,116 @@ 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,6 +662,157 @@ 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())