mirror of
https://github.com/juanfont/headscale.git
synced 2026-05-23 10:42:30 +09:00
state: preserve nil expiry on user owned registration when no default is configured
When a user owned node registers or re registers with a PreAuthKey and the
client sends zero client expiry while node.expiry is set to 0, the expiry
column ends up stored as 0001-01-01 00:00:00 instead of NULL. Two sites in
HandleNodeFromPreAuthKey build a non nil pointer to regReq.Expiry even when
the value is zero time, and the needsDefaultExpiry guard only replaces it
when s.cfg.Node.Expiry > 0, so the pointer to zero time survives to the
database.
Convert an unset regReq.Expiry to nil before handing it off so the
needsDefaultExpiry path is the only place that assigns a non nil pointer.
This is a narrower sibling of #3170 on the user owned PreAuthKey path. The
regression was introduced alongside the fix for #3111 in 6337a3db.
This commit is contained in:
committed by
Kristoffer Dalby
parent
ba251e7b47
commit
0e10ca4e9a
@@ -1119,3 +1119,67 @@ func TestReregistrationAppliesDefaultExpiry(t *testing.T) {
|
|||||||
assert.True(t, node2.Expiry().Get().After(firstExpiry),
|
assert.True(t, node2.Expiry().Get().After(firstExpiry),
|
||||||
"re-registration expiry should be later than initial registration expiry")
|
"re-registration expiry should be later than initial registration expiry")
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// TestReregistrationZeroExpiryStaysNil tests that when a user-owned node
|
||||||
|
// re-registers with zero client expiry and node.expiry is disabled (0),
|
||||||
|
// the node's expiry stays nil rather than being set to a pointer to zero
|
||||||
|
// time. Regression test for the else branch introduced in commit 6337a3db
|
||||||
|
// which assigned `®Req.Expiry` (pointer to time.Time{}) instead of nil,
|
||||||
|
// causing the database row to hold `0001-01-01 00:00:00` instead of NULL.
|
||||||
|
func TestReregistrationZeroExpiryStaysNil(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()
|
||||||
|
|
||||||
|
// Initial registration with zero client expiry
|
||||||
|
regReq := tailcfg.RegisterRequest{
|
||||||
|
Auth: &tailcfg.RegisterResponseAuth{
|
||||||
|
AuthKey: pak.Key,
|
||||||
|
},
|
||||||
|
NodeKey: nodeKey.Public(),
|
||||||
|
Hostinfo: &tailcfg.Hostinfo{
|
||||||
|
Hostname: "reregister-zero-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.False(t, node.Expiry().Valid(),
|
||||||
|
"initial registration with zero expiry and no default should leave expiry nil")
|
||||||
|
|
||||||
|
// Re-register with a new node key but same machine key + user
|
||||||
|
nodeKey2 := key.NewNode()
|
||||||
|
regReq2 := tailcfg.RegisterRequest{
|
||||||
|
Auth: &tailcfg.RegisterResponseAuth{
|
||||||
|
AuthKey: pak.Key,
|
||||||
|
},
|
||||||
|
NodeKey: nodeKey2.Public(),
|
||||||
|
Hostinfo: &tailcfg.Hostinfo{
|
||||||
|
Hostname: "reregister-zero-expiry",
|
||||||
|
},
|
||||||
|
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.False(t, node2.Expiry().Valid(),
|
||||||
|
"re-registration with zero client expiry and no default should leave expiry nil, not pointer to zero time")
|
||||||
|
}
|
||||||
|
|||||||
@@ -2176,7 +2176,9 @@ func (s *State) HandleNodeFromPreAuthKey(
|
|||||||
// Tagged nodes keep their existing expiry (disabled).
|
// Tagged nodes keep their existing expiry (disabled).
|
||||||
// User-owned nodes update expiry from the client request,
|
// User-owned nodes update expiry from the client request,
|
||||||
// falling back to the configured default if the client
|
// falling back to the configured default if the client
|
||||||
// did not request a specific expiry.
|
// did not request a specific expiry. If neither is set,
|
||||||
|
// clear the expiry so the database holds NULL instead of
|
||||||
|
// a pointer to zero time.
|
||||||
if !node.IsTagged() {
|
if !node.IsTagged() {
|
||||||
if !regReq.Expiry.IsZero() {
|
if !regReq.Expiry.IsZero() {
|
||||||
node.Expiry = ®Req.Expiry
|
node.Expiry = ®Req.Expiry
|
||||||
@@ -2184,7 +2186,7 @@ func (s *State) HandleNodeFromPreAuthKey(
|
|||||||
exp := time.Now().Add(s.cfg.Node.Expiry)
|
exp := time.Now().Add(s.cfg.Node.Expiry)
|
||||||
node.Expiry = &exp
|
node.Expiry = &exp
|
||||||
} else {
|
} else {
|
||||||
node.Expiry = ®Req.Expiry
|
node.Expiry = nil
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
})
|
})
|
||||||
@@ -2271,6 +2273,15 @@ func (s *State) HandleNodeFromPreAuthKey(
|
|||||||
pakUser = *pak.User
|
pakUser = *pak.User
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Only pass the client-requested expiry when it is actually set.
|
||||||
|
// A pointer to a zero time.Time gets persisted as "0001-01-01 00:00:00"
|
||||||
|
// rather than NULL, which breaks downstream consumers that distinguish
|
||||||
|
// "no expiry" from "expires at year 1".
|
||||||
|
var reqExpiry *time.Time
|
||||||
|
if !regReq.Expiry.IsZero() {
|
||||||
|
reqExpiry = ®Req.Expiry
|
||||||
|
}
|
||||||
|
|
||||||
var err error
|
var err error
|
||||||
|
|
||||||
finalNode, err = s.createAndSaveNewNode(newNodeParams{
|
finalNode, err = s.createAndSaveNewNode(newNodeParams{
|
||||||
@@ -2281,7 +2292,7 @@ func (s *State) HandleNodeFromPreAuthKey(
|
|||||||
Hostname: hostname,
|
Hostname: hostname,
|
||||||
Hostinfo: validHostinfo,
|
Hostinfo: validHostinfo,
|
||||||
Endpoints: nil, // Endpoints not available in RegisterRequest
|
Endpoints: nil, // Endpoints not available in RegisterRequest
|
||||||
Expiry: ®Req.Expiry,
|
Expiry: reqExpiry,
|
||||||
RegisterMethod: util.RegisterMethodAuthKey,
|
RegisterMethod: util.RegisterMethodAuthKey,
|
||||||
PreAuthKey: pak,
|
PreAuthKey: pak,
|
||||||
ExistingNodeForNetinfo: cmp.Or(existingNodeAnyUser, types.NodeView{}),
|
ExistingNodeForNetinfo: cmp.Or(existingNodeAnyUser, types.NodeView{}),
|
||||||
|
|||||||
Reference in New Issue
Block a user