Feature/peer validator (#1553)

Follow up management-integrations changes

move groups to separated packages to avoid circle dependencies
save location information in Login action

management/server/http/api/openapi.yml

@@ -355,6 +355,7 @@ components:
             - user_id
             - version
             - ui_version
+            - approval_required
     AccessiblePeer:
       allOf:
         - $ref: '#/components/schemas/PeerMinimum'

management/server/http/api/types.gen.go

@@ -470,7 +470,7 @@ type Peer struct {
 	AccessiblePeers []AccessiblePeer `json:"accessible_peers"`
 
 	// ApprovalRequired (Cloud only) Indicates whether peer needs approval
-	ApprovalRequired *bool `json:"approval_required,omitempty"`
+	ApprovalRequired bool `json:"approval_required"`
 
 	// CityName Commonly used English name of the city
 	CityName CityName `json:"city_name"`
@@ -539,7 +539,7 @@ type Peer struct {
 // PeerBase defines model for PeerBase.
 type PeerBase struct {
 	// ApprovalRequired (Cloud only) Indicates whether peer needs approval
-	ApprovalRequired *bool `json:"approval_required,omitempty"`
+	ApprovalRequired bool `json:"approval_required"`
 
 	// CityName Commonly used English name of the city
 	CityName CityName `json:"city_name"`
@@ -611,7 +611,7 @@ type PeerBatch struct {
 	AccessiblePeersCount int `json:"accessible_peers_count"`
 
 	// ApprovalRequired (Cloud only) Indicates whether peer needs approval
-	ApprovalRequired *bool `json:"approval_required,omitempty"`
+	ApprovalRequired bool `json:"approval_required"`
 
 	// CityName Commonly used English name of the city
 	CityName CityName `json:"city_name"`

management/server/http/groups_handler.go

@@ -4,15 +4,15 @@ import (
 	"encoding/json"
 	"net/http"
 
-	"github.com/netbirdio/netbird/management/server/http/api"
-	"github.com/netbirdio/netbird/management/server/http/util"
-	"github.com/netbirdio/netbird/management/server/status"
-
-	"github.com/netbirdio/netbird/management/server"
-	"github.com/netbirdio/netbird/management/server/jwtclaims"
-
 	"github.com/gorilla/mux"
 	log "github.com/sirupsen/logrus"
+
+	"github.com/netbirdio/netbird/management/server"
+	nbgroup "github.com/netbirdio/netbird/management/server/group"
+	"github.com/netbirdio/netbird/management/server/http/api"
+	"github.com/netbirdio/netbird/management/server/http/util"
+	"github.com/netbirdio/netbird/management/server/jwtclaims"
+	"github.com/netbirdio/netbird/management/server/status"
 )
 
 // GroupsHandler is a handler that returns groups of the account
@@ -110,7 +110,7 @@ func (h *GroupsHandler) UpdateGroup(w http.ResponseWriter, r *http.Request) {
 	} else {
 		peers = *req.Peers
 	}
-	group := server.Group{
+	group := nbgroup.Group{
 		ID:                   groupID,
 		Name:                 req.Name,
 		Peers:                peers,
@@ -154,10 +154,10 @@ func (h *GroupsHandler) CreateGroup(w http.ResponseWriter, r *http.Request) {
 	} else {
 		peers = *req.Peers
 	}
-	group := server.Group{
+	group := nbgroup.Group{
 		Name:   req.Name,
 		Peers:  peers,
-		Issued: server.GroupIssuedAPI,
+		Issued: nbgroup.GroupIssuedAPI,
 	}
 
 	err = h.accountManager.SaveGroup(account.Id, user.Id, &group)
@@ -240,7 +240,7 @@ func (h *GroupsHandler) GetGroup(w http.ResponseWriter, r *http.Request) {
 	}
 }
 
-func toGroupResponse(account *server.Account, group *server.Group) *api.Group {
+func toGroupResponse(account *server.Account, group *nbgroup.Group) *api.Group {
 	cache := make(map[string]api.PeerMinimum)
 	gr := api.Group{
 		Id:     group.ID,

management/server/http/groups_handler_test.go

@@ -15,6 +15,7 @@ import (
 	"github.com/magiconair/properties/assert"
 
 	"github.com/netbirdio/netbird/management/server"
+	nbgroup "github.com/netbirdio/netbird/management/server/group"
 	"github.com/netbirdio/netbird/management/server/http/api"
 	"github.com/netbirdio/netbird/management/server/http/util"
 	"github.com/netbirdio/netbird/management/server/jwtclaims"
@@ -28,30 +29,30 @@ var TestPeers = map[string]*nbpeer.Peer{
 	"B": {Key: "B", ID: "peer-B-ID", IP: net.ParseIP("200.200.200.200")},
 }
 
-func initGroupTestData(user *server.User, groups ...*server.Group) *GroupsHandler {
+func initGroupTestData(user *server.User, _ ...*nbgroup.Group) *GroupsHandler {
 	return &GroupsHandler{
 		accountManager: &mock_server.MockAccountManager{
-			SaveGroupFunc: func(accountID, userID string, group *server.Group) error {
+			SaveGroupFunc: func(accountID, userID string, group *nbgroup.Group) error {
 				if !strings.HasPrefix(group.ID, "id-") {
 					group.ID = "id-was-set"
 				}
 				return nil
 			},
-			GetGroupFunc: func(_, groupID, _ string) (*server.Group, error) {
+			GetGroupFunc: func(_, groupID, _ string) (*nbgroup.Group, error) {
 				if groupID != "idofthegroup" {
 					return nil, status.Errorf(status.NotFound, "not found")
 				}
 				if groupID == "id-jwt-group" {
-					return &server.Group{
+					return &nbgroup.Group{
 						ID:     "id-jwt-group",
 						Name:   "Default Group",
-						Issued: server.GroupIssuedJWT,
+						Issued: nbgroup.GroupIssuedJWT,
 					}, nil
 				}
-				return &server.Group{
+				return &nbgroup.Group{
 					ID:     "idofthegroup",
 					Name:   "Group",
-					Issued: server.GroupIssuedAPI,
+					Issued: nbgroup.GroupIssuedAPI,
 				}, nil
 			},
 			GetAccountFromTokenFunc: func(claims jwtclaims.AuthorizationClaims) (*server.Account, *server.User, error) {
@@ -62,10 +63,10 @@ func initGroupTestData(user *server.User, groups ...*server.Group) *GroupsHandle
 					Users: map[string]*server.User{
 						user.Id: user,
 					},
-					Groups: map[string]*server.Group{
-						"id-jwt-group": {ID: "id-jwt-group", Name: "From JWT", Issued: server.GroupIssuedJWT},
-						"id-existed":   {ID: "id-existed", Peers: []string{"A", "B"}, Issued: server.GroupIssuedAPI},
-						"id-all":       {ID: "id-all", Name: "All", Issued: server.GroupIssuedAPI},
+					Groups: map[string]*nbgroup.Group{
+						"id-jwt-group": {ID: "id-jwt-group", Name: "From JWT", Issued: nbgroup.GroupIssuedJWT},
+						"id-existed":   {ID: "id-existed", Peers: []string{"A", "B"}, Issued: nbgroup.GroupIssuedAPI},
+						"id-all":       {ID: "id-all", Name: "All", Issued: nbgroup.GroupIssuedAPI},
 					},
 				}, user, nil
 			},
@@ -118,7 +119,7 @@ func TestGetGroup(t *testing.T) {
 		},
 	}
 
-	group := &server.Group{
+	group := &nbgroup.Group{
 		ID:   "idofthegroup",
 		Name: "Group",
 	}
@@ -153,7 +154,7 @@ func TestGetGroup(t *testing.T) {
 				t.Fatalf("I don't know what I expected; %v", err)
 			}
 
-			got := &server.Group{}
+			got := &nbgroup.Group{}
 			if err = json.Unmarshal(content, &got); err != nil {
 				t.Fatalf("Sent content is not in correct json format; %v", err)
 			}

management/server/http/handler.go

@@ -9,7 +9,6 @@ import (
 	"github.com/rs/cors"
 
 	"github.com/netbirdio/management-integrations/integrations"
-
 	s "github.com/netbirdio/netbird/management/server"
 	"github.com/netbirdio/netbird/management/server/geolocation"
 	"github.com/netbirdio/netbird/management/server/http/middleware"

management/server/http/peers_handler.go

@@ -6,8 +6,10 @@ import (
 	"net/http"
 
 	"github.com/gorilla/mux"
+	log "github.com/sirupsen/logrus"
 
 	"github.com/netbirdio/netbird/management/server"
+	nbgroup "github.com/netbirdio/netbird/management/server/group"
 	"github.com/netbirdio/netbird/management/server/http/api"
 	"github.com/netbirdio/netbird/management/server/http/util"
 	"github.com/netbirdio/netbird/management/server/jwtclaims"
@@ -61,10 +63,18 @@ func (h *PeersHandler) getPeer(account *server.Account, peerID, userID string, w
 
 	groupsInfo := toGroupsInfo(account.Groups, peer.ID)
 
-	netMap := account.GetPeerNetworkMap(peerID, h.accountManager.GetDNSDomain())
+	validPeers, err := h.accountManager.GetValidatedPeers(account)
+	if err != nil {
+		log.Errorf("failed to list appreoved peers: %v", err)
+		util.WriteError(fmt.Errorf("internal error"), w)
+		return
+	}
+
+	netMap := account.GetPeerNetworkMap(peerID, h.accountManager.GetDNSDomain(), validPeers)
 	accessiblePeers := toAccessiblePeers(netMap, dnsDomain)
 
-	util.WriteJSONObject(w, toSinglePeerResponse(peerToReturn, groupsInfo, dnsDomain, accessiblePeers))
+	_, valid := validPeers[peer.ID]
+	util.WriteJSONObject(w, toSinglePeerResponse(peerToReturn, groupsInfo, dnsDomain, accessiblePeers, valid))
 }
 
 func (h *PeersHandler) updatePeer(account *server.Account, user *server.User, peerID string, w http.ResponseWriter, r *http.Request) {
@@ -75,11 +85,18 @@ func (h *PeersHandler) updatePeer(account *server.Account, user *server.User, pe
 		return
 	}
 
-	update := &nbpeer.Peer{ID: peerID, SSHEnabled: req.SshEnabled, Name: req.Name,
-		LoginExpirationEnabled: req.LoginExpirationEnabled}
+	update := &nbpeer.Peer{
+		ID:                     peerID,
+		SSHEnabled:             req.SshEnabled,
+		Name:                   req.Name,
+		LoginExpirationEnabled: req.LoginExpirationEnabled,
+	}
 
 	if req.ApprovalRequired != nil {
-		update.Status = &nbpeer.PeerStatus{RequiresApproval: *req.ApprovalRequired}
+		// todo: looks like that we reset all status property, is it right?
+		update.Status = &nbpeer.PeerStatus{
+			RequiresApproval: *req.ApprovalRequired,
+		}
 	}
 
 	peer, err := h.accountManager.UpdatePeer(account.Id, user.Id, update)
@@ -91,15 +108,24 @@ func (h *PeersHandler) updatePeer(account *server.Account, user *server.User, pe
 
 	groupMinimumInfo := toGroupsInfo(account.Groups, peer.ID)
 
-	netMap := account.GetPeerNetworkMap(peerID, h.accountManager.GetDNSDomain())
+	validPeers, err := h.accountManager.GetValidatedPeers(account)
+	if err != nil {
+		log.Errorf("failed to list appreoved peers: %v", err)
+		util.WriteError(fmt.Errorf("internal error"), w)
+		return
+	}
+	netMap := account.GetPeerNetworkMap(peerID, h.accountManager.GetDNSDomain(), validPeers)
 	accessiblePeers := toAccessiblePeers(netMap, dnsDomain)
 
-	util.WriteJSONObject(w, toSinglePeerResponse(peer, groupMinimumInfo, dnsDomain, accessiblePeers))
+	_, valid := validPeers[peer.ID]
+
+	util.WriteJSONObject(w, toSinglePeerResponse(peer, groupMinimumInfo, dnsDomain, accessiblePeers, valid))
 }
 
 func (h *PeersHandler) deletePeer(accountID, userID string, peerID string, w http.ResponseWriter) {
 	err := h.accountManager.DeletePeer(accountID, peerID, userID)
 	if err != nil {
+		log.Errorf("failed to delete peer: %v", err)
 		util.WriteError(err, w)
 		return
 	}
@@ -138,46 +164,68 @@ func (h *PeersHandler) HandlePeer(w http.ResponseWriter, r *http.Request) {
 
 // GetAllPeers returns a list of all peers associated with a provided account
 func (h *PeersHandler) GetAllPeers(w http.ResponseWriter, r *http.Request) {
-	switch r.Method {
-	case http.MethodGet:
-		claims := h.claimsExtractor.FromRequestContext(r)
-		account, user, err := h.accountManager.GetAccountFromToken(claims)
-		if err != nil {
-			util.WriteError(err, w)
-			return
-		}
-
-		peers, err := h.accountManager.GetPeers(account.Id, user.Id)
-		if err != nil {
-			util.WriteError(err, w)
-			return
-		}
-
-		dnsDomain := h.accountManager.GetDNSDomain()
-
-		respBody := make([]*api.PeerBatch, 0, len(peers))
-		for _, peer := range peers {
-			peerToReturn, err := h.checkPeerStatus(peer)
-			if err != nil {
-				util.WriteError(err, w)
-				return
-			}
-			groupMinimumInfo := toGroupsInfo(account.Groups, peer.ID)
-
-			accessiblePeerNumbers := h.accessiblePeersNumber(account, peer.ID)
-
-			respBody = append(respBody, toPeerListItemResponse(peerToReturn, groupMinimumInfo, dnsDomain, accessiblePeerNumbers))
-		}
-		util.WriteJSONObject(w, respBody)
-		return
-	default:
+	if r.Method != http.MethodGet {
 		util.WriteError(status.Errorf(status.NotFound, "unknown METHOD"), w)
+		return
 	}
+
+	claims := h.claimsExtractor.FromRequestContext(r)
+	account, user, err := h.accountManager.GetAccountFromToken(claims)
+	if err != nil {
+		util.WriteError(err, w)
+		return
+	}
+
+	peers, err := h.accountManager.GetPeers(account.Id, user.Id)
+	if err != nil {
+		util.WriteError(err, w)
+		return
+	}
+
+	dnsDomain := h.accountManager.GetDNSDomain()
+
+	respBody := make([]*api.PeerBatch, 0, len(peers))
+	for _, peer := range peers {
+		peerToReturn, err := h.checkPeerStatus(peer)
+		if err != nil {
+			util.WriteError(err, w)
+			return
+		}
+		groupMinimumInfo := toGroupsInfo(account.Groups, peer.ID)
+
+		accessiblePeerNumbers, _ := h.accessiblePeersNumber(account, peer.ID)
+
+		respBody = append(respBody, toPeerListItemResponse(peerToReturn, groupMinimumInfo, dnsDomain, accessiblePeerNumbers))
+	}
+
+	validPeersMap, err := h.accountManager.GetValidatedPeers(account)
+	if err != nil {
+		log.Errorf("failed to list appreoved peers: %v", err)
+		util.WriteError(fmt.Errorf("internal error"), w)
+		return
+	}
+	h.setApprovalRequiredFlag(respBody, validPeersMap)
+
+	util.WriteJSONObject(w, respBody)
 }
 
-func (h *PeersHandler) accessiblePeersNumber(account *server.Account, peerID string) int {
-	netMap := account.GetPeerNetworkMap(peerID, h.accountManager.GetDNSDomain())
-	return len(netMap.Peers) + len(netMap.OfflinePeers)
+func (h *PeersHandler) accessiblePeersNumber(account *server.Account, peerID string) (int, error) {
+	validatedPeersMap, err := h.accountManager.GetValidatedPeers(account)
+	if err != nil {
+		return 0, err
+	}
+
+	netMap := account.GetPeerNetworkMap(peerID, h.accountManager.GetDNSDomain(), validatedPeersMap)
+	return len(netMap.Peers) + len(netMap.OfflinePeers), nil
+}
+
+func (h *PeersHandler) setApprovalRequiredFlag(respBody []*api.PeerBatch, approvedPeersMap map[string]struct{}) {
+	for _, peer := range respBody {
+		_, ok := approvedPeersMap[peer.Id]
+		if !ok {
+			peer.ApprovalRequired = true
+		}
+	}
 }
 
 func toAccessiblePeers(netMap *server.NetworkMap, dnsDomain string) []api.AccessiblePeer {
@@ -206,7 +254,7 @@ func toAccessiblePeers(netMap *server.NetworkMap, dnsDomain string) []api.Access
 	return accessiblePeers
 }
 
-func toGroupsInfo(groups map[string]*server.Group, peerID string) []api.GroupMinimum {
+func toGroupsInfo(groups map[string]*nbgroup.Group, peerID string) []api.GroupMinimum {
 	var groupsInfo []api.GroupMinimum
 	groupsChecked := make(map[string]struct{})
 	for _, group := range groups {
@@ -230,7 +278,7 @@ func toGroupsInfo(groups map[string]*server.Group, peerID string) []api.GroupMin
 	return groupsInfo
 }
 
-func toSinglePeerResponse(peer *nbpeer.Peer, groupsInfo []api.GroupMinimum, dnsDomain string, accessiblePeer []api.AccessiblePeer) *api.Peer {
+func toSinglePeerResponse(peer *nbpeer.Peer, groupsInfo []api.GroupMinimum, dnsDomain string, accessiblePeer []api.AccessiblePeer, approved bool) *api.Peer {
 	osVersion := peer.Meta.OSVersion
 	if osVersion == "" {
 		osVersion = peer.Meta.Core
@@ -257,7 +305,7 @@ func toSinglePeerResponse(peer *nbpeer.Peer, groupsInfo []api.GroupMinimum, dnsD
 		LastLogin:              peer.LastLogin,
 		LoginExpired:           peer.Status.LoginExpired,
 		AccessiblePeers:        accessiblePeer,
-		ApprovalRequired:       &peer.Status.RequiresApproval,
+		ApprovalRequired:       !approved,
 		CountryCode:            peer.Location.CountryCode,
 		CityName:               peer.Location.CityName,
 	}
@@ -290,7 +338,6 @@ func toPeerListItemResponse(peer *nbpeer.Peer, groupsInfo []api.GroupMinimum, dn
 		LastLogin:              peer.LastLogin,
 		LoginExpired:           peer.Status.LoginExpired,
 		AccessiblePeersCount:   accessiblePeersCount,
-		ApprovalRequired:       &peer.Status.RequiresApproval,
 		CountryCode:            peer.Location.CountryCode,
 		CityName:               peer.Location.CityName,
 	}

management/server/http/policies_handler_test.go

@@ -9,6 +9,7 @@ import (
 	"strings"
 	"testing"
 
+	nbgroup "github.com/netbirdio/netbird/management/server/group"
 	"github.com/netbirdio/netbird/management/server/http/api"
 	"github.com/netbirdio/netbird/management/server/status"
 
@@ -51,7 +52,7 @@ func initPoliciesTestData(policies ...*server.Policy) *Policies {
 					Policies: []*server.Policy{
 						{ID: "id-existed"},
 					},
-					Groups: map[string]*server.Group{
+					Groups: map[string]*nbgroup.Group{
 						"F": {ID: "F"},
 						"G": {ID: "G"},
 					},

management/server/http/setupkeys_handler_test.go

@@ -13,13 +13,12 @@ import (
 	"github.com/gorilla/mux"
 	"github.com/stretchr/testify/assert"
 
-	"github.com/netbirdio/netbird/management/server/http/api"
-	"github.com/netbirdio/netbird/management/server/status"
-
-	"github.com/netbirdio/netbird/management/server/jwtclaims"
-
 	"github.com/netbirdio/netbird/management/server"
+	nbgroup "github.com/netbirdio/netbird/management/server/group"
+	"github.com/netbirdio/netbird/management/server/http/api"
+	"github.com/netbirdio/netbird/management/server/jwtclaims"
 	"github.com/netbirdio/netbird/management/server/mock_server"
+	"github.com/netbirdio/netbird/management/server/status"
 )
 
 const (
@@ -44,7 +43,7 @@ func initSetupKeysTestMetaData(defaultKey *server.SetupKey, newKey *server.Setup
 					SetupKeys: map[string]*server.SetupKey{
 						defaultKey.Key: defaultKey,
 					},
-					Groups: map[string]*server.Group{
+					Groups: map[string]*nbgroup.Group{
 						"group-1": {ID: "group-1", Peers: []string{"A", "B"}},
 						"id-all":  {ID: "id-all", Name: "All"},
 					},

management/server/http/util/util.go

@@ -99,6 +99,8 @@ func WriteError(err error, w http.ResponseWriter) {
 			httpStatus = http.StatusUnprocessableEntity
 		case status.Unauthorized:
 			httpStatus = http.StatusUnauthorized
+		case status.BadRequest:
+			httpStatus = http.StatusBadRequest
 		default:
 		}
 		msg = strings.ToLower(err.Error())