diff --git a/management/server/account.go b/management/server/account.go index 442066bf9..dc8f4d0fd 100644 --- a/management/server/account.go +++ b/management/server/account.go @@ -629,8 +629,8 @@ func (a *Account) GetPeer(peerID string) *Peer { return a.Peers[peerID] } -// AddJWTGroups to account and to user autoassigned groups -func (a *Account) AddJWTGroups(userID string, groups []string) bool { +// SetJWTGroups to account and to user autoassigned groups +func (a *Account) SetJWTGroups(userID string, groupsNames []string) bool { user, ok := a.Users[userID] if !ok { return false @@ -641,13 +641,21 @@ func (a *Account) AddJWTGroups(userID string, groups []string) bool { existedGroupsByName[group.Name] = group } - autoGroups := make(map[string]struct{}) - for _, groupID := range user.AutoGroups { - autoGroups[groupID] = struct{}{} + // remove JWT groups from the autogroups, to sync them again + removed := 0 + jwtAutoGroups := make(map[string]struct{}) + for i, id := range user.AutoGroups { + if group, ok := a.Groups[id]; ok && group.Issued == GroupIssuedJWT { + jwtAutoGroups[group.Name] = struct{}{} + user.AutoGroups = append(user.AutoGroups[:i-removed], user.AutoGroups[i-removed+1:]...) + removed++ + } } + // create JWT groups if they doesn't exist + // and all of them to the autogroups var modified bool - for _, name := range groups { + for _, name := range groupsNames { group, ok := existedGroupsByName[name] if !ok { group = &Group{ @@ -656,16 +664,22 @@ func (a *Account) AddJWTGroups(userID string, groups []string) bool { Issued: GroupIssuedJWT, } a.Groups[group.ID] = group - modified = true } - if _, ok := autoGroups[group.ID]; !ok { - if group.Issued == GroupIssuedJWT { - user.AutoGroups = append(user.AutoGroups, group.ID) + // only JWT groups will be synced + if group.Issued == GroupIssuedJWT { + user.AutoGroups = append(user.AutoGroups, group.ID) + if _, ok := jwtAutoGroups[name]; !ok { modified = true } + delete(jwtAutoGroups, name) } } + // if not empty it means we removed some groups + if len(jwtAutoGroups) > 0 { + modified = true + } + return modified } @@ -1358,23 +1372,58 @@ func (am *DefaultAccountManager) GetAccountFromToken(claims jwtclaims.Authorizat } if claim, ok := claims.Raw[account.Settings.JWTGroupsClaimName]; ok { if slice, ok := claim.([]interface{}); ok { - var groups []string + var groupsNames []string for _, item := range slice { if g, ok := item.(string); ok { - groups = append(groups, g) + groupsNames = append(groupsNames, g) } else { log.Errorf("JWT claim %q is not a string: %v", account.Settings.JWTGroupsClaimName, item) } } + + oldGroups := make([]string, len(user.AutoGroups)) + copy(oldGroups, user.AutoGroups) // if groups were added or modified, save the account - if account.AddJWTGroups(claims.UserId, groups) { + if account.SetJWTGroups(claims.UserId, groupsNames) { if account.Settings.GroupsPropagationEnabled { if user, err := account.FindUser(claims.UserId); err == nil { - account.UserGroupsAddToPeers(claims.UserId, append(user.AutoGroups, groups...)...) + addNewGroups := difference(user.AutoGroups, oldGroups) + removeOldGroups := difference(oldGroups, user.AutoGroups) + account.UserGroupsAddToPeers(claims.UserId, addNewGroups...) + account.UserGroupsRemoveFromPeers(claims.UserId, removeOldGroups...) + account.Network.IncSerial() + if err := am.Store.SaveAccount(account); err != nil { + log.Errorf("failed to save account: %v", err) + } else { + if err := am.updateAccountPeers(account); err != nil { + log.Errorf("failed updating account peers while updating user %s", account.Id) + } + for _, g := range addNewGroups { + if group := account.GetGroup(g); group != nil { + am.storeEvent(user.Id, user.Id, account.Id, activity.GroupAddedToUser, + map[string]any{ + "group": group.Name, + "group_id": group.ID, + "is_service_user": user.IsServiceUser, + "user_name": user.ServiceUserName}) + } + } + for _, g := range removeOldGroups { + if group := account.GetGroup(g); group != nil { + am.storeEvent(user.Id, user.Id, account.Id, activity.GroupRemovedFromUser, + map[string]any{ + "group": group.Name, + "group_id": group.ID, + "is_service_user": user.IsServiceUser, + "user_name": user.ServiceUserName}) + } + } + } + } + } else { + if err := am.Store.SaveAccount(account); err != nil { + log.Errorf("failed to save account: %v", err) } - } - if err := am.Store.SaveAccount(account); err != nil { - log.Errorf("failed to save account: %v", err) } } } else { diff --git a/management/server/account_test.go b/management/server/account_test.go index 828fa8536..119828e20 100644 --- a/management/server/account_test.go +++ b/management/server/account_test.go @@ -1930,7 +1930,7 @@ func TestAccount_GetNextPeerExpiration(t *testing.T) { } } -func TestAccount_AddJWTGroups(t *testing.T) { +func TestAccount_SetJWTGroups(t *testing.T) { // create a new account account := &Account{ Peers: map[string]*Peer{ @@ -1951,13 +1951,13 @@ func TestAccount_AddJWTGroups(t *testing.T) { } t.Run("api group already exists", func(t *testing.T) { - updated := account.AddJWTGroups("user1", []string{"group1"}) + updated := account.SetJWTGroups("user1", []string{"group1"}) assert.False(t, updated, "account should not be updated") assert.Empty(t, account.Users["user1"].AutoGroups, "auto groups must be empty") }) t.Run("add jwt group", func(t *testing.T) { - updated := account.AddJWTGroups("user1", []string{"group1", "group2"}) + updated := account.SetJWTGroups("user1", []string{"group1", "group2"}) assert.True(t, updated, "account should be updated") assert.Len(t, account.Groups, 2, "new group should be added") assert.Len(t, account.Users["user1"].AutoGroups, 1, "new group should be added") @@ -1965,13 +1965,13 @@ func TestAccount_AddJWTGroups(t *testing.T) { }) t.Run("existed group not update", func(t *testing.T) { - updated := account.AddJWTGroups("user1", []string{"group2"}) + updated := account.SetJWTGroups("user1", []string{"group2"}) assert.False(t, updated, "account should not be updated") assert.Len(t, account.Groups, 2, "groups count should not be changed") }) t.Run("add new group", func(t *testing.T) { - updated := account.AddJWTGroups("user2", []string{"group1", "group3"}) + updated := account.SetJWTGroups("user2", []string{"group1", "group3"}) assert.True(t, updated, "account should be updated") assert.Len(t, account.Groups, 3, "new group should be added") assert.Len(t, account.Users["user2"].AutoGroups, 1, "new group should be added") diff --git a/management/server/user.go b/management/server/user.go index 0b61dd60a..19cffb840 100644 --- a/management/server/user.go +++ b/management/server/user.go @@ -610,10 +610,19 @@ func (am *DefaultAccountManager) SaveUser(accountID, initiatorUserID string, upd // need force update all auto groups in any case they will not be dublicated account.UserGroupsAddToPeers(oldUser.Id, update.AutoGroups...) account.UserGroupsRemoveFromPeers(oldUser.Id, removedGroups...) - } - if err = am.Store.SaveAccount(account); err != nil { - return nil, err + account.Network.IncSerial() + if err = am.Store.SaveAccount(account); err != nil { + return nil, err + } + + if err := am.updateAccountPeers(account); err != nil { + log.Errorf("failed updating account peers while updating user %s", accountID) + } + } else { + if err = am.Store.SaveAccount(account); err != nil { + return nil, err + } } defer func() { @@ -641,7 +650,6 @@ func (am *DefaultAccountManager) SaveUser(accountID, initiatorUserID string, upd } else { log.Errorf("group %s not found while saving user activity event of account %s", g, account.Id) } - } for _, g := range addedGroups {