From 7e8930c50748ed2ac431b36e29adcbccc0c2322e Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Sun, 1 Mar 2026 22:54:10 +0000 Subject: [PATCH] hscontrol: add tests for default node key expiry Add tests covering the core expiry scenarios: - Untagged auth key with zero expiry gets configured default - Tagged nodes ignore node.expiry - node.expiry=0 disables default (backwards compatible) - Client-requested expiry takes precedence - Re-registration refreshes the default expiry Updates #1711 --- hscontrol/auth_tags_test.go | 286 ++++++++++++++++++++++++++++++++++++ 1 file changed, 286 insertions(+) diff --git a/hscontrol/auth_tags_test.go b/hscontrol/auth_tags_test.go index cd9d4c96..d9184f4f 100644 --- a/hscontrol/auth_tags_test.go +++ b/hscontrol/auth_tags_test.go @@ -4,6 +4,7 @@ import ( "testing" "time" + "github.com/juanfont/headscale/hscontrol/mapper" "github.com/juanfont/headscale/hscontrol/types" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -11,6 +12,49 @@ import ( "tailscale.com/types/key" ) +// createTestAppWithNodeExpiry creates a test app with a specific node.expiry config. +func createTestAppWithNodeExpiry(t *testing.T, nodeExpiry time.Duration) *Headscale { + t.Helper() + + tmpDir := t.TempDir() + + cfg := types.Config{ + ServerURL: "http://localhost:8080", + NoisePrivateKeyPath: tmpDir + "/noise_private.key", + Node: types.NodeConfig{ + Expiry: nodeExpiry, + }, + Database: types.DatabaseConfig{ + Type: "sqlite3", + Sqlite: types.SqliteConfig{ + Path: tmpDir + "/headscale_test.db", + }, + }, + OIDC: types.OIDCConfig{}, + Policy: types.PolicyConfig{ + Mode: types.PolicyModeDB, + }, + Tuning: types.Tuning{ + BatchChangeDelay: 100 * time.Millisecond, + BatcherWorkers: 1, + }, + } + + app, err := NewHeadscale(&cfg) + require.NoError(t, err) + + app.mapBatcher = mapper.NewBatcherAndMapper(&cfg, app.state) + app.mapBatcher.Start() + + t.Cleanup(func() { + if app.mapBatcher != nil { + app.mapBatcher.Close() + } + }) + + return app +} + // TestTaggedPreAuthKeyCreatesTaggedNode tests that a PreAuthKey with tags creates // a tagged node with: // - Tags from the PreAuthKey @@ -833,3 +877,245 @@ func TestReAuthWithDifferentMachineKey(t *testing.T) { assert.True(t, node2.IsTagged()) assert.ElementsMatch(t, tags, node2.Tags().AsSlice()) } + +// TestUntaggedAuthKeyZeroExpiryGetsDefault tests that when node.expiry is configured +// and a client registers with an untagged auth key without requesting a specific expiry, +// the node gets the configured default expiry. +// This is the core fix for https://github.com/juanfont/headscale/issues/1711 +func TestUntaggedAuthKeyZeroExpiryGetsDefault(t *testing.T) { + t.Parallel() + + nodeExpiry := 180 * 24 * time.Hour // 180 days + app := createTestAppWithNodeExpiry(t, nodeExpiry) + + user := app.state.CreateUserForTest("node-owner") + + pak, err := app.state.CreatePreAuthKey(user.TypedID(), true, false, nil, nil) + require.NoError(t, err) + + machineKey := key.NewMachine() + nodeKey := key.NewNode() + + // Client sends zero expiry (the default behaviour of tailscale up --authkey). + regReq := tailcfg.RegisterRequest{ + Auth: &tailcfg.RegisterResponseAuth{ + AuthKey: pak.Key, + }, + NodeKey: nodeKey.Public(), + Hostinfo: &tailcfg.Hostinfo{ + Hostname: "default-expiry-test", + }, + Expiry: time.Time{}, // zero — no client-requested expiry + } + + resp, err := app.handleRegisterWithAuthKey(regReq, machineKey.Public()) + require.NoError(t, err) + require.True(t, resp.MachineAuthorized) + + node, found := app.state.GetNodeByNodeKey(nodeKey.Public()) + require.True(t, found) + + assert.False(t, node.IsTagged()) + assert.True(t, node.Expiry().Valid(), "node should have expiry set from config default") + assert.False(t, node.IsExpired(), "node should not be expired yet") + + expectedExpiry := time.Now().Add(nodeExpiry) + assert.WithinDuration(t, expectedExpiry, node.Expiry().Get(), 10*time.Second, + "node expiry should be ~180 days from now") +} + +// TestTaggedAuthKeyIgnoresNodeExpiry tests that tagged nodes still get nil +// expiry even when node.expiry is configured. +func TestTaggedAuthKeyIgnoresNodeExpiry(t *testing.T) { + t.Parallel() + + nodeExpiry := 180 * 24 * time.Hour + app := createTestAppWithNodeExpiry(t, nodeExpiry) + + user := app.state.CreateUserForTest("tag-creator") + 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-no-expiry", + }, + Expiry: time.Time{}, + } + + resp, err := app.handleRegisterWithAuthKey(regReq, machineKey.Public()) + require.NoError(t, err) + require.True(t, resp.MachineAuthorized) + + node, found := app.state.GetNodeByNodeKey(nodeKey.Public()) + require.True(t, found) + + assert.True(t, node.IsTagged()) + assert.False(t, node.Expiry().Valid(), + "tagged node should have expiry disabled (nil) even with node.expiry configured") +} + +// TestNodeExpiryZeroDisablesDefault tests that setting node.expiry to 0 +// preserves the old behaviour where nodes registered without a client-requested +// expiry get no expiry (never expire). +func TestNodeExpiryZeroDisablesDefault(t *testing.T) { + t.Parallel() + + // node.expiry = 0 means "no default expiry" + app := createTestAppWithNodeExpiry(t, 0) + + user := app.state.CreateUserForTest("node-owner") + + pak, err := app.state.CreatePreAuthKey(user.TypedID(), true, 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: "no-default-expiry", + }, + Expiry: time.Time{}, // zero + } + + resp, err := app.handleRegisterWithAuthKey(regReq, machineKey.Public()) + require.NoError(t, err) + require.True(t, resp.MachineAuthorized) + + node, found := app.state.GetNodeByNodeKey(nodeKey.Public()) + require.True(t, found) + + assert.False(t, node.IsTagged()) + assert.False(t, node.IsExpired(), "node should not be expired") + + // With node.expiry=0 and zero client expiry, the node gets a zero expiry + // which IsExpired() treats as "never expires" — backwards compatible. + if node.Expiry().Valid() { + assert.True(t, node.Expiry().Get().IsZero(), + "with node.expiry=0 and zero client expiry, expiry should be zero time") + } +} + +// TestClientNonZeroExpiryTakesPrecedence tests that when a client explicitly +// requests an expiry, that value is used instead of the configured default. +func TestClientNonZeroExpiryTakesPrecedence(t *testing.T) { + t.Parallel() + + nodeExpiry := 180 * 24 * time.Hour // 180 days + app := createTestAppWithNodeExpiry(t, nodeExpiry) + + user := app.state.CreateUserForTest("node-owner") + + pak, err := app.state.CreatePreAuthKey(user.TypedID(), true, false, nil, nil) + require.NoError(t, err) + + machineKey := key.NewMachine() + nodeKey := key.NewNode() + + // Client explicitly requests 24h expiry + clientExpiry := time.Now().Add(24 * time.Hour) + + regReq := tailcfg.RegisterRequest{ + Auth: &tailcfg.RegisterResponseAuth{ + AuthKey: pak.Key, + }, + NodeKey: nodeKey.Public(), + Hostinfo: &tailcfg.Hostinfo{ + Hostname: "client-expiry-test", + }, + Expiry: clientExpiry, + } + + resp, err := app.handleRegisterWithAuthKey(regReq, machineKey.Public()) + require.NoError(t, err) + require.True(t, resp.MachineAuthorized) + + node, found := app.state.GetNodeByNodeKey(nodeKey.Public()) + require.True(t, found) + + assert.True(t, node.Expiry().Valid(), "node should have expiry set") + assert.WithinDuration(t, clientExpiry, node.Expiry().Get(), 5*time.Second, + "client-requested expiry should take precedence over node.expiry default") +} + +// TestReregistrationAppliesDefaultExpiry tests that when a node re-registers +// with an untagged auth key and the client sends zero expiry, the configured +// default is applied. +func TestReregistrationAppliesDefaultExpiry(t *testing.T) { + t.Parallel() + + nodeExpiry := 90 * 24 * time.Hour // 90 days + app := createTestAppWithNodeExpiry(t, nodeExpiry) + + user := app.state.CreateUserForTest("node-owner") + + pak, err := app.state.CreatePreAuthKey(user.TypedID(), true, false, nil, nil) + require.NoError(t, err) + + machineKey := key.NewMachine() + nodeKey := key.NewNode() + + // Initial registration with zero expiry + regReq := tailcfg.RegisterRequest{ + Auth: &tailcfg.RegisterResponseAuth{ + AuthKey: pak.Key, + }, + NodeKey: nodeKey.Public(), + Hostinfo: &tailcfg.Hostinfo{ + Hostname: "reregister-test", + }, + Expiry: time.Time{}, + } + + resp, err := app.handleRegisterWithAuthKey(regReq, machineKey.Public()) + require.NoError(t, err) + require.True(t, resp.MachineAuthorized) + + node, found := app.state.GetNodeByNodeKey(nodeKey.Public()) + require.True(t, found) + assert.True(t, node.Expiry().Valid(), "initial registration should get default expiry") + + firstExpiry := node.Expiry().Get() + + // Re-register with a new node key but same machine key + nodeKey2 := key.NewNode() + regReq2 := tailcfg.RegisterRequest{ + Auth: &tailcfg.RegisterResponseAuth{ + AuthKey: pak.Key, + }, + NodeKey: nodeKey2.Public(), + Hostinfo: &tailcfg.Hostinfo{ + Hostname: "reregister-test", + }, + Expiry: time.Time{}, // still zero + } + + resp2, err := app.handleRegisterWithAuthKey(regReq2, machineKey.Public()) + require.NoError(t, err) + require.True(t, resp2.MachineAuthorized) + + node2, found := app.state.GetNodeByNodeKey(nodeKey2.Public()) + require.True(t, found) + assert.True(t, node2.Expiry().Valid(), "re-registration should also get default expiry") + + // The expiry should be refreshed (new 90d from now), not the old one + expectedExpiry := time.Now().Add(nodeExpiry) + assert.WithinDuration(t, expectedExpiry, node2.Expiry().Get(), 10*time.Second, + "re-registration should refresh the default expiry") + assert.True(t, node2.Expiry().Get().After(firstExpiry), + "re-registration expiry should be later than initial registration expiry") +}