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
894e6946dc
commit
1e4fc3f179
@ -14,7 +14,7 @@ import (
|
|||||||
// TestTaggedPreAuthKeyCreatesTaggedNode tests that a PreAuthKey with tags creates
|
// TestTaggedPreAuthKeyCreatesTaggedNode tests that a PreAuthKey with tags creates
|
||||||
// a tagged node with:
|
// a tagged node with:
|
||||||
// - Tags from the PreAuthKey
|
// - 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.
|
// - IsTagged() returns true.
|
||||||
func TestTaggedPreAuthKeyCreatesTaggedNode(t *testing.T) {
|
func TestTaggedPreAuthKeyCreatesTaggedNode(t *testing.T) {
|
||||||
app := createTestApp(t)
|
app := createTestApp(t)
|
||||||
@ -51,11 +51,10 @@ func TestTaggedPreAuthKeyCreatesTaggedNode(t *testing.T) {
|
|||||||
node, found := app.state.GetNodeByNodeKey(nodeKey.Public())
|
node, found := app.state.GetNodeByNodeKey(nodeKey.Public())
|
||||||
require.True(t, found)
|
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.True(t, node.IsTagged(), "Node should be tagged")
|
||||||
assert.ElementsMatch(t, tags, node.Tags().AsSlice(), "Node should have tags from PreAuthKey")
|
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.False(t, node.UserID().Valid(), "Tagged node should not have UserID")
|
||||||
assert.Equal(t, user.ID, node.UserID().Get(), "UserID should track PreAuthKey creator")
|
|
||||||
|
|
||||||
// Verify node is identified correctly
|
// Verify node is identified correctly
|
||||||
assert.True(t, node.IsTagged(), "Tagged node is not user-owned")
|
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.True(t, nodeAfterReauth.IsTagged(), "Node should still be tagged")
|
||||||
assert.ElementsMatch(t, initialTags, nodeAfterReauth.Tags().AsSlice(), "Tags should remain unchanged on re-auth")
|
assert.ElementsMatch(t, initialTags, nodeAfterReauth.Tags().AsSlice(), "Tags should remain unchanged on re-auth")
|
||||||
|
|
||||||
// Verify only one node was created (no duplicates)
|
// Verify only one node was created (no duplicates).
|
||||||
nodes := app.state.ListNodesByUser(types.UserID(user.ID))
|
// Tagged nodes are not indexed by user, so check the global list.
|
||||||
assert.Equal(t, 1, nodes.Len(), "Should have exactly one node")
|
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
|
// 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, node1.Tags().AsSlice(), "First node should have PreAuthKey tags")
|
||||||
assert.ElementsMatch(t, tags, node2.Tags().AsSlice(), "Second 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
|
// Tagged nodes should not have UserID set.
|
||||||
assert.Equal(t, user.ID, node1.UserID().Get(), "First node should track creator")
|
assert.False(t, node1.UserID().Valid(), "First node should not have UserID")
|
||||||
assert.Equal(t, user.ID, node2.UserID().Get(), "Second node should track creator")
|
assert.False(t, node2.UserID().Valid(), "Second node should not have UserID")
|
||||||
|
|
||||||
// Verify we have exactly 2 nodes
|
// Verify we have exactly 2 nodes.
|
||||||
nodes := app.state.ListNodesByUser(types.UserID(user.ID))
|
allNodes := app.state.ListNodes()
|
||||||
assert.Equal(t, 2, nodes.Len(), "Should have exactly two nodes")
|
assert.Equal(t, 2, allNodes.Len(), "Should have exactly two nodes")
|
||||||
}
|
}
|
||||||
|
|
||||||
// TestNonReusableTaggedPreAuthKey tests that a non-reusable PreAuthKey with tags
|
// TestNonReusableTaggedPreAuthKey tests that a non-reusable PreAuthKey with tags
|
||||||
@ -359,9 +359,9 @@ func TestNonReusableTaggedPreAuthKey(t *testing.T) {
|
|||||||
_, err = app.handleRegisterWithAuthKey(regReq2, machineKey2.Public())
|
_, err = app.handleRegisterWithAuthKey(regReq2, machineKey2.Public())
|
||||||
require.Error(t, err, "Should not be able to reuse non-reusable PreAuthKey")
|
require.Error(t, err, "Should not be able to reuse non-reusable PreAuthKey")
|
||||||
|
|
||||||
// Verify only one node was created
|
// Verify only one node was created.
|
||||||
nodes := app.state.ListNodesByUser(types.UserID(user.ID))
|
allNodes := app.state.ListNodes()
|
||||||
assert.Equal(t, 1, nodes.Len(), "Should have exactly one node")
|
assert.Equal(t, 1, allNodes.Len(), "Should have exactly one node")
|
||||||
}
|
}
|
||||||
|
|
||||||
// TestExpiredTaggedPreAuthKey tests that an expired PreAuthKey with tags
|
// TestExpiredTaggedPreAuthKey tests that an expired PreAuthKey with tags
|
||||||
|
|||||||
@ -89,6 +89,77 @@ func TestDestroyUserErrors(t *testing.T) {
|
|||||||
assert.ErrorIs(t, err, ErrUserStillHasNodes)
|
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 {
|
for _, tt := range tests {
|
||||||
|
|||||||
@ -3,8 +3,10 @@ package hscontrol
|
|||||||
import (
|
import (
|
||||||
"context"
|
"context"
|
||||||
"testing"
|
"testing"
|
||||||
|
"time"
|
||||||
|
|
||||||
v1 "github.com/juanfont/headscale/gen/go/headscale/v1"
|
v1 "github.com/juanfont/headscale/gen/go/headscale/v1"
|
||||||
|
"github.com/juanfont/headscale/hscontrol/types"
|
||||||
"github.com/stretchr/testify/assert"
|
"github.com/stretchr/testify/assert"
|
||||||
"github.com/stretchr/testify/require"
|
"github.com/stretchr/testify/require"
|
||||||
"google.golang.org/grpc/codes"
|
"google.golang.org/grpc/codes"
|
||||||
@ -186,13 +188,12 @@ func TestSetTags_TaggedNode(t *testing.T) {
|
|||||||
taggedNode, found := app.state.GetNodeByNodeKey(nodeKey.Public())
|
taggedNode, found := app.state.GetNodeByNodeKey(nodeKey.Public())
|
||||||
require.True(t, found)
|
require.True(t, found)
|
||||||
assert.True(t, taggedNode.IsTagged(), "Node should be tagged")
|
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
|
// Create API server instance
|
||||||
apiServer := newHeadscaleV1APIServer(app)
|
apiServer := newHeadscaleV1APIServer(app)
|
||||||
|
|
||||||
// Test: SetTags should NOT reject tagged nodes with "user-owned" error
|
// Test: SetTags should work on tagged nodes.
|
||||||
// (Even though they have UserID set, IsTagged() identifies them correctly)
|
|
||||||
resp, err := apiServer.SetTags(context.Background(), &v1.SetTagsRequest{
|
resp, err := apiServer.SetTags(context.Background(), &v1.SetTagsRequest{
|
||||||
NodeId: uint64(taggedNode.ID()),
|
NodeId: uint64(taggedNode.ID()),
|
||||||
Tags: []string{"tag:initial"}, // Keep existing tag to avoid ACL validation issues
|
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)")
|
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.
|
// TestExpireApiKey_ByID tests that API keys can be expired by ID.
|
||||||
func TestExpireApiKey_ByID(t *testing.T) {
|
func TestExpireApiKey_ByID(t *testing.T) {
|
||||||
t.Parallel()
|
t.Parallel()
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user