mirror of
https://github.com/juanfont/headscale.git
synced 2026-02-07 20:04:00 +01:00
hscontrol: add tests for deleting users with tagged nodes
Test the tagged-node-survives-user-deletion scenario at two layers: DB layer (users_test.go): - success_user_only_has_tagged_nodes: tagged nodes with nil user_id do not block user deletion and survive it - error_user_has_tagged_and_owned_nodes: user-owned nodes still block deletion even when tagged nodes coexist App layer (grpcv1_test.go): - TestDeleteUser_TaggedNodeSurvives: full registration flow with tagged PreAuthKey verifies nil UserID after registration, absence from nodesByUser index, user deletion succeeds, and tagged node remains in global node list Also update auth_tags_test.go assertions to expect nil UserID on tagged nodes, consistent with the new invariant. Updates #3077
This commit is contained in:
parent
89b23d75e4
commit
d62786b06e
@ -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
|
||||
|
||||
@ -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()
|
||||
|
||||
Loading…
Reference in New Issue
Block a user