diff --git a/pro/controllers/users.go b/pro/controllers/users.go index ef48d1c0..44df7381 100644 --- a/pro/controllers/users.go +++ b/pro/controllers/users.go @@ -690,7 +690,7 @@ func updateUserGroup(w http.ResponseWriter, r *http.Request) { }() // reset configs for service user - go proLogic.UpdatesUserGwAccessOnGrpUpdates(currUserG.NetworkRoles, userGroup.NetworkRoles) + go proLogic.UpdatesUserGwAccessOnGrpUpdates(userGroup.ID, currUserG.NetworkRoles, userGroup.NetworkRoles) logic.ReturnSuccessResponseWithJson(w, r, userGroup, "updated user group") } @@ -781,7 +781,7 @@ func deleteUserGroup(w http.ResponseWriter, r *http.Request) { } }() - go proLogic.UpdatesUserGwAccessOnGrpUpdates(userG.NetworkRoles, make(map[models.NetworkID]map[models.UserRoleID]struct{})) + go proLogic.UpdatesUserGwAccessOnGrpUpdates(userG.ID, userG.NetworkRoles, make(map[models.NetworkID]map[models.UserRoleID]struct{})) logic.ReturnSuccessResponseWithJson(w, r, nil, "deleted user group") } diff --git a/pro/logic/user_mgmt.go b/pro/logic/user_mgmt.go index d7fc1971..d9d6af26 100644 --- a/pro/logic/user_mgmt.go +++ b/pro/logic/user_mgmt.go @@ -1079,27 +1079,14 @@ func UpdatesUserGwAccessOnRoleUpdates(currNetworkAccess, } } -func UpdatesUserGwAccessOnGrpUpdates(oldNetworkRoles, newNetworkRoles map[models.NetworkID]map[models.UserRoleID]struct{}) { - networkChangeMap := make(map[models.NetworkID]map[models.UserRoleID]struct{}) - for netID, networkUserRoles := range oldNetworkRoles { +func UpdatesUserGwAccessOnGrpUpdates(groupID models.UserGroupID, oldNetworkRoles, newNetworkRoles map[models.NetworkID]map[models.UserRoleID]struct{}) { + networkRemovedMap := make(map[models.NetworkID]struct{}) + for netID := range oldNetworkRoles { if _, ok := newNetworkRoles[netID]; !ok { - for netRoleID := range networkUserRoles { - if _, ok := networkChangeMap[netID]; !ok { - networkChangeMap[netID] = make(map[models.UserRoleID]struct{}) - } - networkChangeMap[netID][netRoleID] = struct{}{} - } - } else { - for netRoleID := range networkUserRoles { - if _, ok := newNetworkRoles[netID][netRoleID]; !ok { - if _, ok := networkChangeMap[netID]; !ok { - networkChangeMap[netID] = make(map[models.UserRoleID]struct{}) - } - networkChangeMap[netID][netRoleID] = struct{}{} - } - } + networkRemovedMap[netID] = struct{}{} } } + extclients, err := logic.GetAllExtClients() if err != nil { slog.Error("failed to fetch extclients", "error", err) @@ -1109,33 +1096,56 @@ func UpdatesUserGwAccessOnGrpUpdates(oldNetworkRoles, newNetworkRoles map[models if err != nil { return } - for _, extclient := range extclients { - if _, ok := networkChangeMap[models.NetworkID(extclient.Network)]; ok { - // this extclient's network was removed from group's network roles. - if user, ok := userMap[extclient.OwnerID]; ok { - // super-admins and admins have complete access to the network. - // platform users, at the very least, have access to connect to - // the network. - // service users have no access to the network. - // hence, we delete the extclient and clean up the peers. - if user.PlatformRoleID != models.ServiceUser { - continue - } - err = logic.DeleteExtClientAndCleanup(extclient) - if err != nil { - slog.Error("failed to delete extclient", - "id", extclient.ClientID, "owner", user.UserName, "error", err) + for _, extclient := range extclients { + var shouldDelete bool + user, ok := userMap[extclient.OwnerID] + if !ok { + // user does not exist, delete extclient. + shouldDelete = true + } else { + if user.PlatformRoleID == models.SuperAdminRole || user.PlatformRoleID == models.AdminRole { + // Super-admin and Admin's access is not determined by group membership + // or network roles. Even if a network is removed from the group, they + // continue to have access to the network. + // So, no need to delete the extclient. + shouldDelete = false + } else { + _, hasAccess := user.NetworkRoles[models.NetworkID(extclient.Network)] + if hasAccess { + // The user has access to the network by themselves and not by + // virtue of being a member of the group. + // So, no need to delete the extclient. + shouldDelete = false } else { - if err := mq.PublishDeletedClientPeerUpdate(&extclient); err != nil { - slog.Error("error setting ext peers: " + err.Error()) + _, userInGroup := user.UserGroups[groupID] + _, networkRemoved := networkRemovedMap[models.NetworkID(extclient.Network)] + if userInGroup && networkRemoved { + // This group no longer provides it's members access to the + // network. + // This user is a member of the group and has no direct + // access to the network (either by its platform role or by + // network roles). + // So, delete the extclient. + shouldDelete = true } } } - } + if shouldDelete { + err = logic.DeleteExtClientAndCleanup(extclient) + if err != nil { + slog.Error("failed to delete extclient", + "id", extclient.ClientID, "owner", user.UserName, "error", err) + } else { + if err := mq.PublishDeletedClientPeerUpdate(&extclient); err != nil { + slog.Error("error setting ext peers: " + err.Error()) + } + } + } } + if servercfg.IsDNSMode() { logic.SetDNS() }