From e5e69b1f7543d4da1603a65029cfca8a0b6ecfa3 Mon Sep 17 00:00:00 2001 From: Givi Khojanashvili Date: Mon, 7 Aug 2023 19:44:51 +0400 Subject: [PATCH] Autopropagate peers by JWT groups (#1037) Enhancements to Peer Group Assignment: 1. Auto-assigned groups are now applied to all peers every time a user logs into the network. 2. Feature activation is available in the account settings. 3. API modifications included to support these changes for account settings updates. 4. If propagation is enabled, updates to a user's auto-assigned groups are immediately reflected across all user peers. 5. With the JWT group sync feature active, auto-assigned groups are forcefully updated whenever a peer logs in using user credentials. --- management/server/account.go | 111 ++++++++++++++--- management/server/account_test.go | 115 +++++++++++++++++- management/server/http/accounts_handler.go | 5 +- .../server/http/accounts_handler_test.go | 22 +++- management/server/http/api/openapi.yml | 4 + management/server/http/api/types.gen.go | 3 + management/server/user.go | 9 +- 7 files changed, 246 insertions(+), 23 deletions(-) diff --git a/management/server/account.go b/management/server/account.go index 68302d2ba..7efcd2f1e 100644 --- a/management/server/account.go +++ b/management/server/account.go @@ -139,10 +139,14 @@ type DefaultAccountManager struct { type Settings struct { // PeerLoginExpirationEnabled globally enables or disables peer login expiration PeerLoginExpirationEnabled bool + // PeerLoginExpiration is a setting that indicates when peer login expires. // Applies to all peers that have Peer.LoginExpirationEnabled set to true. PeerLoginExpiration time.Duration + // GroupsPropagationEnabled allows to propagate auto groups from the user to the peer + GroupsPropagationEnabled bool + // JWTGroupsEnabled allows extract groups from JWT claim, which name defined in the JWTGroupsClaimName // and add it to account groups. JWTGroupsEnabled bool @@ -158,6 +162,7 @@ func (s *Settings) Copy() *Settings { PeerLoginExpiration: s.PeerLoginExpiration, JWTGroupsEnabled: s.JWTGroupsEnabled, JWTGroupsClaimName: s.JWTGroupsClaimName, + GroupsPropagationEnabled: s.GroupsPropagationEnabled, } } @@ -624,26 +629,96 @@ func (a *Account) GetPeer(peerID string) *Peer { return a.Peers[peerID] } -// AddJWTGroups to existed groups if they does not exists -func (a *Account) AddJWTGroups(groups []string) (int, error) { - existedGroups := make(map[string]*Group) - for _, g := range a.Groups { - existedGroups[g.Name] = g +// AddJWTGroups to account and to user autoassigned groups +func (a *Account) AddJWTGroups(userID string, groups []string) bool { + user, ok := a.Users[userID] + if !ok { + return false } - var count int + existedGroupsByName := make(map[string]*Group) + for _, group := range a.Groups { + existedGroupsByName[group.Name] = group + } + + autoGroups := make(map[string]struct{}) + for _, groupID := range user.AutoGroups { + autoGroups[groupID] = struct{}{} + } + + var modified bool for _, name := range groups { - if _, ok := existedGroups[name]; !ok { - id := xid.New().String() - a.Groups[id] = &Group{ - ID: id, + group, ok := existedGroupsByName[name] + if !ok { + group = &Group{ + ID: xid.New().String(), Name: name, Issued: GroupIssuedJWT, } - count++ + a.Groups[group.ID] = group + modified = true + } + if _, ok := autoGroups[group.ID]; !ok { + if group.Issued == GroupIssuedJWT { + user.AutoGroups = append(user.AutoGroups, group.ID) + modified = true + } } } - return count, nil + + return modified +} + +// UserGroupsAddToPeers adds groups to all peers of user +func (a *Account) UserGroupsAddToPeers(userID string, groups ...string) { + userPeers := make(map[string]struct{}) + for pid, peer := range a.Peers { + if peer.UserID == userID { + userPeers[pid] = struct{}{} + } + } + + for _, gid := range groups { + group, ok := a.Groups[gid] + if !ok { + continue + } + + groupPeers := make(map[string]struct{}) + for _, pid := range group.Peers { + groupPeers[pid] = struct{}{} + } + + for pid := range userPeers { + groupPeers[pid] = struct{}{} + } + + group.Peers = group.Peers[:0] + for pid := range groupPeers { + group.Peers = append(group.Peers, pid) + } + } +} + +// UserGroupsRemoveFromPeers removes groups from all peers of user +func (a *Account) UserGroupsRemoveFromPeers(userID string, groups ...string) { + for _, gid := range groups { + group, ok := a.Groups[gid] + if !ok { + continue + } + update := make([]string, 0, len(group.Peers)) + for _, pid := range group.Peers { + peer, ok := a.Peers[pid] + if !ok { + continue + } + if peer.UserID != userID { + update = append(update, pid) + } + } + group.Peers = update + } } // BuildManager creates a new DefaultAccountManager with a provided Store @@ -1290,11 +1365,13 @@ func (am *DefaultAccountManager) GetAccountFromToken(claims jwtclaims.Authorizat log.Errorf("JWT claim %q is not a string: %v", account.Settings.JWTGroupsClaimName, item) } } - n, err := account.AddJWTGroups(groups) - if err != nil { - log.Errorf("failed to add JWT groups: %v", err) - } - if n > 0 { + // if groups were added or modified, save the account + if account.AddJWTGroups(claims.UserId, groups) { + if account.Settings.GroupsPropagationEnabled { + if user, err := account.FindUser(claims.UserId); err == nil { + account.UserGroupsAddToPeers(claims.UserId, append(user.AutoGroups, groups...)...) + } + } if err := am.Store.SaveAccount(account); err != nil { log.Errorf("failed to save account: %v", err) } diff --git a/management/server/account_test.go b/management/server/account_test.go index 12458f57a..828fa8536 100644 --- a/management/server/account_test.go +++ b/management/server/account_test.go @@ -216,7 +216,6 @@ func TestAccount_GetPeerNetworkMap(t *testing.T) { assert.Len(t, networkMap.Peers, len(testCase.expectedPeers)) assert.Len(t, networkMap.OfflinePeers, len(testCase.expectedOfflinePeers)) } - } func TestNewAccount(t *testing.T) { @@ -1931,6 +1930,120 @@ func TestAccount_GetNextPeerExpiration(t *testing.T) { } } +func TestAccount_AddJWTGroups(t *testing.T) { + // create a new account + account := &Account{ + Peers: map[string]*Peer{ + "peer1": {ID: "peer1", Key: "key1", UserID: "user1"}, + "peer2": {ID: "peer2", Key: "key2", UserID: "user1"}, + "peer3": {ID: "peer3", Key: "key3", UserID: "user1"}, + "peer4": {ID: "peer4", Key: "key4", UserID: "user2"}, + "peer5": {ID: "peer5", Key: "key5", UserID: "user2"}, + }, + Groups: map[string]*Group{ + "group1": {ID: "group1", Name: "group1", Issued: GroupIssuedAPI, Peers: []string{}}, + }, + Settings: &Settings{GroupsPropagationEnabled: true}, + Users: map[string]*User{ + "user1": {Id: "user1"}, + "user2": {Id: "user2"}, + }, + } + + t.Run("api group already exists", func(t *testing.T) { + updated := account.AddJWTGroups("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"}) + 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") + assert.Contains(t, account.Groups, account.Users["user1"].AutoGroups[0], "groups must contain group2 from user groups") + }) + + t.Run("existed group not update", func(t *testing.T) { + updated := account.AddJWTGroups("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"}) + 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") + assert.Contains(t, account.Groups, account.Users["user2"].AutoGroups[0], "groups must contain group3 from user groups") + }) +} + +func TestAccount_UserGroupsAddToPeers(t *testing.T) { + account := &Account{ + Peers: map[string]*Peer{ + "peer1": {ID: "peer1", Key: "key1", UserID: "user1"}, + "peer2": {ID: "peer2", Key: "key2", UserID: "user1"}, + "peer3": {ID: "peer3", Key: "key3", UserID: "user1"}, + "peer4": {ID: "peer4", Key: "key4", UserID: "user2"}, + "peer5": {ID: "peer5", Key: "key5", UserID: "user2"}, + }, + Groups: map[string]*Group{ + "group1": {ID: "group1", Name: "group1", Issued: GroupIssuedAPI, Peers: []string{}}, + "group2": {ID: "group2", Name: "group2", Issued: GroupIssuedAPI, Peers: []string{}}, + "group3": {ID: "group3", Name: "group3", Issued: GroupIssuedAPI, Peers: []string{}}, + }, + Users: map[string]*User{"user1": {Id: "user1"}, "user2": {Id: "user2"}}, + } + + t.Run("add groups", func(t *testing.T) { + account.UserGroupsAddToPeers("user1", "group1", "group2") + assert.ElementsMatch(t, account.Groups["group1"].Peers, []string{"peer1", "peer2", "peer3"}, "group1 contains users peers") + assert.ElementsMatch(t, account.Groups["group2"].Peers, []string{"peer1", "peer2", "peer3"}, "group2 contains users peers") + }) + + t.Run("add same groups", func(t *testing.T) { + account.UserGroupsAddToPeers("user1", "group1", "group2") + assert.Len(t, account.Groups["group1"].Peers, 3, "peers amount in group1 didn't change") + assert.Len(t, account.Groups["group2"].Peers, 3, "peers amount in group2 didn't change") + }) + + t.Run("add second user peers", func(t *testing.T) { + account.UserGroupsAddToPeers("user2", "group2") + assert.ElementsMatch(t, account.Groups["group2"].Peers, + []string{"peer1", "peer2", "peer3", "peer4", "peer5"}, "group2 contains first and second user peers") + }) +} + +func TestAccount_UserGroupsRemoveFromPeers(t *testing.T) { + account := &Account{ + Peers: map[string]*Peer{ + "peer1": {ID: "peer1", Key: "key1", UserID: "user1"}, + "peer2": {ID: "peer2", Key: "key2", UserID: "user1"}, + "peer3": {ID: "peer3", Key: "key3", UserID: "user1"}, + "peer4": {ID: "peer4", Key: "key4", UserID: "user2"}, + "peer5": {ID: "peer5", Key: "key5", UserID: "user2"}, + }, + Groups: map[string]*Group{ + "group1": {ID: "group1", Name: "group1", Issued: GroupIssuedAPI, Peers: []string{"peer1", "peer2", "peer3"}}, + "group2": {ID: "group2", Name: "group2", Issued: GroupIssuedAPI, Peers: []string{"peer1", "peer2", "peer3", "peer4", "peer5"}}, + "group3": {ID: "group3", Name: "group3", Issued: GroupIssuedAPI, Peers: []string{"peer4", "peer5"}}, + }, + Users: map[string]*User{"user1": {Id: "user1"}, "user2": {Id: "user2"}}, + } + + t.Run("remove groups", func(t *testing.T) { + account.UserGroupsRemoveFromPeers("user1", "group1", "group2") + assert.Empty(t, account.Groups["group1"].Peers, "remove all peers from group1") + assert.ElementsMatch(t, account.Groups["group2"].Peers, []string{"peer4", "peer5"}, "group2 contains only second users peers") + }) + + t.Run("remove group with no peers", func(t *testing.T) { + account.UserGroupsRemoveFromPeers("user1", "group3") + assert.Len(t, account.Groups["group3"].Peers, 2, "peers amount should not change") + }) +} + func createManager(t *testing.T) (*DefaultAccountManager, error) { store, err := createStore(t) if err != nil { diff --git a/management/server/http/accounts_handler.go b/management/server/http/accounts_handler.go index d94bea3fb..a5d7a9501 100644 --- a/management/server/http/accounts_handler.go +++ b/management/server/http/accounts_handler.go @@ -80,12 +80,14 @@ func (h *AccountsHandler) UpdateAccount(w http.ResponseWriter, r *http.Request) if req.Settings.JwtGroupsEnabled != nil { settings.JWTGroupsEnabled = *req.Settings.JwtGroupsEnabled } + if req.Settings.GroupsPropagationEnabled != nil { + settings.GroupsPropagationEnabled = *req.Settings.GroupsPropagationEnabled + } if req.Settings.JwtGroupsClaimName != nil { settings.JWTGroupsClaimName = *req.Settings.JwtGroupsClaimName } updatedAccount, err := h.accountManager.UpdateAccountSettings(accountID, user.Id, settings) - if err != nil { util.WriteError(err, w) return @@ -102,6 +104,7 @@ func toAccountResponse(account *server.Account) *api.Account { Settings: api.AccountSettings{ PeerLoginExpiration: int(account.Settings.PeerLoginExpiration.Seconds()), PeerLoginExpirationEnabled: account.Settings.PeerLoginExpirationEnabled, + GroupsPropagationEnabled: &account.Settings.GroupsPropagationEnabled, JwtGroupsEnabled: &account.Settings.JWTGroupsEnabled, JwtGroupsClaimName: &account.Settings.JWTGroupsClaimName, }, diff --git a/management/server/http/accounts_handler_test.go b/management/server/http/accounts_handler_test.go index 5051f45e1..3b257a703 100644 --- a/management/server/http/accounts_handler_test.go +++ b/management/server/http/accounts_handler_test.go @@ -38,7 +38,6 @@ func initAccountsTestData(account *server.Account, admin *server.User) *Accounts accCopy := account.Copy() accCopy.UpdateSettings(newSettings) return accCopy, nil - }, }, claimsExtractor: jwtclaims.NewClaimsExtractor( @@ -54,7 +53,6 @@ func initAccountsTestData(account *server.Account, admin *server.User) *Accounts } func TestAccounts_AccountsHandler(t *testing.T) { - accountID := "test_account" adminUser := server.NewAdminUser("test_user") @@ -94,6 +92,7 @@ func TestAccounts_AccountsHandler(t *testing.T) { expectedSettings: api.AccountSettings{ PeerLoginExpiration: int(time.Hour.Seconds()), PeerLoginExpirationEnabled: false, + GroupsPropagationEnabled: br(false), JwtGroupsClaimName: sr(""), JwtGroupsEnabled: br(false), }, @@ -110,6 +109,7 @@ func TestAccounts_AccountsHandler(t *testing.T) { expectedSettings: api.AccountSettings{ PeerLoginExpiration: 15552000, PeerLoginExpirationEnabled: true, + GroupsPropagationEnabled: br(false), JwtGroupsClaimName: sr(""), JwtGroupsEnabled: br(false), }, @@ -126,12 +126,30 @@ func TestAccounts_AccountsHandler(t *testing.T) { expectedSettings: api.AccountSettings{ PeerLoginExpiration: 15552000, PeerLoginExpirationEnabled: false, + GroupsPropagationEnabled: br(false), JwtGroupsClaimName: sr("roles"), JwtGroupsEnabled: br(true), }, expectedArray: false, expectedID: accountID, }, + { + name: "PutAccount OK wiht JWT Propagation", + expectedBody: true, + requestType: http.MethodPut, + requestPath: "/api/accounts/" + accountID, + requestBody: bytes.NewBufferString("{\"settings\": {\"peer_login_expiration\": 554400,\"peer_login_expiration_enabled\": true,\"jwt_groups_enabled\":true,\"jwt_groups_claim_name\":\"groups\",\"groups_propagation_enabled\":true}}"), + expectedStatus: http.StatusOK, + expectedSettings: api.AccountSettings{ + PeerLoginExpiration: 554400, + PeerLoginExpirationEnabled: true, + GroupsPropagationEnabled: br(true), + JwtGroupsClaimName: sr("groups"), + JwtGroupsEnabled: br(true), + }, + expectedArray: false, + expectedID: accountID, + }, { name: "Update account failure with high peer_login_expiration more than 180 days", expectedBody: true, diff --git a/management/server/http/api/openapi.yml b/management/server/http/api/openapi.yml index 0f9a67aef..2b18bc295 100644 --- a/management/server/http/api/openapi.yml +++ b/management/server/http/api/openapi.yml @@ -54,6 +54,10 @@ components: description: Period of time after which peer login expires (seconds). type: integer example: 43200 + groups_propagation_enabled: + description: Allows propagate the new user auto groups to peers that belongs to the user + type: boolean + example: true jwt_groups_enabled: description: Allows extract groups from JWT claim and add it to account groups. type: boolean diff --git a/management/server/http/api/types.gen.go b/management/server/http/api/types.gen.go index 3f58b0c70..c11ed9efa 100644 --- a/management/server/http/api/types.gen.go +++ b/management/server/http/api/types.gen.go @@ -129,6 +129,9 @@ type AccountRequest struct { // AccountSettings defines model for AccountSettings. type AccountSettings struct { + // GroupsPropagationEnabled Allows propagate the new user auto groups to peers that belongs to the user + GroupsPropagationEnabled *bool `json:"groups_propagation_enabled,omitempty"` + // JwtGroupsClaimName Name of the claim from which we extract groups names to add it to account groups. JwtGroupsClaimName *string `json:"jwt_groups_claim_name,omitempty"` diff --git a/management/server/user.go b/management/server/user.go index d30a8fa9e..34a879328 100644 --- a/management/server/user.go +++ b/management/server/user.go @@ -260,7 +260,6 @@ func (am *DefaultAccountManager) inviteNewUser(accountID, userID string, invite am.storeEvent(userID, newUser.Id, accountID, activity.UserInvited, nil) return newUser.ToUserInfo(idpUser) - } // GetUser looks up a user by provided authorization claims. @@ -600,6 +599,13 @@ func (am *DefaultAccountManager) SaveUser(accountID, initiatorUserID string, upd } } + if update.AutoGroups != nil && account.Settings.GroupsPropagationEnabled { + removedGroups := difference(oldUser.AutoGroups, update.AutoGroups) + // 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 } @@ -640,7 +646,6 @@ func (am *DefaultAccountManager) SaveUser(accountID, initiatorUserID string, upd } } } - }() if !isNil(am.idpManager) && !newUser.IsServiceUser {