Use global login expiration setting when sending network map (#731)

Peers were considered expired and not sent to remote peers when global expiration was disabled.
netbirdio · Mar 9, 2023 · 9ce8056 · 9ce8056
1 parent c65a5ac
commit 9ce8056
Show file tree

Hide file tree

Showing 5 changed files with 126 additions and 2 deletions.
diff --git a/management/server/account.go b/management/server/account.go
@@ -314,7 +314,7 @@ func (a *Account) GetPeerNetworkMap(peerID, dnsDomain string) *NetworkMap {
 	var expiredPeers []*Peer
 	for _, p := range aclPeers {
 		expired, _ := p.LoginExpired(a.Settings.PeerLoginExpiration)
-		if expired {
+		if a.Settings.PeerLoginExpirationEnabled && expired {
 			expiredPeers = append(expiredPeers, p)
 			continue
 		}

diff --git a/management/server/account_test.go b/management/server/account_test.go
@@ -98,6 +98,122 @@ func verifyNewAccountHasDefaultFields(t *testing.T, account *Account, createdBy
 	}
 }
 
+func TestAccount_GetPeerNetworkMap(t *testing.T) {
+	peerID1 := "peer-1"
+	peerID2 := "peer-2"
+	tt := []struct {
+		name                 string
+		accountSettings      Settings
+		peerID               string
+		expectedPeers        []string
+		expectedOfflinePeers []string
+		peers                map[string]*Peer
+	}{
+		{
+			name:                 "Should return ALL peers when global peer login expiration disabled",
+			accountSettings:      Settings{PeerLoginExpirationEnabled: false, PeerLoginExpiration: time.Hour},
+			peerID:               peerID1,
+			expectedPeers:        []string{peerID2},
+			expectedOfflinePeers: []string{},
+			peers: map[string]*Peer{
+				"peer-1": {
+					ID:       peerID1,
+					Key:      "peer-1-key",
+					IP:       net.IP{100, 64, 0, 1},
+					Name:     peerID1,
+					DNSLabel: peerID1,
+					Status: &PeerStatus{
+						LastSeen:     time.Now(),
+						Connected:    false,
+						LoginExpired: true,
+					},
+					UserID:    userID,
+					LastLogin: time.Now().Add(-time.Hour * 24 * 30 * 30),
+				},
+				"peer-2": {
+					ID:       peerID2,
+					Key:      "peer-2-key",
+					IP:       net.IP{100, 64, 0, 1},
+					Name:     peerID2,
+					DNSLabel: peerID2,
+					Status: &PeerStatus{
+						LastSeen:     time.Now(),
+						Connected:    false,
+						LoginExpired: false,
+					},
+					UserID:                 userID,
+					LastLogin:              time.Now(),
+					LoginExpirationEnabled: true,
+				},
+			},
+		},
+		{
+			name:                 "Should return no peers when global peer login expiration enabled and peers expired",
+			accountSettings:      Settings{PeerLoginExpirationEnabled: true, PeerLoginExpiration: time.Hour},
+			peerID:               peerID1,
+			expectedPeers:        []string{},
+			expectedOfflinePeers: []string{peerID2},
+			peers: map[string]*Peer{
+				"peer-1": {
+					ID:       peerID1,
+					Key:      "peer-1-key",
+					IP:       net.IP{100, 64, 0, 1},
+					Name:     peerID1,
+					DNSLabel: peerID1,
+					Status: &PeerStatus{
+						LastSeen:     time.Now(),
+						Connected:    false,
+						LoginExpired: true,
+					},
+					UserID:                 userID,
+					LastLogin:              time.Now().Add(-time.Hour * 24 * 30 * 30),
+					LoginExpirationEnabled: true,
+				},
+				"peer-2": {
+					ID:       peerID2,
+					Key:      "peer-2-key",
+					IP:       net.IP{100, 64, 0, 1},
+					Name:     peerID2,
+					DNSLabel: peerID2,
+					Status: &PeerStatus{
+						LastSeen:     time.Now(),
+						Connected:    false,
+						LoginExpired: true,
+					},
+					UserID:                 userID,
+					LastLogin:              time.Now().Add(-time.Hour * 24 * 30 * 30),
+					LoginExpirationEnabled: true,
+				},
+			},
+		},
+	}
+
+	netIP := net.IP{100, 64, 0, 0}
+	netMask := net.IPMask{255, 255, 0, 0}
+	network := &Network{
+		Id:     "network",
+		Net:    net.IPNet{IP: netIP, Mask: netMask},
+		Dns:    "netbird.selfhosted",
+		Serial: 0,
+		mu:     sync.Mutex{},
+	}
+
+	for _, testCase := range tt {
+		account := newAccountWithId("account-1", userID, "netbird.io")
+		account.Network = network
+		account.Peers = testCase.peers
+		for _, peer := range account.Peers {
+			all, _ := account.GetGroupAll()
+			account.Groups[all.ID].Peers = append(account.Groups[all.ID].Peers, peer.ID)
+		}
+
+		networkMap := account.GetPeerNetworkMap(testCase.peerID, "netbird.io")
+		assert.Len(t, networkMap.Peers, len(testCase.expectedPeers))
+		assert.Len(t, networkMap.OfflinePeers, len(testCase.expectedOfflinePeers))
+	}
+
+}
+
 func TestNewAccount(t *testing.T) {
 
 	domain := "netbird.io"

diff --git a/management/server/file_store.go b/management/server/file_store.go
@@ -140,6 +140,10 @@ func restore(file string) (*FileStore, error) {
 		// Swap Peer.Key with Peer.ID in the Account.Peers map.
 		migrationPeers := make(map[string]*Peer) // key to Peer
 		for key, peer := range account.Peers {
+			// set LastLogin for the peers that were onboarded before the peer login expiration feature
+			if peer.LastLogin.IsZero() {
+				peer.LastLogin = time.Now()
+			}
 			if peer.ID != "" {
 				continue
 			}

diff --git a/management/server/management_proto_test.go b/management/server/management_proto_test.go
@@ -244,7 +244,7 @@ func Test_SyncProtocol(t *testing.T) {
 	}
 
 	if len(networkMap.GetRemotePeers()) != 3 {
-		t.Fatal("expecting SyncResponse to have NetworkMap with 3 remote peers")
+		t.Fatalf("expecting SyncResponse to have NetworkMap with 3 remote peers, got %d", len(networkMap.GetRemotePeers()))
 	}
 
 	// expired peers come separately.

diff --git a/management/server/testdata/store_with_expired_peers.json b/management/server/testdata/store_with_expired_peers.json
@@ -5,6 +5,10 @@
             "Domain": "test.com",
             "DomainCategory": "private",
             "IsDomainPrimaryAccount": true,
+            "Settings": {
+                "PeerLoginExpirationEnabled": true,
+                "PeerLoginExpiration": 3600000000000
+            },
             "SetupKeys": {
                 "A2C8E62B-38F5-4553-B31E-DD66C696CEBB": {
                     "Key": "A2C8E62B-38F5-4553-B31E-DD66C696CEBB",