Avoid creating duplicate groups with the same name (#1579)

Avoid creating groups with the same name via API calls. 

JWT and integrations still allowed to register groups with duplicated names

management/server/account.go

@@ -40,9 +40,6 @@ const (
 	PublicCategory             = "public"
 	PrivateCategory            = "private"
 	UnknownCategory            = "unknown"
-	GroupIssuedAPI             = "api"
-	GroupIssuedJWT             = "jwt"
-	GroupIssuedIntegration     = "integration"
 	CacheExpirationMax         = 7 * 24 * 3600 * time.Second // 7 days
 	CacheExpirationMin         = 3 * 24 * 3600 * time.Second // 3 days
 	DefaultPeerLoginExpiration = 24 * time.Hour
@@ -556,6 +553,16 @@ func (a *Account) FindUser(userID string) (*User, error) {
 	return user, nil
 }
 
+// FindGroupByName looks for a given group in the Account by name or returns error if the group wasn't found.
+func (a *Account) FindGroupByName(groupName string) (*Group, error) {
+	for _, group := range a.Groups {
+		if group.Name == groupName {
+			return group, nil
+		}
+	}
+	return nil, status.Errorf(status.NotFound, "group %s not found", groupName)
+}
+
 // FindSetupKey looks for a given SetupKey in the Account or returns error if it wasn't found.
 func (a *Account) FindSetupKey(setupKey string) (*SetupKey, error) {
 	key := a.SetupKeys[setupKey]

management/server/group.go

@@ -3,6 +3,7 @@ package server
 import (
 	"fmt"
 
+	"github.com/rs/xid"
 	log "github.com/sirupsen/logrus"
 
 	"github.com/netbirdio/netbird/management/server/activity"
@@ -18,6 +19,12 @@ func (e *GroupLinkError) Error() string {
 	return fmt.Sprintf("group has been linked to %s: %s", e.Resource, e.Name)
 }
 
+const (
+	GroupIssuedAPI         = "api"
+	GroupIssuedJWT         = "jwt"
+	GroupIssuedIntegration = "integration"
+)
+
 // Group of the peers for ACL
 type Group struct {
 	// ID of the group
@@ -29,7 +36,7 @@ type Group struct {
 	// Name visible in the UI
 	Name string
 
-	// Issued of the group
+	// Issued defines how this group was created (enum of "api", "integration" or "jwt")
 	Issued string
 
 	// Peers list of the group
@@ -116,6 +123,29 @@ func (am *DefaultAccountManager) SaveGroup(accountID, userID string, newGroup *G
 		return err
 	}
 
+	if newGroup.ID == "" && newGroup.Issued != GroupIssuedAPI {
+		return status.Errorf(status.InvalidArgument, "%s group without ID set", newGroup.Issued)
+	}
+
+	if newGroup.ID == "" && newGroup.Issued == GroupIssuedAPI {
+
+		existingGroup, err := account.FindGroupByName(newGroup.Name)
+		if err != nil {
+			s, ok := status.FromError(err)
+			if !ok || s.ErrorType != status.NotFound {
+				return err
+			}
+		}
+
+		// avoid duplicate groups only for the API issued groups. Integration or JWT groups can be duplicated as they are
+		// coming from the IdP that we don't have control of.
+		if existingGroup != nil {
+			return status.Errorf(status.AlreadyExists, "group with name %s already exists", newGroup.Name)
+		}
+
+		newGroup.ID = xid.New().String()
+	}
+
 	for _, peerID := range newGroup.Peers {
 		if account.Peers[peerID] == nil {
 			return status.Errorf(status.InvalidArgument, "peer with ID \"%s\" not found", peerID)

management/server/group_test.go

@@ -13,6 +13,41 @@ const (
 	groupAdminUserID = "testingAdminUser"
 )
 
+func TestDefaultAccountManager_CreateGroup(t *testing.T) {
+	am, err := createManager(t)
+	if err != nil {
+		t.Error("failed to create account manager")
+	}
+
+	account, err := initTestGroupAccount(am)
+	if err != nil {
+		t.Error("failed to init testing account")
+	}
+	for _, group := range account.Groups {
+		group.Issued = GroupIssuedIntegration
+		err = am.SaveGroup(account.Id, groupAdminUserID, group)
+		if err != nil {
+			t.Errorf("should allow to create %s groups", GroupIssuedIntegration)
+		}
+	}
+
+	for _, group := range account.Groups {
+		group.Issued = GroupIssuedJWT
+		err = am.SaveGroup(account.Id, groupAdminUserID, group)
+		if err != nil {
+			t.Errorf("should allow to create %s groups", GroupIssuedJWT)
+		}
+	}
+	for _, group := range account.Groups {
+		group.Issued = GroupIssuedAPI
+		group.ID = ""
+		err = am.SaveGroup(account.Id, groupAdminUserID, group)
+		if err == nil {
+			t.Errorf("should not create api group with the same name, %s", group.Name)
+		}
+	}
+}
+
 func TestDefaultAccountManager_DeleteGroup(t *testing.T) {
 	am, err := createManager(t)
 	if err != nil {
@@ -137,7 +172,7 @@ func initTestGroupAccount(am *DefaultAccountManager) (*Account, error) {
 	groupForIntegration := &Group{
 		ID:        "grp-for-integration",
 		AccountID: "account-id",
-		Name:      "Group for users",
+		Name:      "Group for users integration",
 		Issued:    GroupIssuedIntegration,
 		Peers:     make([]string, 0),
 	}

management/server/http/api/openapi.yml

@@ -585,7 +585,10 @@ components:
           type: integer
           example: 2
         issued:
-          description: How group was issued by API or from JWT token
+          description: How the group was issued (api, integration, jwt)
+          type: string
+          enum: ["api", "integration", "jwt"]
+          example: api
           type: string
           example: api
       required:
@@ -1246,7 +1249,7 @@ paths:
   /api/accounts/{accountId}:
     delete:
       summary: Delete an Account
-      description: Deletes an account and all its resources. Only administrators and account owners can delete accounts.
+      description: Deletes an account and all its resources. Only account owners can delete accounts.
       tags: [ Accounts ]
       security:
         - BearerAuth: [ ]

management/server/http/groups_handler.go

@@ -8,8 +8,6 @@ import (
 	"github.com/netbirdio/netbird/management/server/http/util"
 	"github.com/netbirdio/netbird/management/server/status"
 
-	"github.com/rs/xid"
-
 	"github.com/netbirdio/netbird/management/server"
 	"github.com/netbirdio/netbird/management/server/jwtclaims"
 
@@ -151,7 +149,6 @@ func (h *GroupsHandler) CreateGroup(w http.ResponseWriter, r *http.Request) {
 		peers = *req.Peers
 	}
 	group := server.Group{
-		ID:     xid.New().String(),
 		Name:   req.Name,
 		Peers:  peers,
 		Issued: server.GroupIssuedAPI,