From 3ac32fd78abb67139a6b978637490b9cd32bdcf0 Mon Sep 17 00:00:00 2001 From: Givi Khojanashvili Date: Fri, 18 Aug 2023 17:36:05 +0400 Subject: [PATCH] Send network update when propagate user auto-groups (#1084) For peer propagation this commit triggers network map update in two cases: 1) peer login 2) user AutoGroups update Also it issues new activity message about new user group for peer login process. Previous implementation only adds JWT groups to user. This fix also removes JWT groups from user auto assign groups. Pelase note, it also happen when user works with dashboard. --- management/server/account.go | 83 ++++++++++++++++++++++++------- management/server/account_test.go | 10 ++-- management/server/user.go | 16 ++++-- 3 files changed, 83 insertions(+), 26 deletions(-) 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 {