mirror of
https://github.com/juanfont/headscale.git
synced 2026-02-07 20:04:00 +01:00
Merge d62786b06e into cfb308b4a7
This commit is contained in:
commit
de75a941c7
@ -14,7 +14,7 @@ import (
|
||||
// TestTaggedPreAuthKeyCreatesTaggedNode tests that a PreAuthKey with tags creates
|
||||
// a tagged node with:
|
||||
// - Tags from the PreAuthKey
|
||||
// - UserID tracking who created the key (informational "created by")
|
||||
// - Nil UserID (tagged nodes are owned by tags, not a user)
|
||||
// - IsTagged() returns true.
|
||||
func TestTaggedPreAuthKeyCreatesTaggedNode(t *testing.T) {
|
||||
app := createTestApp(t)
|
||||
@ -51,11 +51,10 @@ func TestTaggedPreAuthKeyCreatesTaggedNode(t *testing.T) {
|
||||
node, found := app.state.GetNodeByNodeKey(nodeKey.Public())
|
||||
require.True(t, found)
|
||||
|
||||
// Critical assertions for tags-as-identity model
|
||||
// Tagged nodes are owned by their tags, not a user.
|
||||
assert.True(t, node.IsTagged(), "Node should be tagged")
|
||||
assert.ElementsMatch(t, tags, node.Tags().AsSlice(), "Node should have tags from PreAuthKey")
|
||||
assert.True(t, node.UserID().Valid(), "Node should have UserID tracking creator")
|
||||
assert.Equal(t, user.ID, node.UserID().Get(), "UserID should track PreAuthKey creator")
|
||||
assert.False(t, node.UserID().Valid(), "Tagged node should not have UserID")
|
||||
|
||||
// Verify node is identified correctly
|
||||
assert.True(t, node.IsTagged(), "Tagged node is not user-owned")
|
||||
@ -129,9 +128,10 @@ func TestReAuthDoesNotReapplyTags(t *testing.T) {
|
||||
assert.True(t, nodeAfterReauth.IsTagged(), "Node should still be tagged")
|
||||
assert.ElementsMatch(t, initialTags, nodeAfterReauth.Tags().AsSlice(), "Tags should remain unchanged on re-auth")
|
||||
|
||||
// Verify only one node was created (no duplicates)
|
||||
nodes := app.state.ListNodesByUser(types.UserID(user.ID))
|
||||
assert.Equal(t, 1, nodes.Len(), "Should have exactly one node")
|
||||
// Verify only one node was created (no duplicates).
|
||||
// Tagged nodes are not indexed by user, so check the global list.
|
||||
allNodes := app.state.ListNodes()
|
||||
assert.Equal(t, 1, allNodes.Len(), "Should have exactly one node")
|
||||
}
|
||||
|
||||
// NOTE: TestSetTagsOnUserOwnedNode functionality is covered by gRPC tests in grpcv1_test.go
|
||||
@ -294,13 +294,13 @@ func TestMultipleNodesWithSameReusableTaggedPreAuthKey(t *testing.T) {
|
||||
assert.ElementsMatch(t, tags, node1.Tags().AsSlice(), "First node should have PreAuthKey tags")
|
||||
assert.ElementsMatch(t, tags, node2.Tags().AsSlice(), "Second node should have PreAuthKey tags")
|
||||
|
||||
// Both nodes should track the same creator
|
||||
assert.Equal(t, user.ID, node1.UserID().Get(), "First node should track creator")
|
||||
assert.Equal(t, user.ID, node2.UserID().Get(), "Second node should track creator")
|
||||
// Tagged nodes should not have UserID set.
|
||||
assert.False(t, node1.UserID().Valid(), "First node should not have UserID")
|
||||
assert.False(t, node2.UserID().Valid(), "Second node should not have UserID")
|
||||
|
||||
// Verify we have exactly 2 nodes
|
||||
nodes := app.state.ListNodesByUser(types.UserID(user.ID))
|
||||
assert.Equal(t, 2, nodes.Len(), "Should have exactly two nodes")
|
||||
// Verify we have exactly 2 nodes.
|
||||
allNodes := app.state.ListNodes()
|
||||
assert.Equal(t, 2, allNodes.Len(), "Should have exactly two nodes")
|
||||
}
|
||||
|
||||
// TestNonReusableTaggedPreAuthKey tests that a non-reusable PreAuthKey with tags
|
||||
@ -359,9 +359,9 @@ func TestNonReusableTaggedPreAuthKey(t *testing.T) {
|
||||
_, err = app.handleRegisterWithAuthKey(regReq2, machineKey2.Public())
|
||||
require.Error(t, err, "Should not be able to reuse non-reusable PreAuthKey")
|
||||
|
||||
// Verify only one node was created
|
||||
nodes := app.state.ListNodesByUser(types.UserID(user.ID))
|
||||
assert.Equal(t, 1, nodes.Len(), "Should have exactly one node")
|
||||
// Verify only one node was created.
|
||||
allNodes := app.state.ListNodes()
|
||||
assert.Equal(t, 1, allNodes.Len(), "Should have exactly one node")
|
||||
}
|
||||
|
||||
// TestExpiredTaggedPreAuthKey tests that an expired PreAuthKey with tags
|
||||
|
||||
@ -704,6 +704,29 @@ AND auth_key_id NOT IN (
|
||||
},
|
||||
Rollback: func(db *gorm.DB) error { return nil },
|
||||
},
|
||||
{
|
||||
// Clear user_id on tagged nodes.
|
||||
// Tagged nodes are owned by their tags, not a user.
|
||||
// Previously user_id was kept as "created by" tracking,
|
||||
// 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.
|
||||
// Fixes: https://github.com/juanfont/headscale/issues/3077
|
||||
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 != '';
|
||||
`).Error
|
||||
if err != nil {
|
||||
return fmt.Errorf("clearing user_id on tagged nodes: %w", err)
|
||||
}
|
||||
|
||||
return nil
|
||||
},
|
||||
Rollback: func(db *gorm.DB) error { return nil },
|
||||
},
|
||||
},
|
||||
)
|
||||
|
||||
|
||||
@ -79,6 +79,8 @@ CREATE TABLE nodes(
|
||||
ipv6 text,
|
||||
hostname text,
|
||||
given_name varchar(63),
|
||||
-- user_id is NULL for tagged nodes (owned by tags, not a user).
|
||||
-- Only set for user-owned nodes (no tags).
|
||||
user_id integer,
|
||||
register_method text,
|
||||
tags text,
|
||||
|
||||
@ -48,7 +48,8 @@ func (hsdb *HSDatabase) DestroyUser(uid types.UserID) error {
|
||||
}
|
||||
|
||||
// DestroyUser destroys a User. Returns error if the User does
|
||||
// not exist or if there are nodes associated with it.
|
||||
// not exist or if there are user-owned nodes associated with it.
|
||||
// Tagged nodes have user_id = NULL so they do not block deletion.
|
||||
func DestroyUser(tx *gorm.DB, uid types.UserID) error {
|
||||
user, err := GetUserByID(tx, uid)
|
||||
if err != nil {
|
||||
|
||||
@ -89,6 +89,77 @@ func TestDestroyUserErrors(t *testing.T) {
|
||||
assert.ErrorIs(t, err, ErrUserStillHasNodes)
|
||||
},
|
||||
},
|
||||
{
|
||||
// https://github.com/juanfont/headscale/issues/3077
|
||||
// Tagged nodes have user_id = NULL, so they do not block
|
||||
// user deletion and are unaffected by ON DELETE CASCADE.
|
||||
name: "success_user_only_has_tagged_nodes",
|
||||
test: func(t *testing.T, db *HSDatabase) {
|
||||
t.Helper()
|
||||
|
||||
user, err := db.CreateUser(types.User{Name: "test"})
|
||||
require.NoError(t, err)
|
||||
|
||||
// Create a tagged node with no user_id (the invariant).
|
||||
node := types.Node{
|
||||
ID: 0,
|
||||
Hostname: "tagged-node",
|
||||
RegisterMethod: util.RegisterMethodAuthKey,
|
||||
Tags: []string{"tag:server"},
|
||||
}
|
||||
trx := db.DB.Save(&node)
|
||||
require.NoError(t, trx.Error)
|
||||
|
||||
err = db.DestroyUser(types.UserID(user.ID))
|
||||
require.NoError(t, err)
|
||||
|
||||
// User is gone.
|
||||
_, err = db.GetUserByID(types.UserID(user.ID))
|
||||
require.ErrorIs(t, err, ErrUserNotFound)
|
||||
|
||||
// Tagged node survives.
|
||||
var survivingNode types.Node
|
||||
|
||||
result := db.DB.First(&survivingNode, "id = ?", node.ID)
|
||||
require.NoError(t, result.Error)
|
||||
assert.Nil(t, survivingNode.UserID)
|
||||
assert.Equal(t, []string{"tag:server"}, survivingNode.Tags)
|
||||
},
|
||||
},
|
||||
{
|
||||
// A user who has both tagged and user-owned nodes cannot
|
||||
// be deleted; the user-owned nodes still block deletion.
|
||||
name: "error_user_has_tagged_and_owned_nodes",
|
||||
test: func(t *testing.T, db *HSDatabase) {
|
||||
t.Helper()
|
||||
|
||||
user, err := db.CreateUser(types.User{Name: "test"})
|
||||
require.NoError(t, err)
|
||||
|
||||
// Tagged node: no user_id.
|
||||
taggedNode := types.Node{
|
||||
ID: 0,
|
||||
Hostname: "tagged-node",
|
||||
RegisterMethod: util.RegisterMethodAuthKey,
|
||||
Tags: []string{"tag:server"},
|
||||
}
|
||||
trx := db.DB.Save(&taggedNode)
|
||||
require.NoError(t, trx.Error)
|
||||
|
||||
// User-owned node: has user_id.
|
||||
ownedNode := types.Node{
|
||||
ID: 0,
|
||||
Hostname: "owned-node",
|
||||
UserID: &user.ID,
|
||||
RegisterMethod: util.RegisterMethodAuthKey,
|
||||
}
|
||||
trx = db.DB.Save(&ownedNode)
|
||||
require.NoError(t, trx.Error)
|
||||
|
||||
err = db.DestroyUser(types.UserID(user.ID))
|
||||
require.ErrorIs(t, err, ErrUserStillHasNodes)
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
|
||||
@ -3,8 +3,10 @@ package hscontrol
|
||||
import (
|
||||
"context"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
v1 "github.com/juanfont/headscale/gen/go/headscale/v1"
|
||||
"github.com/juanfont/headscale/hscontrol/types"
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
"google.golang.org/grpc/codes"
|
||||
@ -186,13 +188,12 @@ func TestSetTags_TaggedNode(t *testing.T) {
|
||||
taggedNode, found := app.state.GetNodeByNodeKey(nodeKey.Public())
|
||||
require.True(t, found)
|
||||
assert.True(t, taggedNode.IsTagged(), "Node should be tagged")
|
||||
assert.True(t, taggedNode.UserID().Valid(), "Tagged node should have UserID for tracking")
|
||||
assert.False(t, taggedNode.UserID().Valid(), "Tagged node should not have UserID")
|
||||
|
||||
// Create API server instance
|
||||
apiServer := newHeadscaleV1APIServer(app)
|
||||
|
||||
// Test: SetTags should NOT reject tagged nodes with "user-owned" error
|
||||
// (Even though they have UserID set, IsTagged() identifies them correctly)
|
||||
// Test: SetTags should work on tagged nodes.
|
||||
resp, err := apiServer.SetTags(context.Background(), &v1.SetTagsRequest{
|
||||
NodeId: uint64(taggedNode.ID()),
|
||||
Tags: []string{"tag:initial"}, // Keep existing tag to avoid ACL validation issues
|
||||
@ -283,6 +284,80 @@ func TestDeleteUser_ReturnsProperChangeSignal(t *testing.T) {
|
||||
assert.False(t, changeSignal.IsEmpty(), "DeleteUser should return a non-empty change signal (issue #2967)")
|
||||
}
|
||||
|
||||
// TestDeleteUser_TaggedNodeSurvives tests that deleting a user succeeds when
|
||||
// the user's only nodes are tagged, and that those nodes remain in the
|
||||
// NodeStore with nil UserID.
|
||||
// https://github.com/juanfont/headscale/issues/3077
|
||||
func TestDeleteUser_TaggedNodeSurvives(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
app := createTestApp(t)
|
||||
|
||||
user := app.state.CreateUserForTest("legacy-user")
|
||||
|
||||
// Register a tagged node via the full auth flow.
|
||||
tags := []string{"tag:server"}
|
||||
pak, err := app.state.CreatePreAuthKey(user.TypedID(), true, false, nil, tags)
|
||||
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: "tagged-server",
|
||||
},
|
||||
Expiry: time.Now().Add(24 * time.Hour),
|
||||
}
|
||||
|
||||
resp, err := app.handleRegisterWithAuthKey(regReq, machineKey.Public())
|
||||
require.NoError(t, err)
|
||||
require.True(t, resp.MachineAuthorized)
|
||||
|
||||
// Verify the registered node has nil UserID (enforced invariant).
|
||||
node, found := app.state.GetNodeByNodeKey(nodeKey.Public())
|
||||
require.True(t, found)
|
||||
require.True(t, node.IsTagged())
|
||||
assert.False(t, node.UserID().Valid(),
|
||||
"tagged node should have nil UserID after registration")
|
||||
|
||||
nodeID := node.ID()
|
||||
|
||||
// NodeStore should not list the tagged node under any user.
|
||||
nodesForUser := app.state.ListNodesByUser(types.UserID(user.ID))
|
||||
assert.Equal(t, 0, nodesForUser.Len(),
|
||||
"tagged nodes should not appear in nodesByUser index")
|
||||
|
||||
// Delete the user.
|
||||
changeSignal, err := app.state.DeleteUser(*user.TypedID())
|
||||
require.NoError(t, err)
|
||||
assert.False(t, changeSignal.IsEmpty())
|
||||
|
||||
// Tagged node survives in the NodeStore.
|
||||
nodeAfter, found := app.state.GetNodeByID(nodeID)
|
||||
require.True(t, found, "tagged node should survive user deletion")
|
||||
assert.True(t, nodeAfter.IsTagged())
|
||||
assert.False(t, nodeAfter.UserID().Valid())
|
||||
|
||||
// Tagged node appears in the global list.
|
||||
allNodes := app.state.ListNodes()
|
||||
foundInAll := false
|
||||
|
||||
for _, n := range allNodes.All() {
|
||||
if n.ID() == nodeID {
|
||||
foundInAll = true
|
||||
|
||||
break
|
||||
}
|
||||
}
|
||||
|
||||
assert.True(t, foundInAll, "tagged node should appear in the global node list")
|
||||
}
|
||||
|
||||
// TestExpireApiKey_ByID tests that API keys can be expired by ID.
|
||||
func TestExpireApiKey_ByID(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
@ -425,7 +425,12 @@ func snapshotFromNodes(nodes map[types.NodeID]types.Node, peersFunc PeersFunc) S
|
||||
nodeView := n.View()
|
||||
userID := n.TypedUserID()
|
||||
|
||||
newSnap.nodesByUser[userID] = append(newSnap.nodesByUser[userID], nodeView)
|
||||
// Tagged nodes are owned by their tags, not a user,
|
||||
// so they are not indexed by user.
|
||||
if !n.IsTagged() {
|
||||
newSnap.nodesByUser[userID] = append(newSnap.nodesByUser[userID], nodeView)
|
||||
}
|
||||
|
||||
newSnap.nodesByNodeKey[n.NodeKey] = nodeView
|
||||
|
||||
// Build machine key index
|
||||
@ -531,23 +536,12 @@ func (s *NodeStore) DebugString() string {
|
||||
for userID, nodes := range snapshot.nodesByUser {
|
||||
if len(nodes) > 0 {
|
||||
userName := "unknown"
|
||||
taggedCount := 0
|
||||
|
||||
if len(nodes) > 0 && nodes[0].Valid() {
|
||||
if nodes[0].Valid() && nodes[0].User().Valid() {
|
||||
userName = nodes[0].User().Name()
|
||||
// Count tagged nodes (which have UserID set but are owned by "tagged-devices")
|
||||
for _, n := range nodes {
|
||||
if n.IsTagged() {
|
||||
taggedCount++
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if taggedCount > 0 {
|
||||
sb.WriteString(fmt.Sprintf(" - User %d (%s): %d nodes (%d tagged)\n", userID, userName, len(nodes), taggedCount))
|
||||
} else {
|
||||
sb.WriteString(fmt.Sprintf(" - User %d (%s): %d nodes\n", userID, userName, len(nodes)))
|
||||
}
|
||||
sb.WriteString(fmt.Sprintf(" - User %d (%s): %d nodes\n", userID, userName, len(nodes)))
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@ -472,10 +472,9 @@ func (s *State) DeleteNode(node types.NodeView) (change.Change, error) {
|
||||
|
||||
// Connect marks a node as connected and updates its primary routes in the state.
|
||||
func (s *State) Connect(id types.NodeID) []change.Change {
|
||||
// CRITICAL FIX: Update the online status in NodeStore BEFORE creating change notification
|
||||
// This ensures that when the NodeCameOnline change is distributed and processed by other nodes,
|
||||
// the NodeStore already reflects the correct online status for full map generation.
|
||||
// now := time.Now()
|
||||
// Update online status in NodeStore before creating change notification
|
||||
// so the NodeStore already reflects the correct state when other nodes
|
||||
// process the NodeCameOnline change for full map generation.
|
||||
node, ok := s.nodeStore.UpdateNode(id, func(n *types.Node) {
|
||||
n.IsOnline = new(true)
|
||||
// n.LastSeen = ptr.To(now)
|
||||
@ -488,9 +487,8 @@ func (s *State) Connect(id types.NodeID) []change.Change {
|
||||
|
||||
log.Info().EmbedObject(node).Msg("node connected")
|
||||
|
||||
// Use the node's current routes for primary route update
|
||||
// AllApprovedRoutes() returns only the intersection of announced AND approved routes
|
||||
// We MUST use AllApprovedRoutes() to maintain the security model
|
||||
// Use the node's current routes for primary route update.
|
||||
// AllApprovedRoutes() returns only the intersection of announced and approved routes.
|
||||
routeChange := s.primaryRoutes.SetRoutes(id, node.AllApprovedRoutes()...)
|
||||
|
||||
if routeChange {
|
||||
@ -658,9 +656,8 @@ func (s *State) SetNodeExpiry(nodeID types.NodeID, expiry time.Time) (types.Node
|
||||
|
||||
// SetNodeTags assigns tags to a node, making it a "tagged node".
|
||||
// Once a node is tagged, it cannot be un-tagged (only tags can be changed).
|
||||
// The UserID is preserved as "created by" information.
|
||||
// Setting tags clears UserID since tagged nodes are owned by their tags.
|
||||
func (s *State) SetNodeTags(nodeID types.NodeID, tags []string) (types.NodeView, change.Change, error) {
|
||||
// CANNOT REMOVE ALL TAGS
|
||||
if len(tags) == 0 {
|
||||
return types.NodeView{}, change.Change{}, types.ErrCannotRemoveAllTags
|
||||
}
|
||||
@ -700,7 +697,9 @@ func (s *State) SetNodeTags(nodeID types.NodeID, tags []string) (types.NodeView,
|
||||
// make the exact same change.
|
||||
n, ok := s.nodeStore.UpdateNode(nodeID, func(node *types.Node) {
|
||||
node.Tags = validatedTags
|
||||
// UserID is preserved as "created by" - do NOT set to nil
|
||||
// Tagged nodes are owned by their tags, not a user.
|
||||
node.UserID = nil
|
||||
node.User = nil
|
||||
})
|
||||
|
||||
if !ok {
|
||||
@ -1291,17 +1290,10 @@ func (s *State) createAndSaveNewNode(params newNodeParams) (types.NodeView, erro
|
||||
// Assign ownership based on PreAuthKey
|
||||
if params.PreAuthKey != nil {
|
||||
if params.PreAuthKey.IsTagged() {
|
||||
// TAGGED NODE
|
||||
// Tags from PreAuthKey are assigned ONLY during initial authentication
|
||||
// Tagged nodes are owned by their tags, not a user.
|
||||
// UserID is intentionally left nil.
|
||||
nodeToRegister.Tags = params.PreAuthKey.Proto().GetAclTags()
|
||||
|
||||
// Set UserID to track "created by" (who created the PreAuthKey)
|
||||
if params.PreAuthKey.UserID != nil {
|
||||
nodeToRegister.UserID = params.PreAuthKey.UserID
|
||||
nodeToRegister.User = params.PreAuthKey.User
|
||||
}
|
||||
// If PreAuthKey.UserID is nil, the node is "orphaned" (system-created)
|
||||
|
||||
// Tagged nodes have key expiry disabled.
|
||||
nodeToRegister.Expiry = nil
|
||||
} else {
|
||||
@ -1342,6 +1334,11 @@ func (s *State) createAndSaveNewNode(params newNodeParams) (types.NodeView, erro
|
||||
slices.Sort(nodeToRegister.Tags)
|
||||
nodeToRegister.Tags = slices.Compact(nodeToRegister.Tags)
|
||||
|
||||
// Node is now tagged, so clear user ownership.
|
||||
// Tagged nodes are owned by their tags, not a user.
|
||||
nodeToRegister.UserID = nil
|
||||
nodeToRegister.User = nil
|
||||
|
||||
// Tagged nodes have key expiry disabled.
|
||||
nodeToRegister.Expiry = nil
|
||||
|
||||
@ -1488,7 +1485,10 @@ func (s *State) processReauthTags(
|
||||
wasTagged := node.IsTagged()
|
||||
node.Tags = approvedTags
|
||||
|
||||
// Note: UserID is preserved as "created by" tracking, consistent with SetNodeTags
|
||||
// Tagged nodes are owned by their tags, not a user.
|
||||
node.UserID = nil
|
||||
node.User = nil
|
||||
|
||||
if !wasTagged {
|
||||
log.Info().
|
||||
Uint64(zf.NodeID, uint64(node.ID)).
|
||||
@ -1713,9 +1713,15 @@ func (s *State) HandleNodeFromPreAuthKey(
|
||||
existingNodeSameUser, existsSameUser = s.nodeStore.GetNodeByMachineKey(machineKey, types.UserID(pak.User.ID))
|
||||
}
|
||||
|
||||
// Tagged nodes have nil UserID, so they are indexed under UserID(0)
|
||||
// in nodesByMachineKey. Check there too for tagged PAK re-registration.
|
||||
if !existsSameUser && pak.IsTagged() {
|
||||
existingNodeSameUser, existsSameUser = s.nodeStore.GetNodeByMachineKey(machineKey, 0)
|
||||
}
|
||||
|
||||
// For existing nodes, skip validation if:
|
||||
// 1. MachineKey matches (cryptographic proof of machine identity)
|
||||
// 2. User matches (from the PAK being used)
|
||||
// 2. User/tag ownership matches (from the PAK being used)
|
||||
// 3. Not a NodeKey rotation (rotation requires fresh validation)
|
||||
//
|
||||
// Security: MachineKey is the cryptographic identity. If someone has the MachineKey,
|
||||
@ -1723,9 +1729,7 @@ func (s *State) HandleNodeFromPreAuthKey(
|
||||
// We don't check which specific PAK was used originally because:
|
||||
// - Container restarts may use different PAKs (e.g., env var changed)
|
||||
// - Original PAK may be deleted
|
||||
// - MachineKey + User is sufficient to prove this is the same node
|
||||
//
|
||||
// Note: For tags-only keys, existsSameUser is always false, so we always validate.
|
||||
// - MachineKey + ownership is sufficient to prove this is the same node
|
||||
isExistingNodeReregistering := existsSameUser && existingNodeSameUser.Valid()
|
||||
|
||||
// Check if this is a NodeKey rotation (different NodeKey)
|
||||
@ -1812,11 +1816,9 @@ func (s *State) HandleNodeFromPreAuthKey(
|
||||
|
||||
node.RegisterMethod = util.RegisterMethodAuthKey
|
||||
|
||||
// CRITICAL: Tags from PreAuthKey are ONLY applied during initial authentication
|
||||
// On re-registration, we MUST NOT change tags or node ownership
|
||||
// The node keeps whatever tags/user ownership it already has
|
||||
//
|
||||
// Only update AuthKey reference
|
||||
// Tags from PreAuthKey are only applied during initial registration.
|
||||
// On re-registration the node keeps its existing tags and ownership.
|
||||
// Only update AuthKey reference.
|
||||
node.AuthKey = pak
|
||||
node.AuthKeyID = &pak.ID
|
||||
node.IsOnline = new(false)
|
||||
@ -1869,19 +1871,27 @@ func (s *State) HandleNodeFromPreAuthKey(
|
||||
// Check if node exists with this machine key for a different user
|
||||
existingNodeAnyUser, existsAnyUser := s.nodeStore.GetNodeByMachineKeyAnyUser(machineKey)
|
||||
|
||||
// For user-owned keys, check if node exists for a different user
|
||||
// For tags-only keys (pak.User == nil), this check is skipped
|
||||
if pak.User != nil && existsAnyUser && existingNodeAnyUser.Valid() && existingNodeAnyUser.UserID().Get() != pak.User.ID {
|
||||
// Node exists but belongs to a different user
|
||||
// Create a NEW node for the new user (do not transfer)
|
||||
// This allows the same machine to have separate node identities per user
|
||||
oldUser := existingNodeAnyUser.User()
|
||||
// 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
|
||||
|
||||
if belongsToDifferentUser {
|
||||
// Node exists but belongs to a different user.
|
||||
// Create a new node for the new user (do not transfer).
|
||||
oldUserName := existingNodeAnyUser.User().Name()
|
||||
|
||||
log.Info().
|
||||
Caller().
|
||||
Str(zf.ExistingNodeName, existingNodeAnyUser.Hostname()).
|
||||
Uint64(zf.ExistingNodeID, existingNodeAnyUser.ID().Uint64()).
|
||||
Str(zf.MachineKey, machineKey.ShortString()).
|
||||
Str(zf.OldUser, oldUser.Name()).
|
||||
Str(zf.OldUser, oldUserName).
|
||||
Str(zf.NewUser, pakUsername()).
|
||||
Msg("Creating new node for different user (same machine key exists for another user)")
|
||||
}
|
||||
|
||||
@ -20,22 +20,26 @@ var (
|
||||
ErrRequestedTagsInvalidOrNotPermitted = errors.New("requested tags")
|
||||
)
|
||||
|
||||
// validateNodeOwnership ensures proper node ownership model.
|
||||
// A node must be EITHER user-owned OR tagged (mutually exclusive by behavior).
|
||||
// Tagged nodes CAN have a UserID for "created by" tracking, but the tag is the owner.
|
||||
func validateNodeOwnership(node *types.Node) error {
|
||||
isTagged := node.IsTagged()
|
||||
// ErrTaggedNodeHasUser is returned when a tagged node has a UserID set.
|
||||
var ErrTaggedNodeHasUser = errors.New("tagged node must not have user_id set")
|
||||
|
||||
// Tagged nodes: Must have tags, UserID is optional (just "created by")
|
||||
if isTagged {
|
||||
// validateNodeOwnership ensures proper node ownership model.
|
||||
// A node must be either user-owned or tagged, and these are mutually exclusive:
|
||||
// tagged nodes must not have a UserID, and user-owned nodes must not have tags.
|
||||
func validateNodeOwnership(node *types.Node) error {
|
||||
if node.IsTagged() {
|
||||
if len(node.Tags) == 0 {
|
||||
return fmt.Errorf("%w: %q", ErrNodeMarkedTaggedButHasNoTags, node.Hostname)
|
||||
}
|
||||
// UserID can be set (created by) or nil (orphaned), both valid for tagged nodes
|
||||
|
||||
if node.UserID != nil {
|
||||
return fmt.Errorf("%w: %q", ErrTaggedNodeHasUser, node.Hostname)
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
// User-owned nodes: Must have UserID, must NOT have tags
|
||||
// User-owned nodes must have a UserID.
|
||||
if node.UserID == nil {
|
||||
return fmt.Errorf("%w: %q", ErrNodeHasNeitherUserNorTags, node.Hostname)
|
||||
}
|
||||
@ -59,7 +63,7 @@ func logTagOperation(existingNode types.NodeView, newTags []string) {
|
||||
|
||||
log.Info().
|
||||
EmbedObject(existingNode).
|
||||
Uint("created.by.user", userID).
|
||||
Uint("previous.user", userID).
|
||||
Strs("new.tags", newTags).
|
||||
Msg("Converting user-owned node to tagged node")
|
||||
}
|
||||
|
||||
@ -105,10 +105,8 @@ type Node struct {
|
||||
// parts of headscale.
|
||||
GivenName string `gorm:"type:varchar(63);unique_index"`
|
||||
|
||||
// UserID is set for ALL nodes (tagged and user-owned) to track "created by".
|
||||
// For tagged nodes, this is informational only - the tag is the owner.
|
||||
// For user-owned nodes, this identifies the owner.
|
||||
// Only nil for orphaned nodes (should not happen in normal operation).
|
||||
// UserID identifies the owning user for user-owned nodes.
|
||||
// Nil for tagged nodes, which are owned by their tags.
|
||||
UserID *uint
|
||||
User *User `gorm:"constraint:OnDelete:CASCADE;"`
|
||||
|
||||
|
||||
@ -215,10 +215,8 @@ func (v NodeView) Hostname() string { return v.ж.Hostname }
|
||||
// parts of headscale.
|
||||
func (v NodeView) GivenName() string { return v.ж.GivenName }
|
||||
|
||||
// UserID is set for ALL nodes (tagged and user-owned) to track "created by".
|
||||
// For tagged nodes, this is informational only - the tag is the owner.
|
||||
// For user-owned nodes, this identifies the owner.
|
||||
// Only nil for orphaned nodes (should not happen in normal operation).
|
||||
// UserID identifies the owning user for user-owned nodes.
|
||||
// Nil for tagged nodes, which are owned by their tags.
|
||||
func (v NodeView) UserID() views.ValuePointer[uint] { return views.ValuePointerOf(v.ж.UserID) }
|
||||
|
||||
func (v NodeView) User() UserView { return v.ж.User.View() }
|
||||
|
||||
Loading…
Reference in New Issue
Block a user