diff --git a/management/server/account.go b/management/server/account.go index a800ea97d..8b326d93a 100644 --- a/management/server/account.go +++ b/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] diff --git a/management/server/group.go b/management/server/group.go index be8d3fb0e..43d48e622 100644 --- a/management/server/group.go +++ b/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) diff --git a/management/server/group_test.go b/management/server/group_test.go index e2051a656..3a2195c88 100644 --- a/management/server/group_test.go +++ b/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), } diff --git a/management/server/http/api/openapi.yml b/management/server/http/api/openapi.yml index f4c8910bc..7ec2310af 100644 --- a/management/server/http/api/openapi.yml +++ b/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: [ ] diff --git a/management/server/http/groups_handler.go b/management/server/http/groups_handler.go index c06445690..b37f4fd2f 100644 --- a/management/server/http/groups_handler.go +++ b/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,