db: preserve user_id on untagged nodes with tags='null'

A nil tags slice marshals to JSON `null`; the clear-tagged migration
read that as tagged and cleared user_id. Exclude it, and recover
already-detached nodes from their pre-auth key.

Fixes #3323

CHANGELOG.md

@@ -1,6 +1,18 @@
 # CHANGELOG
 
-## 0.29.0 (202x-xx-xx)
+## 0.30.0 (202x-xx-xx)
+
+**Minimum supported Tailscale client version: v1.xx.0**
+
+## 0.29.1 (2026-06-18)
+
+**Minimum supported Tailscale client version: v1.80.0**
+
+### Changes
+
+- Fix nodes with `tags='null'` losing their assigned user on upgrade [#3325](https://github.com/juanfont/headscale/pull/3325)
+
+## 0.29.0 (2026-06-17)
 
 **Minimum supported Tailscale client version: v1.80.0**
 

flake.lock

@@ -20,11 +20,11 @@
     },
     "nixpkgs": {
       "locked": {
-        "lastModified": 1781398523,
-        "narHash": "sha256-NC4/V9NtyzrCCPRJrt7szMW6zEP2r0XPerXOWsQiBCI=",
+        "lastModified": 1781153106,
+        "narHash": "sha256-yzsroLCcuRG4KdGMxWt0eXKOrRSgQT8/xjYngeq9ujU=",
         "owner": "NixOS",
         "repo": "nixpkgs",
-        "rev": "ef272014ec7a5577ef64ead0aef248f82cf9f3a1",
+        "rev": "9ee75f111a06d7ab2b2f729698a8eff53d54e070",
         "type": "github"
       },
       "original": {

hscontrol/auth_test.go

@@ -1699,7 +1699,7 @@ func TestAuthenticationFlows(t *testing.T) {
 				assert.False(t, resp.NodeKeyExpired)
 
 				// Verify NEW node was created for user2
-				node2, found := app.state.GetNodeByMachineKey(machineKey1.Public(), types.UserID(2))
+				node2, found := app.state.GetNodesByMachineKeyAllUsers(machineKey1.Public())[types.UserID(2)]
 				require.True(t, found, "new node should exist for user2")
 				assert.Equal(t, uint(2), node2.UserID().Get(), "new node should belong to user2")
 
@@ -1707,7 +1707,7 @@ func TestAuthenticationFlows(t *testing.T) {
 				assert.Equal(t, "user2-context", user.Name(), "new node should show user2 username")
 
 				// Verify original node still exists for user1
-				node1, found := app.state.GetNodeByMachineKey(machineKey1.Public(), types.UserID(1))
+				node1, found := app.state.GetNodesByMachineKeyAllUsers(machineKey1.Public())[types.UserID(1)]
 				require.True(t, found, "original node should still exist for user1")
 				assert.Equal(t, uint(1), node1.UserID().Get(), "original node should still belong to user1")
 
@@ -1775,13 +1775,13 @@ func TestAuthenticationFlows(t *testing.T) {
 			validateCompleteResponse: true,
 			validate: func(t *testing.T, resp *tailcfg.RegisterResponse, app *Headscale) { //nolint:thelper
 				// User1's original node should STILL exist (not transferred)
-				node1, found1 := app.state.GetNodeByMachineKey(machineKey1.Public(), types.UserID(1))
+				node1, found1 := app.state.GetNodesByMachineKeyAllUsers(machineKey1.Public())[types.UserID(1)]
 				require.True(t, found1, "user1's original node should still exist")
 				assert.Equal(t, uint(1), node1.UserID().Get(), "user1's node should still belong to user1")
 				assert.Equal(t, nodeKey1.Public(), node1.NodeKey(), "user1's node should have original node key")
 
 				// User2 should have a NEW node created
-				node2, found2 := app.state.GetNodeByMachineKey(machineKey1.Public(), types.UserID(2))
+				node2, found2 := app.state.GetNodesByMachineKeyAllUsers(machineKey1.Public())[types.UserID(2)]
 				require.True(t, found2, "user2 should have new node created")
 				assert.Equal(t, uint(2), node2.UserID().Get(), "user2's node should belong to user2")
 
@@ -2914,7 +2914,7 @@ func TestPreAuthKeyLogoutAndReloginDifferentUser(t *testing.T) {
 	for i := range 2 {
 		node := nodes[i]
 		// User1's original nodes should still be owned by user1
-		registeredNode, found := app.state.GetNodeByMachineKey(node.machineKey.Public(), types.UserID(user1.ID))
+		registeredNode, found := app.state.GetNodesByMachineKeyAllUsers(node.machineKey.Public())[types.UserID(user1.ID)]
 		require.True(t, found, "User1's original node %s should still exist", node.hostname)
 		require.Equal(t, user1.ID, registeredNode.UserID().Get(), "Node %s should still belong to user1", node.hostname)
 		t.Logf("✓ User1's original node %s (ID=%d) still owned by user1", node.hostname, registeredNode.ID().Uint64())
@@ -2923,7 +2923,7 @@ func TestPreAuthKeyLogoutAndReloginDifferentUser(t *testing.T) {
 	for i := 2; i < 4; i++ {
 		node := nodes[i]
 		// User2's original nodes should still be owned by user2
-		registeredNode, found := app.state.GetNodeByMachineKey(node.machineKey.Public(), types.UserID(user2.ID))
+		registeredNode, found := app.state.GetNodesByMachineKeyAllUsers(node.machineKey.Public())[types.UserID(user2.ID)]
 		require.True(t, found, "User2's original node %s should still exist", node.hostname)
 		require.Equal(t, user2.ID, registeredNode.UserID().Get(), "Node %s should still belong to user2", node.hostname)
 		t.Logf("✓ User2's original node %s (ID=%d) still owned by user2", node.hostname, registeredNode.ID().Uint64())
@@ -2935,7 +2935,7 @@ func TestPreAuthKeyLogoutAndReloginDifferentUser(t *testing.T) {
 	for i := 2; i < 4; i++ {
 		node := nodes[i]
 		// Should be able to find a node with user1 and this machine key (the new one)
-		newNode, found := app.state.GetNodeByMachineKey(node.machineKey.Public(), types.UserID(user1.ID))
+		newNode, found := app.state.GetNodesByMachineKeyAllUsers(node.machineKey.Public())[types.UserID(user1.ID)]
 		require.True(t, found, "Should have created new node for user1 with machine key from %s", node.hostname)
 		require.Equal(t, user1.ID, newNode.UserID().Get(), "New node should belong to user1")
 		t.Logf("✓ New node created for user1 with machine key from %s (ID=%d)", node.hostname, newNode.ID().Uint64())
@@ -2984,7 +2984,7 @@ func TestWebFlowReauthDifferentUser(t *testing.T) {
 	require.True(t, resp1.MachineAuthorized, "Should be authorized via pre-auth key")
 
 	// Verify node exists for user1
-	user1Node, found := app.state.GetNodeByMachineKey(machineKey.Public(), types.UserID(user1.ID))
+	user1Node, found := app.state.GetNodesByMachineKeyAllUsers(machineKey.Public())[types.UserID(user1.ID)]
 	require.True(t, found, "Node should exist for user1")
 	require.Equal(t, user1.ID, user1Node.UserID().Get(), "Node should belong to user1")
 	user1NodeID := user1Node.ID()
@@ -3000,7 +3000,7 @@ func TestWebFlowReauthDifferentUser(t *testing.T) {
 	require.NoError(t, err)
 
 	// Verify node is expired
-	user1Node, found = app.state.GetNodeByMachineKey(machineKey.Public(), types.UserID(user1.ID))
+	user1Node, found = app.state.GetNodesByMachineKeyAllUsers(machineKey.Public())[types.UserID(user1.ID)]
 	require.True(t, found, "Node should still exist after logout")
 	require.True(t, user1Node.IsExpired(), "Node should be expired after logout")
 	t.Logf("✓ User1 node expired (logged out)")
@@ -3041,7 +3041,7 @@ func TestWebFlowReauthDifferentUser(t *testing.T) {
 
 	t.Run("user1_original_node_still_exists", func(t *testing.T) {
 		// User1's original node should STILL exist (not transferred to user2)
-		user1NodeAfter, found1 := app.state.GetNodeByMachineKey(machineKey.Public(), types.UserID(user1.ID))
+		user1NodeAfter, found1 := app.state.GetNodesByMachineKeyAllUsers(machineKey.Public())[types.UserID(user1.ID)]
 		assert.True(t, found1, "User1's original node should still exist (not transferred)")
 
 		if !found1 {
@@ -3056,7 +3056,7 @@ func TestWebFlowReauthDifferentUser(t *testing.T) {
 
 	t.Run("user2_has_new_node_created", func(t *testing.T) {
 		// User2 should have a NEW node created (not transfer from user1)
-		user2Node, found2 := app.state.GetNodeByMachineKey(machineKey.Public(), types.UserID(user2.ID))
+		user2Node, found2 := app.state.GetNodesByMachineKeyAllUsers(machineKey.Public())[types.UserID(user2.ID)]
 		assert.True(t, found2, "User2 should have a new node created")
 
 		if !found2 {
@@ -3080,8 +3080,8 @@ func TestWebFlowReauthDifferentUser(t *testing.T) {
 
 	t.Run("both_nodes_share_machine_key", func(t *testing.T) {
 		// Both nodes should have the same machine key (same physical device)
-		user1NodeFinal, found1 := app.state.GetNodeByMachineKey(machineKey.Public(), types.UserID(user1.ID))
-		user2NodeFinal, found2 := app.state.GetNodeByMachineKey(machineKey.Public(), types.UserID(user2.ID))
+		user1NodeFinal, found1 := app.state.GetNodesByMachineKeyAllUsers(machineKey.Public())[types.UserID(user1.ID)]
+		user2NodeFinal, found2 := app.state.GetNodesByMachineKeyAllUsers(machineKey.Public())[types.UserID(user2.ID)]
 
 		require.True(t, found1, "User1 node should exist")
 		require.True(t, found2, "User2 node should exist")

hscontrol/db/db.go

@@ -706,13 +706,20 @@ AND auth_key_id NOT IN (
 				// but this prevents deleting users whose nodes have been
 				// tagged, and the ON DELETE CASCADE FK would destroy the
 				// tagged nodes if the user were deleted.
+				//
+				// A nil tags slice marshals to the JSON literal 'null', so
+				// untagged nodes can carry tags='null'. That spelling must be
+				// excluded alongside '[]' and '' or untagged nodes lose their
+				// user. Nodes already detached by the earlier version of this
+				// migration are repaired by the recovery migration below.
 				// Fixes: https://github.com/juanfont/headscale/issues/3077
+				// Fixes: https://github.com/juanfont/headscale/issues/3323
 				ID: "202602201200-clear-tagged-node-user-id",
 				Migrate: func(tx *gorm.DB) error {
 					err := tx.Exec(`
 UPDATE nodes
 SET user_id = NULL
-WHERE tags IS NOT NULL AND tags != '[]' AND tags != '';
+WHERE tags IS NOT NULL AND tags != '[]' AND tags != '' AND tags != 'null';
 						`).Error
 					if err != nil {
 						return fmt.Errorf("clearing user_id on tagged nodes: %w", err)
@@ -744,6 +751,36 @@ WHERE expiry IS NOT NULL AND expiry < '1900-01-01';
 				},
 				Rollback: func(db *gorm.DB) error { return nil },
 			},
+			{
+				// Recover user_id on untagged nodes detached by the earlier
+				// version of 202602201200-clear-tagged-node-user-id, which
+				// treated tags='null' as tagged and cleared the user. This
+				// repairs databases that already upgraded to 0.29.0; fresh
+				// upgrades are protected by the fixed migration above and find
+				// nothing to repair. Recovery is best-effort: the owner is
+				// re-derived from the node's pre-auth key, so nodes registered
+				// via CLI/OIDC (no pre-auth key) cannot be recovered and must
+				// be reassigned manually.
+				// Fixes: https://github.com/juanfont/headscale/issues/3323
+				ID: "202606181200-recover-null-tags-node-user-id",
+				Migrate: func(tx *gorm.DB) error {
+					err := tx.Exec(`
+UPDATE nodes
+SET user_id = (
+	SELECT pak.user_id FROM pre_auth_keys pak WHERE pak.id = nodes.auth_key_id
+)
+WHERE user_id IS NULL
+	AND auth_key_id IS NOT NULL
+	AND (tags IS NULL OR tags = '' OR tags = '[]' OR tags = 'null');
+						`).Error
+					if err != nil {
+						return fmt.Errorf("recovering user_id on untagged nodes: %w", err)
+					}
+
+					return nil
+				},
+				Rollback: func(db *gorm.DB) error { return nil },
+			},
 		},
 	)
 

hscontrol/db/db_test.go

@@ -198,6 +198,103 @@ func TestSQLiteMigrationAndDataValidation(t *testing.T) {
 				assert.False(t, node5.IsExpired(), "node5 should not be reported as expired")
 			},
 		},
+		// Test for the clear-tagged-node-user-id migration
+		// (202602201200-clear-tagged-node-user-id). A nil tags slice
+		// marshals to the JSON literal 'null', so untagged nodes can carry
+		// tags='null' in the database. The migration must only clear
+		// user_id on genuinely tagged nodes, not on these untagged ones.
+		// Fixes: https://github.com/juanfont/headscale/issues/3323
+		{
+			dbPath: "testdata/sqlite/null_tags_user_id_migration_test.sql",
+			wantFunc: func(t *testing.T, hsdb *HSDatabase) {
+				t.Helper()
+
+				nodes, err := Read(hsdb.DB, func(rx *gorm.DB) (types.Nodes, error) {
+					return ListNodes(rx)
+				})
+				require.NoError(t, err)
+				require.Len(t, nodes, 4, "should have all 4 nodes")
+
+				byHostname := make(map[string]*types.Node, len(nodes))
+				for _, n := range nodes {
+					byHostname[n.Hostname] = n
+				}
+
+				// Node 1 had tags='null' (untagged) and belonged to user2.
+				// The migration must NOT clear its user_id.
+				node1 := byHostname["node1"]
+				require.NotNil(t, node1, "node1 should exist")
+				assert.False(t, node1.IsTagged(), "node1 with tags='null' should be untagged")
+				require.NotNil(t, node1.UserID, "node1 should keep its user assigned")
+				assert.Equal(t, uint(2), *node1.UserID, "node1 should still belong to user2")
+
+				// Node 2 is genuinely tagged; user_id must be cleared.
+				node2 := byHostname["node2"]
+				require.NotNil(t, node2, "node2 should exist")
+				assert.True(t, node2.IsTagged(), "node2 should be tagged")
+				assert.Nil(t, node2.UserID, "node2 (tagged) should have user_id cleared")
+
+				// Node 3 had tags='[]' (untagged); user_id preserved.
+				node3 := byHostname["node3"]
+				require.NotNil(t, node3, "node3 should exist")
+				assert.False(t, node3.IsTagged(), "node3 with tags='[]' should be untagged")
+				require.NotNil(t, node3.UserID, "node3 should keep its user assigned")
+				assert.Equal(t, uint(1), *node3.UserID, "node3 should still belong to user1")
+
+				// Node 4 had tags='' (untagged); user_id preserved.
+				node4 := byHostname["node4"]
+				require.NotNil(t, node4, "node4 should exist")
+				assert.False(t, node4.IsTagged(), "node4 with tags='' should be untagged")
+				require.NotNil(t, node4.UserID, "node4 should keep its user assigned")
+				assert.Equal(t, uint(1), *node4.UserID, "node4 should still belong to user1")
+			},
+		},
+		// Test for the null-tags user_id recovery migration. Databases that
+		// already upgraded to 0.29.0 had user_id wrongly cleared on untagged
+		// nodes with tags='null'. The recovery migration re-derives user_id
+		// from the node's pre-auth key where one exists.
+		// Fixes: https://github.com/juanfont/headscale/issues/3323
+		{
+			dbPath: "testdata/sqlite/recover_null_tags_user_id_migration_test.sql",
+			wantFunc: func(t *testing.T, hsdb *HSDatabase) {
+				t.Helper()
+
+				nodes, err := Read(hsdb.DB, func(rx *gorm.DB) (types.Nodes, error) {
+					return ListNodes(rx)
+				})
+				require.NoError(t, err)
+				require.Len(t, nodes, 4, "should have all 4 nodes")
+
+				byHostname := make(map[string]*types.Node, len(nodes))
+				for _, n := range nodes {
+					byHostname[n.Hostname] = n
+				}
+
+				// Node 1: authkey-registered, orphaned by the bug. The recovery
+				// migration restores user_id from its pre-auth key (user2).
+				node1 := byHostname["node1"]
+				require.NotNil(t, node1, "node1 should exist")
+				require.NotNil(t, node1.UserID, "node1 user_id should be recovered")
+				assert.Equal(t, uint(2), *node1.UserID, "node1 should be recovered to user2")
+
+				// Node 2: genuinely tagged, correctly cleared. Must stay cleared.
+				node2 := byHostname["node2"]
+				require.NotNil(t, node2, "node2 should exist")
+				assert.True(t, node2.IsTagged(), "node2 should be tagged")
+				assert.Nil(t, node2.UserID, "node2 (tagged) must remain cleared")
+
+				// Node 3: CLI-registered, no pre-auth key. Unrecoverable.
+				node3 := byHostname["node3"]
+				require.NotNil(t, node3, "node3 should exist")
+				assert.Nil(t, node3.UserID, "node3 has no pre-auth key to recover from")
+
+				// Node 4: never orphaned; user_id must be untouched.
+				node4 := byHostname["node4"]
+				require.NotNil(t, node4, "node4 should exist")
+				require.NotNil(t, node4.UserID, "node4 user_id should be untouched")
+				assert.Equal(t, uint(1), *node4.UserID, "node4 should still belong to user1")
+			},
+		},
 	}
 
 	for _, tt := range tests {

hscontrol/db/node.go

@@ -143,27 +143,6 @@ func GetNodeByID(tx *gorm.DB, id types.NodeID) (*types.Node, error) {
 	return &mach, nil
 }
 
-func (hsdb *HSDatabase) GetNodeByMachineKey(machineKey key.MachinePublic) (*types.Node, error) {
-	return GetNodeByMachineKey(hsdb.DB, machineKey)
-}
-
-// GetNodeByMachineKey finds a [types.Node] by its [key.MachinePublic] and returns the [types.Node] struct.
-func GetNodeByMachineKey(
-	tx *gorm.DB,
-	machineKey key.MachinePublic,
-) (*types.Node, error) {
-	mach := types.Node{}
-	if result := tx.
-		Preload("AuthKey").
-		Preload("AuthKey.User").
-		Preload("User").
-		First(&mach, "machine_key = ?", machineKey.String()); result.Error != nil {
-		return nil, result.Error
-	}
-
-	return &mach, nil
-}
-
 func (hsdb *HSDatabase) GetNodeByNodeKey(nodeKey key.NodePublic) (*types.Node, error) {
 	return GetNodeByNodeKey(hsdb.DB, nodeKey)
 }
@@ -297,12 +276,16 @@ func RegisterNodeForTest(tx *gorm.DB, node types.Node, ipv4 *netip.Addr, ipv6 *n
 
 	logEvent.Msg("registering test node")
 
-	// If the a new node is registered with the same machine key, to the same user,
-	// update the existing node.
-	// If the same node is registered again, but to a new user, then that is considered
-	// a new node.
-	oldNode, _ := GetNodeByMachineKey(tx, node.MachineKey)
-	if oldNode != nil && oldNode.UserID == node.UserID {
+	// Reuse the existing node's identity only when the same machine
+	// re-registers for the same user; a different user is a new node. Match on
+	// (machine_key, user_id) precisely - a machine key can map to several nodes
+	// (one per user), so a machine-key-only lookup would be ambiguous.
+	var oldNode types.Node
+
+	err := tx.
+		Where("machine_key = ? AND user_id = ?", node.MachineKey.String(), node.UserID).
+		First(&oldNode).Error
+	if err == nil {
 		node.ID = oldNode.ID
 		node.GivenName = oldNode.GivenName
 		node.ApprovedRoutes = oldNode.ApprovedRoutes

hscontrol/db/preauth_keys.go

@@ -15,7 +15,10 @@ import (
 )
 
 var (
-	ErrPreAuthKeyNotFound          = errors.New("auth-key not found")
+	// ErrPreAuthKeyNotFound wraps gorm.ErrRecordNotFound so an unknown or
+	// deleted key is treated as a missing record by callers, which the
+	// registration handler maps to a 401 rather than a raw server error.
+	ErrPreAuthKeyNotFound          = fmt.Errorf("auth-key not found: %w", gorm.ErrRecordNotFound)
 	ErrPreAuthKeyExpired           = errors.New("auth-key expired")
 	ErrSingleUseAuthKeyHasBeenUsed = errors.New("auth-key has already been used")
 	ErrUserMismatch                = errors.New("user mismatch")

hscontrol/db/preauth_keys_test.go

@@ -487,3 +487,16 @@ func TestUsePreAuthKeyAtomicCAS(t *testing.T) {
 		"second UsePreAuthKey error must be a PAKError, got: %v", err)
 	assert.Equal(t, "authkey already used", pakErr.Error())
 }
+
+// TestGetPreAuthKeyUnknownMapsToRecordNotFound ensures an unknown (or deleted)
+// pre-auth key resolves to a record-not-found error, which the registration
+// handler maps to a 401 rather than a raw server error.
+func TestGetPreAuthKeyUnknownMapsToRecordNotFound(t *testing.T) {
+	db, err := newSQLiteTestDB()
+	require.NoError(t, err)
+
+	_, err = db.GetPreAuthKey("nonexistent-key")
+	require.Error(t, err)
+	require.ErrorIs(t, err, gorm.ErrRecordNotFound,
+		"unknown pre-auth key must map to record-not-found (handled as 401)")
+}

hscontrol/db/testdata/sqlite/null_tags_user_id_migration_test.sql

@@ -0,0 +1,85 @@
+-- Test SQL dump for the clear-tagged-node-user-id migration
+-- (202602201200-clear-tagged-node-user-id) against nodes whose tags
+-- column holds the JSON literal 'null'.
+--
+-- A nil Strings slice marshals to the JSON literal `null`, so pre-0.29
+-- databases contain untagged nodes with tags='null'. The migration's
+-- WHERE clause (tags IS NOT NULL AND tags != '[]' AND tags != '') treats
+-- the 4-character string 'null' as "tagged" and wrongly clears user_id,
+-- detaching the node from its owning user on upgrade.
+-- Fixes: https://github.com/juanfont/headscale/issues/3323
+
+PRAGMA foreign_keys=OFF;
+BEGIN TRANSACTION;
+
+-- Migrations table: every entry BEFORE clear-tagged-node-user-id has been
+-- applied. That migration is intentionally absent so it runs against this dump.
+CREATE TABLE `migrations` (`id` text,PRIMARY KEY (`id`));
+INSERT INTO migrations VALUES('202312101416');
+INSERT INTO migrations VALUES('202312101430');
+INSERT INTO migrations VALUES('202402151347');
+INSERT INTO migrations VALUES('2024041121742');
+INSERT INTO migrations VALUES('202406021630');
+INSERT INTO migrations VALUES('202409271400');
+INSERT INTO migrations VALUES('202407191627');
+INSERT INTO migrations VALUES('202408181235');
+INSERT INTO migrations VALUES('202501221827');
+INSERT INTO migrations VALUES('202501311657');
+INSERT INTO migrations VALUES('202502070949');
+INSERT INTO migrations VALUES('202502131714');
+INSERT INTO migrations VALUES('202502171819');
+INSERT INTO migrations VALUES('202505091439');
+INSERT INTO migrations VALUES('202505141324');
+INSERT INTO migrations VALUES('202507021200');
+INSERT INTO migrations VALUES('202510311551');
+INSERT INTO migrations VALUES('202511101554-drop-old-idx');
+INSERT INTO migrations VALUES('202511011637-preauthkey-bcrypt');
+INSERT INTO migrations VALUES('202511122344-remove-newline-index');
+INSERT INTO migrations VALUES('202511131445-node-forced-tags-to-tags');
+INSERT INTO migrations VALUES('202601121700-migrate-hostinfo-request-tags');
+
+-- Users table
+CREATE TABLE `users` (`id` integer PRIMARY KEY AUTOINCREMENT,`created_at` datetime,`updated_at` datetime,`deleted_at` datetime,`name` text,`display_name` text,`email` text,`provider_identifier` text,`provider` text,`profile_pic_url` text);
+INSERT INTO users VALUES(1,'2024-01-01 00:00:00+00:00','2024-01-01 00:00:00+00:00',NULL,'user1','User One','user1@example.com',NULL,NULL,NULL);
+INSERT INTO users VALUES(2,'2024-01-01 00:00:00+00:00','2024-01-01 00:00:00+00:00',NULL,'user2','User Two','user2@example.com',NULL,NULL,NULL);
+
+-- Pre-auth keys table
+CREATE TABLE `pre_auth_keys` (`id` integer PRIMARY KEY AUTOINCREMENT,`key` text,`user_id` integer,`reusable` numeric,`ephemeral` numeric DEFAULT false,`used` numeric DEFAULT false,`tags` text,`created_at` datetime,`expiration` datetime,`prefix` text,`hash` blob,CONSTRAINT `fk_pre_auth_keys_user` FOREIGN KEY (`user_id`) REFERENCES `users`(`id`) ON DELETE SET NULL);
+
+-- API keys table
+CREATE TABLE `api_keys` (`id` integer PRIMARY KEY AUTOINCREMENT,`prefix` text,`hash` blob,`created_at` datetime,`expiration` datetime,`last_seen` datetime);
+
+-- Nodes table - current schema (after the tags rename + last_seen/expiry reordering)
+CREATE TABLE IF NOT EXISTS "nodes" (`id` integer PRIMARY KEY AUTOINCREMENT,`machine_key` text,`node_key` text,`disco_key` text,`endpoints` text,`host_info` text,`ipv4` text,`ipv6` text,`hostname` text,`given_name` varchar(63),`user_id` integer,`register_method` text,`tags` text,`auth_key_id` integer,`last_seen` datetime,`expiry` datetime,`approved_routes` text,`created_at` datetime,`updated_at` datetime,`deleted_at` datetime,CONSTRAINT `fk_nodes_user` FOREIGN KEY (`user_id`) REFERENCES `users`(`id`) ON DELETE CASCADE,CONSTRAINT `fk_nodes_auth_key` FOREIGN KEY (`auth_key_id`) REFERENCES `pre_auth_keys`(`id`));
+
+-- Node 1: tags='null' (untagged, nil slice marshalled to JSON null), owned by user2.
+-- After migration: user_id MUST be preserved (this is the bug).
+INSERT INTO nodes VALUES(1,'mkey:a0ab77456320823945ae0331823e3c0d516fae9585bd42698dfa1ac3d7679e01','nodekey:7c84167ab68f494942de14deb83587fd841843de2bac105b6c670048c1605501','discokey:53075b3c6cad3b62a2a29caea61beeb93f66b8c75cb89dac465236a5bbf57701','[]','{}','100.64.0.1','fd7a:115c:a1e0::1','node1','node1',2,'cli','null',NULL,'2024-01-01 00:00:00+00:00',NULL,'[]','2024-01-01 00:00:00+00:00','2024-01-01 00:00:00+00:00',NULL);
+
+-- Node 2: genuinely tagged, owned by user1.
+-- After migration: user_id MUST be cleared to NULL (tagged nodes are owned by tags).
+INSERT INTO nodes VALUES(2,'mkey:a0ab77456320823945ae0331823e3c0d516fae9585bd42698dfa1ac3d7679e02','nodekey:7c84167ab68f494942de14deb83587fd841843de2bac105b6c670048c1605502','discokey:53075b3c6cad3b62a2a29caea61beeb93f66b8c75cb89dac465236a5bbf57702','[]','{}','100.64.0.2','fd7a:115c:a1e0::2','node2','node2',1,'cli','["tag:server"]',NULL,'2024-01-01 00:00:00+00:00',NULL,'[]','2024-01-01 00:00:00+00:00','2024-01-01 00:00:00+00:00',NULL);
+
+-- Node 3: empty-array tags (untagged), owned by user1.
+-- After migration: user_id MUST be preserved.
+INSERT INTO nodes VALUES(3,'mkey:a0ab77456320823945ae0331823e3c0d516fae9585bd42698dfa1ac3d7679e03','nodekey:7c84167ab68f494942de14deb83587fd841843de2bac105b6c670048c1605503','discokey:53075b3c6cad3b62a2a29caea61beeb93f66b8c75cb89dac465236a5bbf57703','[]','{}','100.64.0.3','fd7a:115c:a1e0::3','node3','node3',1,'cli','[]',NULL,'2024-01-01 00:00:00+00:00',NULL,'[]','2024-01-01 00:00:00+00:00','2024-01-01 00:00:00+00:00',NULL);
+
+-- Node 4: empty-string tags (untagged), owned by user1.
+-- After migration: user_id MUST be preserved.
+INSERT INTO nodes VALUES(4,'mkey:a0ab77456320823945ae0331823e3c0d516fae9585bd42698dfa1ac3d7679e04','nodekey:7c84167ab68f494942de14deb83587fd841843de2bac105b6c670048c1605504','discokey:53075b3c6cad3b62a2a29caea61beeb93f66b8c75cb89dac465236a5bbf57704','[]','{}','100.64.0.4','fd7a:115c:a1e0::4','node4','node4',1,'cli','',NULL,'2024-01-01 00:00:00+00:00',NULL,'[]','2024-01-01 00:00:00+00:00','2024-01-01 00:00:00+00:00',NULL);
+
+-- Policies table (empty)
+CREATE TABLE `policies` (`id` integer PRIMARY KEY AUTOINCREMENT,`created_at` datetime,`updated_at` datetime,`deleted_at` datetime,`data` text);
+
+DELETE FROM sqlite_sequence;
+INSERT INTO sqlite_sequence VALUES('users',2);
+INSERT INTO sqlite_sequence VALUES('nodes',4);
+CREATE INDEX idx_users_deleted_at ON users(deleted_at);
+CREATE UNIQUE INDEX idx_api_keys_prefix ON api_keys(prefix);
+CREATE INDEX idx_policies_deleted_at ON policies(deleted_at);
+CREATE UNIQUE INDEX idx_provider_identifier ON users(provider_identifier) WHERE provider_identifier IS NOT NULL;
+CREATE UNIQUE INDEX idx_name_provider_identifier ON users(name, provider_identifier);
+CREATE UNIQUE INDEX idx_name_no_provider_identifier ON users(name) WHERE provider_identifier IS NULL;
+CREATE UNIQUE INDEX IF NOT EXISTS idx_pre_auth_keys_prefix ON pre_auth_keys(prefix) WHERE prefix IS NOT NULL AND prefix != '';
+
+COMMIT;

hscontrol/db/testdata/sqlite/recover_null_tags_user_id_migration_test.sql

@@ -0,0 +1,87 @@
+-- Test SQL dump for the null-tags user_id RECOVERY migration.
+--
+-- Represents a database that already upgraded to 0.29.0, where the buggy
+-- clear-tagged-node-user-id migration (202602201200) already cleared
+-- user_id on untagged nodes whose tags column held 'null'. The recovery
+-- migration runs against this state and re-derives user_id from the node's
+-- pre-auth key where possible.
+-- Fixes: https://github.com/juanfont/headscale/issues/3323
+
+PRAGMA foreign_keys=OFF;
+BEGIN TRANSACTION;
+
+-- Migrations table: everything through the current last migration has been
+-- applied (this DB already ran the buggy clear-tagged migration). The new
+-- recovery migration is intentionally absent so it runs against this dump.
+CREATE TABLE `migrations` (`id` text,PRIMARY KEY (`id`));
+INSERT INTO migrations VALUES('202312101416');
+INSERT INTO migrations VALUES('202312101430');
+INSERT INTO migrations VALUES('202402151347');
+INSERT INTO migrations VALUES('2024041121742');
+INSERT INTO migrations VALUES('202406021630');
+INSERT INTO migrations VALUES('202409271400');
+INSERT INTO migrations VALUES('202407191627');
+INSERT INTO migrations VALUES('202408181235');
+INSERT INTO migrations VALUES('202501221827');
+INSERT INTO migrations VALUES('202501311657');
+INSERT INTO migrations VALUES('202502070949');
+INSERT INTO migrations VALUES('202502131714');
+INSERT INTO migrations VALUES('202502171819');
+INSERT INTO migrations VALUES('202505091439');
+INSERT INTO migrations VALUES('202505141324');
+INSERT INTO migrations VALUES('202507021200');
+INSERT INTO migrations VALUES('202510311551');
+INSERT INTO migrations VALUES('202511101554-drop-old-idx');
+INSERT INTO migrations VALUES('202511011637-preauthkey-bcrypt');
+INSERT INTO migrations VALUES('202511122344-remove-newline-index');
+INSERT INTO migrations VALUES('202511131445-node-forced-tags-to-tags');
+INSERT INTO migrations VALUES('202601121700-migrate-hostinfo-request-tags');
+INSERT INTO migrations VALUES('202602201200-clear-tagged-node-user-id');
+INSERT INTO migrations VALUES('202605221435-clear-zero-time-node-expiry');
+
+-- Users table
+CREATE TABLE `users` (`id` integer PRIMARY KEY AUTOINCREMENT,`created_at` datetime,`updated_at` datetime,`deleted_at` datetime,`name` text,`display_name` text,`email` text,`provider_identifier` text,`provider` text,`profile_pic_url` text);
+INSERT INTO users VALUES(1,'2024-01-01 00:00:00+00:00','2024-01-01 00:00:00+00:00',NULL,'user1','User One','user1@example.com',NULL,NULL,NULL);
+INSERT INTO users VALUES(2,'2024-01-01 00:00:00+00:00','2024-01-01 00:00:00+00:00',NULL,'user2','User Two','user2@example.com',NULL,NULL,NULL);
+
+-- Pre-auth keys table. Key 1 belongs to user2, key 2 to user1.
+CREATE TABLE `pre_auth_keys` (`id` integer PRIMARY KEY AUTOINCREMENT,`key` text,`user_id` integer,`reusable` numeric,`ephemeral` numeric DEFAULT false,`used` numeric DEFAULT false,`tags` text,`created_at` datetime,`expiration` datetime,`prefix` text,`hash` blob,CONSTRAINT `fk_pre_auth_keys_user` FOREIGN KEY (`user_id`) REFERENCES `users`(`id`) ON DELETE SET NULL);
+INSERT INTO pre_auth_keys VALUES(1,NULL,2,1,false,true,NULL,'2024-01-01 00:00:00+00:00',NULL,'pak1',NULL);
+INSERT INTO pre_auth_keys VALUES(2,NULL,1,1,false,true,NULL,'2024-01-01 00:00:00+00:00',NULL,'pak2',NULL);
+
+-- API keys table
+CREATE TABLE `api_keys` (`id` integer PRIMARY KEY AUTOINCREMENT,`prefix` text,`hash` blob,`created_at` datetime,`expiration` datetime,`last_seen` datetime);
+
+-- Nodes table
+CREATE TABLE IF NOT EXISTS "nodes" (`id` integer PRIMARY KEY AUTOINCREMENT,`machine_key` text,`node_key` text,`disco_key` text,`endpoints` text,`host_info` text,`ipv4` text,`ipv6` text,`hostname` text,`given_name` varchar(63),`user_id` integer,`register_method` text,`tags` text,`auth_key_id` integer,`last_seen` datetime,`expiry` datetime,`approved_routes` text,`created_at` datetime,`updated_at` datetime,`deleted_at` datetime,CONSTRAINT `fk_nodes_user` FOREIGN KEY (`user_id`) REFERENCES `users`(`id`) ON DELETE CASCADE,CONSTRAINT `fk_nodes_auth_key` FOREIGN KEY (`auth_key_id`) REFERENCES `pre_auth_keys`(`id`));
+
+-- Node 1: authkey-registered, tags='null', already orphaned (user_id NULL) by
+-- the buggy migration. auth_key_id=1 (user2). Recovery: user_id -> 2.
+INSERT INTO nodes VALUES(1,'mkey:a0ab77456320823945ae0331823e3c0d516fae9585bd42698dfa1ac3d7679e01','nodekey:7c84167ab68f494942de14deb83587fd841843de2bac105b6c670048c1605501','discokey:53075b3c6cad3b62a2a29caea61beeb93f66b8c75cb89dac465236a5bbf57701','[]','{}','100.64.0.1','fd7a:115c:a1e0::1','node1','node1',NULL,'authkey','null',1,'2024-01-01 00:00:00+00:00',NULL,'[]','2024-01-01 00:00:00+00:00','2024-01-01 00:00:00+00:00',NULL);
+
+-- Node 2: genuinely tagged, user_id correctly cleared. Must stay NULL.
+INSERT INTO nodes VALUES(2,'mkey:a0ab77456320823945ae0331823e3c0d516fae9585bd42698dfa1ac3d7679e02','nodekey:7c84167ab68f494942de14deb83587fd841843de2bac105b6c670048c1605502','discokey:53075b3c6cad3b62a2a29caea61beeb93f66b8c75cb89dac465236a5bbf57702','[]','{}','100.64.0.2','fd7a:115c:a1e0::2','node2','node2',NULL,'authkey','["tag:server"]',2,'2024-01-01 00:00:00+00:00',NULL,'[]','2024-01-01 00:00:00+00:00','2024-01-01 00:00:00+00:00',NULL);
+
+-- Node 3: CLI-registered, tags='null', orphaned, no auth_key_id.
+-- Unrecoverable: must stay NULL.
+INSERT INTO nodes VALUES(3,'mkey:a0ab77456320823945ae0331823e3c0d516fae9585bd42698dfa1ac3d7679e03','nodekey:7c84167ab68f494942de14deb83587fd841843de2bac105b6c670048c1605503','discokey:53075b3c6cad3b62a2a29caea61beeb93f66b8c75cb89dac465236a5bbf57703','[]','{}','100.64.0.3','fd7a:115c:a1e0::3','node3','node3',NULL,'cli','null',NULL,'2024-01-01 00:00:00+00:00',NULL,'[]','2024-01-01 00:00:00+00:00','2024-01-01 00:00:00+00:00',NULL);
+
+-- Node 4: authkey-registered, untouched (user_id still set). Must stay user1.
+INSERT INTO nodes VALUES(4,'mkey:a0ab77456320823945ae0331823e3c0d516fae9585bd42698dfa1ac3d7679e04','nodekey:7c84167ab68f494942de14deb83587fd841843de2bac105b6c670048c1605504','discokey:53075b3c6cad3b62a2a29caea61beeb93f66b8c75cb89dac465236a5bbf57704','[]','{}','100.64.0.4','fd7a:115c:a1e0::4','node4','node4',1,'authkey','null',2,'2024-01-01 00:00:00+00:00',NULL,'[]','2024-01-01 00:00:00+00:00','2024-01-01 00:00:00+00:00',NULL);
+
+-- Policies table (empty)
+CREATE TABLE `policies` (`id` integer PRIMARY KEY AUTOINCREMENT,`created_at` datetime,`updated_at` datetime,`deleted_at` datetime,`data` text);
+
+DELETE FROM sqlite_sequence;
+INSERT INTO sqlite_sequence VALUES('users',2);
+INSERT INTO sqlite_sequence VALUES('pre_auth_keys',2);
+INSERT INTO sqlite_sequence VALUES('nodes',4);
+CREATE INDEX idx_users_deleted_at ON users(deleted_at);
+CREATE UNIQUE INDEX idx_api_keys_prefix ON api_keys(prefix);
+CREATE INDEX idx_policies_deleted_at ON policies(deleted_at);
+CREATE UNIQUE INDEX idx_provider_identifier ON users(provider_identifier) WHERE provider_identifier IS NOT NULL;
+CREATE UNIQUE INDEX idx_name_provider_identifier ON users(name, provider_identifier);
+CREATE UNIQUE INDEX idx_name_no_provider_identifier ON users(name) WHERE provider_identifier IS NULL;
+CREATE UNIQUE INDEX IF NOT EXISTS idx_pre_auth_keys_prefix ON pre_auth_keys(prefix) WHERE prefix IS NOT NULL AND prefix != '';
+
+COMMIT;

hscontrol/state/auth_tagged_expiry_test.go

@@ -10,6 +10,7 @@ import (
 	"github.com/juanfont/headscale/hscontrol/util"
 	"github.com/stretchr/testify/require"
 	"tailscale.com/tailcfg"
+	"tailscale.com/types/key"
 )
 
 // TestTaggedReauthKeepsNilExpiry ensures that when an existing tagged node
@@ -92,3 +93,388 @@ func TestTaggedReauthKeepsNilExpiry(t *testing.T) {
 	require.Nil(t, finalNode.AsStruct().Expiry,
 		"tagged node must keep nil key expiry (tagged nodes never expire)")
 }
+
+// TestTaggedReauthWithReusedUserPAK reproduces issue #3312: a containerized
+// node registered with a user-owned one-shot pre-auth key, then converted to a
+// tagged node (UserID cleared to NULL), is logged out when the container
+// restarts and re-registers with the SAME, now-used TS_AUTHKEY.
+//
+// Root cause: findExistingNodeForPAK (state.go) looks the node up by the PAK's
+// owning user (alice). After tagging, the node is indexed under UserID(0), so
+// the same-user machine-key lookup misses, the re-registration fast-path is
+// skipped, and the already-used one-shot PAK is re-validated and rejected with
+// "authkey already used" — logging the node out.
+//
+// https://github.com/juanfont/headscale/issues/3312
+func TestTaggedReauthWithReusedUserPAK(t *testing.T) {
+	dbPath := t.TempDir() + "/headscale.db"
+	cfg := persistTestConfig(dbPath)
+
+	s, err := NewState(cfg)
+	require.NoError(t, err)
+	t.Cleanup(func() { _ = s.Close() })
+
+	user := s.CreateUserForTest("authkey-user")
+
+	policy := fmt.Sprintf(`{"tagOwners":{"tag:foo":["%s@"]}}`, user.Name)
+	_, err = s.SetPolicy([]byte(policy))
+	require.NoError(t, err)
+
+	// One-shot, user-owned PAK: `headscale preauthkeys create -u 1`.
+	pak, err := s.CreatePreAuthKey(user.TypedID(), false, false, nil, nil)
+	require.NoError(t, err)
+
+	machineKey := key.NewMachine()
+	nodeKey := key.NewNode()
+
+	regReq := tailcfg.RegisterRequest{
+		Auth:     &tailcfg.RegisterResponseAuth{AuthKey: pak.Key},
+		NodeKey:  nodeKey.Public(),
+		Hostinfo: &tailcfg.Hostinfo{Hostname: "authkey-node"},
+		Expiry:   time.Now().Add(24 * time.Hour),
+	}
+
+	// First registration: node joins as alice, the one-shot PAK is consumed.
+	first, _, err := s.HandleNodeFromPreAuthKey(regReq, machineKey.Public())
+	require.NoError(t, err)
+	require.True(t, first.Valid())
+	nodeID := first.ID()
+
+	// `headscale nodes tag -t tag:foo`: convert to a tagged node. This clears
+	// both UserID and User (state.SetNodeTags), diverging the node's ownership
+	// from the still-user-owned PAK.
+	tagged, _, err := s.SetNodeTags(nodeID, []string{"tag:foo"})
+	require.NoError(t, err)
+	require.True(t, tagged.IsTagged(), "precondition: node must be tagged")
+
+	// Container restart: the same node re-registers with the SAME, now-used
+	// one-shot TS_AUTHKEY. The machine key proves identity, so this must
+	// succeed. It currently fails with "authkey already used".
+	second, _, err := s.HandleNodeFromPreAuthKey(regReq, machineKey.Public())
+	require.NoError(t, err,
+		"re-registration with a reused user PAK on a tagged node must not be rejected")
+	require.True(t, second.Valid())
+	require.True(t, second.IsTagged(), "node must remain tagged after re-registration")
+	require.Equal(t, nodeID, second.ID(),
+		"must update the existing node, not create a new one")
+}
+
+// reregisterExpiredUserNodeWithSpentKey registers a user-owned node with a
+// one-shot key, forces it into the expired state, and re-registers with the
+// same spent key. sameNodeKey distinguishes the two re-auth shapes:
+//   - false: the node rotates its node key (normal tailscale client on re-auth)
+//   - true:  the node reuses its node key
+//
+// In both cases an expired node is genuinely re-authenticating and must present
+// a valid key; a spent one-shot key must be rejected.
+func reregisterExpiredUserNodeWithSpentKey(t *testing.T, sameNodeKey bool) (types.NodeView, error) {
+	t.Helper()
+
+	dbPath := t.TempDir() + "/headscale.db"
+	cfg := persistTestConfig(dbPath)
+
+	s, err := NewState(cfg)
+	require.NoError(t, err)
+	t.Cleanup(func() { _ = s.Close() })
+
+	user := s.CreateUserForTest("expired-user")
+
+	pak, err := s.CreatePreAuthKey(user.TypedID(), false, false, nil, nil)
+	require.NoError(t, err)
+
+	machineKey := key.NewMachine()
+	nodeKey := key.NewNode()
+
+	regReq := tailcfg.RegisterRequest{
+		Auth:     &tailcfg.RegisterResponseAuth{AuthKey: pak.Key},
+		NodeKey:  nodeKey.Public(),
+		Hostinfo: &tailcfg.Hostinfo{Hostname: "expired-node"},
+		Expiry:   time.Now().Add(24 * time.Hour),
+	}
+
+	first, _, err := s.HandleNodeFromPreAuthKey(regReq, machineKey.Public())
+	require.NoError(t, err)
+	require.True(t, first.Valid())
+	require.False(t, first.IsTagged(), "precondition: node must be user-owned")
+
+	// Force the node into the expired state.
+	past := time.Now().Add(-1 * time.Hour)
+	_, ok := s.nodeStore.UpdateNode(first.ID(), func(n *types.Node) {
+		n.Expiry = &past
+	})
+	require.True(t, ok)
+
+	reReg := regReq
+	if !sameNodeKey {
+		reReg.NodeKey = key.NewNode().Public()
+	}
+
+	node, _, err := s.HandleNodeFromPreAuthKey(reReg, machineKey.Public())
+
+	return node, err
+}
+
+// TestExpiredUserNodeReusedOneShotKey_RotatedNodeKey: a node rotating its node
+// key on re-auth is already a key rotation, so the key is re-validated.
+func TestExpiredUserNodeReusedOneShotKey_RotatedNodeKey(t *testing.T) {
+	_, err := reregisterExpiredUserNodeWithSpentKey(t, false)
+	require.Error(t, err,
+		"expired node re-authenticating with a rotated node key must present a valid key")
+	require.Contains(t, err.Error(), "authkey already used")
+}
+
+// TestExpiredUserNodeReusedOneShotKey_SameNodeKey: the security boundary must
+// not depend on the client rotating its node key. An expired node re-using its
+// node key must still re-validate the key, otherwise a spent one-shot key
+// silently re-authorises it.
+func TestExpiredUserNodeReusedOneShotKey_SameNodeKey(t *testing.T) {
+	_, err := reregisterExpiredUserNodeWithSpentKey(t, true)
+	require.Error(t, err,
+		"expired node re-registering with the same node key must re-validate its key")
+	require.Contains(t, err.Error(), "authkey already used")
+}
+
+// TestReusableUserPAKReauthOnTaggedNodeNoDuplicate guards against a reusable
+// user pre-auth key creating a second node when it is re-presented for a node
+// that has since been converted to tagged. The node must be updated in place.
+func TestReusableUserPAKReauthOnTaggedNodeNoDuplicate(t *testing.T) {
+	dbPath := t.TempDir() + "/headscale.db"
+	cfg := persistTestConfig(dbPath)
+
+	s, err := NewState(cfg)
+	require.NoError(t, err)
+	t.Cleanup(func() { _ = s.Close() })
+
+	user := s.CreateUserForTest("reusable-user")
+
+	policy := fmt.Sprintf(`{"tagOwners":{"tag:foo":["%s@"]}}`, user.Name)
+	_, err = s.SetPolicy([]byte(policy))
+	require.NoError(t, err)
+
+	pak, err := s.CreatePreAuthKey(user.TypedID(), true, false, nil, nil)
+	require.NoError(t, err)
+
+	machineKey := key.NewMachine()
+	regReq := tailcfg.RegisterRequest{
+		Auth:     &tailcfg.RegisterResponseAuth{AuthKey: pak.Key},
+		NodeKey:  key.NewNode().Public(),
+		Hostinfo: &tailcfg.Hostinfo{Hostname: "reusable-node"},
+		Expiry:   time.Now().Add(24 * time.Hour),
+	}
+
+	first, _, err := s.HandleNodeFromPreAuthKey(regReq, machineKey.Public())
+	require.NoError(t, err)
+
+	_, _, err = s.SetNodeTags(first.ID(), []string{"tag:foo"})
+	require.NoError(t, err)
+
+	second, _, err := s.HandleNodeFromPreAuthKey(regReq, machineKey.Public())
+	require.NoError(t, err)
+	require.True(t, second.IsTagged())
+	require.Equal(t, first.ID(), second.ID(), "must update in place, not duplicate")
+	require.Equal(t, 1, s.ListNodes().Len(), "machine must map to exactly one node")
+}
+
+// TestTaggedPAKReauthConvertsUserOwnedNode ensures presenting a tagged pre-auth
+// key for a machine that already has a user-owned node converts that node in
+// place (same machine, new ownership) rather than creating a duplicate.
+func TestTaggedPAKReauthConvertsUserOwnedNode(t *testing.T) {
+	dbPath := t.TempDir() + "/headscale.db"
+	cfg := persistTestConfig(dbPath)
+
+	s, err := NewState(cfg)
+	require.NoError(t, err)
+	t.Cleanup(func() { _ = s.Close() })
+
+	user := s.CreateUserForTest("owner")
+
+	userPak, err := s.CreatePreAuthKey(user.TypedID(), true, false, nil, nil)
+	require.NoError(t, err)
+
+	machineKey := key.NewMachine()
+	nodeKey := key.NewNode()
+	regReq := tailcfg.RegisterRequest{
+		Auth:     &tailcfg.RegisterResponseAuth{AuthKey: userPak.Key},
+		NodeKey:  nodeKey.Public(),
+		Hostinfo: &tailcfg.Hostinfo{Hostname: "owned-node"},
+		Expiry:   time.Now().Add(24 * time.Hour),
+	}
+
+	owned, _, err := s.HandleNodeFromPreAuthKey(regReq, machineKey.Public())
+	require.NoError(t, err)
+	require.False(t, owned.IsTagged(), "precondition: node is user-owned")
+
+	// A tags-only key re-registers the same machine (same node key).
+	taggedPak, err := s.CreatePreAuthKey(nil, true, false, nil, []string{"tag:foo"})
+	require.NoError(t, err)
+
+	convReq := regReq
+	convReq.Auth = &tailcfg.RegisterResponseAuth{AuthKey: taggedPak.Key}
+
+	converted, _, err := s.HandleNodeFromPreAuthKey(convReq, machineKey.Public())
+	require.NoError(t, err)
+	require.Equal(t, owned.ID(), converted.ID(), "must convert in place, not duplicate")
+	require.True(t, converted.IsTagged(), "node must become tagged")
+	require.Equal(t, []string{"tag:foo"}, converted.Tags().AsSlice())
+	require.Equal(t, 1, s.ListNodes().Len(), "machine must map to exactly one node")
+}
+
+// registerTwoUsersOnOneMachine registers two user-owned nodes that share a
+// machine key (the "create new, do not transfer" multi-user device state) and
+// returns the State and the shared machine key.
+func registerTwoUsersOnOneMachine(t *testing.T) (*State, key.MachinePublic, types.NodeID) {
+	t.Helper()
+
+	dbPath := t.TempDir() + "/headscale.db"
+	cfg := persistTestConfig(dbPath)
+
+	s, err := NewState(cfg)
+	require.NoError(t, err)
+	t.Cleanup(func() { _ = s.Close() })
+
+	u1 := s.CreateUserForTest("u1")
+	u2 := s.CreateUserForTest("u2")
+	mk := key.NewMachine()
+
+	reg := func(pakUser *types.User) types.NodeView {
+		pak, err := s.CreatePreAuthKey(pakUser.TypedID(), true, false, nil, nil)
+		require.NoError(t, err)
+		n, _, err := s.HandleNodeFromPreAuthKey(tailcfg.RegisterRequest{
+			Auth:     &tailcfg.RegisterResponseAuth{AuthKey: pak.Key},
+			NodeKey:  key.NewNode().Public(),
+			Hostinfo: &tailcfg.Hostinfo{Hostname: "multi"},
+			Expiry:   time.Now().Add(24 * time.Hour),
+		}, mk.Public())
+		require.NoError(t, err)
+
+		return n
+	}
+
+	n1 := reg(u1)
+	n2 := reg(u2)
+	require.NotEqual(t, n1.ID(), n2.ID(), "precondition: two distinct nodes share the machine key")
+	require.Equal(t, 2, s.ListNodes().Len())
+
+	return s, mk.Public(), n1.ID()
+}
+
+// TestTaggedPAKReauthRejectsAmbiguousMultiUserNode: a tagged pre-auth key on a
+// machine that has more than one user-owned node cannot know which to convert,
+// so the registration is rejected rather than converting an arbitrary one and
+// orphaning the rest.
+func TestTaggedPAKReauthRejectsAmbiguousMultiUserNode(t *testing.T) {
+	s, mk, _ := registerTwoUsersOnOneMachine(t)
+
+	taggedPak, err := s.CreatePreAuthKey(nil, true, false, nil, []string{"tag:foo"})
+	require.NoError(t, err)
+
+	_, _, err = s.HandleNodeFromPreAuthKey(tailcfg.RegisterRequest{
+		Auth:     &tailcfg.RegisterResponseAuth{AuthKey: taggedPak.Key},
+		NodeKey:  key.NewNode().Public(),
+		Hostinfo: &tailcfg.Hostinfo{Hostname: "multi"},
+		Expiry:   time.Now().Add(24 * time.Hour),
+	}, mk)
+	require.ErrorIs(t, err, ErrAmbiguousNodeOwnership)
+	require.Equal(t, 2, s.ListNodes().Len(), "no node created or converted on rejection")
+}
+
+// TestAuthPathRejectsTaggedAndUserCoexistence: if a machine key ends up with
+// both a tagged node and a user-owned node (impossible per validateNodeOwnership,
+// but reachable by tagging one node of a multi-user device via the admin path),
+// an OIDC re-auth must reject rather than silently converting the tagged node
+// and orphaning the user-owned one.
+func TestAuthPathRejectsTaggedAndUserCoexistence(t *testing.T) {
+	s, mk, n1 := registerTwoUsersOnOneMachine(t)
+
+	// Tag one of the two user-owned nodes -> {0: tagged, u2: user-owned} coexist.
+	_, err := s.SetPolicy([]byte(`{"tagOwners":{"tag:foo":["u1@"]}}`))
+	require.NoError(t, err)
+	tagged, _, err := s.SetNodeTags(n1, []string{"tag:foo"})
+	require.NoError(t, err)
+	require.True(t, tagged.IsTagged())
+
+	// A third user authenticates the same machine via OIDC.
+	u3 := s.CreateUserForTest("u3")
+	regData := &types.RegistrationData{
+		MachineKey: mk,
+		NodeKey:    key.NewNode().Public(),
+		Hostname:   "multi",
+		Hostinfo:   &tailcfg.Hostinfo{Hostname: "multi"},
+	}
+	authID := types.MustAuthID()
+	s.SetAuthCacheEntry(authID, types.NewRegisterAuthRequest(regData))
+
+	_, _, err = s.HandleNodeFromAuthPath(authID, types.UserID(u3.ID), nil, util.RegisterMethodOIDC)
+	require.ErrorIs(t, err, ErrAmbiguousNodeOwnership)
+}
+
+// TestTaggedNodeCanHaveKeyExpiry matches Tailscale: a tagged node has key
+// expiry disabled by default, but it can still be set explicitly (e.g. via
+// `headscale nodes expire`).
+func TestTaggedNodeCanHaveKeyExpiry(t *testing.T) {
+	dbPath := t.TempDir() + "/headscale.db"
+	cfg := persistTestConfig(dbPath)
+
+	s, err := NewState(cfg)
+	require.NoError(t, err)
+	t.Cleanup(func() { _ = s.Close() })
+
+	_, err = s.SetPolicy([]byte(`{"tagOwners":{"tag:foo":["tagger@"]}}`))
+	require.NoError(t, err)
+
+	pak, err := s.CreatePreAuthKey(nil, true, false, nil, []string{"tag:foo"})
+	require.NoError(t, err)
+
+	regReq := tailcfg.RegisterRequest{
+		Auth:     &tailcfg.RegisterResponseAuth{AuthKey: pak.Key},
+		NodeKey:  key.NewNode().Public(),
+		Hostinfo: &tailcfg.Hostinfo{Hostname: "tagged-node"},
+	}
+	node, _, err := s.HandleNodeFromPreAuthKey(regReq, key.NewMachine().Public())
+	require.NoError(t, err)
+	require.True(t, node.IsTagged())
+	require.Nil(t, node.AsStruct().Expiry, "key expiry is disabled by default for tagged nodes")
+
+	expiry := time.Now().Add(24 * time.Hour)
+	after, _, err := s.SetNodeExpiry(node.ID(), &expiry)
+	require.NoError(t, err)
+	require.True(t, after.IsTagged(), "node stays tagged")
+	require.NotNil(t, after.AsStruct().Expiry, "expiry can be set on a tagged node")
+	require.Equal(t, expiry.Unix(), after.AsStruct().Expiry.Unix())
+}
+
+// TestTaggingPreservesNodeExpiry matches Tailscale: changing a node's tags does
+// not alter its key expiry (expiry only changes on re-authentication).
+func TestTaggingPreservesNodeExpiry(t *testing.T) {
+	dbPath := t.TempDir() + "/headscale.db"
+	cfg := persistTestConfig(dbPath)
+
+	s, err := NewState(cfg)
+	require.NoError(t, err)
+	t.Cleanup(func() { _ = s.Close() })
+
+	user := s.CreateUserForTest("owner")
+
+	_, err = s.SetPolicy(fmt.Appendf(nil, `{"tagOwners":{"tag:foo":["%s@"]}}`, user.Name))
+	require.NoError(t, err)
+
+	pak, err := s.CreatePreAuthKey(user.TypedID(), false, false, nil, nil)
+	require.NoError(t, err)
+
+	expiry := time.Now().Add(24 * time.Hour)
+	regReq := tailcfg.RegisterRequest{
+		Auth:     &tailcfg.RegisterResponseAuth{AuthKey: pak.Key},
+		NodeKey:  key.NewNode().Public(),
+		Hostinfo: &tailcfg.Hostinfo{Hostname: "owned-node"},
+		Expiry:   expiry,
+	}
+	node, _, err := s.HandleNodeFromPreAuthKey(regReq, key.NewMachine().Public())
+	require.NoError(t, err)
+	require.NotNil(t, node.AsStruct().Expiry, "precondition: user node has an expiry")
+
+	tagged, _, err := s.SetNodeTags(node.ID(), []string{"tag:foo"})
+	require.NoError(t, err)
+	require.True(t, tagged.IsTagged())
+	require.NotNil(t, tagged.AsStruct().Expiry, "tag change must not clear expiry")
+	require.Equal(t, expiry.Unix(), tagged.AsStruct().Expiry.Unix())
+}

hscontrol/state/endpoint_test.go

@@ -111,3 +111,88 @@ func TestEndpointStorageInNodeStore(t *testing.T) {
 		}
 	}
 }
+
+// TestEndpointBroadcastWorthy verifies the gate that decides whether an
+// endpoint-only delta is worth fanning out to peers as an incremental
+// PeersChangedPatch. A delta that only adds STUN-derived endpoints (or only
+// removes endpoints) is suppressed: it is churny and unlikely to be useful,
+// and disco's callMeMaybe re-derives STUN paths anyway. Only deltas that
+// introduce a genuinely useful (non-STUN) endpoint are broadcast-worthy.
+func TestEndpointBroadcastWorthy(t *testing.T) {
+	local := netip.MustParseAddrPort("192.168.1.5:41641")
+	local2 := netip.MustParseAddrPort("192.168.1.6:41641")
+	stun := netip.MustParseAddrPort("203.0.113.7:41641")
+	stun2 := netip.MustParseAddrPort("203.0.113.8:41641")
+	portmap := netip.MustParseAddrPort("198.51.100.9:41641")
+
+	tests := []struct {
+		name    string
+		stored  []netip.AddrPort
+		newEPs  []netip.AddrPort
+		newType []tailcfg.EndpointType
+		want    bool
+	}{
+		{
+			name:    "adds only a STUN endpoint - suppress",
+			stored:  []netip.AddrPort{local},
+			newEPs:  []netip.AddrPort{local, stun},
+			newType: []tailcfg.EndpointType{tailcfg.EndpointLocal, tailcfg.EndpointSTUN},
+			want:    false,
+		},
+		{
+			name:    "adds only STUN4LocalPort - suppress",
+			stored:  []netip.AddrPort{local},
+			newEPs:  []netip.AddrPort{local, stun},
+			newType: []tailcfg.EndpointType{tailcfg.EndpointLocal, tailcfg.EndpointSTUN4LocalPort},
+			want:    false,
+		},
+		{
+			name:    "adds a useful local endpoint - broadcast",
+			stored:  []netip.AddrPort{stun},
+			newEPs:  []netip.AddrPort{stun, local},
+			newType: []tailcfg.EndpointType{tailcfg.EndpointSTUN, tailcfg.EndpointLocal},
+			want:    true,
+		},
+		{
+			name:    "adds a useful portmapped endpoint - broadcast",
+			stored:  []netip.AddrPort{local},
+			newEPs:  []netip.AddrPort{local, portmap},
+			newType: []tailcfg.EndpointType{tailcfg.EndpointLocal, tailcfg.EndpointPortmapped},
+			want:    true,
+		},
+		{
+			name:    "pure shrink, no additions - suppress",
+			stored:  []netip.AddrPort{local, local2},
+			newEPs:  []netip.AddrPort{local},
+			newType: []tailcfg.EndpointType{tailcfg.EndpointLocal},
+			want:    false,
+		},
+		{
+			name:   "nil types (older client) adding endpoint - broadcast",
+			stored: []netip.AddrPort{local},
+			newEPs: []netip.AddrPort{local, local2},
+			want:   true,
+		},
+		{
+			name:    "only STUN endpoints churn (replace one STUN with another) - suppress",
+			stored:  []netip.AddrPort{local, stun},
+			newEPs:  []netip.AddrPort{local, stun2},
+			newType: []tailcfg.EndpointType{tailcfg.EndpointLocal, tailcfg.EndpointSTUN},
+			want:    false,
+		},
+		{
+			name:    "mixed add: one STUN and one useful - broadcast",
+			stored:  []netip.AddrPort{local},
+			newEPs:  []netip.AddrPort{local, stun, local2},
+			newType: []tailcfg.EndpointType{tailcfg.EndpointLocal, tailcfg.EndpointSTUN, tailcfg.EndpointLocal},
+			want:    true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := endpointBroadcastWorthy(tt.stored, tt.newEPs, tt.newType)
+			assert.Equal(t, tt.want, got)
+		})
+	}
+}

hscontrol/state/node_store.go

@@ -787,44 +787,27 @@ func (s *NodeStore) GetNodeByNodeKey(nodeKey key.NodePublic) (types.NodeView, bo
 	return nodeView, exists
 }
 
-// GetNodeByMachineKey returns a node by its machine key and user ID. The bool indicates if the node exists.
-func (s *NodeStore) GetNodeByMachineKey(machineKey key.MachinePublic, userID types.UserID) (types.NodeView, bool) {
-	timer := prometheus.NewTimer(nodeStoreOperationDuration.WithLabelValues("get_by_machine_key"))
+// GetNodesByMachineKeyAllUsers returns every node sharing machineKey, keyed by
+// owning UserID. Tagged nodes are indexed under UserID(0) (the tagged sentinel);
+// user-owned nodes under their owning UserID. Returns an empty map if none.
+//
+// One machine key can map to several nodes (the same device registered by
+// different users via the "create new, do not transfer" path). Exposing the
+// whole set lets callers decide with full context — index [userID] for an exact
+// match, [0] for a tagged node, or reject when the set is ambiguous — rather
+// than guessing from a single arbitrary pick.
+func (s *NodeStore) GetNodesByMachineKeyAllUsers(machineKey key.MachinePublic) map[types.UserID]types.NodeView {
+	timer := prometheus.NewTimer(nodeStoreOperationDuration.WithLabelValues("get_nodes_by_machine_key_all_users"))
 	defer timer.ObserveDuration()
 
-	nodeStoreOperations.WithLabelValues("get_by_machine_key").Inc()
+	nodeStoreOperations.WithLabelValues("get_nodes_by_machine_key_all_users").Inc()
 
-	snapshot := s.data.Load()
-	if userMap, exists := snapshot.nodesByMachineKey[machineKey]; exists {
-		if node, exists := userMap[userID]; exists {
-			return node, true
-		}
-	}
+	userMap := s.data.Load().nodesByMachineKey[machineKey]
 
-	return types.NodeView{}, false
-}
+	out := make(map[types.UserID]types.NodeView, len(userMap))
+	maps.Copy(out, userMap)
 
-// GetNodeByMachineKeyAnyUser returns the first node with the given machine key,
-// regardless of which user it belongs to. This is useful for scenarios like
-// transferring a node to a different user when re-authenticating with a
-// different user's auth key.
-// If multiple nodes exist with the same machine key (different users), the
-// first one found is returned (order is not guaranteed).
-func (s *NodeStore) GetNodeByMachineKeyAnyUser(machineKey key.MachinePublic) (types.NodeView, bool) {
-	timer := prometheus.NewTimer(nodeStoreOperationDuration.WithLabelValues("get_by_machine_key_any_user"))
-	defer timer.ObserveDuration()
-
-	nodeStoreOperations.WithLabelValues("get_by_machine_key_any_user").Inc()
-
-	snapshot := s.data.Load()
-	if userMap, exists := snapshot.nodesByMachineKey[machineKey]; exists {
-		// Return the first node found (order not guaranteed due to map iteration)
-		for _, node := range userMap {
-			return node, true
-		}
-	}
-
-	return types.NodeView{}, false
+	return out
 }
 
 // DebugString returns debug information about the [NodeStore].

hscontrol/state/node_store_test.go

@@ -1321,3 +1321,63 @@ func TestRebuildPeerMapsWithChangedPeersFunc(t *testing.T) {
 	assert.Equal(t, 1, peers1.Len(), "ListPeers for node1 should return 1")
 	assert.Equal(t, 1, peers2.Len(), "ListPeers for node2 should return 1")
 }
+
+// TestGetNodesByMachineKeyAllUsers ensures the lookup returns every node sharing
+// a machine key keyed by owning UserID (tagged nodes under UserID(0)), so callers
+// see the full set instead of a single arbitrary pick.
+func TestGetNodesByMachineKeyAllUsers(t *testing.T) {
+	mk := key.NewMachine().Public()
+
+	t.Run("empty when absent", func(t *testing.T) {
+		store := NewNodeStore(nil, allowAllPeersFunc, TestBatchSize, TestBatchTimeout)
+
+		store.Start()
+		defer store.Stop()
+
+		require.Empty(t, store.GetNodesByMachineKeyAllUsers(mk))
+	})
+
+	t.Run("returns all user-owned nodes keyed by user", func(t *testing.T) {
+		store := NewNodeStore(nil, allowAllPeersFunc, TestBatchSize, TestBatchTimeout)
+
+		store.Start()
+		defer store.Stop()
+
+		n1 := createTestNode(1, 1, "user1", "node1")
+		n1.MachineKey = mk
+		n2 := createTestNode(2, 2, "user2", "node2")
+		n2.MachineKey = mk
+
+		store.PutNode(n1)
+		store.PutNode(n2)
+
+		all := store.GetNodesByMachineKeyAllUsers(mk)
+		require.Len(t, all, 2)
+		require.Equal(t, types.NodeID(1), all[types.UserID(1)].ID())
+		require.Equal(t, types.NodeID(2), all[types.UserID(2)].ID())
+	})
+
+	t.Run("tagged node indexed under UserID(0)", func(t *testing.T) {
+		store := NewNodeStore(nil, allowAllPeersFunc, TestBatchSize, TestBatchTimeout)
+
+		store.Start()
+		defer store.Stop()
+
+		owned := createTestNode(1, 1, "user1", "node1")
+		owned.MachineKey = mk
+		tagged := createTestNode(3, 3, "user3", "node3")
+		tagged.MachineKey = mk
+		tagged.UserID = nil
+		tagged.User = nil
+		tagged.Tags = []string{"tag:foo"}
+
+		store.PutNode(owned)
+		store.PutNode(tagged)
+
+		all := store.GetNodesByMachineKeyAllUsers(mk)
+		require.Len(t, all, 2)
+		require.Equal(t, types.NodeID(1), all[types.UserID(1)].ID())
+		require.True(t, all[types.UserID(0)].IsTagged())
+		require.Equal(t, types.NodeID(3), all[types.UserID(0)].ID())
+	})
+}

hscontrol/state/persist_test.go

@@ -1,14 +1,18 @@
 package state
 
 import (
+	"errors"
 	"net/netip"
+	"sync"
 	"testing"
+	"time"
 
 	"github.com/juanfont/headscale/hscontrol/db"
 	"github.com/juanfont/headscale/hscontrol/types"
 	"github.com/juanfont/headscale/hscontrol/util"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"gorm.io/gorm"
 	"tailscale.com/tailcfg"
 	"tailscale.com/types/key"
 )
@@ -325,3 +329,256 @@ func TestReauthRejectsNodeKeyClaimedByAnotherMachine(t *testing.T) {
 	require.Error(t, err,
 		"re-auth claiming a NodeKey bound to another machine must be rejected")
 }
+
+// TestReauthPreservesEndpointsWhenClientOmitsThem proves the re-auth/update
+// path keeps a node's live WireGuard endpoints when the originating
+// RegisterRequest carried none. Web/OIDC relogins report endpoints via
+// MapRequest, not register, so RegData.Endpoints is empty; wiping the stored
+// endpoints would advertise the re-keyed node to peers endpoint-less, which
+// drives head/unstable tailscale clients into one-way disco-deafness.
+func TestReauthPreservesEndpointsWhenClientOmitsThem(t *testing.T) {
+	dbPath := t.TempDir() + "/headscale.db"
+	cfg := persistTestConfig(dbPath)
+
+	database, err := db.NewHeadscaleDatabase(cfg)
+	require.NoError(t, err)
+
+	user := database.CreateUserForTest("user")
+	require.NoError(t, database.Close())
+
+	s, err := NewState(cfg)
+	require.NoError(t, err)
+	t.Cleanup(func() { _ = s.Close() })
+
+	machine := key.NewMachine()
+	endpoints := []netip.AddrPort{
+		netip.MustParseAddrPort("192.168.1.5:41641"),
+		netip.MustParseAddrPort("10.0.0.5:41641"),
+	}
+
+	// Node is registered and has reported live endpoints (as after its first
+	// MapRequest).
+	node, err := s.createAndSaveNewNode(newNodeParams{
+		User:           *user,
+		MachineKey:     machine.Public(),
+		NodeKey:        key.NewNode().Public(),
+		DiscoKey:       key.NewDisco().Public(),
+		Hostname:       "node",
+		Endpoints:      endpoints,
+		RegisterMethod: util.RegisterMethodCLI,
+	})
+	require.NoError(t, err)
+	require.Equal(t, endpoints, node.Endpoints().AsSlice(),
+		"precondition: node has live endpoints")
+
+	// Node re-authenticates, rotating its NodeKey. The RegisterRequest carries
+	// no endpoints.
+	updated, err := s.applyAuthNodeUpdate(authNodeUpdateParams{
+		ExistingNode: node,
+		RegData: &types.RegistrationData{
+			MachineKey: machine.Public(),
+			NodeKey:    key.NewNode().Public(),
+			Hostname:   "node",
+			Hostinfo:   &tailcfg.Hostinfo{},
+			Endpoints:  nil,
+		},
+		ValidHostinfo:  &tailcfg.Hostinfo{},
+		Hostname:       "node",
+		User:           user,
+		RegisterMethod: util.RegisterMethodCLI,
+	})
+	require.NoError(t, err)
+
+	assert.Equal(t, endpoints, updated.Endpoints().AsSlice(),
+		"re-auth without reported endpoints must preserve the node's live endpoints")
+}
+
+// TestReauthChange covers the decision both re-auth paths share: a same-user
+// relogin must be an incremental peer patch (so the tailscale client takes its
+// fast patch path), never a whole-node add (which strands a re-keyed,
+// momentarily-endpoint-less peer disco-deaf); a policy change forces a full
+// recompute; a new node is a whole-node add.
+func TestReauthChange(t *testing.T) {
+	n := types.Node{
+		ID:       7,
+		NodeKey:  key.NewNode().Public(),
+		DiscoKey: key.NewDisco().Public(),
+	}
+	node := n.View()
+
+	relogin := reauthChange(node, true, false)
+	assert.Len(t, relogin.PeerPatches, 1, "relogin must be a peer patch")
+	assert.Empty(t, relogin.PeersChanged, "relogin must not be a whole-node add")
+
+	added := reauthChange(node, false, false)
+	assert.Empty(t, added.PeerPatches)
+	assert.Len(t, added.PeersChanged, 1, "a new node must be a whole-node add")
+
+	pol := reauthChange(node, true, true)
+	assert.Empty(t, pol.PeerPatches, "a policy change must not be a peer patch")
+	assert.Empty(t, pol.PeersChanged)
+	assert.False(t, pol.IsEmpty(), "a policy change must be non-empty")
+}
+
+// TestPreAuthKeyReauthRejectsNodeKeyClaimedByAnotherMachine is the pre-auth-key
+// analogue of TestReauthRejectsNodeKeyClaimedByAnotherMachine: re-registering
+// via a pre-auth key must enforce the same 1:1 NodeKey<->MachineKey binding the
+// auth path and poll-time validation enforce, so a node cannot rotate its key
+// to a victim's and poison the NodeStore NodeKey index.
+func TestPreAuthKeyReauthRejectsNodeKeyClaimedByAnotherMachine(t *testing.T) {
+	dbPath := t.TempDir() + "/headscale.db"
+	cfg := persistTestConfig(dbPath)
+
+	s, err := NewState(cfg)
+	require.NoError(t, err)
+	t.Cleanup(func() { _ = s.Close() })
+
+	attacker := s.CreateUserForTest("attacker")
+	victim := s.CreateUserForTest("victim")
+
+	victimMachine := key.NewMachine()
+	victimNodeKey := key.NewNode()
+	_, err = s.createAndSaveNewNode(newNodeParams{
+		User:           *victim,
+		MachineKey:     victimMachine.Public(),
+		NodeKey:        victimNodeKey.Public(),
+		DiscoKey:       key.NewDisco().Public(),
+		Hostname:       "victim",
+		RegisterMethod: util.RegisterMethodCLI,
+	})
+	require.NoError(t, err)
+
+	// Attacker registers its own node with a reusable pre-auth key.
+	pak, err := s.CreatePreAuthKey(attacker.TypedID(), true, false, nil, nil)
+	require.NoError(t, err)
+
+	attackerMachine := key.NewMachine()
+	regReq := tailcfg.RegisterRequest{
+		Auth:     &tailcfg.RegisterResponseAuth{AuthKey: pak.Key},
+		NodeKey:  key.NewNode().Public(),
+		Hostinfo: &tailcfg.Hostinfo{Hostname: "attacker"},
+		Expiry:   time.Now().Add(24 * time.Hour),
+	}
+	_, _, err = s.HandleNodeFromPreAuthKey(regReq, attackerMachine.Public())
+	require.NoError(t, err)
+
+	// Attacker re-registers its own node but supplies the victim's NodeKey.
+	attack := regReq
+	attack.NodeKey = victimNodeKey.Public()
+	_, _, err = s.HandleNodeFromPreAuthKey(attack, attackerMachine.Public())
+	require.ErrorIs(t, err, ErrNodeKeyInUse,
+		"pre-auth-key re-registration claiming another machine's NodeKey must be rejected")
+
+	// The victim still owns its NodeKey.
+	owner, ok := s.GetNodeByNodeKey(victimNodeKey.Public())
+	require.True(t, ok)
+	require.Equal(t, victimMachine.Public(), owner.MachineKey(),
+		"victim's NodeKey index entry must be untouched")
+}
+
+var errInjectedNodeUpdate = errors.New("injected node update failure")
+
+// TestPreAuthKeyReauthRevertsNodeStoreOnDBFailure ensures a failed database
+// write during pre-auth-key re-registration does not leave the NodeStore
+// holding a node key that was never persisted: a restart would reload the old
+// row and the client's current key would no longer resolve, locking it out.
+func TestPreAuthKeyReauthRevertsNodeStoreOnDBFailure(t *testing.T) {
+	dbPath := t.TempDir() + "/headscale.db"
+	cfg := persistTestConfig(dbPath)
+
+	s, err := NewState(cfg)
+	require.NoError(t, err)
+	t.Cleanup(func() { _ = s.Close() })
+
+	user := s.CreateUserForTest("reauth-user")
+
+	pak, err := s.CreatePreAuthKey(user.TypedID(), true, false, nil, nil)
+	require.NoError(t, err)
+
+	machineKey := key.NewMachine()
+	regReq := tailcfg.RegisterRequest{
+		Auth:     &tailcfg.RegisterResponseAuth{AuthKey: pak.Key},
+		NodeKey:  key.NewNode().Public(),
+		Hostinfo: &tailcfg.Hostinfo{Hostname: "reauth-node"},
+		Expiry:   time.Now().Add(24 * time.Hour),
+	}
+	node, _, err := s.HandleNodeFromPreAuthKey(regReq, machineKey.Public())
+	require.NoError(t, err)
+
+	origNodeKey := node.NodeKey()
+
+	// Fail the node row update so the re-registration's database write errors
+	// after the NodeStore has already been mutated.
+	require.NoError(t, s.db.DB.Callback().Update().Before("gorm:update").
+		Register("fail_node_update", func(tx *gorm.DB) {
+			if tx.Statement.Table == "nodes" {
+				_ = tx.AddError(errInjectedNodeUpdate)
+			}
+		}))
+
+	reReg := regReq
+	reReg.NodeKey = key.NewNode().Public() // rotate -> NodeStore mutation, then DB write fails
+	_, _, err = s.HandleNodeFromPreAuthKey(reReg, machineKey.Public())
+	require.NoError(t, s.db.DB.Callback().Update().Remove("fail_node_update"))
+	require.Error(t, err, "re-registration must fail when the database write fails")
+
+	got, ok := s.nodeStore.GetNode(node.ID())
+	require.True(t, ok)
+	require.Equal(t, origNodeKey, got.NodeKey(),
+		"NodeStore must revert to the persisted node key when the write fails")
+}
+
+// TestConcurrentPreAuthKeyRegistrationSameMachineKey ensures concurrent
+// registrations of the same machine key resolve to a single node. Without
+// serialising the find-then-create section, each request sees "no existing
+// node" and creates its own, leaving duplicate nodes and IP allocations for
+// one machine.
+func TestConcurrentPreAuthKeyRegistrationSameMachineKey(t *testing.T) {
+	dbPath := t.TempDir() + "/headscale.db"
+	cfg := persistTestConfig(dbPath)
+
+	s, err := NewState(cfg)
+	require.NoError(t, err)
+	t.Cleanup(func() { _ = s.Close() })
+
+	user := s.CreateUserForTest("concurrent-user")
+
+	pak, err := s.CreatePreAuthKey(user.TypedID(), true, false, nil, nil)
+	require.NoError(t, err)
+
+	machineKey := key.NewMachine()
+
+	const n = 12
+
+	var wg sync.WaitGroup
+
+	start := make(chan struct{})
+	errs := make(chan error, n)
+
+	for range n {
+		wg.Go(func() {
+			regReq := tailcfg.RegisterRequest{
+				Auth:     &tailcfg.RegisterResponseAuth{AuthKey: pak.Key},
+				NodeKey:  key.NewNode().Public(),
+				Hostinfo: &tailcfg.Hostinfo{Hostname: "concurrent-node"},
+				Expiry:   time.Now().Add(24 * time.Hour),
+			}
+
+			<-start
+
+			_, _, err := s.HandleNodeFromPreAuthKey(regReq, machineKey.Public())
+			errs <- err
+		})
+	}
+
+	close(start)
+	wg.Wait()
+	close(errs)
+
+	for err := range errs {
+		require.NoError(t, err)
+	}
+
+	require.Equal(t, 1, s.ListNodes().Len(),
+		"concurrent registrations of one machine key must yield a single node")
+}

hscontrol/state/state.go

@@ -32,6 +32,7 @@ import (
 	"github.com/juanfont/headscale/hscontrol/types/change"
 	"github.com/juanfont/headscale/hscontrol/util"
 	"github.com/juanfont/headscale/hscontrol/util/zlog/zf"
+	"github.com/puzpuzpuz/xsync/v4"
 	"github.com/rs/zerolog"
 	"github.com/rs/zerolog/log"
 	"gorm.io/gorm"
@@ -117,6 +118,13 @@ var ErrRegistrationExpired = errors.New("registration expired")
 // binding.
 var ErrNodeKeyInUse = errors.New("node key already in use by another machine")
 
+// ErrAmbiguousNodeOwnership is returned when a machine key maps to a set of
+// nodes from which the correct one to update or convert cannot be determined:
+// multiple user-owned candidates for a tagged conversion, or a tagged node and
+// a user-owned node coexisting (impossible per validateNodeOwnership). The
+// registration is rejected rather than mutating an arbitrarily-picked node.
+var ErrAmbiguousNodeOwnership = errors.New("machine key maps to ambiguous node ownership")
+
 // sshCheckPair identifies a (source, destination) node pair for
 // SSH check auth tracking.
 type sshCheckPair struct {
@@ -179,6 +187,22 @@ type State struct {
 	// persistNodeToDB so the database row always converges on [NodeStore]
 	// rather than being clobbered by a stale caller snapshot.
 	persistMu sync.Mutex
+
+	// registerLocks serialises registration per machine key so concurrent
+	// registrations of the same machine resolve to a single node instead of
+	// racing the find-then-create section and each creating their own.
+	// ponytail: entries are never pruned; bounded by distinct machine keys
+	// seen, add cleanup on node delete only if it ever matters.
+	registerLocks *xsync.Map[key.MachinePublic, *sync.Mutex]
+}
+
+// lockRegistration serialises registration for a single machine key and
+// returns the unlock function.
+func (s *State) lockRegistration(machineKey key.MachinePublic) func() {
+	mu, _ := s.registerLocks.LoadOrStore(machineKey, &sync.Mutex{})
+	mu.Lock()
+
+	return mu.Unlock
 }
 
 // NewState creates and initializes a new [State] instance, setting up the database,
@@ -272,7 +296,8 @@ func NewState(cfg *types.Config) (*State, error) {
 		nodeStore: nodeStore,
 		pings:     newPingTracker(),
 
-		sshCheckAuth: make(map[sshCheckPair]time.Time),
+		sshCheckAuth:  make(map[sshCheckPair]time.Time),
+		registerLocks: xsync.NewMap[key.MachinePublic, *sync.Mutex](),
 	}, nil
 }
 
@@ -727,12 +752,11 @@ func (s *State) GetNodeByNodeKey(nodeKey key.NodePublic) (types.NodeView, bool)
 	return s.nodeStore.GetNodeByNodeKey(nodeKey)
 }
 
-// GetNodeByMachineKey retrieves a node by its machine key and user ID.
-// The bool indicates if the node exists or is available (like "err not found").
-// The NodeView might be invalid, so it must be checked with .Valid(), which must be used to ensure
-// it isn't an invalid node (this is more of a node error or node is broken).
-func (s *State) GetNodeByMachineKey(machineKey key.MachinePublic, userID types.UserID) (types.NodeView, bool) {
-	return s.nodeStore.GetNodeByMachineKey(machineKey, userID)
+// GetNodesByMachineKeyAllUsers returns every node sharing the machine key,
+// keyed by owning UserID (tagged nodes under UserID(0)). See
+// [NodeStore.GetNodesByMachineKeyAllUsers].
+func (s *State) GetNodesByMachineKeyAllUsers(machineKey key.MachinePublic) map[types.UserID]types.NodeView {
+	return s.nodeStore.GetNodesByMachineKeyAllUsers(machineKey)
 }
 
 // ResolveNode looks up a node by numeric ID, IPv4/IPv6 address, given
@@ -1615,7 +1639,14 @@ func (s *State) applyAuthNodeUpdate(params authNodeUpdateParams) (types.NodeView
 			params.ValidHostinfo,
 		)
 
-		node.Endpoints = regData.Endpoints
+		// Preserve the node's live endpoints when the register request carried
+		// none. Web/OIDC relogins report endpoints via MapRequest, not register,
+		// so RegData.Endpoints is empty; clearing the stored set would advertise
+		// the re-keyed node with no way for peers to reach it. The first
+		// MapRequest restores the live set.
+		if len(regData.Endpoints) > 0 {
+			node.Endpoints = regData.Endpoints
+		}
 		// Do NOT reset IsOnline here. Online status is managed exclusively by
 		// [State.Connect]/[State.Disconnect] in the poll session lifecycle.
 		// Resetting it during re-registration causes a false offline blip: the
@@ -2021,16 +2052,37 @@ func (s *State) HandleNodeFromAuthPath(
 
 	// Lookup existing nodes
 	machineKey := regData.MachineKey
-	existingNodeSameUser, _ := s.nodeStore.GetNodeByMachineKey(machineKey, types.UserID(user.ID))
-	existingNodeAnyUser, _ := s.nodeStore.GetNodeByMachineKeyAnyUser(machineKey)
 
-	// Named conditions - describe WHAT we found, not HOW we check it
-	nodeExistsForSameUser := existingNodeSameUser.Valid()
-	nodeExistsForAnyUser := existingNodeAnyUser.Valid()
-	existingNodeIsTagged := nodeExistsForAnyUser && existingNodeAnyUser.IsTagged()
-	existingNodeOwnedByOtherUser := nodeExistsForAnyUser &&
-		!existingNodeIsTagged &&
-		existingNodeAnyUser.UserID().Get() != user.ID
+	// Serialise registration for this machine so concurrent auth callbacks
+	// resolve to a single node rather than racing the find-then-create section.
+	defer s.lockRegistration(machineKey)()
+
+	all := s.nodeStore.GetNodesByMachineKeyAllUsers(machineKey)
+
+	// Named conditions - describe WHAT we found, not HOW we check it.
+	existingNodeSameUser, nodeExistsForSameUser := all[types.UserID(user.ID)]
+
+	taggedNode, hasTagged := all[0]
+	existingNodeIsTagged := hasTagged && taggedNode.IsTagged()
+
+	var existingNodeOtherUser types.NodeView
+
+	existingNodeOwnedByOtherUser := false
+
+	for uid, n := range all {
+		if uid != 0 && uid != types.UserID(user.ID) && !n.IsTagged() {
+			existingNodeOtherUser = n
+			existingNodeOwnedByOtherUser = true
+		}
+	}
+
+	// A tagged node and a user-owned node cannot legitimately share a machine
+	// key (validateNodeOwnership enforces tags XOR user ownership). If both are
+	// present the machine key is in a corrupt/ambiguous state; reject rather
+	// than converting an arbitrary node and orphaning the other.
+	if existingNodeIsTagged && (nodeExistsForSameUser || existingNodeOwnedByOtherUser) {
+		return types.NodeView{}, change.Change{}, ErrAmbiguousNodeOwnership
+	}
 
 	// Create logger with common fields for all auth operations
 	logger := log.With().
@@ -2060,7 +2112,7 @@ func (s *State) HandleNodeFromAuthPath(
 			return types.NodeView{}, change.Change{}, err
 		}
 	} else if existingNodeIsTagged {
-		updateParams.ExistingNode = existingNodeAnyUser
+		updateParams.ExistingNode = taggedNode
 		updateParams.IsConvertFromTag = true
 
 		finalNode, err = s.applyAuthNodeUpdate(updateParams)
@@ -2068,7 +2120,7 @@ func (s *State) HandleNodeFromAuthPath(
 			return types.NodeView{}, change.Change{}, err
 		}
 	} else if existingNodeOwnedByOtherUser {
-		oldUser := existingNodeAnyUser.User()
+		oldUser := existingNodeOtherUser.User()
 
 		oldUserName := ""
 		if oldUser.Valid() {
@@ -2076,14 +2128,14 @@ func (s *State) HandleNodeFromAuthPath(
 		}
 
 		logger.Info().
-			Str(zf.ExistingNodeName, existingNodeAnyUser.Hostname()).
-			Uint64(zf.ExistingNodeID, existingNodeAnyUser.ID().Uint64()).
+			Str(zf.ExistingNodeName, existingNodeOtherUser.Hostname()).
+			Uint64(zf.ExistingNodeID, existingNodeOtherUser.ID().Uint64()).
 			Str(zf.OldUser, oldUserName).
 			Msg("Creating new node for different user (same machine key exists for another user)")
 
 		finalNode, err = s.createNewNodeFromAuth(
 			logger, user, regData, hostname, hostinfo,
-			expiry, registrationMethod, existingNodeAnyUser,
+			expiry, registrationMethod, existingNodeOtherUser,
 		)
 		if err != nil {
 			return types.NodeView{}, change.Change{}, err
@@ -2115,14 +2167,12 @@ func (s *State) HandleNodeFromAuthPath(
 		return finalNode, change.NodeAdded(finalNode.ID()), fmt.Errorf("updating policy manager nodes: %w", err)
 	}
 
-	var c change.Change
-	if !usersChange.IsEmpty() || !nodesChange.IsEmpty() {
-		c = change.PolicyChange()
-	} else {
-		c = change.NodeAdded(finalNode.ID())
-	}
+	policyChanged := !usersChange.IsEmpty() || !nodesChange.IsEmpty()
 
-	return finalNode, c, nil
+	// nodeExistsForSameUser is true only for a same-user relogin; a tag->user
+	// conversion is excluded, as it changes the peer's User — a structural
+	// change peers must see in full, not a key-rotation patch.
+	return finalNode, reauthChange(finalNode, nodeExistsForSameUser, policyChanged), nil
 }
 
 // createNewNodeFromAuth creates a new node during auth callback.
@@ -2164,21 +2214,60 @@ func (s *State) createNewNodeFromAuth(
 func (s *State) findExistingNodeForPAK(
 	machineKey key.MachinePublic,
 	pak *types.PreAuthKey,
-) (types.NodeView, bool) {
+) (types.NodeView, bool, error) {
+	all := s.nodeStore.GetNodesByMachineKeyAllUsers(machineKey)
+
 	if pak.User != nil {
-		node, exists := s.nodeStore.GetNodeByMachineKey(machineKey, types.UserID(pak.User.ID))
-		if exists {
-			return node, true
+		if node, ok := all[types.UserID(pak.User.ID)]; ok {
+			return node, true, nil
+		}
+
+		// The node may have been converted to a tagged node since it first
+		// registered (SetNodeTags clears UserID, re-indexing it under UserID(0)).
+		// It is still the same machine, proven by the machine key, so recognise
+		// it for re-registration instead of re-validating the spent key or
+		// creating a duplicate node. Re-registration preserves the node's tagged
+		// ownership. See https://github.com/juanfont/headscale/issues/3312.
+		if node, ok := all[0]; ok && node.IsTagged() {
+			return node, true, nil
+		}
+
+		return types.NodeView{}, false, nil
+	}
+
+	// A tagged key re-registers the same machine regardless of how it is
+	// currently owned. An existing tagged node is a plain re-registration. A
+	// single user-owned node is converted to tagged in place (handled by the
+	// caller). More than one user-owned node is ambiguous - we cannot know
+	// which to convert - so reject rather than convert an arbitrary one and
+	// orphan the rest.
+	if pak.IsTagged() {
+		if node, ok := all[0]; ok && node.IsTagged() {
+			return node, true, nil
+		}
+
+		var userOwned types.NodeView
+
+		count := 0
+
+		for uid, node := range all {
+			if uid != 0 && !node.IsTagged() {
+				userOwned = node
+				count++
+			}
+		}
+
+		switch count {
+		case 0:
+			return types.NodeView{}, false, nil
+		case 1:
+			return userOwned, true, nil
+		default:
+			return types.NodeView{}, false, ErrAmbiguousNodeOwnership
 		}
 	}
 
-	// Tagged nodes have nil UserID, so they are indexed under UserID(0)
-	// in nodesByMachineKey. Check there for tagged PAK re-registration.
-	if pak.IsTagged() {
-		return s.nodeStore.GetNodeByMachineKey(machineKey, 0)
-	}
-
-	return types.NodeView{}, false
+	return types.NodeView{}, false, nil
 }
 
 //nolint:gocyclo // sequential validation/update/create paths with security-sensitive ordering
@@ -2186,6 +2275,10 @@ func (s *State) HandleNodeFromPreAuthKey(
 	regReq tailcfg.RegisterRequest,
 	machineKey key.MachinePublic,
 ) (types.NodeView, change.Change, error) {
+	// Serialise registration for this machine so concurrent restarts resolve
+	// to a single node rather than racing the find-then-create section.
+	defer s.lockRegistration(machineKey)()
+
 	pak, err := s.GetPreAuthKey(regReq.Auth.AuthKey)
 	if err != nil {
 		return types.NodeView{}, change.Change{}, err
@@ -2200,7 +2293,10 @@ func (s *State) HandleNodeFromPreAuthKey(
 		return types.TaggedDevices.Name
 	}
 
-	existingNodeSameUser, existsSameUser := s.findExistingNodeForPAK(machineKey, pak)
+	existingNodeSameUser, existsSameUser, err := s.findExistingNodeForPAK(machineKey, pak)
+	if err != nil {
+		return types.NodeView{}, change.Change{}, err
+	}
 
 	// For existing nodes, skip validation if:
 	// 1. MachineKey matches (cryptographic proof of machine identity)
@@ -2219,10 +2315,24 @@ func (s *State) HandleNodeFromPreAuthKey(
 	isNodeKeyRotation := existsSameUser && existingNodeSameUser.Valid() &&
 		existingNodeSameUser.NodeKey() != regReq.NodeKey
 
-	if isExistingNodeReregistering && !isNodeKeyRotation {
-		// Existing node re-registering with same NodeKey: skip validation.
-		// Pre-auth keys are only needed for initial authentication. Critical for
-		// containers that run "tailscale up --authkey=KEY" on every restart.
+	// An expired node is genuinely re-authenticating, not just waking up, so it
+	// must present a valid key. Without this a node that re-uses its NodeKey
+	// after expiry would skip validation and be re-authorised with a spent or
+	// expired key; the boundary must not depend on the client rotating its key.
+	isExpired := existsSameUser && existingNodeSameUser.Valid() &&
+		existingNodeSameUser.IsExpired()
+
+	// A tagged key presented for a currently user-owned node converts that node
+	// to tagged. That is an ownership change, not a plain refresh, so it must
+	// present a valid key rather than ride the skip-validation fast-path.
+	isOwnershipConversion := existsSameUser && existingNodeSameUser.Valid() &&
+		pak.IsTagged() && !existingNodeSameUser.IsTagged()
+
+	if isExistingNodeReregistering && !isNodeKeyRotation && !isExpired && !isOwnershipConversion {
+		// Existing, still-valid node re-registering with same NodeKey: skip
+		// validation. Pre-auth keys are only needed for initial authentication.
+		// Critical for containers that run "tailscale up --authkey=KEY" on every
+		// restart.
 		log.Debug().
 			Caller().
 			Uint64(zf.NodeID, existingNodeSameUser.ID().Uint64()).
@@ -2278,6 +2388,23 @@ func (s *State) HandleNodeFromPreAuthKey(
 			Str(zf.UserName, pakUsername()).
 			Msg("Node re-registering with existing machine key and user, updating in place")
 
+		// Re-registration rotates the NodeKey to the client-supplied value.
+		// Enforce the same 1:1 NodeKey<->MachineKey binding the auth path
+		// (applyAuthNodeUpdate) and poll-time validation enforce: a NodeKey
+		// already bound to a different machine must not be claimed here, or a
+		// re-registering node could rotate its key to a victim's and poison the
+		// NodeStore NodeKey index, denying the victim service.
+		if existing, ok := s.nodeStore.GetNodeByNodeKey(regReq.NodeKey); ok &&
+			existing.MachineKey() != machineKey {
+			return types.NodeView{}, change.Change{}, ErrNodeKeyInUse
+		}
+
+		// Snapshot the pre-update node so the NodeStore can be rolled back if
+		// the database write below fails. The view points at the immutable
+		// pre-update snapshot (UpdateNode swaps in a new one), so this stays
+		// valid after the mutation.
+		priorNode := existingNodeSameUser.AsStruct()
+
 		// Update existing node - NodeStore first, then database
 		updatedNodeView, ok := s.nodeStore.UpdateNode(existingNodeSameUser.ID(), func(node *types.Node) {
 			node.NodeKey = regReq.NodeKey
@@ -2293,8 +2420,17 @@ func (s *State) HandleNodeFromPreAuthKey(
 			node.RegisterMethod = util.RegisterMethodAuthKey
 
 			// Tags from PreAuthKey are only applied during initial registration.
-			// On re-registration the node keeps its existing tags and ownership.
-			// Only update AuthKey reference.
+			// On re-registration the node keeps its existing tags and ownership,
+			// except when a tagged key converts a user-owned node: that adopts
+			// the key's tags and drops user ownership (tagged nodes are
+			// user-less and never expire). Only update AuthKey reference
+			// otherwise.
+			if pak.IsTagged() && !node.IsTagged() {
+				node.Tags = pak.Proto().GetAclTags()
+				node.UserID = nil
+				node.User = nil
+				node.Expiry = nil
+			}
 			node.AuthKey = pak
 			node.AuthKeyID = &pak.ID
 			// Do NOT reset IsOnline here. Online status is managed exclusively by
@@ -2349,6 +2485,13 @@ func (s *State) HandleNodeFromPreAuthKey(
 			return nil, nil //nolint:nilnil // intentional: transaction success
 		})
 		if err != nil {
+			// The NodeStore was updated before the database write. Roll it back
+			// so it does not advertise a registration the database rejected
+			// (e.g. a node key that a restart would not reload).
+			if priorNode != nil {
+				s.nodeStore.PutNode(*priorNode)
+			}
+
 			return types.NodeView{}, change.Change{}, fmt.Errorf("writing node to database: %w", err)
 		}
 
@@ -2363,24 +2506,29 @@ func (s *State) HandleNodeFromPreAuthKey(
 
 		finalNode = updatedNodeView
 	} else {
-		// Node does not exist for this user with this machine key
-		// Check if node exists with this machine key for a different user
-		existingNodeAnyUser, existsAnyUser := s.nodeStore.GetNodeByMachineKeyAnyUser(machineKey)
+		// Node does not exist for this user with this machine key.
+		// For a user-owned key, check whether the machine key is already held
+		// by a node belonging to a different user (tags-only keys skip this;
+		// tagged nodes have no owning user). Any such node yields the same
+		// outcome - create a new node for the new user, do not transfer - so a
+		// single representative is enough.
+		var differentUserNode types.NodeView
 
-		// For user-owned keys, check if node exists for a different user.
-		// Tags-only keys (pak.User == nil) skip this check.
-		// Tagged nodes are also skipped since they have no owning user.
-		existingIsUserOwned := existsAnyUser &&
-			existingNodeAnyUser.Valid() &&
-			!existingNodeAnyUser.IsTagged()
-		belongsToDifferentUser := pak.User != nil &&
-			existingIsUserOwned &&
-			existingNodeAnyUser.UserID().Get() != pak.User.ID
+		belongsToDifferentUser := false
+
+		if pak.User != nil {
+			for uid, node := range s.nodeStore.GetNodesByMachineKeyAllUsers(machineKey) {
+				if uid != 0 && !node.IsTagged() && uid != types.UserID(pak.User.ID) {
+					differentUserNode = node
+					belongsToDifferentUser = true
+				}
+			}
+		}
 
 		if belongsToDifferentUser {
 			// Node exists but belongs to a different user.
 			// Create a new node for the new user (do not transfer).
-			oldUser := existingNodeAnyUser.User()
+			oldUser := differentUserNode.User()
 
 			oldUserName := ""
 			if oldUser.Valid() {
@@ -2389,8 +2537,8 @@ func (s *State) HandleNodeFromPreAuthKey(
 
 			log.Info().
 				Caller().
-				Str(zf.ExistingNodeName, existingNodeAnyUser.Hostname()).
-				Uint64(zf.ExistingNodeID, existingNodeAnyUser.ID().Uint64()).
+				Str(zf.ExistingNodeName, differentUserNode.Hostname()).
+				Uint64(zf.ExistingNodeID, differentUserNode.ID().Uint64()).
 				Str(zf.MachineKey, machineKey.ShortString()).
 				Str(zf.OldUser, oldUserName).
 				Str(zf.NewUser, pakUsername()).
@@ -2430,7 +2578,7 @@ func (s *State) HandleNodeFromPreAuthKey(
 			Expiry:                 reqExpiry,
 			RegisterMethod:         util.RegisterMethodAuthKey,
 			PreAuthKey:             pak,
-			ExistingNodeForNetinfo: cmp.Or(existingNodeAnyUser, types.NodeView{}),
+			ExistingNodeForNetinfo: cmp.Or(differentUserNode, types.NodeView{}),
 		})
 		if err != nil {
 			return types.NodeView{}, change.Change{}, fmt.Errorf("creating new node: %w", err)
@@ -2448,14 +2596,27 @@ func (s *State) HandleNodeFromPreAuthKey(
 		return finalNode, change.NodeAdded(finalNode.ID()), fmt.Errorf("updating policy manager nodes: %w", err)
 	}
 
-	var c change.Change
-	if !usersChange.IsEmpty() || !nodesChange.IsEmpty() {
-		c = change.PolicyChange()
-	} else {
-		c = change.NodeAdded(finalNode.ID())
-	}
+	policyChanged := !usersChange.IsEmpty() || !nodesChange.IsEmpty()
 
-	return finalNode, c, nil
+	return finalNode, reauthChange(finalNode, existsSameUser, policyChanged), nil
+}
+
+// reauthChange returns the [change.Change] to broadcast after an authentication
+// that updated or created a node.
+//
+// A pure relogin (isRelogin: an existing node, same user, with only its NodeKey
+// rotated) is sent as a minimal incremental peer patch via [change.NodeKeyRotated]
+// rather than re-advertising the whole node. A policy change forces a full
+// recompute; any other (new) node is a whole-node add.
+func reauthChange(node types.NodeView, isRelogin, policyChanged bool) change.Change {
+	switch {
+	case policyChanged:
+		return change.PolicyChange()
+	case isRelogin:
+		return change.NodeKeyRotated(node)
+	default:
+		return change.NodeAdded(node.ID())
+	}
 }
 
 // updatePolicyManagerUsers updates the policy manager with current users.
@@ -2656,8 +2817,13 @@ func (s *State) UpdateNodeFromMapRequest(id types.NodeID, req tailcfg.MapRequest
 	updatedNode, ok := s.nodeStore.UpdateNode(id, func(currentNode *types.Node) {
 		peerChange := currentNode.PeerChangeFromMapRequest(req)
 
-		// Track what specifically changed
-		endpointChanged = peerChange.Endpoints != nil
+		// Track what specifically changed. An endpoint delta is only
+		// broadcast-worthy when it adds a useful (non-STUN) endpoint;
+		// STUN-only churn and pure shrinks are suppressed to reduce peer
+		// churn (see endpointBroadcastWorthy). The new set is still stored
+		// via ApplyPeerChange below regardless of this decision.
+		endpointChanged = peerChange.Endpoints != nil &&
+			endpointBroadcastWorthy(currentNode.Endpoints, req.Endpoints, req.EndpointTypes)
 		derpChanged = peerChange.DERPRegion != 0
 		hostinfoChanged = !hostinfoEqual(currentNode.View(), req.Hostinfo)
 
@@ -2841,6 +3007,63 @@ func (s *State) UpdateNodeFromMapRequest(id types.NodeID, req tailcfg.MapRequest
 	return buildMapRequestChangeResponse(id, updatedNode, hostinfoChanged, endpointChanged, derpChanged)
 }
 
+// endpointBroadcastWorthy reports whether an endpoint-only delta is worth
+// fanning out to peers as an incremental PeersChangedPatch. A delta that only
+// adds STUN-derived endpoints — or only removes endpoints — is suppressed:
+// bare STUN endpoints are unlikely to be open and churn a lot (the client
+// re-derives those paths over disco anyway), and a pure shrink is not worth
+// telling peers about. Suppressing this churn keeps peers' views stable.
+//
+// The decision is intentionally conservative: it gates the broadcast only,
+// not storage. The node's full endpoint set (STUN included) is still stored
+// and rides along the next substantive change or full MapResponse, so no
+// reachable path is permanently hidden from peers.
+//
+// Limitation: headscale stores bare []netip.AddrPort with no per-endpoint
+// type, so we can only classify the *new* request's endpoints (via the
+// parallel newTypes slice). We therefore gate on whether any newly-added
+// endpoint (present in new, absent from stored) is useful (non-STUN). When
+// newTypes is absent or shorter than newEPs (older clients), the unknown
+// endpoints are treated as useful, preserving the pre-existing always-broadcast
+// behaviour and never hiding a genuinely new endpoint.
+func endpointBroadcastWorthy(
+	stored, newEPs []netip.AddrPort,
+	newTypes []tailcfg.EndpointType,
+) bool {
+	storedSet := make(map[netip.AddrPort]struct{}, len(stored))
+	for _, ep := range stored {
+		storedSet[ep] = struct{}{}
+	}
+
+	for i, ep := range newEPs {
+		if _, ok := storedSet[ep]; ok {
+			// Already known to peers; not a newly-added endpoint.
+			continue
+		}
+
+		// A newly-added endpoint with no type information (older client)
+		// is treated as useful so we never hide a genuinely new endpoint.
+		t := tailcfg.EndpointUnknownType
+		if i < len(newTypes) {
+			t = newTypes[i]
+		}
+
+		if isUsefulEndpointType(t) {
+			return true
+		}
+	}
+
+	return false
+}
+
+// isUsefulEndpointType reports whether an endpoint type is worth eagerly
+// broadcasting to peers. STUN-derived endpoints are excluded because they are
+// churny and unlikely to be directly reachable; magicsock's disco handles
+// establishing those paths.
+func isUsefulEndpointType(t tailcfg.EndpointType) bool {
+	return t != tailcfg.EndpointSTUN && t != tailcfg.EndpointSTUN4LocalPort
+}
+
 // buildMapRequestChangeResponse determines the appropriate response type for a [tailcfg.MapRequest] update.
 // Hostinfo changes require a full update, while endpoint/DERP changes can use lightweight patches.
 func buildMapRequestChangeResponse(

hscontrol/types/change/change.go

@@ -490,6 +490,35 @@ func EndpointOrDERPUpdate(id types.NodeID, patch *tailcfg.PeerChange) Change {
 	return c
 }
 
+// NodeKeyRotated returns a [Change] for a node re-logging in: its NodeKey (and
+// possibly DiscoKey, key expiry, or endpoints) changed, but nothing structural
+// did. Peers only need those changed fields, so it is sent as the minimal
+// incremental [tailcfg.PeerChange] patch rather than re-advertising the whole
+// node — the smallest update that conveys the rotation, and the least
+// disruptive for peers reconciling it.
+func NodeKeyRotated(node types.NodeView) Change {
+	nk := node.NodeKey()
+	dk := node.DiscoKey()
+
+	// KeyExpiry is always set: the zero value clears any prior expiry on the
+	// peer (un-expire), and a non-zero value carries the new expiry.
+	var expiry time.Time
+	if e, ok := node.Expiry().GetOk(); ok {
+		expiry = e
+	}
+
+	c := PeerPatched("node key rotated (relogin)", &tailcfg.PeerChange{
+		NodeID:    tailcfg.NodeID(node.ID()), //nolint:gosec // NodeID is bounded
+		Key:       &nk,
+		DiscoKey:  &dk,
+		KeyExpiry: &expiry,
+		Endpoints: node.Endpoints().AsSlice(),
+	})
+	c.OriginNode = node.ID()
+
+	return c
+}
+
 // UserAdded returns a [Change] for when a user is added or updated.
 // A full update is sent to refresh user profiles on all nodes.
 func UserAdded() Change {

hscontrol/types/change/change_test.go

@@ -4,11 +4,13 @@ import (
 	"net/netip"
 	"reflect"
 	"testing"
+	"time"
 
 	"github.com/juanfont/headscale/hscontrol/types"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"tailscale.com/tailcfg"
+	"tailscale.com/types/key"
 )
 
 func TestChange_FieldSync(t *testing.T) {
@@ -653,3 +655,34 @@ func TestNodeOnlineOfflineForSubnetRouter(t *testing.T) {
 		})
 	}
 }
+
+// TestNodeKeyRotatedEmitsPatchNotWholeNode proves a relogin is delivered to
+// peers as an incremental peer patch, not a whole-node add. A whole-node add is
+// non-patchifiable on the tailscale client whenever Hostinfo changed (which it
+// does on relogin), forcing the broken NodeMutationAdd path that strands a
+// re-keyed, momentarily-endpoint-less peer.
+func TestNodeKeyRotatedEmitsPatchNotWholeNode(t *testing.T) {
+	expiry := time.Now().Add(24 * time.Hour).UTC()
+	node := types.Node{
+		ID:        7,
+		NodeKey:   key.NewNode().Public(),
+		DiscoKey:  key.NewDisco().Public(),
+		Endpoints: []netip.AddrPort{netip.MustParseAddrPort("192.168.1.9:41641")},
+		Expiry:    &expiry,
+	}
+	view := node.View()
+
+	c := NodeKeyRotated(view)
+
+	assert.False(t, c.IsFull(), "relogin must be a peer patch, not a full update")
+	assert.Empty(t, c.PeersChanged, "relogin must not emit a whole-node PeersChanged")
+	require.Len(t, c.PeerPatches, 1, "relogin must emit exactly one peer patch")
+
+	patch := c.PeerPatches[0]
+	assert.Equal(t, view.ID().NodeID(), patch.NodeID)
+	require.NotNil(t, patch.Key, "patch must carry the rotated NodeKey")
+	assert.Equal(t, node.NodeKey, *patch.Key)
+	require.NotNil(t, patch.KeyExpiry, "patch must carry KeyExpiry to (un)expire the peer")
+	assert.Equal(t, expiry, *patch.KeyExpiry)
+	assert.Equal(t, []netip.AddrPort(node.Endpoints), patch.Endpoints, "patch must carry endpoints")
+}

mkdocs.yml

@@ -111,7 +111,7 @@ extra:
     - icon: fontawesome/brands/discord
       link: https://discord.gg/c84AZQhmpx
   headscale:
-    version: 0.28.0
+    version: 0.29.1
 
 # Extensions
 markdown_extensions: