From 01322ea9e0bebedd214defbc79fde1ed48a61918 Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Sat, 17 Oct 2026 08:57:39 +0000 Subject: [PATCH] db, api/v1: revoke rather than delete an ephemeral pre-auth key in use The v1 list now hides revoked keys, so keys revoked through v2 also drop out of `headscale preauthkeys list`. --- hscontrol/api/v1/preauthkeys.go | 9 ++++-- hscontrol/apiv1_preauthkeys_test.go | 11 +++++++ hscontrol/db/preauth_keys.go | 19 ++++++++++-- hscontrol/db/preauth_keys_test.go | 47 +++++++++++++++++++++++++++++ hscontrol/db/users_test.go | 33 ++++++++++++++++++++ 5 files changed, 114 insertions(+), 5 deletions(-) diff --git a/hscontrol/api/v1/preauthkeys.go b/hscontrol/api/v1/preauthkeys.go index b28692f4a..489d09dd7 100644 --- a/hscontrol/api/v1/preauthkeys.go +++ b/hscontrol/api/v1/preauthkeys.go @@ -202,9 +202,14 @@ func registerPreAuthKeys(api huma.API, b Backend) { out := &listPreAuthKeysOutput{} - out.Body.PreAuthKeys = make([]PreAuthKey, len(preAuthKeys)) + out.Body.PreAuthKeys = make([]PreAuthKey, 0, len(preAuthKeys)) for i := range preAuthKeys { - out.Body.PreAuthKeys[i] = preAuthKeyToResponse(&preAuthKeys[i]) + if preAuthKeys[i].Revoked == nil { + out.Body.PreAuthKeys = append( + out.Body.PreAuthKeys, + preAuthKeyToResponse(&preAuthKeys[i]), + ) + } } return out, nil diff --git a/hscontrol/apiv1_preauthkeys_test.go b/hscontrol/apiv1_preauthkeys_test.go index 2df5f8e6d..4d50cbabc 100644 --- a/hscontrol/apiv1_preauthkeys_test.go +++ b/hscontrol/apiv1_preauthkeys_test.go @@ -142,6 +142,17 @@ func TestAPIV1DeletePreAuthKey(t *testing.T) { } func TestAPIV1ListPreAuthKeys(t *testing.T) { + t.Run("revoked keys are hidden", func(t *testing.T) { + h := newAPIV1Harness(t) + user := h.app.state.CreateUserForTest("revoked-key-user") + pak, err := h.app.state.CreatePreAuthKey(user.TypedID(), true, true, nil, nil) + require.NoError(t, err) + require.NoError(t, h.app.state.DB().RevokePreAuthKey(pak.ID)) + res := h.callHuma(http.MethodGet, "/api/v1/preauthkey", nil) + require.Equal(t, http.StatusOK, res.status) + assert.JSONEq(t, `{"preAuthKeys":[]}`, string(res.body)) + }) + t.Run("empty returns empty array", func(t *testing.T) { h := newAPIV1Harness(t) diff --git a/hscontrol/db/preauth_keys.go b/hscontrol/db/preauth_keys.go index 1389f774b..c3d975eae 100644 --- a/hscontrol/db/preauth_keys.go +++ b/hscontrol/db/preauth_keys.go @@ -339,10 +339,23 @@ func (hsdb *HSDatabase) GetPreAuthKeyByID(id uint64) (*types.PreAuthKey, error) } // DestroyPreAuthKey destroys a preauthkey. Returns error if the [types.PreAuthKey] -// does not exist. This also clears the auth_key_id on any nodes that reference -// this key. +// does not exist. Referenced ephemeral keys are revoked instead, because their +// nodes need the key row to retain ephemerality. Other keys are removed along +// with their nodes' auth_key_id references. func DestroyPreAuthKey(tx *gorm.DB, id uint64) error { return tx.Transaction(func(db *gorm.DB) error { + res := db.Model(&types.Credential{}). + Where("kind = ? AND id = ? AND ephemeral = ?", types.CredentialPreAuthKey, id, true). + Where("id IN (SELECT auth_key_id FROM nodes WHERE auth_key_id IS NOT NULL)"). + Update("revoked", gorm.Expr("COALESCE(revoked, ?)", time.Now())) + if res.Error != nil { + return res.Error + } + + if res.RowsAffected > 0 { + return nil + } + // First, clear the foreign key reference on any nodes using this key err := db.Model(&types.Node{}). Where("auth_key_id = ?", id). @@ -353,7 +366,7 @@ func DestroyPreAuthKey(tx *gorm.DB, id uint64) error { // Then delete the pre-auth key, on the same savepoint as the // node update so both roll back together. - res := db.Unscoped(). + res = db.Unscoped(). Delete(&types.Credential{}, "kind = ? AND id = ?", types.CredentialPreAuthKey, id) if res.Error != nil { return res.Error diff --git a/hscontrol/db/preauth_keys_test.go b/hscontrol/db/preauth_keys_test.go index bcae1e312..4381f46b3 100644 --- a/hscontrol/db/preauth_keys_test.go +++ b/hscontrol/db/preauth_keys_test.go @@ -114,6 +114,53 @@ func TestPreAuthKeyACLTags(t *testing.T) { } } +func TestDestroyPreAuthKeyKeepsReferencedEphemeralKey(t *testing.T) { + db, err := newSQLiteTestDB() + require.NoError(t, err) + + user := db.CreateUserForTest("ephemeral-key-user") + pak, err := db.CreatePreAuthKey(user.TypedID(), true, true, nil, nil) + require.NoError(t, err) + + node := &types.Node{ + Hostname: "ephemeral-key-node", UserID: &user.ID, AuthKeyID: &pak.ID, + } + require.NoError(t, db.DB.Create(node).Error) + require.NoError(t, db.DeletePreAuthKey(pak.ID)) + + nodes, err := db.ListNodes() + require.NoError(t, err) + require.Len(t, nodes, 1) + require.True(t, nodes[0].IsEphemeral(), "reload must preserve ephemerality") + + retained, err := db.GetPreAuthKeyByID(pak.ID) + require.NoError(t, err) + require.NotNil(t, retained.Revoked) + require.Error(t, retained.Validate()) + require.NoError(t, db.DeletePreAuthKey(pak.ID)) + again, err := db.GetPreAuthKeyByID(pak.ID) + require.NoError(t, err) + require.Equal(t, retained.Revoked, again.Revoked) + + cutoff := time.Now().Add(time.Hour) + count, err := db.DestroyRevokedPreAuthKeysBefore(cutoff) + require.NoError(t, err) + require.Zero(t, count) + require.NoError(t, db.DeleteNode(node)) + count, err = db.DestroyRevokedPreAuthKeysBefore(cutoff) + require.NoError(t, err) + require.Equal(t, 1, count) + + _, err = db.GetPreAuthKeyByID(pak.ID) + require.ErrorIs(t, err, gorm.ErrRecordNotFound) + + unused, err := db.CreatePreAuthKey(user.TypedID(), true, true, nil, nil) + require.NoError(t, err) + require.NoError(t, db.DeletePreAuthKey(unused.ID)) + _, err = db.GetPreAuthKeyByID(unused.ID) + require.ErrorIs(t, err, gorm.ErrRecordNotFound) +} + func TestCannotDeleteAssignedPreAuthKey(t *testing.T) { db, err := newSQLiteTestDB() require.NoError(t, err) diff --git a/hscontrol/db/users_test.go b/hscontrol/db/users_test.go index 49dc55d57..561fa4d37 100644 --- a/hscontrol/db/users_test.go +++ b/hscontrol/db/users_test.go @@ -11,6 +11,39 @@ import ( "gorm.io/gorm" ) +func TestDestroyUserKeepsTaggedNodeEphemeral(t *testing.T) { + db, err := newSQLiteTestDB() + require.NoError(t, err) + + user := db.CreateUserForTest("tagged-ephemeral-user") + pak, err := db.CreatePreAuthKey( + user.TypedID(), + true, + true, + nil, + []string{"tag:server"}, + ) + require.NoError(t, err) + + node := &types.Node{ + Hostname: "tagged-ephemeral-node", + Tags: []string{"tag:server"}, + AuthKeyID: &pak.ID, + } + require.NoError(t, db.DB.Create(node).Error) + require.NoError(t, db.DestroyUser(types.UserID(user.ID))) + nodes, err := db.ListNodes() + require.NoError(t, err) + require.Len(t, nodes, 1) + require.True(t, nodes[0].IsTagged()) + require.True(t, nodes[0].IsEphemeral()) + + retained, err := db.GetPreAuthKeyByID(pak.ID) + require.NoError(t, err) + require.NotNil(t, retained.Revoked) + require.Nil(t, retained.UserID) +} + func TestCreateAndDestroyUser(t *testing.T) { db, err := newSQLiteTestDB() require.NoError(t, err)