Skip to content

Commit

Permalink
Check only new groups created via API
Browse files Browse the repository at this point in the history
  • Loading branch information
mlsmaycon committed Mar 15, 2024
1 parent 42d72d1 commit cf4e412
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 15 deletions.
30 changes: 20 additions & 10 deletions management/server/group.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package server
import (
"fmt"

"github.com/rs/xid"
log "github.com/sirupsen/logrus"

"github.com/netbirdio/netbird/management/server/activity"
Expand Down Expand Up @@ -122,18 +123,27 @@ func (am *DefaultAccountManager) SaveGroup(accountID, userID string, newGroup *G
return err
}

group, err := account.FindGroupByName(newGroup.Name)
if err != nil {
s, ok := status.FromError(err)
if !ok || s.ErrorType != status.NotFound {
return err
}
if newGroup.ID == "" && newGroup.Issued == GroupIssuedAPI {
return status.Errorf(status.InvalidArgument, "%s group without ID set", newGroup.Issued)
}

// 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 group != nil && group.Issued == GroupIssuedAPI {
return status.Errorf(status.AlreadyExists, "group with name %s already exists", newGroup.Name)
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 {
Expand Down
22 changes: 20 additions & 2 deletions management/server/group_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,27 @@ func TestDefaultAccountManager_CreateGroup(t *testing.T) {
}

for _, group := range account.Groups {
group.Issued = GroupIssuedAPI
group.ID = ""
err = am.SaveGroup(account.Id, groupAdminUserID, group)
if err == nil && group.Issued == GroupIssuedAPI {
t.Error("should not create group with the same name")
if err == nil {
t.Errorf("should not create api group with the same name, %s", group.Name)
}
}

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)
}
}
}
Expand Down
3 changes: 0 additions & 3 deletions management/server/http/groups_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -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,
Expand Down

0 comments on commit cf4e412

Please sign in to comment.