Add batch delete for groups and users (#2370)

* Refactor user deletion logic and introduce batch delete

* Prevent self-deletion for users

* Add delete multiple groups

* Refactor group deletion with validation

* Fix tests

* Add bulk delete functions for Users and Groups in account manager interface and mocks

* Add tests for DeleteGroups method in group management

* Add tests for DeleteUsers method in users management
This commit is contained in:
Bethuel Mmbaga 2024-08-08 18:01:38 +03:00 committed by GitHub
parent bcce1bf184
commit 0911163146
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 575 additions and 126 deletions

View File

@ -68,6 +68,7 @@ type AccountManager interface {
SaveSetupKey(ctx context.Context, accountID string, key *SetupKey, userID string) (*SetupKey, error) SaveSetupKey(ctx context.Context, accountID string, key *SetupKey, userID string) (*SetupKey, error)
CreateUser(ctx context.Context, accountID, initiatorUserID string, key *UserInfo) (*UserInfo, error) CreateUser(ctx context.Context, accountID, initiatorUserID string, key *UserInfo) (*UserInfo, error)
DeleteUser(ctx context.Context, accountID, initiatorUserID string, targetUserID string) 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 InviteUser(ctx context.Context, accountID string, initiatorUserID string, targetUserID string) error
ListSetupKeys(ctx context.Context, accountID, userID string) ([]*SetupKey, error) ListSetupKeys(ctx context.Context, accountID, userID string) ([]*SetupKey, error)
SaveUser(ctx context.Context, accountID, initiatorUserID string, update *User) (*UserInfo, 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 SaveGroup(ctx context.Context, accountID, userID string, group *nbgroup.Group) error
SaveGroups(ctx context.Context, accountID, userID string, newGroups []*nbgroup.Group) error SaveGroups(ctx context.Context, accountID, userID string, newGroups []*nbgroup.Group) error
DeleteGroup(ctx context.Context, accountId, userId, groupID string) 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) ListGroups(ctx context.Context, accountId string) ([]*nbgroup.Group, error)
GroupAddPeer(ctx context.Context, accountId, groupID, peerID string) error GroupAddPeer(ctx context.Context, accountId, groupID, peerID string) error
GroupDeletePeer(ctx context.Context, accountId, groupID, peerID string) error GroupDeletePeer(ctx context.Context, accountId, groupID, peerID string) error

View File

@ -2,8 +2,12 @@ package server
import ( import (
"context" "context"
"errors"
"fmt" "fmt"
"slices"
nbdns "github.com/netbirdio/netbird/dns"
"github.com/netbirdio/netbird/route"
"github.com/rs/xid" "github.com/rs/xid"
log "github.com/sirupsen/logrus" log "github.com/sirupsen/logrus"
@ -243,7 +247,7 @@ func difference(a, b []string) []string {
return diff return diff
} }
// DeleteGroup object of the peers // DeleteGroup object of the peers.
func (am *DefaultAccountManager) DeleteGroup(ctx context.Context, accountId, userId, groupID string) error { func (am *DefaultAccountManager) DeleteGroup(ctx context.Context, accountId, userId, groupID string) error {
unlock := am.Store.AcquireWriteLockByUID(ctx, accountId) unlock := am.Store.AcquireWriteLockByUID(ctx, accountId)
defer unlock() defer unlock()
@ -253,96 +257,14 @@ func (am *DefaultAccountManager) DeleteGroup(ctx context.Context, accountId, use
return err return err
} }
g, ok := account.Groups[groupID] group, ok := account.Groups[groupID]
if !ok { if !ok {
return nil return nil
} }
// disable a deleting integration group if the initiator is not an admin service user if err = validateDeleteGroup(account, group, userId); err != nil {
if g.Issued == nbgroup.GroupIssuedIntegration { return err
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) delete(account.Groups, groupID)
account.Network.IncSerial() account.Network.IncSerial()
@ -350,13 +272,57 @@ func (am *DefaultAccountManager) DeleteGroup(ctx context.Context, accountId, use
return err 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) am.updateAccountPeers(ctx, account)
return nil 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 // ListGroups objects of the peers
func (am *DefaultAccountManager) ListGroups(ctx context.Context, accountID string) ([]*nbgroup.Group, error) { func (am *DefaultAccountManager) ListGroups(ctx context.Context, accountID string) ([]*nbgroup.Group, error) {
unlock := am.Store.AcquireWriteLockByUID(ctx, accountID) unlock := am.Store.AcquireWriteLockByUID(ctx, accountID)
@ -440,3 +406,102 @@ func (am *DefaultAccountManager) GroupDeletePeer(ctx context.Context, accountID,
return nil 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
}

View File

@ -3,12 +3,14 @@ package server
import ( import (
"context" "context"
"errors" "errors"
"fmt"
"testing" "testing"
nbdns "github.com/netbirdio/netbird/dns" nbdns "github.com/netbirdio/netbird/dns"
nbgroup "github.com/netbirdio/netbird/management/server/group" nbgroup "github.com/netbirdio/netbird/management/server/group"
"github.com/netbirdio/netbird/management/server/status" "github.com/netbirdio/netbird/management/server/status"
"github.com/netbirdio/netbird/route" "github.com/netbirdio/netbird/route"
"github.com/stretchr/testify/assert"
) )
const ( const (
@ -21,7 +23,7 @@ func TestDefaultAccountManager_CreateGroup(t *testing.T) {
t.Error("failed to create account manager") t.Error("failed to create account manager")
} }
account, err := initTestGroupAccount(am) _, account, err := initTestGroupAccount(am)
if err != nil { if err != nil {
t.Error("failed to init testing account") t.Error("failed to init testing account")
} }
@ -56,7 +58,7 @@ func TestDefaultAccountManager_DeleteGroup(t *testing.T) {
t.Error("failed to create account manager") t.Error("failed to create account manager")
} }
account, err := initTestGroupAccount(am) _, account, err := initTestGroupAccount(am)
if err != nil { if err != nil {
t.Error("failed to init testing account") 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" accountID := "testingAcc"
domain := "example.com" domain := "example.com"
@ -236,7 +367,7 @@ func initTestGroupAccount(am *DefaultAccountManager) (*Account, error) {
err := am.Store.SaveAccount(context.Background(), account) err := am.Store.SaveAccount(context.Background(), account)
if err != nil { if err != nil {
return nil, err return nil, nil, err
} }
_ = am.SaveGroup(context.Background(), accountID, groupAdminUserID, groupForRoute) _ = 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, groupForUsers)
_ = am.SaveGroup(context.Background(), accountID, groupAdminUserID, groupForIntegration) _ = 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
} }

View File

@ -42,6 +42,7 @@ type MockAccountManager struct {
SaveGroupFunc func(ctx context.Context, accountID, userID string, group *group.Group) error SaveGroupFunc func(ctx context.Context, accountID, userID string, group *group.Group) error
SaveGroupsFunc func(ctx context.Context, accountID, userID string, groups []*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 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) ListGroupsFunc func(ctx context.Context, accountID string) ([]*group.Group, error)
GroupAddPeerFunc func(ctx context.Context, accountID, groupID, peerID string) error GroupAddPeerFunc func(ctx context.Context, accountID, groupID, peerID string) error
GroupDeletePeerFunc 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) 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) 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 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) 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 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) 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") 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 // ListGroups mock implementation of ListGroups from server.AccountManager interface
func (am *MockAccountManager) ListGroups(ctx context.Context, accountID string) ([]*group.Group, error) { func (am *MockAccountManager) ListGroups(ctx context.Context, accountID string) ([]*group.Group, error) {
if am.ListGroupsFunc != nil { 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") 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 { func (am *MockAccountManager) InviteUser(ctx context.Context, accountID string, initiatorUserID string, targetUserID string) error {
if am.InviteUserFunc != nil { if am.InviteUserFunc != nil {
return am.InviteUserFunc(ctx, accountID, initiatorUserID, targetUserID) return am.InviteUserFunc(ctx, accountID, initiatorUserID, targetUserID)

View File

@ -2,6 +2,7 @@ package server
import ( import (
"context" "context"
"errors"
"fmt" "fmt"
"strings" "strings"
"time" "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 { func (am *DefaultAccountManager) deleteRegularUser(ctx context.Context, account *Account, initiatorUserID, targetUserID string) error {
tuEmail, tuName, err := am.getEmailAndNameOfTargetUser(ctx, account.Id, initiatorUserID, targetUserID) meta, err := am.prepareUserDeletion(ctx, account, 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 { if err != nil {
return err 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) delete(account.Users, targetUserID)
err = am.Store.SaveAccount(ctx, account) err = am.Store.SaveAccount(ctx, account)
if err != nil { if err != nil {
return err return err
} }
meta := map[string]any{"name": tuName, "email": tuEmail, "created_at": tuCreatedAt}
am.StoreEvent(ctx, initiatorUserID, targetUserID, account.Id, activity.UserDeleted, meta) am.StoreEvent(ctx, initiatorUserID, targetUserID, account.Id, activity.UserDeleted, meta)
am.updateAccountPeers(ctx, account) am.updateAccountPeers(ctx, account)
return nil return nil
@ -1190,6 +1158,116 @@ func (am *DefaultAccountManager) getEmailAndNameOfTargetUser(ctx context.Context
return "", "", fmt.Errorf("user info not found for user: %s", targetId) 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) { func findUserInIDPUserdata(userID string, userData []*idp.UserData) (*idp.UserData, bool) {
for _, user := range userData { for _, user := range userData {
if user.ID == userID { if user.ID == userID {

View File

@ -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) { func TestDefaultAccountManager_GetUser(t *testing.T) {
store := newStore(t) store := newStore(t)
defer store.Close(context.Background()) defer store.Close(context.Background())