Merge remote-tracking branch 'origin/main' into merge-sync-serial-bump

management/server/peer.go

@@ -1066,6 +1066,18 @@ func (am *DefaultAccountManager) SyncPeer(ctx context.Context, sync types.PeerSy
 
 	metaDiffAffectsPosture := posture.AffectsPosture(ctx, &metaDiff, resPostureChecks)
 	if requiresPeerUpdate(ctx, isStatusChanged, sync.UpdateAccountPeers, ipv6CapabilityChanged, metaDiffAffectsPosture, metaDiff.VersionChanged(), metaDiff.HostnameChanged()) {
+		// The maps sent out below carry changed content (the peer's version,
+		// hostname, capabilities, or validation state). The serial versions the
+		// distributed map, so it must advance with the content.
+		if err = am.Store.IncrementNetworkSerial(ctx, accountID); err != nil {
+			return nil, nil, nil, 0, fmt.Errorf("increment network serial: %w", err)
+		}
+		// The map built above predates the increment; rebuild it so the syncing
+		// peer's own response also carries the advanced serial.
+		nmap, resPostureChecks, dnsFwdPort, err = am.networkMapController.GetValidatedPeerWithMap(ctx, peerNotValid, accountID, peer.ID)
+		if err != nil {
+			return nil, nil, nil, 0, err
+		}
 		changedPeerIDs := []string{peer.ID}
 		affectedPeerIDs := am.syncPeerAffectedPeers(ctx, accountID, peer.ID, nmap, peerNotValid, metaDiffAffectsPosture)
 		if err = am.networkMapController.OnPeersUpdated(ctx, accountID, changedPeerIDs, affectedPeerIDs); err != nil {
@@ -1230,6 +1242,16 @@ func (am *DefaultAccountManager) LoginPeer(ctx context.Context, login types.Peer
 		return nil, nil, nil, false, err
 	}
 
+	if shouldUpdatePeers {
+		// The maps sent out below carry changed peer content. The serial versions
+		// the distributed map, so it must advance with the content, and before the
+		// network is loaded for the response so the logging-in peer also sees the
+		// advanced serial.
+		if err = am.Store.IncrementNetworkSerial(ctx, accountID); err != nil {
+			return nil, nil, nil, false, fmt.Errorf("increment network serial: %w", err)
+		}
+	}
+
 	network, postureChecks, enableSSH, err := getPeerLoginInfo(ctx, am.Store, accountID, peer, !isRequiresApproval)
 	if err != nil {
 		return nil, nil, nil, false, err

management/server/peer_test.go

@@ -16,11 +16,11 @@ import (
 	"testing"
 	"time"
 
-	"go.uber.org/mock/gomock"
 	"github.com/rs/xid"
 	log "github.com/sirupsen/logrus"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"go.uber.org/mock/gomock"
 	"golang.org/x/exp/maps"
 	"golang.zx2c4.com/wireguard/wgctrl/wgtypes"
 
@@ -2828,6 +2828,104 @@ func TestSyncPeer_IPv6CapabilityChangePropagates(t *testing.T) {
 	})
 }
 
+// TestSyncPeer_PeerUpdateBumpsNetworkSerial ensures that a sync which changes
+// map-relevant peer content (agent version, hostname, capabilities, validation
+// state) advances the network serial before other peers receive the recomputed
+// map. The serial versions the distributed map, so its content must not change
+// under an unchanged serial.
+func TestSyncPeer_PeerUpdateBumpsNetworkSerial(t *testing.T) {
+	manager, _, account, _, peer2, _ := setupNetworkMapTest(t)
+
+	network, err := manager.Store.GetAccountNetwork(context.Background(), store.LockingStrengthNone, account.Id)
+	require.NoError(t, err)
+	serialBefore := network.CurrentSerial()
+
+	t.Run("no bump when nothing changed", func(t *testing.T) {
+		_, _, _, _, err := manager.SyncPeer(context.Background(), types.PeerSync{
+			WireGuardPubKey: peer2.Key,
+			Meta:            peer2.Meta,
+		}, peer2.AccountID)
+		require.NoError(t, err)
+
+		network, err := manager.Store.GetAccountNetwork(context.Background(), store.LockingStrengthNone, account.Id)
+		require.NoError(t, err)
+		assert.Equal(t, serialBefore, network.CurrentSerial(), "an unchanged sync should not advance the serial")
+	})
+
+	t.Run("bump when the agent version changes", func(t *testing.T) {
+		newMeta := peer2.Meta
+		newMeta.WtVersion = "0.99.99"
+
+		_, nmap, _, _, err := manager.SyncPeer(context.Background(), types.PeerSync{
+			WireGuardPubKey: peer2.Key,
+			Meta:            newMeta,
+		}, peer2.AccountID)
+		require.NoError(t, err)
+
+		network, err := manager.Store.GetAccountNetwork(context.Background(), store.LockingStrengthNone, account.Id)
+		require.NoError(t, err)
+		assert.Greater(t, network.CurrentSerial(), serialBefore, "a map-relevant meta change should advance the serial")
+		require.NotNil(t, nmap)
+		assert.Equal(t, network.CurrentSerial(), nmap.Network.CurrentSerial(), "the syncing peer's own map should carry the advanced serial")
+	})
+}
+
+// TestLoginPeer_PeerUpdateBumpsNetworkSerial ensures that a login which changes
+// peer state (an expired login being re-authenticated) advances the network
+// serial before other peers receive the recomputed map, and that the login
+// response itself carries the advanced serial. An unchanged login must leave
+// the serial untouched.
+func TestLoginPeer_PeerUpdateBumpsNetworkSerial(t *testing.T) {
+	manager, _, account, _, _, _ := setupNetworkMapTest(t)
+
+	key, err := wgtypes.GeneratePrivateKey()
+	require.NoError(t, err)
+	userPeer, _, _, _, err := manager.AddPeer(context.Background(), "", "", userID, &nbpeer.Peer{
+		Key:  key.PublicKey().String(),
+		Meta: nbpeer.PeerSystemMeta{Hostname: "login-serial-peer"},
+	}, false)
+	require.NoError(t, err, "unable to add the user-owned peer")
+
+	t.Run("bump when an expired login is re-authenticated", func(t *testing.T) {
+		userPeer.Status.LoginExpired = true
+		require.NoError(t, manager.Store.SavePeer(context.Background(), account.Id, userPeer))
+
+		network, err := manager.Store.GetAccountNetwork(context.Background(), store.LockingStrengthNone, account.Id)
+		require.NoError(t, err)
+		serialBefore := network.CurrentSerial()
+
+		_, loginNetwork, _, _, err := manager.LoginPeer(context.Background(), types.PeerLogin{
+			WireGuardPubKey: userPeer.Key,
+			UserID:          userID,
+			Meta:            userPeer.Meta,
+		})
+		require.NoError(t, err)
+
+		network, err = manager.Store.GetAccountNetwork(context.Background(), store.LockingStrengthNone, account.Id)
+		require.NoError(t, err)
+		assert.Greater(t, network.CurrentSerial(), serialBefore, "re-authenticating an expired login should advance the serial")
+		require.NotNil(t, loginNetwork)
+		assert.Equal(t, network.CurrentSerial(), loginNetwork.CurrentSerial(), "the login response should carry the advanced serial")
+	})
+
+	t.Run("no bump when nothing changed", func(t *testing.T) {
+		network, err := manager.Store.GetAccountNetwork(context.Background(), store.LockingStrengthNone, account.Id)
+		require.NoError(t, err)
+		serialBefore := network.CurrentSerial()
+
+		_, _, _, _, err = manager.LoginPeer(context.Background(), types.PeerLogin{
+			WireGuardPubKey: userPeer.Key,
+			UserID:          userID,
+			Meta:            userPeer.Meta,
+		})
+		require.NoError(t, err)
+
+		network, err = manager.Store.GetAccountNetwork(context.Background(), store.LockingStrengthNone, account.Id)
+		require.NoError(t, err)
+		assert.Equal(t, serialBefore, network.CurrentSerial(), "an unchanged login should not advance the serial")
+	})
+}
+
 func TestUpdatePeer_DnsLabelCollisionWithFQDN(t *testing.T) {
 	manager, _, err := createManager(t)
 	require.NoError(t, err, "unable to create account manager")